harden _always_on against missing blocks and pin skillgen baselines
R1: the 6 always-on blocks were read into module-level constants at import, so a missing/corrupt always_on/*.md crashed `import graphify.__main__` and bricked every CLI command, not just install. Make _always_on lazy + lru_cached, raising a clear "reinstall" error only on the install path that needs the block; a module __getattr__ keeps the legacy constant names importable for the tests. Verified: deleting a block no longer crashes `graphify --version`. skillgen baselines: the self-check guards pinned to the moving `origin/v8` ref, which stops pointing at the pre-split state once the split lands on v8 (the always-on-roundtrip guard then fails in CI and monolith-roundtrip goes vacuous). Pin _v8_baseline_ref, ALWAYS_ON_BASELINE_REF, and the two monolith roundtrip_refs to the immutable pre-split commit SHA, matching the stated "does not track HEAD" intent; update the 4 tests that asserted the old ref string. R2: add tests/test_wheel_packaging.py - build the wheel and assert every references bundle and always-on block ships in it, so a package-data glob miss can't pass the repo-tree guards yet hard-exit `graphify install` for real users. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+47
-20
@@ -1,6 +1,7 @@
|
||||
"""graphify CLI - `graphify install` sets up the Claude Code skill."""
|
||||
|
||||
from __future__ import annotations
|
||||
import functools
|
||||
import json
|
||||
import os
|
||||
import platform
|
||||
@@ -21,6 +22,7 @@ except Exception:
|
||||
_GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out")
|
||||
|
||||
|
||||
@functools.lru_cache(maxsize=None)
|
||||
def _always_on(basename: str) -> str:
|
||||
"""Read a packaged always-on instruction block from graphify/always_on/.
|
||||
|
||||
@@ -32,7 +34,38 @@ def _always_on(basename: str) -> str:
|
||||
bytes here must match the former triple-quoted constant exactly — the
|
||||
always-on-roundtrip validator proves that.
|
||||
"""
|
||||
return (Path(__file__).parent / "always_on" / f"{basename}.md").read_text(encoding="utf-8")
|
||||
path = Path(__file__).parent / "always_on" / f"{basename}.md"
|
||||
try:
|
||||
return path.read_text(encoding="utf-8")
|
||||
except OSError as exc:
|
||||
# Defer to use-time so a missing/corrupt packaged block can't crash module
|
||||
# import (which would brick every CLI command, not just install). Reached
|
||||
# only by an install/integration path that actually needs this block.
|
||||
raise RuntimeError(
|
||||
f"graphify install is incomplete: missing always-on block '{basename}' "
|
||||
f"at {path}. Reinstall graphifyy (e.g. `uv tool install --reinstall graphifyy`)."
|
||||
) from exc
|
||||
|
||||
|
||||
_ALWAYS_ON_ALIASES = {
|
||||
"_CLAUDE_MD_SECTION": "claude-md",
|
||||
"_AGENTS_MD_SECTION": "agents-md",
|
||||
"_GEMINI_MD_SECTION": "gemini-md",
|
||||
"_VSCODE_INSTRUCTIONS_SECTION": "vscode-instructions",
|
||||
"_ANTIGRAVITY_RULES": "antigravity-rules",
|
||||
"_KIRO_STEERING": "kiro-steering",
|
||||
}
|
||||
|
||||
|
||||
def __getattr__(name: str) -> str:
|
||||
# PEP 562: lazily resolve the legacy always-on section constants for external
|
||||
# importers (e.g. the install-string tests). In-module code calls _always_on()
|
||||
# directly; nothing is read at import time, so a missing block can no longer
|
||||
# brick the CLI on `import graphify.__main__` (#1121 follow-up).
|
||||
base = _ALWAYS_ON_ALIASES.get(name)
|
||||
if base is not None:
|
||||
return _always_on(base)
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
|
||||
|
||||
def _default_graph_path() -> str:
|
||||
@@ -587,17 +620,14 @@ def _print_install_usage() -> None:
|
||||
# generated by tools/skillgen and guarded by `skillgen --check`. Reading them at
|
||||
# load keeps the install-string / issue-#580 contract byte-for-byte while letting
|
||||
# a human edit one fragment instead of a triple-quoted literal here.
|
||||
_CLAUDE_MD_SECTION = _always_on("claude-md")
|
||||
|
||||
_CLAUDE_MD_MARKER = "## graphify"
|
||||
|
||||
# AGENTS.md section for Codex, OpenCode, and OpenClaw.
|
||||
# All three platforms read AGENTS.md in the project root for persistent instructions.
|
||||
_AGENTS_MD_SECTION = _always_on("agents-md")
|
||||
|
||||
_AGENTS_MD_MARKER = "## graphify"
|
||||
|
||||
_GEMINI_MD_SECTION = _always_on("gemini-md")
|
||||
|
||||
_GEMINI_MD_MARKER = "## graphify"
|
||||
|
||||
@@ -630,10 +660,10 @@ def gemini_install(project_dir: Path | None = None, *, project: bool = False) ->
|
||||
if target.exists():
|
||||
content = target.read_text(encoding="utf-8")
|
||||
new_content = _replace_or_append_section(
|
||||
content, _GEMINI_MD_MARKER, _GEMINI_MD_SECTION
|
||||
content, _GEMINI_MD_MARKER, _always_on("gemini-md")
|
||||
)
|
||||
else:
|
||||
new_content = _GEMINI_MD_SECTION
|
||||
new_content = _always_on("gemini-md")
|
||||
|
||||
if target.exists() and new_content == target.read_text(encoding="utf-8"):
|
||||
print(f"graphify already configured in {target.resolve()} (no change)")
|
||||
@@ -714,7 +744,6 @@ def gemini_uninstall(project_dir: Path | None = None, *, project: bool = False)
|
||||
|
||||
|
||||
_VSCODE_INSTRUCTIONS_MARKER = "## graphify"
|
||||
_VSCODE_INSTRUCTIONS_SECTION = _always_on("vscode-instructions")
|
||||
|
||||
|
||||
def vscode_install(project_dir: Path | None = None) -> None:
|
||||
@@ -753,7 +782,7 @@ def vscode_install(project_dir: Path | None = None) -> None:
|
||||
if instructions.exists():
|
||||
content = instructions.read_text(encoding="utf-8")
|
||||
new_content = _replace_or_append_section(
|
||||
content, _VSCODE_INSTRUCTIONS_MARKER, _VSCODE_INSTRUCTIONS_SECTION
|
||||
content, _VSCODE_INSTRUCTIONS_MARKER, _always_on("vscode-instructions")
|
||||
)
|
||||
if new_content == content:
|
||||
print(f" {instructions} -> already configured (no change)")
|
||||
@@ -761,7 +790,7 @@ def vscode_install(project_dir: Path | None = None) -> None:
|
||||
instructions.write_text(new_content, encoding="utf-8")
|
||||
print(f" {instructions} -> graphify section {'updated' if _VSCODE_INSTRUCTIONS_MARKER in content else 'added'}")
|
||||
else:
|
||||
instructions.write_text(_VSCODE_INSTRUCTIONS_SECTION, encoding="utf-8")
|
||||
instructions.write_text(_always_on("vscode-instructions"), encoding="utf-8")
|
||||
print(f" {instructions} -> created")
|
||||
|
||||
print()
|
||||
@@ -813,7 +842,6 @@ def vscode_uninstall(project_dir: Path | None = None) -> None:
|
||||
_ANTIGRAVITY_RULES_PATH = Path(".agents") / "rules" / "graphify.md"
|
||||
_ANTIGRAVITY_WORKFLOW_PATH = Path(".agents") / "workflows" / "graphify.md"
|
||||
|
||||
_ANTIGRAVITY_RULES = _always_on("antigravity-rules")
|
||||
|
||||
_ANTIGRAVITY_WORKFLOW = """\
|
||||
---
|
||||
@@ -829,7 +857,6 @@ If no path argument is given, use `.` (current directory).
|
||||
"""
|
||||
|
||||
|
||||
_KIRO_STEERING = _always_on("kiro-steering")
|
||||
|
||||
_KIRO_STEERING_MARKER = "graphify: A knowledge graph of this project"
|
||||
|
||||
@@ -849,13 +876,13 @@ def _kiro_install(project_dir: Path) -> None:
|
||||
steering_dir = project_dir / ".kiro" / "steering"
|
||||
steering_dir.mkdir(parents=True, exist_ok=True)
|
||||
steering_dst = steering_dir / "graphify.md"
|
||||
if steering_dst.exists() and steering_dst.read_text(encoding="utf-8") == _KIRO_STEERING:
|
||||
if steering_dst.exists() and steering_dst.read_text(encoding="utf-8") == _always_on("kiro-steering"):
|
||||
print(f" .kiro/steering/graphify.md -> already configured (no change)")
|
||||
else:
|
||||
# File is wholly graphify-owned. Overwrite on upgrade so older
|
||||
# report-first wording does not silently linger (issue #580).
|
||||
action = "updated" if steering_dst.exists() else "written"
|
||||
steering_dst.write_text(_KIRO_STEERING, encoding="utf-8")
|
||||
steering_dst.write_text(_always_on("kiro-steering"), encoding="utf-8")
|
||||
print(f" .kiro/steering/graphify.md -> always-on steering {action}")
|
||||
|
||||
print()
|
||||
@@ -904,13 +931,13 @@ def _antigravity_install(project_dir: Path) -> None:
|
||||
rules_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
if rules_path.exists():
|
||||
existing = rules_path.read_text(encoding="utf-8")
|
||||
if _ANTIGRAVITY_RULES.strip() != existing.strip():
|
||||
rules_path.write_text(_ANTIGRAVITY_RULES, encoding="utf-8")
|
||||
if _always_on("antigravity-rules").strip() != existing.strip():
|
||||
rules_path.write_text(_always_on("antigravity-rules"), encoding="utf-8")
|
||||
print(f"graphify rule updated at {rules_path.resolve()}")
|
||||
else:
|
||||
print(f"graphify rule already configured at {rules_path.resolve()} (no change)")
|
||||
else:
|
||||
rules_path.write_text(_ANTIGRAVITY_RULES, encoding="utf-8")
|
||||
rules_path.write_text(_always_on("antigravity-rules"), encoding="utf-8")
|
||||
print(f"graphify rule written to {rules_path.resolve()}")
|
||||
|
||||
# 3. Write .agents/workflows/graphify.md
|
||||
@@ -1406,10 +1433,10 @@ def _agents_install(project_dir: Path, platform: str) -> None:
|
||||
if target.exists():
|
||||
content = target.read_text(encoding="utf-8")
|
||||
new_content = _replace_or_append_section(
|
||||
content, _AGENTS_MD_MARKER, _AGENTS_MD_SECTION
|
||||
content, _AGENTS_MD_MARKER, _always_on("agents-md")
|
||||
)
|
||||
else:
|
||||
new_content = _AGENTS_MD_SECTION
|
||||
new_content = _always_on("agents-md")
|
||||
|
||||
if target.exists() and new_content == target.read_text(encoding="utf-8"):
|
||||
print(f"graphify already configured in {target.resolve()} (no change)")
|
||||
@@ -1636,10 +1663,10 @@ def claude_install(project_dir: Path | None = None) -> None:
|
||||
if target.exists():
|
||||
content = target.read_text(encoding="utf-8")
|
||||
new_content = _replace_or_append_section(
|
||||
content, _CLAUDE_MD_MARKER, _CLAUDE_MD_SECTION
|
||||
content, _CLAUDE_MD_MARKER, _always_on("claude-md")
|
||||
)
|
||||
else:
|
||||
new_content = _CLAUDE_MD_SECTION
|
||||
new_content = _always_on("claude-md")
|
||||
|
||||
if target.exists() and new_content == target.read_text(encoding="utf-8"):
|
||||
print(f"graphify already configured in {target.resolve()} (no change)")
|
||||
|
||||
@@ -593,9 +593,9 @@ def test_audit_reads_each_host_against_its_own_v8_body():
|
||||
|
||||
This is the structural fix: a per-host body, so a drop on one host surfaces.
|
||||
"""
|
||||
assert gen._v8_baseline_ref("claude") == "origin/v8:graphify/skill.md"
|
||||
assert gen._v8_baseline_ref("trae") == "origin/v8:graphify/skill-trae.md"
|
||||
assert gen._v8_baseline_ref("vscode") == "origin/v8:graphify/skill-vscode.md"
|
||||
assert gen._v8_baseline_ref("claude") == "47042beb05d1f6dd2186c0c499ae2840ce604ead:graphify/skill.md"
|
||||
assert gen._v8_baseline_ref("trae") == "47042beb05d1f6dd2186c0c499ae2840ce604ead:graphify/skill-trae.md"
|
||||
assert gen._v8_baseline_ref("vscode") == "47042beb05d1f6dd2186c0c499ae2840ce604ead:graphify/skill-vscode.md"
|
||||
|
||||
|
||||
def test_audit_catches_an_induced_per_host_drop():
|
||||
@@ -783,6 +783,6 @@ def test_amp_audit_coverage_passes_against_its_own_v8():
|
||||
confirms every heading single-homes in amp's core + references.
|
||||
"""
|
||||
platforms = gen.load_platforms()
|
||||
assert gen._v8_baseline_ref("amp") == "origin/v8:graphify/skill-amp.md"
|
||||
assert gen._v8_baseline_ref("amp") == "47042beb05d1f6dd2186c0c499ae2840ce604ead:graphify/skill-amp.md"
|
||||
problems = gen.audit_coverage(platforms["amp"])
|
||||
assert problems == [], "\n".join(problems)
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
"""Packaging guard (#1121 follow-up): the 5 skillgen guards check the *repo tree*,
|
||||
not the *built wheel*. A host whose references bundle or always-on block fails to
|
||||
match the `package-data` globs would pass `--check`/`--audit-coverage` yet make
|
||||
`graphify install` hard-exit with "not found in package" for real users.
|
||||
|
||||
This builds the wheel once and asserts every committed skill artifact ships in it.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
PKG = REPO / "graphify"
|
||||
|
||||
|
||||
def _has_build() -> bool:
|
||||
try:
|
||||
subprocess.run(
|
||||
[sys.executable, "-m", "build", "--version"],
|
||||
check=True, capture_output=True,
|
||||
)
|
||||
return True
|
||||
except (subprocess.CalledProcessError, FileNotFoundError):
|
||||
return False
|
||||
|
||||
|
||||
def _expected_artifacts() -> list[Path]:
|
||||
"""Every committed references/*.md (per host) + always_on/*.md block."""
|
||||
refs = sorted((PKG / "skills").glob("*/references/*.md"))
|
||||
always = sorted((PKG / "always_on").glob("*.md"))
|
||||
# Sanity: if these are empty the test wiring is broken, not the wheel.
|
||||
assert refs, "no skills/*/references/*.md found in repo — packaging test mis-wired"
|
||||
assert always, "no always_on/*.md found in repo — packaging test mis-wired"
|
||||
return refs + always
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def wheel_namelist(tmp_path_factory) -> set[str]:
|
||||
if not _has_build():
|
||||
pytest.skip("`python -m build` unavailable (dev extra not installed)")
|
||||
out = tmp_path_factory.mktemp("wheel")
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "-m", "build", "--wheel", "--no-isolation",
|
||||
"--outdir", str(out), str(REPO)],
|
||||
capture_output=True, text=True,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
pytest.skip(f"wheel build failed in this env:\n{proc.stderr[-800:]}")
|
||||
wheels = list(out.glob("graphifyy-*.whl"))
|
||||
assert wheels, "no wheel produced"
|
||||
with zipfile.ZipFile(max(wheels, key=lambda p: p.stat().st_mtime)) as z:
|
||||
return set(z.namelist())
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"artifact",
|
||||
_expected_artifacts(),
|
||||
ids=lambda p: str(p.relative_to(PKG)),
|
||||
)
|
||||
def test_skill_artifact_ships_in_wheel(artifact: Path, wheel_namelist: set[str]) -> None:
|
||||
rel = "graphify/" + artifact.relative_to(PKG).as_posix()
|
||||
assert rel in wheel_namelist, (
|
||||
f"{rel} is committed in the repo but NOT in the built wheel — "
|
||||
f"`graphify install` would hard-exit for this host. Check the "
|
||||
f"[tool.setuptools.package-data] globs in pyproject.toml."
|
||||
)
|
||||
+12
-4
@@ -45,11 +45,19 @@ PLATFORMS_TOML = SKILLGEN_DIR / "platforms.toml"
|
||||
# against ITS OWN v8 body is the per-host guard: a drop that only hits one host
|
||||
# (e.g. trae losing its AGENTS.md integration section) is invisible when every
|
||||
# host is checked against claude's monolith, so the audit must be per-host.
|
||||
#
|
||||
# Baselines are pinned to the immutable pre-split commit SHA, NOT the moving
|
||||
# `origin/v8` ref: once the split lands on v8, `origin/v8` no longer holds the
|
||||
# original monolith bodies / inline constants, so a symbolic ref would compare
|
||||
# the split against itself (vacuous) or fail to find the old constants. The SHA
|
||||
# is an ancestor of origin/v8 and is fetched under the CI `fetch-depth: 0`.
|
||||
_V8_BASELINE_SHA = "47042beb05d1f6dd2186c0c499ae2840ce604ead"
|
||||
|
||||
def _v8_baseline_ref(platform_key: str) -> str:
|
||||
"""The git ref for a split host's own v8 skill body."""
|
||||
"""The git ref for a split host's own pre-split skill body."""
|
||||
if platform_key == "claude":
|
||||
return "origin/v8:graphify/skill.md"
|
||||
return f"origin/v8:graphify/skill-{platform_key}.md"
|
||||
return f"{_V8_BASELINE_SHA}:graphify/skill.md"
|
||||
return f"{_V8_BASELINE_SHA}:graphify/skill-{platform_key}.md"
|
||||
|
||||
# Immutable baseline for --always-on-roundtrip. The six always-on instruction
|
||||
# blocks used to be triple-quoted constants in graphify/__main__.py; they are now
|
||||
@@ -59,7 +67,7 @@ def _v8_baseline_ref(platform_key: str) -> str:
|
||||
# constant byte for byte. It deliberately does NOT track HEAD: once the extraction
|
||||
# lands, HEAD's constants are _always_on(...) calls, not the literals the
|
||||
# validator needs to compare against.
|
||||
ALWAYS_ON_BASELINE_REF = "origin/v8:graphify/__main__.py"
|
||||
ALWAYS_ON_BASELINE_REF = f"{_V8_BASELINE_SHA}:graphify/__main__.py"
|
||||
|
||||
# The always-on instruction blocks: rendered-file basename -> the __main__.py
|
||||
# constant it must reproduce. Rendered to graphify/always_on/<basename>.md from
|
||||
|
||||
@@ -193,10 +193,10 @@ extraction = "verbose"
|
||||
bucket = "monolith"
|
||||
skill_dst = "graphify/skill-aider.md"
|
||||
monolith = "aider"
|
||||
roundtrip_ref = "origin/v8:graphify/skill-aider.md"
|
||||
roundtrip_ref = "47042beb05d1f6dd2186c0c499ae2840ce604ead:graphify/skill-aider.md"
|
||||
|
||||
[platform.devin]
|
||||
bucket = "monolith"
|
||||
skill_dst = "graphify/skill-devin.md"
|
||||
monolith = "devin"
|
||||
roundtrip_ref = "origin/v8:graphify/skill-devin.md"
|
||||
roundtrip_ref = "47042beb05d1f6dd2186c0c499ae2840ce604ead:graphify/skill-devin.md"
|
||||
|
||||
Reference in New Issue
Block a user