From d8064122c083d81a8301ea673501f2695257db90 Mon Sep 17 00:00:00 2001 From: daymade Date: Tue, 4 Aug 2026 20:30:54 +0800 Subject: [PATCH] feat(skill-creator): six corrections a real update cycle exposed (1.22.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distilled from using this skill to ship consecutive updates to an existing skill. Each item is a place the guidance let a predictable mistake through — none are hypothetical improvements. 1. Description budget (Write the SKILL.md) The 1024-character ceiling was never stated anywhere, so an update adding trigger phrases for newly-covered scope blew past it and needed two rounds of compression to land. That ceiling is in direct tension with the existing "make it pushy" advice, and the tension deserves naming rather than discovery while fighting a validator. The sharper point: a mature description usually sits close to the limit, which makes adding a trigger zero-sum — you are deleting an existing one to pay for it. That is a real narrowing of when the skill fires, so it must be a conscious, recorded trade. Also ranks what to sacrifice: prose qualifiers are re-derivable from the body, distinct trigger phrases are not. 2. Registry minimal-diff (Step 8) The marketplace manifest is the single file every skill shares, so it is the likeliest place for two concurrent editors to collide — and the worst place to ship an unrelated formatting change. Scripted bumps silently normalize trailing newline, indent width, key order, unicode escaping. The step now requires a round-trip check that git diff shows only the intended fields. (A scripted bump once added a trailing newline to a manifest that never had one. This commit's own bump was round-tripped under the new rule: one line.) 3. Activation check (Edit Skills at Source Location) "Sync the installed copy" is frequently not work at all, and the workflow never said how to tell. A symlinked skills directory, or a marketplace with source: directory, reads the working tree — the edit is already live. Only cached/copied installs need the official update command. Verification must be by content, not by a recorded version string: one session read a plugin record naming a cache directory that had the new version in its path and nearly reported the update as live. That directory had never been created; the real runtime path was a symlink to the source. 4. Production-as-eval gains a second signal source (Capture Intent) When a skill's output is something that keeps running — a guard, a monitor, a scheduled job, a hook — its own telemetry is eval data, and the first false alarm is the highest-signal record in it. A user correction requires a user to notice and bother; a deployed mechanism reports on itself unprompted, often within a day. A false positive proves a rule you wrote is wrong in a way re-reading never would, so treat the first one as a scheduled eval result rather than an annoyance — the likely finding is that the instruction was too absolute. 5. Concurrency covers branch switching, not just a moved HEAD The existing rule said to re-check HEAD's SHA before committing. That is not the worst thing a parallel session does. A checkout is worktree-wide, so while one session edits on a feature branch, another running `checkout main` + pull silently relocates the whole worktree — and the next commit lands on main, violating the repo's never-commit-to-local-main rule, while the feature branch still points at the old base. Exactly that happened during this work. The pre-commit check gains `git branch --show-current`; `git reflog` is the authoritative post-mortem (it records every "checkout: moving from X to Y" in order); and the repair is deliberately ref-only — `checkout -B` then `branch -f` — because both move refs without touching the working tree, so unlike `reset --hard` they cannot destroy a parallel session's uncommitted work. 6. PR staleness, and the proof step everyone skips The property that makes a shared manifest a collision hotspot also makes any open PR touching it go stale. CONFLICTING is the expected state, not a surprise — this PR itself sat through 69 commits of main. Rebase (not merge) where the repo squash-merges. The conflicts are almost always additive: two authors appended an entry to the same section, so keep both, and --ours/--theirs would silently drop a colleague's line. What is missing from most workflows is the verification AFTER resolving: prove the only difference from the base is your own entry. The section ships a copy-paste check that diffs every version in the base manifest against yours. Finally --force-with-lease rather than bare --force, since its entire value is failing in the one case that matters: somebody else pushed to your branch. Rebased onto current main (69 commits ahead of the original base) and rebuilt without touching the working tree, because a sibling session had uncommitted work in the two files this touches — the plumbing route (temporary index + commit-tree) is the practice item 5 is about. Verified: quick_validate passes; audit_skill_regression compare/classify/verify passes with two candidates (production-as-eval and the HEAD-check rule, both extended in place) classified preserved_or_moved with the original clauses intact and locatable; security_scan clean; manual read-through of every added line for private paths/names; manifest bump round-tripped to a one-line diff. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01F6FQSPAyXY9WZJYUToxXXY --- .claude-plugin/marketplace.json | 2 +- CHANGELOG.md | 1 + daymade-skill/skill-creator/SKILL.md | 42 ++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index ed8e3c6..5ab2799 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -230,7 +230,7 @@ "description": "Daymade skills core suite. Bundles skill creation, quality review, governance, and marketplace development tooling under one shared namespace. Existing-skill edits use an old-vs-new capability audit and content-bound packaging attestation so prompt compression cannot silently delete runtime contracts. When the official skill-creator plugin is also installed, skill-creator detects the coexistence and offers a consent-based, reversible SessionStart routing hook so the daymade edition wins deterministically; on machines without the official plugin nothing is ever installed.", "source": "./daymade-skill", "strict": false, - "version": "1.21.0", + "version": "1.22.0", "category": "suite", "keywords": [ "suite", diff --git a/CHANGELOG.md b/CHANGELOG.md index 3966546..961610c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **claude-migrate-memory-to-doc** (`daymade-claude-code` v1.19.0): migrate Claude Code personal memory (per-project `memory/`) into tool-agnostic reference docs so other AI CLIs auto-loading `AGENTS.md` (Codex primarily; transfers to Cursor) read the same user profile and collaboration preferences. Two-layer `references/` + CLAUDE.md-inline + AGENTS.md-symlink architecture designed around what each tool actually auto-loads; runs inline with multi-agent review and empirical `codex` verification. Also newly registers `claude-migrate-memory-to-doc` in the suite's `skills` array (it had shipped on disk unlisted). ### Changed +- **skill-creator** (`daymade-skill` v1.22.0): six corrections distilled from using the skill to ship consecutive updates to an existing skill, each one a place where the workflow let a predictable mistake through. (1) **Description budget** — the 1024-character ceiling was never stated, so an update that added trigger phrases for newly-covered scope blew past it and took two rounds of compression to land; the guidance now names the limit, notes that it is in direct tension with the "pushy" advice, and makes explicit that near the ceiling **adding a trigger is zero-sum** (you are deleting an existing one to pay for it) — a trade that must be made consciously and recorded, since a silently-dropped trigger phrase narrows when the skill fires and nobody notices until it stops firing for someone. It also ranks what to cut: prose qualifiers are re-derivable from the body, distinct trigger phrases are not. (2) **Registry minimal-diff** — the marketplace manifest is the single file every skill shares and therefore the likeliest concurrent-edit collision; scripted bumps silently normalize trailing newline / indent / key order, so the step now requires a round-trip check that `git diff` shows only the intended fields (a scripted bump once added a trailing newline to a manifest that never had one). (3) **Activation check** — "sync the installed copy" is often not work at all: a symlinked skills dir or a `source: directory` marketplace reads the working tree, so edits are already live, while only cached/copied installs need the official update. Verify by grepping the resolved runtime file for a phrase unique to the new content, never by trusting a recorded version string — one session read a plugin record naming a cache directory with the new version in its path and nearly reported the update as live; that directory had never existed. (4) **Production-as-eval** gains a second signal source: when a skill's output is something that keeps running (guard, monitor, scheduled job, hook), its own telemetry is eval data, and the **first false alarm** is the highest-signal record in it — a user correction needs a user to notice and bother, while a deployed mechanism reports on itself unprompted, and a false positive proves a rule is wrong in a way re-reading never would. (5) **Concurrency now covers branch switching, not just a moved HEAD** — a checkout is worktree-wide, so a sibling session running `checkout main` mid-edit lands your next commit on **main**, violating the repo's never-commit-to-main rule while your feature branch still points at the old base; the pre-commit check gains `git branch --show-current`, with `git reflog` as the authoritative reconstruction and a ref-only repair (`checkout -B` + `branch -f`) that, unlike `reset --hard`, cannot destroy a parallel session's uncommitted work. (6) **PR staleness** — the same property that makes the manifest a collision hotspot makes an open PR go stale, so `CONFLICTING` is the expected state rather than a surprise (this PR itself sat through 69 commits of main). Conflicts there are additive (two authors appended to the same section), so keep both sides, never `--ours`/`--theirs`; and the skipped step that catches a bad resolution is proving afterwards that the **only** difference from the base is your own entry, with a copy-paste check for it. Push with `--force-with-lease`, whose whole value is failing exactly when someone else pushed to your branch. - **frontend-visual-qa** v1.11.0: make the user-supplied target canonical and separate current-render truth from delivery freshness. Web navigation preserves the exact URL while persisted evidence redacts paths, query values, fragments, navigation errors, and target values reflected into rendered labels; reports store label hashes plus a target-string fingerprint. Single-file evidence adds a byte hash, multi-resource files use a resource/dependency manifest, and native apps use an installed-artifact fingerprint. Source, interaction/data, and target-lifecycle authority are independent. The skill now traces source → build → runtime → target → inspected pixels only for freshness/deployment claims, reports “source fixed; verification target stale” without blocking read-only inspection, and requires same-target identity/visual recheck only for fix closure. Every probe header value uses environment indirection, and raw screenshot/report directories are temporary local sensitive evidence. - **git-safety-net** v1.7.0: close the gap between "judge by content, not counts" and *which* content check to trust, plus a shared-worktree hazard. Distilled from a real audit in which three successive content-level instruments each returned a wrong answer before the trial merge settled it: `git cherry` (squash rewrites patch-ids → false UNMERGED), a **three-dot** `diff base...ref` used to ask "what does base lack" (wrong question — under-reported missing files 1 vs 5), and a file-level existence check (a file present on base can still be missing the ref's lines). Adds a diff-form section (two-dot vs three-dot, chosen by the question) and a **fourth supersession rung**: grep the base for the missing file's own name, because a replacement usually documents the removal in prose — a 107-line script absent from the base looked like textbook unique work until its successor's comments read "replaces the old …", "made this worse, not better", "CAUSED the corruption", i.e. deliberately excised harmful code whose "rescue" would have reintroduced a known bug. Also separates generated artifacts (scan markers, lockfiles) and relocated paths from real loss. New Mode D rule: in a shared tree, never aim `reset --hard`/`merge`/`rebase` at "the current branch" — a branch check goes stale the instant it returns, so a parallel session's `switch` redirects your destructive command onto **their** branch; use checkout-independent forms (`git branch -f`, `git fetch origin :`) that name their target. - **claude-code-hooks** (`daymade-claude-code` v1.23.0): add a fifth pattern — Stop hook, the only hook type that can react to Claude's own generated text (`UserPromptSubmit` only ever sees the user's input, a category mistake that caused a real same-day incident: a hook meant to catch Claude inventing an unverified shorthand name never once fired, while repeatedly false-blocking the user's own unrelated typing). Covers the full contract (`last_assistant_message` vs `transcript_path` fallback, the `stop_hook_active` anti-loop check and its JSON-string-vs-Python-truthiness trap), with a runnable, tested skeleton that uses a quoted heredoc instead of `python3 -c "…"` to structurally avoid a newly-cataloged pitfall (#9, 8→9 total): a literal quote or backtick inside a Python *comment* can silently corrupt an embedded multi-line block without `bash -n` catching it — confirmed by extracting and executing the shipped skeleton against 5 real JSON payloads, not just reading it. Also fixes CLAUDE.md / README.md / README.zh-CN.md skill lists, which were missing `docx-creator` and `claude-code-hooks` (both already registered in `marketplace.json`, never synced to the human-facing lists). diff --git a/daymade-skill/skill-creator/SKILL.md b/daymade-skill/skill-creator/SKILL.md index 3a4f44d..2a4abdc 100644 --- a/daymade-skill/skill-creator/SKILL.md +++ b/daymade-skill/skill-creator/SKILL.md @@ -136,7 +136,7 @@ When the source material is *past* session transcripts (the JSONL files under th 3. What's the expected output format? 4. Should we set up test cases to verify the skill works? Skills with objectively verifiable outputs (file transforms, data extraction, code generation, fixed workflow steps) benefit from test cases. Skills with subjective outputs (writing style, art, taste-calibrated reports) often can't use assertions — but "no assertions" is not "no verification". Their verification paths, in order of cost: - **Historical-task replay**: re-run one real prompt the skill has served before, old vs new skill, and compare outputs against the specific rules that changed ("does the new output actually follow the tokens / title grammar this update introduced?"). Cheap, catches "the rule was written but nothing reads it". - - **Production-as-eval**: acknowledge that the real test is the user's next actual use — then make the loop explicit: every user correction afterward is an incident to fold back (the skill's own "迭代/活文档" section), every approval is corpus material. A taste skill that ships without this write-back habit doesn't improve; one that has it converges without ever running a formal eval. + - **Production-as-eval**: acknowledge that the real test is the user's next actual use — then make the loop explicit: every user correction afterward is an incident to fold back (the skill's own "迭代/活文档" section), every approval is corpus material. A taste skill that ships without this write-back habit doesn't improve; one that has it converges without ever running a formal eval. **And when the skill's output is something that keeps running — a guard, a monitor, a scheduled job, a hook — its own telemetry is eval data, and the highest-signal record in it is the first false alarm.** A user correction requires a user to notice and bother; a deployed mechanism reports on itself unprompted, often within a day, and a false positive is the sharpest form of that report because it proves a rule you wrote is wrong in a way no amount of re-reading would have shown. Treat the first one as a scheduled eval result rather than an annoyance: check it before assuming the mechanism misbehaved, because the more likely finding is that the *instruction* was too absolute. (Real instance: a skill prescribed a fail-loud check, the deployed check fired once overnight on a perfectly healthy condition, and the fix was to correct the over-absolute sentence in the skill — nobody complained; the telemetry did.) - **Render + human review** for visual outputs (the skill's own visual-QA gates), never a grep assertion pretending to measure aesthetics. **And the renderer you verify with must be the same engine the deliverable will be consumed in** — whatever previewer is conveniently installed is not a substitute. A thumbnailer whose layout engine differs from the target application will silently *hide* the exact defects you are looking for, and a green verification on the wrong engine is worse than no verification, because it buys false confidence. Real case (2026-07): a .docx was "visually verified" through macOS Quick Look thumbnails, which do not reproduce justified-text stretching; Word showed the document's info blocks blown apart the moment the user opened it. The fix was to install the Word-compatible engine (LibreOffice), convert to PDF, rasterize per page, and read every page. Match the engine, or the verification is theater. **This generalizes past renderers to every verification tool** — parser, linter, validator: it must share an implementation with production, or its green is meaningless. Second case, same shape: an author tried to catch a markup pattern that corrupts the final document by checking at the source stage with a *different* markdown implementation than the production toolchain used — it parsed all three known-bad inputs as perfectly fine, so any pre-check built on it would have silently passed everything. The honest conclusion was that this particular defect is only detectable after the production tool has run, and the check belongs there. **When no available tool shares the production implementation, say the check cannot be done at that stage — do not build the one that can only produce false green.** Suggest the appropriate default based on the skill type, but let the user decide. @@ -388,6 +388,8 @@ Based on the user interview, fill in these components: - **name**: Skill identifier - **description**: When to trigger, what it does. This is the primary triggering mechanism - include both what the skill does AND specific contexts for when to use it. All "when to use" info goes here, not in the body. Note: currently Claude has a tendency to "undertrigger" skills -- to not use them when they'd be useful. To combat this, please make the skill descriptions a little bit "pushy". So for instance, instead of "How to build a simple fast dashboard to display internal Anthropic data.", you might write "How to build a simple fast dashboard to display internal Anthropic data. Make sure to use this skill whenever the user mentions dashboards, data visualization, internal metrics, or wants to display any kind of company data, even if they don't explicitly ask for a 'dashboard.'" + + **Budget it: the description has a hard 1024-character ceiling, and validation rejects anything longer.** This is in direct tension with the "pushy" advice above — every trigger phrase you add for coverage spends budget — so measure before you expand rather than after: `len(description)`, not vibes. The trap is not the first draft (which is rarely near the limit) but the *update years later* that adds triggers for newly-covered scope: a mature description often sits within a few dozen characters of the ceiling, at which point **adding a trigger is zero-sum — you are deleting an existing one to pay for it.** Make that trade consciously and say so in the commit, because a silently-dropped trigger phrase is a real narrowing of when the skill fires, and nobody will notice until it stops triggering for someone. (Seen in practice: an update added triggers for a newly-covered failure mode, pushed the description to 1280 characters, and took two rounds of compression to reach 1003 — the price was three pre-existing trigger phrases, which is a decision that deserved to be explicit rather than discovered while fighting a validator.) When you must cut, prefer phrases whose scenario is still reachable through a synonym or a sibling phrase, keep the ones with no other route in, and remember that qualifiers inside the prose ("on platform X and Y", parenthetical enumerations) are usually cheaper to drop than a distinct trigger phrase — the prose is re-derivable from the body, a trigger phrase is not. - **compatibility**: Required tools, dependencies (optional, rarely needed) - **the rest of the skill :)** @@ -1137,13 +1139,30 @@ find . -path '*/SKILL.md' -maxdepth 4 | rg '(^|/)/SKILL.md$' If the available-skills list points at `~/.codex/skills`, `~/.claude/skills`, or a plugin cache, do not assume that path is source. Locate the repository-backed source first, edit it, validate it, and only then sync the installed copy when the user needs immediate local runtime use. +**Then answer the follow-up question that decides whether "sync the installed copy" is even work: does the runtime already read the source?** Three installs behave differently, and guessing wrong either wastes a sync or ships an edit the user's next session never sees: + +```bash +ls -la ~/.claude/skills/ # a symlink into the repo? -> edits are live already +grep -A3 '""' ~/.claude/plugins/known_marketplaces.json # "source": "directory" -> reads the repo in place +``` + +- **Symlinked** into the source tree → the edit *is* the runtime. Nothing to sync. +- **Marketplace with `source: directory`** pointing at the repo → same: it reads the working tree, so a version bump is bookkeeping for other consumers, not a local activation step. +- **Anything cached/copied** (git-sourced marketplace, a `cp -r` install) → the runtime is a separate copy and genuinely needs the official update command before the new content is live. + +**And verify activation by content, not by a version string.** A registry can record a path or version that does not exist on disk — one session read a plugin record naming a cache directory with the *new* version number in it and nearly reported the update as live; that directory had never been created, and the real runtime path was a symlink to the source all along. The authoritative check is to grep the runtime file for something only the new version contains: + +```bash +grep -c "" /SKILL.md # 0 = not live +``` + ### Concurrent sessions on the same skill repo Power users run several Claude sessions at once, and skill repos are exactly where they collide: while you edit skill A, a sibling session may commit skill B (or even an earlier round of skill A) under you. One real session hit all three symptoms inside an hour — a `Write` rejected because the file changed after reading, and HEAD moving twice mid-task (methodology Case 16). The failure isn't the collision; it's a stale baseline or a clobbering write that silently mixes two sessions' work. Standing rules: 1. **Baseline from a git ref, not from the working tree**, whenever the repo is clean at task start: `git archive | tar -x -C /skill-before` and pass `--baseline-origin git-ref:` to the audit. A tree snapshot taken minutes before someone else's commit is a baseline for a tree that no longer exists. 2. **Re-read before write** when a write is rejected or any time has passed: diff what changed (`git log --oneline -3`, `git show --stat`), fold the other session's intent into your version — their edit usually has a reason — and only then write. -3. **Check HEAD before committing** (`git log --oneline -1`): if it moved since your baseline, re-run the regression `compare` against the new ref before `verify` — the audit tool will reject a stale review anyway ("after skill changed"), so catching it yourself saves a round. +3. **Check HEAD *and which branch you are on* before committing.** `git log --oneline -1` catches a moved SHA: if it moved since your baseline, re-run the regression `compare` against the new ref before `verify` — the audit tool will reject a stale review anyway ("after skill changed"), so catching it yourself saves a round. But a sibling session can do something worse than advance HEAD: **it can switch the branch out from under you**, because a checkout is worktree-wide. Real sequence — `checkout -b feat/x`, edit for a while, and meanwhile another session ran `checkout main` + `pull`; the commit then landed on **main**, violating the repo's "never commit directly to local main" rule while the feature branch still pointed at the old base. So add `git branch --show-current` to the pre-commit check, not just the SHA. When it has already happened, `git reflog` is the authoritative reconstruction (it records each `checkout: moving from X to Y` with order), and the repair — **given a clean worktree** — is `git checkout -B ` followed by `git branch -f main origin/main`: both are ref moves that never touch the working tree, so neither can destroy a parallel session's uncommitted work the way `reset --hard` would. 4. **Stage only your own paths** (`git add `), never `git add .` — the sibling session's uncommitted work must not ride along. (Already the rule for packaging; doubly load-bearing under concurrency.) 5. **One version bump per session outcome**, not per editing round: consecutive same-session rounds on one skill collapse into a single bump — unless an intermediate state was already consumed (committed + pulled by the user or another session), which makes each consumed state its own version. @@ -1496,6 +1515,25 @@ After packaging, update the marketplace registry to include the new or updated s **For updated skills**, bump the version in `plugins[].version` following semver. Any change to a skill's files — even a one-line typo fix — needs a bump: without it, `marketplace update` sees no new version, so **already-installed copies never refresh** and users keep running the old skill while your fix sits unshipped. +**Keep the registry diff minimal — it is the single file every skill shares.** A marketplace manifest is the one place where every concurrent editor collides, so an unrelated formatting change there is far more expensive than the same change anywhere else: it turns a clean three-line bump into a conflict for whoever else is mid-edit. When you script the update (parsing to JSON, mutating, writing back), the rewrite silently normalizes things the file may not have used — trailing newline, indent width, key order, unicode escaping. Round-trip discipline: re-read the file afterwards and run `git diff --stat` on it; **the only lines that may appear are the fields you meant to change.** If extra lines show up, restore the file's original convention rather than shipping the normalization (a scripted bump once added a trailing newline to a manifest that had never had one — one wasted diff line, in the file most likely to be edited by someone else at the same moment). The same instinct applies to any shared registry a skill touches: lockfiles, catalogs, index documents. + +**When a PR outlives a few merges, rebase — then prove you didn't eat anyone's work.** The same property that makes the manifest a collision hotspot makes it the thing that goes stale: an open PR touching the registry and the changelog will conflict as soon as anything else lands, so expect `mergeable: CONFLICTING` rather than treating it as a surprise (one PR hit it after main moved 7 commits in an afternoon). Rebase rather than merge if the repo squash-merges — a merge commit in a squashed history buys nothing. The conflicts themselves are almost always **additive**: two authors each appended their own entry in the same section, so the resolution is to **keep both**, never `--ours`/`--theirs`, which silently discards a colleague's line. + +The step people skip is the one that catches a bad resolution — **after resolving, prove the only difference from the base is yours:** + +```bash +# Every version the base has vs. what your branch has; the diff must contain +# ONLY the entry you bumped. Anything else means the resolution ate someone's work. +python3 -c " +import json,subprocess +mine=json.load(open('')) +base=json.loads(subprocess.run(['git','show','origin/main:'],capture_output=True,text=True).stdout) +b={p['name']:p['version'] for p in base['plugins']}; m={p['name']:p['version'] for p in mine['plugins']} +print({k:(b.get(k),m.get(k)) for k in set(b)|set(m) if b.get(k)!=m.get(k)})" +``` + +Then push with `--force-with-lease`, never a bare `--force`: the lease makes the push fail if the remote moved since you last fetched, which is exactly the case where someone else pushed to your branch and a plain force would erase them. + **Plugin boundaries are not this skill's domain.** Whether to split skills into separate plugins, how to lay out `source`/`skills`, and whether users can toggle skills individually all belong to the packaging/distribution domain — the SSOT is