skill-creator: lazy-load PyYAML in frontmatter parsing#12080
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39300075d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match = _KEY_PATTERN.match(stripped) | ||
| if not match: | ||
| raise YAMLError( | ||
| f"line {line_no}: unsupported YAML syntax: {raw_line.strip()}" |
There was a problem hiding this comment.
_parse_yaml_mapping_shim only accepts key: ... lines, so valid YAML list syntax (- item) is always rejected. quick_validate.py explicitly permits allowed-tools, which is commonly written as a block list; in shim mode this now fails with unsupported YAML syntax even for valid frontmatter, so skill-creator does not reliably work without PyYAML.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Reworked the shim parser from a simple stacked mapping parser into a tokenized, indentation-aware block parser that can parse nested mappings and block sequences (
- item) for frontmatter values such asallowed-tools. codex-rs/core/src/skills/assets/samples/skill-creator/scripts/yaml_compat.pyL63-L213 - Added strict indentation validation so malformed nested content is rejected (including over-indented lines under scalar keys/items), addressing the invalid-indentation acceptance issue noted in review feedback. codex-rs/core/src/skills/assets/samples/skill-creator/scripts/yaml_compat.pyL118-L135 codex-rs/core/src/skills/assets/samples/skill-creator/scripts/yaml_compat.pyL168-L191
- Committed the change (
187811b) and opened a follow-up PR with title “Fix YAML shim handling for block lists and invalid indentation” via themake_prtool.
Testing
- ✅
python3 -m py_compile codex-rs/core/src/skills/assets/samples/skill-creator/scripts/yaml_compat.py codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py - ✅
python3 - <<'PY'(inline assertions validating block-list parsing and rejecting invalid nested indentation)PY - ✅
tmpdir=$(mktemp -d) ... python3 codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py "$tmpdir" ...(valid frontmatter with block list) - ✅
tmpdir=$(mktemp -d) ... python3 codex-rs/core/src/skills/assets/samples/skill-creator/scripts/quick_validate.py "$tmpdir" ...(malformed indentation correctly rejected; command asserts failure) ⚠️ cargo test -p codex-core(fails in this environment: missinglibcapviapkg-configrequired bycodex-linux-sandboxbuild script)⚠️ cargo test -p codex-core --lib(same environment limitation: missinglibcap)⚠️ cargo test --all-features(same environment limitation: missinglibcap)⚠️ apt-get update && apt-get install -y libcap-dev(unable to install dependency due repository/proxy 403 responses in this environment)
| while len(stack) > 1 and indent <= stack[-1][0]: | ||
| stack.pop() | ||
|
|
||
| parent = stack[-1][1] | ||
| if value_text == "": |
There was a problem hiding this comment.
The shim never checks whether deeper indentation is legal for the current parent. After a scalar key (name: foo), an over-indented line ( description: bar) should be a YAML error, but this code silently writes description at the root and returns a dict. quick_validate.py can therefore accept malformed frontmatter that real YAML parsers reject.
Useful? React with 👍 / 👎.
xl-openai
force-pushed
the
xl/skill-create
branch
from
3930007 to
6dc0179
Compare
xl-openai
changed the title
|
@codex review again |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
init-skill should work even without PyYAML