diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 250d482..c7cf437 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -44,7 +44,7 @@ "name": "ghost", "source": "./ghost", "description": "Write, revise, and push blog posts to a Ghost site from Claude Code — a Ghost Admin API MCP plus a plan→draft→revise→push skill set.", - "version": "0.1.5" + "version": "0.1.6" } ] } diff --git a/engineering-standards/.claude-plugin/plugin.json b/engineering-standards/.claude-plugin/plugin.json index 00bb157..b3848db 100644 --- a/engineering-standards/.claude-plugin/plugin.json +++ b/engineering-standards/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "engineering-standards", - "version": "0.1.2", - "description": "Evergreen engineering standards I hold code to — the judgment layer that AI-slop resistance needs on top of the mechanical quality stack. Project-structure conventions (consistent layout + naming so every repo looks the same), worktree isolation (every line of work in its own tree so parallel work never collides), and GitHub API discipline (zipball over per-file, rate-limit backoff), with automation-auth (GitHub App vs PAT) and hook-escape conventions on the way. Pure advisory skills that fire when the situation matches — no tooling, no setup, safe to leave enabled for a project's whole life. Backed by lessons from real incidents.", + "version": "0.2.0", + "description": "Evergreen engineering standards I hold code to \u2014 the judgment layer that AI-slop resistance needs on top of the mechanical quality stack. Project-structure conventions, worktree isolation (every line of work in its own tree; bare primaries need a sibling), GitHub API discipline, exit-code integrity (never read a check's result through a pipe), verify-the-artifact (source is a claim, only the running artifact is evidence), and testing interactions over time (the defects users hit live between two moments, and state tests cannot see them). Pure advisory skills that fire when the situation matches \u2014 no tooling, no setup, safe to leave enabled for a project's whole life. Backed by lessons from real incidents.", "author": { "name": "Court Schuett" }, diff --git a/engineering-standards/skills/exit-code-integrity/SKILL.md b/engineering-standards/skills/exit-code-integrity/SKILL.md new file mode 100644 index 0000000..04055b2 --- /dev/null +++ b/engineering-standards/skills/exit-code-integrity/SKILL.md @@ -0,0 +1,82 @@ +--- +name: exit-code-integrity +description: Use when running any check, test, build, lint, or deploy from a shell — especially when piping its output through tail/head/grep, or wrapping it in a compound command to keep the output short. The rule — a pipeline reports the LAST command's exit code, so `check | tail` turns a failing check into a passing one. Never read a check's result through another command. +--- + +# Exit-code integrity + +## The rule + +**Never pipe a check into another command and then read the result.** + +```bash +just verify | tail -5 && echo "OK" # ← reports tail's exit code. Always 0. +go test ./... | grep -v "^ok" # ← reports grep's. Inverted, too. +npm run build 2>&1 | head -20 # ← reports head's. +``` + +A pipeline's exit status is the status of its **last** command. `tail` succeeds +at tailing a failure. `head` succeeds at heading a stack trace. The check ran, +it failed, and the shell told you it passed. + +## Why this one is worth a skill + +It is not that the mistake is subtle. It is that the mistake is **invisible and +convincing**: you get clean-looking output and a zero status, and then you tell a +human the check passed. A laundered exit code does not just hide a bug — it +converts into a false statement you make to someone who is relying on you. + +Four times in one codebase, by the same hands that had already written the rule +down: + +- A `staticcheck` finding masked by `| tail -1`. +- A `govulncheck` failure on a dependency, masked the same way. +- Again while verifying the very tool built to report coverage honestly. +- `just build 2>&1 | tail -5 && echo "=== BUILD OK ==="` — printed BUILD OK over + a wasm build that had died with `error obtaining VCS status: exit status 128`. + The failure text was **in the printed output**, three lines above the word OK. + +That last one is the tell. The evidence was on screen and the conclusion still +came from the exit code. Reading output is not checking status. + +## The forms it takes + +| Construct | What you read | What you wanted | +| --- | --- | --- | +| `cmd \| tail` / `\| head` / `\| less` | the pager's status | `cmd`'s | +| `cmd \| tee log` | `tee`'s status | `cmd`'s | +| `cmd \| grep FAIL` | **inverted** — `grep` exits 1 when it finds nothing, so a clean run "fails" | `cmd`'s | +| `[ -n "$(cmd)" ]` | whether output was non-empty | `cmd`'s | +| `cmd1 \| cmd2 && deploy` | `cmd2`'s | both | + +`set -o pipefail` fixes pipelines, and is **off by default** in every +non-interactive `sh`/`bash` invocation you did not write yourself. Do not rely +on it being set; rely on not needing it. + +## How to apply + +**Let the check exit on its own, then filter the file.** Capture the status +immediately — the next command overwrites `$?`. + +```bash +just verify > /tmp/verify.log 2>&1; rc=$? +tail -20 /tmp/verify.log +echo "rc=$rc" +``` + +Now the output is for reading and `rc` is for deciding, and they cannot be +confused. If a step must gate another, chain on the command itself +(`cmd && next`) with nothing between them. + +**When output volume is the problem, solve it at the far end.** The reason +people reach for `| tail` is a wall of text. Redirect to a file and tail the +file — same brevity, real status. + +**Red flags in your own drafts.** If a command you are about to run contains +`| tail`, `| head`, `| grep`, or `2>&1 |` and its result will become a claim +about whether something passed, stop and rewrite it. If you already ran it, run +it again properly before reporting. + +**Never report a green from a laundered status.** If you catch it after the +fact, say so plainly and re-run — a correction costs a sentence; an +unverified "it passes" costs whatever gets built on it. diff --git a/engineering-standards/skills/testing-interactions-over-time/SKILL.md b/engineering-standards/skills/testing-interactions-over-time/SKILL.md new file mode 100644 index 0000000..d00d87c --- /dev/null +++ b/engineering-standards/skills/testing-interactions-over-time/SKILL.md @@ -0,0 +1,86 @@ +--- +name: testing-interactions-over-time +description: Use when writing or reviewing tests for a UI, a live-updating view, or anything with a second actor — a poll, a websocket, a background job, another user, an agent. The rule — most real defects are interactions across TIME (something arrives while you are typing; an element grows after it was measured), and a test that sets a state and asserts it cannot see any of them. +--- + +# Test interactions over time, not states + +## The rule + +**A test that sets up a state and asserts that state can only find bugs that are +already visible in a screenshot.** The defects users actually hit live in the +gap between two moments: something arrived while they were typing, something +grew after it was measured, two actors touched the same thing at once. + +To find those, a test must **hold something across a change** — capture it, +cause the event, then re-read the same thing — rather than arrange a world and +describe it. + +## The evidence + +A markdown editor with a full browser test suite. The first time a human used it +for real work, ten defects surfaced in minutes. **None had been found by any +test**, and every one was an interaction over time: + +- **The reply box cleared while being typed into.** A background poll rebuilt + every card on refresh, destroying a half-written sentence with its card. Worse + when the agent was live, because the agent's own reply triggered the repaint — + answering a reviewer actively destroyed what they were writing back. +- **Cards drew on top of each other.** The stack was computed from each card's + *measured* height; a card that grew afterwards — a textarea dragged taller, a + new message arriving — left every position below it stale. +- **The agent's writes woke the agent.** Every mutation reached the notifier, + including the server's own. + +The suite had many tests. They set a state and asserted it. The one check that +had ever caught this class was a caret probe — the only one that held a position +**across** a change. + +A later regression makes the same point from the other side: a layout fix was +verified with "does it overflow?" and "does the count stay on one line?" — both +green — while the actual failure was two elements occupying the same pixels. The +question that would have caught it was never asked. + +## Why state tests structurally cannot see these + +A state test's world has one actor: the test. Its timeline has one moment: now. +Every defect above needs **two actors** (a reviewer and a poll; a layout pass and +a resize) or **two moments** (measured, then changed). Neither is expressible in +`arrange → act → assert` over a single frame, so no amount of adding state tests +increases coverage of this class. It stays exactly zero. + +## How to apply + +**Hold something across the change.** The shape is capture → cause → re-read the +*same* handle, not cause → assert final state. + +```js +const before = await el.boundingBox(); +await somethingElseHappens(); // the poll, the arrival, the other actor +const after = await el.boundingBox(); +expect(after).toEqual(before); +``` + +**Name the second actor explicitly.** Write the test as a sentence with two +subjects: *"the reviewer is typing while the agent replies."* If you cannot name +two, the test is a state test wearing different clothes. + +**Assert on what must NOT change.** State tests assert the new value; over-time +tests assert stability — the caret is still in the sentence, the neighbour did +not move, the draft survived. Bugs in this class are things being *taken away*, +which no assertion about a new value can see. + +**Cover measure-then-grow wherever geometry is computed.** Anything laid out +from measured sizes needs a case where a measured thing changes size afterwards. +This is a bug generator, not a bug: it recurs every time the layout code is +extended. + +**Split claims that sound like one claim.** "Nothing moved" and "what you +clicked is still under the cursor" are different assertions, and only the second +one describes the misclick a user experiences. Likewise "it does not overflow" +and "no two elements share pixels". When a check passes and the bug is still +there, suspect you asserted the neighbouring claim. + +**Let the real thing run.** These defects need real timers, real repaints, a +real second process. A mocked poll fires when the test says so, which removes +the interleaving that *is* the bug. diff --git a/engineering-standards/skills/verify-the-artifact/SKILL.md b/engineering-standards/skills/verify-the-artifact/SKILL.md new file mode 100644 index 0000000..46585d8 --- /dev/null +++ b/engineering-standards/skills/verify-the-artifact/SKILL.md @@ -0,0 +1,88 @@ +--- +name: verify-the-artifact +description: Use when a change's effect is separated from its source by a build step, a bundler, a cascade, or any runtime resolution — CSS, generated assets, compiled bundles, templates, layered config, env precedence. The rule — source is a claim about behaviour; only the running artifact is evidence. Read the resolved value out of the running system rather than reasoning about the source that should have produced it. +--- + +# Verify the artifact, not the source + +## The rule + +**Between the file you edited and the behaviour you want, count the steps. If +there is even one, the file is not evidence.** + +A bundler, a minifier, the CSS cascade, template inheritance, config layering, +env-var precedence, a Docker layer cache, a symlinked `node_modules` — each is a +place where what you wrote and what runs can differ, silently, with no error +anywhere. Verify at the far end: read the resolved value out of the running +system. + +## The incident that names it + +A CSS rule in a real editor: + +```css +.gly-card button { border: 1px solid var(--gly-line); color: inherit; } /* 0,1,1 */ +.gly-thread-delete { border-color: transparent; color: var(--gly-muted); } /* 0,1,0 */ +``` + +The intent was that `delete` — irreversible outside version control — should +render quieter than `resolve` beside it. The rule was written, carefully +commented ("it never becomes the loudest thing on the card"), reviewed, and +shipped. **It never applied once.** The container rule out-specifies it, so the +borderless and the muted were both discarded and delete rendered as an ordinary +pill of equal weight to the verb that keeps every word. + +Nobody reading that stylesheet could see it — and several people read it, +including the author who wrote the comment defending the behaviour. One call to +`getComputedStyle` in a real browser found it in seconds. + +Two more from the same codebase, same shape: + +- **Conflict markers committed inside generated CSS.** The full verification + gate passed green, because that gate compiles Go and Go does not compile CSS. + Only rebuilding the bundle found them. +- **A stale committed bundle.** Source edited, bundle not regenerated, every + check green, the browser serving the previous version of the code. + +## Why source-reading fails specifically + +Reading source answers *"is this rule correct?"* The failure modes above are all +instances of a different question: *"does this rule apply?"* — and that one is +decided by things not present in the file you are reading. Specificity is +decided by every **other** rule. Precedence is decided by the loader. Freshness +is decided by whether someone ran the build. + +You cannot reason your way to the answer from one file, and being careful does +not help, because carefulness is aimed at correctness. + +## How to apply + +**Name the gap before you start.** What sits between this file and the running +behaviour? Bundle? Cascade? Cache? Merge? If the answer is "nothing", source is +fine. Otherwise plan to verify at the far end from the outset — retrofitting +verification after a change is how you end up asserting your own fix. + +**Measure where the value resolves.** + +| Gap | Where evidence lives | +| --- | --- | +| CSS cascade | `getComputedStyle(el)` in a real browser | +| Bundler / minifier | the built file, or the served HTTP response | +| Template inheritance | the rendered output | +| Layered config / env | the process's own resolved config at runtime | +| Container build | `docker run … env`, or the image's actual layers | + +**Make the assertion fail first.** Run it against the *unfixed* system and watch +it fail before you change anything. An assertion only ever run after the fix +cannot distinguish "I fixed it" from "it was never broken" from "I am measuring +the wrong element". This is the whole reason the CSS bug survived review: no +check ever ran that could have gone red. + +**Ask what your gate can physically see.** A gate that is green is only +meaningful over what it reads. A Go test suite is blind to CSS. A type checker +is blind to runtime config. A linter is blind to a stale build output. When a +change lands in a medium the gate does not read, the green is not about your +change at all — and it will feel exactly like the green that is. + +State that blind spot out loud where the checks are documented, so the next +person does not read the same green as coverage. diff --git a/engineering-standards/skills/worktree-isolation/SKILL.md b/engineering-standards/skills/worktree-isolation/SKILL.md index 6438b27..aad1696 100644 --- a/engineering-standards/skills/worktree-isolation/SKILL.md +++ b/engineering-standards/skills/worktree-isolation/SKILL.md @@ -40,6 +40,39 @@ Always `/.worktrees/` — resolved from the **repo the - **Multi-repo workspaces.** When several repos sit under one coordination root and you may be launched from the root *or* from a member, deriving the path from the target repo's root (`git -C "$TARGET" rev-parse --show-toplevel`) makes it identical either way. Don't build cwd-relative or primary-clone-absolute paths. - **Never `/tmp`.** On macOS `/tmp` is a symlink to `/private/tmp`; worktrees under it break tooling that resolves modules in worker threads — vite-node/vitest dies with `Cannot find package …` *before a single test runs*, so suites silently never run and regressions only surface in CI. A worktree under the repo root is already a real path; `pwd -P` is the backstop. +### The exception: a bare primary clone + +**When the primary clone is bare, `/.worktrees/` is inside the git +directory itself — and that breaks build tooling.** Use a sibling instead: + +```bash +git -C "$BARE" worktree add "${BARE}-wt/" -b origin/dev +``` + +A bare repo has no working tree, so `git status` there is fatal (exit 128). Any +tool that walks *up* from your package directory to find the repository root can +land on the bare repo rather than on your worktree, run git there, and fail — +reporting something that names neither worktrees nor bareness: + +``` +error obtaining VCS status: exit status 128 + Use -buildvcs=false to disable VCS stamping. +``` + +That is Go's `-buildvcs` stamping, and it fails **every** `go build` in **every** +worktree nested under a bare primary. It cost an hour to trace, because the error +points at VCS stamping and the actual cause is a directory layout chosen weeks +earlier. Anything else that resolves a repo root by walking upward — coverage +tools, release stampers, monorepo task runners — can hit the same wall. + +`-buildvcs=false` silences it, at the cost of unstamped binaries, and only for +Go. Moving the worktrees out is the fix that works for every tool. + +**Detect it before you place a worktree:** `git rev-parse --is-bare-repository` +returns `true`. If it does, use `${repo}-wt/` and note it in the project's +own docs — a nested worktree that *already* exists will keep failing builds until +it is moved, and the error message will never say why. + ## Starting parallel work To open a *new* parallel line of work, start it isolated from the beginning: diff --git a/ghost/.claude-plugin/plugin.json b/ghost/.claude-plugin/plugin.json index 5ea4665..e5fe236 100644 --- a/ghost/.claude-plugin/plugin.json +++ b/ghost/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "0.1.5", + "version": "0.1.6", "description": "Write, revise, and push blog posts to a Ghost site from Claude Code — a Ghost Admin API MCP plus a plan→draft→revise→push skill set.", "author": { "name": "Court Schuett" diff --git a/ghost/skills/draft-post/SKILL.md b/ghost/skills/draft-post/SKILL.md index a8e30c3..e089cf5 100644 --- a/ghost/skills/draft-post/SKILL.md +++ b/ghost/skills/draft-post/SKILL.md @@ -114,13 +114,26 @@ title: "" status: draft date: tags: [] +generator: "ghost:draft-post" +next: "/ghost:revise-post" --- - - ``` +**The provenance note is front matter, not an HTML comment, and that is load-bearing.** +This template used to emit `` as the +first line of the body. Drafts are reviewed in galley (`galley edit `), which +REFUSES raw HTML outright — and an HTML comment is raw HTML. So every draft this skill +produced was unopenable for review, and the author had not typed a character yet: the +tool put the blocker in before the writing started. Measured 2026-08-22 against a real +corpus — 19 of 24 drafts in ghost-site refused, every one on that banner line, at lines +7-11. Front matter is parsed and skipped by galley, and `push-draft` reads named keys +and ignores the rest, so the provenance survives with nothing downstream to teach. + +DO NOT put an HTML comment back into the emitted draft, here or anywhere else in this +skill. If something new needs to ride along with a draft, it goes in front matter. + Apply every constraint from the style guide as you write: **From Stated voice:** @@ -239,6 +252,7 @@ The greps in Phase 5 catch fixed strings. The tells that make writing read as AI - **Negative-first.** Lead with the reason, not the absence. - _Before:_ "Pages don't get their own tools." → _After:_ "Because a page is the same object as a post, the tools take a `type` argument." - **Slip-narration.** Present the working config, not a blow-by-blow of what broke and how it got fixed. +- **Answering unasked questions.** Pre-emptive explanation of mechanisms, alternatives, dead ends, or edge cases the reader didn't need. A paragraph justifying why some other approach wouldn't work — when the reader never asked about that approach — is a cut. One clause of warning is the ceiling for a dead end, and only when the reader would plausibly hit it. If a sentence exists to head off a hypothetical objection, delete it. Fix these in the file. Only then continue to the hand-off. diff --git a/ghost/skills/revise-post/SKILL.md b/ghost/skills/revise-post/SKILL.md index 0129ab5..42aed0f 100644 --- a/ghost/skills/revise-post/SKILL.md +++ b/ghost/skills/revise-post/SKILL.md @@ -174,6 +174,7 @@ If the style guide is available, also check: - **Retcon** — a fix dressed up as intentional design; describe what the thing does and why, not a story of having meant it all along. - **Overselling** — "I set one rule at the start", "the part I lean on"; understate. - **Negative-first** — lead with the reason, not the absence ("Because a page is the same object as a post…," not "Pages don't get their own tools"). +- **Answering unasked questions** — pre-emptive explanation of mechanisms, alternatives, dead ends, or edge cases the reader didn't need. One clause of warning is the ceiling for a dead end, and only when the reader would plausibly hit it. If a sentence exists to head off a hypothetical objection, delete it. #### Axis 3 — Content and accuracy