feat(codex): add --no-model-invocation integration option

docs/integrations/codex.md

@@ -26,10 +26,51 @@ Other integrations also target `.agents/skills/`; review
 [shared multi-install rules](cli.md#can-i-install-multiple-integrations-in-the-same-project)
 before installing another agent that shares this directory.
 
+## User-Invocable-Only Skills
+
+To stop Codex invoking the core Spec Kit command skills implicitly:
+
+```bash
+specify init my-project --integration codex --integration-options="--no-model-invocation"
+```
+
+This writes `agents/openai.yaml` with `policy.allow_implicit_invocation: false`
+next to each core command skill. OpenAI's
+[skills documentation](https://learn.chatgpt.com/docs/build-skills) says Codex
+then does not invoke the skill implicitly from the prompt, and explicit `$skill`
+invocation still works. It does not say whether such a skill keeps its name and
+description in Codex's context.
+
+- Extension skills stay model-invocable, because mandatory extension hooks rely
+  on the model invoking them. If an extension's mandatory hook runs a core
+  command (its `command:` is a core `speckit.<command>`), leave this option off:
+  the model cannot run that hook while the core skill is user-invocable only.
+- The option is stored with the integration's other options and reapplied by
+  `specify integration upgrade codex`. Passing `--integration-options` to
+  `upgrade` replaces all stored options (to keep both, pass
+  `--integration-options="--events false --no-model-invocation"`);
+  `--integration-options=""` clears them and undoes the policy. Re-running
+  `specify init` likewise stores only the options passed to it, and re-running
+  it without the option undoes the policy too.
+- Undoing the policy (clearing the option or `specify integration uninstall
+  codex`) deletes the `agents/openai.yaml` files Spec Kit generated. An
+  `agents/openai.yaml` that already existed, or that you edited, keeps its other
+  keys: only `policy.allow_implicit_invocation` is set, and later removed again.
+  Such a file is rewritten as plain YAML, so its comments and formatting are
+  not kept. One that already sets it to `false` is left untouched. If an existing file
+  cannot be read, is not valid YAML, or is not a mapping, setup stops with an
+  error naming the file and leaves it unchanged. Skills that commands other than
+  `init`, `install` and `upgrade` register (for example presets that add core
+  skills) get no `agents/openai.yaml` until `specify integration upgrade codex
+  --force` is run.
+- Preset overrides rewrite only `SKILL.md`, so the sibling `agents/openai.yaml`
+  of an overridden core skill is left in place.
+
 ## Options and Workflow Dispatch
 
 `--integration-options="--skills"` explicitly selects the skills layout, which
-is already the only supported layout for this integration. Native events use
+is already the only supported layout for this integration, and
+`--no-model-invocation` is described above. Native events use
 `.codex/config.toml`; `--events false` disables Spec Kit event handlers.
 See [runtime events](cli.md#runtime-events).
 

src/specify_cli/integrations/codex/__init__.py

@@ -5,12 +5,20 @@
 """
 
 from __future__ import annotations
-from pathlib import Path
 
+import hashlib
+import logging
 from collections.abc import Mapping, Sequence
+from pathlib import Path, PurePosixPath
 from typing import Any
 
+import yaml
+
+from .._file_changes import write_bytes
 from ..base import IntegrationOption, SkillsIntegration
+from ..manifest import IntegrationManifest, _validate_record_path
+
+logger = logging.getLogger(__name__)
 
 
 class CodexIntegration(SkillsIntegration):
@@ -44,6 +52,221 @@ class CodexIntegration(SkillsIntegration):
     events_config_file = ".codex/config.toml"
     events_format = "toml"
 
+    _GENERATED_POLICY: dict[str, Any] = {"policy": {"allow_implicit_invocation": False}}
+
+    @staticmethod
+    def _load_openai_yaml(path: Path, rel: str) -> dict[str, Any]:
+        """Parse an existing ``openai.yaml``; raise rather than guess at unusable content."""
+        try:
+            loaded = yaml.safe_load(path.read_text(encoding="utf-8"))
+        except (yaml.YAMLError, OSError, UnicodeDecodeError) as exc:
+            raise ValueError(f"Cannot update {rel}: {exc}") from exc
+        if loaded is None:  # an empty file holds nothing to preserve
+            return {}
+        if not isinstance(loaded, dict):
+            raise ValueError(
+                f"Cannot update {rel}: expected a YAML mapping, got {type(loaded).__name__}"
+            )
+        policy = loaded.get("policy")
+        if policy is not None and not isinstance(policy, dict):
+            raise ValueError(
+                f"Cannot update {rel}: 'policy' must be a mapping, got {type(policy).__name__}"
+            )
+        return loaded
+
+    def _is_policy_rel(self, rel: str) -> bool:
+        parts = PurePosixPath(rel).parts
+        skills = PurePosixPath(self.registrar_config["dir"]).parts
+        return (
+            len(parts) == len(skills) + 3
+            and parts[: len(skills)] == skills
+            and parts[-2:] == ("agents", "openai.yaml")
+        )
+
+    def _prior_manifest(self, project_root: Path) -> IntegrationManifest | None:
+        """Return the on-disk manifest, which setup's callers save only afterwards."""
+        try:
+            return IntegrationManifest.load(self.key, project_root)
+        except FileNotFoundError:
+            return None
+        except (OSError, ValueError) as exc:
+            logger.warning(
+                "Could not read prior manifest for '%s'; leaving earlier "
+                "agents/openai.yaml policies in place: %s",
+                self.key, exc,
+            )
+            return None
+
+    def _is_generated(self, root: Path, rel: str, prior: IntegrationManifest | None) -> bool:
+        """Whether ``rel`` is a policy-only file an earlier install generated.
+
+        Content equality also counts, so a CRLF checkout or an added comment
+        does not turn Spec Kit's own file into a user file.
+        """
+        path = root / rel
+        if prior is None or prior.ownership_modes.get(rel) != "whole":
+            return False
+        if path.is_symlink() or not path.is_file():
+            return False
+        try:
+            if hashlib.sha256(path.read_bytes()).hexdigest() == prior.files[rel]:
+                return True
+            return self._load_openai_yaml(path, rel) == self._GENERATED_POLICY
+        except (OSError, ValueError):
+            return False
+
+    def _plan_policy(
+        self, project_root: Path, rel: str, prior: IntegrationManifest | None
+    ) -> tuple[str, str, str] | None:
+        """Return ``(rel, content, ownership)`` to write, or ``None`` to leave a user file alone."""
+        path = project_root / rel
+        # Checks every ancestor for symlinks before anything is read or written.
+        _validate_record_path(Path(rel), project_root)
+        tracked = prior is not None and rel in prior.files
+        if not path.exists():
+            data: dict[str, Any] = {}
+            ownership = "whole"
+        elif not path.is_file():
+            raise ValueError(f"Cannot update {rel}: not a regular file")
+        else:
+            data = self._load_openai_yaml(path, rel)
+            ownership = "whole" if self._is_generated(project_root, rel, prior) else "partial"
+        policy = data.get("policy") or {}
+        if ownership == "partial" and not tracked and policy.get("allow_implicit_invocation") is False:
+            # The user already set it; claiming it would let cleanup remove their setting.
+            return None
+        policy["allow_implicit_invocation"] = False
+        data["policy"] = policy
+        return rel, yaml.safe_dump(data, sort_keys=False), ownership
+
+    def _remove_policy(self, project_root: Path, rel: str) -> bool:
+        """Drop the managed policy from a user-owned file; return whether it is now policy-free."""
+        path = project_root / rel
+        try:
+            _validate_record_path(Path(rel), project_root)
+            if not path.is_file():
+                return False
+            data = self._load_openai_yaml(path, rel)
+            policy = data.get("policy")
+            if isinstance(policy, dict) and policy.get("allow_implicit_invocation") is False:
+                del policy["allow_implicit_invocation"]
+                if not policy:
+                    del data["policy"]
+                write_bytes(path, yaml.safe_dump(data, sort_keys=False).encode("utf-8"))
+        except (OSError, ValueError) as exc:
+            logger.warning("Leaving %s unchanged: %s", rel, exc)
+            return False
+        return True
+
+    def _release_prior_policies(
+        self,
+        project_root: Path,
+        manifest: IntegrationManifest,
+        prior: IntegrationManifest,
+        keep: set[str],
+    ) -> None:
+        """Undo policies an earlier install wrote that this setup no longer writes.
+
+        Unmodified generated files are deleted; in user files only the policy is removed.
+        """
+        generated = IntegrationManifest(self.key, project_root, version="policy-cleanup")
+        modes = prior.ownership_modes
+        for rel, recorded_hash in prior.files.items():
+            if rel in keep or not self._is_policy_rel(rel):
+                continue
+            if self._is_generated(project_root, rel, prior):
+                generated._files[rel] = recorded_hash
+                continue
+            cleaned = self._remove_policy(project_root, rel)
+            path = project_root / rel
+            if (modes[rel] == "whole" or not cleaned) and path.is_file() and not path.is_symlink():
+                # A generated file the user edited is theirs now, even if it no
+                # longer parses. Track it as partial so upgrade's stale-file
+                # pass, which deletes whole files with force, leaves it alone;
+                # a file whose policy could not be removed stays tracked so a
+                # later run can retry.
+                try:
+                    manifest.record_existing(rel, ownership="partial")
+                except (OSError, ValueError) as exc:
+                    logger.warning("Could not keep tracking %s: %s", rel, exc)
+        # Each entry was checked above, content-equal ones included.
+        generated.uninstall(project_root, force=True, remove_manifest=False)
+
+    @staticmethod
+    def _is_core_command_skill_dir(name: str) -> bool:
+        from specify_cli.extensions import CORE_COMMAND_NAMES  # lazy: avoids circular import
+
+        return name.startswith("speckit-") and (
+            name.removeprefix("speckit-") in CORE_COMMAND_NAMES
+        )
+
+    def setup(
+        self,
+        project_root: Path,
+        manifest: IntegrationManifest,
+        parsed_options: dict[str, Any] | None = None,
+        **opts: Any,
+    ) -> list[Path]:
+        # Codex has no SKILL.md frontmatter flag; the policy lives in a sibling
+        # ``agents/openai.yaml``. It is written for core command skills only,
+        # since mandatory extension hooks rely on the model invoking them.
+        root = project_root.resolve()
+        enabled = (parsed_options or {}).get("no_model_invocation") is True
+        prior = self._prior_manifest(root)
+        if enabled:
+            # Reject unusable existing policy files before anything is written.
+            from specify_cli.extensions import CORE_COMMAND_NAMES  # lazy: avoids circular import
+
+            skills = PurePosixPath(self.registrar_config["dir"])
+            for name in sorted(CORE_COMMAND_NAMES):
+                rel = (skills / f"speckit-{name}" / "agents" / "openai.yaml").as_posix()
+                agents_dir = (root / rel).parent
+                if agents_dir.exists() or agents_dir.is_symlink():
+                    self._plan_policy(root, rel, prior)
+        created = super().setup(
+            project_root, manifest, parsed_options=parsed_options, **opts
+        )
+        planned: list[tuple[str, str, str]] = []
+        if enabled:
+            for skill_file in [f for f in created if f.name == "SKILL.md"]:
+                if self._is_core_command_skill_dir(skill_file.parent.name):
+                    dest = skill_file.parent / "agents" / "openai.yaml"
+                    plan = self._plan_policy(root, dest.relative_to(root).as_posix(), prior)
+                    if plan is not None:
+                        planned.append(plan)
+        for rel, content, ownership in planned:
+            created.append(manifest.record_file(rel, content, ownership=ownership))
+        if prior is not None:
+            self._release_prior_policies(root, manifest, prior, {rel for rel, _, _ in planned})
+        return created
+
+    def teardown(
+        self,
+        project_root: Path,
+        manifest: IntegrationManifest,
+        *,
+        force: bool = False,
+    ) -> tuple[list[Path], list[Path]]:
+        # ``manifest.uninstall`` never touches partial files, so remove the
+        # managed policy from user-owned ``openai.yaml`` files here, and do not
+        # report the ones cleaned as preserved modified files. Generated files
+        # are left to ``uninstall``: content-equal ones are re-hashed so it
+        # deletes them, and edited ones lose only the policy unless forced.
+        root = project_root.resolve()
+        cleaned: set[Path] = set()
+        for rel, mode in manifest.ownership_modes.items():
+            if not self._is_policy_rel(rel):
+                continue
+            if mode != "whole":
+                if self._remove_policy(root, rel):
+                    cleaned.add(root / rel)
+            elif self._is_generated(root, rel, manifest):
+                manifest.record_existing(rel)
+            elif not force:
+                self._remove_policy(root, rel)
+        removed, skipped = super().teardown(project_root, manifest, force=force)
+        return removed, [path for path in skipped if path not in cleaned]
+
     def build_exec_args(
         self,
         prompt: str,
@@ -77,4 +300,15 @@ def options(cls) -> list[IntegrationOption]:
                 help="Install as agent skills (default for Codex)",
             )
         )
+        opts.append(
+            IntegrationOption(
+                "--no-model-invocation",
+                is_flag=True,
+                default=False,
+                help=(
+                    "Install core command skills as user-invocable only "
+                    "(agents/openai.yaml: allow_implicit_invocation: false)"
+                ),
+            )
+        )
         return opts