core/tools: drop process workspace/todo globals; require bound state

This commit is contained in:
2026-07-24 16:32:33 +08:00
parent 4215a8cea0
commit 1d718582ac
19 changed files with 126 additions and 235 deletions
+13 -19
View File
@@ -26,7 +26,6 @@ from plyngent.config.models import DatabaseConfig
from plyngent.memory import MemoryStore
from plyngent.prompting import NonInteractiveBackend, configure_prompting
from plyngent.runtime import ProviderNotSupportedError, create_client
from plyngent.tools import set_workspace_root
if TYPE_CHECKING:
from collections.abc import Mapping
@@ -115,16 +114,8 @@ def _read_prompt_text(prompt: str | None, *, stdin_isatty: bool) -> str | None:
return text or None
def _setup_workspace_and_hooks(
store: ConfigStore,
workspace: Path,
*,
interactive: bool,
) -> None:
_ = set_workspace_root(workspace)
from plyngent.tools import set_path_denylist
set_path_denylist(store.agent_config.path_denylist or None)
def _setup_hooks(*, interactive: bool) -> None:
"""Install interactive limit hooks (workspace policy is set on ReplState)."""
if interactive:
install_cli_limit_hooks()
else:
@@ -217,7 +208,7 @@ async def _run_chat( # noqa: C901, PLR0912, PLR0915 — chat orchestration
if store.recoverable_providers:
_warn_recoverable_providers(store.recoverable_providers)
_setup_workspace_and_hooks(store, workspace, interactive=interactive)
_setup_hooks(interactive=interactive)
# --yes forces sticky YOLO; else derive from config.confirm_destructive.
from plyngent.cli.state import YoloMode
@@ -297,6 +288,12 @@ async def _run_chat( # noqa: C901, PLR0912, PLR0915 — chat orchestration
interactive_limits=interactive,
yolo=yolo,
)
# Path denylist and policy confirm live on instance.workspace (no process bag).
state.instance_state.workspace.path_denylist = tuple(store.agent_config.path_denylist or ())
if interactive:
from plyngent.cli.limits import prompt_policy_command_confirm
state.instance_state.workspace.policy_confirm_hook = prompt_policy_command_confirm
# Seed cache if we already fetched; else warm in background for Tab.
if remote_ids is not None:
state.seed_remote_models(remote_ids)
@@ -323,20 +320,17 @@ async def _run_chat( # noqa: C901, PLR0912, PLR0915 — chat orchestration
return EXIT_OK
finally:
await memory.close()
from plyngent.tools.workspace import clear_policy_allowed_commands, set_policy_confirm_hook
set_policy_confirm_hook(None)
clear_policy_allowed_commands()
# PTY + temp workspace cleanup via instance shutdown when state exists.
state_obj = locals().get("state")
if isinstance(state_obj, ReplState):
state_obj.instance_state.workspace.policy_confirm_hook = None
state_obj.instance_state.workspace.policy_allowed_commands.clear()
await state_obj.instance_state.shutdown()
else:
# No ReplState: only PTY class cleanup (temps require instance allowlist).
from plyngent.tools.process.pty_session import PtyManager
from plyngent.tools.temp_workspace import cleanup_temporary_workspaces
PtyManager.close_all()
_ = cleanup_temporary_workspaces()
def _configure_logging(level: str) -> None:
@@ -384,7 +378,7 @@ def main(ctx: click.Context, log_level: str) -> None:
)
@click.option("--provider", "provider_name", default=None, help="Provider name from config.")
@click.option("--model", default=None, help="Model id.")
@click.option("--tools/--no-tools", default=True, show_default=True, help="Enable DEFAULT_TOOLS.")
@click.option("--tools/--no-tools", default=True, show_default=True, help="Enable catalog tools (local surface).")
@click.option(
"--workspace",
type=click.Path(path_type=Path, file_okay=False, exists=True),
+5 -4
View File
@@ -350,9 +350,10 @@ async def prompt_workspace_mismatch_async(
def install_cli_limit_hooks() -> None:
"""Register interactive continue hooks, policy confirm, and prompt cancel-pause."""
"""Register interactive continue hooks and prompt cancel-pause.
Command denylist policy confirm is set on :class:`~plyngent.tools.context.InstanceState`
when the REPL starts (no process-global hook).
"""
configure_prompting(pause_factory=pause_task_cancel_for_prompt)
PtyManager.set_limit_continue_hook(prompt_continue_limit)
from plyngent.tools.workspace import set_policy_confirm_hook
set_policy_confirm_hook(prompt_policy_command_confirm)
+4 -13
View File
@@ -18,11 +18,7 @@ from plyngent.cli.models_source import (
)
from plyngent.memory.database.store import normalize_workspace
from plyngent.runtime import create_client
from plyngent.tools import (
InstanceState,
SessionState,
set_workspace_root,
)
from plyngent.tools import InstanceState, SessionState
from plyngent.tools.view import MemoryViewStore, session_data_view
if TYPE_CHECKING:
@@ -80,8 +76,6 @@ class ReplState:
self.workspace = Path(self.workspace).expanduser().resolve()
self.instance_state.workspace_root = self.workspace
self.instance_state.workspace.root = self.workspace
# Process-global workspace remains for tools/tests that run without instance bind.
_ = set_workspace_root(self.workspace)
self.session_state = self._session_data_for_todo()
self.agent = self._make_agent()
self.sync_display_flags()
@@ -165,11 +159,10 @@ class ReplState:
)
def _bind_todo_tools(self) -> None:
"""Bind session/instance state for tools (prefer context over process globals).
"""Bind session/instance state for tools.
Seeds ``session.data["todo"]`` from the live stack and attaches
:meth:`_todo_on_change` so memory persist no longer depends on
process-global ``set_todo_stack(on_change=...)``.
:meth:`_todo_on_change` for memory persist.
"""
self.session_state.session_id = self.session_id
self.session_state.todo = self.todo_stack
@@ -397,10 +390,9 @@ class ReplState:
from plyngent.cli.selection import select_model, select_provider
from plyngent.runtime import ProviderNotSupportedError
from plyngent.tools import set_path_denylist
self.config.reload()
set_path_denylist(self.config.agent_config.path_denylist or None)
self.instance_state.workspace.path_denylist = tuple(self.config.agent_config.path_denylist or ())
selectable = self.config.selectable_providers()
preferred_provider = self.provider_name if self.provider_name in selectable else None
@@ -438,7 +430,6 @@ class ReplState:
self.workspace = resolved
self.instance_state.workspace_root = resolved
self.instance_state.workspace.root = resolved
_ = set_workspace_root(resolved)
if hasattr(self, "agent") and self.agent.tools is not None:
self.agent.tools.set_instance_state(self.instance_state)
-5
View File
@@ -41,8 +41,6 @@ from .process import write_pty_keys as write_pty_keys
from .temp_workspace import cleanup_temporary_workspaces as cleanup_temporary_workspaces
from .temp_workspace import new_temporary_workspace as new_temporary_workspace
from .todo import TODO_TOOLS as TODO_TOOLS
from .todo import get_todo_stack as get_todo_stack
from .todo import set_todo_stack as set_todo_stack
from .todo import todo_clear as todo_clear
from .todo import todo_list as todo_list
from .todo import todo_pop as todo_pop
@@ -82,6 +80,3 @@ from .workspace import set_path_denylist as set_path_denylist
from .workspace import set_policy_confirm_hook as set_policy_confirm_hook
from .workspace import set_policy_confirm_timeout as set_policy_confirm_timeout
from .workspace import set_workspace_root as set_workspace_root
# Deprecated alias: prefer default_tool_definitions() (catalog select).
DEFAULT_TOOLS = [*FILE_TOOLS, *PROCESS_TOOLS, *VCS_TOOLS, *CHAT_TOOLS, *TODO_TOOLS]
+6 -2
View File
@@ -211,9 +211,13 @@ def _ensure_builtin_definitions_registered(catalog: ToolCatalog) -> None:
import time (usually the process catalog). Test ``catalog_scope(empty=True)``
overrides must still see builtins for ``default_tool_definitions``.
"""
from plyngent.tools import DEFAULT_TOOLS
from plyngent.tools.chat import CHAT_TOOLS
from plyngent.tools.file import FILE_TOOLS
from plyngent.tools.process import PROCESS_TOOLS
from plyngent.tools.todo import TODO_TOOLS
from plyngent.tools.vcs import VCS_TOOLS
for definition in DEFAULT_TOOLS:
for definition in (*FILE_TOOLS, *PROCESS_TOOLS, *VCS_TOOLS, *CHAT_TOOLS, *TODO_TOOLS):
if catalog.get(definition.name) is None:
catalog.register(definition, source=_BUILTIN_SOURCE)
+2 -3
View File
@@ -70,10 +70,9 @@ class SessionState:
session_id: int | str | None = None
data: PersistentDataView[Any] = field(default_factory=_default_session_data)
# Live domain object during migration (also under data["todo"] when views bind).
# Live domain object (also under data["todo"] when views bind).
todo: TodoStack | None = None
# Host hook after todo tools mutate (e.g. CLI memory persist). Prefer this over
# process-global set_todo_stack(on_change=...) when a session is bound.
# Host hook after todo tools mutate (e.g. CLI memory persist).
on_todo_change: Callable[[], None] | None = None
# Soft-confirm grants live map: tool_name → True (Phase 1 key is tool name).
# Durable copy lives under data["grants"] (see plyngent.tools.grants).
+5 -1
View File
@@ -47,8 +47,12 @@ def cleanup_temporary_workspaces() -> int:
"""Remove directories created by :func:`new_temporary_workspace` (chat exit).
Only deletes paths still under the system temp dir. Returns the number of
directories removed.
directories removed. No-op when no instance is bound.
"""
from plyngent.tools.context import get_instance
if get_instance() is None:
return 0
removed = 0
for path in pop_owned_temporary_workspaces():
if not _is_under_system_temp(path):
+4 -47
View File
@@ -9,84 +9,41 @@ if TYPE_CHECKING:
from plyngent.agent.todo_stack import TodoStack, TodoStatus
_stack: TodoStack | None = None
_on_change: Callable[[], None] | None = None
_VALID_STATUS = frozenset({"pending", "in_progress", "done", "cancelled"})
def set_todo_stack(stack: TodoStack | None, *, on_change: Callable[[], None] | None = None) -> None:
"""Bind the session todo stack for tool handlers (and optional persist hook).
Process-global bind remains for CLI/agent hosts that keep a live
:class:`~plyngent.agent.todo_stack.TodoStack` outside the view store.
Prefer session ``data["todo"]`` transactions when available.
"""
global _stack, _on_change # noqa: PLW0603 — intentional process bind
_stack = stack
_on_change = on_change
def get_todo_stack() -> TodoStack | None:
return _stack
def _notify() -> None:
"""Fire host persist hooks: prefer session.on_todo_change, else process bind."""
"""Fire host persist hook on the bound session (if any)."""
from plyngent.tools.context import get_session
session = get_session()
if session is not None and session.on_todo_change is not None:
session.on_todo_change()
return
if _on_change is not None:
_on_change()
def _process_stack() -> TodoStack:
if _stack is None:
msg = "todo stack is not available in this session"
raise RuntimeError(msg)
return _stack
async def _with_todo_stack(mutator: Callable[[TodoStack], str]) -> str:
"""Run *mutator* against the session todo stack and publish changes.
Prefer::
Requires a bound :class:`~plyngent.tools.context.SessionState`::
async with session.data:
stack = session.data["todo"].typed(TodoStack)
...
When a host already bound ``session.todo`` / process ``set_todo_stack``,
mutate that live object and still refresh the view buffer when a session
is bound so commits stay consistent.
"""
from plyngent.agent.todo_stack import TodoStack
from plyngent.tools.context import get_session
session = get_session()
if session is None:
result = mutator(_process_stack())
_notify()
return result
from plyngent.tools.context import require_session
session = require_session()
result = ""
async with session.data as data:
# Prefer host-bound live stack (CLI keeps TodoStack for nags / memory).
if session.todo is not None:
stack = session.todo
elif _stack is not None:
stack = _stack
session.todo = stack
else:
stack = data["todo"].typed(TodoStack)
session.todo = stack
# Keep view domain + buffer in sync for durable commit.
data["todo"].store(stack)
result = mutator(stack)
# View commit serialized to_raw; host on_todo_change may persist CLI memory.
_notify()
return result
+37 -68
View File
@@ -51,9 +51,8 @@ class WorkspaceError(ValueError):
class WorkspacePolicy:
"""Workspace path/command policy for one agent host / instance.
Prefer hanging this on :class:`~plyngent.tools.context.InstanceState.workspace`.
Module-level APIs still fall back to a process-global policy when no instance
is bound (tests and early boot).
Lives on :class:`~plyngent.tools.context.InstanceState.workspace`. There is
no process-global policy bag — hosts must bind instance state around tool use.
"""
root: Path | None = None
@@ -66,79 +65,54 @@ class WorkspacePolicy:
policy_confirm_timeout_seconds: float = DEFAULT_POLICY_CONFIRM_TIMEOUT_SECONDS
# Process fallback when no InstanceState is bound.
_process_policy = WorkspacePolicy()
def _bound_instance() -> InstanceState | None:
from plyngent.tools.context import get_instance
return get_instance()
def active_workspace_policy() -> WorkspacePolicy:
"""Return the policy bag for the bound instance, else the process fallback."""
def require_bound_instance() -> InstanceState:
"""Return the bound instance or raise :class:`WorkspaceError`."""
instance = _bound_instance()
if instance is not None:
return instance.workspace
return _process_policy
if instance is None:
msg = "instance state is not bound; host must set InstanceState around tool execution"
raise WorkspaceError(msg)
return instance
def active_workspace_policy() -> WorkspacePolicy:
"""Return the policy bag for the bound instance (required)."""
return require_bound_instance().workspace
def set_workspace_root(root: Path | str) -> Path:
"""Set the workspace root used by tools; returns the resolved root.
Writes the active policy bag (instance when bound, else process) and keeps
``instance.workspace_root`` in sync for callers that read the facet.
"""
"""Set the workspace root on the bound instance; returns the resolved root."""
path = Path(root).expanduser().resolve()
if not path.is_dir():
msg = f"workspace root is not a directory: {path}"
raise WorkspaceError(msg)
policy = active_workspace_policy()
policy.root = path
instance = _bound_instance()
if instance is not None:
instance.workspace_root = path
# When bound, also keep process root as a bootstrapped default for tools
# that briefly run without context (compat).
if _process_policy.root is None:
_process_policy.root = path
else:
_process_policy.root = path
instance = require_bound_instance()
instance.workspace.root = path
instance.workspace_root = path
return path
def get_workspace_root() -> Path:
"""Return the configured workspace root.
Prefers a bound instance's ``workspace_root`` / policy root; otherwise the
process-global root from :func:`set_workspace_root`.
"""
instance = _bound_instance()
if instance is not None:
if instance.workspace_root is not None:
return instance.workspace_root
if instance.workspace.root is not None:
return instance.workspace.root
if _process_policy.root is None:
msg = "workspace root is not set; call set_workspace_root() first"
raise WorkspaceError(msg)
return _process_policy.root
"""Return the bound instance workspace root."""
instance = require_bound_instance()
if instance.workspace_root is not None:
return instance.workspace_root
if instance.workspace.root is not None:
return instance.workspace.root
msg = "workspace root is not set on the bound instance"
raise WorkspaceError(msg)
def clear_workspace_root() -> None:
"""Clear workspace root (mainly for tests). Does not clear allowlist.
When an instance is bound, clears both the instance facet and the process
fallback so ``get_workspace_root`` cannot revive the old process root.
"""
policy = active_workspace_policy()
policy.root = None
instance = _bound_instance()
if instance is not None:
instance.workspace_root = None
instance.workspace.root = None
_process_policy.root = None
"""Clear workspace root on the bound instance (mainly for tests)."""
instance = require_bound_instance()
instance.workspace.root = None
instance.workspace_root = None
def set_path_denylist(patterns: list[str] | tuple[str, ...] | None) -> None:
@@ -155,7 +129,6 @@ def set_command_denylist(names: list[str] | tuple[str, ...] | frozenset[str] | N
"""Set denied command basenames (None restores defaults)."""
policy = active_workspace_policy()
policy.command_denylist = DEFAULT_COMMAND_DENYLIST if names is None else frozenset(names)
# Session overrides only make sense against the active denylist.
policy.policy_allowed_commands &= policy.command_denylist
@@ -190,7 +163,7 @@ def clear_policy_allowed_commands() -> None:
def grant_policy_command(basename: str) -> None:
"""Allow *basename* for the rest of this process despite the denylist."""
"""Allow *basename* for this instance despite the denylist."""
name = basename.strip()
if name:
active_workspace_policy().policy_allowed_commands.add(name)
@@ -249,22 +222,17 @@ def remove_workspace_allowlist(root: Path | str) -> None:
policy.allowlist.remove(path)
def _primary_roots(policy: WorkspacePolicy) -> list[Path]:
"""Primary workspace roots: bound instance facet then policy root."""
def _primary_roots(instance: InstanceState, policy: WorkspacePolicy) -> list[Path]:
roots: list[Path] = []
instance = _bound_instance()
if instance is not None and instance.workspace_root is not None:
if instance.workspace_root is not None:
roots.append(instance.workspace_root)
if policy.root is not None and policy.root not in roots:
roots.append(policy.root)
# Process root as last resort when instance policy has no root yet.
if _process_policy.root is not None and _process_policy.root not in roots:
roots.append(_process_policy.root)
return roots
def _under_any_root(resolved: Path, policy: WorkspacePolicy) -> bool:
roots = _primary_roots(policy)
def _under_any_root(resolved: Path, instance: InstanceState, policy: WorkspacePolicy) -> bool:
roots = _primary_roots(instance, policy)
roots.extend(policy.allowlist)
for root in roots:
try:
@@ -281,13 +249,14 @@ def resolve_path(path: str | Path) -> Path:
Relative paths resolve against the **primary** workspace root. Absolute
paths may also land under a temporary workspace allowlist entry.
"""
policy = active_workspace_policy()
instance = require_bound_instance()
policy = instance.workspace
root = get_workspace_root()
candidate = Path(path)
if not candidate.is_absolute():
candidate = root / candidate
resolved = candidate.expanduser().resolve()
if not _under_any_root(resolved, policy):
if not _under_any_root(resolved, instance, policy):
msg = f"path escapes workspace root ({root}): {path}"
raise WorkspaceError(msg)
# Normalize separators so denylist entries like ``/secrets/`` match on Windows.