mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
* Initial plan * Extract agent context updates into bundled agent-context extension * Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: address review comments on agent-context extension - bash: parse init-options.json with a single python3 invocation instead of three separate read_json_field calls, for parity with the PowerShell ConvertFrom-Json approach and to avoid divergent error semantics - bash: use parameter expansion to strip PROJECT_ROOT prefix from plan path instead of sed interpolation, avoiding special-character fragility - powershell: limit Get-ChildItem to -Depth 1 so plan.md discovery matches the bash glob specs/*/plan.md (one level deep) — fixes cross-platform inconsistency with nested plan.md files - powershell: replace Substring+Length relative-path with [System.IO.Path]::GetRelativePath for robustness across case/PSDrive differences - __init__.py: move agent-context extension install to after save_init_options so init-options.json is present when hooks run - __init__.py: seed context_markers in init-options only when context_file is truthy; avoids noise for integrations without a context file - integrations/base.py: narrow blanket except Exception in _resolve_context_markers to ImportError / (OSError, ValueError) so unexpected bugs surface instead of being silently swallowed * fix: gate context_markers in _update_init_options_for_integration on context_file Apply the same gating logic used during `specify init`: only write context_markers to init-options.json when the integration actually has a context_file set. When switching to an integration without a context file the stale markers are removed, keeping the two init paths consistent. * fix: move context_file/context_markers from init-options.json to agent-context extension config * Potential fix for pull request finding 'Unused global variable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * fix: clarify local import comment in agents.py * Fix remaining agent-context review findings * Fix follow-up agent-context review issues * Address review feedback: narrow except, improve PyYAML messaging, surface config-written note * Fix double-space in PyYAML install hint message * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Address latest agent-context review feedback * Harden bash config parse output handling * Clarify ImportError-only fallback comment * Apply review feedback: drop dead try/except, guard ext-config creation, explicit ConvertFrom-Yaml check * Remove redundant $Options = $null in PS1 catch block * Add constitution directives, deprecation warning, agent-context auto-install, and init flow fix - Add constitution-loading directive to specify, clarify, tasks, checklist, taskstoissues commands - Add deprecation warning (v0.12.0) in upsert_context_section() - Auto-install agent-context extension during specify init - Move context_file from init-options.json to agent-context extension config - Add tests: deprecation warning, corrupt config, constitution directives - Update file inventories across all integration tests * Address review: fix init ordering, test coverage, and hermes inventory - Move agent-context extension install after init-options.json is saved so skill registration can read ai_skills + integration key - Write extension config after install (avoids template overwriting context_file) - Fix test_defaults_when_markers_field_missing to truly test missing markers key - Update hermes tests to allow extension-installed agent-context skill * Address review: chmod ordering, preserve markers, PS1 Python check, YAML key order - Move ensure_executable_scripts after agent-context extension install so extension scripts get execute bits set - Use preserve_markers=True on reinit to keep user-customized markers - Add Python 3 version check in PowerShell fallback (matching bash behavior) - Add sort_keys=False to yaml.safe_dump for stable config output * Address review: path traversal guards and docstring fix - Reject absolute paths and '..' segments in context_file in both bash and PowerShell scripts to prevent writes outside the project root - Fix docstring in _update_init_options_for_integration to accurately describe marker preservation behavior * Address review: strict enabled check, docstring, segment-level path traversal - Use 'is not False' for enabled check so only literal False disables - Update upsert_context_section docstring to mention disabled-extension return - Fix path traversal guards to check actual path segments, not substrings (allows filenames like 'notes..md' while rejecting '../' traversal) * Address review: UnicodeError handling, missing extension warning - Add UnicodeError to exception tuples in _load_agent_context_config and _resolve_context_markers so garbled UTF-8 config files fall back to defaults - Emit error (with reinstall command) instead of silent skip when bundled agent-context extension is not found during init * Address review: bash backslash traversal guard, wheel packaging - Reject backslash separators and Windows drive-letter paths in bash context_file validation (prevents traversal on Git-Bash/Windows) - Add extensions/agent-context to pyproject.toml force-include so the bundled extension is included in wheel builds * Address review: write extension config before init-options.json - Reorder writes in _update_init_options_for_integration so the agent-context extension config is updated first; if it fails, init-options.json remains consistent with the previous state --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
363 lines
15 KiB
Python
363 lines
15 KiB
Python
"""Tests for HermesIntegration.
|
|
|
|
Hermes is special among SkillsIntegration subclasses: it writes skills
|
|
to ``~/.hermes/skills/`` (global) rather than the project-local
|
|
``.hermes/skills/`` directory. A project-local marker (empty directory)
|
|
is created so extension commands (e.g. git) can detect Hermes.
|
|
|
|
All tests that touch ``~/.hermes/`` use ``monkeypatch`` to isolate
|
|
``Path.home()`` to a temp directory so the test suite is hermetic and
|
|
non-destructive to a developer's real Hermes installation.
|
|
"""
|
|
|
|
from pathlib import Path
|
|
|
|
from specify_cli.integrations import get_integration
|
|
from specify_cli.integrations.manifest import IntegrationManifest
|
|
|
|
from .test_integration_base_skills import SkillsIntegrationTests
|
|
|
|
|
|
def _fake_home(tmp_path: Path) -> Path:
|
|
"""Create and return an isolated home directory under *tmp_path*."""
|
|
home = tmp_path / "home"
|
|
home.mkdir(exist_ok=True)
|
|
return home
|
|
|
|
|
|
class TestHermesIntegration(SkillsIntegrationTests):
|
|
KEY = "hermes"
|
|
FOLDER = ".hermes/"
|
|
COMMANDS_SUBDIR = "skills"
|
|
REGISTRAR_DIR = "~/.hermes/skills"
|
|
CONTEXT_FILE = "AGENTS.md"
|
|
|
|
# -- Hermes-specific setup: skills go to ~/.hermes/skills/ -------------
|
|
|
|
def test_setup_writes_to_global_skills_dir(self, tmp_path, monkeypatch):
|
|
"""Skills are written to ~/.hermes/skills/, not project-local."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
created = i.setup(tmp_path, m)
|
|
skill_files = [f for f in created if "scripts" not in f.parts]
|
|
|
|
assert len(skill_files) > 0, "No skill files were created"
|
|
for f in skill_files:
|
|
# Every skill file should be under ~/.hermes/skills/speckit-*/
|
|
expected_prefix = str(home / ".hermes" / "skills")
|
|
assert str(f).startswith(expected_prefix), (
|
|
f"{f} is not under ~/.hermes/skills/"
|
|
)
|
|
|
|
def test_local_marker_dir_created(self, tmp_path, monkeypatch):
|
|
"""Project-local .hermes/skills/ should exist but be empty."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
i.setup(tmp_path, m)
|
|
marker = tmp_path / ".hermes" / "skills"
|
|
assert marker.is_dir(), "Marker directory was not created"
|
|
# Should be empty (no SKILL.md files)
|
|
children = list(marker.iterdir())
|
|
assert children == [], f"Marker directory should be empty, got: {children}"
|
|
|
|
# -- Override shared tests that assume project-local skills ------------
|
|
|
|
def test_setup_writes_to_correct_directory(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes writes to global, not project-local."""
|
|
self.test_setup_writes_to_global_skills_dir(tmp_path, monkeypatch)
|
|
|
|
def test_plan_references_correct_context_file(self, tmp_path, monkeypatch):
|
|
"""Plan skill goes to global dir, but we check it still references AGENTS.md."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
if not i.context_file:
|
|
return
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
i.setup(tmp_path, m)
|
|
# Find the plan skill in global ~/.hermes/skills/
|
|
plan_file = home / ".hermes" / "skills" / "speckit-plan" / "SKILL.md"
|
|
assert plan_file.exists(), f"Plan skill {plan_file} not created globally"
|
|
content = plan_file.read_text(encoding="utf-8")
|
|
assert i.context_file in content, (
|
|
f"Plan skill should reference {i.context_file!r} but it was not found"
|
|
)
|
|
assert "__CONTEXT_FILE__" not in content, (
|
|
"Plan skill has unprocessed __CONTEXT_FILE__ placeholder"
|
|
)
|
|
|
|
def test_all_files_tracked_in_manifest(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes does not track skills in the project manifest
|
|
since they live globally. Only project-local files (scripts,
|
|
templates, context) are tracked."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
created = i.setup(tmp_path, m)
|
|
for f in created:
|
|
# Global files (in ~/.hermes/) are not tracked in manifest
|
|
if str(f).startswith(str(home)):
|
|
continue
|
|
rel = f.resolve().relative_to(tmp_path.resolve()).as_posix()
|
|
assert rel in m.files, f"{rel} not tracked in manifest"
|
|
|
|
def test_install_uninstall_roundtrip(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes uninstall removes global skills + local marker."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
created = i.install(tmp_path, m)
|
|
assert len(created) > 0
|
|
m.save()
|
|
# All SKILL.md files should exist globally
|
|
for f in created:
|
|
if "SKILL.md" in str(f):
|
|
assert f.exists(), f"{f} does not exist"
|
|
# Global skills are removed on teardown without needing force
|
|
removed, skipped = i.teardown(tmp_path, m, force=False)
|
|
for f in created:
|
|
if "SKILL.md" in str(f):
|
|
assert not f.exists(), f"{f} should have been removed"
|
|
# Local marker should be gone
|
|
assert not (tmp_path / ".hermes" / "skills").exists()
|
|
|
|
def test_modified_file_survives_uninstall(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes global skills are ALWAYS removed on uninstall
|
|
(they live outside the project root and aren't hash-tracked in the
|
|
manifest), so a modified global skill is still removed — matching
|
|
the standard behaviour where all integration files are cleaned up."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
created = i.install(tmp_path, m)
|
|
m.save()
|
|
# Pick a global skill file
|
|
skill_files = [f for f in created if "SKILL.md" in str(f)]
|
|
assert len(skill_files) > 0
|
|
modified_file = skill_files[0]
|
|
modified_file.write_text("user modified this", encoding="utf-8")
|
|
removed, skipped = i.uninstall(tmp_path, m)
|
|
assert not modified_file.exists(), (
|
|
"Modified global skill should be removed on teardown (standard behaviour)"
|
|
)
|
|
|
|
def test_modified_global_skill_removed_on_teardown(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes global skills are removed on uninstall regardless
|
|
of the force flag, matching standard integration behaviour."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
created = i.install(tmp_path, m)
|
|
m.save()
|
|
# Pick a global skill file
|
|
skill_files = [f for f in created if "SKILL.md" in str(f)]
|
|
assert len(skill_files) > 0
|
|
modified_file = skill_files[0]
|
|
modified_file.write_text("user modified this", encoding="utf-8")
|
|
# Global skills are removed on teardown regardless of force flag
|
|
removed, skipped = i.teardown(tmp_path, m, force=False)
|
|
assert not modified_file.exists(), (
|
|
"Modified global skill should be removed on teardown (standard behaviour)"
|
|
)
|
|
|
|
def test_pre_existing_skills_not_removed(self, tmp_path, monkeypatch):
|
|
"""Pre-existing non-speckit global skills should survive Hermes uninstall."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
# Create a foreign skill in the global dir first
|
|
global_skills_dir = i._hermes_home_skills_dir()
|
|
foreign_dir = global_skills_dir / "other-tool"
|
|
foreign_dir.mkdir(parents=True, exist_ok=True)
|
|
(foreign_dir / "SKILL.md").write_text("# Foreign skill\n")
|
|
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
i.setup(tmp_path, m)
|
|
|
|
# Run teardown to verify foreign skill survives uninstall
|
|
i.teardown(tmp_path, m)
|
|
|
|
assert (foreign_dir / "SKILL.md").exists(), (
|
|
"Foreign skill was removed by teardown"
|
|
)
|
|
|
|
def test_hook_sections_explain_dotted_command_conversion(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes skills live in global ~/.hermes/skills/."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
i.setup(tmp_path, m)
|
|
specify_skill = home / ".hermes" / "skills" / "speckit-specify" / "SKILL.md"
|
|
assert specify_skill.exists()
|
|
content = specify_skill.read_text(encoding="utf-8")
|
|
assert "replace dots" in content, (
|
|
"speckit-specify should explain dotted hook command conversion"
|
|
)
|
|
assert content.count("replace dots") == content.count(
|
|
"- For each executable hook, output the following"
|
|
)
|
|
|
|
def test_complete_file_inventory_sh(self, tmp_path, monkeypatch):
|
|
"""Override: Hermes init produces no local SKILL.md files,
|
|
only the empty .hermes/skills/ marker."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
project = tmp_path / f"inventory-sh-{self.KEY}"
|
|
project.mkdir()
|
|
old_cwd = Path.cwd()
|
|
import os
|
|
try:
|
|
os.chdir(project)
|
|
result = CliRunner().invoke(app, [
|
|
"init", "--here", "--integration", self.KEY,
|
|
"--script", "sh", "--no-git", "--ignore-agent-tools",
|
|
], catch_exceptions=False)
|
|
finally:
|
|
os.chdir(old_cwd)
|
|
assert result.exit_code == 0, f"init failed: {result.output}"
|
|
actual = sorted(
|
|
p.relative_to(project).as_posix()
|
|
for p in project.rglob("*") if p.is_file()
|
|
)
|
|
# Ensure no core .hermes/skills/speckit-*/SKILL.md in project dir
|
|
# (extension-installed skills like agent-context-update may appear)
|
|
hermes_skill_files = [
|
|
f for f in actual
|
|
if f.startswith(".hermes/skills/speckit-")
|
|
and "agent-context" not in f
|
|
]
|
|
assert hermes_skill_files == [], (
|
|
f"Expected no local core SKILL.md files, found: {hermes_skill_files}"
|
|
)
|
|
# Ensure the marker exists (empty dir won't appear in file listing)
|
|
assert (project / ".hermes" / "skills").is_dir()
|
|
|
|
def test_complete_file_inventory_ps(self, tmp_path, monkeypatch):
|
|
"""Override: Same as sh variant but for PowerShell script type."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
project = tmp_path / f"inventory-ps-{self.KEY}"
|
|
project.mkdir()
|
|
old_cwd = Path.cwd()
|
|
import os
|
|
try:
|
|
os.chdir(project)
|
|
result = CliRunner().invoke(app, [
|
|
"init", "--here", "--integration", self.KEY,
|
|
"--script", "ps", "--no-git", "--ignore-agent-tools",
|
|
], catch_exceptions=False)
|
|
finally:
|
|
os.chdir(old_cwd)
|
|
assert result.exit_code == 0, f"init failed: {result.output}"
|
|
actual = sorted(
|
|
p.relative_to(project).as_posix()
|
|
for p in project.rglob("*") if p.is_file()
|
|
)
|
|
# Ensure no core .hermes/skills/speckit-*/SKILL.md in project dir
|
|
# (extension-installed skills like agent-context-update may appear)
|
|
hermes_skill_files = [
|
|
f for f in actual
|
|
if f.startswith(".hermes/skills/speckit-")
|
|
and "agent-context" not in f
|
|
]
|
|
assert hermes_skill_files == [], (
|
|
f"Expected no local core SKILL.md files, found: {hermes_skill_files}"
|
|
)
|
|
assert (project / ".hermes" / "skills").is_dir()
|
|
|
|
def test_install_uninstall_cleanup(self, tmp_path, monkeypatch):
|
|
"""Verify global skills are cleaned and local marker is removed."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
i = get_integration(self.KEY)
|
|
m = IntegrationManifest(self.KEY, tmp_path)
|
|
created = i.setup(tmp_path, m)
|
|
|
|
# Verify global skills exist
|
|
global_skills = [
|
|
f for f in created
|
|
if "SKILL.md" in str(f)
|
|
and str(f).startswith(str(home / ".hermes"))
|
|
]
|
|
assert len(global_skills) > 0
|
|
for f in global_skills:
|
|
assert f.exists()
|
|
|
|
# Verify local marker exists
|
|
assert (tmp_path / ".hermes" / "skills").is_dir()
|
|
|
|
# Teardown — global skills removed without needing force=True
|
|
removed, skipped = i.teardown(tmp_path, m, force=False)
|
|
|
|
# Global skills removed
|
|
for f in global_skills:
|
|
assert not f.exists(), f"{f} should have been removed"
|
|
|
|
# Local marker removed
|
|
assert not (tmp_path / ".hermes" / "skills").exists(), (
|
|
"Local marker should be removed on teardown"
|
|
)
|
|
|
|
|
|
class TestHermesAutoPromote:
|
|
"""--ai hermes auto-promotes to integration path."""
|
|
|
|
def test_ai_hermes_without_ai_skills_auto_promotes(self, tmp_path, monkeypatch):
|
|
"""--ai hermes should work the same as --integration hermes,
|
|
creating global skills and a local marker."""
|
|
home = _fake_home(tmp_path)
|
|
monkeypatch.setattr(Path, "home", lambda: home)
|
|
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
runner = CliRunner()
|
|
target = tmp_path / "test-proj"
|
|
result = runner.invoke(app, [
|
|
"init", str(target),
|
|
"--ai", "hermes",
|
|
"--no-git",
|
|
"--ignore-agent-tools",
|
|
"--script", "sh",
|
|
])
|
|
|
|
assert result.exit_code == 0, f"init --ai hermes failed: {result.output}"
|
|
# Skills should be in global ~/.hermes/skills/
|
|
assert (home / ".hermes" / "skills" / "speckit-plan" / "SKILL.md").exists()
|
|
# Local marker should exist
|
|
assert (target / ".hermes" / "skills").is_dir()
|
|
# No core SKILL.md files in project-local dir
|
|
# (extension-installed skills like agent-context-update may appear)
|
|
local_skills = [
|
|
d for d in (target / ".hermes" / "skills").iterdir()
|
|
if "agent-context" not in d.name
|
|
]
|
|
assert local_skills == [], f"Local skills dir should be empty, got: {local_skills}"
|