fix(cross-llm-pr-a): post-merge guard fixes — set -euo pipefail, code-fence skip, SKILL.md TodoWrite#17
Conversation
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Post-merge adjustments to the skill-content grep guard to reduce false positives and align its behavior/documentation with cross-host portability rules for skills under skills/ and agents/.
Changes:
- Strengthen guard script safety defaults (
set -euo pipefail). - Extend the guard’s skip logic and allowlist to reduce false positives (fenced-code handling, additional
ALLOWED_FILES). - Update writing-skills documentation to describe fenced-code skipping.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/skill-content-grep.sh |
Tightens bash error handling and modifies AWK scanning to attempt skipping fenced code blocks; expands allowlist. |
skills/writing-skills/SKILL.md |
Documents that the guard skips fenced code blocks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| BEGIN { skip = 0; fence = 0; ln = 0 } | ||
| { | ||
| ln++ | ||
| if (/^[[:space:]]*```[[:space:]]*$/) { fence = !fence; next } | ||
| if (fence) { next } |
| The guard also skips fenced code blocks (``` ... ```) so tool names can | ||
| appear freely in example code without triggering a false positive. |
| The guard also skips fenced code blocks (``` ... ```) so tool names can | ||
| appear freely in example code without triggering a false positive. | ||
|
|
|
@copilot review |
Here's a summary of issues found and addressed (commit Bug: Fence regex didn't handle language-annotated fences Pre-existing prose token violations (exposed by the fence fix)
|
All issues from the previous review are addressed in commit
|
|
@copilot review |
No new issues found. The PR is in a clean state — |
There was a problem hiding this comment.
Pull request overview
Post-merge fixes to the skill-content grep guard and related skill docs to reduce false positives and keep host-specific tokens properly contained for cross-LLM portability.
Changes:
- Hardened
tests/skill-content-grep.sh(set -euo pipefail), added fenced-code-block skipping, and expandedALLOWED_FILES. - Updated writing-skills docs/checklist to keep
TodoWriteprose inside an exclusive<host: claude-code>block and documented the new fence-skip behavior. - Adjusted multiple skills to avoid unguarded Claude-specific tokens in host-neutral prose (e.g., wrapping
TodoWriteguidance) and updated alignment-check to reference model tiers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/skill-content-grep.sh | Strengthens guard execution and extends skip logic (fences) + allowed-file exceptions. |
| skills/writing-skills/SKILL.md | Documents fence skipping and wraps TodoWrite checklist instruction in a claude-only host block. |
| skills/subagent-driven-development/SKILL.md | Moves Claude-specific details into <host: claude-code> blocks in a few sections. |
| skills/executing-plans/SKILL.md | Rephrases step 1 to be host-neutral and gates TodoWrite instruction behind a claude-only block. |
| skills/alignment-check/SKILL.md | Switches prose from a branded model name to the balanced tier. |
Comments suppressed due to low confidence (1)
skills/alignment-check/SKILL.md:33
- This section now says to dispatch a
balanced-tier agent, but the example block still specifiesmodel: sonnet. Update the example to match the new guidance (or adjust the prose) so readers don’t get conflicting instructions.
Dispatch a `balanced`-tier agent to perform the comparison:
Agent tool (general-purpose, model: sonnet):
description: "Check alignment: design vs plan"
prompt: |
</details>
---
💡 <a href="/GoCodeAlone/claude-superpowers/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
| if (/^[[:space:]]*```[a-zA-Z0-9_+-]*[[:space:]]*$/) { fence = !fence; next } | ||
| if (fence) { next } |
| <host: claude-code> | ||
| Use TodoWrite to create one todo per task. | ||
| </host> |
| 1. Read plan, extract all tasks | ||
| <host: claude-code> | ||
| and create TodoWrite for task tracking | ||
| </host> |
There was a problem hiding this comment.
Pull request overview
Updates the repository’s portability “grep guard” and several skills/docs to prevent false positives and align host-specific tokens with the <host: claude-code> conditioning approach introduced in PR #14.
Changes:
- Harden
tests/skill-content-grep.shwithset -euo pipefailand skip ``` fenced code blocks during scanning. - Allow two Claude-specific writing reference docs to contain otherwise-forbidden tokens by adding them to
ALLOWED_FILES. - Wrap/adjust several skills’ prose to avoid unguarded Claude-specific tokens (e.g.,
TodoWrite,TeamCreate).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/skill-content-grep.sh | Adds set -euo pipefail, skips fenced code blocks, and expands ALLOWED_FILES. |
| skills/writing-skills/SKILL.md | Documents fenced-block skipping; wraps a TodoWrite instruction in <host: claude-code>. |
| skills/subagent-driven-development/SKILL.md | Moves Claude-only tool detection/instructions behind <host: claude-code> blocks. |
| skills/executing-plans/SKILL.md | Makes “task tracking” instruction host-neutral with Claude-only TodoWrite details gated. |
| skills/alignment-check/SKILL.md | Updates prose to tier terminology (balanced) for portability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -670,7 +674,9 @@ Deploying untested skills = deploying untested code. It's a violation of quality | |||
|
|
|||
| ## Skill Creation Checklist (TDD Adapted) | |||
|
|
|||
| Dispatch a `balanced`-tier agent to perform the comparison: | ||
|
|
||
| ``` | ||
| Agent tool (general-purpose, model: sonnet): |
| @@ -74,6 +78,7 @@ For each plan task, create a TaskCreate with: | |||
| Then create corresponding review tasks: | |||
| - "Review spec: <task name>" (blockedBy: implement task) | |||
| - "Review quality: <task name>" (blockedBy: spec review task) | |||
| </host> | |||
| 1. Read plan, extract all tasks | ||
| <host: claude-code> | ||
| and create TodoWrite for task tracking |
|
@copilot review |
Addressed all four findings in commit
|
|
@copilot review |
The PR is clean. All previous review findings have been addressed and Here's a summary of the full change set across the 6 commits:
|
There was a problem hiding this comment.
Pull request overview
This PR applies post-merge fixes to the skill-content portability guard and related skill docs so the guard is stricter about script failures while reducing false positives from examples/reference docs.
Changes:
- Harden
tests/skill-content-grep.shwithset -euo pipefail, add skipping of ``` fenced code blocks, and allowlist two Claude-specific writing reference docs. - Update
skills/writing-skills/SKILL.mdto document fence-skipping behavior and wrap the TodoWrite checklist instruction in a<host: claude-code>block. - Make several skills more host-neutral by replacing Claude-specific references with generic phrasing and/or wrapping Claude-only tool instructions in
<host: claude-code>blocks.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/skill-content-grep.sh | Makes the grep guard fail-fast and ignore fenced code examples; allowlists two reference docs. |
| skills/writing-skills/SKILL.md | Documents fenced-block skipping and conditions TodoWrite instruction under claude-code. |
| skills/subagent-driven-development/SKILL.md | Conditions Claude-only Agent Teams/tool guidance under claude-code and makes prose more host-neutral. |
| skills/executing-plans/SKILL.md | Replaces host-specific TodoWrite instruction with a host-neutral step + claude-code conditioned guidance. |
| skills/alignment-check/SKILL.md | Replaces model brand references with tier-based balanced wording. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot review |
No issues found. The automated review on this round generated no comments, and |
|
@copilot review |
No issues found. |
…wording The grep guard now skips fenced code blocks (3+ backtick fences with width-tracking so nested fences don't desync the toggle). Without this, any skill that shows host-specific tool names in example code would falsely trip the guard. Also tightens set -u → set -euo pipefail, adds ALLOWED_FILES entries for the two writing-skills reference docs that discuss Anthropic tool names as subject matter, and improves the Skill Creation Checklist wording to be host-neutral (outer text) with a claude-code-specific refinement in a <host: claude-code> block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352
force-pushed
the
fix/cross-llm-pr-a-guard-followup
branch
from
73542b4 to
4fedd57
Compare
|
@copilot review |
|
@copilot review |
No issues found. |
No issues found. |
There was a problem hiding this comment.
Pull request overview
This PR applies post-merge fixes to the markdown portability “grep guard” and related writing-skills documentation, reducing false positives and aligning the guard script with repo conventions.
Changes:
- Update
tests/skill-content-grep.shto useset -euo pipefail, skip fenced code blocks, and allow two Claude-specific reference docs underskills/writing-skills/. - Document the fenced-code-block skipping behavior in
skills/writing-skills/SKILL.md. - Wrap
TodoWritechecklist guidance in an exclusive<host: claude-code>block.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/skill-content-grep.sh |
Tightens bash error handling, extends AWK skipping logic to fenced code blocks, and whitelists select reference docs. |
skills/writing-skills/SKILL.md |
Documents guard behavior and adjusts checklist text so Claude-only tool references are host-gated. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The guard also skips fenced code blocks (`` ``` `` … `` ``` ``, including | ||
| opening fences with info strings like `` ```bash ``) so tool names can |
|
@copilot review |
Summary
Post-merge fixes for PR #14 (Cross-LLM PR-A) addressing 4 issues exposed after the grep guard landed on main.
Issue 1:
set -u→set -euo pipefailGuard script lacked
-eandpipefail, so errors infind/awksubcommands could silently produce false PASS. Fixed per repo convention.Issue 2: Fenced code blocks not skipped
The AWK logic only skipped
<host: claude-code>blocks. Tokens used as examples inside ``` fences (e.g.TodoWritein example code) triggered false positives. Extended AWK to skip fenced code blocks.Issue 3: Writing-skills reference docs flagged
skills/writing-skills/anthropic-best-practices.mdandpersuasion-principles.mdreference Claude model names (Sonnet,Opus,Haiku) andTodoWritein prose — these are Claude-specific reference documents by nature, not portable skill bodies. Added toALLOWED_FILES.Issue 4:
skills/writing-skills/SKILL.md:673—TodoWritein proseThe Skill Creation Checklist instruction
Use TodoWrite to create todoswas outside any host block. Wrapped in<host: claude-code>.Verification
Test plan
set -euo pipefailin guard scriptanthropic-best-practices.mdandpersuasion-principles.mdin ALLOWED_FILESSKILL.md:673wrapped in<host: claude-code>🤖 Generated with Claude Code