feat(cross-llm): add grep guard, model-tiers table, marker convention (PR-A)#14
Conversation
There was a problem hiding this comment.
Pull request overview
Foundation work for the cross-LLM portability rollout (Group A): introduces a regression guard against host-specific bleed in skills, defines shared “model tier” vocabulary across hosts, and documents the <host: …> marker convention in the writing-skills guidance.
Changes:
- Add
tests/skill-content-grep.shto flag forbidden host-specific tokens outside<host: claude-code>blocks (with allowlisted exceptions). - Add
agents/model-tiers.mdto map role-based tiers (fast/balanced/frontier/coding-specialist) to host-specific model identifiers. - Document host-conditional sections and forbidden-token policy in
skills/writing-skills/SKILL.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/skill-content-grep.sh | New grep/awk-based portability guard for skill/agent markdown content |
| skills/writing-skills/SKILL.md | Documents <host: …> marker syntax + forbidden token policy |
| agents/model-tiers.md | Central table mapping tier roles to per-host model identifiers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TOKENS=( | ||
| TodoWrite TaskCreate TaskUpdate TaskList TaskGet | ||
| TeamCreate TeamDelete SendMessage EnterPlanMode | ||
| Sonnet Opus Haiku |
There was a problem hiding this comment.
The forbidden model-name tokens are case-sensitive (grep -w), but the repo already contains lowercase model identifiers like model: "sonnet" in skills. As written, the guard will flag Sonnet but miss sonnet/opus/haiku, which undermines the portability check. Consider adding lowercase variants to TOKENS (or doing case-insensitive matching just for these model names) so existing bleed is caught consistently.
| Sonnet Opus Haiku | |
| Sonnet Opus Haiku | |
| sonnet opus haiku |
| fail=0 | ||
| tmp="$(mktemp)" | ||
| trap 'rm -f "$tmp"' EXIT | ||
|
|
||
| # Find all markdown files under skills/ and agents/. | ||
| find skills agents -type f -name '*.md' -print0 \ | ||
| | while IFS= read -r -d '' file; do | ||
| # Skip explicitly allowed files. | ||
| for allowed in "${ALLOWED_FILES[@]}"; do | ||
| if [ "$file" = "$allowed" ]; then | ||
| continue 2 | ||
| fi | ||
| done | ||
|
|
||
| # Single-pass AWK: | ||
| # - Tracks original line numbers so error output cites source locations. | ||
| # - Skips <host: ...> blocks where "claude-code" appears anywhere in the | ||
| # (possibly comma-separated) host list, e.g.: | ||
| # <host: claude-code> → skip | ||
| # <host: codex, claude-code> → skip | ||
| # <host: codex, opencode> → do NOT skip | ||
| # - Emits "LINENO:content" for every non-skipped line. | ||
| annotated="$(awk ' | ||
| BEGIN { skip = 0; ln = 0 } | ||
| { | ||
| ln++ | ||
| if (/^[[:space:]]*<host:/) { | ||
| if (/claude-code/) { skip = 1; next } | ||
| } | ||
| if (/^[[:space:]]*<\/host>/) { skip = 0; next } | ||
| if (!skip) { print ln ":" $0 } | ||
| } | ||
| ' "$file")" | ||
|
|
||
| for token in "${TOKENS[@]}"; do | ||
| # Whole-word grep. || true prevents non-zero exit on no matches from | ||
| # propagating as a script failure. | ||
| matches="$(printf '%s\n' "$annotated" | grep -w "$token" || true)" | ||
| if [ -n "$matches" ]; then | ||
| printf '%s\n' "$matches" | sed "s|^|$file:|" >> "$tmp" | ||
| fail=1 | ||
| fi |
There was a problem hiding this comment.
fail is set but never read, and because the while ...; do ...; done loop is fed by a pipe, any updates to fail happen in a subshell and wouldn’t propagate anyway. This is currently misleading dead code—either remove fail entirely (since the tmp file is the real signal), or restructure the loop (e.g., process substitution) and use fail as the exit condition.
| | Role | Claude Code | Codex | OpenCode | Cursor | | ||
| |---|---|---|---|---| | ||
| | `fast` | `haiku` | `gpt-5.4-mini` | host-pass-through | host-pass-through | | ||
| | `balanced` | `sonnet` | `gpt-5.4` | host-pass-through | host-pass-through | | ||
| | `frontier` | `opus` | `gpt-5.5` | host-pass-through | host-pass-through | | ||
| | `coding-specialist` | `sonnet` | `gpt-5.3-codex` | host-pass-through | host-pass-through | |
There was a problem hiding this comment.
The markdown table is written with a double leading pipe (|| ...), which renders as an extra empty first column in most markdown parsers. If the intent is a 5-column table, switch each row to a single leading | (and keep the trailing |) so columns line up correctly.
- grep.sh: add lowercase sonnet/opus/haiku to TOKENS (case coverage) - grep.sh: fix AWK to skip only exclusive <host: claude-code> blocks, not comma-separated multi-host tags like <host: codex, claude-code> - grep.sh: remove dead fail=1 inside while-pipe subshell - grep.sh: remove tests/skill-content-grep.sh from ALLOWED_FILES (tests/ is not scanned by find; entry was dead code) - writing-skills: clarify markers must appear on their own lines - writing-skills: document that lowercase sonnet/opus/haiku are also forbidden outside host-conditional blocks - writing-skills: note that multi-host blocks are NOT guard-exempt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # - Skips ONLY exclusive <host: claude-code> blocks (no other hosts). | ||
| # A block like <host: codex, claude-code> is NOT skipped because its | ||
| # content is also shown to codex users who must not see Claude-only tokens. | ||
| # <host: claude-code> → skip | ||
| # <host: codex, claude-code> → do NOT skip | ||
| # <host: codex, opencode> → do NOT skip |
There was a problem hiding this comment.
PR description says <host: codex, claude-code> should be skipped when claude-code appears anywhere in the host list, but the implementation (and SKILL.md guidance) only skips blocks tagged exclusively with claude-code. Please align the PR description/contract with the actual behavior, or adjust the skip logic to match what downstream PRs expect.
| ### Forbidden tokens outside markers | ||
|
|
||
| The grep guard at `tests/skill-content-grep.sh` rejects these tokens outside | ||
| a `<host: claude-code>` block: | ||
|
|
||
| `TodoWrite`, `TaskCreate`, `TaskUpdate`, `TaskList`, `TaskGet`, | ||
| `TeamCreate`, `TeamDelete`, `SendMessage`, `EnterPlanMode`, | ||
| `Sonnet`, `Opus`, `Haiku` (and their lowercase forms `sonnet`, `opus`, `haiku`). | ||
|
|
There was a problem hiding this comment.
This section lists the forbidden tokens (TodoWrite, TaskCreate, Sonnet, etc.) outside any <host: claude-code> block, but tests/skill-content-grep.sh scans skills/**.md and will flag these exact strings. To avoid the guard failing permanently (even after PR-B/C/D clean up other skills), move the literal token list into a <host: claude-code> block (or otherwise exempt this file/section explicitly).
| skips blocks tagged exclusively with `claude-code`. Nested markers are not | ||
| allowed. |
There was a problem hiding this comment.
SKILL.md states “Nested markers are not allowed”, but the grep guard doesn’t detect or error on nested <host: …> blocks (it just toggles a single skip flag). Either add explicit validation in the guard (fail with a clear message) or soften this documentation to match what is actually enforced.
| skips blocks tagged exclusively with `claude-code`. Nested markers are not | |
| allowed. | |
| skips blocks tagged exclusively with `claude-code`. Avoid nested markers; | |
| they are not supported by the documented format and are not explicitly | |
| validated by the current guard. |
| "agents/model-tiers.md" | ||
| ) | ||
|
|
||
| fail=0 |
There was a problem hiding this comment.
fail=0 is declared but never used. Remove it to avoid dead code, or wire it into the script’s exit logic if you intended to accumulate a failure flag.
| fail=0 |
| # - Markers must appear at the start of a line (after optional whitespace). | ||
| # - Emits "LINENO:content" for every non-skipped line. | ||
| annotated="$(awk ' | ||
| BEGIN { skip = 0; ln = 0 } | ||
| { | ||
| ln++ | ||
| if (/^[[:space:]]*<host:[[:space:]]*claude-code[[:space:]]*>/) { skip = 1; next } | ||
| if (/^[[:space:]]*<\/host>/) { skip = 0; next } |
There was a problem hiding this comment.
The AWK start/end marker regexes aren’t end-of-line anchored, so lines like <host: claude-code> trailing text or </host> trailing text will still be treated as markers. This contradicts the documented requirement that markers appear on their own lines; consider anchoring to EOL (allowing only trailing whitespace) for both open/close tags.
| # - Markers must appear at the start of a line (after optional whitespace). | |
| # - Emits "LINENO:content" for every non-skipped line. | |
| annotated="$(awk ' | |
| BEGIN { skip = 0; ln = 0 } | |
| { | |
| ln++ | |
| if (/^[[:space:]]*<host:[[:space:]]*claude-code[[:space:]]*>/) { skip = 1; next } | |
| if (/^[[:space:]]*<\/host>/) { skip = 0; next } | |
| # - Markers must appear on their own lines (after optional leading | |
| # whitespace, with only optional trailing whitespace). | |
| # - Emits "LINENO:content" for every non-skipped line. | |
| annotated="$(awk ' | |
| BEGIN { skip = 0; ln = 0 } | |
| { | |
| ln++ | |
| if (/^[[:space:]]*<host:[[:space:]]*claude-code[[:space:]]*>[[:space:]]*$/) { skip = 1; next } | |
| if (/^[[:space:]]*<\/host>[[:space:]]*$/) { skip = 0; next } |
- grep.sh: remove dead fail=0 variable (no longer used after subshell fix) - grep.sh: add end-of-line anchors ($) to AWK host-marker regexes so lines like '<host: claude-code> trailing text' are NOT treated as markers - writing-skills: change 'Nested markers are not allowed' to 'not supported — behaviour is undefined' to match actual guard behaviour (no detection) - writing-skills: wrap forbidden-tokens list in <host: claude-code> block so the guard does not flag the documentation file that lists them - PR description: updated to clarify exclusive-only skip rule (<host: codex, claude-code> is NOT skipped) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Foundation work for the cross-LLM portability rollout (Group A):
tests/skill-content-grep.sh— AWK-based regression guard that exits 1 if a forbidden host-specific token appears inskills/oragents/outside an allowed context. Guard skips ONLY blocks tagged exclusively with<host: claude-code>(markers must occupy the whole line). Multi-host blocks like<host: codex, claude-code>are NOT skipped because their content is also shown to non-Claude users. Tokens: Claude-Code tool names + model-brand names in both title-case and lowercase.agents/model-tiers.md— Role vocabulary table (fast/balanced/frontier/coding-specialist) resolved to host-specific model identifiers. Skills cite roles; hosts resolve to brand names.skills/writing-skills/SKILL.md— Documents the<host: …>marker convention: syntax, recognised host names, when to use vs host-neutral phrasing, and the forbidden-token list (wrapped in<host: claude-code>so the guard doesn't flag it).Runtime launch transcript
Build: n/a (bash script + markdown)
Launch:
Failure-signature scrape: clean (script exits with expected non-zero on current tree)
Verdict: PASS (guard detects existing violations correctly; Group I skills produce no output)
🤖 Generated with Claude Code