mirror of
https://github.com/hansjone/oclaw.git
synced 2026-10-08 23:33:16 +08:00
Converge manager out of product roles and drop dead inherit prefs.
Specialists-only binding/prewarm/MCP allowlists fold legacy manager aliases into generalist; stop persisting confirm/plan prefs and strip inherit from prompt signatures. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
bc20b41ee2
commit
454c841af7
20 changed files with 131 additions and 290 deletions
|
|
@ -118,12 +118,12 @@ class AdminSkillsApiTests(unittest.TestCase):
|
|||
self.assertIn("available_roles", body)
|
||||
self.assertIn("mapping", body)
|
||||
roles = list(body.get("available_roles") or [])
|
||||
self.assertIn("manager", roles)
|
||||
self.assertNotIn("manager", roles)
|
||||
self.assertIn("generalist", roles)
|
||||
mapping = dict(body.get("mapping") or {})
|
||||
for r in roles:
|
||||
mapping.setdefault(r, [])
|
||||
mapping["generalist"] = ["bind_demo_skill"]
|
||||
mapping["manager"] = ["bind_demo_skill"]
|
||||
s = self.client.post(
|
||||
"/admin/api/skills/binding",
|
||||
json={"enabled": True, "mapping": mapping},
|
||||
|
|
@ -167,10 +167,8 @@ class AdminSkillsApiTests(unittest.TestCase):
|
|||
json={
|
||||
"enabled": True,
|
||||
"mapping": {
|
||||
"manager": ["effective_demo_skill"],
|
||||
"generalist": [],
|
||||
"generalist": ["effective_demo_skill"],
|
||||
"ops": [],
|
||||
"image": [],
|
||||
"memory": [],
|
||||
},
|
||||
},
|
||||
|
|
@ -182,12 +180,12 @@ class AdminSkillsApiTests(unittest.TestCase):
|
|||
body = r.json() or {}
|
||||
self.assertTrue(body.get("ok"))
|
||||
items = list(body.get("items") or [])
|
||||
self.assertTrue(any(str(x.get("role") or "") == "manager" for x in items))
|
||||
mgr = next((x for x in items if str(x.get("role") or "") == "manager"), {})
|
||||
self.assertGreaterEqual(int(mgr.get("workspace_total") or 0), 1)
|
||||
self.assertGreaterEqual(int(mgr.get("total") or 0), int(mgr.get("workspace_total") or 0))
|
||||
self.assertIn("workspace_docs_only", mgr)
|
||||
self.assertIn("workspace_resolved_tool_match", mgr)
|
||||
self.assertFalse(any(str(x.get("role") or "") == "manager" for x in items))
|
||||
gen = next((x for x in items if str(x.get("role") or "") == "generalist"), {})
|
||||
self.assertGreaterEqual(int(gen.get("workspace_total") or 0), 1)
|
||||
self.assertGreaterEqual(int(gen.get("total") or 0), int(gen.get("workspace_total") or 0))
|
||||
self.assertIn("workspace_docs_only", gen)
|
||||
self.assertIn("workspace_resolved_tool_match", gen)
|
||||
|
||||
def test_skills_retry_install_registry(self) -> None:
|
||||
pkg = Path(self._tmp.name) / "pkg2"
|
||||
|
|
|
|||
|
|
@ -13,15 +13,16 @@ def _set_project_root(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
|||
|
||||
def test_build_role_system_context_reads_runtime_workspaces(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||
_set_project_root(monkeypatch, tmp_path)
|
||||
ws = tmp_path / "runtime" / "workspaces" / "main"
|
||||
ws = tmp_path / "runtime" / "workspaces" / "generalist"
|
||||
ws.mkdir(parents=True, exist_ok=True)
|
||||
(ws / "SOUL.md").write_text("main soul", encoding="utf-8")
|
||||
(ws / "ROLE_SYSTEM.md").write_text("main role system", encoding="utf-8")
|
||||
(ws / "SOUL.md").write_text("generalist soul", encoding="utf-8")
|
||||
(ws / "ROLE_SYSTEM.md").write_text("generalist role system", encoding="utf-8")
|
||||
# Legacy manager/main aliases resolve to the generalist workspace.
|
||||
out = loader_mod.build_role_system_context("manager")
|
||||
assert "# SOUL" in out
|
||||
assert "main soul" in out
|
||||
assert "generalist soul" in out
|
||||
assert "# ROLE_SYSTEM" in out
|
||||
assert "main role system" in out
|
||||
assert "generalist role system" in out
|
||||
|
||||
|
||||
def test_build_role_system_context_cache_invalidates_on_file_change(
|
||||
|
|
|
|||
|
|
@ -114,14 +114,22 @@ class McpAdapterTests(unittest.TestCase):
|
|||
server_id="echo-b",
|
||||
tools=[{"tool_name": "ping_b", "description": "Ping B", "parameters": {"type": "object", "properties": {}}}],
|
||||
)
|
||||
# Legacy "manager" alias resolves to generalist; when generalist has its own
|
||||
# binding, that wins (manager key is not merged).
|
||||
store.set_setting("mcp_specialist_server_binding", '{"manager":["echo-a"],"generalist":["echo-b"]}')
|
||||
m_specs = materialize_mcp_tools_for_specialist(store, specialist="manager")
|
||||
g_specs = materialize_mcp_tools_for_specialist(store, specialist="generalist")
|
||||
m_names = {x.name for x in m_specs}
|
||||
g_names = {x.name for x in g_specs}
|
||||
self.assertIn("mcp__echo-a__ping_a", m_names)
|
||||
self.assertNotIn("mcp__echo-b__ping_b", m_names)
|
||||
self.assertIn("mcp__echo-b__ping_b", m_names)
|
||||
self.assertNotIn("mcp__echo-a__ping_a", m_names)
|
||||
self.assertIn("mcp__echo-b__ping_b", g_names)
|
||||
# When only manager key exists, generalist falls back to that binding.
|
||||
store.set_setting("mcp_specialist_server_binding", '{"manager":["echo-a"]}')
|
||||
g2 = materialize_mcp_tools_for_specialist(store, specialist="generalist")
|
||||
g2_names = {x.name for x in g2}
|
||||
self.assertIn("mcp__echo-a__ping_a", g2_names)
|
||||
self.assertNotIn("mcp__echo-b__ping_b", g2_names)
|
||||
|
||||
def test_binding_empty_json_object_falls_back_to_all_mcp_for_specialist(self) -> None:
|
||||
"""{} 不应把每个专家都当成「已绑定但列表为空」而屏蔽全部 MCP。"""
|
||||
|
|
|
|||
|
|
@ -13,22 +13,8 @@ class _DummyStore:
|
|||
return str(self._settings.get(key, ""))
|
||||
|
||||
|
||||
def test_get_manager_prompt_prebuild_includes_structured_skills(monkeypatch) -> None:
|
||||
def test_get_manager_prompt_prebuild_is_legacy_stub(monkeypatch) -> None:
|
||||
monkeypatch.setattr(pp, "discover_specialist_ids", lambda: ("generalist", "ops", "memory", "image"))
|
||||
monkeypatch.setattr(
|
||||
pp,
|
||||
"list_experts",
|
||||
lambda: [
|
||||
{"id": "generalist", "files": {"ROLE_SYSTEM.md": "General specialist for broad tasks."}},
|
||||
{"id": "ops", "files": {"ROLE_SYSTEM.md": "Ops specialist for runtime and services."}},
|
||||
],
|
||||
)
|
||||
monkeypatch.setattr(pp, "expert_workspace_signature_token", lambda: ("sig",))
|
||||
|
||||
def _ctx(_role: str, template_vars: dict[str, Any] | None = None) -> str:
|
||||
return f"CTX\n{str((template_vars or {}).get('MANAGER_DYNAMIC_EXPERTS_HINT') or '')}"
|
||||
|
||||
monkeypatch.setattr(pp, "build_role_system_context", _ctx)
|
||||
|
||||
out = pp.get_manager_prompt_prebuild(
|
||||
store=_DummyStore(),
|
||||
|
|
@ -37,20 +23,11 @@ def test_get_manager_prompt_prebuild_includes_structured_skills(monkeypatch) ->
|
|||
memory_enabled=True,
|
||||
)
|
||||
assert "generalist" in str(out.get("allowed_fixed") or "")
|
||||
assert "- generalist:" in str(out.get("manager_context") or "")
|
||||
assert "General specialist for broad tasks." in str(out.get("manager_context") or "")
|
||||
assert "ops" in str(out.get("allowed_fixed") or "")
|
||||
assert str(out.get("manager_context") or "") == ""
|
||||
|
||||
|
||||
def test_warm_startup_prompt_prebuild_warms_all_roles(monkeypatch) -> None:
|
||||
monkeypatch.setattr(
|
||||
pp,
|
||||
"get_manager_prompt_prebuild",
|
||||
lambda **_k: {
|
||||
"manager_context": "manager_ctx",
|
||||
"allowed_fixed": ("generalist", "ops"),
|
||||
"allowed_fixed_quoted": '"generalist", "ops"',
|
||||
},
|
||||
)
|
||||
def test_warm_startup_prompt_prebuild_warms_specialists_only(monkeypatch) -> None:
|
||||
monkeypatch.setattr(pp, "discover_specialist_ids", lambda: ("generalist", "ops"))
|
||||
monkeypatch.setattr(pp, "build_role_system_context", lambda role, template_vars=None: f"{role}_ctx")
|
||||
|
||||
|
|
@ -71,20 +48,13 @@ def test_warm_startup_prompt_prebuild_warms_all_roles(monkeypatch) -> None:
|
|||
assert out["ok"] is True
|
||||
role_map = captured.get("role_base_systems") if isinstance(captured, dict) else {}
|
||||
assert isinstance(role_map, dict)
|
||||
assert "manager" in role_map
|
||||
assert "manager" not in role_map
|
||||
assert "generalist" in role_map
|
||||
assert "ops" in role_map
|
||||
|
||||
|
||||
def test_runtime_prewarm_prompts_snapshot_returns_roles(monkeypatch) -> None:
|
||||
monkeypatch.setattr(pp, "discover_specialist_ids", lambda: ("generalist", "ops"))
|
||||
monkeypatch.setattr(
|
||||
pp,
|
||||
"get_manager_prompt_prebuild",
|
||||
lambda **_k: {
|
||||
"manager_context": "manager_ctx",
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(pp, "build_role_system_context", lambda role, template_vars=None: f"{role}_ctx")
|
||||
monkeypatch.setattr(pp, "get_executor_prompt_static", lambda **kwargs: f"exec::{kwargs.get('skill_binding_role')}")
|
||||
monkeypatch.setattr(pp, "default_registry", lambda **kwargs: object())
|
||||
|
|
@ -92,54 +62,32 @@ def test_runtime_prewarm_prompts_snapshot_returns_roles(monkeypatch) -> None:
|
|||
out = pp.runtime_prewarm_prompts_snapshot(store=_DummyStore())
|
||||
assert out["ok"] is True
|
||||
prompts = out.get("prompts") or {}
|
||||
assert "manager" in prompts
|
||||
assert "manager" not in prompts
|
||||
assert "generalist" in prompts
|
||||
assert "ops" in prompts
|
||||
assert prompts["manager"].get("system_prompt") == "exec::manager"
|
||||
assert prompts["generalist"].get("system_prompt") == "exec::generalist"
|
||||
assert prompts["ops"].get("system_prompt") == "exec::ops"
|
||||
assert "manager_system_prompt" not in prompts["manager"]
|
||||
assert "executor_system_prompt" not in prompts["manager"]
|
||||
assert "manager_user_scaffold" not in prompts["manager"]
|
||||
|
||||
aliased = pp.runtime_prewarm_prompts_snapshot(store=_DummyStore(), role="manager")
|
||||
assert aliased["ok"] is True
|
||||
assert aliased.get("roles") == ["generalist"]
|
||||
assert "generalist" in (aliased.get("prompts") or {})
|
||||
|
||||
|
||||
def test_manager_prompt_prebuild_cache_invalidates_on_workspace_revision_change(monkeypatch) -> None:
|
||||
token = {"v": 1}
|
||||
calls = {"ctx": 0}
|
||||
|
||||
monkeypatch.setattr(pp, "discover_specialist_ids", lambda: ("generalist", "ops"))
|
||||
monkeypatch.setattr(
|
||||
pp,
|
||||
"list_experts",
|
||||
lambda: [{"id": "generalist", "files": {"ROLE_SYSTEM.md": "General specialist"}}],
|
||||
)
|
||||
monkeypatch.setattr(pp, "expert_workspace_signature_token", lambda: ("revision", token["v"]))
|
||||
|
||||
def _ctx(_role: str, template_vars: dict[str, Any] | None = None) -> str:
|
||||
calls["ctx"] += 1
|
||||
return f"CTX\n{str((template_vars or {}).get('MANAGER_DYNAMIC_EXPERTS_HINT') or '')}"
|
||||
|
||||
monkeypatch.setattr(pp, "build_role_system_context", _ctx)
|
||||
|
||||
_ = pp.get_manager_prompt_prebuild(
|
||||
def test_manager_prompt_prebuild_stub_is_stateless(monkeypatch) -> None:
|
||||
monkeypatch.setattr(pp, "discover_specialist_ids", lambda: ("generalist", "ops", "memory"))
|
||||
a = pp.get_manager_prompt_prebuild(
|
||||
store=_DummyStore(),
|
||||
registry=object(),
|
||||
base_url="",
|
||||
memory_enabled=True,
|
||||
)
|
||||
_ = pp.get_manager_prompt_prebuild(
|
||||
b = pp.get_manager_prompt_prebuild(
|
||||
store=_DummyStore(),
|
||||
registry=object(),
|
||||
base_url="",
|
||||
memory_enabled=True,
|
||||
memory_enabled=False,
|
||||
)
|
||||
assert calls["ctx"] == 1
|
||||
|
||||
token["v"] = 2
|
||||
_ = pp.get_manager_prompt_prebuild(
|
||||
store=_DummyStore(),
|
||||
registry=object(),
|
||||
base_url="",
|
||||
memory_enabled=True,
|
||||
)
|
||||
assert calls["ctx"] == 2
|
||||
assert a.get("manager_context") == ""
|
||||
assert "memory" not in (b.get("allowed_fixed") or ())
|
||||
assert "memory" in (a.get("allowed_fixed") or ())
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ def test_collect_respects_role_binding_union(tmp_path: Path, monkeypatch) -> Non
|
|||
store.set_setting(SKILL_ROLE_BINDING_ENABLED_SETTING, "1")
|
||||
mapping = {r: [] for r in ordered_binding_roles()}
|
||||
mapping["generalist"] = ["skill-alpha"]
|
||||
# Legacy manager bindings fold into generalist.
|
||||
mapping["manager"] = ["skill-beta"]
|
||||
store.set_setting(SKILL_ROLE_BINDING_KEY, json.dumps(mapping))
|
||||
|
||||
|
|
@ -141,14 +142,15 @@ def test_collect_unfiltered_when_binding_disabled(tmp_path: Path, monkeypatch) -
|
|||
|
||||
|
||||
def test_normalize_drops_unknown_skills(tmp_path: Path) -> None:
|
||||
roles = ["manager", "generalist"]
|
||||
roles = ["generalist", "ops"]
|
||||
out = normalize_skill_role_binding(
|
||||
mapping_raw={"manager": ["nope", "skill-x"], "generalist": ["skill-x"]},
|
||||
mapping_raw={"manager": ["nope", "skill-x"], "generalist": ["skill-x"], "ops": ["nope"]},
|
||||
valid_skill_names={"skill-x"},
|
||||
available_roles=roles,
|
||||
)
|
||||
assert out["manager"] == ["skill-x"]
|
||||
assert "manager" not in out
|
||||
assert out["generalist"] == ["skill-x"]
|
||||
assert out["ops"] == []
|
||||
|
||||
|
||||
def test_skill_role_binding_env_overrides_store_value(tmp_path: Path, monkeypatch) -> None:
|
||||
|
|
|
|||
|
|
@ -11,7 +11,8 @@ def test_unknown_dynamic_specialist_defaults_to_minimum_expert_permissions(monke
|
|||
def test_agent_role_ids_uses_runtime_discovery(monkeypatch) -> None:
|
||||
monkeypatch.setattr(specialists_mod, "discover_specialist_ids", lambda: ("generalist", "ops", "qa"))
|
||||
got = specialists_mod.agent_role_ids()
|
||||
assert got[0] == specialists_mod.MANAGER_AGENT_ID
|
||||
assert got[0] == "generalist"
|
||||
assert "manager" not in set(got)
|
||||
assert "qa" in set(got)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ def test_tool_wire_freeze_default_on(monkeypatch) -> None:
|
|||
monkeypatch.setattr(dl, "_prepare_llm_tools", lambda **kwargs: [])
|
||||
monkeypatch.setenv("AIA_TOOL_WIRE_FROZEN_ON_STARTUP", "")
|
||||
store = _DummyStore()
|
||||
_ = dl.warm_tool_wire_cache(store=store, tools=object(), base_url="", roles=["manager"])
|
||||
_ = dl.warm_tool_wire_cache(store=store, tools=object(), base_url="", roles=["generalist"])
|
||||
st = dl.tool_wire_freeze_status(store=store)
|
||||
assert st["enabled"] is True
|
||||
assert st["frozen"] is True
|
||||
|
|
@ -24,7 +24,7 @@ def test_tool_wire_freeze_default_on(monkeypatch) -> None:
|
|||
def test_tool_wire_freeze_disabled_by_setting(monkeypatch) -> None:
|
||||
monkeypatch.setattr(dl, "_prepare_llm_tools", lambda **kwargs: [])
|
||||
store = _DummyStore({"AIA_TOOL_WIRE_FROZEN_ON_STARTUP": "0"})
|
||||
_ = dl.warm_tool_wire_cache(store=store, tools=object(), base_url="", roles=["manager"])
|
||||
_ = dl.warm_tool_wire_cache(store=store, tools=object(), base_url="", roles=["generalist"])
|
||||
st = dl.tool_wire_freeze_status(store=store)
|
||||
assert st["enabled"] is False
|
||||
assert st["frozen"] is False
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue