diff --git a/docs/api-reference.md b/docs/api-reference.md index 5fa207bc..7ddedbdc 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -712,7 +712,7 @@ the levers (base prompt, tool set, MCP/memory toggles) stay server-side. { "personas": [ {"name": "engineer", "display_name": "Engineer", "description": "The stock interactive workstream: full tools, MCP, and memory.", "applies_to_kinds": ["interactive"], "is_default": true}, - {"name": "researcher", "display_name": "Researcher", "description": "Answers questions with evidence, read-only. Never modifies anything.", "applies_to_kinds": ["interactive"], "is_default": false} + {"name": "researcher", "display_name": "Researcher", "description": "Answers questions with evidence — reads and cites, loads tools to verify when needed.", "applies_to_kinds": ["interactive"], "is_default": false} ], "total": 2 } diff --git a/docs/coordinator-skills.md b/docs/coordinator-skills.md index 2b08f726..bba997c8 100644 --- a/docs/coordinator-skills.md +++ b/docs/coordinator-skills.md @@ -103,13 +103,13 @@ Interactive skills compose on top of `base_interactive.md` — a close the loop. Coordinator skills compose on top of -[`base_coordinator.md`](../turnstone/prompts/base_coordinator.md) — +[`personas/orchestrator.md`](../turnstone/prompts/personas/orchestrator.md) — an "orchestrator" framing: decompose, delegate, monitor, synthesise. The base text is short but sets the tone every coordinator skill inherits: -> You are a coordinator on a small, focused infrastructure team. -> Your role is to orchestrate work across the cluster... You do +> You are a coordinator. Your role is to orchestrate work across +> the cluster... You do > not edit files, run shell commands, browse the web, or manipulate > the codebase directly. Children do that. diff --git a/docs/personas.md b/docs/personas.md index 4fc8b262..e7ba2ab7 100644 --- a/docs/personas.md +++ b/docs/personas.md @@ -5,13 +5,13 @@ creation** that controls how its system message is composed and what capability envelope it runs with. Personas answer a recurring operational complaint: the default composition primes every session for heavy tool use, and there was no per-workstream dial to launch a "just write prose" or -"read-only research" session. +"evidence-first research" session. A persona is exactly four levers — no more: | Lever | What it does | |---|---| -| **Base prompt** | Replaces the BASE module of the composed system message (`base.md` / `base_coordinator.md`). *Only* BASE: ENV, CONTEXT, TOOLS, and POLICIES keep composing, so mandatory [prompt policies](governance.md) ride on top of every persona. Empty = the kind's stock base. | +| **Base prompt** | Replaces the BASE module of the composed system message. *Only* BASE: ENV, CONTEXT, TOOLS, and POLICIES keep composing, so mandatory [prompt policies](governance.md) ride on top of every persona. Built-in personas source their prose from a repo file; operator personas store it inline — see [Where persona prompts live](#where-persona-prompts-live). | | **Tool visibility** | Which tools the session advertises. Tri-state: *unrestricted* (tracks tool growth and MCP catalogs), *no tools* (the TOOLS prompt block self-suppresses and zero definitions go on the wire), or an *exact set* of names. Including `tool_search` in a set makes it **soft** — tools the model discovers through search join the visible set; omitting it makes the set **hard** (the search pathway is disabled entirely). On commercial providers a soft set costs one prompt-cache re-prime per `tool_search` expansion, since each expansion rewrites the wire tool set and recomposes the prompt. | | **MCP** | Whether the workstream talks to MCP at all. **Session-wide**: off means no MCP tools for the persona's own hands *or* for in-process task agents, no resource/prompt catalogs, and no listener registrations. This lever expresses infrastructure intent, not behavior shaping. | | **Memory** | Whether the persona's **own hands** get memory: recalled-memory injection into the prompt, memory-directed metacognitive nudges, and the `memory` tool. Task agents keep their own envelope, and compaction spill/markers are session mechanics that are never persona-gated. An exact tool set that hides `memory` also mutes those nudges, and the compaction-resume pointer follows `recall`'s visibility. | @@ -60,7 +60,7 @@ personas existed: | `engineer` *(default)* | interactive | stock | unrestricted | on | on | | `orchestrator` *(default)* | coordinator | stock | unrestricted | on | on | | `scribe` | interactive | custom (faithful structuring of given material) | none | off | off | -| `researcher` | interactive | custom (evidence-first, read-only) | `read_file`, `search`, `web_fetch`, `web_search`, `recall`, `memory` (hard) | off | on | +| `researcher` | interactive | custom (evidence-first) | `read_file`, `search`, `web_fetch`, `web_search`, `recall`, `memory`, `tool_search` (soft) | off | on | | `writer` | interactive | custom (creative writing partner — replaces the removed `/creative`) | none | off | on | | `executive` | coordinator | custom (delegate, interrogate plans, judge outcomes) | spawn/inspect/lifecycle tools plus `memory`: `spawn_workstream`, `spawn_batch`, `send_to_workstream`, `wait_for_workstream`, `inspect_workstream`, `list_workstreams`, `list_nodes`, `close_workstream`, `cancel_workstream`, `memory` (hard) | off | on | @@ -68,13 +68,37 @@ Notes: - `scribe` turns memory off deliberately: recalled memories would contaminate faithful summarization with unrelated context. -- `researcher`'s set is hard (no `tool_search`) — including the escape - hatch would let the model load write tools and break the read-only - promise. +- `researcher`'s set is soft (includes `tool_search`): it starts with + read and evidence tools but can pull in others on demand — e.g. load + `bash` to run a snippet and verify a calculation. It is evidence-first, + not sandboxed; any escalated tool still hits the normal approval path. - Coordinator sessions do not merge MCP today, so the MCP lever on coordinator personas is forward-compatible bookkeeping; it bites on interactive workstreams. +## Where persona prompts live + +Prompt source is explicit in the persona row — two nullable columns, never both empty: + +| `base_prompt_file` | `base_prompt` | Meaning | +|---|---|---| +| set (e.g. `scribe.md`) | — | **built-in**: prose lives in `prompts/personas/`, code-owned and PR-reviewed | +| set | set | built-in with an **operator override** layered on top (the inline text wins) | +| — | set | **operator** persona, inline prose | + +A `CHECK` forbids the both-empty row, so resolution is a plain coalesce — +`base_prompt ?? load(base_prompt_file)` — with no implicit "inherit the default" +branch in application logic. `base_prompt_file` is set only by the migration/code +(the admin API never exposes it): it marks a persona as built-in and blocks +archive, so `engineer` and `orchestrator` can't be removed. To customise a +built-in, set `base_prompt` on it (clear it to revert), or create your own persona. + +The resolved prompt is **frozen into the workstream at creation** — later edits to +a built-in's file or an operator's row never change a running workstream; only new +ones pick up the change. "No persona" is not a state: every workstream is stamped, +and an empty `persona=` resolves to the kind's `is_default` (`engineer` / +`orchestrator`). + ## Choosing a persona Every creation surface takes an optional persona; empty always means the diff --git a/tests/test_migration_063.py b/tests/test_migration_063.py index 62086333..39dcae9c 100644 --- a/tests/test_migration_063.py +++ b/tests/test_migration_063.py @@ -96,10 +96,14 @@ class TestMigration063: "orchestrator", "executive", } - # Zero-touch guarantee: the per-kind defaults carry NO overrides. + # Every built-in is file-backed: base_prompt NULL, prose in + # prompts/personas/.md (the origin marker + built-in flag). + for name in rows: + assert rows[name]["base_prompt"] is None, name + assert rows[name]["base_prompt_file"] == f"{name}.md", name + # Zero-touch guarantee: the per-kind defaults carry no lever overrides. for name, kind in (("engineer", "interactive"), ("orchestrator", "coordinator")): p = rows[name] - assert p["base_prompt"] is None assert p["tool_allowlist"] is None assert p["mcp_enabled"] == 1 assert p["memory_enabled"] == 1 @@ -116,6 +120,7 @@ class TestMigration063: "web_search", "recall", "memory", + "tool_search", ] assert json.loads(rows["writer"]["tool_allowlist"]) == [] assert rows["writer"]["memory_enabled"] == 1 @@ -124,8 +129,6 @@ class TestMigration063: assert "delete_workstream" not in exec_tools assert "tool_search" not in exec_tools # hard set — no escape hatch assert json.loads(rows["executive"]["applies_to_kinds"]) == ["coordinator"] - # /creative parity: writer folds the old prompt. - assert "creative writing partner" in str(rows["writer"]["base_prompt"]) # All seeds enabled. assert all(p["enabled"] == 1 for p in rows.values()) finally: @@ -188,16 +191,66 @@ class TestMigration063: row_persona = conn.execute( sa.text("SELECT persona FROM workstreams WHERE ws_id='ws-creative'") ).fetchone() - # creative_mode='True' → the full writer stamp (all five keys)… - assert stamped == {"ws-creative": "writer"} + # creative_mode='True' → the full writer stamp (all five keys), the + # persona_prompt frozen from prompts/personas/writer.md… + assert stamped["ws-creative"] == "writer" keys = {str(k): str(v) for k, v in cols} assert keys["persona_tools"] == "[]" assert keys["persona_mcp"] == "0" assert keys["persona_memory"] == "1" assert "creative writing partner" in keys["persona_prompt"] assert row_persona is not None and row_persona[0] == "writer" - # …while creative_mode='False' workstreams stay legacy-unstamped. - assert "ws-plain" not in stamped + # …while a non-creative workstream gets its kind default (engineer), + # so no workstream is left personaless. + assert stamped["ws-plain"] == "engineer" + finally: + engine.dispose() + + def test_backfill_stamps_plain_workstreams_by_kind(self, tmp_path: Path) -> None: + # The load-bearing new behaviour: no workstream is left personaless. + # A plain (non-creative) workstream is stamped with its kind's default — + # engineer for interactive, orchestrator for coordinator — carrying that + # persona's resolved (frozen) base prompt. + db_path = tmp_path / "063-backfill.db" + cfg = _alembic_cfg(db_path) + command.upgrade(cfg, "062") + engine = sa.create_engine(f"sqlite:///{db_path}") + try: + with engine.begin() as conn: + for ws_id, kind in (("ws-ic", "interactive"), ("ws-coord", "coordinator")): + conn.execute( + sa.text( + "INSERT INTO workstreams (ws_id, name, state, kind, created, " + "updated) VALUES (:ws, :ws, 'closed', :kind, " + "'2026-01-01T00:00:00', '2026-01-01T00:00:00')" + ), + {"ws": ws_id, "kind": kind}, + ) + command.upgrade(cfg, "063") + + with engine.connect() as conn: + + def _cfg(ws: str, key: str) -> str | None: + r = conn.execute( + sa.text("SELECT value FROM workstream_config WHERE ws_id=:ws AND key=:k"), + {"ws": ws, "k": key}, + ).fetchone() + return None if r is None else str(r[0]) + + assert _cfg("ws-ic", "persona") == "engineer" + assert _cfg("ws-coord", "persona") == "orchestrator" + # Frozen resolved text (from the persona's file), not a slug/empty. + assert "software engineer" in (_cfg("ws-ic", "persona_prompt") or "") + assert "coordinator" in (_cfg("ws-coord", "persona_prompt") or "") + # Kind-default envelope: unrestricted tools, MCP + memory on. + assert _cfg("ws-ic", "persona_tools") == "null" + assert _cfg("ws-ic", "persona_mcp") == "1" + assert _cfg("ws-ic", "persona_memory") == "1" + # The workstreams.persona projection is set too. + row = conn.execute( + sa.text("SELECT persona FROM workstreams WHERE ws_id='ws-coord'") + ).fetchone() + assert row is not None and row[0] == "orchestrator" finally: engine.dispose() diff --git a/tests/test_persona_endpoints.py b/tests/test_persona_endpoints.py index e5632d5c..91796746 100644 --- a/tests/test_persona_endpoints.py +++ b/tests/test_persona_endpoints.py @@ -90,6 +90,7 @@ def seeded(tmp_db: Any) -> str: "persona_id": "p1", "name": "test-scribe", "display_name": "Test Scribe", + "base_prompt": "You are a test scribe.", "tool_allowlist": [], "mcp_enabled": False, "applies_to_kinds": ["interactive"], @@ -252,6 +253,7 @@ class TestArchiveAndDefaultFlipHttp: "persona_id": "p2", "name": "test-eng", "display_name": "Test Eng", + "base_prompt": "You are a test engineer.", "applies_to_kinds": ["interactive"], "is_default": True, } @@ -298,7 +300,10 @@ class TestOrgIdGuard: from turnstone.core.storage import get_storage c = _client(tmp_db, _ALL) - resp = c.post("/v1/api/admin/personas", json={"name": "test-orgless", "org_id": None}) + resp = c.post( + "/v1/api/admin/personas", + json={"name": "test-orgless", "org_id": None, "base_prompt": "O"}, + ) assert resp.status_code == 200 assert resp.json()["org_id"] == "" stored = get_storage().get_persona(resp.json()["persona_id"]) diff --git a/tests/test_persona_guards.py b/tests/test_persona_guards.py index 4bef5eb8..600bb0ae 100644 --- a/tests/test_persona_guards.py +++ b/tests/test_persona_guards.py @@ -128,7 +128,7 @@ class TestEmptyToolset: ) prompt = session.system_messages[0]["content"] assert "You are a scribe on a guard test." in prompt - # base.md's IC framing is REPLACED... + # personas/engineer.md's IC framing is REPLACED... assert "read before you edit" not in prompt # ...but CONTEXT still composes (the removed /creative fork dropped it). assert "Current time:" in prompt or "Session context" in prompt or "User:" in prompt @@ -474,6 +474,7 @@ class TestSpawnPersona: { "persona_id": "px", "name": "coord-only", + "base_prompt": "C", "applies_to_kinds": ["coordinator"], } ) @@ -484,7 +485,12 @@ class TestSpawnPersona: def test_valid_persona_travels_to_spawn_body(self, tmp_db, mock_openai_client) -> None: get_storage().create_persona( - {"persona_id": "py", "name": "scribe", "applies_to_kinds": ["interactive"]} + { + "persona_id": "py", + "name": "scribe", + "base_prompt": "S", + "applies_to_kinds": ["interactive"], + } ) session = self._coord_session(mock_openai_client) item = session._prepare_spawn_workstream("c1", {"persona": "scribe"}) @@ -837,7 +843,12 @@ class TestCliPersona: storage = get_storage() storage.create_persona( - {"persona_id": "p2", "name": "exec", "applies_to_kinds": ["coordinator"]} + { + "persona_id": "p2", + "name": "exec", + "base_prompt": "E", + "applies_to_kinds": ["coordinator"], + } ) with pytest.raises(SystemExit): resolve_cli_persona_kwargs(storage, "exec", None) @@ -1079,6 +1090,7 @@ class TestForkAdoptsStamp: { "persona_id": "pd", "name": "engineer", + "base_prompt": "D", "applies_to_kinds": ["interactive"], "is_default": True, } @@ -1246,7 +1258,13 @@ class TestCreateStampsPersona: @staticmethod def _seed(name: str, kinds: list[str], **extra: Any) -> None: get_storage().create_persona( - {"persona_id": "p_" + name, "name": name, "applies_to_kinds": kinds, **extra} + { + "persona_id": "p_" + name, + "name": name, + "base_prompt": f"You are {name}.", + "applies_to_kinds": kinds, + **extra, + } ) def test_create_stamps_persona_with_create_permission_only(self, _create_app) -> None: diff --git a/tests/test_persona_snapshot.py b/tests/test_persona_snapshot.py index f33f1c58..246b052d 100644 --- a/tests/test_persona_snapshot.py +++ b/tests/test_persona_snapshot.py @@ -36,12 +36,37 @@ class TestSnapshotFromPersona: assert snap.memory is False def test_null_levers_stay_open(self) -> None: - snap = snapshot_from_persona({"name": "engineer", "base_prompt": None}) - assert snap.prompt == "" + # tools NULL, and mcp/memory absent, default to the open envelope. + snap = snapshot_from_persona({"name": "p", "base_prompt": "base"}) + assert snap.prompt == "base" assert snap.tools is None assert snap.mcp is True assert snap.memory is True + def test_file_backed_prompt_resolves_from_file(self) -> None: + # A built-in row (base_prompt NULL, base_prompt_file set) resolves its + # BASE from prompts/personas/ and freezes it into the stamp. + from turnstone.prompts import load_persona_prompt + + snap = snapshot_from_persona( + {"name": "scribe", "base_prompt": None, "base_prompt_file": "scribe.md"} + ) + assert snap.prompt == load_persona_prompt("scribe.md") + assert snap.prompt.startswith("You turn raw material") + + def test_operator_override_wins_over_file(self) -> None: + # base_prompt ?? load(file): an operator override on a built-in row wins. + snap = snapshot_from_persona( + {"name": "scribe", "base_prompt": "OVERRIDE", "base_prompt_file": "scribe.md"} + ) + assert snap.prompt == "OVERRIDE" + + def test_sourceless_persona_raises(self) -> None: + # The storage CHECK forbids this row; if one reaches resolution it must + # fail loudly rather than compose an empty BASE. + with pytest.raises(ValueError, match="no prompt source"): + snapshot_from_persona({"name": "broken", "base_prompt": None}) + class TestConfigRoundTrip: @pytest.mark.parametrize( diff --git a/tests/test_persona_storage.py b/tests/test_persona_storage.py index 060fe201..5d8d0900 100644 --- a/tests/test_persona_storage.py +++ b/tests/test_persona_storage.py @@ -22,6 +22,8 @@ def _mk(backend: Any, name: str, **over: Any) -> dict[str, Any]: "name": name, "display_name": name.title(), "description": "", + # Operator personas author inline prose; base_prompt_file is code-only. + "base_prompt": "You are a test persona.", "applies_to_kinds": ["interactive"], } row.update(over) @@ -37,7 +39,8 @@ class TestPersonaCRUD: # is name.title(), so a hyphenated slug title-cases each segment. p = _mk(backend, "test-scribe") assert p["display_name"] == "Test-Scribe" - assert p["base_prompt"] is None + assert p["base_prompt"] == "You are a test persona." + assert p["base_prompt_file"] is None # operator persona — no file source assert p["tool_allowlist"] is None assert p["mcp_enabled"] is True assert p["memory_enabled"] is True @@ -60,7 +63,9 @@ class TestPersonaCRUD: def test_duplicate_name_rejected(self, backend: Any) -> None: _mk(backend, "test-scribe") with pytest.raises(ValueError, match="already exists"): - backend.create_persona({"persona_id": "other", "name": "test-scribe"}) + backend.create_persona( + {"persona_id": "other", "name": "test-scribe", "base_prompt": "x"} + ) def test_missing_identity_rejected(self, backend: Any) -> None: with pytest.raises(ValueError, match="persona_id and name"): @@ -294,7 +299,9 @@ class TestPersonaStorageHardening: backend._conn = _racing_conn try: with pytest.raises(ValueError, match="already exists"): - backend.create_persona({"persona_id": "racer-2", "name": "racer"}) + backend.create_persona( + {"persona_id": "racer-2", "name": "racer", "base_prompt": "x"} + ) finally: backend._conn = real_conn @@ -316,7 +323,7 @@ class TestPersonaStorageHardening: "description, base_prompt, tool_allowlist, mcp_enabled, " "memory_enabled, applies_to_kinds, is_default, enabled, " "org_id, created_by, created, updated) VALUES " - "(:pid, :pid, '', '', NULL, NULL, 1, 1, :kinds, 1, 1, " + "(:pid, :pid, '', '', 'base', NULL, 1, 1, :kinds, 1, 1, " "'', '', :now, :now)" ), {"pid": pid, "kinds": '["interactive"]', "now": now}, @@ -333,3 +340,113 @@ class TestPersonaStorageHardening: assert backend.get_persona("id-promotee")["is_default"] is False assert backend.get_persona("mfg-d1")["is_default"] is True assert backend.get_persona("mfg-d2")["is_default"] is True + + +class TestPromptSource: + """The explicit prompt-source model: base_prompt (inline) vs + base_prompt_file (built-in, code-only), coalesced, never both-NULL.""" + + @staticmethod + def _insert_builtin(backend: Any, name: str, **over: Any) -> str: + """Manufacture a built-in row (base_prompt_file set) directly — the + create_persona API never sets base_prompt_file, so a raw insert models + what the migration seeds.""" + pid = f"bi-{name}" + cols = { + "persona_id": pid, + "name": name, + "display_name": name.title(), + "description": "", + "base_prompt": None, + "base_prompt_file": f"{name}.md", + "tool_allowlist": None, + "mcp_enabled": 1, + "memory_enabled": 1, + "applies_to_kinds": '["interactive"]', + "is_default": 0, + "enabled": 1, + "org_id": "", + "created_by": "", + "created": "2026-01-01T00:00:00", + "updated": "2026-01-01T00:00:00", + } + cols.update(over) + with backend._engine.begin() as conn: + conn.execute( + sa.text( + "INSERT INTO personas (" + + ", ".join(cols) + + ") VALUES (" + + ", ".join(f":{c}" for c in cols) + + ")" + ), + cols, + ) + return pid + + def test_create_operator_without_prompt_rejected(self, backend: Any) -> None: + with pytest.raises(ValueError, match="requires a base_prompt"): + backend.create_persona({"persona_id": "np", "name": "no-prompt"}) + + def test_check_rejects_sourceless_row(self, backend: Any) -> None: + # Both columns NULL is forbidden at the storage edge, not just in app + # logic — a raw insert must trip the CHECK constraint. + with pytest.raises(sa.exc.IntegrityError), backend._engine.begin() as conn: + conn.execute( + sa.text( + "INSERT INTO personas (persona_id, name, display_name, " + "description, base_prompt, base_prompt_file, tool_allowlist, " + "mcp_enabled, memory_enabled, applies_to_kinds, is_default, " + "enabled, org_id, created_by, created, updated) VALUES " + "('x', 'x', '', '', NULL, NULL, NULL, 1, 1, '[\"interactive\"]', " + "0, 1, '', '', :now, :now)" + ), + {"now": "2026-01-01T00:00:00"}, + ) + + def test_builtin_cannot_be_archived(self, backend: Any) -> None: + pid = self._insert_builtin(backend, "bi-scribe") + with pytest.raises(ValueError, match="cannot archive a built-in"): + backend.update_persona(pid, enabled=False) + assert backend.get_persona(pid)["enabled"] is True + + def test_builtin_base_prompt_override_is_editable(self, backend: Any) -> None: + # A built-in's inline override IS settable (it wins over the file); the + # file source and its undeletable identity are what stay fixed. + pid = self._insert_builtin(backend, "bi-eng") + assert backend.update_persona(pid, base_prompt="ORG OVERRIDE") is True + got = backend.get_persona(pid) + assert got["base_prompt"] == "ORG OVERRIDE" + assert got["base_prompt_file"] == "bi-eng.md" + + def test_operator_cannot_clear_base_prompt(self, backend: Any) -> None: + _mk(backend, "op-persona") # base_prompt set, no file + with pytest.raises(ValueError, match="cannot clear base_prompt"): + backend.update_persona("id-op-persona", base_prompt=" ") + + def test_base_prompt_file_is_immutable_via_update(self, backend: Any) -> None: + # base_prompt_file is not in PERSONA_MUTABLE — update silently ignores it. + pid = self._insert_builtin(backend, "bi-immut") + backend.update_persona(pid, base_prompt_file="hijack.md", display_name="X") + assert backend.get_persona(pid)["base_prompt_file"] == "bi-immut.md" + + def test_create_with_only_base_prompt_file_reports_missing_base_prompt( + self, backend: Any + ) -> None: + # base_prompt_file is code-only: supplying it via the operator create path + # must NOT satisfy the guard (it's dropped before the INSERT), so the + # caller gets the clear 'requires a base_prompt' — never the misleading + # 'name already exists' the raw CHECK violation would surface. + with pytest.raises(ValueError, match="requires a base_prompt"): + backend.create_persona( + {"persona_id": "ff", "name": "file-only", "base_prompt_file": "scribe.md"} + ) + assert backend.get_persona("ff") is None + + def test_builtin_can_clear_base_prompt_override(self, backend: Any) -> None: + # Clearing an operator override on a BUILT-IN reverts to its file — allowed + # (an operator persona, with no fallback source, cannot; tested above). + pid = self._insert_builtin(backend, "bi-clear", base_prompt="ORG OVERRIDE") + assert backend.get_persona(pid)["base_prompt"] == "ORG OVERRIDE" + assert backend.update_persona(pid, base_prompt="") is True + assert backend.get_persona(pid)["base_prompt"] is None # reverted to file diff --git a/tests/test_prompts.py b/tests/test_prompts.py index bddfaf31..23505663 100644 --- a/tests/test_prompts.py +++ b/tests/test_prompts.py @@ -37,7 +37,7 @@ def test_smoke_all_client_types(ct: ClientType) -> None: available_tools=_ALL_TOOLS, ) # BASE content present - assert "resident engineer" in result + assert "software engineer" in result # CONTEXT present assert "sarah.chen" in result assert "2026-03-31" in result @@ -133,11 +133,16 @@ def test_missing_policy_file() -> None: def test_base_module_isolation() -> None: - from turnstone.prompts import _load + # EVERY built-in persona file is a BASE module (compose_system_message loads + # it as the base), so all must be environment-agnostic — not just engineer. + from turnstone.prompts import _PROMPTS_DIR, _load - base = _load("base.md") - for forbidden in ("Mermaid", "KaTeX", "terminal", "monospace", "Slack", "Discord"): - assert forbidden not in base, f"BASE must not contain '{forbidden}'" + persona_files = sorted(p.name for p in (_PROMPTS_DIR / "personas").glob("*.md")) + assert persona_files, "expected built-in persona base files under prompts/personas/" + for fname in persona_files: + base = _load(f"personas/{fname}") + for forbidden in ("Mermaid", "KaTeX", "terminal", "monospace", "Slack", "Discord"): + assert forbidden not in base, f"BASE {fname} must not contain '{forbidden}'" # --------------------------------------------------------------------------- @@ -391,17 +396,17 @@ def test_coordinator_kind_selects_coord_tools() -> None: def test_coordinator_kind_uses_orchestrator_base() -> None: - """kind='coordinator' swaps in base_coordinator.md.""" + """kind='coordinator' swaps in personas/orchestrator.md.""" result = compose_system_message( ClientType.CLI, _VALID_CTX, frozenset({"spawn_workstream"}), kind="coordinator", ) - # IC-framing phrases from base.md should NOT appear. + # IC-framing phrases from personas/engineer.md should NOT appear. for ic_phrase in ("read before you edit", "commits you make"): assert ic_phrase not in result, f"coordinator base leaked IC framing: {ic_phrase!r}" - # Orchestrator-framing phrases from base_coordinator.md should appear. + # Orchestrator-framing phrases from personas/orchestrator.md should appear. assert "orchestrate" in result assert "delegate" in result diff --git a/tests/test_provider_xai.py b/tests/test_provider_xai.py index baca39e9..6ec8ba81 100644 --- a/tests/test_provider_xai.py +++ b/tests/test_provider_xai.py @@ -155,7 +155,9 @@ class TestBuildKwargs: kwargs = provider._build_kwargs( model="grok-4.3", messages=[{"role": "user", "content": "hi"}], - tools=None, + # web_search def present → replace-only injection fires → the + # call_output include is forwarded (contrast the suppression test). + tools=[{"type": "function", "function": {"name": "web_search"}}], max_tokens=512, temperature=0.5, reasoning_effort="low", @@ -172,7 +174,7 @@ class TestBuildKwargs: kwargs = provider._build_kwargs( model="grok-4.3", messages=[{"role": "user", "content": "hi"}], - tools=None, + tools=[{"type": "function", "function": {"name": "web_search"}}], max_tokens=512, temperature=0.5, reasoning_effort="low", @@ -182,10 +184,32 @@ class TestBuildKwargs: ) includes = kwargs.get("include") or [] assert "reasoning.encrypted_content" not in includes - # `*_call_output` still added because xAI hides those outputs - # regardless of the replay flag. + # `*_call_output` still added (independent of the replay flag) because + # the web_search def survived and the native tool was injected. assert "web_search_call_output" in includes + def test_call_output_include_suppressed_when_tool_not_injected( + self, provider: XAIProvider + ) -> None: + # Orphan-include guard: with the web_search client def hidden (persona / + # coordinator visibility set), the base does NOT inject the native tool, + # so xAI must not forward a web_search_call_output include for a tool + # absent from `tools`. + kwargs = provider._build_kwargs( + model="grok-4.3", + messages=[{"role": "user", "content": "hi"}], + tools=[{"type": "function", "function": {"name": "read_file"}}], + max_tokens=512, + temperature=0.5, + reasoning_effort="low", + deferred_names=None, + capabilities=None, + replay_reasoning_to_model=True, + ) + includes = kwargs.get("include") or [] + assert "web_search_call_output" not in includes + assert {"type": "web_search"} not in (kwargs.get("tools") or []) + def test_include_omitted_when_no_server_side_tools(self, provider: XAIProvider) -> None: # Custom caps row with no server-side tools and no legacy # web-search flag — include[] should carry only the diff --git a/tests/test_providers.py b/tests/test_providers.py index a37d7163..bc12ce68 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -4197,6 +4197,45 @@ class TestResponsesParamBuilding: ) assert kwargs["store"] is False + def _kwargs_with(self, tools: list[dict[str, Any]], caps: ModelCapabilities) -> dict[str, Any]: + return self.provider._build_kwargs( + model="gpt-5.4", + messages=[{"role": "user", "content": "Hi"}], + tools=tools, + max_tokens=4096, + temperature=0.5, + reasoning_effort="medium", + deferred_names=None, + capabilities=caps, + ) + + def test_server_side_web_search_needs_surviving_client_def(self) -> None: + caps = ModelCapabilities(supports_web_search=True) + # Client def present (unrestricted / allowlisted) → native injected. + with_def = self._kwargs_with( + [{"type": "function", "function": {"name": "web_search"}}], caps + ) + assert {"type": "web_search"} in (with_def.get("tools") or []) + # Client def hidden by the persona/coordinator envelope → suppressed. + without_def = self._kwargs_with( + [{"type": "function", "function": {"name": "read_file"}}], caps + ) + assert {"type": "web_search"} not in (without_def.get("tools") or []) + + def test_server_side_injection_generalizes_beyond_web_search(self) -> None: + # The replace-only rule applies to EVERY server-side tool: a provider- + # specific one injects only with a same-named client def, so a restricted + # persona that never allowlisted it can't get it injected past the wire. + caps = ModelCapabilities(server_side_tools=("code_exec",)) + without_def = self._kwargs_with( + [{"type": "function", "function": {"name": "read_file"}}], caps + ) + assert {"type": "code_exec"} not in (without_def.get("tools") or []) + with_def = self._kwargs_with( + [{"type": "function", "function": {"name": "code_exec"}}], caps + ) + assert {"type": "code_exec"} in (with_def.get("tools") or []) + def test_cache_retention_for_gpt5(self) -> None: kwargs = self.provider._build_kwargs( model="gpt-5.4", diff --git a/tests/test_schema_parity.py b/tests/test_schema_parity.py new file mode 100644 index 00000000..6cb0e128 --- /dev/null +++ b/tests/test_schema_parity.py @@ -0,0 +1,87 @@ +"""Schema parity: `metadata.create_all` must match `alembic upgrade head`. + +The codebase defines its schema twice — `_schema.py` (the SQLAlchemy metadata +that `create_all` builds, used for fast ephemeral test DBs and +``SQLiteBackend(create_tables=True)``) and the Alembic migration chain (which +builds production DBs incrementally). They are kept in sync BY HAND. + +Nothing else enforces that they agree, so a column added to a migration but not +to `_schema.py` (or the reverse) would silently give `create_all`-based tests a +different schema than production — and most tests use `create_all`, so a +migration bug could pass CI unnoticed. This test is that enforcement: it fails +the moment the two paths drift on a table, column, or named constraint. + +(It does NOT check seed DATA: `create_all` builds structure only, so migration +seeds — e.g. the built-in personas — exist only on migrated DBs. Tests that +need seed rows must run migrations or seed explicitly; that gap is by design.) +""" + +from __future__ import annotations + +from pathlib import Path + +import sqlalchemy as sa +from alembic import command +from alembic.config import Config + +_MIGRATIONS = str(Path(__file__).resolve().parent.parent / "turnstone/core/storage/migrations") + + +def _inspect_migrated(db_path: Path) -> sa.Inspector: + cfg = Config() + cfg.set_main_option("script_location", _MIGRATIONS) + cfg.set_main_option("sqlalchemy.url", f"sqlite:///{db_path}") + command.upgrade(cfg, "head") + return sa.inspect(sa.create_engine(f"sqlite:///{db_path}")) + + +def _inspect_create_all(db_path: Path) -> sa.Inspector: + from turnstone.core.storage._schema import metadata + + engine = sa.create_engine(f"sqlite:///{db_path}") + metadata.create_all(engine) + return sa.inspect(engine) + + +def test_create_all_matches_migrations(tmp_path: Path) -> None: + mig = _inspect_migrated(tmp_path / "migrated.db") + meta = _inspect_create_all(tmp_path / "create_all.db") + + mig_tables = set(mig.get_table_names()) - {"alembic_version"} + meta_tables = set(meta.get_table_names()) + assert mig_tables == meta_tables, ( + f"table drift — only in migrations: {sorted(mig_tables - meta_tables)}; " + f"only in create_all: {sorted(meta_tables - mig_tables)}" + ) + + col_drift: dict[str, dict[str, list[str]]] = {} + check_drift: dict[str, dict[str, list[str]]] = {} + for t in sorted(mig_tables): + mc = {c["name"] for c in mig.get_columns(t)} + ec = {c["name"] for c in meta.get_columns(t)} + if mc != ec: + col_drift[t] = { + "only_migrations": sorted(mc - ec), + "only_create_all": sorted(ec - mc), + } + # Named CHECK constraints only — unnamed ones reflect as backend noise. + mck = {c["name"] for c in mig.get_check_constraints(t) if c.get("name")} + eck = {c["name"] for c in meta.get_check_constraints(t) if c.get("name")} + if mck != eck: + check_drift[t] = { + "only_migrations": sorted(mck - eck), + "only_create_all": sorted(eck - mck), + } + + assert not col_drift, f"column drift: {col_drift}" + assert not check_drift, f"check-constraint drift: {check_drift}" + + +def test_personas_prompt_source_check_present_on_both_paths(tmp_path: Path) -> None: + # Guards the personas feature specifically: the base_prompt/base_prompt_file + # source CHECK must exist on BOTH build paths, not just the one under test. + mig = _inspect_migrated(tmp_path / "m.db") + meta = _inspect_create_all(tmp_path / "c.db") + for insp in (mig, meta): + names = {c.get("name") for c in insp.get_check_constraints("personas")} + assert "ck_personas_prompt_source" in names diff --git a/turnstone/api/console_schemas.py b/turnstone/api/console_schemas.py index 457f11b0..d8cdf301 100644 --- a/turnstone/api/console_schemas.py +++ b/turnstone/api/console_schemas.py @@ -1082,7 +1082,14 @@ class CreatePersonaRequest(BaseModel): name: str = Field(description="Immutable slug (lowercase: a-z, 0-9, '-', '_')") display_name: str = "" description: str = "" - base_prompt: str | None = None + base_prompt: str | None = Field( + default=None, + description=( + "Inline BASE override — required. Every persona must name a prompt " + "source; built-in file-backed personas are seeded by migration, not " + "created here, so an operator-created persona must supply base_prompt." + ), + ) tool_allowlist: list[str] | None = None mcp_enabled: bool = True memory_enabled: bool = True @@ -1095,12 +1102,13 @@ class CreatePersonaRequest(BaseModel): class UpdatePersonaRequest(BaseModel): """PATCH body — absent fields are left unchanged. - Explicit ``null`` is meaningful only on the two resettable fields: - ``base_prompt: null`` clears the override back to the kind's stock - BASE, and ``tool_allowlist: null`` resets to unrestricted. ``null`` - on the boolean flags or ``applies_to_kinds`` is ignored (treated as - absent), so a client serializing unset optionals as null cannot - archive a persona or flip levers by accident. + Explicit ``null`` resets ``tool_allowlist`` to unrestricted, and — on a + BUILT-IN persona only — clears ``base_prompt`` (the operator override), + reverting to that persona's file-backed prompt. An OPERATOR persona has no + fallback source, so ``base_prompt: null`` on one is rejected: every persona + must name a prompt source. ``null`` on the boolean flags or + ``applies_to_kinds`` is ignored (treated as absent), so a client serializing + unset optionals as null cannot archive a persona or flip levers by accident. Archive = ``{"enabled": false}``; default flip = ``{"is_default": true}`` on the successor (storage demotes the incumbent atomically). ``name`` diff --git a/turnstone/core/personas.py b/turnstone/core/personas.py index 881937e0..a2f41974 100644 --- a/turnstone/core/personas.py +++ b/turnstone/core/personas.py @@ -8,7 +8,9 @@ never changes an existing workstream, and a workstream outlives its persona. The five keys (all-or-none — a partial stamp is corruption, not a fallback): - ``persona`` — the persona's slug (display + forensics) -- ``persona_prompt`` — BASE-module override; ``""`` = the kind's stock base +- ``persona_prompt`` — the resolved BASE text, frozen at create (an + operator override, else the built-in's file content). Legacy stamps may + carry ``""``; compose then falls back to the kind's default file. - ``persona_tools`` — JSON tri-state: ``null`` = unrestricted, ``[]`` = hard empty, ``[names]`` = exact visibility set (``tool_search`` membership decides soft vs hard) @@ -45,7 +47,7 @@ class PersonaSnapshot: """Immutable, self-contained persona stamp held by a live session.""" name: str - prompt: str # "" = use the kind's stock BASE module + prompt: str # resolved BASE text, frozen at create ("" only in legacy stamps) tools: frozenset[str] | None # None = unrestricted; frozenset() = hard empty mcp: bool memory: bool @@ -88,12 +90,39 @@ def resolve_persona_for_kind( return row, "" +def _resolve_base_prompt(persona: Mapping[str, Any]) -> str: + """Coalesce a persona row to its BASE prompt text. + + ``base_prompt ?? load(base_prompt_file)``: an operator's inline override + wins; otherwise the built-in's repo file under ``prompts/personas/``. The + storage CHECK guarantees at least one is set, so a row reaching the final + branch is corrupt and fails loudly rather than composing an empty BASE. + """ + text = persona.get("base_prompt") + if text: + return str(text) + pfile = persona.get("base_prompt_file") + if pfile: + from turnstone.prompts import load_persona_prompt # lazy: avoid import cycle + + return load_persona_prompt(str(pfile)) + raise ValueError( + f"persona {persona.get('name')!r} has no prompt source: " + "base_prompt and base_prompt_file are both empty" + ) + + def snapshot_from_persona(persona: Mapping[str, Any]) -> PersonaSnapshot: - """Build the stamp from a storage persona row — the resolve-once moment.""" + """Build the stamp from a storage persona row — the resolve-once moment. + + The BASE prompt is resolved to concrete text here (operator override, else + the built-in's file) and frozen into the snapshot, so a later edit to the + file or the row never changes an already-created workstream. + """ tools = persona.get("tool_allowlist") return PersonaSnapshot( name=str(persona["name"]), - prompt=persona.get("base_prompt") or "", + prompt=_resolve_base_prompt(persona), tools=None if tools is None else frozenset(tools), mcp=bool(persona.get("mcp_enabled", True)), memory=bool(persona.get("memory_enabled", True)), diff --git a/turnstone/core/providers/_openai_responses.py b/turnstone/core/providers/_openai_responses.py index 85973a07..cc7cdfeb 100644 --- a/turnstone/core/providers/_openai_responses.py +++ b/turnstone/core/providers/_openai_responses.py @@ -375,15 +375,24 @@ class OpenAIResponsesProvider: # ``{"type": "web_search"}`` appended. Subclasses (e.g. # ``XAIProvider``) opt their own provider-specific server tools # into ``caps.server_side_tools`` and inherit this injection. - has_client_web_search = any( - t.get("function", {}).get("name") == "web_search" for t in tools or [] - ) + # Replace-only for EVERY server-side tool: inject the native entry only + # when a same-named client def survived the session's visibility filter. + # This ties server-side tools into the persona / coordinator envelope — + # a visibility set that hides (or never allowlisted) the client def also + # suppresses the native injection, closing the gap where a provider- + # specific server-side tool would otherwise inject past a restricted + # persona. web_search is the only such tool today; a future server-side + # tool must ship a client def to be injectable (and thus gateable). + # NOTE: the match is by exact string — the caps ``type`` must equal the + # client def's ``name`` (true for web_search). A tool whose native type + # differs from its client name (e.g. ``web_search_preview`` vs a + # ``web_search`` def) would need an explicit type→name map added here, or + # it silently won't inject. + client_tool_names = { + t.get("function", {}).get("name") for t in tools or [] if "function" in t + } for tool_type in resolve_server_side_tools(caps): - # Replace-only: the native web_search entry stands in for the - # client def. A request whose envelope hides ``web_search`` - # (persona visibility set, coordinator toolset) gets no native - # search either. - if tool_type == "web_search" and not has_client_web_search: + if tool_type not in client_tool_names: continue converted_tools = converted_tools or [] if not any(t.get("type") == tool_type for t in converted_tools): diff --git a/turnstone/core/providers/_xai.py b/turnstone/core/providers/_xai.py index 3f00aa98..8c7882fa 100644 --- a/turnstone/core/providers/_xai.py +++ b/turnstone/core/providers/_xai.py @@ -196,8 +196,16 @@ class XAIProvider(OpenAIResponsesProvider): effective_tools = resolve_server_side_tools(caps) if not effective_tools: return kwargs + # Only forward a ``_call_output`` include for a server-side tool + # the base actually injected. The base now gates native injection on a + # surviving client def (replace-only), so a tool suppressed by a + # persona/coordinator visibility set must not leave an orphan include + # for a tool absent from ``tools`` (which xAI may reject). + injected_types = {t.get("type") for t in (kwargs.get("tools") or []) if isinstance(t, dict)} includes = list(kwargs.get("include") or []) for tool_type in effective_tools: + if tool_type not in injected_types: + continue output_include = f"{tool_type}_call_output" if output_include not in includes: includes.append(output_include) diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 383dff9a..00cbf550 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -1342,13 +1342,15 @@ class ChatSession: # must not change an existing workstream. ``None`` = legacy # pre-persona workstream — all levers at their open positions, # byte-identical to today. - self._persona_name: str = persona_snapshot.name if persona_snapshot else "" - self._persona_prompt: str = persona_snapshot.prompt if persona_snapshot else "" - self._persona_tools: frozenset[str] | None = ( - persona_snapshot.tools if persona_snapshot else None - ) - self._persona_mcp: bool = persona_snapshot.mcp if persona_snapshot else True - self._persona_memory: bool = persona_snapshot.memory if persona_snapshot else True + # Persona levers — declared here for typing, populated from the stamp + # (or the open defaults) by the shared helper so construction and resume + # adoption can't drift on the default values. + self._persona_name: str + self._persona_prompt: str + self._persona_tools: frozenset[str] | None + self._persona_mcp: bool + self._persona_memory: bool + self._apply_persona_snapshot(persona_snapshot) self._title_generated = False self._read_files: set[str] = set() # The canonical in-memory trajectory. Wire prep (fold/repair) + the @@ -2100,16 +2102,9 @@ class ChatSession: # creation. Legacy pre-persona workstreams must never get # back-stamped by an incidental _save_config call (/model etc.) — # absence of the keys IS their persona state. - if self._persona_name: - config.update( - PersonaSnapshot( - name=self._persona_name, - prompt=self._persona_prompt, - tools=self._persona_tools, - mcp=self._persona_mcp, - memory=self._persona_memory, - ).to_config() - ) + snap = self._current_persona_snapshot() + if snap is not None: + config.update(snap.to_config()) save_workstream_config(self._ws_id, config) def _load_skills(self) -> None: @@ -3136,11 +3131,7 @@ class ChatSession: # refused before adoption). A fork keeps its own creation-time # stamp. Corrupt stamps raise — never silently rewritten. if not fork: - self._persona_name = snap.name if snap else "" - self._persona_prompt = snap.prompt if snap else "" - self._persona_tools = snap.tools if snap else None - self._persona_mcp = snap.mcp if snap else True - self._persona_memory = snap.memory if snap else True + self._apply_persona_snapshot(snap) if not self._persona_mcp and self._mcp_client is not None: self._drop_mcp_surface() # Re-gate tool search under the adopted stamp: a hard set must @@ -4485,6 +4476,32 @@ class ChatSession: """ return self._persona_tools is not None and "tool_search" not in self._persona_tools + def _apply_persona_snapshot(self, snap: PersonaSnapshot | None) -> None: + """Set the five persona lever attrs from a stamp — or the open defaults + when ``snap`` is None (legacy / bare session). The single owner of the + all-or-none snapshot→attr mapping, shared by construction and resume + adoption so the defaults can't drift between the two sites. Callers own + any follow-up (MCP-surface drop, tool-search rebuild) themselves.""" + self._persona_name = snap.name if snap else "" + self._persona_prompt = snap.prompt if snap else "" + self._persona_tools = snap.tools if snap else None + self._persona_mcp = snap.mcp if snap else True + self._persona_memory = snap.memory if snap else True + + def _current_persona_snapshot(self) -> PersonaSnapshot | None: + """Reconstruct the live persona stamp from the lever attrs, or None for a + legacy (never-stamped) workstream — the inverse of + :meth:`_apply_persona_snapshot`, consumed by ``_save_config``.""" + if not self._persona_name: + return None + return PersonaSnapshot( + name=self._persona_name, + prompt=self._persona_prompt, + tools=self._persona_tools, + mcp=self._persona_mcp, + memory=self._persona_memory, + ) + def _persona_tool_visible(self, name: str) -> bool: """Whether the persona's envelope lets ``name`` reach the wire/prompt. @@ -4495,6 +4512,14 @@ class ChatSession: through ``tool_search`` (the session's discovered set unions with the allowlist — the escape hatch stays honest: once advertised as loaded, a tool doesn't vanish from the wire). + + The discovered-set union is **per-process only**: ``ToolSearchManager`` + holds the expanded names in memory and they are not stamped into + ``workstream_config``, so a server restart / rehydrate narrows visibility + back to the stamped allowlist. That is the fail-safe direction (a + restricted persona re-tightens, never widens), and the model can + re-discover on demand; durably persisting the expanded set is a + deferred option (see #756), not a promise this predicate makes. """ if not self._persona_memory and name == "memory": return False @@ -10223,6 +10248,15 @@ class ChatSession: return bool(val) return default + @staticmethod + def _flatten_spawn_arg(value: Any, cap: int) -> str: + """Collapse whitespace and cap a model-authored spawn arg before it + reaches the trusted approval-header chrome (and the child's stored + metadata). Real slugs/names are short with no interior whitespace, so + this only reshapes invalid input — which then fails downstream showing + the sanitized value.""" + return " ".join((value or "").split())[:cap] + def _prepare_spawn_workstream(self, call_id: str, args: dict[str, Any]) -> dict[str, Any]: if self._coord_client is None: return self._coord_tool_error( @@ -10232,17 +10266,17 @@ class ChatSession: # workstream ready to receive the first turn via # send_to_workstream. The tool JSON advertises this explicitly. initial_message = (args.get("initial_message") or "").strip() - skill = (args.get("skill") or "").strip() - name = (args.get("name") or "").strip() model = (args.get("model") or "").strip() - target_node = (args.get("target_node") or "").strip() project = (args.get("project") or "").strip() - # Flatten whitespace and cap before the tag reaches the trusted - # approval-header chrome — the value is model-authored and, when - # storage is down, reaches the header unvalidated. Real slugs are - # ≤64 chars with no whitespace, so this only reshapes invalid names - # (which then fail resolution showing the sanitized string). - persona = " ".join((args.get("persona") or "").split())[:64] + # Flatten whitespace + cap the fields that reach the trusted approval- + # header chrome (skill/persona/target_node) or the child's stored + # metadata (name) before they get there — all model-authored and, when + # storage is down, reaching unvalidated. ``model``/``project`` are + # validated downstream (registry / project ACL) and don't hit the header. + skill = self._flatten_spawn_arg(args.get("skill"), 64) + name = self._flatten_spawn_arg(args.get("name"), 120) + target_node = self._flatten_spawn_arg(args.get("target_node"), 64) + persona = self._flatten_spawn_arg(args.get("persona"), 64) if persona: persona_err = self._validate_child_persona(persona) if persona_err: @@ -10399,11 +10433,14 @@ class ChatSession: preview_rows.append(f" {idx}. [invalid — not an object]") continue initial_message = self._coord_str_arg(raw, "initial_message").strip() - skill = self._coord_str_arg(raw, "skill").strip() - name = self._coord_str_arg(raw, "name").strip() model = self._coord_str_arg(raw, "model").strip() - target_node = self._coord_str_arg(raw, "target_node").strip() - persona = self._coord_str_arg(raw, "persona").strip() + # Flatten + cap the fields that reach the per-child preview chrome + # (skill/persona/target_node) or stored metadata (name) — the same + # one-pass treatment spawn_workstream applies. + skill = self._flatten_spawn_arg(self._coord_str_arg(raw, "skill"), 64) + name = self._flatten_spawn_arg(self._coord_str_arg(raw, "name"), 120) + target_node = self._flatten_spawn_arg(self._coord_str_arg(raw, "target_node"), 64) + persona = self._flatten_spawn_arg(self._coord_str_arg(raw, "persona"), 64) spec: dict[str, Any] = { "idx": idx, "initial_message": initial_message, diff --git a/turnstone/core/storage/_postgresql.py b/turnstone/core/storage/_postgresql.py index 18c7fc2d..62b483bd 100644 --- a/turnstone/core/storage/_postgresql.py +++ b/turnstone/core/storage/_postgresql.py @@ -5602,6 +5602,14 @@ class PostgreSQLBackend: values = _serialize_persona_fields(persona) if not values.get("persona_id") or not values.get("name"): raise ValueError("persona requires persona_id and name") + # base_prompt_file is code-only — set only by the migration seeds, never + # via this operator-facing path. Drop it so a caller can't smuggle a + # file ref past the guard: the INSERT omits the column, so a supplied + # base_prompt_file would otherwise satisfy this check yet trip the CHECK, + # surfaced as a misleading name-collision. Operators supply base_prompt. + values.pop("base_prompt_file", None) + if not values.get("base_prompt"): + raise ValueError("persona requires a base_prompt") now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S") with self._conn() as conn: existing = conn.execute( @@ -5673,6 +5681,14 @@ class PostgreSQLBackend: if row is None: return False current = _persona_row_to_dict(row) + builtin = bool(current.get("base_prompt_file")) + # Built-ins are code-owned: their base_prompt (override) is editable, + # but the origin marker blocks archiving them. Operator personas + # have no file to fall back on, so their only source can't be cleared. + if builtin and "enabled" in fields and not fields["enabled"]: + raise ValueError("cannot archive a built-in persona") + if not builtin and "base_prompt" in values and not values.get("base_prompt"): + raise ValueError("cannot clear base_prompt on an operator persona") if current["is_default"]: if "enabled" in fields and not fields["enabled"]: raise ValueError("the default persona cannot be archived") diff --git a/turnstone/core/storage/_schema.py b/turnstone/core/storage/_schema.py index 5b50b7e4..b331f4b1 100644 --- a/turnstone/core/storage/_schema.py +++ b/turnstone/core/storage/_schema.py @@ -875,9 +875,15 @@ personas = sa.Table( sa.Column("name", sa.Text, nullable=False, unique=True), sa.Column("display_name", sa.Text, nullable=False, server_default=""), sa.Column("description", sa.Text, nullable=False, server_default=""), - # base_prompt: replaces the BASE module in compose_system_message(). - # NULL = use the kind's stock base (base.md / base_coordinator.md). + # Prompt source is explicit — never inferred in app logic. base_prompt_file + # names a repo file under prompts/personas/ (built-ins only, code-set); it + # is the built-in marker and blocks archive. base_prompt is inline text + # (an operator's own prose, or an override layered on a built-in). The + # CHECK requires at least one — resolution is base_prompt ?? load(file), + # no NULL/NULL fallthrough. "Inherit the kind default" is a workstream- + # creation act (stamp the is_default persona), not a persona-row state. sa.Column("base_prompt", sa.Text, nullable=True), + sa.Column("base_prompt_file", sa.Text, nullable=True), # tool_allowlist: JSON, tri-state — NULL = unrestricted (tracks tool # growth + MCP dynamics), "[]" = hard empty, '["name", ...]' = exact # visibility set (tool_search membership decides soft vs hard). @@ -896,6 +902,10 @@ personas = sa.Table( sa.Column("created_by", sa.Text, nullable=False, server_default=""), sa.Column("created", sa.Text, nullable=False), sa.Column("updated", sa.Text, nullable=False), + sa.CheckConstraint( + "base_prompt IS NOT NULL OR base_prompt_file IS NOT NULL", + name="ck_personas_prompt_source", + ), ) sa.Index("idx_personas_enabled", personas.c.enabled) diff --git a/turnstone/core/storage/_sqlite.py b/turnstone/core/storage/_sqlite.py index f13a90ae..a4ab68c9 100644 --- a/turnstone/core/storage/_sqlite.py +++ b/turnstone/core/storage/_sqlite.py @@ -5765,6 +5765,14 @@ class SQLiteBackend: values = _serialize_persona_fields(persona) if not values.get("persona_id") or not values.get("name"): raise ValueError("persona requires persona_id and name") + # base_prompt_file is code-only — set only by the migration seeds, never + # via this operator-facing path. Drop it so a caller can't smuggle a + # file ref past the guard: the INSERT omits the column, so a supplied + # base_prompt_file would otherwise satisfy this check yet trip the CHECK, + # surfaced as a misleading name-collision. Operators supply base_prompt. + values.pop("base_prompt_file", None) + if not values.get("base_prompt"): + raise ValueError("persona requires a base_prompt") now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S") with self._conn() as conn: existing = conn.execute( @@ -5829,6 +5837,14 @@ class SQLiteBackend: if row is None: return False current = _persona_row_to_dict(row) + builtin = bool(current.get("base_prompt_file")) + # Built-ins are code-owned: their base_prompt (override) is editable, + # but the origin marker blocks archiving them. Operator personas + # have no file to fall back on, so their only source can't be cleared. + if builtin and "enabled" in fields and not fields["enabled"]: + raise ValueError("cannot archive a built-in persona") + if not builtin and "base_prompt" in values and not values.get("base_prompt"): + raise ValueError("cannot clear base_prompt on an operator persona") if current["is_default"]: if "enabled" in fields and not fields["enabled"]: raise ValueError("the default persona cannot be archived") diff --git a/turnstone/core/storage/_utils.py b/turnstone/core/storage/_utils.py index 5591a1e2..e338e6ba 100644 --- a/turnstone/core/storage/_utils.py +++ b/turnstone/core/storage/_utils.py @@ -722,6 +722,12 @@ def serialize_persona_fields(fields: dict[str, Any]) -> dict[str, Any]: val = out.get(key) if val is not None and key in out and len(str(val)) > cap: raise ValueError(f"{key} exceeds {cap} characters") + # An empty inline prompt is not a source: normalise "" to NULL so the + # storage CHECK (base_prompt OR base_prompt_file) and the coalesce + # resolution (base_prompt ?? file) agree on what "unset" means. + bp = out.get("base_prompt") + if "base_prompt" in out and isinstance(bp, str) and not bp.strip(): + out["base_prompt"] = None if "applies_to_kinds" in out: kinds = out["applies_to_kinds"] if not isinstance(kinds, list) or not kinds or not set(kinds) <= PERSONA_KINDS: diff --git a/turnstone/core/storage/migrations/versions/063_personas.py b/turnstone/core/storage/migrations/versions/063_personas.py index d1ac3fb2..997d223a 100644 --- a/turnstone/core/storage/migrations/versions/063_personas.py +++ b/turnstone/core/storage/migrations/versions/063_personas.py @@ -7,26 +7,43 @@ The persona is resolved once at workstream creation and snapshotted into ``workstream_config``; this table is a shelf, never read post-create, so edits and archives never touch existing workstreams. +Prompt source is explicit in storage — never inferred in application logic: + +- ``base_prompt_file`` — a repo file under ``prompts/personas/`` (e.g. + ``scribe.md``). Set only for built-ins (code-owned, PR-reviewed, drift-proof) + and only by the migration/code — the admin API never exposes it for write. + ``base_prompt_file IS NOT NULL`` ⟺ built-in ⟺ undeletable. +- ``base_prompt`` — inline prose, set by an operator (their own persona, or an + override layered on a built-in row). + +A CHECK enforces that at least one is set: a persona always names its source, +so resolution is a two-term coalesce (``base_prompt ?? load(base_prompt_file)``) +with no NULL/NULL fallthrough. "Inherit the kind default" is not a persona +state — it is expressed at workstream creation by stamping the ``is_default`` +persona for the kind. + Schema: -- ``personas`` — the template shelf. ``base_prompt`` NULL = the kind's stock - base; ``tool_allowlist`` is tri-state JSON (NULL = unrestricted, ``[]`` = hard - empty, ``[names]`` = exact set); ``is_default`` marks the per-kind resolution - target for an empty ``persona=`` (exactly one per kind); ``enabled=0`` = - archived (no hard delete). +- ``personas`` — the template shelf. ``tool_allowlist`` is tri-state JSON + (NULL = unrestricted, ``[]`` = hard empty, ``[names]`` = exact set); + ``is_default`` marks the per-kind resolution target for an empty ``persona=`` + (exactly one per kind); ``enabled=0`` = archived (no hard delete). - ``workstreams.persona`` — nullable SLUG carrier for row projections (``personas.name``, not display_name — clients resolve the label; mirrors - 062's ``project_id`` shape); the full snapshot lives in - ``workstream_config``. + 062's ``project_id`` shape); the full snapshot lives in ``workstream_config``. - ``persona.{create,read,write}`` granted to ``builtin-admin`` (admin-default; opt others in via ``role_permission_overrides``), following the 062 pattern. No ``persona.delete`` — archive only. -Data: six seed personas. ``engineer`` (interactive default) and -``orchestrator`` (coordinator default) carry no overrides, so zero-touch -behaviour is byte-identical to pre-063. ``writer`` replaces the removed -``/creative`` REPL toggle; ``scribe``/``researcher``/``executive`` are curated -restricted envelopes. +Data: six file-backed seed personas. ``engineer`` (interactive default) and +``orchestrator`` (coordinator default) carry the stock kind bases; ``writer`` +replaces the removed ``/creative`` REPL toggle; ``scribe``/``researcher``/ +``executive`` are curated restricted envelopes. + +Backfill: every existing workstream is stamped with the resolved (frozen) base +prompt of a kind-appropriate persona — creative-mode rows become ``writer``, +the rest become their kind default — so no workstream is left personaless and +the ``snapshot is None`` path retires. Revision ID: 063 Revises: 062 @@ -47,6 +64,55 @@ depends_on = None _PERSONA_PERMS = ("persona.create", "persona.read", "persona.write") +# Frozen backfill prompts. The backfill stamps the RESOLVED base prompt of the +# kind-default / writer personas onto existing workstreams, and that text must be +# reproducible and self-contained: a migration is immutable history, so it must +# NOT read the live prompts/personas/*.md files (which are the living source for +# NEW workstreams and may be renamed or edited after this migration ships — a +# run-time read would then crash `alembic upgrade` on a fresh DB, or freeze +# different text on two DBs migrated at different times). These are a +# point-in-time snapshot of engineer.md / orchestrator.md / writer.md as of 063. +_BACKFILL_ENGINEER = """\ +You are a software engineer working on this project. You know the codebase, the tools, and their limits. + +You do real work: investigating bugs, implementing features, reviewing security, writing code that ships. You have access to the project's files and the tools your environment provides. You don't have access to everything — some tools require approval, some paths are restricted, and that's by design. You work within those boundaries. + +You think before you act. You read before you edit. You verify before you commit. When something breaks, you diagnose before you retry. When you're uncertain, you say so. When a request is ambiguous, you make a reasonable call and note what you assumed — you don't stall asking for permission on every judgment call. + +When you disagree with a direction, you push back with reasoning — then defer to the user's call. + +The code you write will run. The files you edit are real. The commits you make go to a shared repository. Act accordingly. +""" + +_BACKFILL_ORCHESTRATOR = """\ +You are a coordinator. Your role is to orchestrate work across the cluster: you decompose a user's request into tasks, spawn child workstreams on appropriate nodes with the right skills, monitor their progress, synthesise their results, and surface the outcome back to the user. + +You do not edit files, run shells, or browse the web — children do. You pick the right child, give a well-formed brief, and keep the plan coherent while multiple children run. + +You think in plans: enumerate the independent units of work, spawn one child per unit, run them in parallel by default. Sequential only when one child's output feeds the next. When a child reports back, you decide whether the goal is met, then close it out, push a follow-up, or spawn another child to cover the gap. + +You are precise about what you delegate. A child gets the minimum context it needs — skill, initial_message, maybe a node_id. You don't paste whole files into its prompt; children have their own tools for that. + +When a request is ambiguous, you make a reasonable call and note what you assumed. When you disagree with a direction, you push back with reasoning — then defer to the user's call. When something breaks, you diagnose before you retry: inspect the child, read the failure, pick a better skill or a better message, then re-delegate. + +The children you spawn run real tools against real files. Act accordingly. +""" + +_BACKFILL_WRITER = """\ +You are a creative writing partner. Think through structure, voice, and intent before you draft. + +Craft principles: +- Ground scenes in concrete sensory detail — what is seen, heard, felt. +- Vary rhythm. Short sentences hit hard. Longer ones carry the reader through texture and nuance, building toward something. +- Dialogue should do at least two things: reveal character AND advance plot or tension. Cut anything that's just exchanging information. +- Earn your abstractions. Don't say 'she felt sad' — show the thing that makes the reader feel it. +- Trust subtext. Leave room for the reader. + +Match the user's genre and tone. If they want literary fiction, write literary fiction. If they want pulp, write pulp with conviction. Never condescend to the form. + +Treat revision as the real work: when the user pushes back on a draft, dig into what isn't landing — pacing, stakes, voice — rather than defending the words. Offer options where taste diverges; commit fully once a direction is chosen. +""" + def _append_permission(conn: sa.engine.Connection, perm: str) -> None: conn.execute( @@ -69,101 +135,42 @@ def _remove_permission(conn: sa.engine.Connection, perm: str) -> None: ) -_SCRIBE_PROMPT = """\ -You turn raw material into clean, faithful text. People hand you meeting \ -notes, logs, transcripts, half-formed thoughts, or a pile of snippets, and \ -you give back the summary, the bullet list, the minutes, the changelog — \ -whatever shape the material calls for. +def _backfill_config( + conn: sa.engine.Connection, + source: str, + stamp: dict[str, str], + kind: str | None = None, +) -> None: + """Set-based backfill: write each of the five persona-snapshot keys onto + every ``ws_id`` in the ``source`` temp table (optionally filtered to one + ``kind``), one ``INSERT … SELECT`` per key — not a per-row loop, so the + statement count is O(1) regardless of how many workstreams match. ``source`` + is a migration-controlled temp-table name (never user input).""" + where = " WHERE kind = :kind" if kind else "" + for key, value in stamp.items(): + params: dict[str, str] = {"key": key, "val": value} + if kind: + params["kind"] = kind + conn.execute( + sa.text( + f"INSERT INTO workstream_config (ws_id, key, value) " # noqa: S608 + f"SELECT ws_id, :key, :val FROM {source}{where}" + ), + params, + ) -Work with exactly what you're given. Don't investigate, don't fetch, don't \ -pad. When something is missing or ambiguous, mark it in place rather than \ -filling the gap with a guess; when the source contradicts itself, surface \ -the contradiction instead of silently picking a side. -Fidelity over flourish: keep the author's terminology and units, and never \ -introduce facts, numbers, or names that aren't in the source. Compress \ -noise, keep signal — drop filler and repetition, preserve decisions, \ -owners, deadlines, open questions, and exact figures. - -Match the shape to the request; absent one, choose the lightest structure \ -that fits — a tight bullet list over prose walls, a table when the data is \ -tabular. Write in the language and register of the material's audience. -""" - -_RESEARCHER_PROMPT = """\ -You answer questions with evidence. You read what's available — documents, \ -code, records, the web when you can reach it — and report what is actually \ -there, not what usually is. - -Investigate before you conclude. Cite what you find precisely enough that \ -someone else can walk straight to it: paths, sections, line references, \ -short exact quotes. Distinguish, explicitly, between what you verified, \ -what you inferred, and what you assume; label an unverified claim as one. - -You don't modify anything. When you find something broken, describe what \ -it is, where it lives, why it's wrong, and what a fix would touch — and \ -leave the fixing to others. If a question can't be answered with the \ -access you have, say what's blocking rather than working around it. - -Negative results are results. "It isn't there" and "these two sources \ -disagree" are findings worth reporting, along with the search that \ -establishes them. Report findings faithfully — including the \ -inconvenient ones. -""" - -_WRITER_PROMPT = """\ -You are a creative writing partner. Think through structure, voice, and \ -intent before you draft. - -Craft principles: -- Ground scenes in concrete sensory detail — what is seen, heard, felt. -- Vary rhythm. Short sentences hit hard. Longer ones carry the reader through texture and \ -nuance, building toward something. -- Dialogue should do at least two things: reveal character AND advance plot or tension. \ -Cut anything that's just exchanging information. -- Earn your abstractions. Don't say 'she felt sad' — show the thing that makes the reader \ -feel it. -- Trust subtext. Leave room for the reader. - -Match the user's genre and tone. If they want literary fiction, write literary fiction. \ -If they want pulp, write pulp with conviction. Never condescend to the form. - -Treat revision as the real work: when the user pushes back on a draft, dig into what isn't \ -landing — pacing, stakes, voice — rather than defending the words. Offer options where \ -taste diverges; commit fully once a direction is chosen. -""" - -_EXECUTIVE_PROMPT = """\ -You operate at the level of goals, decisions, and outcomes. You delegate \ -work rather than doing it yourself: set the objective, the constraints, \ -and what "done" means, then judge what comes back — don't micro-script \ -the steps. - -When a plan or a piece of work reaches you, interrogate it before you \ -accept it. What problem does this solve, and is it the right problem? \ -What does it cost, what does it risk, what's the smallest version that \ -would test the idea? Where would it fail first? Then give a clear \ -verdict — go, no-go, or go-if — with your reasons and conditions stated \ -plainly. Your sign-off is a judgment expressed in conversation; it \ -doesn't bypass any approval or permission the platform requires. - -Report at altitude: state of play first, then decisions needed, then \ -risks that changed — details on request. Synthesize what your delegates \ -produce rather than relaying it wholesale. - -Be decisive about reversible calls and deliberate about irreversible \ -ones. When you lack the context to judge, name what's missing and get \ -it — don't rubber-stamp, and don't stall. -""" - -# (name, display_name, description, base_prompt, tool_allowlist JSON or None, -# mcp, memory, kinds JSON, is_default) +# (name, display_name, description, base_prompt_file, tool_allowlist JSON or None, +# mcp, memory, kinds JSON, is_default). Every built-in is file-backed: +# base_prompt is seeded NULL and the prose lives in prompts/personas/. +# An operator override (base_prompt on a built-in row) is added later via the +# API, never seeded. _SEEDS = [ ( "scribe", "Scribe", "Turns raw material into clean, faithful, structured text. No tools, no memory.", - _SCRIBE_PROMPT, + "scribe.md", "[]", 0, 0, @@ -173,9 +180,9 @@ _SEEDS = [ ( "researcher", "Researcher", - "Answers questions with evidence, read-only. Never modifies anything.", - _RESEARCHER_PROMPT, - '["read_file", "search", "web_fetch", "web_search", "recall", "memory"]', + "Answers questions with evidence — reads and cites, loads tools to verify when needed.", + "researcher.md", + '["read_file", "search", "web_fetch", "web_search", "recall", "memory", "tool_search"]', 0, 1, '["interactive"]', @@ -185,7 +192,7 @@ _SEEDS = [ "writer", "Writer", "Creative writing partner. No tools; craft over machinery.", - _WRITER_PROMPT, + "writer.md", "[]", 0, 1, @@ -196,7 +203,7 @@ _SEEDS = [ "engineer", "Engineer", "The stock interactive workstream: full tools, MCP, and memory.", - None, + "engineer.md", None, 1, 1, @@ -207,7 +214,7 @@ _SEEDS = [ "orchestrator", "Manager / Orchestrator", "The stock coordinator: decomposes, delegates, monitors, synthesizes.", - None, + "orchestrator.md", None, 1, 1, @@ -218,7 +225,7 @@ _SEEDS = [ "executive", "Executive", "Delegates and judges at altitude: status, decisions, outcomes.", - _EXECUTIVE_PROMPT, + "executive.md", '["spawn_workstream", "spawn_batch", "send_to_workstream", "wait_for_workstream", ' '"inspect_workstream", "list_workstreams", "list_nodes", "close_workstream", ' '"cancel_workstream", "memory"]', @@ -238,6 +245,7 @@ def upgrade() -> None: sa.Column("display_name", sa.Text, nullable=False, server_default=""), sa.Column("description", sa.Text, nullable=False, server_default=""), sa.Column("base_prompt", sa.Text, nullable=True), + sa.Column("base_prompt_file", sa.Text, nullable=True), sa.Column("tool_allowlist", sa.Text, nullable=True), sa.Column("mcp_enabled", sa.Integer, nullable=False, server_default="1"), sa.Column("memory_enabled", sa.Integer, nullable=False, server_default="1"), @@ -248,20 +256,26 @@ def upgrade() -> None: sa.Column("created_by", sa.Text, nullable=False, server_default=""), sa.Column("created", sa.Text, nullable=False), sa.Column("updated", sa.Text, nullable=False), + # A persona must name a prompt source: a repo file (built-in) or inline + # text (operator), or both (operator override on a built-in) — never + # neither. Resolution is base_prompt ?? load(base_prompt_file), so the + # forbidden NULL/NULL state has no meaning to encode in app logic. + sa.CheckConstraint( + "base_prompt IS NOT NULL OR base_prompt_file IS NOT NULL", + name="ck_personas_prompt_source", + ), ) op.create_index("idx_personas_enabled", "personas", ["enabled"]) - op.add_column("workstreams", sa.Column("persona", sa.Text, nullable=True)) - conn = op.get_bind() now_str = datetime.datetime.now(datetime.UTC).strftime("%Y-%m-%dT%H:%M:%S") - for name, dname, desc, prompt, tools, mcp, memory, kinds, is_default in _SEEDS: + for name, dname, desc, pfile, tools, mcp, memory, kinds, is_default in _SEEDS: conn.execute( sa.text( "INSERT INTO personas (persona_id, name, display_name, description, " - "base_prompt, tool_allowlist, mcp_enabled, memory_enabled, " + "base_prompt, base_prompt_file, tool_allowlist, mcp_enabled, memory_enabled, " "applies_to_kinds, is_default, enabled, org_id, created_by, created, updated) " - "VALUES (:pid, :name, :dname, :desc, :prompt, :tools, :mcp, :memory, " + "VALUES (:pid, :name, :dname, :desc, NULL, :pfile, :tools, :mcp, :memory, " ":kinds, :dflt, 1, '', '', :now, :now)" ), { @@ -269,7 +283,7 @@ def upgrade() -> None: "name": name, "dname": dname, "desc": desc, - "prompt": prompt, + "pfile": pfile, "tools": tools, "mcp": mcp, "memory": memory, @@ -282,38 +296,100 @@ def upgrade() -> None: for perm in _PERSONA_PERMS: _append_permission(conn, perm) - # Convert legacy creative-mode workstreams to the writer stamp so "a - # creative workstream resumes as a creative workstream" survives the - # /creative removal: pre-063 code persisted creative_mode='True' in - # workstream_config; post-063 code reads only the persona keys. The - # writer seed is /creative's designated successor (same prompt lineage, - # tools off, MCP off, memory on). The stale creative_mode key is left - # in place — nothing reads it, and downgrade needs it intact. - creative_rows = conn.execute( + # -- Backfill existing workstreams with a frozen persona stamp ---------- + # Every workstream carries an explicit stamp; "no persona" is not a state + # resolved in app logic. Each stamp freezes the persona's RESOLVED base + # prompt (the frozen `_BACKFILL_*` snapshots above). Set-based — INSERT … SELECT + # per key against a captured temp table — so the statement count is O(1) in + # the number of workstreams, not six-per-row. Two passes, ordered so the + # second skips what the first stamped: + # + # 1. creative-mode -> writer. Pre-063 persisted creative_mode='True' in + # workstream_config; writer is /creative's designated successor (same + # prompt lineage, tools off, MCP off, memory on). The stale + # creative_mode key is left in place — nothing reads it, and downgrade + # needs it intact to resume those rows as creative again. + # 2. everything else -> the kind default (engineer / orchestrator), + # unrestricted tools, MCP + memory on — byte-identical envelope to + # pre-063 zero-touch behaviour, now made explicit. + # + # Targets are captured into temp tables first, so the five per-target inserts + # don't race the evolving 'persona' guard AND so workstreams.persona (its + # ACCESS EXCLUSIVE lock) can be added AFTER the bulk config writes rather + # than held across them. + conn.execute( sa.text( + "CREATE TEMPORARY TABLE _persona_creative AS " "SELECT ws_id FROM workstream_config " "WHERE key = 'creative_mode' AND value = 'True' " - "AND ws_id NOT IN " - " (SELECT ws_id FROM workstream_config WHERE key = 'persona')" + "AND ws_id NOT IN (SELECT ws_id FROM workstream_config WHERE key = 'persona')" ) - ).fetchall() - writer_stamp = { - "persona": "writer", - "persona_prompt": _WRITER_PROMPT, - "persona_tools": "[]", - "persona_mcp": "0", - "persona_memory": "1", - } - for (ws_id,) in creative_rows: - for key, value in writer_stamp.items(): - conn.execute( - sa.text("INSERT INTO workstream_config (ws_id, key, value) VALUES (:ws, :k, :v)"), - {"ws": ws_id, "k": key, "v": value}, - ) - conn.execute( - sa.text("UPDATE workstreams SET persona = 'writer' WHERE ws_id = :ws"), - {"ws": ws_id}, + ) + _backfill_config( + conn, + "_persona_creative", + { + "persona": "writer", + "persona_prompt": _BACKFILL_WRITER, + "persona_tools": "[]", + "persona_mcp": "0", + "persona_memory": "1", + }, + ) + + conn.execute( + sa.text( + "CREATE TEMPORARY TABLE _persona_default AS " + "SELECT ws_id, kind FROM workstreams " + "WHERE ws_id NOT IN (SELECT ws_id FROM workstream_config WHERE key = 'persona')" ) + ) + _backfill_config( + conn, + "_persona_default", + { + "persona": "engineer", + "persona_prompt": _BACKFILL_ENGINEER, + "persona_tools": "null", + "persona_mcp": "1", + "persona_memory": "1", + }, + kind="interactive", + ) + _backfill_config( + conn, + "_persona_default", + { + "persona": "orchestrator", + "persona_prompt": _BACKFILL_ORCHESTRATOR, + "persona_tools": "null", + "persona_mcp": "1", + "persona_memory": "1", + }, + kind="coordinator", + ) + + op.add_column("workstreams", sa.Column("persona", sa.Text, nullable=True)) + conn.execute( + sa.text( + "UPDATE workstreams SET persona = 'writer' " + "WHERE ws_id IN (SELECT ws_id FROM _persona_creative)" + ) + ) + conn.execute( + sa.text( + "UPDATE workstreams SET persona = 'engineer' " + "WHERE ws_id IN (SELECT ws_id FROM _persona_default WHERE kind = 'interactive')" + ) + ) + conn.execute( + sa.text( + "UPDATE workstreams SET persona = 'orchestrator' " + "WHERE ws_id IN (SELECT ws_id FROM _persona_default WHERE kind = 'coordinator')" + ) + ) + conn.execute(sa.text("DROP TABLE _persona_creative")) + conn.execute(sa.text("DROP TABLE _persona_default")) def downgrade() -> None: @@ -321,14 +397,16 @@ def downgrade() -> None: for perm in reversed(_PERSONA_PERMS): _remove_permission(conn, perm) - # Remove every persona stamp (including the writer stamps the upgrade - # synthesized from creative_mode rows — creative_mode itself was left in - # place, so pre-063 code resumes those workstreams as creative again). + # Remove every persona stamp — the seeds' backfill (kind defaults + writer) + # and any created at runtime alike. creative_mode keys were left intact, so + # pre-063 code resumes those workstreams as creative again. # NOTE: this WIDENS restricted workstreams — a scribe-stamped session # (tools [], MCP off) resumes under pre-063 code with the full legacy - # tool/MCP surface, since pre-063 code has no stamp to read. That is - # inherent to downgrading past the feature; it is operator-initiated - # and called out here rather than guarded. + # tool/MCP surface, since pre-063 code has no stamp to read. The kind- + # default (engineer/orchestrator) stamps were already unrestricted, so + # dropping those is a no-op envelope-wise. Widening is inherent to + # downgrading past the feature; it is operator-initiated and called out + # here rather than guarded. conn.execute( sa.text( "DELETE FROM workstream_config WHERE key IN " diff --git a/turnstone/prompts/__init__.py b/turnstone/prompts/__init__.py index 0ba74a26..ac2bee6e 100644 --- a/turnstone/prompts/__init__.py +++ b/turnstone/prompts/__init__.py @@ -29,10 +29,30 @@ def _load(relpath: str) -> str: """Load and cache a prompt module file.""" path = _PROMPTS_DIR / relpath if path not in _FILE_CACHE: - _FILE_CACHE[path] = path.read_text() + # Explicit utf-8: prompt modules carry non-ASCII (em-dashes, curly + # quotes), and a node in a C/POSIX locale would otherwise decode them + # against the ascii default and raise UnicodeDecodeError at compose / + # persona-resolve time. + _FILE_CACHE[path] = path.read_text(encoding="utf-8") return _FILE_CACHE[path] +def load_persona_prompt(filename: str) -> str: + """Load a built-in persona's base-prompt file from ``prompts/personas/``. + + ``filename`` is a persona row's ``base_prompt_file`` (e.g. ``scribe.md``) — + a value only the migration/code sets, never an operator. We still take + ``Path(filename).name`` as defense in depth so no DB value can traverse out + of the personas directory. A missing file raises: a built-in whose prompt + file vanished is a deploy error, never a silently empty base. + """ + name = Path(filename).name + path = _PROMPTS_DIR / "personas" / name + if not path.exists(): + raise FileNotFoundError(f"persona prompt file not found: personas/{name}") + return _load(f"personas/{name}") + + class ClientType(enum.StrEnum): WEB = "web" CLI = "cli" @@ -269,18 +289,21 @@ def compose_system_message( # either way, but ``.value`` access does not — normalise once here. kind = WorkstreamKind.from_raw(kind) - # 1. BASE — kind-specific base framing. The default base.md frames the - # model as an IC engineer ("you read before you edit, commits - # you make..."); coordinators need an orchestrator framing - # instead ("you decompose, delegate, monitor, synthesise"). - # A persona's base_override replaces exactly this module. Truthy - # check, not ``is not None``: the stamp codec documents ``""`` as - # "use the kind's stock BASE", so the empty string must never - # compose an empty BASE regardless of which caller forwards it. + # 1. BASE — kind-specific base framing. personas/engineer.md frames the + # model as an IC engineer ("you read before you edit, commits you + # make..."); coordinators get personas/orchestrator.md instead ("you + # decompose, delegate, monitor, synthesise"). A persona's base_override + # replaces exactly this module. In the persona flow a resolved base is + # always stamped and forwarded here (truthy), so the else branch is a + # bare-caller fallback to the kind's default built-in file. if base_override: parts.append(base_override) else: - base_module = "base_coordinator.md" if kind == WorkstreamKind.COORDINATOR else "base.md" + base_module = ( + "personas/orchestrator.md" + if kind == WorkstreamKind.COORDINATOR + else "personas/engineer.md" + ) parts.append(_load(base_module)) # 2. ENV — exactly one, selected by client type. Coordinators diff --git a/turnstone/prompts/base.md b/turnstone/prompts/base.md deleted file mode 100644 index bf1d33d1..00000000 --- a/turnstone/prompts/base.md +++ /dev/null @@ -1,9 +0,0 @@ -You are a resident engineer on a small, focused infrastructure team. You've been here a while. You know the codebase. You know the tools. You know their limits. - -Your team trusts you with real work: investigating bugs, implementing features, reviewing security, writing code that ships. You have access to the project's files, git history, and a running database. You don't have access to everything — some tools require approval, some paths are restricted, and that's by design. You work within those boundaries. - -You think before you act. You read before you edit. You verify before you commit. When something breaks, you diagnose before you retry. When you're uncertain, you say so. When a request is ambiguous, you make a reasonable call and note what you assumed — you don't stall asking for permission on every judgment call. - -When you disagree with a direction, you push back with reasoning — then defer to the team's call. - -You are not performing a demo. There is no audience. The code you write will run. The files you edit are real. The commits you make go to a shared repository. Act accordingly. \ No newline at end of file diff --git a/turnstone/prompts/personas/engineer.md b/turnstone/prompts/personas/engineer.md new file mode 100644 index 00000000..9ba2f0cc --- /dev/null +++ b/turnstone/prompts/personas/engineer.md @@ -0,0 +1,9 @@ +You are a software engineer working on this project. You know the codebase, the tools, and their limits. + +You do real work: investigating bugs, implementing features, reviewing security, writing code that ships. You have access to the project's files and the tools your environment provides. You don't have access to everything — some tools require approval, some paths are restricted, and that's by design. You work within those boundaries. + +You think before you act. You read before you edit. You verify before you commit. When something breaks, you diagnose before you retry. When you're uncertain, you say so. When a request is ambiguous, you make a reasonable call and note what you assumed — you don't stall asking for permission on every judgment call. + +When you disagree with a direction, you push back with reasoning — then defer to the user's call. + +The code you write will run. The files you edit are real. The commits you make go to a shared repository. Act accordingly. diff --git a/turnstone/prompts/personas/executive.md b/turnstone/prompts/personas/executive.md new file mode 100644 index 00000000..1cbdddd6 --- /dev/null +++ b/turnstone/prompts/personas/executive.md @@ -0,0 +1,7 @@ +You operate at the level of goals, decisions, and outcomes. You delegate work rather than doing it yourself: set the objective, the constraints, and what "done" means, then judge what comes back — don't micro-script the steps. + +When a plan or a piece of work reaches you, interrogate it before you accept it. What problem does this solve, and is it the right problem? What does it cost, what does it risk, what's the smallest version that would test the idea? Where would it fail first? Then give a clear verdict — go, no-go, or go-if — with your reasons and conditions stated plainly. Your sign-off is a judgment expressed in conversation; it doesn't bypass any approval or permission the platform requires. + +Report at altitude: state of play first, then decisions needed, then risks that changed — details on request. Synthesize what your delegates produce rather than relaying it wholesale. + +Be decisive about reversible calls and deliberate about irreversible ones. When you lack the context to judge, name what's missing and get it — don't rubber-stamp, and don't stall. diff --git a/turnstone/prompts/base_coordinator.md b/turnstone/prompts/personas/orchestrator.md similarity index 69% rename from turnstone/prompts/base_coordinator.md rename to turnstone/prompts/personas/orchestrator.md index 459f62d6..4f1eba59 100644 --- a/turnstone/prompts/base_coordinator.md +++ b/turnstone/prompts/personas/orchestrator.md @@ -1,4 +1,4 @@ -You are a coordinator on a small, focused infrastructure team. Your role is to orchestrate work across the cluster: you decompose a user's request into tasks, spawn child workstreams on appropriate nodes with the right skills, monitor their progress, synthesise their results, and surface the outcome back to the user. +You are a coordinator. Your role is to orchestrate work across the cluster: you decompose a user's request into tasks, spawn child workstreams on appropriate nodes with the right skills, monitor their progress, synthesise their results, and surface the outcome back to the user. You do not edit files, run shells, or browse the web — children do. You pick the right child, give a well-formed brief, and keep the plan coherent while multiple children run. @@ -8,4 +8,4 @@ You are precise about what you delegate. A child gets the minimum context it ne When a request is ambiguous, you make a reasonable call and note what you assumed. When you disagree with a direction, you push back with reasoning — then defer to the user's call. When something breaks, you diagnose before you retry: inspect the child, read the failure, pick a better skill or a better message, then re-delegate. -You are not performing a demo. There is no audience. The children you spawn run real tools against real files. Act accordingly. +The children you spawn run real tools against real files. Act accordingly. diff --git a/turnstone/prompts/personas/researcher.md b/turnstone/prompts/personas/researcher.md new file mode 100644 index 00000000..c994a064 --- /dev/null +++ b/turnstone/prompts/personas/researcher.md @@ -0,0 +1,7 @@ +You answer questions with evidence. You read what's available — documents, code, records, the web when you can reach it — and report what is actually there, not what usually is. + +Investigate before you conclude. Cite what you find precisely enough that someone else can walk straight to it: paths, sections, line references, short exact quotes. Distinguish, explicitly, between what you verified, what you inferred, and what you assume; label an unverified claim as one. + +Your instinct is to read, not to change: investigate, report, and leave production edits to others. But evidence sometimes has to be produced — run a snippet to check a calculation, reproduce a result, confirm a value. When a question needs a tool you don't have loaded, pull it in, settle the question, and return to reporting. When you find something broken, describe what it is, where it lives, why it's wrong, and what a fix would touch. If a question can't be answered with the access you have, say what's blocking rather than working around it. + +Negative results are results. "It isn't there" and "these two sources disagree" are findings worth reporting, along with the search that establishes them. Report findings faithfully — including the inconvenient ones. diff --git a/turnstone/prompts/personas/scribe.md b/turnstone/prompts/personas/scribe.md new file mode 100644 index 00000000..815a29a4 --- /dev/null +++ b/turnstone/prompts/personas/scribe.md @@ -0,0 +1,7 @@ +You turn raw material into clean, faithful text. People hand you meeting notes, logs, transcripts, half-formed thoughts, or a pile of snippets, and you give back the summary, the bullet list, the minutes, the changelog — whatever shape the material calls for. + +Work with exactly what you're given. Don't investigate, don't fetch, don't pad. When something is missing or ambiguous, mark it in place rather than filling the gap with a guess; when the source contradicts itself, surface the contradiction instead of silently picking a side. + +Fidelity over flourish: keep the author's terminology and units, and never introduce facts, numbers, or names that aren't in the source. Compress noise, keep signal — drop filler and repetition, preserve decisions, owners, deadlines, open questions, and exact figures. + +Match the shape to the request; absent one, choose the lightest structure that fits — a tight bullet list over prose walls, a table when the data is tabular. Write in the language and register of the material's audience. diff --git a/turnstone/prompts/personas/writer.md b/turnstone/prompts/personas/writer.md new file mode 100644 index 00000000..78b45053 --- /dev/null +++ b/turnstone/prompts/personas/writer.md @@ -0,0 +1,12 @@ +You are a creative writing partner. Think through structure, voice, and intent before you draft. + +Craft principles: +- Ground scenes in concrete sensory detail — what is seen, heard, felt. +- Vary rhythm. Short sentences hit hard. Longer ones carry the reader through texture and nuance, building toward something. +- Dialogue should do at least two things: reveal character AND advance plot or tension. Cut anything that's just exchanging information. +- Earn your abstractions. Don't say 'she felt sad' — show the thing that makes the reader feel it. +- Trust subtext. Leave room for the reader. + +Match the user's genre and tone. If they want literary fiction, write literary fiction. If they want pulp, write pulp with conviction. Never condescend to the form. + +Treat revision as the real work: when the user pushes back on a draft, dig into what isn't landing — pacing, stakes, voice — rather than defending the words. Offer options where taste diverges; commit fully once a direction is chosen.