Phase 0 Research — ToolSurfaceContract Residual Closeout

The four plan-phase decisions, resolved by inspecting the merged code on feat/tool-surface-contract-residuals (= current main + PR #1948).

D-1 — #1940 canonical finding-code vocabulary + manifest schema

  • Decision: Implement exactly the four codes already specified in kitty-specs/tool-surface-contract-01KV2K2P/data-model.md — profile-source-invalid, profile-name-invalid, profile-overlay-conflict, profile-sentinel-skipped — and add the manifest fields source_path, source_hash, projection_version to reach the mandated 8-field provenance. Do not invent new names.
  • Rationale: src/specify_cli/tool_surface/findings.py currently defines only 3 of the 7 profile codes (native-agent-profile-missing, native-agent-profile-drift, profile-projection-unsupported); the other 4 exist only in data-model.md. profiles/manifest.py serializes 6 fields (profile_urn, source_layer, tool_key, output_path, format, file_hash). DIRECTIVE_010 (Specification Fidelity) + C-004 require implementing the spec'd vocabulary verbatim. Existing 3 codes are append-only/stable — must not be renamed.
  • Emit conditions (from data-model.md): profile-source-invalid = canonical profile YAML fails schema/repository validation; profile-name-invalid = profile ID/name invalid for the target native format; profile-overlay-conflict = overlay resolution ambiguous/unsafe; profile-sentinel-skipped = sentinel/internal profile intentionally not projected (info-severity, never silent).
  • Alternatives considered: collapse into the existing generic codes (rejected — loses the diagnostic specificity the AC mandates); add a referenced_tests-style schema extension (rejected — out of scope, and #1948 already folded such fields into existing schema lists).
  • Manifest migration note: loader must read pre-existing 6-field entries without crashing (treat the 3 new fields as optional-on-read, written-on-projection) so existing .kittify/agent-profiles-manifest.json files survive an upgrade.

D-2 — #1941 registry-backed SKILL_ONLY_AGENTS / VALID_AGENTS

  • Decision: Derive SKILL_ONLY_AGENTS from command_installer.SUPPORTED_AGENTS (the existing canonical skill-only-tool roster) and keep VALID_AGENTS = set(AGENT_DIR_TO_KEY.values()) | SKILL_ONLY_AGENTS as a derived union — removing the duplicated literal.
  • Rationale: config.py:51 hardcodes SKILL_ONLY_AGENTS = {"codex","vibe","pi","letta"}, byte-identical to command_installer.SUPPORTED_AGENTS = ("codex","vibe","pi","letta") (command_installer.py:45). This is connascence-of-value across two locations (connascence-analysis); collapsing to one source is a behavior-preserving reduction (the values are equal today, so accept/reject is unchanged — pinned by test_agent_config_compat.py).
  • Alternatives considered: route through the full tool_surface registry (rejected for now — heavier, and command_installer.SUPPORTED_AGENTS is already the canonical skill-only source; registry-routing is a larger refactor outside this residual's scope). Leave as-is (rejected — that's the AC violation).
  • Risk: importing command_installer into config.py — verify no import cycle (config.py already imports from skills indirectly; _register_skill_agent does a local import of command_installer, so a module-top import must be cycle-checked, else keep the existing lazy-import pattern and compute the set at module load via a small helper).

D-3 — #1942 docs-lint CI enforcement mechanism

  • Decision: Do both — (a) re-mark tests/specify_cli/tool_surface/test_docs.py from unit to integration, and (b) add a tool_surface entry to the dorny/paths-filter config + the shard mapping in .github/workflows/ci-quality.yml so the integration-collecting integration-tests-core-misc shard runs it on changes to src/specify_cli/tool_surface/ or tests/specify_cli/tool_surface/.
  • Rationale: ci-quality.yml shards select by BOTH a paths-filter (which files changed → which shard runs) AND a pytest -m marker (fast vs git_repo or integration or architectural). The current unit marker is excluded by every shard's -m selector, AND there is no tool_surface path-filter entry — a double invisibility. Fixing only the marker without a path filter (or vice versa) still leaves the gate uncollected. The integration shard's -m '... or integration or ...' selector will collect an integration-marked test once the path filter triggers the shard.
  • Verification: the IC-00 adversarial test (inject an unregistered .agents/skills/spec-kitty.* doc reference) must turn that shard RED in CI; a clean tree stays green (NFR-004).
  • Alternatives considered: a dedicated standalone CI job (rejected — more workflow surface than reusing the existing core-misc integration shard); leave unit + only add path filter (rejected — marker still excludes it).

D-4 — #1965 deterministic locate_project_root / doctor-skills test

  • Decision: Make SPECIFY_REPO_ROOT authoritative in src/specify_cli/core/paths.py::locate_project_root — when the env var is set and the path exists, return get_main_repo_root(env_path) regardless of whether it contains .kittify/ (drop the (env_path / KITTIFY_DIR).is_dir() precondition at paths.py:79). Add a regression test (test-first) that sets SPECIFY_REPO_ROOT to a .kittify-less temp dir and asserts resolution returns it, not the ambient checkout.
  • Rationale: line 79 only honors the override when the target also has .kittify/; the doctor-skills error-schema test points SPECIFY_REPO_ROOT at a .kittify-less temp dir, so the guard fails and Tier-2 walk-up finds the real checkout → leaks ambient ~/.claude state into the supposedly-isolated test. The docstring already declares the env var "highest priority", so honoring it fully is the principled fix.
  • C-003 scope check: real .kittify/ projects set SPECIFY_REPO_ROOT (or don't) to a dir that has .kittify/ → they hit the same get_main_repo_root(env_path) branch either way → no behavior change for them. The only changed behavior is for an explicitly-set, existing, non-.kittify path, which today is silently ignored (surprising) and after the fix is honored (correct). Keep the env_path.exists() guard (non-existent paths still fall through). Audit the sibling locate_project_root in core/project_resolver.py and __init__.py:52 so they don't reintroduce the leak for their callers (e.g. spec-kitty lint).
  • Alternatives considered: isolate the test only (monkeypatch cwd + Path.home) without touching paths.py (rejected — leaves the underlying "override silently ignored" surprise in place; the env-var contract should mean what the docstring says).