Skip to content

fix: harden legacy skill retirement gate [Story PRO-14.5] - #723

Merged
rafaelscosta merged 3 commits into
mainfrom
feat/pro-14-5-legacy-shim-retirement
May 10, 2026
Merged

fix: harden legacy skill retirement gate [Story PRO-14.5]#723
rafaelscosta merged 3 commits into
mainfrom
feat/pro-14-5-legacy-shim-retirement

Conversation

@rafaelscosta

@rafaelscosta rafaelscosta commented May 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • harden the legacy Codex skill retirement gate in the sync and validation flow
  • add regression coverage for Codex skill sync and validation behavior
  • document the legacy slash-command shim retirement path for PRO-14.5

Validation

  • npm ci
  • npm run sync:skills:codex
  • npm run validate:codex-skills:self-test
  • npm run validate:codex-sync
  • npm run validate:codex-integration PASS with existing generated extra warning
  • npm run sync:ide:check
  • npm run validate:gemini-sync
  • npm run validate:gemini-integration PASS with existing .gemini/rules.md warning
  • npm run lint
  • npm run typecheck
  • npm test -- --runInBand --forceExit
  • npm run build unavailable: no build script in this repo
  • npm run validate:publish
  • git diff --check

Notes

  • Existing warnings noted above are non-blocking and predate this PR handoff.

Summary by CodeRabbit

  • Documentation

    • Added migration guide for retiring legacy slash-command shims; documented removal criteria, compatibility window, and rollback steps.
    • Updated starter commands in skill docs: added *task, *workflow; removed *exit, *update-manifest.
  • Chores

    • Improved skill synchronization and stricter validation to better detect legacy aliases, duplicate payloads, and intentional redirects.
    • Normalized legacy skill identifier handling to ensure consistent canonical/legacy alias mapping.
  • Tests

    • Expanded integration and unit tests to cover new validation behaviors and legacy-alias classifications.

Review Change Stack

Review Change Stack

@vercel

vercel Bot commented May 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
aiox-core Ready Ready Preview, Comment May 10, 2026 1:22am

Request Review

@github-actions github-actions Bot added type: test Test coverage and quality area: docs Documentation (docs/) labels May 10, 2026
@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e3a9cc50-ee19-42d0-9b2b-dcc406796236

📥 Commits

Reviewing files that changed from the base of the PR and between 4ed1fb6 and 4e7b7c4.

📒 Files selected for processing (1)
  • tests/integration/codex-skills-sync.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/integration/codex-skills-sync.test.js

Walkthrough

Normalizes aios-aiox- IDs, adds content-based validation and classification for legacy alias directories (recording legacyAliases and duplicatePayloads), updates tests to cover new classifications, and adds migration/retirement documentation and manifest metadata updates.

Changes

Legacy Skill Alias Migration & Validation

Layer / File(s) Summary
Skill ID Normalization
.aiox-core/infrastructure/scripts/codex-skills-sync/index.js
getSkillId now converts incoming aios- identifiers to aiox-; getLegacySkillId produces aios- legacy IDs preserving aios-, converting aiox-aios-, and prefixing unprefixed IDs with aios-.
Validation Helper Functions
.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js
Added filesystem-safe readers and content inspectors: hasFullActivationPayload, hasLegacyAliasIntent, getThinLegacyRedirectIssues, isIntentionalLegacyAlias, and classifyLegacySkillAlias to identify full activation payloads and classify legacy alias directories.
Strict Validation Logic
.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js
Strict mode builds legacyID and canonical-path maps, classifies each legacy alias (fatal vs non-fatal), emits errors/warnings accordingly, and appends classifications to legacyAliases.
Duplicate Payload Detection
.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js
Orphaned aiox-*/aios-* dirs with full activation payloads are matched to expected canonical skills, recorded in duplicatePayloads, and reported as errors.
Validation Result Shape
.aiox-core/infrastructure/scripts/codex-skills-sync/validate.js
Validation results (early-return and normal paths) now include legacyAliases and duplicatePayloads arrays.
Unit Tests
tests/unit/codex-skills-validate.test.js
Assertions updated to expect legacyAliases and duplicatePayloads in missing-directory results and to assert legacy-alias classification entries for strict-mode tests.
Integration Tests
tests/integration/codex-skills-sync.test.js
Added/expanded tests: getSkillId('aios-dev')aiox-dev; strict-mode tests for duplicate full-payload rejection, intentional redirect acceptance, non-thin alias rejection, and orphaned duplicate detection.
Migration Gate Documentation
docs/migration/PRO-14.5-legacy-slash-command-shim-retirement.md
New gate document listing inventory, blocking rules (legacy payloads, duplicates/orphans), removal criteria, compatibility window, validation evidence, and rollback steps.
Configuration & Metadata
.codex/skills/aiox-master/SKILL.md, .aiox-core/install-manifest.yaml
Updated starter commands (*task, *workflow added; *exit, *update-manifest removed) and refreshed manifest generated_at, file SHA256 hashes, and sizes for changed scripts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

area: agents, area: core

Suggested reviewers

  • Pedrovaleriolopez
  • oalanicolas
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: hardening the legacy skill retirement gate for PRO-14.5, which is the primary objective reflected throughout the changeset (validation logic updates, documentation, and test coverage).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pro-14-5-legacy-shim-retirement

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage Report

Coverage report not available

📈 Full coverage report available in Codecov


Generated by PR Automation (Story 6.1)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/integration/codex-skills-sync.test.js (1)

7-15: ⚡ Quick win

Use absolute imports in the updated test import block.

The new/modified imports still use relative paths. Please switch these to absolute-style resolution to match repo conventions.

♻️ Proposed update
 const {
   syncSkills,
   buildSkillContent,
   getSkillId,
   getLegacySkillId,
-} = require('../../.aiox-core/infrastructure/scripts/codex-skills-sync/index');
+} = require(path.join(
+  process.cwd(),
+  '.aiox-core/infrastructure/scripts/codex-skills-sync/index',
+));
 const {
   validateCodexSkills,
-} = require('../../.aiox-core/infrastructure/scripts/codex-skills-sync/validate');
+} = require(path.join(
+  process.cwd(),
+  '.aiox-core/infrastructure/scripts/codex-skills-sync/validate',
+));

As per coding guidelines, **/*.{js,jsx,ts,tsx}: Use absolute imports instead of relative imports in all code.

🤖 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 `@tests/integration/codex-skills-sync.test.js` around lines 7 - 15, The test
imports are using relative paths; replace them with your project's
absolute-style module resolution so the imports for syncSkills,
buildSkillContent, getSkillId, getLegacySkillId, and validateCodexSkills come
from the repository's absolute import roots (e.g., the package/module alias used
across the repo) instead of '../../.aiox-core/…' paths; update the require(...)
calls to the corresponding absolute module specifiers to match repo conventions
and lint rules.
🤖 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.

Nitpick comments:
In `@tests/integration/codex-skills-sync.test.js`:
- Around line 7-15: The test imports are using relative paths; replace them with
your project's absolute-style module resolution so the imports for syncSkills,
buildSkillContent, getSkillId, getLegacySkillId, and validateCodexSkills come
from the repository's absolute import roots (e.g., the package/module alias used
across the repo) instead of '../../.aiox-core/…' paths; update the require(...)
calls to the corresponding absolute module specifiers to match repo conventions
and lint rules.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5e5bb181-e7e0-4713-a32f-ef5f7f7ecd6c

📥 Commits

Reviewing files that changed from the base of the PR and between 27af655 and 0ec1c66.

📒 Files selected for processing (7)
  • .aiox-core/infrastructure/scripts/codex-skills-sync/index.js
  • .aiox-core/infrastructure/scripts/codex-skills-sync/validate.js
  • .aiox-core/install-manifest.yaml
  • .codex/skills/aiox-master/SKILL.md
  • docs/migration/PRO-14.5-legacy-slash-command-shim-retirement.md
  • tests/integration/codex-skills-sync.test.js
  • tests/unit/codex-skills-validate.test.js

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 10, 2026
coderabbitai[bot]
coderabbitai Bot previously requested changes May 10, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/integration/codex-skills-sync.test.js`:
- Around line 13-15: Replace the relative require for validateCodexSkills with
the project-root absolute path and do the same for the other relative import on
the preceding line; specifically change
require('../../.aiox-core/infrastructure/scripts/codex-skills-sync/validate') to
require('@aiox-core/infrastructure/scripts/codex-skills-sync/validate') so the
symbol validateCodexSkills is imported via the absolute module name (also update
the other require on the line above to use the equivalent '@aiox-core/...'
absolute path).
🪄 Autofix (Beta)

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: CHILL

Plan: Pro

Run ID: 8d40135c-69aa-4fac-a9c6-99966343dd52

📥 Commits

Reviewing files that changed from the base of the PR and between 0ec1c66 and 4ed1fb6.

📒 Files selected for processing (4)
  • .aiox-core/infrastructure/scripts/codex-skills-sync/validate.js
  • .aiox-core/install-manifest.yaml
  • docs/migration/PRO-14.5-legacy-slash-command-shim-retirement.md
  • tests/integration/codex-skills-sync.test.js
✅ Files skipped from review due to trivial changes (1)
  • .aiox-core/install-manifest.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .aiox-core/infrastructure/scripts/codex-skills-sync/validate.js

Comment on lines +13 to +15
const {
validateCodexSkills,
} = require('../../.aiox-core/infrastructure/scripts/codex-skills-sync/validate');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify relative imports in this test file.
# Expected: matches indicate imports that should be migrated to absolute style.
rg -nP "require\\(['\"]\\.{1,2}/" tests/integration/codex-skills-sync.test.js

Repository: SynkraAI/aiox-core

Length of output: 231


Convert relative imports to absolute imports.

Lines 13–15 (and line 12) use relative require paths, which violates the project's absolute import requirement for JavaScript files. Update both imports to use absolute paths from the project root.

const {
  validateCodexSkills,
} = require('@aiox-core/infrastructure/scripts/codex-skills-sync/validate');
🤖 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 `@tests/integration/codex-skills-sync.test.js` around lines 13 - 15, Replace
the relative require for validateCodexSkills with the project-root absolute path
and do the same for the other relative import on the preceding line;
specifically change
require('../../.aiox-core/infrastructure/scripts/codex-skills-sync/validate') to
require('@aiox-core/infrastructure/scripts/codex-skills-sync/validate') so the
symbol validateCodexSkills is imported via the absolute module name (also update
the other require on the line above to use the equivalent '@aiox-core/...'
absolute path).

@rafaelscosta
rafaelscosta dismissed coderabbitai[bot]’s stale review May 10, 2026 01:26

Resolved in 4e7b7c4; imports now use process.cwd() path.join style and CI is green.

@rafaelscosta

Copy link
Copy Markdown
Collaborator Author

PRO-14.5 is ready for codeowner review.

Current head: 4e7b7c4

Gates:

  • CI green after final push: CodeQL, ESLint, TypeScript, Jest Node 18/20/22/24/25, install manifest, IDE sync, compatibility parity, brownfield/install smoke, dependency validation, Pro machine ID stability, SYNAPSE benchmark, Vercel and CodeRabbit status.
  • Architect re-review: PASS. The earlier thin-redirect blocker was fixed in 4ed1fb6.
  • QA gate: PASS.
  • CodeRabbit CHANGES_REQUESTED was stale and dismissed after the import fix in 4e7b7c4.

Remaining blocker is only repository rule/codeowner review from @oalanicolas and/or @Pedrovaleriolopez.

@rafaelscosta
rafaelscosta merged commit c25b66e into main May 10, 2026
40 checks passed
@rafaelscosta
rafaelscosta deleted the feat/pro-14-5-legacy-shim-retirement branch May 10, 2026 15:49
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 5.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

tuanmedeiros pushed a commit to tuanmedeiros/aios-core-synkraay that referenced this pull request Jun 2, 2026
* fix: harden legacy skill retirement gate [Story PRO-14.5]

* fix: enforce thin legacy skill aliases [Story PRO-14.5]

* test: align codex skill imports [Story PRO-14.5]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation (docs/) released type: test Test coverage and quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant