Skip to content

Commit 459ff11

Browse files
committed
fix(config): resolve scoped config for read-only system-prompt dump
pythinker system-prompt fell back to an empty Config() whenever ~/.pythinker/config.toml was absent, silently dropping project/local scoped settings (e.g. extra_skill_dirs) and printing an inaccurate prompt. Add a persist=False mode to load_config/_load_scoped that runs the full user→project→ local merge with no disk side effects — no share-dir/lock creation, default seeding, JSON→TOML migration, or auto-gitignore, and skipping the project-trust read (trust only gates hooks, which no read-only consumer renders). The default persist=True path is byte-for-byte unchanged. Regression test covers no-user-config + project-config-present and asserts no config is seeded.
1 parent f6fdfc0 commit 459ff11

3 files changed

Lines changed: 109 additions & 17 deletions

File tree

src/pythinker_code/cli/system_prompt.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,17 @@ def system_prompt(
5555
"""
5656
from pythinker_host.path import HostPath
5757

58-
from pythinker_code.config import Config, get_config_file, load_config
58+
from pythinker_code.config import load_config
5959
from pythinker_code.soul.agent import render_agent_system_prompt
6060

6161
resolved = agent_file if agent_file is not None else _resolve_agent_file(agent)
6262
if not resolved.exists():
6363
raise typer.BadParameter(f"Agent spec not found: {resolved}")
6464

65-
config = load_config() if get_config_file(create=False).expanduser().exists() else Config()
65+
# Resolve the merged user/project/local scoped config so the dump reflects
66+
# runtime behaviour even before a user config file exists. persist=False keeps
67+
# the command read-only: no share-dir/lock creation, seeding, or auto-gitignore.
68+
config = load_config(persist=False)
6669
wd = (
6770
HostPath.unsafe_from_local_path(work_dir.resolve())
6871
if work_dir is not None

src/pythinker_code/config.py

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,26 @@ def _apply_env_vars(merged: dict[str, Any], provenance: dict[str, Any]) -> None:
337337
_set_nested(provenance, path, f"env {env_key}")
338338

339339

340-
def _load_scoped(project_root: Path | None) -> Config:
340+
def _load_scoped(project_root: Path | None, *, persist: bool = True) -> Config:
341341
"""Run the five-step scoped config resolution pipeline.
342342
343343
Steps: Ingest → Guard → Merge → Env → Validate.
344344
Returns a fully-validated Config with source_scopes populated.
345+
346+
When ``persist`` is False the pipeline is read-only: it merges the same
347+
user/project/local scopes but performs no disk writes (no share-dir/lock
348+
creation, no migration, no default seeding, no auto-gitignore) and skips the
349+
project-trust read (which itself creates the trust-lock file). Trust only
350+
gates project *hooks*, which no read-only consumer renders or executes, so
351+
skipping it leaves the merged config faithful for those callers.
345352
"""
346353
from pythinker_code.utils.gitignore import ensure_gitignored
347354

348355
# ── INGEST ────────────────────────────────────────────────────────────
349-
default_user_file = get_config_file().expanduser().resolve(strict=False)
356+
# When read-only, do not let get_config_file() create the share dir.
357+
default_user_file = get_config_file(create=persist).expanduser().resolve(strict=False)
350358
# Trigger JSON→TOML migration if needed (existing logic)
351-
if not default_user_file.exists():
359+
if persist and not default_user_file.exists():
352360
migration_error = _migrate_json_config_to_toml()
353361
if migration_error is not None:
354362
raise ConfigError(
@@ -371,7 +379,7 @@ def _read_toml(path: Path) -> dict[str, Any]:
371379
# If the user config file still doesn't exist after migration (e.g. corrupt JSON
372380
# was backed up but no TOML was written), seed it with defaults so subsequent
373381
# runs have a concrete starting point — matching the legacy single-file behaviour.
374-
if not user_file.exists():
382+
if persist and not user_file.exists():
375383
default_cfg = get_default_config()
376384
logger.debug("No config file found, creating default config: {config}", config=default_cfg)
377385
save_config(default_cfg, user_file)
@@ -384,11 +392,18 @@ def _read_toml(path: Path) -> dict[str, Any]:
384392
stripped_hook_files: list[str] = []
385393

386394
if project_root is not None:
387-
from pythinker_code.project_trust import is_project_trusted
388-
389-
project_trusted = is_project_trusted(project_root)
390395
project_file = project_root / ".pythinker" / "config.toml"
391396
local_file = project_root / ".pythinker" / "config.local.toml"
397+
if persist:
398+
from pythinker_code.project_trust import is_project_trusted
399+
400+
project_trusted = is_project_trusted(project_root)
401+
else:
402+
# Read-only callers skip the trust read — it creates the trust-lock
403+
# file under the share dir. Trust only gates project hooks, which a
404+
# read-only consumer never executes, so read every scope and leave any
405+
# hooks merged-but-inert rather than touching disk.
406+
project_trusted = True
392407
if project_trusted:
393408
project_dict = _read_toml(project_file)
394409
local_dict = _read_toml(local_file)
@@ -464,12 +479,14 @@ def _read_toml(path: Path) -> dict[str, Any]:
464479
config.source_scopes["project"] = project_file.resolve(strict=False)
465480
if local_file is not None and local_file.exists():
466481
config.source_scopes["local"] = local_file.resolve(strict=False)
467-
# Auto-gitignore local config so it is never accidentally committed
468-
ensure_gitignored(
469-
project_root, # type: ignore[arg-type]
470-
".pythinker/config.local.toml",
471-
comment="Added by pythinker",
472-
)
482+
# Auto-gitignore local config so it is never accidentally committed.
483+
# Skip this write for read-only callers.
484+
if persist:
485+
ensure_gitignored(
486+
project_root, # type: ignore[arg-type]
487+
".pythinker/config.local.toml",
488+
comment="Added by pythinker",
489+
)
473490

474491
return config
475492

@@ -1235,7 +1252,7 @@ def get_default_config() -> Config:
12351252
)
12361253

12371254

1238-
def load_config(config_file: Path | None = None) -> Config:
1255+
def load_config(config_file: Path | None = None, *, persist: bool = True) -> Config:
12391256
"""Load configuration, resolving up to three scopes when no explicit file is given.
12401257
12411258
When *config_file* is None (the default), the scoped pipeline runs:
@@ -1245,10 +1262,16 @@ def load_config(config_file: Path | None = None) -> Config:
12451262
When *config_file* is given explicitly (e.g. via --config), that single
12461263
file is loaded directly with no scope resolution — preserving the legacy
12471264
behaviour used by tests and the CLI --config flag.
1265+
1266+
Pass ``persist=False`` for read-only callers (e.g. ``pythinker system-prompt``)
1267+
that must resolve the merged scoped config without any disk side effects:
1268+
no share-dir creation, no default-config seeding, no JSON→TOML migration,
1269+
no project-trust lock, and no auto-gitignore. Only the scoped (``config_file
1270+
is None``) branch honours the flag; an explicit path is already read-or-seed.
12481271
"""
12491272
if config_file is None:
12501273
project_root = find_project_root(Path.cwd())
1251-
return _load_scoped(project_root)
1274+
return _load_scoped(project_root, persist=persist)
12521275

12531276
# ── Explicit path: legacy single-file load (unchanged) ────────────────
12541277
default_config_file = get_config_file().expanduser().resolve(strict=False)

tests/cli/test_system_prompt_cli.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
from __future__ import annotations
44

5+
from pathlib import Path
6+
from typing import Any
7+
58
import pytest
69
import typer
710

@@ -23,3 +26,66 @@ def test_resolve_builtin_role_agent() -> None:
2326
def test_resolve_unknown_agent_raises() -> None:
2427
with pytest.raises(typer.BadParameter):
2528
_resolve_agent_file("does-not-exist-xyz")
29+
30+
31+
def test_system_prompt_uses_project_config_without_user_config(
32+
tmp_path: Path,
33+
monkeypatch: pytest.MonkeyPatch,
34+
) -> None:
35+
"""Regression: system-prompt must merge project-scoped config even when no
36+
user config file exists, and must not create the user config as a side effect.
37+
38+
Old behaviour: fell back to a bare ``Config()`` when ``~/.pythinker/config.toml``
39+
was absent, so project-scoped values (e.g. ``extra_skill_dirs``) were silently
40+
dropped. Fix: calls ``load_config(persist=False)`` which runs the full
41+
user+project+local merge pipeline read-only.
42+
"""
43+
# ── project dir with a .git marker so find_project_root() resolves ──────
44+
project = tmp_path / "proj"
45+
project.mkdir()
46+
(project / ".git").mkdir()
47+
pythinker_dir = project / ".pythinker"
48+
pythinker_dir.mkdir()
49+
(pythinker_dir / "config.toml").write_text(
50+
'extra_skill_dirs = ["/tmp/proj-only-skill-dir"]\n', encoding="utf-8"
51+
)
52+
53+
# ── chdir into the project so load_config() picks up the project scope ──
54+
monkeypatch.chdir(project)
55+
56+
# ── the autouse _isolate_share_dir fixture points PYTHINKER_SHARE_DIR at
57+
# a fresh empty tmp dir, so no user config.toml exists there ──────────
58+
59+
# ── spy: replace render_agent_system_prompt with an async stub that
60+
# records the config arg. Patch the source module because the CLI
61+
# imports it lazily inside the callback. ────────────────────────────────
62+
captured: dict[str, Any] = {}
63+
64+
async def _spy_render(agent_file: Path, work_dir: Any, config: Any) -> str:
65+
captured["config"] = config
66+
return "STUBBED PROMPT"
67+
68+
monkeypatch.setattr(
69+
"pythinker_code.soul.agent.render_agent_system_prompt",
70+
_spy_render,
71+
)
72+
73+
# ── invoke the CLI callback directly (matches existing file style) ───────
74+
from pythinker_code.cli.system_prompt import system_prompt
75+
76+
system_prompt(agent_file=DEFAULT_AGENT_FILE, work_dir=project)
77+
78+
# ── the spy must have been called ────────────────────────────────────────
79+
assert "config" in captured, "render_agent_system_prompt was never called"
80+
81+
# ── project-scoped value must be present in the resolved config ──────────
82+
# On the old Config() fallback extra_skill_dirs == [] (default), so this fails.
83+
assert captured["config"].extra_skill_dirs == ["/tmp/proj-only-skill-dir"]
84+
85+
# ── project scope must appear in source_scopes (further proof of merge) ──
86+
assert "project" in captured["config"].source_scopes
87+
88+
# ── no user config.toml must have been seeded as a side effect ───────────
89+
from pythinker_code.config import get_config_file
90+
91+
assert not get_config_file(create=False).expanduser().resolve(strict=False).exists()

0 commit comments

Comments
 (0)