diff --git a/docs/integrations/cli.md b/docs/integrations/cli.md index c320a0f5e6..22b84a47bb 100644 --- a/docs/integrations/cli.md +++ b/docs/integrations/cli.md @@ -411,6 +411,7 @@ semantics, examples, and migration caveats live in the relevant agent guide: | GitHub Copilot | [Skills and commands layouts](copilot.md#commands-layout) | | IBM Bob | [Legacy commands and migration](bob.md#legacy-commands-and-migration) | | Kimi Code | [Legacy migration](kimi.md#legacy-migration) | +| opencode | [Hidden Spec Kit files](opencode.md#hidden-spec-kit-files) | Other agent-specific details are listed in the [integration directory](index.md). diff --git a/docs/integrations/opencode.md b/docs/integrations/opencode.md index 4453b31c3a..ab1c9ab369 100644 --- a/docs/integrations/opencode.md +++ b/docs/integrations/opencode.md @@ -37,3 +37,25 @@ Workflow steps use `opencode run`. Slash-prefixed prompts become `--command ` with the remaining text as the message; other prompts are passed as message text. The adapter forwards a configured model with `-m` and requests `--format json` for structured output. + +## Hidden Spec Kit Files + +OpenCode 1.18.31 and 1.19.3 on darwin arm64 discover files with a glob tool that calls ripgrep without `--hidden`. In a temporary Spec Kit tree, `opencode debug rg files --glob '.specify/**/*'` (the same `Ripgrep.glob` call the glob tool makes, with no model involved) returned no paths. `**/*.md` on that same command returned only a visible `README.md`, and missed `.specify/memory/constitution.md`, `.specify/templates/spec-template.md`, `.specify/templates/plan-template.md`, and `.specify/scripts/bash/check-prerequisites.sh`. After the project root `.ignore` contained: + +```text +!.specify/ +!.specify/** +``` + +those patterns returned the constitution, both templates, and the script. A catch-all `**/*` already listed `.specify` files before the exceptions. `!.specify/**` alone still returned no `.specify/**/*` matches; the directory rule is what re-includes the hidden directory on this ripgrep path. Cloud-model fallback prompts and OpenCode 2.x were outside this check. + +Opt in at install or upgrade: + +```bash +specify init my-project --integration opencode --integration-options="--configure-ignore-file" +specify integration upgrade opencode --integration-options="--configure-ignore-file" +``` + +The flag appends whichever of those two rules is not already an exact line, placing them at the end of the root `.ignore`. A later line that excludes `.specify`, a parent directory such as `.specify/memory`, or a file under it (including a template or script init has not created yet, such as `.specify/templates/plan-template.md`) is rejected; move `!.specify/` and `!.specify/**` below it and rerun. A child exception does not uncover a file while its parent directory stays excluded. Bytes already in the file stay as a prefix, including UTF-8 non-ASCII text, CRLF, and a missing final newline. Non-UTF-8 content is rejected before command files are written; convert the file to UTF-8 and rerun, because ripgrep stops at the first invalid byte. Default installs leave `.ignore` untouched. The file is recorded with partial ownership, so `specify integration uninstall` and `specify integration uninstall --force` both leave it in place, exceptions included. Upgrades still compare its hash: adding a user rule blocks a plain `specify integration upgrade opencode` until that change is resolved or `--force` is used, and `--force` also overwrites edited command files. Deleting the two lines is not a lasting opt-out while the stored flag remains, because the next upgrade restores them. Clear the stored flag with `specify integration upgrade opencode --integration-options=""` before removing the lines. + +`.ignore` is a shared ignore file. OpenCode's glob tool sees the rules because it searches with ripgrep, and other tools that read `.ignore` (including fd) can start showing `.specify` as well. `specify integration upgrade opencode` replays the stored flag. Passing `--integration-options` replaces the whole stored set (repeat `--configure-ignore-file` alongside any other flags, such as `--events false`); `--integration-options=""` clears it. diff --git a/src/specify_cli/integrations/opencode/__init__.py b/src/specify_cli/integrations/opencode/__init__.py index 73f1ae24fe..ee2ff4e466 100644 --- a/src/specify_cli/integrations/opencode/__init__.py +++ b/src/specify_cli/integrations/opencode/__init__.py @@ -1,10 +1,23 @@ """opencode integration.""" +import os +import stat from collections.abc import Mapping, Sequence from pathlib import Path from typing import Any -from ..base import MarkdownIntegration +import pathspec + +from ..base import IntegrationOption, MarkdownIntegration +from ..manifest import IntegrationManifest + +# OpenCode 1.18.31 and 1.19.3 glob through ripgrep without ``--hidden``. +# Root ``.ignore`` rules ``!.specify/`` and ``!.specify/**`` make +# ``.specify/**/*`` return constitution, template, and script paths on +# that path (``opencode debug rg files --glob``, the same ``Ripgrep.glob`` +# call the glob tool makes). ``!.specify/**`` alone does not. OpenCode 2.x +# was not part of that check. +_SPECIFY_IGNORE_RULES = ("!.specify/", "!.specify/**") class OpencodeIntegration(MarkdownIntegration): @@ -42,6 +55,47 @@ class OpencodeIntegration(MarkdownIntegration): events_config_file = "opencode.json" events_format = "ts-plugin" + @classmethod + def options(cls) -> list[IntegrationOption]: + opts = super().options() + opts.append( + IntegrationOption( + "--configure-ignore-file", + is_flag=True, + default=False, + help=( + "Append !.specify/ and !.specify/** to the project root " + ".ignore so OpenCode can glob hidden Spec Kit files. " + "Uninstall leaves .ignore in place." + ), + ) + ) + return opts + + def setup( + self, + project_root: Path, + manifest: IntegrationManifest, + parsed_options: dict[str, Any] | None = None, + **opts: Any, + ) -> list[Path]: + """Validate an explicit ``.ignore`` opt-in, then render commands. + + Command files stay on the Markdown layout from ``super().setup()``. + ``--configure-ignore-file`` appends missing ``!.specify/`` and + ``!.specify/**`` rules and records the root file as partial + ownership. Without the flag the root ``.ignore`` is left alone. + """ + configure = (parsed_options or {}).get("configure_ignore_file") is True + if configure: + _preflight_root_ignore(manifest) + created = super().setup( + project_root, manifest, parsed_options=parsed_options, **opts + ) + if configure: + created.append(_configure_root_ignore(manifest)) + return created + def build_exec_args( self, prompt: str, @@ -74,3 +128,131 @@ def build_exec_args( if message: args.append(message) return args + + +def _ignore_newline(raw: bytes) -> bytes: + return b"\r\n" if b"\r\n" in raw else b"\n" + + +def _reject_hidden_specify_paths(path: Path, updated: bytes) -> None: + """Fail when *updated* still hides Spec Kit paths, evaluated in file order.""" + from ..._assets import _locate_core_pack, _repo_root + + patterns = [] + for line in updated.decode("utf-8").splitlines(): + try: + patterns.append(pathspec.GitIgnoreSpec.from_lines([line]).patterns[0]) + except (ValueError, IndexError): + continue + + def ignored(candidate: str) -> bool: + state = False + for pattern in patterns: + hit = pattern.include is not None and pattern.match_file(candidate) is not None + state = bool(pattern.include) if hit else state + return state + + def hides(probe: str) -> bool: + parts = [part for part in probe.split("/") if part] + built, ancestors = "", parts if probe.endswith("/") else parts[:-1] + for part in ancestors: + built = f"{built}/{part}" if built else part + if ignored(built + "/"): + return True + return not probe.endswith("/") and ignored(probe) + + pack, repo = _locate_core_pack(), _repo_root() + probes = {".specify/", ".specify/.gitignore", ".specify/memory/constitution.md"} + for kind in ("templates", "scripts"): + base = (pack if pack and (pack / kind).is_dir() else repo) / kind + walk = () if not base.is_dir() else (base.iterdir() if kind == "templates" else base.rglob("*")) + probes.update( + f".specify/{kind}/{item.relative_to(base).as_posix()}" + for item in walk + if item.is_file() and "__pycache__" not in item.parts + and not item.name.startswith(".") and item.name != "vscode-settings.json" + ) + specify = path.parent / ".specify" + if specify.is_dir() and not specify.is_symlink(): + probes.update(f.relative_to(path.parent).as_posix() for f in specify.rglob("*") if f.is_file()) + hidden = sorted(probe for probe in probes if hides(probe)) + if hidden: + raise ValueError( + f"A later exclusion in {path} hides {hidden[0]}. " + "Move !.specify/ and !.specify/** below it, then rerun." + ) + + +def _with_specify_ignore_rules(raw: bytes) -> bytes: + """Append missing Spec Kit rules, keeping *raw* as a byte prefix. + + New rules use the file's newline (``\\n`` when *raw* is empty) and a + trailing newline. Exact ASCII lines are matched without decoding. + Non-UTF-8 files are rejected before this runs and are not preserved. + """ + if not raw: + newline = b"\n" + body = newline.join(rule.encode("ascii") for rule in _SPECIFY_IGNORE_RULES) + return body + newline + + present = set(raw.splitlines()) + missing = [ + rule + for rule in _SPECIFY_IGNORE_RULES + if rule.encode("ascii") not in present + ] + if not missing: + return raw + + newline = _ignore_newline(raw) + addition = b"" if raw.endswith(b"\n") else newline + addition += newline.join(rule.encode("ascii") for rule in missing) + newline + return raw + addition + + +def _preflight_root_ignore(manifest: IntegrationManifest) -> tuple[Path, bytes, bytes]: + path = manifest.project_root / ".ignore" + if path.is_symlink(): + raise ValueError(f"Refusing to configure symlinked .ignore: {path}") + if path.exists() and not path.is_file(): + raise ValueError( + f"Refusing to configure .ignore because it is not a regular file: {path}" + ) + try: + raw = path.read_bytes() if path.is_file() else b"" + except OSError as exc: + raise OSError(f"Cannot read .ignore at {path}") from exc + try: + raw.decode("utf-8") + except UnicodeDecodeError as exc: + raise ValueError( + f"{path} is not UTF-8 ({exc.reason} at byte {exc.start}). " + "Convert it to UTF-8 and rerun; ripgrep stops at the first invalid byte." + ) from exc + updated = _with_specify_ignore_rules(raw) + _reject_hidden_specify_paths(path, updated) + if updated != raw and path.exists() and not os.access(path, os.W_OK): + raise PermissionError(f"Cannot update .ignore at {path}") + if updated != raw and not os.access(path.parent, os.W_OK): + if path.exists(): + raise PermissionError(f"Cannot update .ignore at {path}") + raise PermissionError(f"Cannot create .ignore at {path}") + return path, raw, updated + + +def _configure_root_ignore(manifest: IntegrationManifest) -> Path: + """Recheck root ``.ignore``, write it atomically, and record partial ownership.""" + path, raw, updated = _preflight_root_ignore(manifest) + if updated == raw and path.is_file(): + from .._file_changes import before_file_change + + before_file_change(path) + manifest.record_existing(".ignore", ownership="partial") + return path + + from ...shared_infra import _write_shared_bytes + + mode = stat.S_IMODE(path.stat().st_mode) if path.exists() else 0o644 + _write_shared_bytes(manifest.project_root, path, updated, mode=mode) + manifest.record_existing(".ignore", ownership="partial") + return path diff --git a/tests/integrations/test_integration_opencode.py b/tests/integrations/test_integration_opencode.py index 947f885d8f..b2b1dce40c 100644 --- a/tests/integrations/test_integration_opencode.py +++ b/tests/integrations/test_integration_opencode.py @@ -1,6 +1,12 @@ """Tests for OpencodeIntegration.""" +import json +import os +import stat import warnings +from unittest.mock import patch + +import pytest from specify_cli.agents import CommandRegistrar from specify_cli.integrations import get_integration @@ -8,6 +14,8 @@ from .test_integration_base_markdown import MarkdownIntegrationTests +_OPT_IN = {"configure_ignore_file": True} + class TestOpencodeIntegration(MarkdownIntegrationTests): KEY = "opencode" @@ -200,3 +208,319 @@ def test_setup_writes_to_canonical_dir(self, tmp_path): assert canonical.is_dir() assert not legacy.exists() assert any(canonical.glob("speckit.*.md")) + + def test_configure_ignore_file_opt_in_records_partial_and_renders_commands(self, tmp_path): + """Opt-in appends both rules, renders commands, and records partial ownership.""" + integration = get_integration(self.KEY) + manifest = IntegrationManifest(self.KEY, tmp_path) + created = integration.setup(tmp_path, manifest, parsed_options=_OPT_IN) + + ignore = tmp_path / ".ignore" + assert ignore.read_bytes() == b"!.specify/\n!.specify/**\n" + assert any(path.resolve() == ignore.resolve() for path in created) + assert manifest.ownership_modes[".ignore"] == "partial" + assert ".ignore" in manifest.files + + canonical = tmp_path / ".opencode" / "commands" + command_files = list(canonical.glob("speckit.*.md")) + assert command_files + plan = canonical / integration.command_filename("plan") + text = plan.read_text(encoding="utf-8") + assert "{SCRIPT}" not in text + assert "__AGENT__" not in text + + def test_configure_ignore_file_preserves_prefix_and_repeats_cleanly(self, tmp_path): + """Existing bytes stay a prefix; missing rules are appended once.""" + integration = get_integration(self.KEY) + cases = { + "empty": (b"", b"!.specify/\n!.specify/**\n"), + "lf-non-ascii-no-final-newline": ( + "café keep".encode(), + "café keep\n!.specify/\n!.specify/**\n".encode(), + ), + "crlf-non-ascii-no-final-newline": ( + "café\r\nkeep".encode(), + "café\r\nkeep\r\n!.specify/\r\n!.specify/**\r\n".encode(), + ), + "dir-rule-only": (b"!.specify/\n", b"!.specify/\n!.specify/**\n"), + "contents-rule-only": (b"!.specify/**\n", b"!.specify/**\n!.specify/\n"), + "both-rules": ( + "café\r\n!.specify/\r\n!.specify/**\r\n".encode(), + "café\r\n!.specify/\r\n!.specify/**\r\n".encode(), + ), + "broader-exclusions": ( + b"*\n.*\n.specify/**\n", + b"*\n.*\n.specify/**\n!.specify/\n!.specify/**\n", + ), + } + for name, (original, expected) in cases.items(): + root = tmp_path / name + root.mkdir() + ignore = root / ".ignore" + ignore.write_bytes(original) + ignore.chmod(0o640) + before_mode = stat.S_IMODE(ignore.stat().st_mode) + manifest = IntegrationManifest(self.KEY, root) + integration.setup(root, manifest, parsed_options=_OPT_IN) + assert ignore.read_bytes() == expected + assert ignore.read_bytes().startswith(original) + assert stat.S_IMODE(ignore.stat().st_mode) == before_mode + assert manifest.ownership_modes[".ignore"] == "partial" + integration.setup( + root, IntegrationManifest(self.KEY, root), parsed_options=_OPT_IN + ) + assert ignore.read_bytes() == expected + assert stat.S_IMODE(ignore.stat().st_mode) == before_mode + + def test_configure_ignore_file_rejects_non_utf8_and_overrides(self, tmp_path): + integration = get_integration(self.KEY) + for index, (original, match) in enumerate(( + (b"# caf\xe9\n", "Convert it to UTF-8"), + (b"!.specify/\n!.specify/**\n.specify/**\n", "later exclusion"), + (b"!.specify/\n!.specify/**\n/.specify/templates/**\n", "later exclusion"), + (b"!.specify/\n!.specify/**\n.specify/memory/custom.md\n", "custom.md"), + (b"!.specify/\n!.specify/**\n.specify/templates/plan-template.md\n", "plan-template.md"), + (b"!.specify/\n!.specify/**\n.specify/memory\n!.specify/memory/*.md\n", "constitution.md"), + )): + root = tmp_path / str(index) + (root / ".specify" / "memory").mkdir(parents=True) + (root / ".specify" / "memory" / "custom.md").write_text("x\n") + (root / ".ignore").write_bytes(original) + with pytest.raises(ValueError, match=match): + integration.setup(root, IntegrationManifest(self.KEY, root), parsed_options=_OPT_IN) + assert (root / ".ignore").read_bytes() == original and not (root / ".opencode").exists() + restored = b"!.specify/\n!.specify/**\n.specify/**\n!.specify/**\n" + (tmp_path / ".ignore").write_bytes(restored) + integration.setup(tmp_path, IntegrationManifest(self.KEY, tmp_path), parsed_options=_OPT_IN) + assert (tmp_path / ".ignore").read_bytes() == restored + kept = b"!.specify/\n!.specify/**\n.specify/memory\n!.specify/memory/\n!.specify/memory/**\n" + reinclude = tmp_path / "reinclude" + reinclude.mkdir() + (reinclude / ".ignore").write_bytes(kept) + integration.setup(reinclude, IntegrationManifest(self.KEY, reinclude), parsed_options=_OPT_IN) + assert (reinclude / ".ignore").read_bytes() == kept + + def test_without_flag_and_other_integrations_leave_ignore_untouched(self, tmp_path): + plain = tmp_path / "plain" + plain.mkdir() + get_integration(self.KEY).setup(plain, IntegrationManifest(self.KEY, plain)) + assert not (plain / ".ignore").exists() + + other_root = tmp_path / "other" + other_root.mkdir() + (other_root / ".ignore").write_bytes(b"user\n") + get_integration("gemini").setup( + other_root, IntegrationManifest("gemini", other_root) + ) + assert (other_root / ".ignore").read_bytes() == b"user\n" + + def test_flag_parses_with_events_and_persists_across_upgrade(self, tmp_path): + from specify_cli.integrations._helpers import _parse_integration_options + from tests.specify_cli.integrations._helpers import _init_project, _run_in_project + + integration = get_integration(self.KEY) + parsed = _parse_integration_options( + integration, "--configure-ignore-file --events false" + ) + assert parsed == {"configure_ignore_file": True, "events": "false"} + names = [option.name for option in integration.options()] + assert "--events" in names + assert "--configure-ignore-file" in names + + project = _init_project( + tmp_path, "opencode", "--configure-ignore-file --events false" + ) + ignore = project / ".ignore" + state = json.loads( + (project / ".specify" / "integration.json").read_text(encoding="utf-8") + ) + settings = state["integration_settings"]["opencode"] + assert settings["parsed_options"] == { + "configure_ignore_file": True, + "events": "false", + } + + ignore.write_bytes(b"keep me\n") + result = _run_in_project(project, ["integration", "upgrade", "opencode", "--force"]) + assert result.exit_code == 0, result.output + assert ignore.read_bytes() == b"keep me\n!.specify/\n!.specify/**\n" + result = _run_in_project(project, ["integration", "upgrade", "opencode", "--force"]) + assert result.exit_code == 0, result.output + assert ignore.read_bytes() == b"keep me\n!.specify/\n!.specify/**\n" + + def test_uninstall_preserves_partial_ignore_even_when_forced(self, tmp_path): + integration = get_integration(self.KEY) + for force in (False, True): + root = tmp_path / f"force-{force}" + root.mkdir() + manifest = IntegrationManifest(self.KEY, root) + integration.setup(root, manifest, parsed_options=_OPT_IN) + ignore = root / ".ignore" + expected = ignore.read_bytes() + b"user-rule\n" + ignore.write_bytes(expected) + removed, skipped = integration.teardown(root, manifest, force=force) + assert ignore.read_bytes() == expected + assert ignore.resolve() not in {path.resolve() for path in removed} + assert ignore.resolve() in {path.resolve() for path in skipped} + + def test_configure_ignore_file_rejects_unsafe_destinations(self, tmp_path): + integration = get_integration(self.KEY) + outside = tmp_path / "outside-secret" + outside.write_bytes(b"secret\n") + + linked = tmp_path / "linked" + linked.mkdir() + integration.setup(linked, IntegrationManifest(self.KEY, linked)) + plan = linked / ".opencode" / "commands" / integration.command_filename("plan") + plan.write_bytes(b"custom-command\n") + link = linked / ".ignore" + link.symlink_to(outside) + with pytest.raises(ValueError, match="symlinked"): + integration.setup(linked, IntegrationManifest(self.KEY, linked), parsed_options=_OPT_IN) + assert plan.read_bytes() == b"custom-command\n" + assert link.is_symlink() + assert outside.read_bytes() == b"secret\n" + + directory = tmp_path / "directory" + directory.mkdir() + ignore_dir = directory / ".ignore" + ignore_dir.mkdir() + (ignore_dir / "user.txt").write_text("keep", encoding="utf-8") + with pytest.raises(ValueError, match="not a regular file"): + integration.setup( + directory, IntegrationManifest(self.KEY, directory), parsed_options=_OPT_IN + ) + assert not (directory / ".opencode").exists() + assert (ignore_dir / "user.txt").read_text(encoding="utf-8") == "keep" + + unreadable = tmp_path / "unreadable" + unreadable.mkdir() + blocked = unreadable / ".ignore" + blocked.write_bytes(b"user\n") + blocked.chmod(0) + try: + with pytest.raises(OSError): + integration.setup( + unreadable, + IntegrationManifest(self.KEY, unreadable), + parsed_options=_OPT_IN, + ) + finally: + blocked.chmod(0o644) + assert blocked.read_bytes() == b"user\n" + assert not (unreadable / ".opencode").exists() + + readonly = tmp_path / "readonly" + readonly.mkdir() + frozen = readonly / ".ignore" + frozen.write_bytes(b"user\n") + frozen.chmod(0o444) + try: + with pytest.raises(OSError): + integration.setup( + readonly, IntegrationManifest(self.KEY, readonly), parsed_options=_OPT_IN + ) + finally: + frozen.chmod(0o644) + assert frozen.read_bytes() == b"user\n" + assert not (readonly / ".opencode").exists() + + failing = tmp_path / "write-fail" + failing.mkdir() + target = failing / ".ignore" + original = b"user-rule-keep-this-prefix-intact\n" + target.write_bytes(original) + real_fdopen = os.fdopen + + def fdopen_fail(fd, mode="r", *args, **kwargs): + stream = real_fdopen(fd, mode, *args, **kwargs) + if mode != "wb": + return stream + raw_write = stream.write + + class _Stream: + def write(self, data): + payload = bytes(data) + if b"!.specify/" in payload: + raw_write(payload[:8]) + raise OSError("disk full") + return raw_write(data) + + def __enter__(self): + return self + + def __exit__(self, *exc): + stream.close() + return False + + return _Stream() + + with patch("os.fdopen", fdopen_fail): + with pytest.raises(OSError, match="disk full"): + integration.setup( + failing, IntegrationManifest(self.KEY, failing), parsed_options=_OPT_IN + ) + assert target.read_bytes() == original + assert (failing / ".opencode" / "commands").is_dir() + assert list(failing.glob("..ignore.*")) == [] + + @pytest.mark.skipif( + os.name == "nt", + reason="Windows reports directories writable regardless of mode bits", + ) + def test_configure_ignore_file_rejects_writable_ignore_in_readonly_root(self, tmp_path): + integration = get_integration(self.KEY) + root = tmp_path / "readonly-root" + root.mkdir() + integration.setup(root, IntegrationManifest(self.KEY, root)) + plan = root / ".opencode" / "commands" / integration.command_filename("plan") + plan.write_bytes(b"custom-command\n") + ignore = root / ".ignore" + ignore.write_bytes(b"user\n") + ignore.chmod(0o644) + root.chmod(0o555) + try: + assert os.access(ignore, os.W_OK) + assert not os.access(root, os.W_OK) + with pytest.raises(PermissionError, match="Cannot update .ignore"): + integration.setup( + root, IntegrationManifest(self.KEY, root), parsed_options=_OPT_IN + ) + finally: + root.chmod(0o755) + assert ignore.read_bytes() == b"user\n" + assert plan.read_bytes() == b"custom-command\n" + + def test_upgrade_rolls_back_existing_ignore_when_a_later_step_fails(self, tmp_path): + from specify_cli.integrations import installer + from tests.specify_cli.integrations._helpers import _init_project, _run_in_project + + project = _init_project(tmp_path, "opencode") + ignore = project / ".ignore" + original = "café rule\r\n".encode() + ignore.write_bytes(original) + # Built-in upgrades open the file journal when an external package + # record is present. The sentinel starts that transaction without + # loading another adapter. + with ( + patch.object(installer, "read_records", return_value={"sample": {"id": "sample"}}), + patch.object(installer, "load_installed_integrations", return_value=[]), + patch( + "specify_cli.integrations.command_upgrade._write_integration_json", + side_effect=OSError("later failure"), + ), + ): + result = _run_in_project( + project, + [ + "integration", + "upgrade", + "opencode", + "--force", + "--integration-options", + "--configure-ignore-file", + ], + ) + assert result.exit_code != 0 + assert "later failure" in result.output + assert ignore.read_bytes() == original