Repository navigation
fix(workos-widgets): make SKILL.md frontmatter valid YAML - #61
Conversation
…alid YAML The workos-widgets description was a plain scalar containing ": ", which strict YAML parsers (js-yaml 4, yaml 2, PyYAML/apm) reject. Rewrite it as a folded block scalar with the parsed text unchanged, and add a spec that strictly parses every tracked SKILL.md frontmatter and requires a non-empty name and description. Fixes #58
release-please rewrites plugins/workos/.codex-plugin/plugin.json on every release (0.8.1, 0.8.2) and expands its short arrays, which leaves format:check red. Ignore it like CHANGELOG.md and pnpm-lock.yaml.
 [Low impact] The changes stay within skill metadata checks and formatter scope. The YAML fix appears safe to merge, with a non-blocking gap in automated test coverage. Findings
Fix with agent prompt### Issue 1
scripts/tests/skill-frontmatter.spec.ts:26-31
The new test runs through `pnpm test`, but no GitHub workflow runs that command. The pull-request workflow only checks the PR title, and the release workflow builds and publishes without running tests. Another invalid description can therefore merge and ship without this check catching it. Add a pull-request job that installs dependencies and runs `pnpm test`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryFixes the Widgets skill’s YAML description without changing its wording.
Acknowledgment: Riker requested the formatter exclusion because release automation rewrites the manifest. The supplied description attributes approval to the author, Reviews (1) · Last reviewed commit: "chore: let oxfmt skip the release-bot-wr..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| describe('SKILL.md frontmatter', () => { | ||
| it('finds tracked SKILL.md files', () => { | ||
| expect(skillFiles.length, 'git ls-files found no SKILL.md files').toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it.each(skillFiles)('%s parses as strict YAML with a name and description', (path) => { |
There was a problem hiding this comment.
The new test runs through pnpm test, but no GitHub workflow runs that command. The pull-request workflow only checks the PR title, and the release workflow builds and publishes without running tests. Another invalid description can therefore merge and ship without this check catching it. Add a pull-request job that installs dependencies and runs pnpm test.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/tests/skill-frontmatter.spec.ts
Line: 26-31
Comment:
**CI skips the new test**
The new test runs through `pnpm test`, but no GitHub workflow runs that command. The pull-request workflow only checks the PR title, and the release workflow builds and publishes without running tests. Another invalid description can therefore merge and ship without this check catching it. Add a pull-request job that installs dependencies and runs `pnpm test`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Good catch: no workflow runs pnpm test today. Adding PR CI is being raised as a separate follow-up and isn't part of this fix. For now the spec catches the problem whenever pnpm test is run.
There was a problem hiding this comment.
Understood. The test still provides the intended check whenever pnpm test runs, and adding PR CI can be handled separately. I’ll withdraw this as a blocker for the current fix.
Why
Decided
pnpm format:checkfails at the base commit 0fb1cfb itself, before any of my changes, onplugins/workos/.codex-plugin/plugin.json. That file is outside this task. The release-please bot's release 0.8.1 commit (b9cb597) re-serialised that file and expanded two short arr… → Go with (B), in its own commit, for examplechore: let oxfmt skip the release-bot-written codex plugin.json. Add"plugins/workos/.codex-plugin/plugin.json"toignorePatternsin.oxfmtrc.json, and don't reformat the file itself. Don't touch any other file, and keep the existing ignore entries… (Riker's call)plugin.jsonis no longer format-checked, but it is written by release-please anyway). Publish it as asked? → A: Publish it as asked (the author's answer)Proven
git merge-base --is-ancestor 0fb1cfb HEAD && git diff --quiet 0fb1cfb HEAD -- package.json pnpm-lock.yaml CHANGELOG.md release-please-config.json && test -n "$(git ls-files 'scripts/tests/*.spec.ts' | xargs grep -l -F 'SKILL.md')" && (pnpm install --offline --frozen-lockfile >/dev/null 2>&1 || pnpm install --prefer-offline --frozen-lockfile >/dev/null 2>&1) && node --input-type=module -e "import {parse} from 'yaml'; import {readFileSync} from 'fs'; import {execSync} from 'child_process'; const files=execSync('git ls-files \"*SKILL.md\"').toString().trim().split('\n').filter(Boolean); if(files.length<2) throw new Error('too few SKILL.md'); for (const f of files){const m=readFileSync(f,'utf8').match(/^---\r?\n([\s\S]*?)\r?\n---/); if(!m) throw new Error('no frontmatter '+f); const d=parse(m[1],{strict:true,uniqueKeys:true}); if(!d||typeof d.name!=='string'||!d.name||typeof d.description!=='string'||!d.description) throw new Error('no name/description '+f); const old=execSync('git show 0fb1cfb:'+f).toString().match(/^---\r?\n([\s\S]*?)\r?\n---/)[1].split('\n').find(l=>l.startsWith('description: ')); if(old&&!/^description: [>|'\"]/.test(old)&&old.slice(13).trim()!==d.description) throw new Error('description text changed '+f)}" && pnpm test && pnpm lint && pnpm format:checkpassed on 4f9cb44 (run 1).Opened as a draft by Riker (job 245).