refactor: move bot_bottle_root/host_db_path to paths; kill the monkeypatch (#352)
Create bot_bottle/paths.py as the canonical home for the app-root path helpers (bot_bottle_root, host_db_path, HOST_DB_FILENAME) — foundational, not supervise- or db-specific. `bot_bottle_root()` now honours a BOT_BOTTLE_ROOT env override. Repoint every consumer (supervise, supervise_types, db_store, queue_store, audit_store, store_manager, bottle_state, cli/supervise, docker/cleanup, orchestrator/registry) at paths; remove the definitions (and supervise's duplicate host_db_path) and the now-dead `import sys`. Add paths.py to the sidecar bundle (Dockerfile.sidecars) for the flat-import copies. Tests: replace ~12 files' monkeypatching of supervise.bot_bottle_root (and the flat/pkg/supervise_types triple-patch dance) with a single `use_bottle_root()` helper that sets BOT_BOTTLE_ROOT — every module and flat/package copy reads the same env var, so one override covers them all. Net -97 lines. Behaviour-preserving: full unit suite unchanged (only the pre-existing /bin/sleep sidecar-init errors remain). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
This commit is contained in:
@@ -6,11 +6,13 @@ import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
try:
|
||||
from .supervise_types import AuditEntry, host_db_path
|
||||
from .supervise_types import AuditEntry
|
||||
from .paths import host_db_path
|
||||
from .db_store import DbStore
|
||||
from .migrations import TableMigrations
|
||||
except ImportError:
|
||||
from supervise_types import AuditEntry, host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from supervise_types import AuditEntry # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from paths import host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from db_store import DbStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ from __future__ import annotations
|
||||
import shutil
|
||||
import subprocess
|
||||
|
||||
from ... import supervise as _supervise
|
||||
from ...paths import bot_bottle_root
|
||||
from ...log import info, warn
|
||||
from . import util as docker_mod
|
||||
from .bottle_cleanup_plan import DockerBottleCleanupPlan
|
||||
@@ -94,7 +94,7 @@ def _list_orphan_state_dirs(
|
||||
ANY backend — used so this docker-side check doesn't reap a
|
||||
running non-docker bottle's state dir (the layout is shared
|
||||
across backends)."""
|
||||
state_root = _supervise.bot_bottle_root() / "state"
|
||||
state_root = bot_bottle_root() / "state"
|
||||
if not state_root.is_dir():
|
||||
return []
|
||||
orphans: list[str] = []
|
||||
|
||||
@@ -36,7 +36,7 @@ from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
from . import supervise as _supervise
|
||||
from .paths import bot_bottle_root
|
||||
|
||||
|
||||
# Directory layout: ~/.bot-bottle/state/<identity>/...
|
||||
@@ -163,7 +163,7 @@ def read_metadata(identity: str) -> BottleMetadata | None:
|
||||
def bottle_state_dir(identity: str) -> Path:
|
||||
"""Per-bottle state directory on the host. Created lazily by the
|
||||
write helpers; readers tolerate its absence."""
|
||||
return _supervise.bot_bottle_root() / _STATE_SUBDIR / identity
|
||||
return bot_bottle_root() / _STATE_SUBDIR / identity
|
||||
|
||||
|
||||
def per_bottle_dockerfile_path(identity: str) -> Path:
|
||||
|
||||
@@ -19,7 +19,7 @@ from dataclasses import dataclass
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
|
||||
from .. import supervise as _supervise
|
||||
from ..paths import bot_bottle_root
|
||||
from ..bottle_state import read_metadata
|
||||
from ..backend.docker.egress_apply import (
|
||||
EgressApplyError,
|
||||
@@ -276,7 +276,7 @@ def _write_crash_log(exc: BaseException) -> Path:
|
||||
)
|
||||
entry = f"=== supervise crash {stamp} ===\n{body}\n"
|
||||
try:
|
||||
log_dir = _supervise.bot_bottle_root() / "logs"
|
||||
log_dir = bot_bottle_root() / "logs"
|
||||
log_dir.mkdir(parents=True, exist_ok=True)
|
||||
path = log_dir / "supervise-crash.log"
|
||||
with path.open("a", encoding="utf-8") as fh:
|
||||
|
||||
+1
-25
@@ -11,30 +11,6 @@ except ImportError:
|
||||
from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
|
||||
|
||||
# The single shared host state DB. All bot-bottle SQLite stores (supervise
|
||||
# queue, audit, the orchestrator registry) co-tenant this one file — the
|
||||
# TableMigrations schema_key namespaces each store's tables.
|
||||
HOST_DB_FILENAME = "bot-bottle.db"
|
||||
|
||||
|
||||
def host_db_path() -> Path:
|
||||
"""Path to the shared host state DB, `<bot_bottle_root>/db/bot-bottle.db`.
|
||||
|
||||
Kept in its own `db/` subdirectory (not directly under the root) so a
|
||||
backend that can only bind-mount *directories* can share this one file
|
||||
with a sidecar without exposing the root's other contents (git-gate
|
||||
keys, per-bottle state, ...).
|
||||
|
||||
Resolves `bot_bottle_root` from `supervise_types` at call time (a lazy
|
||||
import — avoids a load-time cycle, and lets a monkey-patch of
|
||||
`supervise_types.bot_bottle_root` propagate here)."""
|
||||
try:
|
||||
from . import supervise_types as _st
|
||||
except ImportError: # flat imports inside the sidecar bundle
|
||||
import supervise_types as _st # type: ignore[import-not-found,no-redef] # pylint: disable=import-error,no-name-in-module
|
||||
return _st.bot_bottle_root() / "db" / HOST_DB_FILENAME
|
||||
|
||||
|
||||
class DbVersionError(Exception):
|
||||
"""Raised when the on-disk schema is behind the current migration list."""
|
||||
|
||||
@@ -80,4 +56,4 @@ class DbStore:
|
||||
pass
|
||||
|
||||
|
||||
__all__ = ["DbStore", "DbVersionError", "HOST_DB_FILENAME", "host_db_path"]
|
||||
__all__ = ["DbStore", "DbVersionError"]
|
||||
|
||||
@@ -35,8 +35,9 @@ import time
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
from ..db_store import DbStore, host_db_path
|
||||
from ..db_store import DbStore
|
||||
from ..migrations import TableMigrations
|
||||
from ..paths import host_db_path
|
||||
|
||||
# 256 bits of urandom, URL-safe — unguessable per-bottle identity token.
|
||||
IDENTITY_TOKEN_BYTES = 32
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
"""Foundational filesystem paths for bot-bottle.
|
||||
|
||||
`bot_bottle_root()` is the app data root — state, queue, audit logs,
|
||||
git-gate keys, and the shared DB all live under it. It defaults to
|
||||
`~/.bot-bottle` and is overridable with the **`BOT_BOTTLE_ROOT`** env var.
|
||||
|
||||
The env override is the single knob for redirecting the root: the test
|
||||
suite points it at a throwaway dir instead of monkey-patching the function
|
||||
(every module and every flat/package copy reads the same env var, so one
|
||||
override covers them all), and operators can relocate the root if needed.
|
||||
|
||||
This module has no bot-bottle imports, so it is safe to import from any
|
||||
layer (and to COPY flat into the sidecar bundle).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
# The single shared host state DB. All bot-bottle SQLite stores (supervise
|
||||
# queue, audit, the orchestrator registry) co-tenant this one file — the
|
||||
# TableMigrations schema_key namespaces each store's tables.
|
||||
HOST_DB_FILENAME = "bot-bottle.db"
|
||||
|
||||
|
||||
def bot_bottle_root() -> Path:
|
||||
"""The app data root — `$BOT_BOTTLE_ROOT` if set, else `~/.bot-bottle`."""
|
||||
override = os.environ.get("BOT_BOTTLE_ROOT")
|
||||
return Path(override) if override else Path.home() / ".bot-bottle"
|
||||
|
||||
|
||||
def host_db_path() -> Path:
|
||||
"""Path to the shared host state DB, `<root>/db/bot-bottle.db`.
|
||||
|
||||
Kept in its own `db/` subdirectory (not directly under the root) so a
|
||||
backend that can only bind-mount *directories* can share this one file
|
||||
with a sidecar without exposing the root's other contents (git-gate
|
||||
keys, per-bottle state, ...)."""
|
||||
return bot_bottle_root() / "db" / HOST_DB_FILENAME
|
||||
|
||||
|
||||
__all__ = ["HOST_DB_FILENAME", "bot_bottle_root", "host_db_path"]
|
||||
@@ -7,11 +7,13 @@ import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
try:
|
||||
from .supervise_types import Proposal, Response, host_db_path
|
||||
from .supervise_types import Proposal, Response
|
||||
from .paths import host_db_path
|
||||
from .db_store import DbStore
|
||||
from .migrations import TableMigrations
|
||||
except ImportError:
|
||||
from supervise_types import Proposal, Response, host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from supervise_types import Proposal, Response # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from paths import host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from db_store import DbStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
|
||||
|
||||
@@ -23,13 +23,10 @@ class StoreManager:
|
||||
|
||||
def __init__(self, db_path: Path | None = None) -> None:
|
||||
if db_path is None:
|
||||
# Lazy import to avoid a circular dependency: supervise imports
|
||||
# StoreManager at module level; StoreManager must not import
|
||||
# supervise at module level in return.
|
||||
try:
|
||||
from .supervise import host_db_path
|
||||
from .paths import host_db_path
|
||||
except ImportError:
|
||||
from supervise import host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
from paths import host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||
db_path = host_db_path()
|
||||
self.db_path = db_path
|
||||
|
||||
|
||||
+6
-14
@@ -41,7 +41,6 @@ try:
|
||||
from .supervise_types import (
|
||||
ACTION_OPERATOR_EDIT,
|
||||
AuditEntry,
|
||||
HOST_DB_FILENAME,
|
||||
Proposal,
|
||||
Response,
|
||||
STATUSES,
|
||||
@@ -54,13 +53,11 @@ try:
|
||||
TOOL_EGRESS_TOKEN_ALLOW,
|
||||
TOOL_GITLEAKS_ALLOW,
|
||||
TOOL_LIST_EGRESS_ROUTES,
|
||||
bot_bottle_root,
|
||||
)
|
||||
except ImportError:
|
||||
from supervise_types import ( # type: ignore[import-not-found,no-redef] # pylint: disable=import-error,no-name-in-module
|
||||
ACTION_OPERATOR_EDIT,
|
||||
AuditEntry,
|
||||
HOST_DB_FILENAME,
|
||||
Proposal,
|
||||
Response,
|
||||
STATUSES,
|
||||
@@ -73,10 +70,15 @@ except ImportError:
|
||||
TOOL_EGRESS_TOKEN_ALLOW,
|
||||
TOOL_GITLEAKS_ALLOW,
|
||||
TOOL_LIST_EGRESS_ROUTES,
|
||||
bot_bottle_root,
|
||||
)
|
||||
|
||||
|
||||
try:
|
||||
from .paths import bot_bottle_root, host_db_path
|
||||
except ImportError: # flat imports inside the sidecar bundle
|
||||
from paths import bot_bottle_root, host_db_path # type: ignore[import-not-found,no-redef] # pylint: disable=import-error,no-name-in-module
|
||||
|
||||
|
||||
SUPERVISE_HOSTNAME = "supervise"
|
||||
SUPERVISE_PORT = 9100
|
||||
|
||||
@@ -106,16 +108,6 @@ def audit_dir() -> Path:
|
||||
return bot_bottle_root() / "audit"
|
||||
|
||||
|
||||
def host_db_path() -> Path:
|
||||
# Calls bot_bottle_root() through this module's globals so that patches
|
||||
# on supervise.bot_bottle_root propagate to callers going through
|
||||
# supervise.host_db_path().
|
||||
#
|
||||
# Kept in its own "db" subdirectory (see supervise_types.host_db_path
|
||||
# for why) — must stay in sync with that copy.
|
||||
return bot_bottle_root() / "db" / HOST_DB_FILENAME
|
||||
|
||||
|
||||
def audit_log_path(component: str, slug: str) -> Path:
|
||||
return audit_dir() / f"{component}-{slug}.log"
|
||||
|
||||
|
||||
@@ -1,14 +1,11 @@
|
||||
"""Shared types and path helpers for supervise stores (PRD 0013).
|
||||
"""Shared types for supervise stores (PRD 0013).
|
||||
|
||||
Extracted from supervise.py so queue_store and audit_store can import
|
||||
Proposal, Response, AuditEntry, and host_db_path without creating a
|
||||
circular import (supervise imports from queue_store/audit_store and
|
||||
vice-versa).
|
||||
Proposal, Response, and AuditEntry without creating a circular import
|
||||
(supervise imports from queue_store/audit_store and vice-versa).
|
||||
|
||||
host_db_path / HOST_DB_FILENAME now live in db_store (shared DB infra) and
|
||||
are re-exported here for the historical import path. db_store.host_db_path
|
||||
resolves bot_bottle_root from this module lazily, so patching
|
||||
supervise_types.bot_bottle_root still propagates to the DB path.
|
||||
The path helpers (`bot_bottle_root`, `host_db_path`, `HOST_DB_FILENAME`)
|
||||
live in `paths` — import them from there.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -17,16 +14,6 @@ import dataclasses
|
||||
import uuid
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
|
||||
# host_db_path / HOST_DB_FILENAME now live in db_store (shared DB infra);
|
||||
# re-exported here for the historical import path. db_store resolves
|
||||
# bot_bottle_root from this module lazily, so patching
|
||||
# supervise_types.bot_bottle_root still propagates.
|
||||
try:
|
||||
from .db_store import HOST_DB_FILENAME, host_db_path
|
||||
except ImportError: # flat imports inside the sidecar bundle
|
||||
from db_store import HOST_DB_FILENAME, host_db_path # type: ignore[import-not-found,no-redef] # pylint: disable=import-error,no-name-in-module
|
||||
|
||||
TOOL_EGRESS_BLOCK = "egress-block"
|
||||
TOOL_EGRESS_ALLOW = "egress-allow"
|
||||
@@ -49,10 +36,6 @@ STATUSES: tuple[str, ...] = (STATUS_APPROVED, STATUS_MODIFIED, STATUS_REJECTED)
|
||||
ACTION_OPERATOR_EDIT = "operator-edit"
|
||||
|
||||
|
||||
def bot_bottle_root() -> Path:
|
||||
return Path.home() / ".bot-bottle"
|
||||
|
||||
|
||||
def _require_str(raw: dict[str, object], key: str) -> str:
|
||||
value = raw.get(key)
|
||||
if not isinstance(value, str):
|
||||
@@ -164,7 +147,6 @@ class AuditEntry:
|
||||
__all__ = [
|
||||
"ACTION_OPERATOR_EDIT",
|
||||
"AuditEntry",
|
||||
"HOST_DB_FILENAME",
|
||||
"Proposal",
|
||||
"Response",
|
||||
"STATUSES",
|
||||
@@ -177,6 +159,4 @@ __all__ = [
|
||||
"TOOL_EGRESS_TOKEN_ALLOW",
|
||||
"TOOL_GITLEAKS_ALLOW",
|
||||
"TOOL_LIST_EGRESS_ROUTES",
|
||||
"bot_bottle_root",
|
||||
"host_db_path",
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user