diff --git a/docs/MODEL_SEND_OBSERVABILITY.md b/docs/MODEL_SEND_OBSERVABILITY.md index 3bf479c57..07f8ad89e 100644 --- a/docs/MODEL_SEND_OBSERVABILITY.md +++ b/docs/MODEL_SEND_OBSERVABILITY.md @@ -2,7 +2,9 @@ The physical-send observability contract is implemented in `ouroboros/model_send_seal.py`, wired at -`llm_attempt._candidate_before_dispatch` and swept from `server_maintenance`; +`llm_attempt._candidate_before_dispatch` and swept by the session-custodied +history child `ouroboros/startup_historical_audit.py` (launched after +supervisor readiness, off the readiness path); `tests/test_model_send_seal.py` verifies it. The claim is deliberately narrow: a reconstruction mismatch is an observability fact, not a dispatch gate. @@ -109,8 +111,9 @@ bounded, local, and on the same drive the record was just written to. ### 3.3 Reverse direction (audit, `model_send` only) -A bounded reconciliation sweep (rides the existing startup-sweep family in -`server_maintenance`, not a new scheduler): +A bounded reconciliation sweep (runs inside the session-custodied history +child `startup_historical_audit.py`, started by `server.py::_run_supervisor` +after readiness — not on the readiness path, not a new scheduler): - every `model_send` seal ⟶ exactly one attempt row in the usage-accounting replay (any terminal state, including refused-before-dispatch); @@ -220,8 +223,9 @@ from "our two copies agree" to "the durable record agrees with the wire". - `model_send_seal.py` stamps the physical-candidate manifest, reads back the durable projection, writes typed mismatch facts and reconciles both join directions. The seal is an additive key under the existing manifest schema. -- `server_maintenance.py` runs the bounded reconciliation through the existing - startup sweep. Unknown accounting evidence does not become an orphan claim; +- `startup_historical_audit.py` runs the bounded reconciliation as one + session-custodied child per generation, launched after supervisor readiness + (#1195 F1). Unknown accounting evidence does not become an orphan claim; the sweep records facts without deleting records or fabricating attempts. - `tests/test_model_send_seal.py` covers reconstruction, typed divergence, non-blocking dispatch and reverse joins. Compacted history is resolved through diff --git a/docs/inventories/FACADE_INVENTORY.md b/docs/inventories/FACADE_INVENTORY.md index eb8fc7f18..2859e4aa7 100644 --- a/docs/inventories/FACADE_INVENTORY.md +++ b/docs/inventories/FACADE_INVENTORY.md @@ -2,7 +2,7 @@ AST-derived inventory of compatibility facades, regenerated by `python scripts/regenerate_inventories.py`. Do not edit. A facade row is any runtime module whose top-level `from import ...` statements carry the `noqa: F401` re-export marker — the codebase's declared "this binding exists for its binding, not for this module's own use" convention (reference FACADE_CONSUMERS method). Leaf domains come from `ouroboros/domains.toml`; a leaf outside the facade's domain is marked ✗ (that edge also appears in the manifest's pinned direction matrix). `tests/test_generated_inventories.py` pins byte-identity, so any re-export surface change must regenerate this file. -- facade modules: **60**; marked re-export bindings: **2312**; cross-domain facade→leaf pairs: **132** +- facade modules: **60**; marked re-export bindings: **2313**; cross-domain facade→leaf pairs: **132** | facade | domain | bindings | leaves | |---|---|---:|---| @@ -12,7 +12,7 @@ AST-derived inventory of compatibility facades, regenerated by `python scripts/r | `ouroboros/config.py` | D12 | 133 | `ouroboros/model_slots.py` (17)
`ouroboros/provider_models.py` (6 ✗D02)
`ouroboros/review_model_routes.py` (10)
`ouroboros/runtime_limits.py` (62)
`ouroboros/settings_defaults.py` (19)
`ouroboros/settings_integrity.py` (4)
`ouroboros/settings_scales.py` (13)
`ouroboros/update_channels.py` (2) | | `ouroboros/context.py` | D03 | 4 | `ouroboros/context_runtime_facts.py` (4) | | `ouroboros/delegate_custody.py` | D07 | 10 | `ouroboros/delegate_custody_reconcile.py` (9)
`ouroboros/delegate_evidence.py` (1) | -| `ouroboros/extension_loader.py` | D14 | 100 | `ouroboros/contracts/plugin_api.py` (7 ✗D19)
`ouroboros/extension_child_catalog.py` (8)
`ouroboros/extension_companion.py` (3)
`ouroboros/extension_import_staging.py` (6)
`ouroboros/extension_isolated_deps.py` (4)
`ouroboros/extension_liveness.py` (8)
`ouroboros/extension_plugin_api.py` (6)
`ouroboros/extension_registry_state.py` (20)
`ouroboros/extension_surface_names.py` (12)
`ouroboros/extension_ui_validation.py` (5)
`ouroboros/gateway/host_service.py` (1 ✗D11)
`ouroboros/provider_models.py` (1 ✗D02)
`ouroboros/skill_loader.py` (14)
`ouroboros/skill_token.py` (1)
`ouroboros/tools/skill_exec.py` (1)
`ouroboros/utils.py` (3 ✗D18) | +| `ouroboros/extension_loader.py` | D14 | 101 | `ouroboros/contracts/plugin_api.py` (7 ✗D19)
`ouroboros/extension_child_catalog.py` (8)
`ouroboros/extension_companion.py` (3)
`ouroboros/extension_import_staging.py` (6)
`ouroboros/extension_isolated_deps.py` (4)
`ouroboros/extension_liveness.py` (8)
`ouroboros/extension_plugin_api.py` (6)
`ouroboros/extension_registry_state.py` (20)
`ouroboros/extension_surface_names.py` (12)
`ouroboros/extension_ui_validation.py` (5)
`ouroboros/gateway/host_service.py` (1 ✗D11)
`ouroboros/provider_models.py` (1 ✗D02)
`ouroboros/skill_loader.py` (15)
`ouroboros/skill_token.py` (1)
`ouroboros/tools/skill_exec.py` (1)
`ouroboros/utils.py` (3 ✗D18) | | `ouroboros/gateway/_helpers.py` | D11 | 2 | `ouroboros/jsonl_tail.py` (2 ✗D18) | | `ouroboros/gateway/contracts.py` | D11 | 7 | `ouroboros/gateway/decision_contracts.py` (2)
`ouroboros/gateway/history_contracts.py` (1)
`ouroboros/gateway/schedule_contracts.py` (4) | | `ouroboros/gateway/history.py` | D11 | 1 | `ouroboros/gateway/cost_breakdown.py` (1) | diff --git a/ouroboros/extension_liveness.py b/ouroboros/extension_liveness.py index 389a87fc0..ef673d4b8 100644 --- a/ouroboros/extension_liveness.py +++ b/ouroboros/extension_liveness.py @@ -17,7 +17,7 @@ from ouroboros.extension_registry_state import _extensions, _load_failures, _loc from ouroboros.skill_loader import ( LoadedSkill, _sanitize_skill_name, - discover_selected_skill_candidates, + discover_skill_identity, grant_status_for_skill, skill_conflict_status, ) @@ -191,7 +191,7 @@ def runtime_state_for_skill_name( elif skills is not None: selected = list(skills) else: - selected = discover_selected_skill_candidates(drive_root, skill_name, repo_path=resolved_repo_path) + selected = discover_skill_identity(drive_root, skill_name, repo_path=resolved_repo_path) peer_projection = list(skills) if skills is not None else list( discover_skill_peers(drive_root, repo_path=resolved_repo_path) ) diff --git a/ouroboros/extension_loader.py b/ouroboros/extension_loader.py index 44b2fa0c1..73be49a71 100644 --- a/ouroboros/extension_loader.py +++ b/ouroboros/extension_loader.py @@ -132,7 +132,7 @@ from ouroboros.extension_surface_names import ( extension_surface_name, # noqa: F401 parse_extension_surface_name, # noqa: F401 ) -from ouroboros.skill_loader import _SKILL_DIR_CACHE_NAMES, _sanitize_skill_name, LoadedSkill, SkillPayloadUnreadable, compute_content_hash, discover_skills, discover_selected_skill_candidates, find_skill, grant_status_for_skill, requested_core_setting_keys, skill_conflict_status, skill_review_gate, skill_state_dir, skill_state_dir_path # noqa: F401 +from ouroboros.skill_loader import _SKILL_DIR_CACHE_NAMES, _sanitize_skill_name, LoadedSkill, SkillPayloadUnreadable, compute_content_hash, discover_skills, discover_selected_skill_candidates, discover_skill_identity, find_skill, grant_status_for_skill, requested_core_setting_keys, skill_conflict_status, skill_review_gate, skill_state_dir, skill_state_dir_path # noqa: F401 from ouroboros.skill_token import SkillToken # noqa: F401 from ouroboros.tools.skill_exec import _scrub_env # noqa: F401 from ouroboros.utils import atomic_write_json, read_json_dict, utc_now_iso # noqa: F401 @@ -323,7 +323,7 @@ def reconcile_extension( from ouroboros.config import get_skills_repo_path resolved_repo_path = get_skills_repo_path() if repo_path is None else repo_path - peers = list(skills) if skills is not None else discover_selected_skill_candidates( + peers = list(skills) if skills is not None else discover_skill_identity( drive_root, skill_name, repo_path=resolved_repo_path ) if selected_skill is not None: diff --git a/ouroboros/extension_process_runner.py b/ouroboros/extension_process_runner.py index 855ace1b7..a0a2121c9 100644 --- a/ouroboros/extension_process_runner.py +++ b/ouroboros/extension_process_runner.py @@ -865,7 +865,7 @@ def _load_child_extension(skill_name: str, drive_root: pathlib.Path, repo_dir: p from ouroboros.config import load_settings from ouroboros.extension_loader import load_extension from ouroboros.settings_integrity import _next_task_setting - from ouroboros.skill_loader import discover_selected_skill_candidates + from ouroboros.skill_loader import discover_skill_identity def settings_reader(): live = load_settings() @@ -876,7 +876,7 @@ def _load_child_extension(skill_name: str, drive_root: pathlib.Path, repo_dir: p return {**{key: value for key, value in live.items() if not _next_task_setting(key)}, **task_settings} - skills = discover_selected_skill_candidates(drive_root, skill_name, repo_path=str(skills_repo_path)) + skills = discover_skill_identity(drive_root, skill_name, repo_path=str(skills_repo_path)) skill = next((item for item in skills if item.name == skill_name), None) if skill is None: raise ExtensionProcessError(f"extension skill {skill_name!r} is missing") diff --git a/ouroboros/skill_conflicts.py b/ouroboros/skill_conflicts.py index 149b789a1..415384833 100644 --- a/ouroboros/skill_conflicts.py +++ b/ouroboros/skill_conflicts.py @@ -23,7 +23,7 @@ def enabled_skill_conflicts(skill: Any, skills: List[Any]) -> List[str]: A declaration on either side is authoritative, so one-sided manifests are enforced symmetrically. Missing and disabled peers are deliberately inert. """ - declared = set(skill.manifest.conflicts or []) + declared = set(skill.conflicts or ()) conflicts = { peer.name for peer in skills diff --git a/ouroboros/skill_loader.py b/ouroboros/skill_loader.py index 0a2083a05..9f98fda90 100644 --- a/ouroboros/skill_loader.py +++ b/ouroboros/skill_loader.py @@ -1406,6 +1406,34 @@ def discover_selected_skill_candidates( ) +def discover_skill_identity( + drive_root: pathlib.Path, + name: str, + *, + repo_path: str | None = None, +) -> List[LoadedSkill]: + """Load one identity with ORDINARY discovery semantics, nothing else read. + + Same inventory and collision rules as ``discover_skills`` restricted to one + canonical name — no manifestless opt-in, so a directory without a manifest + beside a valid skill of the same name stays invisible here exactly as it is + to passive discovery. This is the resolver every EXECUTION caller uses + (liveness, reconcile, the extension child); the repair/publication lanes + keep ``discover_selected_skill_candidates`` and its deliberately stricter + manifestless ambiguity. + """ + if repo_path is None: + from ouroboros.config import get_skills_repo_path + + repo_path = get_skills_repo_path() + safe = _sanitize_skill_name(name) + candidates = tuple( + item for item in _skill_location_inventory(drive_root, repo_path=repo_path) + if item.name == safe + ) + return _load_skill_location_candidates(candidates, drive_root=drive_root) + + def find_skill( drive_root: pathlib.Path, name: str, @@ -1414,8 +1442,7 @@ def find_skill( ) -> Optional[LoadedSkill]: """Return one skill by name, including broken manifests with ``load_error``.""" safe = _sanitize_skill_name(name) - candidates = tuple(item for item in _skill_location_inventory(drive_root, repo_path=repo_path) if item.name == safe) - for skill in _load_skill_location_candidates(candidates, drive_root=drive_root): + for skill in discover_skill_identity(drive_root, name, repo_path=repo_path): if skill.name == safe: return skill return None @@ -1560,7 +1587,7 @@ __all__ = [ "AutoGrantOutcome", "LoadedSkill", "HASH_EXEMPT_CONTROL_FILENAMES", "SkillReviewState", "auto_grant_if_enabled", "VALID_REVIEW_STATUSES", "compute_content_hash", "reduce_skill_content_hash", "discover_skills", - "discover_selected_skill_candidates", "find_skill", + "discover_selected_skill_candidates", "discover_skill_identity", "find_skill", "enabled_skill_conflicts", "skill_conflict_status", "grant_status_for_skill", "is_self_authored_skill_dir", "list_available_for_execution", "load_enabled", "load_review_state", "load_skill_grants", "load_skill", diff --git a/tests/test_extension_manifestless_neighbour.py b/tests/test_extension_manifestless_neighbour.py new file mode 100644 index 000000000..ce11cbfa0 --- /dev/null +++ b/tests/test_extension_manifestless_neighbour.py @@ -0,0 +1,66 @@ +"""Execution discovery ignores a manifestless directory beside a valid skill. + +Regression for the #1195 F4 review finding: the execution callers (liveness, +reconcile, the extension child) must resolve a selected identity with ORDINARY +discovery semantics. Only the repair/publication lane may opt a manifestless +directory in as a hidden ambiguity; a reviewed extension next to a same-named +directory without a manifest keeps executing. +""" +from __future__ import annotations + +import pathlib + +from ouroboros import extension_loader +from ouroboros.skill_loader import ( + discover_selected_skill_candidates, + discover_skill_identity, +) + +from tests._extension_loader_shared import _prepare_extension +from tests._extension_loader_shared import ( # noqa: F401 (autouse fixture applies on import) + _clear_loader_state, +) + + +def _manifestless_neighbour(drive_root: pathlib.Path, name: str) -> pathlib.Path: + # Same canonical identity, no SKILL.md: the layout the publish preflight + # treats as ambiguous and ordinary discovery treats as absent. + neighbour = drive_root / "skills" / "external" / name + neighbour.mkdir(parents=True) + (neighbour / "notes.txt").write_text("not a skill\n", encoding="utf-8") + return neighbour + + +def test_manifestless_neighbour_is_invisible_to_execution_discovery(tmp_path): + loaded, repo_root, drive_root = _prepare_extension( + tmp_path, "neighbour_case", "def register(api):\n pass\n", permissions=[], + ) + _manifestless_neighbour(drive_root, loaded.name) + + ordinary = discover_skill_identity(drive_root, loaded.name, repo_path=str(repo_root)) + assert [s.identity_collision for s in ordinary] == [False] + assert ordinary[0].skill_dir.resolve() == loaded.skill_dir.resolve() + + # The stricter repair/publication resolver still sees the ambiguity. + strict = discover_selected_skill_candidates(drive_root, loaded.name, repo_path=str(repo_root)) + assert len(strict) == 2 and all(s.identity_collision for s in strict) + + +def test_reviewed_extension_still_loads_beside_a_manifestless_neighbour(tmp_path): + loaded, repo_root, drive_root = _prepare_extension( + tmp_path, "neighbour_exec", "def register(api):\n pass\n", permissions=[], + ) + _manifestless_neighbour(drive_root, loaded.name) + + state = extension_loader.reconcile_extension( + loaded.name, drive_root, lambda: {}, repo_path=str(repo_root), + ) + assert state["action"] == "extension_loaded", state + assert state["live_loaded"] is True + + runtime = extension_loader.runtime_state_for_skill_name( + loaded.name, drive_root, repo_path=str(repo_root), + ) + assert runtime["live_loaded"] is True + assert runtime.get("identity_collision", False) is False + assert runtime["reason"] != "missing" diff --git a/tests/test_extension_process_runner.py b/tests/test_extension_process_runner.py index 48a734594..8d947b75b 100644 --- a/tests/test_extension_process_runner.py +++ b/tests/test_extension_process_runner.py @@ -113,14 +113,14 @@ def test_child_extension_load_reuses_one_discovered_peer_snapshot(tmp_path, monk permissions=[], ) calls = 0 - real_discover = skill_loader.discover_selected_skill_candidates + real_discover = skill_loader.discover_skill_identity def counted_discover(*args, **kwargs): nonlocal calls calls += 1 return real_discover(*args, **kwargs) - monkeypatch.setattr(skill_loader, "discover_selected_skill_candidates", counted_discover) + monkeypatch.setattr(skill_loader, "discover_skill_identity", counted_discover) monkeypatch.setattr("ouroboros.config.load_settings", lambda: {}) runner._load_child_extension( diff --git a/tests/test_extension_reconcile.py b/tests/test_extension_reconcile.py index dcc2e1b56..87e8383a9 100644 --- a/tests/test_extension_reconcile.py +++ b/tests/test_extension_reconcile.py @@ -166,14 +166,14 @@ def test_reconcile_loads_selected_payload_without_full_peer_discovery(tmp_path, permissions=[], ) calls = 0 - real_discover = extension_loader.discover_selected_skill_candidates + real_discover = extension_loader.discover_skill_identity def counted_discover(*args, **kwargs): nonlocal calls calls += 1 return real_discover(*args, **kwargs) - monkeypatch.setattr(extension_loader, "discover_selected_skill_candidates", counted_discover) + monkeypatch.setattr(extension_loader, "discover_skill_identity", counted_discover) state = extension_loader.reconcile_extension( loaded.name, diff --git a/tests/test_skill_peer_inventory_differential.py b/tests/test_skill_peer_inventory_differential.py index eb6e65658..4e5733e17 100644 --- a/tests/test_skill_peer_inventory_differential.py +++ b/tests/test_skill_peer_inventory_differential.py @@ -245,3 +245,24 @@ def test_skill_peer_is_immutable_and_non_executable(): assert not hasattr(peer, "content_hash") assert not hasattr(peer, "review") assert not hasattr(peer, "manifest") + + +def test_a_peer_is_a_valid_subject_of_the_conflict_projection(tmp_path): + """Duck typing holds on BOTH sides: a SkillPeer subject reaches the same verdict. + + `enabled_skill_conflicts` reads `skill.conflicts`, never `skill.manifest`, + because `SkillPeer` deliberately has no manifest. + """ + drive = tmp_path / "drive" + skills = drive / "skills" / "external" + _write(skills / "declares", _manifest("declares", conflicts=("target",))) + _write(skills / "target", _manifest("target")) + save_enabled(drive, "declares", True) + save_enabled(drive, "target", True) + full = {s.name: s for s in discover_skills(drive, repo_path="")} + peers = {p.name: p for p in discover_skill_peers(drive, repo_path="")} + for name in ("declares", "target"): + as_full = skill_conflict_status(full[name], list(peers.values())) + as_peer = skill_conflict_status(peers[name], list(peers.values())) + assert as_peer is not None, name + assert as_full == as_peer, name