The skill was copied from a Rust project (spellkave); rewrite its examples and references for this Go repo. - binary env var SPELLKAVE_BIN -> FORGEJO_MCP_BIN (default forgejo-mcp) - evidence examples cargo/Rust -> go test (real forgejo-mcp tests) - capability/path examples -> merge-pull-request, pkg/, operation/ - drop dead tooling refs (just check-demos, check-spec-demo-anchors.sh, schemas/spec-driven template) that do not exist here; replace with manual anchor/scenario parity review - strip spellkave project jargon (PoC2, Tier-C pilot)
4 KiB
Authoring Smells
Seven concrete antipatterns surfaced from demo review. Each has a one-line do/don't. All apply to both new and retrofit demos.
1. Absolute build paths (replayability)
DON'T: PATH=/Users/alice/src/forgejo-mcp--20260425:$PATH forgejo-mcp …
DO: Use ${FORGEJO_MCP_BIN:-forgejo-mcp} and add a ## Replay setup block
at the top of the demo documenting how to point FORGEJO_MCP_BIN at a local
build. Absolute paths from the author's worktree break replayability for
every other developer and reviewer.
2. Doubled H2 + H4 scenario headings (heading dedup)
DON'T:
## AC1 — Config, state, and cache have distinct homes
#### Scenario: Config, state, and cache have distinct homes
- **WHEN** …
DO: Drop the ## AC<n> — H2 entirely. The #### Scenario: H4 inside
the anchor block is the canonical proof heading. The H2 was scaffolding for
the old AC-based format; in anchored mode it creates a redundant, stale
navigation layer.
3. AC numbering with insertion gaps (false continuity)
DON'T: ## AC1 … ## AC3 … ## AC5 after inserting new scenarios between
existing ones, leaving gaps in the sequence.
DO: Drop AC numbering entirely on retrofit. Use the scenario heading as the sole identifier. Numbered headings create false continuity when sequences evolve; the spec itself is the authoritative ordering.
4. Stale [openspec/changes/<old-name>] provenance links (link rot)
DON'T: Keep a full multi-field header block (**Spec:** … **Change:** … **PR:** …) pointing at a change directory that has since been merged or
superseded.
DO: Replace the block with a single provenance one-liner:
*Originally captured for `<old-cap>` / PR #N; merged into `<new-cap>` on YYYY-MM-DD.*
Do not try to keep multi-field provenance alive after consolidation — it rots faster than one line of prose.
5. Heterogeneous block shape (inconsistent evidence shape)
DON'T: Mix terse cmd + output blocks in some scenarios with rich
lead-in prose + cmd + output + contrast note blocks in others within the
same demo.
DO: Pick one shape per demo and apply it consistently:
- Terse: machine anchor → human anchor → quoted scenario →
cmdblock →outputblock - Rich: machine anchor → human anchor → quoted scenario → lead-in sentence →
cmdblock →outputblock → contrast / explanation
Heterogeneous shape makes the demo harder to scan and signals that the demo grew by accretion rather than authoring intent.
6. Test-name-only proof fitness (proof opacity)
DON'T: Use test-invocation evidence where the test function name does
not directly map to the scenario's THEN clause, leaving the reader unable
to verify the mapping without reading the test source.
DO: When a test name doesn't clearly map to THEN, downgrade to
unit-test-output and add an assertion-naming comment:
<!-- evidence-kind: unit-test-output -->
<!-- proves: asserts that a bare token is rejected, not forged into an identity — see TestExtractToken/bare_token_rejected -->
```bash
go test ./operation/ -run TestExtractToken
This is the D5 design decision: proof fitness must be legible at demo-read
time, not only at test-source-read time.
## 7. Absolute repo paths in evidence commands (path portability)
**DON'T:** Hard-code the worktree directory in paths inside evidence blocks:
```bash
$ /Users/alice/src/forgejo-mcp--20260425/scripts/run_fixtures.sh
DO: Use repo-root-relative paths (e.g. scripts/run_fixtures.sh,
run from repo root) or ${SHOWBOAT_REPO:-$(git rev-parse --show-toplevel)}
for scripts that require an absolute prefix. Document the variable in the
## Replay setup block and add export SHOWBOAT_REPO="$(pwd)" to the
setup instructions.
This smell differs from smell #1 (binary path): it applies to any file path
inside a command that is baked from the author's worktree location — not just
PATH= entries. Both must be replaced on retrofit.