Skip to content

docs(skills): close the five principle gaps and one factual error the QA wave exposed - #9470

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-9387-skills-principle-gaps
Aug 18, 2026
Merged

docs(skills): close the five principle gaps and one factual error the QA wave exposed#9470
hotlong merged 2 commits into
mainfrom
claude/issue-9387-skills-principle-gaps

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#9387

Skills changes are ADR-class (maintainer ruling 2026-08-11: 「所有 skills 的更新和 adr 类似,需要人工审核」). This is a draft PR awaiting a human: ⛔ do not merge, do not enable auto-merge, do not enqueue.

Six entries, all six confirmed against the current files before writing. Diff is +47 / -3 across exactly three files; nothing under docs/qa/** and nothing under .claude/skills/pm-dispatch/**.

Premise verification — I checked "the skill says X" before correcting it

The card states each gap as "the skill says X, the measurement showed Y". All six halves held on origin/main, with two refinements worth reviewing:

#entrypremise verdict
1subagent fleetConfirmed. checklist-test carries 「派发 runner 子代理时,必须用 opus」; checklist-author carries 「一轮全量 sweep ≈ 5 个 hunter + 8 个 writer agent」. Neither states a fallback.
2cold-environment costConfirmed, one correction. The text is 「整轮只构建一次」 as stated — but it sits at the end of §1, not §2 as the card says. Corrected in place.
3tier orderingConfirmed. No tiering anywhere in checklist-test. RUNNER rule 6 reads as quoted.
4done definitionConfirmed. checklist-test §4 owns the run-issue title convention and states no consulted fraction; RUNNER.md defines no title, so this correction has one home and does not diverge from the runner protocol.
5disclosure ruleConfirmed, and stronger than "silence". See below.
6?id= factual errorConfirmed against source, not just the card. See below.

Entry 5 is not merely an omission — the skill actively instructs the disclosure

checklist-author does carry the rule (「安全敏感的发现,没有维护者的决定永不公开立单」) and checklist-test carries none, as the card says. But checklist-test §4 goes further than silence: it requires 「每个 fail 一条复现规则」 in the public run issue, and RUNNER rule 2 states unconditionally that "a fail with no reproduction rule in its issue is not a completed verdict." A runner following the skill faithfully is therefore instructed to publish the reproduction — which is exactly what happened.

So the guardrail could not just be added beside those; it has to name them and override them. It does:

本条压过 §4 的「每个 fail 一条复现规则」与 RUNNER 规则 2 的同款要求

One deviation from the card's proposed wording, flagged for review: the card drafts the rule in English. checklist-test's Guardrails are Chinese throughout, and the card asks for 「the same register as 不伪造覆盖」 — an English block would not be in that register. I wrote it in Chinese, preserving every element of the drafted rule (all three publication surfaces named, the detail withheld pending maintainer record, hold-and-stop, and the closing principle that getting a defect fixed never requires handing anyone a working exploit). Substance unchanged; say the word if you want the English text instead.

Residual, and it is not mine to fix: RUNNER rule 2's unconditional sentence still lives in docs/qa/platform-checklist/RUNNER.md, which this PR must not touch. The override is stated from the skill side only. A companion one-line carve-out in RUNNER rule 2 would make the two agree at the source rather than by precedence — worth a follow-up card if you agree.

Entry 6 verified against the route, not just the card

Confirmed three independent ways rather than taking the card's word:

  • Source — the filter compares against the App document's name; App declares no id of its own.
  • Pinpackages/rest/src/meta-app-publish-gate.test.ts asserts a non-matching id returns a 200 with an empty list, not a 404.
  • Real namesshowcase_app, setup, studio, account (crm_app for the CRM example). com.objectstack.setup is a package id, which packages/cli/src/adr-0048-app-split.test.ts makes explicit by addressing the item as name setup within package com.objectstack.setup.

The error was live in the skill for a specific reason worth noting: the ?id= line sat directly beneath the /_console/apps/ examples, which legitimately do use package-shaped ids. The two are different addressing schemes one line apart, so the fix names the distinction rather than only swapping the placeholder.

Entry 2 — durable rule, measurement as the reason

Per the dispatch note, the wave's numbers are the justification, not the rule. No token count and no absolute minute figure is pinned; the text states the shape (environment dominates, cold containers spend their budget before the first verdict, roughly two thirds of areas left unrun) and the decision rule: default to reusing one provisioned tree; a cold container per round is justified only when the round must outlive the dispatching session.

Entry 3 is written the same way — proportions ("about four in ten pinned, hand-driving is about a third") rather than 77 / 62 / 47, which drifts with the ledger.

Line discipline

checklist-test 131 → 163, checklist-author 55 → 61, dogfood-verification 142 → 148. Nothing doubled; every added line is a rule, and no lessons-learned prose or "experience" section was added, per the 2026-08-12 filter. The tiering went into §1 rather than becoming a new section — I broadened §1's title instead of numbering a 1b, and confirmed nothing cross-references checklist-test §1.

Note for the record: scripts/pm/check-skill-line-ratchet.mjs does not cover these three files — its ceilings are the pm-dispatch surface plus .claude/agents/os-dev.md. I kept to the discipline anyway rather than treating the absent ceiling as headroom, and did not add ceilings here (out of scope for this card).

Verification — gate union re-run on the final commit cb8694ce9

Derived runnably from the changed paths (node scripts/pm/dispatch-gates.mjs), not recalled:

✓ check-nul-bytes: OK (6130 text files, no raw ASCII control bytes)
✓ doc authoring guard: 377 files clean — no bare metadata literals
✓ check-skill-frame-sync: 4 copies of the decision frame structurally isomorphic across 3 files
✓ check:doc-formula-expressions: 22 formula examples across 396 files / 1411 TS blocks clean

Plus a manual control-byte sweep over the three changed files (grep -naP over the C0 range): clean.

node scripts/check-adr-merge-approval.mjs could not run locally — it reached GitHub unauthenticated and got HTTP 401. Its output confirms it selected this PR correctly (.claude/skills/** touched -- consulting reviews), which is the ADR-class gate doing its job; it needs CI's credentials to render a verdict. That verdict is the human review this PR is waiting on.

No changeset: this diff touches only internal agent tooling under .claude/, publishes no package, and changes nothing user-facing — skip-changeset applied.

Related work — out of scope here

The companion card #9386 has already landed via #9427, so docs/qa/** was not in flight while this ran — the dispatch's "running in parallel" note is stale. Its RUNNER.md environment-facts section now documents the same ?id= behaviour independently, and this PR's dogfood correction agrees with it. Out of scope: #9386 and #9296 both remain untouched by this change.


Generated by Claude Code

… QA wave exposed
checklist-test gains four rules and one guardrail; checklist-author gains the
fallback half that applies to its hunter/writer fleet; dogfood-verification §1
gets the `?id=` correction.
- Subagent fallback: both skills mandate a fleet neither can guarantee exists.
A round with no Task tool may run the angles sequentially, but must DECLARE
the deviation — a silent collapse to one reader leaves the findings intact
and voids the "nothing else was missed" conclusion, with nothing in the
output distinguishing the two.
- Cold-environment cost: the dominant per-round cost is environment
provisioning, not the test items. Default to reusing one provisioned tree; a
cold container per round is justified only when the round must outlive the
dispatching session.
- Tier ordering: pinned items first, batched per area (one vitest invocation
evidences a whole area's pins), then unpinned P0/P1 by hand, then unpinned
P2 deferred. The hand-driving load is about a third of the item count.
- Done definition: an area is done only when every item has a verdict. A round
that cannot finish files what it has plus a handoff list naming the unreached
items, and its title states the consulted fraction.
- Access-control disclosure guardrail (checklist-test Guardrails): never
publish a reproduction for an authentication or authorization hole anywhere
on GitHub — not the run issue, not a tracking card, not a comment. This
overrides the per-fail reproduction-rule requirement in §4 and RUNNER rule 2,
which are otherwise unconditional; a tracking card is a public issue too.
- Factual: `?id=` on `/api/v1/meta/app` keys on the app NAME, never the package
id. A package id returns an empty items list, which reads exactly like
missing metadata.
Skills changes are ADR-class (maintainer ruling 2026-08-11:
「所有 skills 的更新和 adr 类似,需要人工审核」) — draft PR, human merge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ai9gUdihXjg7FumzsBNSSs
@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026 — with Claude
@github-actionsgithub-actionsBot added the documentation Improvements or additions to documentation label Aug 18, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM note on the red check — no fix is coming from this side, by design.

ADR maintainer approval failed on cb8694ce9. That is the gate working, not a defect: this PR touches .claude/skills/**, the gate consults reviews, finds no maintainer approval, and refuses. Its red is the accurate report of this PR's actual state — awaiting a human.

Recording the posture explicitly, because "get CI green" is the reflex this particular gate exists to defeat: the only legitimate way to turn this check green is a maintainer approval. I will not weaken it, re-scope it, exempt this path, or re-run it hoping for a different answer. Under the repo's own rules, weakening a gate is a human action — and a gate that an agent can talk its way past protects nothing.

So this PR stays draft, unmerged, unqueued, visibly awaiting review, and the wave's round report lists it under "awaiting a human merge" rather than as done.

Two things from the PR body I am acting on separately, so they are not lost in a draft that may sit for a while:

  1. The residual is real and I have verified it.RUNNER.md:55 still reads "A fail with no reproduction rule in its issue is not a completed verdict" — unconditional, on main as of ad217b192. This PR correctly could not touch docs/qa/**, so the access-control carve-out currently holds by precedence from the skill side only, and the two documents disagree at the source. Filed as a follow-up so it is fixed where it is written rather than where it is overridden.
  2. The sharper diagnosis in entry 5 is worth preserving in review. The card said checklist-test was silent on access-control disclosure. It is not silent — §4 requires a reproduction rule in the public run issue, and RUNNER rule 2 backs that unconditionally. A runner following the skill faithfully was therefore instructed to publish the reproduction, which is precisely what happened in this wave. That is a materially worse defect than an omission, and the guardrail had to name and override those two rather than sit beside them.

Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review August 18, 2026 05:24
@hotlong
hotlong added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 097fe96Aug 18, 2026
21 of 22 checks passed
@hotlong
hotlong deleted the claude/issue-9387-skills-principle-gaps branch August 18, 2026 05:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skills: five principle gaps the #9296 wave exposed in checklist-test / checklist-author / dogfood-verification

3 participants

@os-zhuang@hotlong@claude