From 4fe028379239dd94dc434f32eda8cf58e8c62291 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Tue, 6 Oct 2026 14:20:44 +0300 Subject: [PATCH 1/6] fix(hooks): deliver the skill nudge to the model as additionalContext Under Claude Code, skill-nudge.sh printed {"systemMessage": ...}. The hooks reference defines systemMessage as a warning shown to the user, so the nudge never reached the model it is meant to steer. It now prints hookSpecificOutput.additionalContext with hookEventName PostToolUse, which Claude Code injects as a system reminder next to the tool result. Cursor still gets a plain line. hooks.json quotes ${CLAUDE_PLUGIN_ROOT} in each command, clearing the `claude plugin validate` warning that a path with a space splits the command. format-on-save.sh described $CLAUDE_FILE_PATH as set by Claude Code; no host sets it. It stays as an override for tests and manual runs; the stdin payload's tool_input.file_path (always absolute) is the real source. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 ++-- README.md | 2 +- docs/install.md | 2 +- docs/testing.md | 2 +- hooks/format-on-save.sh | 6 +++--- hooks/hooks.json | 6 +++--- hooks/skill-nudge.sh | 6 ++++-- scripts/hooks-test.sh | 6 +++--- 8 files changed, 18 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 00e0081..4b7995f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,8 +51,8 @@ Shared assets (skills, agents) are consumed by both hosts; host-specific manifes - **Cursor hooks**: `hooks/cursor-hooks.json`, object `{ "hooks": { "sessionStart": [...], "afterFileEdit": [...] } }`; the command runs from the plugin root (a **workspace-relative** path, **not** `${CLAUDE_PLUGIN_ROOT}`). Wires `session-start.sh` (`sessionStart`) and `format-on-save.sh` + `skill-nudge.sh` (`afterFileEdit`). - **Shared hook scripts** (host-agnostic, so either manifest can invoke them; all exit 0 always): - `hooks/session-start.sh`: detects `go.mod` / `*.go` and prints one Go-standards banner line naming the components. - - `hooks/format-on-save.sh`: after a `*.go` Write/Edit, runs `gofumpt -w` (or `gofmt -w -s`) on that single file; resolves the path from `$CLAUDE_FILE_PATH` or the stdin tool-payload JSON; silent no-op if no formatter is installed. - - `hooks/skill-nudge.sh`: after a `*.go` Write/Edit, names ONE matching go-coding skill for that edit, once per skill per session; a hook `systemMessage` under Claude Code, a plain line under Cursor. + - `hooks/format-on-save.sh`: after a `*.go` Write/Edit, runs `gofumpt -w` (or `gofmt -w -s`) on that single file; resolves the path from the stdin tool-payload JSON (`$CLAUDE_FILE_PATH` overrides it for tests); silent no-op if no formatter is installed. + - `hooks/skill-nudge.sh`: after a `*.go` Write/Edit, names ONE matching go-coding skill for that edit, once per skill per session; `hookSpecificOutput.additionalContext` under Claude Code (a `systemMessage` would reach only the user), a plain line under Cursor. - **References**: `references/golangci.v2.yml`, the shipped golangci-lint v2 reference config. Shared reference material lives here, **not** under `commands/`. - **MCP config** *(not present)*: `.mcp.json` only if the plugin later integrates an MCP server. - **Scripts**: `scripts/validate.sh` (wraps the stdlib-only `scripts/validate.py`), `scripts/hooks-test.sh`, `scripts/usage-report.py` (adoption measurement; see [docs/testing.md](docs/testing.md#measuring-adoption)). diff --git a/README.md b/README.md index 6405826..32e28fc 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ See [docs/install.md](docs/install.md) for marketplace, local-development, updat | Agent `go-reviewer` | Report-only, context-isolated Go reviewer for what linters miss. Returns severity-ranked findings and dispatches no sub-agents. Its tool grant excludes `Write` and `Edit` but includes `Bash` to run the linters, so report-only is a contract it keeps rather than a sandbox that enforces it, and `Skill`, so it loads `go-coding:go-coding` and the focused skills before it reviews. | | Session-start hook | Detects a Go workspace (`go.mod` or `*.go`) and prints one standards line; dual-host. | | Format-on-save hook | After each `Write`/`Edit` of a `*.go` file, runs `gofumpt -w` (or `gofmt -w -s`) on that file, on the host; dual-host. A silent no-op when no formatter is installed. | -| Skill-nudge hook | After each `Write`/`Edit` of a `*.go` file, names one matching go-coding skill, once per skill per session; dual-host. Arrives as a hook `systemMessage` under Claude Code and as a plain line under Cursor. | +| Skill-nudge hook | After each `Write`/`Edit` of a `*.go` file, names one matching go-coding skill, once per skill per session; dual-host. Arrives as hook `additionalContext` (model context) under Claude Code and as a plain line under Cursor. | | Lint config `references/golangci.v2.yml` | Reference golangci-lint v2 config (`modernize` plus the stack linters). | | Cursor rule `go-context.mdc` | `**/*.go`-scoped guidance mirroring the router for Cursor. | diff --git a/docs/install.md b/docs/install.md index 3f76c3c..8c60ef5 100644 --- a/docs/install.md +++ b/docs/install.md @@ -98,6 +98,6 @@ The plugin ships three host-agnostic hooks (Claude `hooks/hooks.json`, Cursor `h - **`session-start.sh`**: on session start, detects a Go workspace (`go.mod`/`*.go`) and prints one standards line. - **`format-on-save.sh`**: after each edit of a `*.go` file (Claude `PostToolUse` on `Write`/`Edit`; Cursor `afterFileEdit`), runs **`gofumpt -w`** on that file, or **`gofmt -w -s`** when `gofumpt` is not installed. It is **host-only** (no container round-trip), **edits the file in place**, and is a **silent no-op** when no Go formatter is on `PATH`, so it needs Go (which ships `gofmt`) or `gofumpt` on the host. It never blocks an edit. This is per-file formatting only; run `golangci-lint` and your tests via CI/`make` for full-tree checks. -- **`skill-nudge.sh`**: after each edit of a `*.go` file (same trigger as `format-on-save.sh`), names one matching go-coding skill for that edit (a `_test.go` file → `go-testing`; goroutine/channel/`sync`/`atomic`/`errgroup` content → `go-concurrency`; `fmt.Errorf`/`errors.*` → `go-errors`), once per skill per session. Delivered as a hook `systemMessage` under Claude Code (reaches the model's context on exit 0) or a plain line under Cursor. It never blocks an edit. +- **`skill-nudge.sh`**: after each edit of a `*.go` file (same trigger as `format-on-save.sh`), names one matching go-coding skill for that edit (a `_test.go` file → `go-testing`; goroutine/channel/`sync`/`atomic`/`errgroup` content → `go-concurrency`; `fmt.Errorf`/`errors.*` → `go-errors`), once per skill per session. Delivered as `hookSpecificOutput.additionalContext` under Claude Code, which reaches the model's context as a system reminder (a top-level `systemMessage` is shown only to the user), or a plain line under Cursor. It never blocks an edit. > The Cursor wiring targets the `afterFileEdit` event; if your Cursor version exposes a different post-edit event or payload shape, adjust `hooks/cursor-hooks.json` and the path extraction in `format-on-save.sh` to match. diff --git a/docs/testing.md b/docs/testing.md index 16c29a1..c7f424a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -38,7 +38,7 @@ Load your working copy with `--plugin-dir` (see [install.md](install.md)), then - **`go-coding` router**: ask for a Go review or idiom help; it should route to the enforcing tool and the focused skill. - **Standards skills**: a topic prompt should engage the matching skill (for example error wrapping → `go-errors`, a flaky time-based test → `go-testing`/`go-concurrency`, linter setup → `go-lint-setup`). - **Format-on-save hook**: save a deliberately mis-formatted `*.go` file; `format-on-save.sh` should reformat that one file in place (`gofumpt -w`, or `gofmt -w -s` when `gofumpt` is absent) and say nothing when neither is installed. -- **Skill-nudge hook**: edit a `_test.go` file; the nudge should name `go-coding:go-testing` (as a `systemMessage` under Claude Code, a plain line under Cursor), and the model should **act** on it by loading the skill; the line appearing in the transcript is not enough. A second edit to a `_test.go` file in the same session should be silent (once per skill per session), and so should an edit that does not itself touch the topic: a doc-comment fix in a file that defines a sentinel elsewhere must not claim the edit touches an error path. +- **Skill-nudge hook**: edit a `_test.go` file; the nudge should name `go-coding:go-testing` (as `additionalContext` under Claude Code, which adds no visible transcript line; confirm delivery with `claude --debug`; a plain line under Cursor), and the model should **act** on it by loading the skill; delivery alone is not enough. A second edit to a `_test.go` file in the same session should be silent (once per skill per session), and so should an edit that does not itself touch the topic: a doc-comment fix in a file that defines a sentinel elsewhere must not claim the edit touches an error path. - **`go-reviewer` agent**: ask for a Go code review; it loads `go-coding:go-coding` and the focused skills for the diff with the Skill tool, returns severity-ranked findings that cite them, and does not spawn sub-agents. - **`/go-lint-setup`**: run it in a Go repo without a golangci-lint config and confirm it writes the reference v2 config; run it in a repo that already has one and confirm it does not overwrite it unprompted. - **Cursor rule**: in Cursor, open a `.go` file and confirm `go-context.mdc` attaches. diff --git a/hooks/format-on-save.sh b/hooks/format-on-save.sh index 53a389d..3455a1e 100755 --- a/hooks/format-on-save.sh +++ b/hooks/format-on-save.sh @@ -8,14 +8,14 @@ # recommends installing one), and ALWAYS exits 0 so it can never block an edit. # # File-path resolution, in order: -# 1. $CLAUDE_FILE_PATH — set by Claude Code for Write/Edit hooks (fast path). -# 2. tool payload JSON on stdin — Claude (`tool_input.file_path`) or Cursor +# 1. $CLAUDE_FILE_PATH — explicit override for tests and manual runs; no host sets it. +# 2. tool payload JSON on stdin — Claude (`tool_input.file_path`, always absolute) or Cursor # `afterFileEdit` (`file_path`). Extracted without a jq/python dependency. set -u f="${CLAUDE_FILE_PATH:-}" -# Fall back to the JSON the host pipes in on stdin (Cursor; newer Claude payloads). +# Otherwise read the JSON payload both hosts pipe in on stdin. # Guard on a non-tty stdin so a manual run without a pipe doesn't block on `cat`. if [ -z "$f" ] && [ ! -t 0 ]; then payload="$(cat)" diff --git a/hooks/hooks.json b/hooks/hooks.json index e0e0ff2..8bb9f85 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -5,7 +5,7 @@ "hooks": [ { "type": "command", - "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/session-start.sh" + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/session-start.sh\"" } ] } @@ -16,12 +16,12 @@ "hooks": [ { "type": "command", - "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/format-on-save.sh", + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/format-on-save.sh\"", "timeout": 20 }, { "type": "command", - "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/skill-nudge.sh", + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/skill-nudge.sh\"", "timeout": 5 } ] diff --git a/hooks/skill-nudge.sh b/hooks/skill-nudge.sh index 7c01112..6e716e1 100755 --- a/hooks/skill-nudge.sh +++ b/hooks/skill-nudge.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # PostToolUse / afterFileEdit hook: after a Go file is edited, name ONE go-coding skill the edit -# calls for — printed as a hook systemMessage (Claude Code) or a plain line (Cursor). Deterministic +# calls for — printed as hookSpecificOutput.additionalContext (Claude Code) or a plain line (Cursor). Deterministic # trigger for three focused skills a usage analysis showed load far less often than the router: # go-testing, go-concurrency, go-errors. Always exits 0; never blocks an edit. # @@ -46,7 +46,9 @@ marker="${TMPDIR:-/tmp}/go-coding-nudge.${sid}.${skill}" # variable/external text instead, escape it first — printf does no JSON escaping of its own. msg="› go-coding: ${what} touches ${topic} — load go-coding:${skill} before continuing." if [ -n "${CLAUDE_PLUGIN_ROOT:-}" ] || printf '%s' "$payload" | grep -q '"hook_event_name"'; then - printf '{"systemMessage":"%s"}\n' "$msg" # Claude Code: systemMessage reaches the model's context on exit 0 + # Claude Code: additionalContext is the channel that reaches the model; a top-level + # systemMessage is only shown to the user (https://code.claude.com/docs/en/hooks). + printf '{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"%s"}}\n' "$msg" else printf '%s\n' "$msg" # Cursor afterFileEdit: plain line fi diff --git a/scripts/hooks-test.sh b/scripts/hooks-test.sh index 3193e64..3666d49 100755 --- a/scripts/hooks-test.sh +++ b/scripts/hooks-test.sh @@ -85,7 +85,7 @@ rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$sedit"* printf 'package a\nimport "fmt"\nvar e = fmt.Errorf("x")\n' > "$t/e2.go" sjson="$s-json"; rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$sjson."* out="$(CLAUDE_PLUGIN_ROOT=/x hook j "$(p_edit "$sjson" "$t/e2.go" '' 'fmt.Errorf')")" -chk "CLAUDE_PLUGIN_ROOT delivers a systemMessage" "$out" '{"systemMessage":' +chk "CLAUDE_PLUGIN_ROOT delivers additionalContext" "$out" '{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":' rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$sjson."* # hook_event_name alone must be enough — CLAUDE_PLUGIN_ROOT is not set for hooks in every context, @@ -93,8 +93,8 @@ rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$sjson."* shook="$s-hookevent"; rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$shook."* out="$(printf '%s' "$(p_edit "$shook" "$t/e2.go" '' 'fmt.Errorf')" | env -u CLAUDE_PLUGIN_ROOT bash "$here/hooks/skill-nudge.sh")"; st=$? [ "$st" -eq 0 ] || { echo "FAIL hook_event_name without CLAUDE_PLUGIN_ROOT: exited $st"; fails=$((fails+1)); } -chk "hook_event_name alone delivers a systemMessage" "$out" '{"systemMessage":' -chk "that systemMessage names the skill" "$out" "go-coding:go-errors" +chk "hook_event_name alone delivers additionalContext" "$out" '"additionalContext":' +chk "that additionalContext names the skill" "$out" "go-coding:go-errors" rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$shook."* scursor="$s-cursor"; rm -f "${TMPDIR:-/tmp}/go-coding-nudge.$scursor."* From c954e955742d7834fe330f1cf57cbe7c42ac5de9 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Tue, 6 Oct 2026 14:20:56 +0300 Subject: [PATCH 2/6] chore: check the go-lint-setup scaffold config against the reference Both config copies said "keep in sync" and the advice-equals-tooling error already spoke of "both config copies", but nothing compared them. The linter-references check now fails when the scaffold block in skills/go-lint-setup/SKILL.md and references/golangci.v2.yml differ in linters.default, enabled linters or enabled formatters. A new --selftest case proves the check fires on drift and stays quiet on a clean tree. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- docs/testing.md | 2 +- scripts/validate.py | 37 +++++++++++++++++++++++++++++++++---- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4b7995f..6fbf67c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,7 @@ This plugin encodes **Go (golang) coding standards**. Guidance must be grounded When a recommendation derives from one of the above, attribute it explicitly and distinguish cited rules from inference. -**Advice == tooling.** Every linter taught in a component must be enabled in `references/golangci.v2.yml`. Analyzers a skill teaches as opt-in (`shadow` in `go-idioms`) are the deliberate exception: each is taught with the config line that switches it on, and is not added to the reference config at a refresh. The `go-idioms` Fixer column must match `go tool fix help` on the floor-minor toolchain. The Google readability tie-break sentence must read identically in the `go-coding` router, `rules/go-context.mdc`, and `go-reviewer`. `scripts/validate.py` enforces all three. +**Advice == tooling.** Every linter taught in a component must be enabled in `references/golangci.v2.yml`, and the `/go-lint-setup` scaffold block must enable the same set. Analyzers a skill teaches as opt-in (`shadow` in `go-idioms`) are the deliberate exception: each is taught with the config line that switches it on, and is not added to the reference config at a refresh. The `go-idioms` Fixer column must match `go tool fix help` on the floor-minor toolchain. The Google readability tie-break sentence must read identically in the `go-coding` router, `rules/go-context.mdc`, and `go-reviewer`. `scripts/validate.py` enforces all three. **Refreshing the skills against current Go practice** (e.g. "check current Go best practices and update the skills"): follow the **source registry and procedure** in [docs/authoring.md](docs/authoring.md#refreshing-the-standards-baseline-source-registry). It lists every source to re-read, in order, plus the version-gating rules. Re-read them; never refresh from memory. Two recurring traps: the *released* Go version is not whatever `go.dev/doc/go1.NN` renders (check the release history), and the stdlib APIs a skill is missing are usually the ones that landed *after* it was written. diff --git a/docs/testing.md b/docs/testing.md index c7f424a..c022332 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -26,7 +26,7 @@ Structural checks: Three invariants specific to this plugin: -- **Advice equals tooling**: every linter a component teaches (through `--enable-only=…` or "the `` linter") must be enabled in `references/golangci.v2.yml`, so no skill tells an agent to rely on a linter the reference config does not ship. +- **Advice equals tooling**: every linter a component teaches (through `--enable-only=…` or "the `` linter") must be enabled in `references/golangci.v2.yml`, so no skill tells an agent to rely on a linter the reference config does not ship; and the `/go-lint-setup` scaffold block must enable the same default, linters and formatters as that file, so a scaffolded repo lints with the config the components are checked against. - **Fixer column**: when a Go toolchain at the floor minor (`GO_FLOOR_MINOR`) is on `PATH`, the `go-idioms` **Fixer** column is verified against `go tool fix help`: plain names must be registered, † names must not be, so a renamed or retired fixer fails the build instead of shipping as advice. Locally it soft-skips without that toolchain; CI installs it. - **Tie-break sentence**: the Google readability tie-break sentence, which ranks clarity first and consistency last (`TIE_BREAK_SENTENCE` in the script holds the exact text), must appear verbatim in each of the `go-coding` router, `rules/go-context.mdc`, and `go-reviewer`. diff --git a/scripts/validate.py b/scripts/validate.py index 26a6ecb..a3db563 100644 --- a/scripts/validate.py +++ b/scripts/validate.py @@ -14,7 +14,8 @@ Claude Code silently ignores so the agent inherits *all* tools — flagged as an error); * advice == tooling: every linter a component *teaches* (via ``--enable-only=...`` or "the `` linter") must be enabled in ``references/golangci.v2.yml`` — a skill must - not tell agents to rely on a linter no config copy ships; + not tell agents to rely on a linter no config copy ships — and the go-lint-setup + scaffold block must enable the same set as that file; * when a Go toolchain at the floor minor (``GO_FLOOR_MINOR``) is on PATH, the go-idioms **Fixer** column is verified against ``go tool fix help``: plain names must be registered, † names must not be. Soft-skips locally without Go; CI installs the floor toolchain. @@ -199,6 +200,15 @@ def validate_rules(): err(f"{rel}: frontmatter missing 'description'") +def _config_entries(text: str) -> tuple: + """(linters.default, enabled linters, enabled formatters) of a golangci-lint v2 config, + comments ignored. Order-insensitive: the copies must enable the same set, not list it alike.""" + linters, _, formatters = text.partition("formatters:") + default = re.search(r"^\s+default:\s*([a-z]+)", linters, re.MULTILINE) + pick = lambda t: sorted(re.findall(r"^\s+-\s+([a-z0-9-]+)", t, re.MULTILINE)) + return (default.group(1) if default else None), pick(linters), pick(formatters) + + def validate_linter_references(): """Advice == tooling: every linter a component *teaches* must be shipped by the reference lint config. A linter counts as taught when a component names it as the enforcing tool — @@ -210,6 +220,18 @@ def validate_linter_references(): if not ref.is_file(): return # Only the `linters:` section — formatters are a different contract. + # The /go-lint-setup scaffold block is the second config copy: a repo scaffolded from it + # must lint with exactly what the reference config enables. + scaffold = ROOT / "skills" / "go-lint-setup" / "SKILL.md" + if scaffold.is_file(): + block = re.search(r"^```yaml\n(.*?)^```", scaffold.read_text(), re.DOTALL | re.MULTILINE) + if not block: + err(f"{scaffold.relative_to(ROOT)}: no ```yaml scaffold block to compare with " + f"references/golangci.v2.yml") + elif _config_entries(block.group(1)) != _config_entries(ref.read_text()): + err(f"{scaffold.relative_to(ROOT)}: the scaffold block {_config_entries(block.group(1))} " + f"differs from references/golangci.v2.yml {_config_entries(ref.read_text())} — " + f"keep the two config copies identical") linters_section = ref.read_text().split("formatters:")[0] allowed = set(re.findall(r"^\s+-\s+([a-z0-9-]+)", linters_section, re.MULTILINE)) allowed |= STANDARD_LINTERS @@ -699,12 +721,18 @@ def _selftest_tree(root: Path, *, break_it=None): (root / "hooks" / "hooks.json").write_text(json.dumps(SELFTEST_HOOKS_CLAUDE)) cursor = {"hooks": {"afterFileEdit": []}} if break_it == "hook_parity" else SELFTEST_HOOKS_CURSOR (root / "hooks" / "cursor-hooks.json").write_text(json.dumps(cursor)) - (root / "references" / "golangci.v2.yml").write_text("linters:\n enable:\n - revive\n") + ref_config = "linters:\n enable:\n - revive\n" + (root / "references" / "golangci.v2.yml").write_text(ref_config) + (root / "skills" / "go-lint-setup").mkdir() + scaffold = ref_config + (" - errorlint\n" if break_it == "scaffold_drift" else "") + (root / "skills" / "go-lint-setup" / "SKILL.md").write_text( + f"---\nname: go-lint-setup\ndescription: scaffolds\n---\n\n```yaml\n{scaffold}```\n") inventory = "" if break_it == "doc_inventory" else "go-thing " for doc in ("README.md", "AGENTS.md"): - (root / doc).write_text(f"# Doc\n\n{inventory}go-coding go-reviewer go-checker a\n") + (root / doc).write_text(f"# Doc\n\n{inventory}go-coding go-lint-setup go-reviewer go-checker a\n") for doc in ("testing.md", "install.md"): - (root / "docs" / doc).write_text(f"# Doc\n\n{inventory}go-coding go-reviewer go-checker a\n") + (root / "docs" / doc).write_text( + f"# Doc\n\n{inventory}go-coding go-lint-setup go-reviewer go-checker a\n") SELFTEST_CASES = ( @@ -717,6 +745,7 @@ def _selftest_tree(root: Path, *, break_it=None): ("agent declares allowed-tools", "agent_tools", (lambda: validate_md_components("agents", require_name=True, is_agent=True),)), ("taught linter not in the reference config", "taught_linter", (validate_linter_references,)), + ("scaffold config drifts from the reference", "scaffold_drift", (validate_linter_references,)), ("Google tie-break sentence drifts between files", "tie_break", (validate_tie_break_parity,)), ) From edd63e5ef9da4c6a0415d8877dad32de969b0983 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Tue, 6 Oct 2026 14:20:56 +0300 Subject: [PATCH 3/6] fix(skills): add go-testing's can-fail rule and drop repeated guidance go-testing's description (and the router's Route-then-load table) send can-fail control work to go-testing, but its body never had the rule. It now does. Each of these was stated two or three times and now appears once: - go-idioms: the Go 1.27 go fix changes (table intro, a loose paragraph, and the 1.27 section); strings.CutLast/bytes.CutLast move into the 1.27 table, and the self-referential-constraints note moves to the no-fixer list. - go-coding: the gopls-lsp advice, a paragraph restating Route-then-load, and the orchestrator instruction; drops a reference to "the project research" the consuming model cannot see. - go-errors: the %v-instead-of-%w warning folds into the %w rule. - go-testing: -race, the cryptotest/t.Parallel warning; the compressed-bytes note moves under "Compare stable results". - go-lint-setup: the migrate details and the version-pinning advice; the pointer for what each linter does now names the file that explains it. go-lint-setup pre-approves Bash(golangci-lint *) rather than all of Bash: allowed-tools grants are not gated by workspace trust. Co-Authored-By: Claude Opus 5.5 --- skills/go-coding/SKILL.md | 9 ++------ skills/go-errors/SKILL.md | 9 ++++---- skills/go-idioms/SKILL.md | 41 +++++++++++++---------------------- skills/go-lint-setup/SKILL.md | 21 +++++++++--------- skills/go-testing/SKILL.md | 22 +++++++++++-------- 5 files changed, 45 insertions(+), 57 deletions(-) diff --git a/skills/go-coding/SKILL.md b/skills/go-coding/SKILL.md index 1c50143..02cdccb 100644 --- a/skills/go-coding/SKILL.md +++ b/skills/go-coding/SKILL.md @@ -6,7 +6,7 @@ description: Go coding-standards router — Go 1.26.4+ (1.27 supported, its addi # go-coding — Go standards router Route the Go task to the right standard and tool — this skill is a router, not an encyclopedia. -Two principles from the project research drive it: +Two principles drive it: - **Deterministic beats prose.** Whatever a formatter or linter enforces, run the tool — don't reason it out by hand. The plugin's value is judgment the model lacks, not re-deriving tooling. @@ -24,10 +24,6 @@ Two principles from the project research drive it: | Concurrency (goroutine leaks, ctx lifecycle, atomics) | `go test -race ./...`, `go vet ./...` | `go-concurrency` | | Testing (table-driven, `t.Parallel`, `t.Context`, `B.Loop`, `testing/synctest`) | `go test -race ./...`; use `testing/synctest` for time/concurrency tests | `go-testing` | | Layout, naming & API surface (`internal/`, imports, initialisms, receiver type, in-band errors, struct literals, doc comments) | `golangci-lint run --enable-only=revive` (`var-naming`, `receiver-naming`, `exported`, `blank-imports`, `dot-imports`), `go vet` (`composites`), `gofmt` for doc-comment layout; the rest is judgment | `go-layout` | -| Code intelligence (defs/refs/diagnostics/rename/vulncheck) | install the **`gopls-lsp`** plugin | — | - -Open the focused `go-*` skill for the topic — it carries the cited rules and the judgment; run the -tool in the middle column to enforce them. Don't invent rules: each skill cites its sources. ## Route, then load @@ -90,8 +86,7 @@ Dispatch the `go-reviewer` agent — a report-only, context-isolated reviewer th applies the review-heuristics catalog and returns severity-ranked findings on a diff or file. If a workflow already owns the reviewer seat, that reviewer loads the focused skills itself instead — -one review seat per diff. Orchestrators: put the "Route, then load" table into every implementer and -reviewer brief. +one review seat per diff. `/go-lint-setup` scaffolds the reference golangci-lint v2 config into a repo. diff --git a/skills/go-errors/SKILL.md b/skills/go-errors/SKILL.md index 65c3d87..bad67e4 100644 --- a/skills/go-errors/SKILL.md +++ b/skills/go-errors/SKILL.md @@ -12,11 +12,10 @@ Deterministic backstop: `golangci-lint run --enable-only=errorlint`, plus `errch - **Wrap with `%w` when the caller may need to inspect the cause** (Go 1.13): `return fmt.Errorf("read config %s: %w", path, err)`. Use `%v` *only* to deliberately sever the - chain (e.g. to avoid leaking an internal error type across an API boundary) — and say so. - Put `%w` **last** so the message reads outside-in; a leading `%w` is right only when the sentinel - *is* the sentence: `fmt.Errorf("%w: %s", ErrNotFound, key)`. -- **The `%v`-where-`%w` trap:** formatting a cause with `%v` discards the chain, so downstream - `errors.Is`/`errors.As` silently fail. `errorlint` flags it. + chain (e.g. to avoid leaking an internal error type across an API boundary) — and say so; + anywhere else `%v` discards the chain and downstream `errors.Is`/`errors.As` silently fail + (`errorlint` flags it). Put `%w` **last** so the message reads outside-in; a leading `%w` is right + only when the sentinel *is* the sentence: `fmt.Errorf("%w: %s", ErrNotFound, key)`. - **Inspect with `errors.Is` (sentinel) / `errors.AsType[E]` (typed)** — never `err == ErrX` or a type assertion once any layer wraps, or the result is *sentinel breakage* (the comparison silently stops matching). `if perr, ok := errors.AsType[*fs.PathError](err); ok { … }` (Go 1.26): the diff --git a/skills/go-idioms/SKILL.md b/skills/go-idioms/SKILL.md index d3be8db..dbd9515 100644 --- a/skills/go-idioms/SKILL.md +++ b/skills/go-idioms/SKILL.md @@ -16,22 +16,18 @@ go fix ./... # apply; - runs one, golangci-lint run --enable-only=modernize --fix # the x/tools modernize suite — includes the † fixers below ``` -Both draw on the same `golang.org/x/tools` engine as gopls, but golangci-lint pins its own (usually -newer) snapshot of it — that gap is what the **†** marker below tracks. This skill explains *why* -and catches what review notices before the tool runs. The **baseline is Go 1.26.4+** (Go 1.27 is -supported too; its additions are flagged as hints in **Newer in Go 1.27** below), so every row -below applies as written — the `Since` column is provenance: it explains why older code looks -different, and what an older module would have to bump to before adopting the idiom. +Both draw on the same `golang.org/x/tools` engine as gopls; golangci-lint pins its own, usually +newer, snapshot, which is the gap the **†** marker below tracks. On the **Go 1.26.4+** baseline every +row applies as written; the `Since` column is provenance (why older code looks different, and what +an older module must bump to before adopting the idiom). ## Prefer → over (since) The **Fixer** column names the analyzer that owns each rewrite. Plain = registered in the Go 1.26.4 toolchain's `go fix` (ground truth: `go tool fix help`; per-fixer docs: `go tool fix help `). **†** = only in the newer `x/tools` suite so far — golangci-lint's `modernize` and gopls run it, the -1.26.4 toolchain's `go fix` does not. `—` = no fixer exists: review has to catch it. Two † rows -below (`atomictypes`, `slicesbackward`) graduate into the stock `go fix` on 1.27, which also renames -`waitgroup` → `waitgroupgo` and drops `fmtappendf` () — see -**Newer in Go 1.27** below rather than reading that as a change to this table. +1.26.4 toolchain's `go fix` does not. `—` = no fixer exists: review has to catch it. This table is +the 1.26 floor; the 1.27 toolchain's fixer changes are in **Newer in Go 1.27**. | Prefer | Over | Since | Fixer | |---|---|---|---| @@ -58,14 +54,12 @@ below (`atomictypes`, `slicesbackward`) graduate into the stock `go fix` on 1.27 | `for b.Loop()` | `for i := 0; i < b.N; i++` → `go-testing` | 1.24 | `bloop` † | | typed `atomic.Int64` | bare-int `atomic.Add*` → `go-concurrency` | 1.19 | `atomictypes` † | -Idioms are a moving target — let the tool (pinned to the repo's toolchain) be the source of -truth so advice never drifts from the user's `go fix`. Go 1.26 also lifts the ban on a generic type -referencing itself in its own type-parameter list (e.g. `type Adder[A Adder[A]] interface{ Add(A) A }`), -so self-referential constraints no longer need a workaround — but that's a hand-written pattern, not -something a modernizer rewrites. - ## Modern, but no fixer automates it +- **Self-referential constraints** (1.26): a generic type may reference itself in its own + type-parameter list (`type Adder[A Adder[A]] interface{ Add(A) A }`), so the old workaround is no + longer needed. + - **`os.OpenRoot(dir)` → `*os.Root`** (1.24) for anything that opens a caller-supplied path: its methods cannot escape the directory, including via symlink. Replaces `filepath.Join` plus hand-written `..`/prefix checks — the traversal-bug pattern those checks keep getting wrong. @@ -87,18 +81,12 @@ something a modernizer rewrites. Earlier in a case a `break` still does work — it leaves the `switch` from that point — and a labelled `break` leaves the enclosing loop instead; neither of those is dead text. -*Go 1.27 (released 2026-08-19, ) graduates several † fixers into the toolchain's -`go fix` (`atomictypes`, `slicesbackward`, plus new `embedlit` and `unsafefuncs`), renames `waitgroup` -→ `waitgroupgo`, and drops `fmtappendf`; it also lands `encoding/json/v2` + `encoding/json/jsontext` -(v1 is reimplemented on v2, opt out with `GOEXPERIMENT=nojsonv2`) and `strings.CutLast`/`bytes.CutLast`. -Source: . See **Newer in Go 1.27** below for the hints these enable — none -of it is required on the 1.26 floor.* - ## Newer in Go 1.27 (hints, not requirements) -Go 1.27 is additive over 1.26 — every 1.26 rule above still applies unchanged, and nothing here is -required while a module's `go` directive stays at 1.26. Once a repo's toolchain (and `go` directive) -moves to 1.27, these are worth reaching for. +Go 1.27 (released 2026-08-19, ) is additive over 1.26: every rule above still +applies, and nothing here is required while a module's `go` directive stays at 1.26. Once the +toolchain and `go` directive move to 1.27, these are worth reaching for. Its `go fix` also renames +`waitgroup` → `waitgroupgo` and drops `fmtappendf`. | Idiom (available from 1.27) | Supersedes / complements | Fixer / linter | Since | Source | |---|---|---|---|---| @@ -107,6 +95,7 @@ moves to 1.27, these are worth reaching for. | `atomictypes` — graduates into the stock `go fix` (previously † golangci-lint-only, row above) | raw `sync/atomic` functions | `atomictypes` | 1.27 | [go.dev/doc/go1.27](https://go.dev/doc/go1.27) | | `slicesbackward` — graduates into the stock `go fix` (previously † golangci-lint-only, row above) | `for i := len(s)-1; i >= 0; i--` | `slicesbackward` | 1.27 | [go.dev/doc/go1.27](https://go.dev/doc/go1.27) | | `embedlit` — initialise a field promoted from an embedded struct directly in the parent literal: `T{U: U{x: 1}}` → `T{x: 1}` | the nested literal an embedded struct used to require | `embedlit` | 1.27 | [modernize](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize) | +| `strings.CutLast` / `bytes.CutLast` | `LastIndex` + manual slicing | — | 1.27 | [go.dev/doc/go1.27](https://go.dev/doc/go1.27) | | `unsafefuncs` — `unsafe.Pointer(uintptr(ptr) + uintptr(n))` → `unsafe.Add(ptr, n)` | hand-rolled unsafe pointer arithmetic (`unsafe.Add` itself is 1.17; the fixer is new) | `unsafefuncs` | 1.27 | [modernize](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize) | - **`go test` runs the `stdversion` vet check by default from 1.27.** The check itself is not new — diff --git a/skills/go-lint-setup/SKILL.md b/skills/go-lint-setup/SKILL.md index 1d4d9cd..db1cb19 100644 --- a/skills/go-lint-setup/SKILL.md +++ b/skills/go-lint-setup/SKILL.md @@ -2,7 +2,11 @@ name: go-lint-setup description: Scaffold, adopt, or debug the golangci-lint v2 config in a Go repo. This skill should be used when the user runs `/go-lint-setup`, asks to "set up", "scaffold", "add" or "bootstrap" golangci-lint or a `.golangci.yml`, or asks why golangci-lint v2 rejects a config, how to migrate a v1 config, which linters the default set enables, how to adopt modernize/errorlint in an existing repo, what `golangci-lint fmt` does, how to write a `linters.exclusions` rule, or how to suppress a finding with `//nolint`. Writes the reference v2 config (modernize + stack linters); never overwrites an existing one unprompted. Go only. argument-hint: optional target path (defaults to .golangci.yml) -allowed-tools: Read, Write, Glob, Bash +allowed-tools: + - Read + - Write + - Glob + - Bash(golangci-lint *) --- # go-lint-setup — scaffold, adopt, or debug golangci-lint v2 @@ -23,13 +27,11 @@ Steps: `.golangci.json`. If one exists, do **not** overwrite it: show how it differs from the reference and ask before changing anything. If it's a **v1** config (no `version` key and/or an `enable-all`/top-level `linters:` list), warn that v1 will not parse under golangci-lint v2 and - offer to migrate — the supported path is `golangci-lint migrate` (in-place, keeps a `.bck` backup, - drops comments), not a hand-port. + offer `golangci-lint migrate` (see *Adopting or debugging an existing config*), not a hand-port. 2. **Write** the config below to `.golangci.yml` (or the path given in `$ARGUMENTS`). 3. **Report how to run it:** `golangci-lint run`, and `golangci-lint run --fix` for the auto-fixable - findings (`modernize` + the formatters). Suggest pinning an exact `golangci-lint` version in CI in - one place (the action's `version:` input) with an automated bump PR — see *Adopting or debugging - an existing config* below; don't invent a version number here, point at the releases page. + findings (`modernize` + the formatters). Suggest pinning an exact version in CI, as + *Discipline once adopted* below describes; name no version number. Config to write (mirrors `references/golangci.v2.yml` — keep the two in sync): @@ -57,8 +59,7 @@ formatters: - goimports ``` -For what each linter does and why, see *Adopting or debugging an existing config* below, or the -inline comments in `references/golangci.v2.yml`. +For what each linter does, see the inline comments in `references/golangci.v2.yml`. ## Adopting or debugging an existing config @@ -72,8 +73,8 @@ golangci-lint **v2** (Mar 2025) changed the config schema from v1 — **a v1 con - **Exclusions moved under `linters`**: v1's `issues.exclude-rules` → `linters.exclusions.rules`, and `issues.exclude-dirs`/`exclude-files` → `linters.exclusions.paths`. `linters-settings` split into `linters.settings` + `formatters.settings`. A config that still uses the old key names — - `issues:`, `linters-settings:`, `enable-all` — is v1 and needs `golangci-lint migrate` (Step 1 - above), not a hand-port; `migrate` rewrites in place, keeps a `.golangci.bck.yml` backup, and + `issues:`, `linters-settings:`, `enable-all` — is v1 and needs `golangci-lint migrate`, not a + hand-port; `migrate` rewrites in place, keeps a `.golangci.bck.yml` backup, and takes `--format {yml,yaml,toml,json}` — it drops comments and unknown/deprecated keys, so re-add comments and diff the result. diff --git a/skills/go-testing/SKILL.md b/skills/go-testing/SKILL.md index 689e8e1..b6eaf70 100644 --- a/skills/go-testing/SKILL.md +++ b/skills/go-testing/SKILL.md @@ -5,10 +5,15 @@ description: Idiomatic Go testing. This skill should be used when the user write # go-testing — Go testing -Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, `go test -fuzz`. +Deterministic backstop: `go test -race ./...` (non-negotiable for code touching goroutines; wire it +into CI), `go test -bench`, `go test -fuzz`. ## Rules +- **Prove each guard can fail.** A test for a guard (a validation, a bounds check, a refusal) must + turn red when the guard is deleted or inverted; a test that still passes was never testing it. + "Temporarily broke it by hand and it failed" is not evidence: commit the case only the guard + rejects, so the refactor that later drops the guard fails the build. - **Table-driven tests:** a named-case slice + `t.Run(tc.name, func(t *testing.T){ … })`. Since Go 1.22 the `tc := tc` copy is unnecessary — drop it (`modernize`/`copyloopvar` flag it). - **`t.Parallel()`** on independent tests to cut wall-clock; watch for shared mutable state and @@ -33,7 +38,6 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, key/value metadata into `go test -json` output. - **Benchmarks: `for b.Loop() { … }`** (Go 1.24) — it handles timer reset and run scaling; replaces `for i := 0; i < b.N; i++` plus manual `b.ResetTimer()`. -- **`-race` is non-negotiable** for any code touching goroutines; wire it into CI. - **Goroutine-leak detection:** `go.uber.org/goleak` — `goleak.VerifyTestMain(m)` or per-test `defer goleak.VerifyNone(t)`. - **`testing/synctest` (stable since Go 1.25) is the default for time/concurrency tests** — timeouts, @@ -51,14 +55,11 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, bytes. **Golden files** (an `-update` flag writing `testdata/*.golden`) for large structured output. A golden pins *shape*, not behaviour — when it records something another system executes (SQL, wire requests, rendered configs), pair it with at least one test that executes the artefact for - real; a snapshot can be stable and wrong. (Go 1.27) Never assert on compressed bytes verbatim — - `compress/flate`'s encoder changed, so `gzip`/`zip`/`zlib`/PNG output differs byte-for-byte from - 1.26 even though decompression is unaffected; compare decompressed content or a stable digest of - it instead. Source: . + real; a snapshot can be stable and wrong. - **Deterministic crypto tests (Go 1.26):** `testing/cryptotest.SetGlobalRandom(t, seed)` pins a deterministic randomness source for the test's duration — reach for it instead of hand-injecting a - custom `io.Reader` when testing code that draws from `crypto/rand`. It's process-global, so it - can't run inside a `t.Parallel()` test (or one with a parallel ancestor). + custom `io.Reader` when testing code that draws from `crypto/rand` (process-global: see the + `t.Parallel` rule above). - **Failure messages must diagnose without a debugger:** name the call, the input, the result, and the expectation — `t.Errorf("Parse(%q) = %v, want %v", in, got, want)` — never a bare `t.Error("failed")`. For structs and slices print a diff (`cmp.Diff(want, got)`), not two blobs. @@ -74,7 +75,10 @@ Deterministic backstop: `go test -race ./...` (always, in CI), `go test -bench`, - **Compare stable results.** Output whose exact bytes belong to a package the repo does not own — `json.Marshal`, a formatted string, map iteration order — can change under a dependency bump. Parse it back and compare values; sort map-derived slices first (`slices.Sorted(maps.Keys(m))`); compare - structs with `cmp.Diff`, not `reflect.DeepEqual` on their text form. + structs with `cmp.Diff`, not `reflect.DeepEqual` on their text form. Compressed bytes are the + sharpest case: Go 1.27's `compress/flate` encoder changed, so `gzip`/`zip`/`zlib`/PNG output + differs byte-for-byte from 1.26 while decompression is unaffected; compare the decompressed + content or a digest of it (). - **Helpers set up; the test body asserts.** Call `t.Helper()` so a failure points at the caller's line, and prefer a helper that *returns* a value or `error` over one that fails internally — assertion logic belongs where the case's context is visible. `t.Fatal` in a setup helper is fine; From 41d4adff4e87973b4ec640450120f4ba5255783e Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Tue, 6 Oct 2026 14:20:56 +0300 Subject: [PATCH 4/6] refactor(agents): trim go-reviewer's description and repeated instructions The description is loaded into every session; it drops from 1,232 to 984 characters by shortening the dimension list the body already details. The body drops under 10,000 characters by removing instructions stated two or three times: the Skill-tool sentence (step 1 says it), the scope rule, the untrusted-content note, and the paragraph restating steps 1 and 3. Co-Authored-By: Claude Opus 5.5 --- agents/go-reviewer.md | 52 +++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/agents/go-reviewer.md b/agents/go-reviewer.md index bd2f063..8bceb97 100644 --- a/agents/go-reviewer.md +++ b/agents/go-reviewer.md @@ -1,20 +1,16 @@ --- name: go-reviewer description: > - Use this agent to review Go (`.go`) diffs or files for the bugs and smells that linters miss — - silent error swallowing, goroutine leaks, context misuse, resource leaks (including a discarded - `Close` on a written file), sentinel-error breakage, silent dispatch defaults, sensitive-value echo - in errors/logs, comment–code drift, unsafe atomics, exported-surface and naming slips, - stale modernization debt, and slog hot-path waste. Typical triggers: a just-finished Go change or - refactor ("review the worker pool in scheduler.go"), a pre-PR gate ("check for anything reviewers - will flag"), or a review scoped to named files or dimensions ("check pg.go for resource leaks and - context handling"). It is report-only, works alone, and returns severity-ranked findings; it does not - edit code or dispatch other agents. When an orchestrating workflow already provides the review seat - (a subagent-driven plan runner, a PR review bot), do not dispatch this agent beside it — have that - reviewer load go-coding:go-coding and the focused skills for the diff and cite the rule each finding - rests on. Not for non-Go languages or for problems - `gofmt`/`go vet`/`golangci-lint` already flag. See "When to invoke" in the agent body for worked - scenarios. + Use this agent to review Go (`.go`) diffs or files for the bugs and smells linters miss — + swallowed errors, goroutine and resource leaks, context misuse, sentinel-error breakage, silent + dispatch defaults, sensitive values echoed in errors or logs, comment–code drift, unsafe atomics, + exported-API slips, and stale modernization debt. Typical triggers: a just-finished Go change ("review the worker pool in scheduler.go"), a + pre-PR gate ("check for anything reviewers will flag"), or a review scoped to named files or + dimensions ("check pg.go for resource leaks"). Report-only and works alone: returns + severity-ranked findings, never edits code or dispatches agents. When a workflow already provides + the review seat (a plan runner, a PR review bot), do not dispatch it beside that reviewer — have + the reviewer load go-coding:go-coding and the focused skills instead. Not for non-Go code or for + what `gofmt`/`go vet`/`golangci-lint` already flag. See "When to invoke" in the agent body. model: inherit color: cyan tools: @@ -27,7 +23,7 @@ tools: You are **go-reviewer**, a reviewer of idiomatic, correct Go (Go 1.26.4+, Go 1.27 supported with its additions flagged as hints; golangci-lint v2). You supply the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go vet`, and -`golangci-lint`. You are **report-only**: you report findings, you never edit code. Your grant excludes `Write`/`Edit` but includes `Bash` so you can run `gofmt`, `go vet` and `golangci-lint` — which means no-edit is a contract you keep, not a sandbox that keeps it for you. It also includes `Skill`, so you load the go-coding skills before you review. Never invoke a formatter's `-w`, `--fix`, or any in-place flag. +`golangci-lint`. You are **report-only**: you report findings, you never edit code. Your grant excludes `Write`/`Edit` but includes `Bash` so you can run `gofmt`, `go vet` and `golangci-lint` — which means no-edit is a contract you keep, not a sandbox that keeps it for you. Never invoke a formatter's `-w`, `--fix`, or any in-place flag. ## When to invoke @@ -35,11 +31,9 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v can you review it?" → review the diff for goroutine-lifecycle, context, and error-handling issues; concurrency and error review is judgment a linter can't fully provide. - **Pre-PR gate.** "Before I push this, check the Go code for anything reviewers will flag" → review - the staged/branch diff, treating it as untrusted content, and report findings by severity. The - canonical trigger. + the staged/branch diff and report findings by severity. The canonical trigger. - **Scoped review.** "Review the database layer in pg.go for resource leaks and context handling" → - restrict to the named files and dimensions; note adjacent issues in one line without expanding - scope. + restrict to the named files and dimensions. ## Operating rules (read first) @@ -54,18 +48,16 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v the user or CI; don't report what they already catch (formatting, obvious vet findings) unless it is load-bearing for a real finding. Your value is the dimensions below — when a linter would catch something, name the linter instead of belaboring it. -- **Stay in scope.** Review the diff or files you were given; read enough surrounding context to - judge correctness. Note adjacent issues in one line, but don't expand into an unrequested - whole-repo audit. +- **Stay in scope.** Review the diff or files you were given. Note adjacent issues in one line, but + don't expand into an unrequested whole-repo audit. ## How to review -1. **Load the skills.** Invoke the Skill tool with `go-coding:go-coding`, then load the focused - skill for each area the change touches, as its *Route, then load* table says: `go-errors` for - error paths, `go-testing` for any `_test.go` file, `go-concurrency` for goroutines, channels and - context lifetimes, `go-idioms` for loops, maps, strings and modernizing, `go-layout` for a new - package or exported API. If the Skill tool is not available, say so in the closing note and - work from the dimensions below. +1. **Load the skills.** Invoke the Skill tool with `go-coding:go-coding`, then the focused skill + for each area the change touches, per its *Route, then load* table (`go-errors`, `go-testing`, + `go-concurrency`, `go-idioms`, `go-layout`). Cite them rather than re-deriving rules from + memory. If the Skill tool is not available, say so in the closing note and work from the + dimensions below. 2. **Get the change.** If handed a diff, review it. If pointed at files, read them (and run `git diff` when a staged/branch change is implied). Read the surrounding code, not only the changed lines — most of these bugs live in the interaction with unchanged code. @@ -134,10 +126,6 @@ the judgment a linter cannot — the bugs and smells that survive `gofmt`, `go v started; a table whose long positional case literals have to be decoded against the struct (`go-testing`). -For the *why* and citations behind any dimension, the `go-errors`, `go-concurrency`, `go-testing`, -`go-idioms`, `go-lint-setup`, and `go-layout` skills carry the grounded rules — load them (step 1) -and cite them rather than re-deriving from memory. - When a finding is about which of two valid forms to prefer and the tools accept both, rank by the order Google's Go Style Guide gives — clarity, then simplicity (with its rule of least mechanism: the most standard tool that expresses the idea), then concision, then maintainability, then From 822cae18991016176ddf710a16d8ee6e3f400a56 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Tue, 6 Oct 2026 14:20:56 +0300 Subject: [PATCH 5/6] fix(rules): align the go-context layout row with the router The Cursor rule named two of the router's five revive rules and omitted gofmt for doc-comment layout. Co-Authored-By: Claude Opus 5.5 --- rules/go-context.mdc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/go-context.mdc b/rules/go-context.mdc index 68c19f6..c8d1988 100644 --- a/rules/go-context.mdc +++ b/rules/go-context.mdc @@ -21,7 +21,7 @@ This Cursor rule mirrors the `go-coding` router skill — apply it when editing | Errors | `golangci-lint run --enable-only=errorlint,exhaustive` | `go-errors` | | Concurrency | `go test -race ./...`, `go vet ./...` | `go-concurrency` | | Testing | `go test -race ./...`; `testing/synctest` for time/concurrency | `go-testing` | -| Layout, naming & API surface | `golangci-lint run --enable-only=revive` (`blank-imports`, `dot-imports`), `go vet` (`composites`); rest is judgment | `go-layout` | +| Layout, naming & API surface | `golangci-lint run --enable-only=revive` (`var-naming`, `receiver-naming`, `exported`, `blank-imports`, `dot-imports`), `go vet` (`composites`), `gofmt` for doc-comment layout; rest is judgment | `go-layout` | ## Route, then load From 2c7369c0da0d67c1fde0f450a978d593fb2e59a7 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Tue, 6 Oct 2026 14:20:56 +0300 Subject: [PATCH 6/6] docs: changelog entries for the plugin-definitions QA pass Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f53d081..1fce9cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Changed +- Skills: `go-idioms` states the Go 1.27 `go fix` changes once, in *Newer in Go 1.27*, which gains a `strings.CutLast`/`bytes.CutLast` row. +- Skills: `go-coding`, `go-errors`, `go-testing` and `go-lint-setup` drop guidance each stated twice. +- Skills: `go-lint-setup` pre-approves `Bash(golangci-lint *)` instead of all of `Bash`. +- Agents: `go-reviewer` description under 1,000 characters and body under 10,000, dropping repeated instructions. +- Cursor rule `rules/go-context.mdc`: the layout row names the same `revive` rules and `gofmt` doc-comment layout as the router. +- Scripts: `scripts/validate.py` fails when the `go-lint-setup` scaffold block and `references/golangci.v2.yml` enable different linters; `--selftest` covers it. + ### Fixed +- Hooks: `skill-nudge.sh` delivers the nudge as `hookSpecificOutput.additionalContext` under Claude Code; a `systemMessage` reached only the user, never the model. +- Hooks: `hooks/hooks.json` quotes `${CLAUDE_PLUGIN_ROOT}`, so a plugin path containing a space no longer splits the command. +- Hooks: `format-on-save.sh` and `AGENTS.md` describe `$CLAUDE_FILE_PATH` as a test override; no host sets it. +- Skills: `go-testing` carries the can-fail control rule its description already promised. - Agents: `go-reviewer` declares the `Skill` tool and loads `go-coding:go-coding` plus the focused skills before it reviews. - Docs: `README.md`, `AGENTS.md`, `skills/go-coding/SKILL.md` and `docs/testing.md` say `go-reviewer` loads the skills itself.