Skip to content

ums: skill-builder must regenerate codex-skills wrappers + run validate before push - #233

Merged
dem-extra1 merged 2 commits into
mainfrom
ums-skill-builder-codex-sync
Jun 26, 2026
Merged

ums: skill-builder must regenerate codex-skills wrappers + run validate before push#233
dem-extra1 merged 2 commits into
mainfrom
ums-skill-builder-codex-sync

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

A new skill needs its generated codex-skills// wrapper committed alongside
the source, or the validate CI job fails on the wrapper-sync check. The Ship-it
steps didn't mention it — surfaced while adding the check-rendered-refs skill
(PR #231). Add scripts/sync-codex-skill-wrappers.py + scripts/validate-skills.py
to the commit block and a paragraph explaining why.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

…te before push

A new skill needs its generated codex-skills/<name>/ wrapper committed alongside
the source, or the `validate` CI job fails on the wrapper-sync check. The Ship-it
steps didn't mention it — surfaced while adding the check-rendered-refs skill
(PR #231). Add `scripts/sync-codex-skill-wrappers.py` + `scripts/validate-skills.py`
to the commit block and a paragraph explaining why.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Driving this PR to clean — back off until done.

@claude

This comment has been minimized.

Comment thread skills/skill-builder/SKILL.md Outdated
Comment thread skills/skill-builder/SKILL.md Outdated
…er in git add

Addresses PR #233 review: (1) the parenthetical quoted the success message; a
reader debugging a red CI log searches for the failure string, so quote
"Codex skill wrappers are out of sync:" instead. (2) the git add code block
staged only codex-skills/<name>; add the <alias> source + wrapper so a
copy-paste of both-skill-plus-alias doesn't push a stale alias wrapper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD round 1

Finding Disposition
Parenthetical quoted the success string Codex skill wrappers are in sync, not what shows in a red CI log Addressed — now quotes the failure line Codex skill wrappers are out of sync: (sync script line 215).
git add block staged codex-skills/<name> but omitted the alias dir/wrapper Addressed — block now stages skills/<alias>/SKILL.md + codex-skills/<alias> too.

Pushed in e2b2cca. Re-review runs on the push.

@claude

claude Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 18s —— View job


Re-review after e2b2cca

  • Read current diff
  • Verify fix 1: failure string now quoted
  • Verify fix 2: alias dir staged in git add block
  • Check for new issues

Both findings from the prior review are correctly addressed. No new issues found.

Fix 1 — verified. skills/skill-builder/SKILL.md lines 193–194 now read:

the validate CI job fails if it's out of sync (the red log reads Codex skill wrappers are out of sync:).`

This matches sync-codex-skill-wrappers.py line 215 (print("Codex skill wrappers are out of sync:")), the actual failure output. The success string (Codex skill wrappers are in sync) no longer appears in the prose.

Fix 2 — verified. The git add block (lines 179–186) now stages both canonical and alias paths:

git add skills/<name>/SKILL.md codex-skills/<name> \
        skills/<alias>/SKILL.md codex-skills/<alias> \
        memories/preferences.md

The inline comment (incl. the generated wrappers, and the alias dir if you made one) makes the conditional nature clear for skills without aliases.

No new findings. PR is clean.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Done — PR is fully clean and free.

@dem-extra1
dem-extra1 merged commit 88385a9 into main Jun 26, 2026
4 checks passed
@dem-extra1
dem-extra1 deleted the ums-skill-builder-codex-sync branch June 26, 2026 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants