DOJ-3774, DOJ-3775: Add IDT CI workflow + ${CLAUDE_PLUGIN_ROOT} defensive note#17
Conversation
…sive note
DOJ-3774 — Add .github/workflows/lint.yml with three checks:
1. JSON schema syntax validation (assets/schemas/**/*.schema.json)
2. YAML frontmatter linting (commands/, skills/*/SKILL.md, agents/)
with required `description` field
3. Agent reference resolution — every ${CLAUDE_PLUGIN_ROOT}/agents/<name>.md
reference inside skills/commands must point to a file shipping in
this repo. Bare `agents/<name>.md` references are deliberately not
checked (consumer-shipped overlays like the translation pipeline use
consumer-relative paths).
Validated against post-DOJ-3773 main: all three checks pass cleanly
(3 schemas, 46 frontmatter files, 7 agents indexed). Targets the bug
class Greptile flagged across the DOJ-3708 chain.
DOJ-3775 — Add §6.1 "Plugin runtime expectations" to
assets/runtime/overlay-protocol.md plus a new failure-mode row in §6
covering empty / unresolved ${CLAUDE_PLUGIN_ROOT}. Documents that
authoring skills must emit a visible warning and skip overlay invocation
gracefully when the host runtime does not expand the token (e.g. direct
`claude` CLI invocation outside a plugin context). Contract is verified
by integration tests planned for DOJ-3710 (Phase 2).
Both issues bundled in a single PR per feedback_sequential_implementation
(no parallel branches in same repo).
Closes DOJ-3774
Closes DOJ-3775
Created by Claude Code on behalf of @andres
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptileai review |
…tion bug
The first workflow run failed with "workflow file issue" because the
indented `python3 - <<'PY'` heredoc in each step passed the script body
to Python with leading whitespace, making the body unparseable. Moving
the three checks to standalone Python scripts under scripts/ci/ keeps
the workflow YAML minimal and the scripts independently runnable
locally for development.
Behavior is identical:
- check_json_schemas.py — assets/schemas/**/*.schema.json must parse
- check_frontmatter.py — commands/, skills/*/SKILL.md, agents/ must
have either no frontmatter or a parseable mapping with `description`
- check_agent_references.py — every ${CLAUDE_PLUGIN_ROOT}/agents/<name>.md
reference must resolve
Verified locally: 3 schemas, 46 frontmatter files, 7 agents — all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
Greptile SummaryThis PR introduces a three-step CI lint workflow (JSON schema validation, YAML frontmatter linting, agent reference resolution) and extends Path to 5/5 Confidence
Confidence Score: 4/5Safe to merge; all findings are P2 and do not affect CI correctness or runtime behavior. P2s only → ceiling of 4/5. The checked counter bug produces misleading output but exit code is still correct. The regex gap and missing workflow permissions are hygiene issues with no immediate impact. scripts/ci/check_frontmatter.py (counter logic), scripts/ci/check_agent_references.py (regex coverage), .github/workflows/lint.yml (permissions + SHA pinning)
|
| Filename | Overview |
|---|---|
| .github/workflows/lint.yml | New CI workflow with 3 lint steps; missing permissions block and actions not pinned to commit SHAs (two P2 security hygiene issues). |
| scripts/ci/check_frontmatter.py | YAML frontmatter linter; checked counter incremented even when description validation fails, making the success message inaccurate. |
| scripts/ci/check_agent_references.py | Agent reference resolver; regex [a-z][a-z0-9-]+ silently skips agent names containing underscores, producing false negatives. |
| scripts/ci/check_json_schemas.py | JSON schema syntax validator; straightforward and correct — no issues found. |
| assets/runtime/overlay-protocol.md | Adds §6.1 and a new failure-mode row documenting ${CLAUDE_PLUGIN_ROOT} defensive behavior; prose is clear and well-structured. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
PR[PR / push to main] --> WF[lint workflow]
WF --> S1[Step 1: JSON schema validation]
WF --> S2[Step 2: YAML frontmatter linting]
WF --> S3[Step 3: Agent reference resolution]
S1 -->|parse error| FAIL1[Fail]
S1 -->|all valid| PASS1[N schemas valid]
S2 -->|missing/invalid description| FAIL2[Fail]
S2 -->|no frontmatter| SKIP[Skip file]
S2 -->|all valid| PASS2[N files valid]
S3 -->|ref not in agents/| FAIL3[Fail]
S3 -->|all resolve| PASS3[All refs resolve]
PASS1 & PASS2 & PASS3 --> GREEN[Workflow passes]
FAIL1 & FAIL2 & FAIL3 --> RED[Workflow fails]
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
scripts/ci/check_frontmatter.py:54-59
`checked` is incremented even when the description is invalid. When `description` is present but empty or the wrong type, the error is appended but there's no `continue` before `checked += 1`, so the success message (e.g. "46 file(s) valid") includes files that actually failed validation. The exit code is still correct, but the output is misleading and could mask issues during triage.
```suggestion
desc = fm.get('description')
if not isinstance(desc, str) or not desc.strip():
errors.append(
f'{path}: frontmatter "description" is empty or not a string'
)
continue
checked += 1
```
### Issue 2 of 4
scripts/ci/check_agent_references.py:32-34
The regex `[a-z][a-z0-9-]+` excludes underscores from agent names. If any agent file uses underscores (e.g. `agents/translation_reviewer.md`), any `${CLAUDE_PLUGIN_ROOT}/agents/translation_reviewer.md` reference in skills/commands would silently pass the check even if the file doesn't exist — a false negative in the guard this CI step is meant to provide.
```suggestion
ref_pattern = re.compile(
r'\$\{CLAUDE_PLUGIN_ROOT\}/agents/([a-z][a-z0-9_-]+)\.md'
)
```
### Issue 3 of 4
.github/workflows/lint.yml:33-35
**Missing `permissions` block** — without explicit permissions, the workflow inherits the repo's default token scope (read+write in many org setups). Since this job only reads files, locking it down to `contents: read` reduces blast radius if the workflow is ever compromised. Add a `permissions: contents: read` block at the job or workflow level.
### Issue 4 of 4
.github/workflows/lint.yml:38-42
**Actions not pinned to commit SHAs** — `actions/checkout@v4` and `actions/setup-python@v5` are mutable tags. If either action's tag is re-pointed to a malicious commit, this workflow would silently execute attacker-controlled code on every PR. Pinning to a full SHA (e.g. `actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683`) provides supply-chain integrity.
Reviews (1): Last reviewed commit: "DOJ-3774: Move lint logic to scripts/ci/..." | Re-trigger Greptile
| desc = fm.get('description') | ||
| if not isinstance(desc, str) or not desc.strip(): | ||
| errors.append( | ||
| f'{path}: frontmatter "description" is empty or not a string' | ||
| ) | ||
| checked += 1 |
There was a problem hiding this comment.
checked is incremented even when the description is invalid. When description is present but empty or the wrong type, the error is appended but there's no continue before checked += 1, so the success message (e.g. "46 file(s) valid") includes files that actually failed validation. The exit code is still correct, but the output is misleading and could mask issues during triage.
| desc = fm.get('description') | |
| if not isinstance(desc, str) or not desc.strip(): | |
| errors.append( | |
| f'{path}: frontmatter "description" is empty or not a string' | |
| ) | |
| checked += 1 | |
| desc = fm.get('description') | |
| if not isinstance(desc, str) or not desc.strip(): | |
| errors.append( | |
| f'{path}: frontmatter "description" is empty or not a string' | |
| ) | |
| continue | |
| checked += 1 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/ci/check_frontmatter.py
Line: 54-59
Comment:
`checked` is incremented even when the description is invalid. When `description` is present but empty or the wrong type, the error is appended but there's no `continue` before `checked += 1`, so the success message (e.g. "46 file(s) valid") includes files that actually failed validation. The exit code is still correct, but the output is misleading and could mask issues during triage.
```suggestion
desc = fm.get('description')
if not isinstance(desc, str) or not desc.strip():
errors.append(
f'{path}: frontmatter "description" is empty or not a string'
)
continue
checked += 1
```
How can I resolve this? If you propose a fix, please make it concise.| ref_pattern = re.compile( | ||
| r'\$\{CLAUDE_PLUGIN_ROOT\}/agents/([a-z][a-z0-9-]+)\.md' | ||
| ) |
There was a problem hiding this comment.
The regex
[a-z][a-z0-9-]+ excludes underscores from agent names. If any agent file uses underscores (e.g. agents/translation_reviewer.md), any ${CLAUDE_PLUGIN_ROOT}/agents/translation_reviewer.md reference in skills/commands would silently pass the check even if the file doesn't exist — a false negative in the guard this CI step is meant to provide.
| ref_pattern = re.compile( | |
| r'\$\{CLAUDE_PLUGIN_ROOT\}/agents/([a-z][a-z0-9-]+)\.md' | |
| ) | |
| ref_pattern = re.compile( | |
| r'\$\{CLAUDE_PLUGIN_ROOT\}/agents/([a-z][a-z0-9_-]+)\.md' | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/ci/check_agent_references.py
Line: 32-34
Comment:
The regex `[a-z][a-z0-9-]+` excludes underscores from agent names. If any agent file uses underscores (e.g. `agents/translation_reviewer.md`), any `${CLAUDE_PLUGIN_ROOT}/agents/translation_reviewer.md` reference in skills/commands would silently pass the check even if the file doesn't exist — a false negative in the guard this CI step is meant to provide.
```suggestion
ref_pattern = re.compile(
r'\$\{CLAUDE_PLUGIN_ROOT\}/agents/([a-z][a-z0-9_-]+)\.md'
)
```
How can I resolve this? If you propose a fix, please make it concise.| jobs: | ||
| lint-plugin: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Missing
permissions block — without explicit permissions, the workflow inherits the repo's default token scope (read+write in many org setups). Since this job only reads files, locking it down to contents: read reduces blast radius if the workflow is ever compromised. Add a permissions: contents: read block at the job or workflow level.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/lint.yml
Line: 33-35
Comment:
**Missing `permissions` block** — without explicit permissions, the workflow inherits the repo's default token scope (read+write in many org setups). Since this job only reads files, locking it down to `contents: read` reduces blast radius if the workflow is ever compromised. Add a `permissions: contents: read` block at the job or workflow level.
How can I resolve this? If you propose a fix, please make it concise.| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v5 | ||
| with: |
There was a problem hiding this comment.
Actions not pinned to commit SHAs —
actions/checkout@v4 and actions/setup-python@v5 are mutable tags. If either action's tag is re-pointed to a malicious commit, this workflow would silently execute attacker-controlled code on every PR. Pinning to a full SHA (e.g. actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683) provides supply-chain integrity.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/lint.yml
Line: 38-42
Comment:
**Actions not pinned to commit SHAs** — `actions/checkout@v4` and `actions/setup-python@v5` are mutable tags. If either action's tag is re-pointed to a malicious commit, this workflow would silently execute attacker-controlled code on every PR. Pinning to a full SHA (e.g. `actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683`) provides supply-chain integrity.
How can I resolve this? If you propose a fix, please make it concise.1. check_frontmatter.py — add `continue` after empty-description error so `checked` only counts files that fully passed validation. Output now reflects reality even when some files have malformed descriptions. 2. check_agent_references.py — expand regex to allow underscores in agent names ([a-z0-9_-]). The previous pattern would silently skip references to e.g. agents/translation_reviewer.md, producing false negatives in the very guard this step provides. 3. lint.yml — add `permissions: contents: read` block at workflow level. The job only reads files; locking the GITHUB_TOKEN to read-only reduces blast radius if the workflow is ever compromised. 4. lint.yml — pin actions/checkout and actions/setup-python to full commit SHAs (v4.2.2 → 11bd719..., v5.6.0 → a26af69...) instead of mutable version tags. Tags can be re-pointed; SHAs cannot. Also pin pyyaml to 6.0.2 for reproducible runs. All 4 are P2 hygiene issues per Greptile review on PR #17. Verified locally: 3 schemas, 46 frontmatter files, 7 agents — all still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
.github/workflows/lint.ymlwith 3 pre-flight checks (JSON schema validation, YAML frontmatter linting, agent reference resolution)${CLAUDE_PLUGIN_ROOT}defensive-resolution note (§6.1 + new failure-mode row) toassets/runtime/overlay-protocol.mdfeedback_sequential_implementation.md— no parallel branches in same repoFiles changed
.github/workflows/lint.yml(new, 161 LoC) — DOJ-3774assets/runtime/overlay-protocol.md(+41 LoC) — DOJ-3775DOJ-3774 detail — three CI checks
Each check is a distinct workflow step with localized error messages, designed to catch the bug class Greptile flagged across the DOJ-3708 migration chain (DOJ-3771/3772/3773):
assets/schemas/**/*.schema.jsonmust parse as valid JSON.commands/*.md,skills/*/SKILL.md,agents/*.mdeither has no frontmatter (skipped, e.g. translation pipeline support docs) or has a parseable mapping with a non-emptydescriptionfield.${CLAUDE_PLUGIN_ROOT}/agents/<name>.mdreference inside skills/commands resolves to a file shipping in this repo.Calibration note for Step 3
The agent reference check deliberately scopes to
${CLAUDE_PLUGIN_ROOT}/agents/<name>.mdpatterns and ignores bareagents/<name>.mdmentions. The latter are consumer-relative references (e.g.commands/_translation-pipeline.mdreferences consumer-shippedagents/translator.md,agents/proofreader.md,agents/translation-reviewer.mdfrom the consumer repo, not from IDT). Flagging those would produce false positives on legitimate cross-plugin patterns. The IDT-internal scope is the correct boundary.DOJ-3775 detail — defensive-resolution note
Added two pieces to
assets/runtime/overlay-protocol.md:${CLAUDE_PLUGIN_ROOT}is empty or unresolved" — graceful skip with visible warning, no crash, no silent no-op.${CLAUDE_PLUGIN_ROOT})" — documents the contract with the host runtime, the in-plugin and out-of-plugin cases, and the defensive behavior authoring skills must implement. Note explicitly references DOJ-3710 (Phase 2) integration tests as the verification mechanism.Test plan
yaml.safe_load)Out of scope
${CLAUDE_PLUGIN_ROOT}failure path — that issue owns the test coverage; this PR documents the expected behavior so authoring skills can be written defensively nowCreated by Claude Code on behalf of @andres