fix(implement,todo-check): apply authoritative-text-rules in todo-check - #230
Open
ultimatile wants to merge 25 commits into
Open
ultimatile wants to merge 25 commits into
ultimatile wants to merge 25 commits into
Conversation
todo-check now applies authoritative-text-rules conditionally when the scope description names surfaces that could hold text an agent executes as instructions, reading its items in main context so the writer holds those guarantees while the plan is still revisable. done-check retains its independent fresh-context audit unchanged. Callers describing todo-check's coverage as quality-list-only (frontmatter description, README skill-table row, three implement/SKILL.md sentences) are generalized to point at whichever rule sets todo-check applies.
…ription - Compress single-reading and clause-composition quick-reference rows to mnemonic level, per authoritative-text-rules SSOT: "a runner may carry a compressed mnemonic of an item, but never the item's full trigger, sweep, concern conditions, or N/A criterion". The two rows previously inlined the item bodies' sweep enumerations. - Split the intro paragraph's shared firing-rule clause into per-runner arms: done-check on its own diff-path firing rule at its Step 2, todo-check on the scope-description firing rule at Step 3. - Replace "dispatch this rule set" with "apply this rule set" — dispatch in these skill bodies means spawn a fresh-context subagent, but the authoritative-text preview stays in main context. - Bind the <rule-set> placeholder in the quick-reference intro to the two named rule sets explicitly. - Align the top-of-file "Item definitions live in `quality-list`" line with the widened rule-set coverage, mirroring done-check's wording.
… lane vocabulary - Fold the authoritative-text-rules SSOT presence check into the "when the firing rule fires" branch. Previously read as unconditional, which would halt a preflight whose scope never activated the rule set (e.g. a code-only unit in a repo where the SSOT is not installed). - Replace Step 5's "single-lane `?` row" domain descriptor and pass-2 arm with "`?` row that is not a half of a dual-lane row". The prior wording carried quality-list's lane vocabulary; authoritative-text rows have no lane tag, so no literal arm matched them. Exclusion-based descriptor covers both cleanly and lets the pass-1 auth-text arm added earlier reach its recording site.
"the drafting sweep" in the single-reading quick-reference row had no
antecedent — could read as either a self-review during drafting or
done-check's later audit sweep. Rewrite to name the action explicitly
("plan to hold the ambiguity-guard checkpoints during drafting").
Previous rewording of the Step 5 domain and pass-2 arm from "single-lane" to "not a half of a dual-lane row" was intended to accommodate lane-tag-less authoritative-text rows, but a row is never a half — halves are its internal parts. A ported-code-attribution return of mechanical ⊘ / contextual ? produces a dual-lane `?` row at Step 4; the reworded domain would double-count it (pass 1 arm 1 and pass 2 arm 2 both firing without stated precedence). Drop "a half of" from both descriptors: "a `?` row that is not a dual-lane row" now covers authoritative-text rows (which are not dual-lane) while excluding dual-lane rows as intended.
Trim explanatory prose the code-review gate kept surfacing: - Firing rule for authoritative-text-rules: replace the enumerated instruction-file conventions with a delegation to that SSOT's Scope section. The copy would drift when the SSOT adds a new convention and the firing rule would silently skip scopes that only name the new form. - Drop the "the rule approximates because it has to" rationale paragraph and the presence-check redundant clause. The retained sentences state what to do; the cost-side reasoning was narrative. - Drop the Step 3 tail rationale explaining why authoritative-text items are read in main context — already covered in the intro paragraph. - Step 5 pass-1 preamble: "by the lane of the check being settled" → "by the row type" (auth-text rows have no lane tag). - Step 4 merge sentence: name all three row sources — mechanical subagent, quality-list contextual, and — when the firing rule fired — authoritative-text. The old wording named only the first two, out of sync with the append clause below. - Output-format tail: defer to Step 4 for the assembled table rather than restating the row-domain rule for both rule sets. Closes the drift-prone duplication a prior done-check noted. - clause-composition mnemonic: replace "plan the rule-set fix" (ambiguous between "identify the rule set" and "plan the edit to it") with plain wording pointing at the item body for both steps.
… qualifier
Two injections from earlier fixes:
- Quick-reference intro dropped the "contextual" qualifier when the
paragraph was widened to cover both rule sets. Reads as if Step 3
loads any quality-list body in main context, contradicting Step 2's
prohibition on loading purely-mechanical bodies there. Restore the
qualifier for the quality-list path.
- Firing rule referenced authoritative-text-rules' Scope section for
the qualification test. Reader would need that SSOT loaded to
evaluate — but the presence check comes after the firing rule, so
an uninstalled SSOT would raise raw file-not-found before reaching
the halt Step 0 defined. Match done-check's path-level heuristic
("could be text an agent executes as instructions"); the SSOT's
Scope section decides authoritatively inside the pass, after the
presence check has cleared.
Move the activation decision for `authoritative-text-rules` into Step 0 alongside language detection, reading the same plan or task description input. Rename the step to "Resolve the active rule sets" (plural). Two independent activation conditions, either sufficient: - the description names a path qualifying under the SSOT's Scope section (skill body, rule / item file, `CLAUDE.md` / `AGENTS.md`, or a file under `.claude/rules|commands|agents/**`); or - the description declares the change will write new authoritative-text content. Do not activate on uncertainty. `done-check` catches surfaces the description missed at its own Step 2. Step 3 processes authoritative-text items in main context when Step 0 activated the rule set; Step 4 appends those rows below the `quality-list` rows. Line 10 records the residual runner-input asymmetry: `done-check` inspects the diff's paths, `todo-check` reads the description.
Step 1's mid-implementation paragraph stated a contract on the plan or task description (naming authoritative-text surfaces the current unit will touch), but Step 0's activation had already read the input by then. Move the caller-side obligation into Step 0's activation section where the timing matches when the decision is made.
…lock - Line 12's pointer for the mid-implementation contract retargets from Step 1 to Step 0 to match where the contract now lives. - Line 29's "do not attempt to widen the list here" was two-way readable: as "do not edit the inline gloss", and as "do not activate on surfaces outside the gloss". Rephrase as "quick-reference gloss of its recurring instances, not an exhaustive replacement — an edge surface the gloss omits is still admitted when the Scope section admits it". - Step 4's code block scope was implicit in a single line at its top. Add a one-line header stating the block covers `quality-list` and `authoritative-text-rules` is appended after.
…hrasing Rephrase Step 0 activation condition 1: "the description names a path that qualifies under the SSOT's Scope section — a skill body, a rule or item definition file, `CLAUDE.md` / `AGENTS.md`, a file under `.claude/rules/`, `.claude/commands/`, `.claude/agents/`, or an equivalent an agent tool defines". The previous "a path sits under a location — a skill body" mixed containers with file kinds under one verb. Drop the caller-side invocation-input contract paragraph. Activation reads the plan or task description; a description too thin to activate `authoritative-text-rules` is thin for the whole preflight, not activation specifically, so the paragraph stated an obligation the runner cannot enforce and had no runtime arm when unmet. Drop line 12's pointer to that paragraph.
The restriction "Read only the contextual-lane bodies — plus ported-code-attribution — not a purely mechanical-lane item's body" was written when `quality-list` was the only rule set here. `authoritative-text-rules` items have no lane tag and would be blocked by the exhaustive permission. Scope the restriction to `quality-list`, and reference the paragraph below where `authoritative-text-rules` bodies are read on their own rule.
…t Step 3 - Line 29 stated the SSOT's Scope section is the authority and "an edge surface the gloss omits is still admitted when Scope admits it". That read as an instruction to open the SSOT file at Step 0, which the presence check does not gate until after activation fires. Rephrase: Step 0's activation reads only the inline gloss; a surface outside the gloss stays inactive here, and `done-check` catches it at its own Step 2. - Line 134 called Step 0's admission an "approximate reading", which invited reading Step 3 as a filter that could contradict Step 0's decision. Rephrase: Step 0 used the gloss, this pass consults the full Scope section, and the filter is the per-file classification Scope defines (a path under an admitted directory that is a lockfile or script comes out N/A).
There was a problem hiding this comment.
🟡 Changes recommended
Address the three unresolved moderate findings in the implementation and preflight activation wording.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Extends todo-check to preflight authoritative-text rules when applicable, with related documentation and version updates.
Changes:
- Adds conditional authoritative-text preflight logic and mnemonics.
- Generalizes
implementpreflight handling. - Updates README documentation.
- Bumps marketplace version to
2026.9.8.
File summaries
| File | Summary |
|---|---|
skills/todo-check/SKILL.md |
Adds authoritative-text preflight behavior. |
skills/implement/SKILL.md |
Generalizes preflight item handling. |
README.md |
Updates the todo-check description. |
.claude-plugin/marketplace.json |
Bumps the plugin version. |
Review details
Suppressed comments (2)
skills/implement/SKILL.md:46
- This still gates the caller on a code change. For a skill/rule/
CLAUDE.md-only implementation there may be no code change, so/implementcan skiptodo-checkentirely and never produce the new authoritative-text setup rows. Invoke the preflight before any implementation change, not only before code changes.
Before any code change, invoke `todo-check` against the plan, passing the root stated above, to extract the active preflight items and their setup actions for this work. Hand the resulting △ rows to `quaere-execution`'s Plan step so the unit checks already include them. A preflight that halts emits no table at all, so there are no △ rows to hand on; it has surfaced something to the user, and the unit does not start. Re-invoke `todo-check` between units when the next unit changes the active item set (e.g., it introduces a new public API → `impact-verification` and `paired-artifact-drift` become active).
skills/todo-check/SKILL.md:246
- All five authoritative-text item bodies trigger on additions, modifications, and removals, but these mnemonics frame the setup only around writing or emitting. For a planned deletion (for example, removing a routing branch or a consuming step), the corresponding sweep is not actually requested, so preflight can miss the item's setup work. Phrase each mnemonic to cover adding, modifying, and removing the governed text.
- **`case-space-totality`** — Before writing condition→outcome authoritative text, enumerate the domain axes the rule branches on; plan for each cell to reach exactly one outcome and for mirrored cases to be treated symmetrically or excluded with a reason.
- **`single-reading`** — Before writing sentences in authoritative text, plan to hold the ambiguity-guard checkpoints during drafting; the item body holds the catalogue.
- **`clause-composition`** — Before editing a clause in a rule set holding more than one unit, plan the inbound + outbound reference sweep; the item body defines how to identify the rule set and derive the search keys.
- **`executor-fitness`** — Before writing a step that names an executor, list what the step will demand and quote the written definition of the executor's inputs; plan to reconcile any gap before writing the step.
- **`consumer-closure`** — Before emitting a value or imposing an obligation, freeze the emitted-value / obligation list up front and plan to identify each consuming step or receiver.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…b axes Three consistency fixes across the preflight surface: - `implement` gated `todo-check` on "any code change". A skill body / rule file / agent-instruction change is not a code change and would skip the preflight, so authoritative-text setup rows would never materialize for that class of implementation. Broaden the trigger to cover code edits, skill-body edits, rule-file edits, and agent- instruction file edits. - `todo-check` activation condition 1's inline gloss ended with "or an equivalent an agent tool defines". That clause admits cross-tool surfaces at Step 0 while the rest of the design leaves them to `done-check`. Drop the clause so the gloss is a closed set of Claude surfaces; cross-tool equivalents fall to `done-check` per its independent diff-time check. - `todo-check`'s quick-reference mnemonics were phrased "Before writing / emitting", covering additions only. The five `authoritative-text- rules` items' triggers cover additions, modifications, and removals. Rephrase each mnemonic to name all three axes so a planned deletion invokes the corresponding sweep.
"Before any implementation change" is the trigger.
The four-item enumeration that followed
("a code edit, a skill-body edit, a rule-file edit,
or an agent-instruction file edit")
invites a case-space-totality read
without narrowing or widening what the trigger already says.
The frontmatter `description` is a routing hint for skill selection. The "Framed as 'what to set up so done-check's rows pass at the end'" clause is a mental model, not routing information; it also has a consumer in the SKILL body's paragraph pairing todo-check with done-check's audit question.
"There the two runners diverge on receiver:" introduced a coined "receiver" term whose only definition was the colon-list that followed it. Merge the two into a single sentence anchored on the rule set: "For `authoritative-text-rules`, `done-check` spawns a second fresh-context subagent ...; `todo-check` reads those items in main context ...".
Removed nine sentences and sentence-tails whose "so what?" answer for the executor was none: - the two intro paragraphs contrasting `todo-check` with `done-check` and stating the runner-input asymmetry. - Step 0 activation's "surface outside the gloss stays inactive ... done-check catches it" and its "worth the item bodies it loads" motivation tail. - Step 1 mid-implementation's "preflight runs against work that does not exist yet" preflight-semantics gloss and the `done-check` comparison after it. - Step 2 header's "fresh context removes the author's blindspot" motivation and the "prompt carries the two absolute paths" restated paragraph. - Step 3 header's "conversation / research history the subagent lacks" motivation and the "These need plan / intent / review history" restatement. - Quick-reference intro's "compressed mnemonic of the lens-shift" meta and the "subagent never reads this file, so a mnemonic would have no consumer" why on the mechanical-item absence.
There was a problem hiding this comment.
🔵 Needs a closer look
The unresolved moderate trigger issue can violate the intended code-only/no-load behavior.
Review details
Suppressed comments (1)
skills/todo-check/SKILL.md:22
- This path-only trigger activates the rule set for every file under
.claude/rules/,.claude/commands/, or.claude/agents/, but the SSOT classifies per file and explicitly excludes source code and lockfiles. A code-only plan such as.claude/commands/build.pytherefore makes Step 3 read all five authoritative-text item bodies, violating the code-only/no-load behavior and the “do not activate on uncertainty” contract. Require the description to identify the named file as agent-executed instruction text before this directory trigger fires.
- the description names a path that qualifies under the SSOT's Scope section — a skill body, a rule or item definition file, `CLAUDE.md` / `AGENTS.md`, or a file under `.claude/rules/`, `.claude/commands/`, `.claude/agents/`; or
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Address the two unresolved moderate findings in skills/todo-check/SKILL.md.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
skills/todo-check/SKILL.md:119
- This branch conflates "no surface is known to qualify" with a settled non-authoritative scope. Step 1 says an unsettled scope fact must surface as
? unknown, but a plan can leave whether a file contains agent-executed instructions unresolved; the current logic then emits N/A rows, skips all item bodies, and never asks for the missing scope decision. Distinguish a known exclusion from an unresolved classification and route the latter through the existing?/Step 5 flow, explicitly stating which way uncertainty errs.
**Process `authoritative-text-rules` items.** Read `<SKILLS_DIR>/authoritative-text-rules/SKILL.md`'s Scope section and apply it per-file to Step 1's paths and content descriptions. If no surface qualifies, emit five ⊘ N/A rows (one per item in the SSOT's Items index) with reason "no authoritative-text surface in scope", and do not open any item body. If some surface qualifies, read the Items index and each item's `<SKILLS_DIR>/authoritative-text-rules/items/<slug>.md` body, and determine △ active / ⊘ N/A / ? unknown per item on the same terms as the `quality-list` contextual items above.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate findings and one nit remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
skills/implement/SKILL.md:46
- This replacement removes the warning that the final audit is driven by the actual diff rather than only by the plan. That asymmetry still exists: a later implementation can add an authoritative-text file that Step 1 did not name, so todo-check will not preview it while done-check will audit it. Without a generalized reminder here, implementers can treat the preflight rows as exhaustive.
Before any implementation change, invoke `todo-check` against the plan, passing the root stated above, to extract the active preflight items and their setup actions for this work. Hand the resulting △ rows to `quaere-execution`'s Plan step so the unit checks already include them. A preflight that halts emits no table at all, so there are no △ rows to hand on; it has surfaced something to the user, and the unit does not start. Re-invoke `todo-check` between units when the next unit changes the active item set (e.g., it introduces a new public API → `impact-verification` and `paired-artifact-drift` become active).
skills/todo-check/SKILL.md:119
- This hardcodes the no-surface result to five rows, but
authoritative-text-rulesdeclares its Items index to be the single source of truth and requires runners to derive the item set from it. Adding or removing an item would make this branch emit an incomplete or overlong report; render one N/A row per item currently read from the index instead.
**Process `authoritative-text-rules` items.** Read `<SKILLS_DIR>/authoritative-text-rules/SKILL.md`'s Scope section and apply it per-file to Step 1's paths and content descriptions. If no surface qualifies, emit five ⊘ N/A rows (one per item in the SSOT's Items index) with reason "no authoritative-text surface in scope", and do not open any item body. If some surface qualifies, read the Items index and each item's `<SKILLS_DIR>/authoritative-text-rules/items/<slug>.md` body, and determine △ active / ⊘ N/A / ? unknown per item on the same terms as the `quality-list` contextual items above.
skills/todo-check/SKILL.md:119
- Applying the SSOT Scope verbatim here reintroduces the cross-tool activation that the earlier review thread removed from todo-check: the Scope section includes “the equivalents other tools define” (for example, other agents' instruction-file locations). A plan naming such a file will now load authoritative-text preflight bodies, although the established contract leaves those equivalents to done-check's own path-based dispatch. Keep todo-check's activation input narrowed to the intended Claude surfaces, or update that contract and the PR notes consistently.
**Process `authoritative-text-rules` items.** Read `<SKILLS_DIR>/authoritative-text-rules/SKILL.md`'s Scope section and apply it per-file to Step 1's paths and content descriptions. If no surface qualifies, emit five ⊘ N/A rows (one per item in the SSOT's Items index) with reason "no authoritative-text surface in scope", and do not open any item body. If some surface qualifies, read the Items index and each item's `<SKILLS_DIR>/authoritative-text-rules/items/<slug>.md` body, and determine △ active / ⊘ N/A / ? unknown per item on the same terms as the `quality-list` contextual items above.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
…count - Step 3's `authoritative-text-rules` branch now names the direction it errs on a Scope classification Step 1's description cannot settle: treat the file as not qualifying, and let `done-check` catch it against the actual file content. This makes #227's error-direction requirement explicit. - Replace "emit five ⊘ N/A rows" with "emit an ⊘ N/A row per item in the SSOT's Items index"; the SSOT's index is the authoritative source of the item set, and a hardcoded count drifts if items are added or removed.
There was a problem hiding this comment.
🔵 Needs a closer look
Clarify Step 0’s authoritative-text activation wording so it is tied to the Scope result.
Review details
Suppressed comments (1)
skills/todo-check/SKILL.md:113
- Step 0 always resolves and presence-checks
authoritative-text-rules; it does not define a conditional activation. Consequently, “when Step 0 activated that rule set” can be read as requiring these bodies on every code-only preflight, contradicting the next paragraph's no-qualifying-file path and its no-item-body guarantee. Tie this read to the Scope result instead.
For each selected contextual item, `Read` its `<SKILLS_DIR>/quality-list/items/<slug>.md` body — plus every `lang-<lang>.md` addendum section for a language Step 0 detected, self-loaded here — before deciding its status. From `quality-list`, read only the contextual-lane bodies — plus `ported-code-attribution`'s own body, which the undeclared-port half is decided from even though the item is index-tagged `mechanical (+ contextual half)` — not a purely mechanical-lane item's body. `authoritative-text-rules` bodies are read on their own rule stated below when Step 0 activated that rule set. For each, determine one of:
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
…in Step 3 The sentence "`authoritative-text-rules` bodies are read on their own rule stated below when Step 0 activated that rule set." carried old activation vocabulary. Step 0 no longer defines a conditional activation, so the conditional read as always-true and clashed with the next paragraph's guarantee that no item body is opened when Scope admits no file. The next paragraph makes the actual conditions explicit, so the linking sentence is not needed.
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate findings concern the runner-input caveat and contradictory body-loading instruction.
Review details
Suppressed comments (3)
skills/implement/SKILL.md:46
- The generalized preflight sentence removed the only caveat that
done-checkmay apply a rule set that this plan-level preflight did not preview. The two runners still use different inputs: this preflight classifies the planned descriptions, whiledone-checkaudits the actual diff, so an implementation can add an authoritative-text surface that has no preflight setup row. The PR description says this distinction should remain explicit; retain a short warning here.
Before any implementation change, invoke `todo-check` against the plan, passing the root stated above, to extract the active preflight items and their setup actions for this work. Hand the resulting △ rows to `quaere-execution`'s Plan step so the unit checks already include them. A preflight that halts emits no table at all, so there are no △ rows to hand on; it has surfaced something to the user, and the unit does not start. Re-invoke `todo-check` between units when the next unit changes the active item set (e.g., it introduces a new public API → `impact-verification` and `paired-artifact-drift` become active).
skills/todo-check/SKILL.md:231
- The new quick-reference bullets copy the authoritative items' triggers and core sweep/concern conditions (for example, add/modify/remove, domain cells, mirrored cases, and emitted-obligation tracing).
authoritative-text-rules/SKILL.md:8permits only a compressed mnemonic and explicitly forbids copying full triggers or sweeps, because that creates a manual-sync surface. Keep these as short setup mnemonics and leave the detailed procedure to each item body.
- **`case-space-totality`** — Before adding, modifying, or removing condition→outcome authoritative text, enumerate the domain axes the rule branches on; plan for each cell to reach exactly one outcome and for mirrored cases to be treated symmetrically or excluded with a reason.
- **`single-reading`** — Before adding, modifying, or removing sentences in authoritative text, plan to hold the ambiguity-guard checkpoints during drafting; the item body holds the catalogue.
- **`clause-composition`** — Before adding, modifying, or removing a clause in a rule set holding more than one unit, plan the inbound + outbound reference sweep; the item body defines how to identify the rule set and derive the search keys.
- **`executor-fitness`** — Before adding, modifying, or removing a step that names an executor, list what the step will demand and quote the written definition of the executor's inputs; plan to reconcile any gap before making the change.
- **`consumer-closure`** — Before adding, modifying, or removing text that emits a value or imposes an obligation, freeze the emitted-value / obligation list up front and plan to identify each consuming step or receiver.
skills/todo-check/SKILL.md:208
- This quick-reference sentence says Step 3 reads every authoritative-text item body, but the processing rule above explicitly skips all of those bodies when Scope admits no planned file. That contradiction can cause a code-only preflight to load the authoritative item bodies, violating the conditional/no-body path described in this PR. State the Scope-admitted condition in this sentence as well.
A row is **not** the applicability authority and decides nothing: Step 3 reads each item's body (`<SKILLS_DIR>/quality-list/items/<slug>.md` for a `quality-list` contextual item, `<SKILLS_DIR>/authoritative-text-rules/items/<slug>.md` for an authoritative-text one) plus any applicable addendum, and that — with the index as the item set — decides whether it applies. Consult a row for its setup framing once the body has marked the item active.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
…onics, condition body-read on Scope
Three fixes:
- `implement` Step 3.0 gained a one-line caveat that a file the diff
carries but the plan does not name has no preflight row here; the
Step 4 `done-check` catches it against the diff. The two runners still
read different inputs, and that gap needs a visible mention on the
caller side.
- The quick-reference mnemonics for the `authoritative-text-rules`
items restated the SSOT's triggers ("Before adding, modifying, or
removing …") and concern conditions ("each cell to reach exactly one
outcome and mirrored cases treated symmetrically …"). The SSOT
explicitly permits only a compressed mnemonic. Shorten each to a
single-line preflight action leaving triggers, sweep, and concern
conditions to the item body.
- The quick-reference sentence that describes what Step 3 reads
claimed it opens each authoritative-text item body unconditionally.
Qualify it: only when Scope admits at least one file in the planned
change.
There was a problem hiding this comment.
🟡 Changes recommended
The authoritative-text Items index must be explicitly read before the no-qualifying-surface branch.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
… on Scope The Items index read was inside the "some file qualifies" branch, so the "no file qualifies" branch had no written step to discover its row domain even though it must emit one row per index entry. Move the index read to the shared setup; item body reads stay inside the qualifying branch so a code-only preflight still opens no item body.
…led branch Two why fragments on the Scope-unsettled directive: - "Scope classifies by content, and description alone may not decide" — why the description does not settle the classification. - "— `done-check` catches it against the actual file content" — reassurance about another skill's behavior. Neither adds an action for the executor; the directive "treat that file as not qualifying here" stands on its own.
The five-item mnemonic transcription duplicated the `authoritative-text- rules` SSOT and introduced the manual-sync surface the SSOT itself forbids in its line-8 rule. The quick-reference section stays `quality-list`-specific; the `authoritative-text-rules` item bodies Step 3 loads already carry each item's audit form, and no separate preflight lens-shift is required in this file.
There was a problem hiding this comment.
🔵 Needs a closer look
A moderate unresolved comment requests concise preflight setup mnemonics for authoritative-text items.
Review details
Suppressed comments (1)
skills/todo-check/SKILL.md:119
- Step 3 only says to read each authoritative-text audit body and classify it; it never supplies the preflight reading/setup that the issue requires. Those bodies define post-hoc audit sweeps, and removing all authoritative-text entries from
Preflight framing per itemleaves the author to invent the lens for an active row. This also misreads the SSOT:skills/authoritative-text-rules/SKILL.md:8explicitly permits a compressed runner mnemonic as long as it does not copy the full trigger, sweep, concern conditions, or N/A criterion. Add one concise preflight mnemonic per indexed item (or explicitly record why an item has no setup) before treating this branch as a complete preflight.
**Process `authoritative-text-rules` items.** Read `<SKILLS_DIR>/authoritative-text-rules/SKILL.md`'s Scope section and its Items index; then apply Scope per-file to Step 1's paths and content descriptions. When Step 1's description does not settle whether a named file qualifies, treat that file as not qualifying here. If no file qualifies, emit an ⊘ N/A row per item in the Items index with reason "no authoritative-text surface in scope", and do not open any item body. If some file qualifies, read each item's `<SKILLS_DIR>/authoritative-text-rules/items/<slug>.md` body and determine △ active / ⊘ N/A / ? unknown per item on the same terms as the `quality-list` contextual items above.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
todo-checkpreviously appliedquality-listonly, whiledone-checkapplies bothquality-listandauthoritative-text-rules. A change touching a skill body or an agent-instruction file could therefore reachdone-checkwith no preflight setup for its authoritative-text items. This PR extendstodo-checkto applyauthoritative-text-rulesalongsidequality-list, letting the SSOT's Scope section own the per-file admissibility decision.Closes #227. Refs #185.
Changes
skills/todo-check/SKILL.md:quality-listat itsSKILL.mdpluslang-<lang>.mdaddenda;authoritative-text-rulesat itsSKILL.md) and presence-checks both.authoritative-text-rules' Scope section and its Items index; then applies Scope per-file to Step 1's paths and content descriptions. When no file qualifies, Step 3 emits an ⊘ N/A row per item in the Items index with reason "no authoritative-text surface in scope" and opens no item body — so a code-only preflight loads no authoritative-text body. When some file qualifies, Step 3 reads each item's body and determines△ active/⊘ N/A/? unknownper item on the same terms as thequality-listcontextual items.authoritative-text-rulesrows below thequality-listrows unconditionally, with no coverage check or lane merging.?authoritative-text row in main context to△ activewith a setup action or⊘ N/Awith a reason, the same two outcomes it uses for a?quality-listcontextual row.skills/implement/SKILL.md: Step 3.0 / 3.0.2 / 3.3 phrasing generalized from "quality-listitems" to "preflight items", and Step 3.0 records that a file the diff carries but the plan does not name has no preflight setup row here — the Step 4done-checkcatches it against the diff.README.md: Thetodo-checkskill row now reads "Preflight sweep of quality-list and authoritative-text-rules items, before / during implementation"..claude-plugin/marketplace.json: Version bump 2026.9.7 → 2026.9.8.Impact
Callers of
todo-check(currentlyimplement) receive△setup rows forauthoritative-text-rulesitems when Step 1's paths or content descriptions include an authoritative-text surface admitted by that rule set's Scope section. Code-only planned scopes stay unchanged: Step 3 emits an⊘ N/Arow perauthoritative-text-rulesitem and opens no item body when Scope admits nothing, so no such body reaches main context (#185's constraint).Verification
The repository ships skill bodies and has no test suite, so verification is textual.
pre-commit run --from-ref main --to-ref HEADreportsmdformatpassed;ruff checkandruff formatskipped for no files to check.python3 -c "import json; print(json.load(open('.claude-plugin/marketplace.json'))['metadata']['version'])"prints2026.9.8, one patch abovemain's2026.9.7.rg -n 'authoritative-text-rules/SKILL\.md' skills/todo-check/SKILL.mdreturns three hits: Step 0's presence check, Step 3's Scope-and-index read, and Step 4's row-index reference.todo-checkrow ofREADME.mdreads "Preflight sweep of quality-list and authoritative-text-rules items, before / during implementation".Notes
todo-checkanddone-checksee different inputs — the plan or task description vs. the diff's paths. A file the eventual diff will carry but the plan does not name will not surface anyauthoritative-text-rulesfinding in preflight;done-checkcatches it against the diff.Plan-vs-actual delta
Scope items 1–3 (trigger and error direction, receiver for lane-less items, preflight reading per item) are addressed in Changes above.
Scope item 4 (whether these items get rows in
Preflight framing per item) was answered no in this PR: noauthoritative-text-rulesmnemonic rows are added to that quick-reference section, which staysquality-list-specific. An earlier version of this branch added five ATR mnemonics for symmetry with the existingquality-listlist; that transcription duplicates the SSOT and creates a manual-sync surface the SSOT itself forbids, so the addition was reverted. The pre-existingquality-listquick-reference exhibits the same drift risk and is filed as #232 for follow-up.Scope item 5 (whether a run whose
researchrule-set baseline already loaded the set re-reads it here) is answered by the implementation, not by an explicit sentence: Step 3 reads the SSOT's Scope section, its Items index, and its item bodies as needed, regardless of whether an earlier session loaded them.Out of scope (W1b, W2, the five item definitions themselves) is honored. Every Acceptance criterion is met, including the residual runner-input asymmetry which is stated in the Notes section above rather than left as a bare fact.