From 1f04889449bbde60874de70ac30ac3849a6f2c38 Mon Sep 17 00:00:00 2001 From: worldmozara Date: Fri, 9 Oct 2026 21:40:27 +0800 Subject: [PATCH] cli/skills: open the skill directory instead of printing SKILL.md --- AGENTS.md | 2 +- README.md | 2 +- src/plyngent/cli/editor.py | 38 ++++++++++++++-------- src/plyngent/cli/slash.py | 45 +++++++++++++++++++------- tests/test_cli/test_editor.py | 44 +++++++++++++++++++++++-- tests/test_cli/test_skills_cmd.py | 54 +++++++++++++++++++++++++++---- 6 files changed, 150 insertions(+), 35 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f6b0c09..63b3f46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,7 +117,7 @@ Shared interactive I/O: `ask` / `choose` / `form` / `confirm` with pluggable bac Click app + readline REPL. Entry: `plyngent` / `python -m plyngent`. - **`plyngent chat`**: provider/model (flags or interactive; Tab via readline in `prompting`); sessions store `provider_name`/`model` and restore on resume; SQLite via `[database]` (file DB under user data when url unset; explicit `url = ":memory:"` kept + warn); workspace-bound; resume latest for cwd/`--workspace` by default (`--new` / `--session`). One-shot: `-p/--prompt` and non-TTY stdin; exit codes 0/1/2/3; `--yes` (YOLO on), `--auto-continue` (auto-raise limits), `--stream/--no-stream`, `--quiet`. Root `--log-level`. Enabled `[mcp]` servers start before the REPL (`_start_mcp_manager`, per-server failures warn on stderr) and are closed on exit (`McpManager.aclose`); each server spawns in its own session/process group (`start_new_session` / `CREATE_NEW_PROCESS_GROUP`), so a terminal Ctrl+C does not kill it. -- Slash: Click group in `cli/slash.py` + `awaitlet` for async work; Tab completer from registry + ParamType `shell_complete`. `/history` is turn-oriented (`cli/transcript.py`): a turn runs from a user row to the next one, rows carry **display ordinals** (never DB row numbers; restart with the session), the turn's ends (user + final answer) always print in full, `-v` expands every row (one-liners), `-vv` prints full bodies + local rows, `--preview` collapses every row, and `N`/`last [N]` address turns while `--message N` addresses one row. Multiline `"""` … `"""`; `/edit` via `$VISUAL`/`$EDITOR` (blocking only). `/yolo on|off|once` for soft destructive confirms. `/model --persist` / `/models --persist` write model catalog entries into TOML. `/todos` for human show/push/pop/clear of the todo stack. `/skills` (`list` default) prints the discovered skills with their roots, shadowed copies, and frontmatter notes; `/skills search PATTERN` and `/skills read NAME [--file F]` run the matching tool handlers (the slash path binds the tool context itself); `/skills reload` rescans the roots and rebuilds tools. `/mcp` (`list` default) prints per-server status/tool counts + an `instructions` preview when the server sent usage guidance, + last stderr lines; `/mcp reconnect` re-reads config, restarts every enabled server, and rebuilds the tool registry. `/grants` lists directory-access grants (`revoke `; config grants point at the TOML); `/status` includes the grant count and the visible skill count (or `off` when `[skills].enabled` is false). +- Slash: Click group in `cli/slash.py` + `awaitlet` for async work; Tab completer from registry + ParamType `shell_complete`. `/history` is turn-oriented (`cli/transcript.py`): a turn runs from a user row to the next one, rows carry **display ordinals** (never DB row numbers; restart with the session), the turn's ends (user + final answer) always print in full, `-v` expands every row (one-liners), `-vv` prints full bodies + local rows, `--preview` collapses every row, and `N`/`last [N]` address turns while `--message N` addresses one row. Multiline `"""` … `"""`; `/edit` via `$VISUAL`/`$EDITOR` (blocking only). `/yolo on|off|once` for soft destructive confirms. `/model --persist` / `/models --persist` write model catalog entries into TOML. `/todos` for human show/push/pop/clear of the todo stack. `/skills` (`list` default) prints the discovered skills with their roots, shadowed copies, and frontmatter notes; `/skills search PATTERN` runs the matching tool handler (the slash path binds the tool context itself); `/skills open NAME` reveals the skill's directory in the system file manager; `/skills reload` rescans the roots and rebuilds tools. `/mcp` (`list` default) prints per-server status/tool counts + an `instructions` preview when the server sent usage guidance, + last stderr lines; `/mcp reconnect` re-reads config, restarts every enabled server, and rebuilds the tool registry. `/grants` lists directory-access grants (`revoke `; config grants point at the TOML); `/status` includes the grant count and the visible skill count (or `off` when `[skills].enabled` is false). - `/btw` side questions: forked session; main transcript/DB untouched. `--tools=read` (default) exposes only `READ_ONLY` tools under `read_only_context()` (fetch=GET) with the main session view; `--tools=no` off; `--tools=full` whole-registry clone with fresh session. Tools off: default falls back to no tools; explicit `read`/`full` errors. The model gets an `aside` notice as its last message before the question (unsaved exchange + tool scope), shown to the user as `[notice] aside: …`. - Explicit `/resume` or `--session` from another workspace prompts: **keep** / **update** / **abort**. A session resumed by a **freshly started process** (`--session` / default latest) also gets a `resume` notice about what a restart drops (PTY sessions, running commands, temp workspaces) — resuming inside a live process pushes nothing. - Host notices (`agent/notices.py`): system-level messages appended as trailing `developer` rows, never system-prompt edits — `Notice.to_message()` for the model, `.summary` for the user via `cli.display.echo_notice` (`--quiet` hides the user line only). Durable hosts persist them (`ChatAgent.push_notice`); a side agent without memory keeps its notice local. Trailing developer messages never end a turn, so `/retry` still sees an interrupted turn behind a notice or checkpoint. diff --git a/README.md b/README.md index 5b7d4d4..4d48418 100644 --- a/README.md +++ b/README.md @@ -303,7 +303,7 @@ Type `/help` in the REPL for the live list. Common ones: | `/models` | List config + remote `GET /models` (always re-fetches) | | `/models --persist` | Merge remote catalog into TOML for this provider | | `/todos` | Todo/task stack: list, push, pop, done, clear | -| `/skills [list\|search\|read\|reload]` | List discovered skills, regex-search them, print a `SKILL.md` (or one bundled file), or rescan the roots | +| `/skills [list\|search\|open\|reload]` | List discovered skills, regex-search them, open one's directory in the system file manager, or rescan the roots | | `/grants` | Directory-access grants: list, or `revoke ` | | `/config` | Edit `plyngent.toml` ($VISUAL/$EDITOR or system open); reload after blocking editor | | `/quit` | Leave the REPL | diff --git a/src/plyngent/cli/editor.py b/src/plyngent/cli/editor.py index 4c036bc..5f1b726 100644 --- a/src/plyngent/cli/editor.py +++ b/src/plyngent/cli/editor.py @@ -112,11 +112,17 @@ def _run_blocking_editor(editor_cmd: str, path: Path) -> None: raise click.ClickException(msg) -def _open_with_system_default(path: Path) -> None: +def _open_failure(reason: str, hint: str) -> str: + """Compose an open failure message, appending the caller's fallback hint.""" + return f"{reason}; {hint}" if hint else reason + + +def _open_with_system_default(path: Path, *, hint: str = "") -> None: """Open *path* with the OS file association (non-blocking). Linux: ``xdg-open``; macOS: ``open``; Windows: ``os.startfile``. - Does not wait for the application to exit. + Does not wait for the application to exit. *hint* names the caller's own + fallback (an editor, a printed path) and is appended to failures. """ resolved = str(path.resolve()) if sys.platform == "win32": @@ -141,19 +147,22 @@ def _open_with_system_default(path: Path) -> None: stderr=subprocess.DEVNULL, ) except FileNotFoundError as exc: - cmd = argv[0] - msg = ( - f"{cmd} not found and neither VISUAL nor EDITOR is set; " - "set VISUAL or EDITOR to a blocking editor (e.g. nano, vim, codium --wait)" - ) - raise click.ClickException(msg) from exc + raise click.ClickException(_open_failure(f"{argv[0]} not found", hint)) from exc except OSError as exc: - msg = f"failed to open with system default: {exc}" - raise click.ClickException(msg) from exc + raise click.ClickException(_open_failure(f"failed to open with system default: {exc}", hint)) from exc if completed.returncode != 0: - msg = f"system open failed (exit {completed.returncode}); set VISUAL or EDITOR to a blocking editor" - raise click.ClickException(msg) + raise click.ClickException(_open_failure(f"system open failed (exit {completed.returncode})", hint)) + + +def open_directory(path: Path) -> None: + """Reveal *path* (a directory) in the OS file manager (non-blocking). + + A folder is not an editor target, so ``$VISUAL`` / ``$EDITOR`` are never + consulted: the platform opener decides. The caller prints *path* too, which + is the way in when the host has no opener at all. + """ + _open_with_system_default(path, hint="open the printed path manually") def open_in_editor( @@ -189,7 +198,10 @@ def open_in_editor( msg = "neither VISUAL nor EDITOR is set" raise click.ClickException(msg) - _open_with_system_default(path) + _open_with_system_default( + path, + hint="set VISUAL or EDITOR to a blocking editor (e.g. nano, vim, codium --wait)", + ) return "system" diff --git a/src/plyngent/cli/slash.py b/src/plyngent/cli/slash.py index a928490..b91c40d 100644 --- a/src/plyngent/cli/slash.py +++ b/src/plyngent/cli/slash.py @@ -18,6 +18,7 @@ if TYPE_CHECKING: from plyngent.agent import ToolRegistry from plyngent.cli.state import ReplState, YoloMode + from plyngent.skills import SkillStore _COMPACT_PREVIEW = 400 _ON_OFF_CHOICES = ("on", "off") @@ -1244,21 +1245,41 @@ def mcp_slash_cmd(state: ReplState, action: str | None) -> None: _print_mcp_status(state) +def _open_skill_directory(store: SkillStore, name: str) -> None: + """Reveal a skill's directory in the host file manager (``/skills open``). + + Reading a skill is a file-manager job, not a render of the model-facing + ``skill_read`` output; the printed path is the fallback on a headless host. + """ + from plyngent.cli.editor import open_directory + + skill = store.find(name) + if skill is None: + click.echo(f"error: unknown skill {name!r}; /skills lists the discovered ones") + return + try: + open_directory(skill.path) + except click.ClickException as exc: + click.echo(f"error: {exc}") + click.echo(f"skill directory: {skill.path}") + return + click.echo(f"opened {skill.path}") + + @slash.command("skills") -@click.argument("action", required=False, type=click.Choice(["list", "search", "read", "reload"])) +@click.argument("action", required=False, type=click.Choice(["list", "search", "open", "reload"])) @click.argument("target", required=False) -@click.option("--file", "file", default="", help="File inside the skill (with ``read``).") @click.pass_obj -def skills_slash_cmd(state: ReplState, action: str | None, target: str | None, file: str) -> None: - """List skills, search or read them, or rescan the roots. +def skills_slash_cmd(state: ReplState, action: str | None, target: str | None) -> None: + """List skills, search them, open one's directory, or rescan the roots. ``/skills`` or ``/skills list`` — discovered skills with their source root. ``/skills search PATTERN`` — regex over every skill's contents. - ``/skills read NAME [--file F]`` — print a skill's SKILL.md (or one file). + ``/skills open NAME`` — reveal the skill's directory in the file manager. ``/skills reload`` — rescan the roots after an external edit. """ from plyngent.tools.context import bind_tool_context - from plyngent.tools.skills import SKILL_TOOL_NAMES, format_skill_table, skill_read, skill_search + from plyngent.tools.skills import SKILL_TOOL_NAMES, format_skill_table, skill_search if state.skills is None: click.echo("skills: disabled (set [skills].enabled = true)") @@ -1269,6 +1290,12 @@ def skills_slash_cmd(state: ReplState, action: str | None, target: str | None, f _slash_rebuild_tools_if_on(state) click.echo(f"skills reloaded: {len(state.skills.skills)} visible; tools rebuilt") return + if act == "open": + if not target: + msg = "/skills open requires a skill name" + raise click.UsageError(msg) + _open_skill_directory(state.skills, target) + return # The tool handlers read the bound instance (the registry binds it for a # model call; a slash command has to do it itself). with bind_tool_context(instance=state.instance_state, session=state.session_state): @@ -1278,12 +1305,6 @@ def skills_slash_cmd(state: ReplState, action: str | None, target: str | None, f raise click.UsageError(msg) click.echo(_await(skill_search.handler(target))) return - if act == "read": - if not target: - msg = "/skills read requires a skill name" - raise click.UsageError(msg) - click.echo(_await(skill_read.handler(target, file or "SKILL.md"))) - return click.echo(format_skill_table(state.skills)) if not state.tools_enabled: click.secho( diff --git a/tests/test_cli/test_editor.py b/tests/test_cli/test_editor.py index ba81723..30bd004 100644 --- a/tests/test_cli/test_editor.py +++ b/tests/test_cli/test_editor.py @@ -2,6 +2,7 @@ from __future__ import annotations from pathlib import Path +import click import pytest from click.testing import CliRunner @@ -9,6 +10,7 @@ from plyngent.cli.app import main from plyngent.cli.editor import ( ensure_config_file, get_editor, + open_directory, open_in_editor, prompt_edit_config, resolve_config_path, @@ -70,7 +72,8 @@ def test_open_in_editor_system_fallback( path = tmp_path / "x.toml" opened: list[str] = [] - def fake_system(p: Path) -> None: + def fake_system(p: Path, *, hint: str = "") -> None: + assert hint opened.append(str(p)) monkeypatch.setattr("plyngent.cli.editor._open_with_system_default", fake_system) @@ -80,6 +83,43 @@ def test_open_in_editor_system_fallback( assert path.is_file() +def test_open_directory_uses_the_system_opener_even_with_an_editor( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + opened: list[Path] = [] + + def fake_system(p: Path, *, hint: str = "") -> None: + assert hint + opened.append(p) + + # A folder is not an editor target: $EDITOR must not be consulted. + monkeypatch.setenv("EDITOR", "codium --wait") + monkeypatch.setattr("plyngent.cli.editor._open_with_system_default", fake_system) + directory = tmp_path / "acme-deploy" + directory.mkdir() + + open_directory(directory) + + assert opened == [directory] + + +def test_open_directory_names_the_printed_path_when_the_opener_is_missing( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + def missing(*args: object, **kwargs: object) -> object: + del args, kwargs + msg = "xdg-open" + raise FileNotFoundError(msg) + + monkeypatch.setattr("sys.platform", "linux") + monkeypatch.setattr("plyngent.cli.editor.subprocess.run", missing) + + with pytest.raises(click.ClickException, match=r"xdg-open not found; open the printed path"): + open_directory(tmp_path) + + def test_open_in_editor_no_fallback_requires_editor( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -227,7 +267,7 @@ def test_config_edit_system_fallback_message( monkeypatch.delenv("VISUAL", raising=False) monkeypatch.setattr( "plyngent.cli.editor._open_with_system_default", - lambda p: None, + lambda p, **kwargs: None, ) runner = CliRunner() result = runner.invoke(main, ["config", "edit", "--config", str(path)]) diff --git a/tests/test_cli/test_skills_cmd.py b/tests/test_cli/test_skills_cmd.py index d9f2223..68ac767 100644 --- a/tests/test_cli/test_skills_cmd.py +++ b/tests/test_cli/test_skills_cmd.py @@ -99,7 +99,7 @@ async def test_skills_are_discovered_from_a_configured_root(tmp_path: Path) -> N await state.memory.close() -async def test_slash_skills_lists_search_and_reads( +async def test_slash_skills_lists_and_searches( skills_state: ReplState, capsys: pytest.CaptureFixture[str], ) -> None: @@ -112,11 +112,53 @@ async def test_slash_skills_lists_search_and_reads( assert await handle_slash(skills_state, "/skills search pdftotext") is True assert "acme-deploy/SKILL.md:" in capsys.readouterr().out - assert await handle_slash(skills_state, "/skills read acme-deploy") is True - assert "description: Deploy the acme stack" in capsys.readouterr().out - assert await handle_slash(skills_state, "/skills read acme-deploy --file SKILL.md") is True - assert "acme-deploy" in capsys.readouterr().out +async def test_slash_skills_open_reveals_the_skill_directory( + skills_state: ReplState, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + opened: list[Path] = [] + monkeypatch.setattr("plyngent.cli.editor._open_with_system_default", lambda path, **kw: opened.append(path)) + + assert await handle_slash(skills_state, "/skills open acme-deploy") is True + + assert skills_state.skills is not None + skill = skills_state.skills.find("acme-deploy") + assert skill is not None + assert opened == [skill.path] + assert f"opened {skill.path}" in capsys.readouterr().out + + +async def test_slash_skills_open_falls_back_to_the_printed_path( + skills_state: ReplState, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + import click + + def no_opener(path: Path, **kwargs: object) -> None: + del path, kwargs + msg = "xdg-open not found" + raise click.ClickException(msg) + + monkeypatch.setattr("plyngent.cli.editor._open_with_system_default", no_opener) + assert await handle_slash(skills_state, "/skills open acme-deploy") is True + + assert skills_state.skills is not None + skill = skills_state.skills.find("acme-deploy") + assert skill is not None + out = capsys.readouterr().out + assert "error: xdg-open not found" in out + assert f"skill directory: {skill.path}" in out + + +async def test_slash_skills_open_reports_an_unknown_skill( + skills_state: ReplState, + capsys: pytest.CaptureFixture[str], +) -> None: + assert await handle_slash(skills_state, "/skills open nope") is True + assert "unknown skill 'nope'" in capsys.readouterr().out async def test_slash_skills_reload_picks_up_a_new_skill( @@ -150,7 +192,7 @@ async def test_slash_skills_reports_a_disabled_feature(tmp_path: Path, capsys: p async def test_slash_skills_requires_a_target(skills_state: ReplState, capsys: pytest.CaptureFixture[str]) -> None: - for command in ("/skills search", "/skills read"): + for command in ("/skills search", "/skills open"): assert await handle_slash(skills_state, command) is True assert "requires" in capsys.readouterr().err