docs: mark phase 3 complete, record the skills and caching findings
Two findings that changed the plan rather than confirming it:
23. Skills require a tool literally named `read`. Curator's tools are all
domain-specific, so every --skill argument was discarded in silence. The
planned split into curator-core / video-arr / books-ingest was inert before
it was written; the policy stays in APPEND_SYSTEM.md. memo-inbox is
unaffected because it registers a restricted `read` override, which is why
the earlier note generalised wrongly from it.
24. A long-lived session is worth far more than the startup it saves: 99.97% of
input read from cache on a continuing conversation against 0% on a new one.
That is what makes the generated tool list necessary rather than merely
tidy -- anything varying at the front of the prompt destroys it -- and it
makes rotation a cost to be bounded rather than applied eagerly.
profile.toml now describes the phase-3 configuration that is actually deployed,
including that the empty `skills` list is a finding and not an oversight.
pi_rpc gains --system-prompt support and no longer guesses whether a `read` tool
will exist; extension_registers_read has to be stated.
harness-layering.md records what transfers from a widely-shared account of
building a personal coding harness on pi, and what does not. The layering frame
holds and the cache-hit figure was the useful part. Its central recommendation --
installing third-party packages -- is disqualifying for an unattended agent
holding tracker credentials, and its discipline layer (AGENTS.md) is precisely
what we block, because it is discovered from every parent directory.
This commit is contained in:
+40
-9
@@ -171,6 +171,16 @@ class PiLaunchConfig:
|
||||
# stays auditable instead of being summarised away. Empty means --no-session.
|
||||
session_id_prefix: str = ""
|
||||
|
||||
# --- personality ------------------------------------------------------
|
||||
# system_prompt REPLACES pi's coding-assistant prompt. Note that pi then
|
||||
# omits the tool list and guidelines entirely: the customPrompt branch of
|
||||
# dist/core/system-prompt.js returns before they are assembled, so
|
||||
# promptSnippet and promptGuidelines never reach the model and the prompt
|
||||
# file must enumerate the tools itself.
|
||||
# See docs/pi-runtime-notes.md section 22.
|
||||
system_prompt: Path | None = None
|
||||
append_system_prompt: Path | None = None
|
||||
|
||||
# --- layer 1: loading isolation --------------------------------------
|
||||
extensions: tuple[Path, ...] = ()
|
||||
skills: tuple[Path, ...] = ()
|
||||
@@ -201,6 +211,10 @@ class PiLaunchConfig:
|
||||
# --- receipts ---------------------------------------------------------
|
||||
receipt_tools: frozenset[str] = frozenset()
|
||||
|
||||
# Set by a scenario whose extension registers a tool named ``read``. Only
|
||||
# affects whether loading skills is warned about; see _read_reachable.
|
||||
extension_registers_read: bool = False
|
||||
|
||||
# --- environment ------------------------------------------------------
|
||||
env_allowlist: tuple[str, ...] = DEFAULT_ENV_ALLOWLIST
|
||||
extra_env: tuple[tuple[str, str], ...] = ()
|
||||
@@ -214,23 +228,35 @@ class PiLaunchConfig:
|
||||
)
|
||||
if self.skills and not self._read_reachable():
|
||||
LOG.warning(
|
||||
"PiLaunchConfig for %r loads skills but no 'read' tool is reachable; "
|
||||
"the skills section will be omitted from the system prompt and the "
|
||||
"skill bodies will be unloadable "
|
||||
"(see docs/pi-runtime-notes.md section 1)",
|
||||
"PiLaunchConfig for %r loads %d skill(s) but no tool named 'read' "
|
||||
"will be active, so pi omits the skills section entirely and the "
|
||||
"skills have no effect whatsoever. Either set "
|
||||
"extension_registers_read=True if the extension provides one, or "
|
||||
"put the content in the system prompt instead "
|
||||
"(see docs/pi-runtime-notes.md sections 1 and 23)",
|
||||
self.display_name,
|
||||
len(self.skills),
|
||||
)
|
||||
|
||||
def _read_reachable(self) -> bool:
|
||||
"""Whether an active tool named ``read`` can plausibly exist.
|
||||
"""Whether a tool named ``read`` will actually be active.
|
||||
|
||||
Pi only emits the skills section when ``read`` is active. With
|
||||
``no_builtin_tools`` the extension is expected to register a restricted
|
||||
``read`` override; with an explicit allowlist ``read`` must be named.
|
||||
Pi emits the skills section only when ``read`` is active. This used to
|
||||
assume that ``no_builtin_tools`` implied an extension supplying a
|
||||
restricted ``read`` override, which is how memo-inbox is built -- but an
|
||||
agent whose tools are all domain-specific has no ``read`` at all, and
|
||||
then every ``--skill`` argument is silently discarded. Measured: with
|
||||
tools [query_library, lookup_online, counts] the prompt contained no
|
||||
skills section and no skill names, with and without --system-prompt.
|
||||
|
||||
So this no longer guesses. An extension that registers ``read`` must say
|
||||
so.
|
||||
"""
|
||||
if self.tools:
|
||||
return "read" in self.tools
|
||||
return self.no_builtin_tools or not self.no_extensions or bool(self.extensions)
|
||||
if not self.no_builtin_tools:
|
||||
return True # the built-in read is active
|
||||
return self.extension_registers_read
|
||||
|
||||
def build_env(self) -> dict[str, str]:
|
||||
env = {k: os.environ[k] for k in self.env_allowlist if k in os.environ}
|
||||
@@ -272,6 +298,11 @@ class PiLaunchConfig:
|
||||
if self.approve:
|
||||
args += ["--approve"]
|
||||
|
||||
if self.system_prompt:
|
||||
args += ["--system-prompt", str(self.system_prompt)]
|
||||
if self.append_system_prompt:
|
||||
args += ["--append-system-prompt", str(self.append_system_prompt)]
|
||||
|
||||
args += [
|
||||
"--provider", self.provider,
|
||||
"--model", self.model,
|
||||
|
||||
@@ -15,6 +15,10 @@ import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
|
||||
from dataclasses import replace # noqa: E402
|
||||
import pathlib # noqa: E402
|
||||
import tempfile # noqa: E402
|
||||
|
||||
from pi_rpc import PiLaunchConfig, PiRpcClient # noqa: E402
|
||||
|
||||
REPO = Path(__file__).resolve().parents[4]
|
||||
@@ -52,6 +56,28 @@ async def main() -> int:
|
||||
failures += not check("--tools absent (registry allowlist would block dynamic tools)",
|
||||
"--tools" not in args)
|
||||
|
||||
print("== system prompt and the skills/read interaction ==")
|
||||
sp = pathlib.Path(tempfile.mkdtemp()) / "SYSTEM.md"
|
||||
sp.write_text("marker", encoding="utf-8")
|
||||
withprompt = replace(cfg, system_prompt=sp, append_system_prompt=sp)
|
||||
prompt_args = withprompt.build_args(None)
|
||||
failures += not check("--system-prompt passed through", "--system-prompt" in prompt_args)
|
||||
failures += not check("--append-system-prompt passed through",
|
||||
"--append-system-prompt" in prompt_args)
|
||||
|
||||
# A dedicated agent whose tools are all domain-specific has no 'read', and pi
|
||||
# then discards every --skill argument silently. Measured, not assumed.
|
||||
noread = replace(cfg, skills=(sp.parent,), no_builtin_tools=True,
|
||||
extension_registers_read=False)
|
||||
failures += not check("skills without a read tool are flagged as ineffective",
|
||||
not noread._read_reachable())
|
||||
failures += not check("an extension that registers read is trusted",
|
||||
replace(noread, extension_registers_read=True)._read_reachable())
|
||||
failures += not check("an explicit allowlist naming read counts",
|
||||
replace(noread, tools=("read", "counts"))._read_reachable())
|
||||
failures += not check("an allowlist without read does not",
|
||||
not replace(noread, tools=("counts",))._read_reachable())
|
||||
|
||||
print("== env is minimal ==")
|
||||
os.environ["SMOKE_FAKE_SECRET"] = "must-not-propagate"
|
||||
env = cfg.build_env()
|
||||
|
||||
Reference in New Issue
Block a user