q792602257andClaude Opus 4.6
b94abde92a
Consumes the external Subscription Platform as source of truth for skill content; reuses skills_learning domain models (extended with KnowledgeSkill) and workflow.skill_exec resolver. HTTP/MCP deps land in api/ per CONSTITUTION.md; synced skills use a physically separate SQLite file (tasks/skills.sqlite3) to preserve the skill-authoring capability boundary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
477 lines
15 KiB
Python
477 lines
15 KiB
Python
"""Tests for api/skill_sync.py.
|
|
|
|
Covers task 3.7: create/update/remove sync scenarios, push-triggered
|
|
immediate sync, query-time revocation re-check, and failure-preserves-cache.
|
|
All tests use a FakeSubscriptionClient implementing the Protocol — no real
|
|
HTTP traffic.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from typing import Any
|
|
|
|
import pytest
|
|
|
|
from api.skill_sync import (
|
|
HttpSubscriptionClient,
|
|
SkillSyncRunner,
|
|
SyncDelta,
|
|
SyncOutcome,
|
|
_parse_sync_payload,
|
|
register_skill_sync_webhook,
|
|
)
|
|
from skills_learning.models import (
|
|
FlowStep,
|
|
FlowTemplateSkill,
|
|
KnowledgeSkill,
|
|
SkillMetadata,
|
|
)
|
|
from storage.skill_catalog import SkillCatalogStore
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Test fakes and helpers
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
class FakeSubscriptionClient:
|
|
"""In-process implementation of the SubscriptionClient Protocol."""
|
|
|
|
def __init__(self) -> None:
|
|
self.skills_by_sub: dict[str, list[Any]] = {}
|
|
self.delta_overrides: dict[str, SyncDelta] = {}
|
|
self.next_failure: Exception | None = None
|
|
self.call_log: list[tuple[str, int | None]] = []
|
|
|
|
def set_entitled_skills(self, sub_id: str, skills: list[Any]) -> None:
|
|
self.skills_by_sub[sub_id] = list(skills)
|
|
|
|
def set_delta(self, sub_id: str, delta: SyncDelta) -> None:
|
|
self.delta_overrides[sub_id] = delta
|
|
|
|
def fail_next_with(self, exc: Exception) -> None:
|
|
self.next_failure = exc
|
|
|
|
def fetch_entitled_skills(
|
|
self,
|
|
subscription_id: str,
|
|
since_version: int | None = None,
|
|
) -> SyncDelta:
|
|
self.call_log.append((subscription_id, since_version))
|
|
if self.next_failure is not None:
|
|
exc = self.next_failure
|
|
self.next_failure = None
|
|
raise exc
|
|
if subscription_id in self.delta_overrides:
|
|
return self.delta_overrides[subscription_id]
|
|
skills = self.skills_by_sub.get(subscription_id, [])
|
|
return SyncDelta(
|
|
skills=list(skills),
|
|
removed_ids=[],
|
|
latest_version=1,
|
|
is_full_replace=True,
|
|
)
|
|
|
|
|
|
def _knowledge(skill_id: str, name: str, content: str) -> KnowledgeSkill:
|
|
return KnowledgeSkill(
|
|
metadata=SkillMetadata(
|
|
id=skill_id,
|
|
name=name,
|
|
kind="knowledge",
|
|
source="subscription",
|
|
),
|
|
content=content,
|
|
)
|
|
|
|
|
|
def _flow(skill_id: str, name: str, steps: list[tuple[str, dict]]) -> FlowTemplateSkill:
|
|
return FlowTemplateSkill(
|
|
metadata=SkillMetadata(
|
|
id=skill_id,
|
|
name=name,
|
|
kind="flow_template",
|
|
source="subscription",
|
|
),
|
|
steps=[FlowStep(tool_name=t, args=a) for t, a in steps],
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def store(tmp_path):
|
|
return SkillCatalogStore(db_path=tmp_path / "skills.sqlite3")
|
|
|
|
|
|
@pytest.fixture
|
|
def fake():
|
|
return FakeSubscriptionClient()
|
|
|
|
|
|
@pytest.fixture
|
|
def runner(store, fake):
|
|
return SkillSyncRunner(
|
|
store=store,
|
|
client=fake,
|
|
subscriptions=["sub-a"],
|
|
poll_interval=0.01,
|
|
)
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Task 3.3 / 3.5: poll loop apply scenarios (create / update / remove)
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_tick_creates_new_skills_in_catalog(store, fake, runner):
|
|
fake.set_entitled_skills(
|
|
"sub-a",
|
|
[_knowledge("k1", "Knowledge", "body"), _flow("f1", "Flow", [("tap", {})])],
|
|
)
|
|
outcomes = runner.tick()
|
|
assert outcomes["sub-a"].success is True
|
|
assert outcomes["sub-a"].fetched == 2
|
|
ids = {m.id for m in store.list_skills({"sub-a"})}
|
|
assert ids == {"k1", "f1"}
|
|
|
|
|
|
def test_tick_updates_existing_skill_content(store, fake, runner):
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "v1")])
|
|
runner.tick()
|
|
fetched = store.get_skill("k1", {"sub-a"})
|
|
assert isinstance(fetched, KnowledgeSkill)
|
|
assert fetched.content == "v1"
|
|
|
|
# Sync again with new content
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "v2-updated")])
|
|
runner.tick()
|
|
fetched = store.get_skill("k1", {"sub-a"})
|
|
assert isinstance(fetched, KnowledgeSkill)
|
|
assert fetched.content == "v2-updated"
|
|
|
|
|
|
def test_tick_removes_skills_no_longer_entitled(store, fake, runner):
|
|
fake.set_entitled_skills(
|
|
"sub-a",
|
|
[_knowledge("k1", "K", "c"), _knowledge("k2", "K2", "c")],
|
|
)
|
|
runner.tick()
|
|
assert {m.id for m in store.list_skills({"sub-a"})} == {"k1", "k2"}
|
|
|
|
# Full replace with only k1 → k2 must disappear
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner.tick()
|
|
assert {m.id for m in store.list_skills({"sub-a"})} == {"k1"}
|
|
|
|
|
|
def test_tick_applies_incremental_delta_via_upsert_and_remove(store, fake, runner):
|
|
# Seed with full sync
|
|
fake.set_entitled_skills(
|
|
"sub-a",
|
|
[_knowledge("k1", "K1", "c1"), _knowledge("k2", "K2", "c2")],
|
|
)
|
|
runner.tick()
|
|
|
|
# Now simulate an incremental delta: k1 content updated, k2 removed, k3 added
|
|
fake.set_delta(
|
|
"sub-a",
|
|
SyncDelta(
|
|
skills=[_knowledge("k1", "K1", "c1-updated"), _knowledge("k3", "K3", "c3")],
|
|
removed_ids=["k2"],
|
|
latest_version=5,
|
|
is_full_replace=False,
|
|
),
|
|
)
|
|
outcomes = runner.tick()
|
|
assert outcomes["sub-a"].success is True
|
|
assert outcomes["sub-a"].fetched == 2
|
|
|
|
ids = {m.id for m in store.list_skills({"sub-a"})}
|
|
assert ids == {"k1", "k3"}
|
|
fetched = store.get_skill("k1", {"sub-a"})
|
|
assert isinstance(fetched, KnowledgeSkill)
|
|
assert fetched.content == "c1-updated"
|
|
|
|
|
|
def test_tick_skips_unknown_subscriptions_in_outcome(store, fake):
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner = SkillSyncRunner(
|
|
store=store,
|
|
client=fake,
|
|
subscriptions=["sub-a", "sub-b"],
|
|
poll_interval=0.01,
|
|
)
|
|
outcomes = runner.tick()
|
|
# sub-b has no skills set → empty delta, success, fetched=0
|
|
assert outcomes["sub-b"].success is True
|
|
assert outcomes["sub-b"].fetched == 0
|
|
assert {m.id for m in store.list_skills({"sub-a"})} == {"k1"}
|
|
assert store.list_skills({"sub-b"}) == []
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Task 3.6: sync failure preserves cache + records last_error
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_tick_failure_preserves_cache_and_records_error(store, fake, runner):
|
|
# Initial successful sync
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner.tick()
|
|
assert store.get_skill("k1", {"sub-a"}) is not None
|
|
|
|
# Next tick fails — cache must remain, error recorded
|
|
fake.fail_next_with(RuntimeError("503 Service Unavailable"))
|
|
outcomes = runner.tick()
|
|
assert outcomes["sub-a"].success is False
|
|
assert "503 Service Unavailable" in (outcomes["sub-a"].error or "")
|
|
assert outcomes["sub-a"].fetched == 0
|
|
|
|
# Cache preserved
|
|
fetched = store.get_skill("k1", {"sub-a"})
|
|
assert isinstance(fetched, KnowledgeSkill)
|
|
assert fetched.content == "c"
|
|
|
|
# last_error recorded in subscriptions table
|
|
with store._connect() as conn:
|
|
row = conn.execute(
|
|
"SELECT last_error FROM subscriptions WHERE id = ?", ("sub-a",)
|
|
).fetchone()
|
|
assert row is not None
|
|
assert "503 Service Unavailable" in (row["last_error"] or "")
|
|
|
|
|
|
def test_tick_failure_then_recovery_clears_error(store, fake, runner):
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner.tick()
|
|
|
|
fake.fail_next_with(ConnectionError("network down"))
|
|
runner.tick()
|
|
# Successful sync after failure must clear last_error
|
|
runner.tick()
|
|
with store._connect() as conn:
|
|
row = conn.execute(
|
|
"SELECT last_error FROM subscriptions WHERE id = ?", ("sub-a",)
|
|
).fetchone()
|
|
assert row["last_error"] is None
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Task 3.5: query-time revocation before next sync
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_revocation_takes_effect_immediately_without_resync(store, fake, runner):
|
|
"""Marking a subscription inactive hides its skills on the next query,
|
|
even if no further sync tick has run."""
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner.tick()
|
|
assert store.get_skill("k1", {"sub-a"}) is not None
|
|
|
|
# Revoke sub-a without syncing again
|
|
store._set_subscription_state("sub-a", active=False)
|
|
|
|
# Query-time re-check (storage._effective_active) must hide the skill
|
|
assert store.list_skills({"sub-a"}) == []
|
|
assert store.get_skill("k1", {"sub-a"}) is None
|
|
# Union with another (still-active) subscription still excludes sub-a's skills
|
|
store._register_subscription("sub-b")
|
|
assert store.list_skills({"sub-a", "sub-b"}) == []
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Task 3.4: push-triggered (webhook) immediate sync
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_webhook_triggers_immediate_sync_of_all_subscriptions(store, fake):
|
|
from fastapi import FastAPI
|
|
from fastapi.testclient import TestClient
|
|
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner = SkillSyncRunner(
|
|
store=store,
|
|
client=fake,
|
|
subscriptions=["sub-a"],
|
|
poll_interval=999.0, # never auto-poll
|
|
)
|
|
app = FastAPI()
|
|
register_skill_sync_webhook(app, runner)
|
|
|
|
with TestClient(app) as client:
|
|
response = client.post("/webhooks/skill-sync")
|
|
assert response.status_code == 200
|
|
body = response.json()
|
|
assert body["ok"] is True
|
|
assert body["outcomes"]["sub-a"]["success"] is True
|
|
# Catalog now reflects the synced skill without any tick() call from us
|
|
assert {m.id for m in store.list_skills({"sub-a"})} == {"k1"}
|
|
|
|
|
|
def test_webhook_with_subscription_id_syncs_only_that_subscription(store, fake):
|
|
from fastapi import FastAPI
|
|
from fastapi.testclient import TestClient
|
|
|
|
fake.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
fake.set_entitled_skills("sub-b", [_knowledge("k2", "K2", "c")])
|
|
runner = SkillSyncRunner(
|
|
store=store,
|
|
client=fake,
|
|
subscriptions=["sub-a", "sub-b"],
|
|
poll_interval=999.0,
|
|
)
|
|
app = FastAPI()
|
|
register_skill_sync_webhook(app, runner)
|
|
|
|
with TestClient(app) as client:
|
|
response = client.post(
|
|
"/webhooks/skill-sync", json={"subscription_id": "sub-a"}
|
|
)
|
|
assert response.status_code == 200
|
|
body = response.json()
|
|
assert set(body["outcomes"].keys()) == {"sub-a"}
|
|
# Only sub-a was synced
|
|
assert {m.id for m in store.list_skills({"sub-a"})} == {"k1"}
|
|
# sub-b was not synced in this call
|
|
assert store.list_skills({"sub-b"}) == []
|
|
|
|
|
|
def test_register_webhook_rejects_non_fastapi_app():
|
|
runner = object() # not a SkillSyncRunner, not a FastAPI app
|
|
with pytest.raises(TypeError):
|
|
register_skill_sync_webhook("not-an-app", runner) # type: ignore[arg-type]
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# Background poll loop
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_start_background_polls_until_stopped(store, fake):
|
|
call_count = {"n": 0}
|
|
|
|
class CountingClient(FakeSubscriptionClient):
|
|
def fetch_entitled_skills(self, subscription_id, since_version=None):
|
|
call_count["n"] += 1
|
|
return super().fetch_entitled_skills(subscription_id, since_version)
|
|
|
|
client = CountingClient()
|
|
client.set_entitled_skills("sub-a", [_knowledge("k1", "K", "c")])
|
|
runner = SkillSyncRunner(
|
|
store=store,
|
|
client=client,
|
|
subscriptions=["sub-a"],
|
|
poll_interval=0.01,
|
|
)
|
|
runner.start_background()
|
|
try:
|
|
# Allow at least 2 polls
|
|
import time
|
|
|
|
time.sleep(0.05)
|
|
finally:
|
|
runner.stop_background()
|
|
assert call_count["n"] >= 2
|
|
|
|
|
|
# ----------------------------------------------------------------------
|
|
# HttpSubscriptionClient: payload parsing + httpx transport (mocked)
|
|
# ----------------------------------------------------------------------
|
|
|
|
|
|
def test_parse_sync_payload_handles_knowledge_and_flow():
|
|
payload = {
|
|
"skills": [
|
|
{"id": "k1", "name": "K", "kind": "knowledge", "content": "body"},
|
|
{
|
|
"id": "f1",
|
|
"name": "F",
|
|
"kind": "flow_template",
|
|
"steps": [{"tool_name": "tap", "args": {"x": 1}}],
|
|
"parameters": {"x": {"type": "int"}},
|
|
},
|
|
],
|
|
"removed_ids": ["old1"],
|
|
"latest_version": 7,
|
|
"is_full_replace": False,
|
|
}
|
|
delta = _parse_sync_payload(payload)
|
|
assert len(delta.skills) == 2
|
|
assert isinstance(delta.skills[0], KnowledgeSkill)
|
|
assert isinstance(delta.skills[1], FlowTemplateSkill)
|
|
assert delta.skills[1].steps[0].tool_name == "tap"
|
|
assert delta.removed_ids == ["old1"]
|
|
assert delta.latest_version == 7
|
|
assert delta.is_full_replace is False
|
|
|
|
|
|
def test_parse_sync_payload_defaults_to_full_replace():
|
|
delta = _parse_sync_payload({"skills": []})
|
|
assert delta.skills == []
|
|
assert delta.removed_ids == []
|
|
assert delta.latest_version is None
|
|
assert delta.is_full_replace is True
|
|
|
|
|
|
def test_http_client_uses_injected_httpx_client_and_parses_response(monkeypatch):
|
|
captured: dict[str, Any] = {}
|
|
|
|
class FakeResponse:
|
|
def raise_for_status(self) -> None:
|
|
return None
|
|
|
|
def json(self) -> dict[str, Any]:
|
|
return {"skills": [{"id": "k1", "name": "K", "kind": "knowledge"}]}
|
|
|
|
class FakeHttpxClient:
|
|
def get(self, url, *, params=None, headers=None, timeout=None):
|
|
captured["url"] = url
|
|
captured["params"] = params
|
|
captured["headers"] = headers
|
|
captured["timeout"] = timeout
|
|
return FakeResponse()
|
|
|
|
def close(self) -> None:
|
|
captured["closed"] = True
|
|
|
|
fake_http = FakeHttpxClient()
|
|
client = HttpSubscriptionClient(
|
|
base_url="https://subscription.example.com/",
|
|
auth_token="secret",
|
|
client=fake_http,
|
|
)
|
|
delta = client.fetch_entitled_skills("sub-a")
|
|
assert captured["url"] == "https://subscription.example.com/subscriptions/sub-a/skills"
|
|
assert captured["headers"] == {"Authorization": "Bearer secret"}
|
|
assert len(delta.skills) == 1
|
|
assert isinstance(delta.skills[0], KnowledgeSkill)
|
|
client.close()
|
|
assert captured.get("closed") is True
|
|
|
|
|
|
def test_http_client_raises_on_status_error_propagates_to_runner(store):
|
|
"""HTTP errors surface as a failed SyncOutcome, not a raised exception."""
|
|
|
|
class BoomResponse:
|
|
def raise_for_status(self):
|
|
raise RuntimeError("simulated 500")
|
|
|
|
class FailingClient:
|
|
def get(self, url, *, params=None, headers=None, timeout=None):
|
|
return BoomResponse()
|
|
|
|
def close(self):
|
|
pass
|
|
|
|
http_client = HttpSubscriptionClient(
|
|
base_url="https://subscription.example.com",
|
|
client=FailingClient(),
|
|
)
|
|
runner = SkillSyncRunner(
|
|
store=store,
|
|
client=http_client,
|
|
subscriptions=["sub-a"],
|
|
)
|
|
outcomes = runner.tick()
|
|
assert outcomes["sub-a"].success is False
|
|
assert "simulated 500" in (outcomes["sub-a"].error or "")
|