Commit Graph

5 Commits

Author SHA1 Message Date
Dinakar Sarbada 87577ff126 test: guard gemini extension version 2026-05-17 00:00:59 -07:00
Trevin Chow 73dc6b9996 refactor: consolidate SKILL.md version regex into lib/skill_meta.py
The same `^version:\s*"([^"]+)"\s*$` regex (or a slight variant) was
duplicated across three files: render.py inline, test_plugin_contract.py
local helper, test_version_consistency.py local helper. A future change
to the SKILL.md frontmatter version format would have needed to update
three places without any compile-time pressure to keep them in sync.

New skills/last30days/scripts/lib/skill_meta.py provides:
- `_VERSION_RE` private compiled pattern (accepts double-quoted,
  single-quoted, or unquoted YAML version scalars per the widening
  landed in 997708a)
- `read_skill_version(skill_md_path: Path) -> str | None` helper that
  catches OSError + UnicodeDecodeError and returns None on miss

Callers updated:
- render.py::_skill_version now calls skill_meta.read_skill_version
  inside the SKILL.md fallback loop, returning `read_skill_version(...) or "?"`.
  Semantically equivalent to the old break-after-first-SKILL.md logic.
- test_plugin_contract.py and test_version_consistency.py import the
  helper instead of defining the regex inline. Both files use the
  established sys.path.insert pattern.

Added tests/test_skill_meta.py with 6 direct unit tests covering the
helper's full contract: missing file, undecodable bytes, no-version-line,
and all three quoting styles (double, single, unquoted). Previously the
helper was only exercised transitively through render._skill_version().

Added test_skill_md_uses_double_quoted_version to
test_version_consistency.py — the old per-test regex incidentally
asserted "this repo's SKILL.md uses double-quotes" by being strict;
the shared helper accepts all three styles, so the assertion is now
explicit instead of implicit.

Code-reviewed by ce-code-review (8 reviewers); safe_auto fixes applied
inline (rename to _VERSION_RE, group or-chain instead of generator,
docstring tightened, dropped unnecessary `from __future__ import
annotations`, tightened signature to Path-only).

Conftest.py refactor for the sys.path.insert duplication across ~20 test
files filed as issue #411 — out of scope for this PR (touches many
files, separate concern).

Test results: 23 passed in the affected test set (16 prior + 6 new
test_skill_meta tests + 1 new double-quote assertion). Full suite shows
same 13 pre-existing failures as main; zero new failures.
2026-05-16 18:17:33 -07:00
Trevin Chow 997708ad48 refactor(skill): apply ce-code-review fixes — bump to 3.2.2, fallback tests, comparison resolver
12 fixes from the multi-agent code review on PR #400:

Version 3.2.1 -> 3.2.2 across all manifests (SKILL.md frontmatter + body
header, pyproject.toml, .claude-plugin/{plugin,marketplace}.json, sync.sh
cache path). The PR ships observable behavior changes (STEP 0 logic flip,
resolver order change, badge fallback) that should not silently appear
under the same version number — the new fallback reads SKILL.md version
directly so the badge would otherwise be misleading.

render.py::_skill_version:
- `import re` moved to module top
- _VERSION_RE extracted as a module-level compiled pattern that accepts
  double-quoted, single-quoted, and unquoted YAML version scalars
- `break` -> `continue` on corrupt manifest, so a corrupt inner manifest
  no longer shadows a valid outer one
- Wrap SKILL.md read_text() in try/except for UnicodeDecodeError to keep
  badge emission from crashing on mis-encoded SKILL.md
- Docstring clarifies precedence; inline comment marks the fallback boundary
  between the manifest walk and the SKILL.md walk

tests/test_skill_version.py (new): 7 unit tests for the fallback paths
(manifest absent, manifest corrupt, corrupt-inner + valid-outer, both
absent, SKILL.md without version, single-quoted, unquoted).

tests/test_plugin_contract.py: tombstone test asserting .codex-plugin/
stays removed (was the only CI guard against accidental reintroduction).

SKILL.md:
- STEP 0 bash echoes CLAUDE_CACHE_LATEST so the model can see the
  resolved value when deciding whether to hop
- "Both shapes ship in the wild" comment now names the two cache layouts
  (nested {cache}/{version}/skills/last30days/ vs flat {cache}/{version}/)
- Comparison-mode bash invocation gets its own inline SKILL_ROOT resolver
  (latent gap: the contract tells the model to skip Step 1 on comparison
  queries, so SKILL_ROOT was previously unset there)

CHANGELOG.md: [Unreleased] entries for the resolver rewrite and the
breaking removal of Codex native-plugin support.

All 9 reviewer personas surfaced findings; 3 cross-reviewer corroboration
clusters were promoted (import re, "both shapes" comment, missing fallback
tests). Maintainability follow-up flagged: regex now duplicated across
render.py and 2 test files; could consolidate via shared lib/skill_meta.py
helper in a future PR.
2026-05-15 21:45:25 -07:00
Trevin Chow c913e1cf89 refactor(skill): SKILL.md-relative path resolver, drop Codex native plugin
STEP 0 (CANONICAL PATH SELF-CHECK) used to force any SKILL.md load that wasn't
under $HOME/.claude/plugins/cache/last30days-skill/last30days/{version}/ to
re-Read from there. That guard is Claude-Code-specific (defends against the
marketplaces/ stale-clone bug) and broke under non-Claude installers like
`npx skills add`, ~/.codex/skills/, and ~/.agents/skills/.

The new STEP 0 narrows the check to its actual target: fire only when the
loaded SKILL.md path contains /.claude/plugins/marketplaces/. Every other
install path is trusted. The 2026-04-22 incident workaround is preserved
without breaking other harnesses.

Step 1 SKILL_ROOT resolver collapses the Codex-first / Claude-fallback /
CWD-fallback chain into a single precedence walk: Claude plugin cache
(versioned) first, then ~/.codex/skills, ~/.agents/skills, repo checkout,
./.skills/last30days (npx skills install dir), CWD, and GEMINI_EXTENSION_DIR.

Also drops Codex native plugin support: .codex-plugin/plugin.json is deleted,
the badge VERSION jq fallback in line 108 stops looking at it, and render.py's
_skill_version no longer scans for it. Codex users install via `npx skills add`
or the per-harness skill dir going forward.

render.py::_skill_version gains a SKILL.md frontmatter fallback so the badge
no longer emits `v?` on install dirs that sync.sh populates (which don't
include .claude-plugin/plugin.json).
2026-05-15 21:44:55 -07:00
Claire Novotny 17caa0526d ci: validate plugin contract on pull requests 2026-04-24 12:05:39 -04:00