chore(ci): update skills - #214
Conversation
- fix missing skill details - align pre commit hooks to pnpm scripts - fix agent cloud env var - add project authoring skill for standing up new projects Signed-off-by: Cory Rylan <crylan@nvidia.com>
github-actions
Bot
added
scope(ci)
scope(internals)
scope(cli)
dependencies
📝 WalkthroughWalkthroughChangesRepository tooling and guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Author
participant ScaffoldCLI
participant Repository
participant NewProject
Author->>ScaffoldCLI: provide project name, reference, and description
ScaffoldCLI->>Repository: read tracked reference files and integration targets
ScaffoldCLI->>NewProject: create normalized project files
ScaffoldCLI->>Repository: update workspace, CI, site, metadata, lint, and Knip configuration
ScaffoldCLI-->>Author: report planned or applied changes
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/agent-upkeep/SKILL.md:
- Line 141: Update the verification guidance around the “Done when” criterion
and the commands in the verification section to consistently cover all
applicable project scripts, including optional test:lighthouse and test:visual
invocations, or narrow the completion criterion to only the listed commands.
Ensure users cannot satisfy the procedure while omitting suites that the
repository guidance identifies as applicable.
In @.agents/skills/authoring-documentation/SKILL.md:
- Line 31: Update the Pre-commit bullet in the documentation to insert a space
after the colon, so the text reads “lint-staged): automatically” while
preserving the rest of the wording.
In @.agents/skills/authoring-projects/scripts/scaffold-project.mjs:
- Around line 20-22: Update the description validation in scaffold-project’s
argument-validation flow to reject values containing the JSDoc terminator
sequence */ before file planning or generation. Preserve the existing
single-line check and fail through the established fail validation path with a
clear --description error.
- Around line 392-406: Update the catch block in applyChanges to remove every
path from changesToApply that is absent from originals, while retaining
restoration of existing files and projectDirectory cleanup. Add a regression
test that forces a write failure after the documentation page is created and
verifies the newly created page is removed.
In @.agents/skills/authoring-projects/SKILL.md:
- Around line 41-48: The verification checklist in
.agents/skills/authoring-projects/SKILL.md lines 41-48 must include the metadata
generation command alongside the existing project checks. Update the next-step
output in .agents/skills/authoring-projects/scripts/scaffold-project.mjs lines
65-68 to instruct users to run metadata generation before final verification.
- Line 50: Update the CI guidance in the numbered workflow step so it explicitly
distinguishes changes requiring broader CI from changes covered by project-local
checks: require broader CI for changes affecting shared infrastructure, build or
release configuration, cross-project interfaces, generated artifacts, or
multiple packages, while allowing targeted project checks for isolated changes.
Preserve the existing final-diff inspection requirements and follow the
repository’s technical writing style.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b1a785a0-94a7-406c-ac04-579cb464f94d
📒 Files selected for processing (12)
.agents/hooks/pre-tool-use-bash.sh.agents/skills/agent-upkeep/SKILL.md.agents/skills/authoring-documentation/SKILL.md.agents/skills/authoring-projects/SKILL.md.agents/skills/authoring-projects/references/integration-checklist.md.agents/skills/authoring-projects/scripts/scaffold-project.mjs.agents/skills/guidance-build-system/SKILL.md.husky/pre-commitNOTICE.mdpackage.jsonprojects/cli/NOTICE.mdprojects/internals/BUILD.md
| This is success, not failure. It means your fix worked and the stale entry is still on disk. Resolve it by pruning. Do **not** revert your fix, do not re-run `--suppress-rule` to make the message go away, and do not add `--pass-on-unpruned-suppressions`, which only hides the condition and leaves the ratchet slipping backwards. | ||
|
|
||
| **Done when**: the rule's suppression count strictly decreases, no stale suppressions remain, the file's public API is byte-identical, `pnpm run lint` passes with no new inline disables, and `pnpm run test` and `pnpm run test:types` pass. | ||
| **Done when**: the rule's suppression count strictly decreases, no stale suppressions remain, the file's public API is byte-identical, and every applicable project verification script passes with no new inline disables. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run every applicable project test script before marking the task done.
Line 141 requires every applicable project verification script, but Lines 174-187 list only lint, test, test:types, and component test:axe/test:ssr. The repository guidance also lists test:lighthouse and test:visual. A user can complete this procedure without running those suites. Add optional invocations for these scripts, or narrow the completion criterion to the commands this procedure covers.
Suggested verification additions
mise exec -- pnpm run --if-present test
mise exec -- pnpm run --if-present test:types
+mise exec -- pnpm run --if-present test:lighthouse
+mise exec -- pnpm run --if-present test:visualAlso applies to: 174-187
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 59: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 120: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 189: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 261: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 262: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 263: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 264: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[error] 68: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/agent-upkeep/SKILL.md at line 141, Update the verification
guidance around the “Done when” criterion and the commands in the verification
section to consistently cover all applicable project scripts, including optional
test:lighthouse and test:visual invocations, or narrow the completion criterion
to only the listed commands. Ensure users cannot satisfy the procedure while
omitting suites that the repository guidance identifies as applicable.
| ### How Vale Runs | ||
|
|
||
| - **Pre-commit** (via lint-staged):automatically lints staged `*.ts` and `*.md` files | ||
| - **Pre-commit** (via lint-staged):automatically lints staged non-test `*.ts` and `*.md` files |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a space after the colon.
Line 31 renders lint-staged):automatically. Change it to lint-staged): automatically.
As per coding guidelines, Markdown documentation files are linted by Vale and should follow the repository's technical writing style.
Suggested wording fix
-- **Pre-commit** (via lint-staged):automatically lints staged non-test `*.ts` and `*.md` files
+- **Pre-commit** (via lint-staged): automatically lints staged non-test `*.ts` and `*.md` files📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Pre-commit** (via lint-staged):automatically lints staged non-test `*.ts` and `*.md` files | |
| - **Pre-commit** (via lint-staged): automatically lints staged non-test `*.ts` and `*.md` files |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/authoring-documentation/SKILL.md at line 31, Update the
Pre-commit bullet in the documentation to insert a space after the colon, so the
text reads “lint-staged): automatically” while preserving the rest of the
wording.
Source: Coding guidelines
| assertKebabCase(name, '--name'); | ||
| assertKebabCase(referenceName, '--reference'); | ||
| if (description.includes('\n')) fail('--description must be a single line.'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject JSDoc terminators in --description.
A value containing */ passes validation. Line 171 inserts that value into a JSDoc line. The generated component can then contain invalid TypeScript. Reject this sequence before planning files.
Proposed fix
-if (description.includes('\n')) fail('--description must be a single line.');
+if (description.includes('\n')) fail('--description must be a single line.');
+if (description.includes('*/')) fail('--description must not contain "*/".');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assertKebabCase(name, '--name'); | |
| assertKebabCase(referenceName, '--reference'); | |
| if (description.includes('\n')) fail('--description must be a single line.'); | |
| assertKebabCase(name, '--name'); | |
| assertKebabCase(referenceName, '--reference'); | |
| if (description.includes('\n')) fail('--description must be a single line.'); | |
| if (description.includes('*/')) fail('--description must not contain "*/".'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/authoring-projects/scripts/scaffold-project.mjs around lines
20 - 22, Update the description validation in scaffold-project’s
argument-validation flow to reject values containing the JSDoc terminator
sequence */ before file planning or generation. Preserve the existing
single-line check and fail through the established fail validation path with a
clear --description error.
| function applyChanges(changesToApply) { | ||
| const originals = new Map(); | ||
| for (const [absolutePath] of changesToApply) { | ||
| if (existsSync(absolutePath)) originals.set(absolutePath, readFileSync(absolutePath, 'utf8')); | ||
| } | ||
|
|
||
| try { | ||
| for (const [absolutePath, content] of changesToApply) { | ||
| mkdirSync(path.dirname(absolutePath), { recursive: true }); | ||
| writeFileSync(absolutePath, content, 'utf8'); | ||
| } | ||
| } catch (error) { | ||
| for (const [absolutePath, content] of originals) writeFileSync(absolutePath, content, 'utf8'); | ||
| rmSync(projectDirectory, { recursive: true, force: true }); | ||
| throw error; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove every newly created file during rollback.
originals contains only existing files. If a write fails after Line 401 creates projects/site/src/docs/<name>/index.md, Lines 404-405 restore updates and remove only projects/<name>. The documentation page remains and blocks the next scaffold attempt. Remove each planned path that was not in originals.
Proposed fix
} catch (error) {
for (const [absolutePath, content] of originals) writeFileSync(absolutePath, content, 'utf8');
+ for (const [absolutePath] of changesToApply) {
+ if (!originals.has(absolutePath)) rmSync(absolutePath, { force: true });
+ }
rmSync(projectDirectory, { recursive: true, force: true });
throw error;
}Add a regression test that forces a write failure after the documentation page is created.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function applyChanges(changesToApply) { | |
| const originals = new Map(); | |
| for (const [absolutePath] of changesToApply) { | |
| if (existsSync(absolutePath)) originals.set(absolutePath, readFileSync(absolutePath, 'utf8')); | |
| } | |
| try { | |
| for (const [absolutePath, content] of changesToApply) { | |
| mkdirSync(path.dirname(absolutePath), { recursive: true }); | |
| writeFileSync(absolutePath, content, 'utf8'); | |
| } | |
| } catch (error) { | |
| for (const [absolutePath, content] of originals) writeFileSync(absolutePath, content, 'utf8'); | |
| rmSync(projectDirectory, { recursive: true, force: true }); | |
| throw error; | |
| function applyChanges(changesToApply) { | |
| const originals = new Map(); | |
| for (const [absolutePath] of changesToApply) { | |
| if (existsSync(absolutePath)) originals.set(absolutePath, readFileSync(absolutePath, 'utf8')); | |
| } | |
| try { | |
| for (const [absolutePath, content] of changesToApply) { | |
| mkdirSync(path.dirname(absolutePath), { recursive: true }); | |
| writeFileSync(absolutePath, content, 'utf8'); | |
| } | |
| } catch (error) { | |
| for (const [absolutePath, content] of originals) writeFileSync(absolutePath, content, 'utf8'); | |
| for (const [absolutePath] of changesToApply) { | |
| if (!originals.has(absolutePath)) rmSync(absolutePath, { force: true }); | |
| } | |
| rmSync(projectDirectory, { recursive: true, force: true }); | |
| throw error; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/authoring-projects/scripts/scaffold-project.mjs around lines
392 - 406, Update the catch block in applyChanges to remove every path from
changesToApply that is absent from originals, while retaining restoration of
existing files and projectDirectory cleanup. Add a regression test that forces a
write failure after the documentation page is created and verifies the newly
created page is removed.
| 7. Run the project checks listed in its `DEVELOPMENT.md`, then verify the repository integrations: | ||
|
|
||
| ```shell | ||
| mise exec -- pnpm -C projects/<component-name> run ci | ||
| mise exec -- pnpm -C projects/<component-name> run test:lighthouse | ||
| mise exec -- pnpm -C projects/site run build | ||
| mise exec -- pnpm run lint:knip | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Run metadata generation before final verification.
The checklist requires metadata build as a minimum check. The workflow and CLI completion message omit it. Users can finish with stale metadata API output.
.agents/skills/authoring-projects/SKILL.md#L41-L48: add the metadata generation command to the verification block..agents/skills/authoring-projects/scripts/scaffold-project.mjs#L65-L68: add metadata generation to the printed next-step list.
📍 Affects 2 files
.agents/skills/authoring-projects/SKILL.md#L41-L48(this comment).agents/skills/authoring-projects/scripts/scaffold-project.mjs#L65-L68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/authoring-projects/SKILL.md around lines 41 - 48, The
verification checklist in .agents/skills/authoring-projects/SKILL.md lines 41-48
must include the metadata generation command alongside the existing project
checks. Update the next-step output in
.agents/skills/authoring-projects/scripts/scaffold-project.mjs lines 65-68 to
instruct users to run metadata generation before final verification.
| mise exec -- pnpm run lint:knip | ||
| ``` | ||
|
|
||
| 8. Run broader CI in proportion to the change. Inspect the final diff for generated artifacts, missing build edges, copied visual baselines, and unrelated lockfile churn. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Define the broader CI selection rule.
“In proportion to the change” does not define which changes require broader CI. State when to run broader CI and when project checks are sufficient.
Proposed wording
-8. Run broader CI in proportion to the change. Inspect the final diff for generated artifacts, missing build edges, copied visual baselines, and unrelated lockfile churn.
+8. Run broader CI when changes affect shared repository integrations. For local project changes, run the affected project checks. Inspect the final diff for generated artifacts, missing build edges, copied visual baselines, and unrelated lockfile churn.As per coding guidelines, Markdown documentation files must follow the repository's technical writing style, including Google developer documentation style and write-good rules with project-specific customizations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 8. Run broader CI in proportion to the change. Inspect the final diff for generated artifacts, missing build edges, copied visual baselines, and unrelated lockfile churn. | |
| 8. Run broader CI when changes affect shared repository integrations. For local project changes, run the affected project checks. Inspect the final diff for generated artifacts, missing build edges, copied visual baselines, and unrelated lockfile churn. |
🧰 Tools
🪛 LanguageTool
[style] ~50-~50: ‘in proportion to’ might be wordy. Consider a shorter alternative.
Context: ...run lint:knip ``` 8. Run broader CI in proportion to the change. Inspect the final diff for ...
(EN_WORDINESS_PREMIUM_IN_PROPORTION_TO)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/authoring-projects/SKILL.md at line 50, Update the CI
guidance in the numbered workflow step so it explicitly distinguishes changes
requiring broader CI from changes covered by project-local checks: require
broader CI for changes affecting shared infrastructure, build or release
configuration, cross-project interfaces, generated artifacts, or multiple
packages, while allowing targeted project checks for isolated changes. Preserve
the existing final-diff inspection requirements and follow the repository’s
technical writing style.
Sources: Coding guidelines, Linters/SAST tools
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores