feat: enhance expert invitation handling with case-insensitive matching and session management
This commit is contained in:
@@ -199,12 +199,17 @@ class ExpertCommand(Command):
|
||||
)
|
||||
return
|
||||
|
||||
dispatchable = {s.name for s in _dispatchable_experts()}
|
||||
if target not in dispatchable:
|
||||
# Completion matches case-insensitively; honour the same here by
|
||||
# resolving a case-variant to the on-disk name before membership.
|
||||
by_lower = {s.name.lower(): s.name for s in _dispatchable_experts()}
|
||||
canonical = by_lower.get(target.lower())
|
||||
if canonical is None:
|
||||
from ...tools.skills_manager import list_expert_skills
|
||||
|
||||
installed = {s.name for s in list_expert_skills(include_system=True)}
|
||||
if target not in installed:
|
||||
installed = {
|
||||
s.name.lower() for s in list_expert_skills(include_system=True)
|
||||
}
|
||||
if target.lower() not in installed:
|
||||
ctx.ui.append_system(
|
||||
f"No expert skill named '{target}'. `/experts` lists "
|
||||
"installed ones.",
|
||||
@@ -218,12 +223,12 @@ class ExpertCommand(Command):
|
||||
)
|
||||
return
|
||||
|
||||
if target in runtime.active_teams:
|
||||
runtime.active_teams = [n for n in runtime.active_teams if n != target]
|
||||
ctx.ui.append_system(f"Dismissed expert: {target}", style="dim")
|
||||
if canonical in runtime.active_teams:
|
||||
runtime.active_teams = [n for n in runtime.active_teams if n != canonical]
|
||||
ctx.ui.append_system(f"Dismissed expert: {canonical}", style="dim")
|
||||
else:
|
||||
runtime.active_teams = [*runtime.active_teams, target]
|
||||
ctx.ui.append_system(f"Invited expert: {target}", style="green")
|
||||
runtime.active_teams = [*runtime.active_teams, canonical]
|
||||
ctx.ui.append_system(f"Invited expert: {canonical}", style="green")
|
||||
# Case (c): expert was installed after agent construction, so it
|
||||
# will not reach ``task()`` until the graph is rebuilt. Cheap
|
||||
# always-print hint mirrors the /install-skill success message.
|
||||
|
||||
@@ -182,8 +182,21 @@ class ResumeCommand(Command):
|
||||
if restored_workspace:
|
||||
ctx.workspace_dir = restored_workspace
|
||||
|
||||
switched_thread = resolved != ctx.thread_id
|
||||
ctx.thread_id = resolved
|
||||
|
||||
# Invitations are session-scoped (see ChannelRuntime.active_teams);
|
||||
# resuming a different thread is a session switch, so release them —
|
||||
# uniform with /new. Resuming the current thread keeps them.
|
||||
runtime = ctx.channel_runtime
|
||||
if switched_thread and runtime is not None and runtime.active_teams:
|
||||
dismissed = list(runtime.active_teams)
|
||||
runtime.active_teams = []
|
||||
ctx.ui.append_system(
|
||||
f"Dismissed experts on session switch: {', '.join(dismissed)}",
|
||||
style="dim",
|
||||
)
|
||||
|
||||
# Signal session change to UI
|
||||
if hasattr(ctx.ui, "handle_session_resume"):
|
||||
await ctx.ui.handle_session_resume(resolved, restored_workspace)
|
||||
@@ -214,18 +227,19 @@ class NewCommand(Command):
|
||||
category = "Session"
|
||||
|
||||
async def execute(self, ctx: CommandContext, args: list[str]) -> None:
|
||||
# ``/new`` means fresh state — release any invited experts before
|
||||
# starting the new session. Uniform with the ``ChannelRuntime.clear``
|
||||
# path on channel shutdown; avoids the "why is idea-brainstorm still
|
||||
# active in my new thread?" surprise. Users who want to reuse an
|
||||
# invite in the next thread can re-invite explicitly.
|
||||
# ``/new`` means fresh state — release any invited experts. Uniform
|
||||
# with the ``ChannelRuntime.clear`` path on channel shutdown; avoids
|
||||
# the "why is idea-brainstorm still active in my new thread?"
|
||||
# surprise. Users who want to reuse an invite in the next thread can
|
||||
# re-invite explicitly. Cleared only after the new session actually
|
||||
# exists, so a failed start leaves the current session intact.
|
||||
runtime = ctx.channel_runtime
|
||||
dismissed: list[str] = []
|
||||
if runtime is not None and runtime.active_teams:
|
||||
dismissed = list(runtime.active_teams)
|
||||
runtime.active_teams = []
|
||||
await ctx.ui.start_new_session()
|
||||
if dismissed:
|
||||
runtime.active_teams = []
|
||||
ctx.ui.append_system(
|
||||
f"Dismissed experts on new session: {', '.join(dismissed)}",
|
||||
style="dim",
|
||||
|
||||
@@ -293,12 +293,14 @@ class _V3EventProcessor:
|
||||
description=description if isinstance(description, str) else "",
|
||||
).data
|
||||
]
|
||||
raw_duration = payload.get("duration_ms")
|
||||
duration_ms = raw_duration if isinstance(raw_duration, int) else 0
|
||||
if phase == "complete":
|
||||
return [
|
||||
self.emitter.panel_dispatch_complete(
|
||||
eval_id=eval_id_str,
|
||||
dispatch_id=dispatch_id,
|
||||
duration_ms=payload.get("duration_ms", 0),
|
||||
duration_ms=duration_ms,
|
||||
).data
|
||||
]
|
||||
if phase == "error":
|
||||
@@ -307,7 +309,7 @@ class _V3EventProcessor:
|
||||
self.emitter.panel_dispatch_error(
|
||||
eval_id=eval_id_str,
|
||||
dispatch_id=dispatch_id,
|
||||
duration_ms=payload.get("duration_ms", 0),
|
||||
duration_ms=duration_ms,
|
||||
error=error if isinstance(error, str) else "",
|
||||
).data
|
||||
]
|
||||
|
||||
@@ -17,11 +17,12 @@ story.
|
||||
|
||||
State schema
|
||||
------------
|
||||
Extends ``DeepAgentState`` with ``skill_name`` and ``output_path`` as
|
||||
optional keys. Both arrive via the ``payload`` the main agent passes; the
|
||||
middleware validates presence and halts with a clear error message when
|
||||
either is missing (rather than falling back to an ambient default that
|
||||
would silently produce the wrong survey).
|
||||
Extends ``DeepAgentState`` with ``skill_name`` as an optional key,
|
||||
injected by construction when the spec is built. The middleware validates
|
||||
its presence and halts with a clear error message when missing (rather
|
||||
than falling back to an ambient default that would silently produce the
|
||||
wrong survey). Run-specific values such as the output path travel in the
|
||||
task description, not in state.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -586,7 +586,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
# collision guard. Coerce to the parent dir name in both cases.
|
||||
return _info(
|
||||
frontmatter.get("name") or parent.name,
|
||||
frontmatter.get("description", "(no description)"),
|
||||
frontmatter.get("description") or "(no description)",
|
||||
tags,
|
||||
type_=type_,
|
||||
role=str(role),
|
||||
|
||||
@@ -170,6 +170,17 @@ class TestExpertToggle:
|
||||
assert ctx.channel_runtime.active_teams == ["idea-brainstorm"]
|
||||
assert any("Invited expert: idea-brainstorm" in text for text, _ in ui.lines)
|
||||
|
||||
async def test_invite_matches_name_case_insensitively(self):
|
||||
"""Execute honours the same case-insensitive match as completion."""
|
||||
ctx, ui = _make_ctx()
|
||||
with patch(
|
||||
"EvoScientist.tools.skills_manager.list_expert_skills",
|
||||
return_value=[_FakeSkillInfo(name="idea-brainstorm")],
|
||||
):
|
||||
await ExpertCommand().execute(ctx, args=["Idea-Brainstorm"])
|
||||
assert ctx.channel_runtime.active_teams == ["idea-brainstorm"]
|
||||
assert any("Invited expert: idea-brainstorm" in text for text, _ in ui.lines)
|
||||
|
||||
async def test_toggle_dismisses_when_already_invited(self):
|
||||
ctx, ui = _make_ctx(active_teams=["idea-brainstorm"])
|
||||
with patch(
|
||||
|
||||
@@ -85,3 +85,24 @@ class TestNewCommand:
|
||||
ctx = CommandContext(agent=None, thread_id="tid", ui=ui, channel_runtime=None)
|
||||
await NewCommand().execute(ctx, [])
|
||||
ui.append_system.assert_not_called()
|
||||
|
||||
async def test_failed_session_start_keeps_invitations(self):
|
||||
"""A raising start_new_session leaves the current session's invites."""
|
||||
import pytest
|
||||
|
||||
from EvoScientist.commands.base import ChannelRuntime, CommandContext
|
||||
from EvoScientist.commands.implementation.session import NewCommand
|
||||
|
||||
ui = MagicMock()
|
||||
ui.start_new_session = AsyncMock(side_effect=RuntimeError("gateway down"))
|
||||
runtime = ChannelRuntime()
|
||||
runtime.active_teams = ["idea-brainstorm"]
|
||||
ctx = CommandContext(
|
||||
agent=None,
|
||||
thread_id="tid",
|
||||
ui=ui,
|
||||
channel_runtime=runtime,
|
||||
)
|
||||
with pytest.raises(RuntimeError):
|
||||
await NewCommand().execute(ctx, [])
|
||||
assert runtime.active_teams == ["idea-brainstorm"]
|
||||
|
||||
@@ -117,3 +117,50 @@ class TestResumeCommand:
|
||||
assert ctx.workspace_dir == "/keep"
|
||||
# Callback still fires with the metadata value (empty string)
|
||||
ui.handle_session_resume.assert_awaited_once_with("tid", "")
|
||||
|
||||
|
||||
class TestResumeClearsInvitedExperts:
|
||||
"""Invitations are session-scoped: switching threads dismisses them."""
|
||||
|
||||
def _runtime(self, invited):
|
||||
from EvoScientist.commands.base import ChannelRuntime
|
||||
|
||||
runtime = ChannelRuntime()
|
||||
runtime.active_teams = list(invited)
|
||||
return runtime
|
||||
|
||||
async def test_switching_thread_dismisses_and_announces(self):
|
||||
from EvoScientist.commands.implementation.session import ResumeCommand
|
||||
|
||||
ctx, ui = _ctx(
|
||||
thread_id="current",
|
||||
thread_store=FakeThreadStore(resolved_thread_id="other-tid"),
|
||||
)
|
||||
ctx.channel_runtime = self._runtime(["idea-brainstorm"])
|
||||
await ResumeCommand().execute(ctx, ["other-tid"])
|
||||
assert ctx.channel_runtime.active_teams == []
|
||||
msgs = [c.args[0] for c in ui.append_system.call_args_list]
|
||||
assert any(
|
||||
"Dismissed experts on session switch: idea-brainstorm" in m for m in msgs
|
||||
)
|
||||
|
||||
async def test_resuming_current_thread_keeps_invitations(self):
|
||||
from EvoScientist.commands.implementation.session import ResumeCommand
|
||||
|
||||
ctx, ui = _ctx(
|
||||
thread_id="current",
|
||||
thread_store=FakeThreadStore(resolved_thread_id="current"),
|
||||
)
|
||||
ctx.channel_runtime = self._runtime(["idea-brainstorm"])
|
||||
await ResumeCommand().execute(ctx, ["current"])
|
||||
assert ctx.channel_runtime.active_teams == ["idea-brainstorm"]
|
||||
msgs = [c.args[0] for c in ui.append_system.call_args_list]
|
||||
assert not any("Dismissed experts" in m for m in msgs)
|
||||
|
||||
async def test_failed_resolution_keeps_invitations(self):
|
||||
from EvoScientist.commands.implementation.session import ResumeCommand
|
||||
|
||||
ctx, _ui = _ctx(thread_store=FakeThreadStore())
|
||||
ctx.channel_runtime = self._runtime(["idea-brainstorm"])
|
||||
await ResumeCommand().execute(ctx, ["nope"])
|
||||
assert ctx.channel_runtime.active_teams == ["idea-brainstorm"]
|
||||
|
||||
@@ -101,6 +101,22 @@ class TestParseSkillMd:
|
||||
assert result.name == "no-frontmatter-skill"
|
||||
assert result.description == "(no description)"
|
||||
|
||||
def test_parse_with_empty_description_value(self, tmp_path):
|
||||
"""A present-but-empty ``description:`` parses to YAML None; guard it."""
|
||||
skill_dir = tmp_path / "empty-desc-skill"
|
||||
skill_dir.mkdir()
|
||||
(skill_dir / "SKILL.md").write_text(
|
||||
"""---
|
||||
name: empty-desc-skill
|
||||
description:
|
||||
---
|
||||
|
||||
# Body
|
||||
"""
|
||||
)
|
||||
result = _parse_skill_md(skill_dir / "SKILL.md")
|
||||
assert result.description == "(no description)"
|
||||
|
||||
def test_parse_with_partial_frontmatter(self, tmp_path):
|
||||
skill_dir = tmp_path / "partial-skill"
|
||||
skill_dir.mkdir()
|
||||
|
||||
@@ -1321,6 +1321,25 @@ class TestPanelDispatchEvents:
|
||||
assert completes[0]["id"] == "ptc_task_abc12345"
|
||||
assert completes[0]["duration_ms"] == 1234
|
||||
|
||||
async def test_complete_event_with_null_duration_normalizes_to_zero(self):
|
||||
agent = FakeV3Agent(
|
||||
[
|
||||
custom_subagent_event(
|
||||
{
|
||||
"type": "subagent",
|
||||
"phase": "complete",
|
||||
"id": "ptc_task_abc12345",
|
||||
"eval_id": "ci_eval_1",
|
||||
"duration_ms": None,
|
||||
}
|
||||
),
|
||||
]
|
||||
)
|
||||
events = await collect_events(agent)
|
||||
completes = [e for e in events if e.get("type") == "panel_dispatch_complete"]
|
||||
assert len(completes) == 1
|
||||
assert completes[0]["duration_ms"] == 0
|
||||
|
||||
async def test_error_event_becomes_panel_dispatch_error(self):
|
||||
agent = FakeV3Agent(
|
||||
[
|
||||
|
||||
Reference in New Issue
Block a user