299 lines
13 KiB
Python
299 lines
13 KiB
Python
"""Architecture-conformance checks for the control plane and for
|
|
skill-driven projects (software-engineering skill, G2 frozen artefacts).
|
|
|
|
Zero third-party dependencies; standard library only. This module
|
|
imports no ``cloud.*`` module, so it can never violate the layer bans
|
|
it enforces, and it can be reused verbatim against a skill project's
|
|
own directory tree (see ``check_skill_project``).
|
|
|
|
The four checks, each with a stated rule and a stated reason:
|
|
|
|
1. Contract version (``check_contract_version``). Rule mirrors the
|
|
skill's contract-freeze change protocol: after freeze, a version
|
|
change is allowed only with a coordinator-approved change record
|
|
that names backward compatibility, the affected clients, and (for
|
|
a breaking change) a regression task per affected client; the
|
|
version must advance by exactly one. Reason: a silent contract
|
|
drift is how parallel client threads diverge; the protocol is the
|
|
only sanctioned path, so the check enforces the protocol instead
|
|
of merely documenting it.
|
|
|
|
2. Forbidden directories (``is_within`` / ``check_task_writes``).
|
|
Rule reuses the two directory definitions that already exist and
|
|
invents no third one: on the control plane a task's own directory
|
|
is its tenant work directory (``worker._tenant_dirs`` ``work``);
|
|
in a skill project a client thread's own directory is its
|
|
``apps/<client>/`` tree. Any write outside the own directory --
|
|
including ``..`` escapes and symlink escapes, both resolved before
|
|
comparison -- is a violation. Reason: cross-directory writes are
|
|
how one tenant/client corrupts another's worktree.
|
|
|
|
3. Forbidden imports (``check_layer_imports``). Bans were chosen by
|
|
auditing the current tree, not by wishful layering; every ban
|
|
below passes on the baseline, and the repo-level test pins that:
|
|
|
|
- Foundation modules (``db``, ``models``, ``queue``,
|
|
``logging_config``) must not import orchestration/service
|
|
modules (``server``, ``worker``, ``pr_exec``, ``webhooks``,
|
|
``autoscaler``, ``cost``). Reason: the foundation is imported
|
|
by everything; an upward import is a cycle waiting to happen
|
|
and drags service state into pure helpers.
|
|
- The execution module (``sandbox``) must not import persistence
|
|
or orchestration (``db``, ``server``, ``worker``, ``cost``,
|
|
``auth``, ``pr``, ``pr_exec``, ``webhooks``). Reason: executors
|
|
must stay swappable and testable with fake runners; decisions
|
|
about mounts and networks arrive as arguments, not by reading
|
|
the database.
|
|
- No cloud module except the driver-adjacent ones (``db``,
|
|
``backup``, ``migrations``) may open a database connection
|
|
directly (``sqlite3.connect`` or any ``psycopg*`` import).
|
|
Service modules must obtain connections through
|
|
``cloud.db.connect`` or a Store. Reason: the backend switch
|
|
(sqlite/postgres) lives in exactly one place; a direct driver
|
|
call bypasses it. Deliberately NOT banned: ``server.py``'s use
|
|
of ``sqlite3.Row`` / ``sqlite3.IntegrityError`` as type adapters
|
|
on connections obtained from ``cloud.db`` -- that is adaptation,
|
|
not a bypass, and banning it would fail the baseline for no
|
|
safety gain. The test asserts the narrow form (no ``connect``
|
|
call, no driver import) rather than the broad form.
|
|
|
|
Only static ``ast`` imports are seen; dynamic imports
|
|
(``importlib``) are out of scope and are noted, not claimed.
|
|
|
|
4. Frozen stack (``check_frozen_stack``). Rule mirrors the skill's
|
|
G2 rule "frozen stack must not be swapped mid-flight": a client
|
|
whose stack entry changes, or disappears, after freeze fails
|
|
unless an approved change record covers it; adding a brand-new
|
|
client (no frozen entry) is an addition, not a swap, and passes.
|
|
Reason: the shared core is built against the frozen table; a
|
|
mid-flight swap silently invalidates it.
|
|
|
|
All checkers return plain data (dicts/lists), never raise for a
|
|
violation, so CI can report every violation in one run instead of
|
|
stopping at the first.
|
|
"""
|
|
|
|
import ast
|
|
from pathlib import Path
|
|
|
|
FOUNDATION_MODULES = frozenset({"db", "models", "queue", "logging_config"})
|
|
ORCHESTRATION_MODULES = frozenset(
|
|
{"server", "worker", "pr_exec", "webhooks", "autoscaler", "cost"})
|
|
EXECUTION_MODULES = frozenset({"sandbox"})
|
|
EXECUTION_BANNED = frozenset(
|
|
{"db", "server", "worker", "cost", "auth", "pr", "pr_exec", "webhooks"})
|
|
DRIVER_ADJACENT_MODULES = frozenset({"db", "backup", "migrations"})
|
|
|
|
|
|
# ---------------------------------------------------------------- contract
|
|
|
|
def parse_version(text):
|
|
"""Parse ``v1`` / ``1`` to int; anything else is None (unparseable
|
|
is a failure, never silently treated as the frozen version)."""
|
|
if text is None:
|
|
return None
|
|
text = str(text).strip()
|
|
if text.startswith("v"):
|
|
text = text[1:]
|
|
if text.isdigit():
|
|
return int(text)
|
|
return None
|
|
|
|
|
|
def _change_record_problems(change, breaking):
|
|
"""Missing pieces of a contract-change record, per the protocol."""
|
|
problems = []
|
|
if not isinstance(change, dict):
|
|
return ["change record missing (frozen contract changed without "
|
|
"a coordinator-approved change record)"]
|
|
if not change.get("approved"):
|
|
problems.append("change not coordinator-approved")
|
|
if change.get("backward_compatible") is None:
|
|
problems.append("backward compatibility not declared")
|
|
affected = change.get("affected_clients")
|
|
if not affected or not isinstance(affected, list):
|
|
problems.append("affected clients not listed")
|
|
if breaking and not change.get("regression_task"):
|
|
problems.append("breaking change without a regression task")
|
|
return problems
|
|
|
|
|
|
def check_contract_version(frozen, current, change=None):
|
|
"""Return ``{"ok": bool, "reasons": [...]}``.
|
|
|
|
Equal versions pass regardless of ``change``. A different version
|
|
passes only when it is exactly frozen + 1 and ``change`` satisfies
|
|
the protocol; a breaking change is one where
|
|
``backward_compatible`` is False.
|
|
"""
|
|
frozen_n = parse_version(frozen)
|
|
current_n = parse_version(current)
|
|
if frozen_n is None or current_n is None:
|
|
return {"ok": False,
|
|
"reasons": ["unparseable contract version: frozen=%r "
|
|
"current=%r" % (frozen, current)]}
|
|
if current_n == frozen_n:
|
|
return {"ok": True, "reasons": []}
|
|
reasons = []
|
|
if current_n != frozen_n + 1:
|
|
reasons.append("contract version must advance by exactly one "
|
|
"(frozen v%d -> v%d, not v%d)"
|
|
% (frozen_n, frozen_n + 1, current_n))
|
|
breaking = isinstance(change, dict) and \
|
|
change.get("backward_compatible") is False
|
|
reasons.extend(_change_record_problems(change, breaking))
|
|
return {"ok": not reasons, "reasons": reasons}
|
|
|
|
|
|
# ------------------------------------------------------------- frozen stack
|
|
|
|
def check_frozen_stack(frozen, current, change=None):
|
|
"""``frozen``/``current`` map client -> stack string.
|
|
|
|
Changed or removed entries fail without an approved change record
|
|
(same shape as the contract-change record, minus the version
|
|
rules); added clients pass as additions.
|
|
"""
|
|
frozen = frozen or {}
|
|
current = current or {}
|
|
reasons = []
|
|
swapped = sorted(c for c in frozen
|
|
if c not in current or current[c] != frozen[c])
|
|
if not swapped:
|
|
return {"ok": True, "reasons": [], "swapped": []}
|
|
problems = _change_record_problems(change, breaking=False)
|
|
if problems:
|
|
reasons.append("frozen stack changed for %s without an approved "
|
|
"change record: %s" % (swapped, "; ".join(problems)))
|
|
return {"ok": not reasons, "reasons": reasons, "swapped": swapped}
|
|
|
|
|
|
# ------------------------------------------------------------ directories
|
|
|
|
def is_within(root, candidate):
|
|
"""True iff ``candidate`` resolves inside ``root`` (symlinks and
|
|
``..`` resolved first, so both escape routes are caught)."""
|
|
try:
|
|
root_p = Path(root).resolve()
|
|
cand_p = Path(candidate).resolve()
|
|
except (OSError, RuntimeError):
|
|
return False
|
|
return cand_p == root_p or root_p in cand_p.parents
|
|
|
|
|
|
def check_task_writes(own_dir, paths):
|
|
"""Return the subset of ``paths`` written outside ``own_dir``."""
|
|
return [p for p in paths if not is_within(own_dir, p)]
|
|
|
|
|
|
# ----------------------------------------------------------------- imports
|
|
|
|
def scan_imports(source):
|
|
"""Static import scan of one module's source.
|
|
|
|
Returns ``{"cloud": set, "driver_connect": bool, "driver_import": bool}``:
|
|
``cloud`` holds the ``cloud.<name>`` top-level names imported;
|
|
``driver_connect`` is True on any ``sqlite3.connect`` call;
|
|
``driver_import`` is True on any ``psycopg*`` import. A bare
|
|
``import sqlite3`` (Row/exception adapters) sets neither flag.
|
|
"""
|
|
cloud_imports = set()
|
|
driver_connect = False
|
|
driver_import = False
|
|
try:
|
|
tree = ast.parse(source)
|
|
except SyntaxError:
|
|
return {"cloud": cloud_imports,
|
|
"driver_connect": driver_connect,
|
|
"driver_import": driver_import,
|
|
"parse_error": True}
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Import):
|
|
for alias in node.names:
|
|
if alias.name == "sqlite3":
|
|
continue
|
|
if alias.name.startswith("psycopg"):
|
|
driver_import = True
|
|
if alias.name == "cloud" or alias.name.startswith("cloud."):
|
|
parts = alias.name.split(".")
|
|
if len(parts) > 1:
|
|
cloud_imports.add(parts[1])
|
|
elif isinstance(node, ast.ImportFrom):
|
|
if node.module and node.module.startswith("psycopg"):
|
|
driver_import = True
|
|
if node.module == "cloud" or (node.module or "").startswith("cloud."):
|
|
if node.module == "cloud":
|
|
for alias in node.names:
|
|
cloud_imports.add(alias.name)
|
|
else:
|
|
cloud_imports.add(node.module.split(".")[1])
|
|
elif isinstance(node, ast.Call):
|
|
func = node.func
|
|
if isinstance(func, ast.Attribute) and func.attr == "connect":
|
|
value = func.value
|
|
if isinstance(value, ast.Name) and value.id == "sqlite3":
|
|
driver_connect = True
|
|
return {"cloud": cloud_imports,
|
|
"driver_connect": driver_connect,
|
|
"driver_import": driver_import}
|
|
|
|
|
|
def check_layer_imports(sources):
|
|
"""``sources`` maps module name -> source text. Returns a list of
|
|
violation dicts; empty means the layer rules hold."""
|
|
violations = []
|
|
for module, source in sorted(sources.items()):
|
|
scanned = scan_imports(source)
|
|
if scanned.get("parse_error"):
|
|
violations.append({"module": module, "rule": "parse",
|
|
"detail": "source does not parse; "
|
|
"imports cannot be verified"})
|
|
continue
|
|
if module in FOUNDATION_MODULES:
|
|
bad = scanned["cloud"] & ORCHESTRATION_MODULES
|
|
if bad:
|
|
violations.append({"module": module,
|
|
"rule": "foundation_must_not_import_orchestration",
|
|
"detail": sorted(bad)})
|
|
if module in EXECUTION_MODULES:
|
|
bad = scanned["cloud"] & EXECUTION_BANNED
|
|
if bad:
|
|
violations.append({"module": module,
|
|
"rule": "execution_must_not_import_persistence_or_orchestration",
|
|
"detail": sorted(bad)})
|
|
if module not in DRIVER_ADJACENT_MODULES:
|
|
if scanned["driver_connect"] or scanned["driver_import"]:
|
|
violations.append({"module": module,
|
|
"rule": "service_must_not_open_driver_connection_directly",
|
|
"detail": "use cloud.db.connect or a Store"})
|
|
return violations
|
|
|
|
|
|
def scan_cloud_package(repo_root):
|
|
"""Run ``check_layer_imports`` over every ``cloud/*.py`` in a repo
|
|
checkout (tests and caches excluded). This is the repo-level test
|
|
that makes a layer violation fail CI."""
|
|
cloud_dir = Path(repo_root) / "cloud"
|
|
sources = {}
|
|
if cloud_dir.is_dir():
|
|
for path in sorted(cloud_dir.glob("*.py")):
|
|
sources[path.stem] = path.read_text(encoding="utf-8")
|
|
return check_layer_imports(sources)
|
|
|
|
|
|
def check_skill_project(project_root, frozen_version, current_version,
|
|
frozen_stack, current_stack, own_dir, writes,
|
|
change=None):
|
|
"""One-call conformance for a skill project: contract version,
|
|
frozen stack, and directory writes together. Returns a dict with
|
|
one entry per check plus an overall ``ok``."""
|
|
contract = check_contract_version(frozen_version, current_version,
|
|
change=change)
|
|
stack = check_frozen_stack(frozen_stack, current_stack, change=change)
|
|
outside = check_task_writes(own_dir, writes)
|
|
directory = {"ok": not outside,
|
|
"reasons": (["writes outside own directory: %s" % outside]
|
|
if outside else [])}
|
|
return {"ok": contract["ok"] and stack["ok"] and directory["ok"],
|
|
"contract": contract, "stack": stack, "directory": directory}
|