fix(scripts): add agent reference repair runtime#632
Open
rafaelscosta wants to merge 3 commits intomainfrom
Open
fix(scripts): add agent reference repair runtime#632rafaelscosta wants to merge 3 commits intomainfrom
rafaelscosta wants to merge 3 commits intomainfrom
Conversation
rafaelscosta
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
area: agents
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a legacy-aware skill ID helper and validation changes, makes IDE sync respect an optional projectRoot, and introduces a new repair script to remove legacy agent/skill artifacts, re-sync artifacts, and validate resulting state. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI/Entry Point
participant Script as repair-agent-references
participant AgentLoader as Agent Metadata Loader
participant ArtifactBuilder as Legacy Artifact Builder
participant FileSystem as File System
participant SyncEngine as Sync Engine
participant Validators as Validators
participant Reporter as Report Generator
CLI->>Script: Execute with options
Script->>Script: Parse args & resolve paths
Script->>AgentLoader: Load agents from source
AgentLoader-->>Script: Agent metadata
Script->>ArtifactBuilder: Build legacy artifacts list
ArtifactBuilder-->>Script: Artifacts to remove
Script->>FileSystem: Remove or record artifacts (dry-run skips)
FileSystem-->>Script: Removed/missing records
Script->>SyncEngine: Run syncSkills & commandSync
SyncEngine-->>Script: Regenerated artifacts
Script->>Validators: Validate local/global skills & Gemini commands
Validators-->>Script: Validation results
Script->>Reporter: Aggregate and format report (human/JSON)
Reporter-->>CLI: Output result & set exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js (1)
68-71:⚠️ Potential issue | 🟡 MinorEarly return is missing the
legacyfield.The early return when
skillsDirdoesn't exist omits the newly addedlegacyarray, which could cause downstream code to fail if it expects that property.Proposed fix
if (!fs.existsSync(resolved.skillsDir)) { errors.push(`Skills directory not found: ${resolved.skillsDir}`); - return { ok: false, checked: 0, expected: 0, errors, warnings, missing: [], orphaned: [] }; + return { ok: false, checked: 0, expected: 0, errors, warnings, missing: [], orphaned: [], legacy: [] }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js around lines 68 - 71, The early return when resolved.skillsDir is missing returns an object without the new legacy property; update the return value in the skills directory existence check to include legacy: [] alongside the existing properties (ok, checked, expected, errors, warnings, missing, orphaned) so downstream code that reads the legacy array (from this validation flow) won't break—locate the check using resolved.skillsDir and the surrounding errors.push call in validate.js and add the legacy field to the returned object.
🧹 Nitpick comments (2)
.aiox-core/infrastructure/scripts/repair-agent-references.js (2)
42-44: Minor duplication:isParsableAgentis defined in both files.This helper is identical to the one in
validate.js(Line 32-34). Consider importing it fromvalidate.jsor extracting it to a shared utility to avoid drift.Option: Import from validate.js
Update
validate.jsto exportisParsableAgent:module.exports = { validateCodexSkills, validateSkillContent, parseArgs, getDefaultOptions, isParsableAgent, // Add this export };Then in
repair-agent-references.js:-const { validateCodexSkills } = require('./codex-skills-sync/validate'); +const { validateCodexSkills, isParsableAgent } = require('./codex-skills-sync/validate'); -function isParsableAgent(agent) { - return !agent.error || agent.error === 'YAML parse failed, using fallback extraction'; -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/infrastructure/scripts/repair-agent-references.js around lines 42 - 44, Duplicate helper isParsableAgent exists in both validate.js and repair-agent-references.js; remove the duplicate by exporting isParsableAgent from validate.js (add isParsableAgent to module.exports alongside validateCodexSkills, validateSkillContent, parseArgs, getDefaultOptions) and update repair-agent-references.js to import/require and use that exported isParsableAgent instead of redefining it (or alternatively move isParsableAgent into a new shared utility module and require it from both places).
91-109: Consider wrappingfs.removeSyncin try/catch.If
removeSyncfails (e.g., permission denied, file locked), the script will throw and abort mid-cleanup. Wrapping in try/catch would allow partial cleanup to complete and report failures in the result.Proposed enhancement
function removeLegacyArtifacts(artifacts, options) { const removed = []; const skipped = []; + const failed = []; for (const artifact of artifacts) { if (!fs.existsSync(artifact.path)) { skipped.push({ ...artifact, reason: 'not-found' }); continue; } if (!options.dryRun) { - fs.removeSync(artifact.path); + try { + fs.removeSync(artifact.path); + } catch (error) { + failed.push({ ...artifact, reason: error.message }); + continue; + } } removed.push(artifact); } - return { removed, skipped }; + return { removed, skipped, failed }; }Then update
repairAgentReferencesandformatHumanReportto handle thefailedarray.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/infrastructure/scripts/repair-agent-references.js around lines 91 - 109, The removal loop in removeLegacyArtifacts currently calls fs.removeSync directly and can throw, aborting the whole run; wrap the fs.removeSync call in a try/catch to catch failures (when options.dryRun is false), push failed removals into a new failed array (include artifact and error/reason), continue processing other artifacts, and return { removed, skipped, failed }; then update repairAgentReferences and formatHumanReport to accept and surface the failed array (show artifact path and error/reason) so failures are reported alongside removed/skipped results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js:
- Around line 68-71: The early return when resolved.skillsDir is missing returns
an object without the new legacy property; update the return value in the skills
directory existence check to include legacy: [] alongside the existing
properties (ok, checked, expected, errors, warnings, missing, orphaned) so
downstream code that reads the legacy array (from this validation flow) won't
break—locate the check using resolved.skillsDir and the surrounding errors.push
call in validate.js and add the legacy field to the returned object.
---
Nitpick comments:
In @.aiox-core/infrastructure/scripts/repair-agent-references.js:
- Around line 42-44: Duplicate helper isParsableAgent exists in both validate.js
and repair-agent-references.js; remove the duplicate by exporting
isParsableAgent from validate.js (add isParsableAgent to module.exports
alongside validateCodexSkills, validateSkillContent, parseArgs,
getDefaultOptions) and update repair-agent-references.js to import/require and
use that exported isParsableAgent instead of redefining it (or alternatively
move isParsableAgent into a new shared utility module and require it from both
places).
- Around line 91-109: The removal loop in removeLegacyArtifacts currently calls
fs.removeSync directly and can throw, aborting the whole run; wrap the
fs.removeSync call in a try/catch to catch failures (when options.dryRun is
false), push failed removals into a new failed array (include artifact and
error/reason), continue processing other artifacts, and return { removed,
skipped, failed }; then update repairAgentReferences and formatHumanReport to
accept and surface the failed array (show artifact path and error/reason) so
failures are reported alongside removed/skipped results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4be2f5c4-1d05-4413-9ea7-97f947f12e3a
📒 Files selected for processing (5)
.aiox-core/infrastructure/scripts/codex-skills-sync/index.js.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js.aiox-core/infrastructure/scripts/ide-sync/index.js.aiox-core/infrastructure/scripts/repair-agent-references.jspackage.json
rafaelscosta
commented
Apr 16, 2026
Collaborator
Author
rafaelscosta
left a comment
rafaelscosta
left a comment
There was a problem hiding this comment.
Re-checking mergeability after code owner approval.
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
1 participant
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.
Runtime-only publish for the agent-reference repair flow.
Scope is intentionally limited to five runtime files:
Intent:
aios-*aliases for core agentsaiox-*Codex/Gemini artifactsnpm run repair:agent-referencesValidation:
git apply --checkon the runtime-only patch: passnpm run lint: pass with existing repo baseline of 236 warnings, 0 errorsnpm run typecheck: passnpm test: 1 failing test intests/integration/onboarding-smoke.test.jsthat expects the legacy greetingAgent dev loaded; the branch now produces the canonical Dex greeting. This is outside the runtime-only publish scope and was not included in the remote diff.Unrelated dirty files were intentionally excluded from this PR.
Summary by CodeRabbit