Add a pytest-discovered tests/conftest.py for the last30days scripts path and
remove duplicate per-file sys.path.insert boilerplate from tests.
Normalize affected imports to rely on the shared scripts path and remove the
now-unneeded E402 suppressions.
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.
Every job sync.sh did has a better replacement:
- Per-harness skill dirs (~/.claude/skills, ~/.codex/skills, ~/.agents/skills):
`npx skills add . -g -y` writes to every detected harness's home dir and
uses symlinks by default. Edits propagate live — no re-deploy step.
- Hermes (~/.hermes/skills/research/last30days):
`hermes skills install mvanhorn/last30days-skill --force` pulls from
GitHub and handles the deploy itself. The script wrapping was redundant.
- OpenClaw variant: `clawhub install last30days-official` is what users
already run per the README; the maintainer doesn't need a separate
variant-deploy step in the public repo's scripts.
- Claude marketplace cache (~/.claude/plugins/cache/...): this was a
"test against the official install path" hack we shouldn't have been
recommending. With PR #400's resolver collapse, STEP 0 no longer
enforces the cache as the only valid SKILL.md location. Just install
the skill normally via `npx skills` or the marketplace.
Cleanup:
- DELETE skills/last30days/scripts/sync.sh
- tests/test_version_consistency.py — drop test_sync_cache_path_uses_skill_version
- CLAUDE.md — replace the sync.sh command + rule with `npx skills add . -g -y`
- HERMES_SETUP.md — Installation now uses `hermes skills install --force`;
developer-alternative section shows the symlink pattern for live editing
- render.py — _skill_version docstring no longer attributes the
".claude-plugin absent" case to sync.sh; explains it via per-harness
install paths in general
- .github/PULL_REQUEST_TEMPLATE.md — drop the "Ran bash scripts/sync.sh"
checklist item
CHANGELOG and historical docs (release notes, plan files) keep their
existing sync.sh mentions as accurate history.
sync.sh was written against the layout of mvanhorn/last30days-skill-private
(`.../cache/last30days-skill-private/last30days-3/{version}`) and that path
was never updated when this public repo got its own copy. Running sync.sh
from here populated the BETA channel's cache (`/last30days-beta`) instead
of this repo's own `/last30days` cache, so devs working in this repo could
not test their changes via the public slash command without waiting for a
marketplace release.
Path now derives from this repo's own manifests:
- marketplace name `last30days-skill` (.claude-plugin/marketplace.json)
- plugin name `last30days` (.claude-plugin/plugin.json)
Drops the `last30days-3-nogem` target along with it - that's a private-repo
variant with no public equivalent.
Updates test_sync_cache_path_uses_skill_version to assert the new path
pattern and clarifies the COMMON_TARGETS comment so the next person editing
it understands which marketplace/plugin name segments come from where.
The regression test from #290 walks the filesystem via Path.rglob, so
docs/plans/*.md files (gitignored, created by internal planning) trip
the assertion on any dev machine that has run ce:plan in this repo.
Fresh clones and CI never see them, but local runs fail.
Adding docs to skip_dirs keeps the guard narrow to first-class source
files while letting internal planning docs reference old paths
verbatim.
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Yong-yuan-X