fix(config): harden onboarding and command menu (#70)
This commit is contained in:
+7
-10
@@ -11,6 +11,7 @@ import typer
|
|||||||
from . import __version__
|
from . import __version__
|
||||||
from .backends import EngineBackend
|
from .backends import EngineBackend
|
||||||
from .config import ConfigError, load_or_init_config, write_config
|
from .config import ConfigError, load_or_init_config, write_config
|
||||||
|
from .config_migrations import migrate_config
|
||||||
from .engines import get_backend, list_backends
|
from .engines import get_backend, list_backends
|
||||||
from .lockfile import LockError, LockHandle, acquire_lock, token_fingerprint
|
from .lockfile import LockError, LockHandle, acquire_lock, token_fingerprint
|
||||||
from .logging import get_logger, setup_logging
|
from .logging import get_logger, setup_logging
|
||||||
@@ -19,7 +20,6 @@ from .settings import (
|
|||||||
TakopiSettings,
|
TakopiSettings,
|
||||||
load_settings,
|
load_settings,
|
||||||
load_settings_if_exists,
|
load_settings_if_exists,
|
||||||
require_telegram,
|
|
||||||
validate_settings_data,
|
validate_settings_data,
|
||||||
)
|
)
|
||||||
from .transports import SetupResult, get_transport, list_transports
|
from .transports import SetupResult, get_transport, list_transports
|
||||||
@@ -54,14 +54,6 @@ def _resolve_transport_id(override: str | None) -> str:
|
|||||||
return raw.strip()
|
return raw.strip()
|
||||||
|
|
||||||
|
|
||||||
def load_and_validate_config(
|
|
||||||
path: str | Path | None = None,
|
|
||||||
) -> tuple[TakopiSettings, Path, str, int]:
|
|
||||||
settings, config_path = load_settings(path)
|
|
||||||
token, chat_id = require_telegram(settings, config_path)
|
|
||||||
return settings, config_path, token, chat_id
|
|
||||||
|
|
||||||
|
|
||||||
def acquire_config_lock(config_path: Path, token: str | None) -> LockHandle:
|
def acquire_config_lock(config_path: Path, token: str | None) -> LockHandle:
|
||||||
fingerprint = token_fingerprint(token) if token else None
|
fingerprint = token_fingerprint(token) if token else None
|
||||||
try:
|
try:
|
||||||
@@ -199,7 +191,8 @@ def _should_run_interactive() -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def _setup_needs_config(setup: SetupResult) -> bool:
|
def _setup_needs_config(setup: SetupResult) -> bool:
|
||||||
return any(issue.title == "create a config" for issue in setup.issues)
|
config_titles = {"create a config", "configure telegram"}
|
||||||
|
return any(issue.title in config_titles for issue in setup.issues)
|
||||||
|
|
||||||
|
|
||||||
def _fail_missing_config(path: Path) -> None:
|
def _fail_missing_config(path: Path) -> None:
|
||||||
@@ -361,6 +354,10 @@ def init(
|
|||||||
) -> None:
|
) -> None:
|
||||||
"""Register the current repo as a Takopi project."""
|
"""Register the current repo as a Takopi project."""
|
||||||
config, config_path = load_or_init_config()
|
config, config_path = load_or_init_config()
|
||||||
|
if config_path.exists():
|
||||||
|
applied = migrate_config(config, config_path=config_path)
|
||||||
|
if applied:
|
||||||
|
write_config(config, config_path)
|
||||||
|
|
||||||
cwd = Path.cwd()
|
cwd = Path.cwd()
|
||||||
project_path = resolve_main_worktree_root(cwd) or cwd
|
project_path = resolve_main_worktree_root(cwd) or cwd
|
||||||
|
|||||||
@@ -169,6 +169,22 @@ class TakopiSettings(BaseSettings):
|
|||||||
)
|
)
|
||||||
return raw
|
return raw
|
||||||
|
|
||||||
|
def transport_config(
|
||||||
|
self, transport_id: str, *, config_path: Path
|
||||||
|
) -> dict[str, Any]:
|
||||||
|
if transport_id == "telegram":
|
||||||
|
return self.transports.telegram.model_dump()
|
||||||
|
extra = self.transports.model_extra or {}
|
||||||
|
raw = extra.get(transport_id)
|
||||||
|
if raw is None:
|
||||||
|
return {}
|
||||||
|
if not isinstance(raw, dict):
|
||||||
|
raise ConfigError(
|
||||||
|
f"Invalid `transports.{transport_id}` in {config_path}; "
|
||||||
|
"expected a table."
|
||||||
|
)
|
||||||
|
return raw
|
||||||
|
|
||||||
def to_projects_config(
|
def to_projects_config(
|
||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
|
|||||||
@@ -296,7 +296,7 @@ def _build_bot_commands(
|
|||||||
cmd = entry.engine.lower()
|
cmd = entry.engine.lower()
|
||||||
if cmd in seen:
|
if cmd in seen:
|
||||||
continue
|
continue
|
||||||
commands.append({"command": cmd, "description": f"start {cmd}"})
|
commands.append({"command": cmd, "description": f"use agent: {cmd}"})
|
||||||
seen.add(cmd)
|
seen.add(cmd)
|
||||||
for alias, project in projects.projects.items():
|
for alias, project in projects.projects.items():
|
||||||
cmd = alias.lower()
|
cmd = alias.lower()
|
||||||
@@ -308,7 +308,7 @@ def _build_bot_commands(
|
|||||||
alias=project.alias,
|
alias=project.alias,
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
commands.append({"command": cmd, "description": f"project {cmd}"})
|
commands.append({"command": cmd, "description": f"work on: {cmd}"})
|
||||||
seen.add(cmd)
|
seen.add(cmd)
|
||||||
if "cancel" not in seen:
|
if "cancel" not in seen:
|
||||||
commands.append({"command": "cancel", "description": "cancel run"})
|
commands.append({"command": "cancel", "description": "cancel run"})
|
||||||
|
|||||||
@@ -68,8 +68,12 @@ def _display_path(path: Path) -> str:
|
|||||||
return str(path)
|
return str(path)
|
||||||
|
|
||||||
|
|
||||||
def config_issue(path: Path) -> SetupIssue:
|
_CREATE_CONFIG_TITLE = "create a config"
|
||||||
return SetupIssue("create a config", (f" {_display_path(path)}",))
|
_CONFIGURE_TELEGRAM_TITLE = "configure telegram"
|
||||||
|
|
||||||
|
|
||||||
|
def config_issue(path: Path, *, title: str) -> SetupIssue:
|
||||||
|
return SetupIssue(title, (f" {_display_path(path)}",))
|
||||||
|
|
||||||
|
|
||||||
def check_setup(
|
def check_setup(
|
||||||
@@ -91,10 +95,15 @@ def check_setup(
|
|||||||
try:
|
try:
|
||||||
require_telegram(settings, config_path)
|
require_telegram(settings, config_path)
|
||||||
except ConfigError:
|
except ConfigError:
|
||||||
issues.append(config_issue(config_path))
|
issues.append(config_issue(config_path, title=_CONFIGURE_TELEGRAM_TITLE))
|
||||||
except ConfigError:
|
except ConfigError:
|
||||||
issues.extend(backend_issues)
|
issues.extend(backend_issues)
|
||||||
issues.append(config_issue(config_path))
|
title = (
|
||||||
|
_CONFIGURE_TELEGRAM_TITLE
|
||||||
|
if config_path.exists() and config_path.is_file()
|
||||||
|
else _CREATE_CONFIG_TITLE
|
||||||
|
)
|
||||||
|
issues.append(config_issue(config_path, title=title))
|
||||||
return SetupResult(issues=issues, config_path=config_path)
|
return SetupResult(issues=issues, config_path=config_path)
|
||||||
|
|
||||||
issues.extend(backend_issues)
|
issues.extend(backend_issues)
|
||||||
@@ -413,6 +422,9 @@ def interactive_setup(*, force: bool) -> bool:
|
|||||||
return False
|
return False
|
||||||
else:
|
else:
|
||||||
console.print("no agents found on PATH. install one to continue.")
|
console.print("no agents found on PATH. install one to continue.")
|
||||||
|
save_anyway = _confirm("save config anyway?", default=False)
|
||||||
|
if not save_anyway:
|
||||||
|
return False
|
||||||
|
|
||||||
config_preview = _render_config(
|
config_preview = _render_config(
|
||||||
_mask_token(token),
|
_mask_token(token),
|
||||||
@@ -434,7 +446,20 @@ def interactive_setup(*, force: bool) -> bool:
|
|||||||
|
|
||||||
raw_config: dict[str, Any] = {}
|
raw_config: dict[str, Any] = {}
|
||||||
if config_path.exists():
|
if config_path.exists():
|
||||||
|
try:
|
||||||
raw_config = read_raw_toml(config_path)
|
raw_config = read_raw_toml(config_path)
|
||||||
|
except ConfigError as exc:
|
||||||
|
console.print(f"[yellow]warning:[/] config is malformed: {exc}")
|
||||||
|
backup = config_path.with_suffix(".toml.bak")
|
||||||
|
try:
|
||||||
|
shutil.copyfile(config_path, backup)
|
||||||
|
except OSError as copy_exc:
|
||||||
|
console.print(
|
||||||
|
f"[yellow]warning:[/] failed to back up config: {copy_exc}"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
console.print(f" backed up to {_display_path(backup)}")
|
||||||
|
raw_config = {}
|
||||||
merged = dict(raw_config)
|
merged = dict(raw_config)
|
||||||
if default_engine is not None:
|
if default_engine is not None:
|
||||||
merged["default_engine"] = default_engine
|
merged["default_engine"] = default_engine
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ from takopi.model import EngineId, ResumeToken, TakopiEvent
|
|||||||
from takopi.telegram.render import prepare_telegram
|
from takopi.telegram.render import prepare_telegram
|
||||||
from takopi.runners.codex import CodexRunner
|
from takopi.runners.codex import CodexRunner
|
||||||
from takopi.runners.mock import Advance, Emit, Raise, Return, ScriptRunner, Wait
|
from takopi.runners.mock import Advance, Emit, Raise, Return, ScriptRunner, Wait
|
||||||
|
from takopi.settings import load_settings, require_telegram
|
||||||
from takopi.transport import MessageRef, RenderedMessage, SendOptions
|
from takopi.transport import MessageRef, RenderedMessage, SendOptions
|
||||||
from tests.factories import action_completed, action_started
|
from tests.factories import action_completed, action_started
|
||||||
|
|
||||||
@@ -76,8 +77,8 @@ def _return_runner(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_load_and_validate_config_rejects_empty_token(tmp_path) -> None:
|
def test_require_telegram_rejects_empty_token(tmp_path) -> None:
|
||||||
from takopi import cli
|
from takopi.config import ConfigError
|
||||||
|
|
||||||
config_path = tmp_path / "takopi.toml"
|
config_path = tmp_path / "takopi.toml"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
@@ -86,12 +87,13 @@ def test_load_and_validate_config_rejects_empty_token(tmp_path) -> None:
|
|||||||
encoding="utf-8",
|
encoding="utf-8",
|
||||||
)
|
)
|
||||||
|
|
||||||
with pytest.raises(cli.ConfigError, match="bot token"):
|
with pytest.raises(ConfigError, match="bot token"):
|
||||||
cli.load_and_validate_config(config_path)
|
settings, _ = load_settings(config_path)
|
||||||
|
require_telegram(settings, config_path)
|
||||||
|
|
||||||
|
|
||||||
def test_load_and_validate_config_rejects_string_chat_id(tmp_path) -> None:
|
def test_load_settings_rejects_string_chat_id(tmp_path) -> None:
|
||||||
from takopi import cli
|
from takopi.config import ConfigError
|
||||||
|
|
||||||
config_path = tmp_path / "takopi.toml"
|
config_path = tmp_path / "takopi.toml"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
@@ -100,8 +102,8 @@ def test_load_and_validate_config_rejects_string_chat_id(tmp_path) -> None:
|
|||||||
encoding="utf-8",
|
encoding="utf-8",
|
||||||
)
|
)
|
||||||
|
|
||||||
with pytest.raises(cli.ConfigError, match="chat_id"):
|
with pytest.raises(ConfigError, match="chat_id"):
|
||||||
cli.load_and_validate_config(config_path)
|
load_settings(config_path)
|
||||||
|
|
||||||
|
|
||||||
def test_codex_extract_resume_finds_command() -> None:
|
def test_codex_extract_resume_finds_command() -> None:
|
||||||
|
|||||||
@@ -32,9 +32,10 @@ def test_check_setup_marks_missing_codex(monkeypatch, tmp_path: Path) -> None:
|
|||||||
assert result.ok is False
|
assert result.ok is False
|
||||||
|
|
||||||
|
|
||||||
def test_check_setup_marks_missing_config(monkeypatch) -> None:
|
def test_check_setup_marks_missing_config(monkeypatch, tmp_path: Path) -> None:
|
||||||
backend = engines.get_backend("codex")
|
backend = engines.get_backend("codex")
|
||||||
monkeypatch.setattr(onboarding.shutil, "which", lambda _name: "/usr/bin/codex")
|
monkeypatch.setattr(onboarding.shutil, "which", lambda _name: "/usr/bin/codex")
|
||||||
|
monkeypatch.setattr(onboarding, "HOME_CONFIG_PATH", tmp_path / "takopi.toml")
|
||||||
|
|
||||||
def _raise() -> None:
|
def _raise() -> None:
|
||||||
raise onboarding.ConfigError("Missing config file")
|
raise onboarding.ConfigError("Missing config file")
|
||||||
@@ -68,4 +69,4 @@ def test_check_setup_marks_invalid_chat_id(monkeypatch, tmp_path: Path) -> None:
|
|||||||
result = onboarding.check_setup(backend)
|
result = onboarding.check_setup(backend)
|
||||||
|
|
||||||
titles = {issue.title for issue in result.issues}
|
titles = {issue.title for issue in result.issues}
|
||||||
assert "create a config" in titles
|
assert "configure telegram" in titles
|
||||||
|
|||||||
@@ -148,3 +148,81 @@ def test_interactive_setup_preserves_projects(monkeypatch, tmp_path) -> None:
|
|||||||
saved = config_path.read_text(encoding="utf-8")
|
saved = config_path.read_text(encoding="utf-8")
|
||||||
assert "[projects.z80]" in saved
|
assert "[projects.z80]" in saved
|
||||||
assert 'path = "/tmp/repo"' in saved
|
assert 'path = "/tmp/repo"' in saved
|
||||||
|
|
||||||
|
|
||||||
|
def test_interactive_setup_no_agents_aborts(monkeypatch, tmp_path) -> None:
|
||||||
|
config_path = tmp_path / "takopi.toml"
|
||||||
|
monkeypatch.setattr(onboarding, "HOME_CONFIG_PATH", config_path)
|
||||||
|
|
||||||
|
backend = EngineBackend(id="codex", build_runner=lambda _cfg, _path: None)
|
||||||
|
monkeypatch.setattr(onboarding, "list_backends", lambda: [backend])
|
||||||
|
monkeypatch.setattr(onboarding.shutil, "which", lambda _cmd: None)
|
||||||
|
|
||||||
|
monkeypatch.setattr(onboarding, "_confirm", _queue_values([True, False]))
|
||||||
|
monkeypatch.setattr(
|
||||||
|
onboarding.questionary, "password", _queue(["123456789:ABCdef"])
|
||||||
|
)
|
||||||
|
|
||||||
|
def _fake_run(func, *args, **kwargs):
|
||||||
|
if func is onboarding._get_bot_info:
|
||||||
|
return {"username": "my_bot"}
|
||||||
|
if func is onboarding._wait_for_chat:
|
||||||
|
return onboarding.ChatInfo(
|
||||||
|
chat_id=123,
|
||||||
|
username="alice",
|
||||||
|
title=None,
|
||||||
|
first_name="Alice",
|
||||||
|
last_name=None,
|
||||||
|
chat_type="private",
|
||||||
|
)
|
||||||
|
if func is onboarding._send_confirmation:
|
||||||
|
return True
|
||||||
|
raise AssertionError(f"unexpected anyio.run target: {func}")
|
||||||
|
|
||||||
|
monkeypatch.setattr(onboarding.anyio, "run", _fake_run)
|
||||||
|
|
||||||
|
assert onboarding.interactive_setup(force=False) is False
|
||||||
|
assert not config_path.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_interactive_setup_recovers_from_malformed_toml(monkeypatch, tmp_path) -> None:
|
||||||
|
config_path = tmp_path / "takopi.toml"
|
||||||
|
bad_toml = 'transport = "telegram"\n[transports\n'
|
||||||
|
config_path.write_text(bad_toml, encoding="utf-8")
|
||||||
|
monkeypatch.setattr(onboarding, "HOME_CONFIG_PATH", config_path)
|
||||||
|
|
||||||
|
backend = EngineBackend(id="codex", build_runner=lambda _cfg, _path: None)
|
||||||
|
monkeypatch.setattr(onboarding, "list_backends", lambda: [backend])
|
||||||
|
monkeypatch.setattr(onboarding.shutil, "which", lambda _cmd: "/usr/bin/codex")
|
||||||
|
|
||||||
|
monkeypatch.setattr(onboarding, "_confirm", _queue_values([True, True, True]))
|
||||||
|
monkeypatch.setattr(
|
||||||
|
onboarding.questionary, "password", _queue(["123456789:ABCdef"])
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(onboarding.questionary, "select", _queue(["codex"]))
|
||||||
|
|
||||||
|
def _fake_run(func, *args, **kwargs):
|
||||||
|
if func is onboarding._get_bot_info:
|
||||||
|
return {"username": "my_bot"}
|
||||||
|
if func is onboarding._wait_for_chat:
|
||||||
|
return onboarding.ChatInfo(
|
||||||
|
chat_id=123,
|
||||||
|
username="alice",
|
||||||
|
title=None,
|
||||||
|
first_name="Alice",
|
||||||
|
last_name=None,
|
||||||
|
chat_type="private",
|
||||||
|
)
|
||||||
|
if func is onboarding._send_confirmation:
|
||||||
|
return True
|
||||||
|
raise AssertionError(f"unexpected anyio.run target: {func}")
|
||||||
|
|
||||||
|
monkeypatch.setattr(onboarding.anyio, "run", _fake_run)
|
||||||
|
|
||||||
|
assert onboarding.interactive_setup(force=True) is True
|
||||||
|
backup = config_path.with_suffix(".toml.bak")
|
||||||
|
assert backup.exists()
|
||||||
|
assert backup.read_text(encoding="utf-8") == bad_toml
|
||||||
|
saved = config_path.read_text(encoding="utf-8")
|
||||||
|
assert "[transports.telegram]" in saved
|
||||||
|
assert 'bot_token = "123456789:ABCdef"' in saved
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ from typer.testing import CliRunner
|
|||||||
|
|
||||||
from takopi import cli
|
from takopi import cli
|
||||||
from takopi.config import ConfigError
|
from takopi.config import ConfigError
|
||||||
|
from takopi.config_store import read_raw_toml
|
||||||
from takopi.settings import TakopiSettings
|
from takopi.settings import TakopiSettings
|
||||||
|
|
||||||
|
|
||||||
@@ -50,6 +51,29 @@ def test_init_writes_project(monkeypatch, tmp_path) -> None:
|
|||||||
assert 'worktree_base = "main"' in saved
|
assert 'worktree_base = "main"' in saved
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_migrates_legacy_config(monkeypatch, tmp_path) -> None:
|
||||||
|
config_path = tmp_path / "takopi.toml"
|
||||||
|
config_path.write_text('bot_token = "token"\nchat_id = 123\n', encoding="utf-8")
|
||||||
|
monkeypatch.setattr("takopi.config.HOME_CONFIG_PATH", config_path)
|
||||||
|
monkeypatch.setattr(cli, "resolve_default_base", lambda _: "main")
|
||||||
|
|
||||||
|
repo_path = tmp_path / "repo"
|
||||||
|
repo_path.mkdir()
|
||||||
|
monkeypatch.chdir(repo_path)
|
||||||
|
|
||||||
|
runner = CliRunner()
|
||||||
|
result = runner.invoke(cli.app, ["init", "z80"])
|
||||||
|
assert result.exit_code == 0
|
||||||
|
|
||||||
|
raw = read_raw_toml(config_path)
|
||||||
|
assert "bot_token" not in raw
|
||||||
|
assert "chat_id" not in raw
|
||||||
|
assert raw["transport"] == "telegram"
|
||||||
|
assert raw["transports"]["telegram"]["bot_token"] == "token"
|
||||||
|
assert raw["transports"]["telegram"]["chat_id"] == 123
|
||||||
|
assert "z80" in raw.get("projects", {})
|
||||||
|
|
||||||
|
|
||||||
def test_projects_default_engine_unknown() -> None:
|
def test_projects_default_engine_unknown() -> None:
|
||||||
config = {"projects": {"z80": {"path": "/tmp/repo", "default_engine": "nope"}}}
|
config = {"projects": {"z80": {"path": "/tmp/repo", "default_engine": "nope"}}}
|
||||||
settings = TakopiSettings.model_validate(config)
|
settings = TakopiSettings.model_validate(config)
|
||||||
|
|||||||
@@ -136,6 +136,42 @@ def test_engine_config_none_and_invalid(tmp_path: Path) -> None:
|
|||||||
settings.engine_config("codex", config_path=config_path)
|
settings.engine_config("codex", config_path=config_path)
|
||||||
|
|
||||||
|
|
||||||
|
def test_transport_config_telegram_and_extra(tmp_path: Path) -> None:
|
||||||
|
config_path = tmp_path / "takopi.toml"
|
||||||
|
settings = TakopiSettings.model_validate(
|
||||||
|
{
|
||||||
|
"transport": "telegram",
|
||||||
|
"transports": {"telegram": {"bot_token": "token", "chat_id": 123}},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
telegram = settings.transport_config("telegram", config_path=config_path)
|
||||||
|
assert telegram["bot_token"] == "token"
|
||||||
|
assert telegram["chat_id"] == 123
|
||||||
|
|
||||||
|
settings = TakopiSettings.model_validate(
|
||||||
|
{
|
||||||
|
"transport": "telegram",
|
||||||
|
"transports": {
|
||||||
|
"telegram": {"bot_token": "token", "chat_id": 123},
|
||||||
|
"discord": None,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert settings.transport_config("discord", config_path=config_path) == {}
|
||||||
|
|
||||||
|
settings = TakopiSettings.model_validate(
|
||||||
|
{
|
||||||
|
"transport": "telegram",
|
||||||
|
"transports": {
|
||||||
|
"telegram": {"bot_token": "token", "chat_id": 123},
|
||||||
|
"discord": "nope",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
)
|
||||||
|
with pytest.raises(ConfigError, match="transports.discord"):
|
||||||
|
settings.transport_config("discord", config_path=config_path)
|
||||||
|
|
||||||
|
|
||||||
def test_bot_token_none_allowed() -> None:
|
def test_bot_token_none_allowed() -> None:
|
||||||
settings = TakopiSettings.model_validate(
|
settings = TakopiSettings.model_validate(
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user