v0.2.2 audit pass: rename/validate/log/reindex fixes, CI, maintainer handoff#3
Draft
betmoar wants to merge 3 commits into
Draft
v0.2.2 audit pass: rename/validate/log/reindex fixes, CI, maintainer handoff#3betmoar wants to merge 3 commits into
betmoar wants to merge 3 commits into
Conversation
…docs Correctness fixes (each locked in by a new test): - rename: rewrite markdown links [text](old.md) as well as [[old]] wiki links; previously md links were left dangling after every rename even though v0.2.1 made them part of the link graph (okf_common.rewrite_links) - validate: new C10 ERROR — id must be a kebab-case string; a non-kebab id was invisible to the link extractor and index parser, producing an index WARN that reindex could never clear; YAML-boolean ids (on/yes/no) get an actionable message - append-log: options parsed only before the message (POSIX style, plus `--` support) so option-like words inside the message are no longer silently consumed out of the append-only record - reindex: escape | and newlines in generated table cells; refuse to scaffold index.md into a directory with no OKF markers anywhere - okf_common: fallback parser keeps url: continuation lines in sources entries; parse_frontmatter/body_of now share _fm_lines' strict delimiter scan so all parsers agree on the same frontmatter block - validate: X1 message says "body link" instead of mislabeling markdown links as wiki-links Infrastructure: - GitHub Actions CI: 3.9/3.13, suite run with and without PyYAML, gate shell syntax check, release-consistency gate - scripts/check-release.py: version<->CHANGELOG coupling, hooks/commands reference integrity, allowed-tools presence - New tests for append-log.py and okf-gate.sh (previously untested); 63 -> 87 tests - CLAUDE.md maintainer handoff: load-bearing map, named invariants, touch-X-update-Y couplings, change playbooks, prioritized backlog Docs: conformance.md C10 + X1 wording, spec.md enforcement notes, rename command prose (also dropped its stale claim of regenerating links:), README highlights. Version 0.2.2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAPb7KWqxXYjCnkbmTYx8X
CI's no-PyYAML step exposed a parser-agreement gap: unquoted `id: on` is boolean True under PyYAML (so C10 rejects it) but was the plain string "on" under the fallback parser — the same file validated differently depending on the environment. The fallback now resolves YAML 1.1 special scalars (on/yes/no/true/false/off, null/~) exactly like PyYAML; quoted forms remain strings in both. Also fix the test-suite env pollution that masked this locally: TestFallbackParser deleted OKF_NO_YAML outright instead of restoring the runner-set value, silently flipping the rest of the suite back to PyYAML. Helper now saves/restores; new test locks the invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KAPb7KWqxXYjCnkbmTYx8X
There was a problem hiding this comment.
Pull request overview
This PR is a v0.2.2 “audit pass” hardening sweep across the OKF plugin, aligning tooling behavior with the spec (rename/link rewriting, stricter id validation, safer reindexing, and parser consistency) while adding CI and release-gating to prevent regressions.
Changes:
- Make
/cc-okf:renamerewrite both wiki-links and markdown link targets via a sharedrewrite_links()helper. - Add conformance rule C10 (kebab-case
id) and harden parsing/validation behaviors (YAML 1.1 scalar coercions, frontmatter delimiter consistency). - Add new tests, CI workflow, and a release consistency gate (
scripts/check-release.py) plus documentation/maintainer handoff updates.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_validate.py | Adds tests for new C10 kebab-case id validation behavior. |
| tests/test_rename.py | Verifies rename rewrites markdown links and leaves no dangling-link warnings. |
| tests/test_reindex.py | Tests table-cell escaping and guardrails for reindexing in non-OKF directories. |
| tests/test_okf_common.py | Adds coverage for rewrite_links, fallback parser canonical forms, and delimiter consistency. |
| tests/test_gate.py | Adds contract tests for the always-on SessionStart gate script. |
| tests/test_append_log.py | Adds subprocess-level CLI contract tests for append-log.py. |
| skills/okf/SKILL.md | Updates skill docs to reflect rename rewriting both link forms. |
| skills/okf/references/spec.md | Updates spec wording for rename behavior and new C10 rationale. |
| skills/okf/references/conformance.md | Documents new C10 rule and updates X1 warning wording. |
| scripts/validate.py | Implements C10 validation and improves dangling-link warning wording. |
| scripts/rename.py | Switches body rewriting to shared okf_common.rewrite_links(). |
| scripts/reindex.py | Escapes markdown table cells, adds OKF-marker guardrail, and renames legacy link stripping step. |
| scripts/okf_common.py | Adds YAML 1.1 scalar coercions, fallback parser fixes, delimiter unification, and rewrite_links(). |
| scripts/check-release.py | New release-consistency checker (version/CHANGELOG, hooks, commands, referenced scripts). |
| scripts/append-log.py | Changes arg parsing to POSIX-style (options only before message; supports --). |
| README.md | Adds v0.2.2 highlights and documents verification commands. |
| commands/rename.md | Updates command prose to match rename behavior (both link forms; no links: regen). |
| CLAUDE.md | Adds maintainer handoff: architecture, invariants, couplings, playbooks, and backlog. |
| CHANGELOG.md | Adds 0.2.2 release notes matching implemented behaviors. |
| .github/workflows/test.yml | New CI workflow (3.9/3.13, PyYAML/no-PyYAML modes, bash -n gate, check-release). |
| .claude-plugin/plugin.json | Bumps plugin version to 0.2.2. |
Comments suppressed due to low confidence (1)
scripts/validate.py:113
- C10 validation is gated by
if cid:and required-field checks use truthiness. YAML 1.1 boolean-like ids such asid: noparse toFalse, so the validator currently reports a misleading “missing required frontmatter field: id” and never emits the intended kebab-case/quoting error. Also, whencidis non-string/invalid, it’s still inserted intoids/parsed, which can produce confusing downstream warnings (e.g.,missing concepts [True]). Use presence/None checks instead of truthiness, and only add toids/parsedwhencidis a valid kebab-case string.
for field in REQUIRED_FIELDS:
if not fm.get(field):
err(scope, f"missing required frontmatter field: {field}")
cid = fm.get("id")
stem = fname[:-3]
if cid:
# C10: id must be a kebab-case STRING. A non-string id usually means
# YAML 1.1 coerced it (on/yes/no/true -> bool); non-kebab ids are
# invisible to the link extractor and the index-row parser, which
# would otherwise produce a WARN loop that /cc-okf:reindex can never
# clear.
if not isinstance(cid, str) or not oc.ID_RE.match(cid):
err(scope, f"id '{cid}' is not kebab-case "
f"(^[a-z0-9][a-z0-9-]*$); quote YAML-boolean-like "
f"ids such as 'on'/'yes'/'no'")
if cid != stem:
err(scope, f"id '{cid}' does not match filename stem '{stem}'")
if cid in ids:
err(scope, f"duplicate id '{cid}' (also in {ids[cid]})")
else:
ids[cid] = scope
parsed[cid] = {"fm": fm, "body": oc.body_of(text), "scope": scope,
"text": text}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… ids
Review follow-up (Copilot): YAML 1.1 coerces `id: no` to boolean False,
so the truthy required-field check misreported it as "missing required
frontmatter field: id" and C10 never fired. Required-field and id checks
now use presence (None / blank string) instead of truthiness, so falsy
boolean ids get the actionable kebab-case/quoting error.
Also stop registering non-kebab ids in ids/parsed: the coerced value
leaked into downstream reporting ("index.md: missing concepts [True]").
C10 is the sole, actionable error for an invalid id.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KAPb7KWqxXYjCnkbmTYx8X
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
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.
Full-codebase audit and hardening pass. Every finding below was reproduced by execution before fixing, and every fix ships with a test that locks it in (63 → 89 tests, all green in three parser modes: PyYAML installed,
OKF_NO_YAML=1, and PyYAML genuinely absent).Correctness fixes
/cc-okf:renamenow rewrites markdown links. v0.2.1 made[text](old.md)links part of the link graph, but rename only rewrote[[old]]wiki links — every rename silently dangled the markdown references. New sharedokf_common.rewrite_links()handles both forms; targets that don't resolve to the old id (external URLs, subdirs,old-v2.md) are untouched, and prefixes/fragments are preserved (concepts/old.md#frag→concepts/new.md#frag).idmust be a kebab-case string. The spec already said MUST, but the validator never checked. A non-kebab id was invisible to the link extractor and the index-row parser — producing anindex.mdWARN that/cc-okf:reindexcould never clear. YAML-boolean-like ids (on,yes,no) get an actionable "quote it" message; falsy booleans (id: no→False) are caught via presence checks instead of truthiness, and invalid ids are kept out of downstream reporting (review follow-up).on/yes/no/true/false/off→ bool,null/~→ None; quoted forms stay strings). Previously the same file meant different things depending on which parser was installed — caught by CI's no-PyYAML step on the first push.append-log.pyno longer eats words from the message.--action/--bundleappearing mid-message were reparsed as options (verified:note about --action flags usagewas logged as action=flags, message=note about usage— silent corruption of an append-only record). Options are now recognized only before the message, POSIX-style, with--support.index.mdcells escape|and newlines — a raw pipe in any frontmatter value shifted every later column.reindexrefuses to scaffoldindex.mdinto a non-OKF directory (noconcepts/,index.md,log.md, or.okf/here or in any ancestor) — protects against wrong-cwd runs while keeping the quick-start bootstrap path working.url:continuation lines insources— it previously couldn't parse the spec's own canonical citation form, blinding rule C9 whenever PyYAML is absent.---delimiter.parse_frontmatter/body_ofaccepted--- anythingas a close while the splicer required an exact---line — the same file could parse two different ways. Both now share_fm_lines.body link to '<id>'instead of mislabeling markdown links as wiki-links;commands/rename.mdno longer claims it regenerates thelinks:field removed in v0.2.1.Infrastructure
.github/workflows/test.yml): Python 3.9 + 3.13, test suite run three ways (no PyYAML installed,OKF_NO_YAML=1forced, PyYAML installed),bash -non the SessionStart gate, and the release gate.scripts/check-release.py: enforces the version↔CHANGELOG coupling, validatesplugin.json/hooks.json, and checks every${CLAUDE_PLUGIN_ROOT}script referenced by hooks/commands exists and that commands declareallowed-tools.tests/test_append_log.py(CLI contract, append-only invariant) andtests/test_gate.py(the always-on SessionStart gate: silent when dormant, valid hook JSON when active).CLAUDE.mdmaintainer handoff: load-bearing map ranked by blast radius, named invariants with the tests that lock them, "if you touch X update Y" couplings, decision playbooks (adding a field, changing rule severity, editing the gate), known landmines, and a prioritized backlog.Version bumped to 0.2.2 with a full CHANGELOG section; spec/conformance/SKILL/README updated in lockstep with the rule and behavior changes.
Verification
python3 -m unittest discover -s tests→ 89 tests OKOKF_NO_YAML=1 python3 -m unittest discover -s tests→ 89 tests OKpython3 scripts/check-release.py→ OK