From 064d9bf554739e66e631bf1adefff346eda4d286 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Tue, 6 Oct 2026 01:12:57 +0000 Subject: [PATCH] Projects as configuration entries (M1.4, D8) - Config: `projects: {name: entry}` with repo, base_branch, workspace, build_host, push credential, labels (defaults added to every issue), secrets by ${VAR} reference with visibility (commands | harness), and egress (recorded, not enforced). Every problem in every entry is reported at once; an inline secret, a bad visibility, a repeated repo or a non-slug name is an error. - The old loop runs exactly one project (only_project) and refuses more. - The new loop is per project: each its own clone, workspace (with its commands-visible secrets, redacted on return), push credential and milestones; intake polls every project's repository; items are scoped to their project for every run, publish and PR watch. Issues are named project#14 when there is more than one project, and a bare #14 that matches two is asked about. Switching projects parks the one worked on last (all others after a boot). - A restatement that ends in a question is sent back to the driver, not posted: the e2e driver kept asking "Is this the right interpretation?", which a plain yes can't answer. Tests: config entries and errors, two-project assembly, two-project loop tests (intake, own workspaces and repos, #14 in both, do-next, label defaults, parking across projects), the property walk over two projects with a new own_repository_only property (an item using the first project's repo fails 15 of 16 seeds), and the restatement check. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- docs/Development-Plan.md | 14 +- nisse-dev.example.yaml | 28 +- nisse-dev/src/nisse_dev/agent.py | 39 +-- nisse-dev/src/nisse_dev/config.py | 319 ++++++++++++++++----- nisse-dev/src/nisse_dev/loop/agent.py | 327 +++++++++++++++------- nisse-dev/src/nisse_dev/loop/build.py | 59 ++-- nisse-dev/src/nisse_dev/loop/driver.py | 19 +- nisse-dev/src/nisse_dev/loop/items.py | 10 +- nisse-dev/tests/test_agent_build.py | 71 ++++- nisse-dev/tests/test_config.py | 158 +++++++++-- nisse-dev/tests/test_loop_labels.py | 29 ++ nisse-dev/tests/test_loop_milestones.py | 22 +- nisse-dev/tests/test_loop_projects.py | 188 +++++++++++++ nisse-dev/tests/test_loop_properties.py | 124 +++++--- tests/integration/nisse-test.yaml | 29 +- tests/integration/test_end_to_end.py | 9 +- tests/integration/test_end_to_end_loop.py | 9 +- 17 files changed, 1139 insertions(+), 315 deletions(-) create mode 100644 nisse-dev/tests/test_loop_projects.py diff --git a/docs/Development-Plan.md b/docs/Development-Plan.md index e34a1d1..f64f09d 100644 --- a/docs/Development-Plan.md +++ b/docs/Development-Plan.md @@ -39,7 +39,7 @@ Only `nisse-dev` has real code; `nisse-core` and `nisse-ops` are placeholders. | **D1–D2** Workflow | partial | On the new loop: issue intake, restatement and go-ahead (M2.1), labels (M2.3); no documents in the repo | | **D3–D4** Workspace, git | partial | Idempotent publishing; no fresh workspace (#62), `git add -A` (#80) | | **D5** Review | partial | PR conversation comments only (#69), watch stall (#70) | -| **D7–D8** Deployment, projects | none | Runs from a venv as a systemd unit on this box; one project in config | +| **D7–D8** Deployment, projects | partial | Runs from a venv as a systemd unit on this box; project entries in config (M1.4); egress not enforced | | **O**, **X** | none | No container image yet | | `specs/` | done for now | `orchestrator.qnt` (current loop), `loop.qnt` (the architecture, #91) | @@ -198,11 +198,21 @@ new loop. **M2.7 Conversation.** *Pre-approved.* Explanations, progress in prose, asking mid-work (L5, #42, #77). -**M1.4 Projects** (D8). *Pre-approved.* +**M1.4 Projects** (D8). *Pre-approved.* **Done** (this PR). - The project entry in configuration: repository, default branch, build VM, label defaults, secrets by name with their visibility, egress additions. - Acceptance: the bring-up configuration is one project entry; a second project needs only a second entry. +- `projects: {name: entry}`. The old loop runs exactly one project and refuses + more. On the new loop each project has its own clone, workspace, push credential + and milestones. Issues are named `project#14` when two projects could both have + `#14`, and a bare `#14` that matches two is asked about. Switching projects parks + the one worked on last. +- Secrets are by `${VAR}` reference only, `commands` or `harness`. `commands` + secrets reach `run_command` as environment variables, redacted from output; + `harness` secrets are held for later. Egress is recorded, not enforced (D7). +- Deferred: config reload without a restart (with M2.5's entry step if wanted); + the trust dial's configuration (M3.3). **M1.5 The agent as a container image.** *Pre-approved.* - A Dockerfile for `nisse-dev`, stateless with a volume for state; a Gitea Actions diff --git a/nisse-dev.example.yaml b/nisse-dev.example.yaml index 32e5bc3..1ae1da8 100644 --- a/nisse-dev.example.yaml +++ b/nisse-dev.example.yaml @@ -9,15 +9,26 @@ # silently doing nothing. state_db: ~/nisse-demo-workspace/state.db -workspace: /root/work/demo # the clone, on the build host -project: demo # loop: new # the new conversational loop (M2.1); default old (control.py) -build_host: # L13: every command runs here, over SSH - address: build-demo.example - key: ~/.ssh/nisse-dev # the agent's key for the build host - # user: root - # port: 22 +# D8: one entry per project. A second project is a second entry (the old loop +# runs exactly one). Changed by PR like the rest of this file. +projects: + demo: + repo: owner/demo-repo + base_branch: main + workspace: /root/work/demo # the clone, on the build host + build_host: # L13: every command runs here, over SSH + address: build-demo.example + key: ~/.ssh/nisse-dev # the agent's key for the build host + # user: root + # port: 22 + push_key: ~/.ssh/push-demo-repo # a deploy key with write access (D4) + # labels: [discuss] # added to every issue's own (D1) + # secrets: # by name; values only by reference (D7) + # DATABASE_URL: {value: "${DEMO_DATABASE_URL}", visible: commands} + # DEPLOY_TOKEN: {value: "${DEMO_DEPLOY_TOKEN}", visible: harness} + # egress: [pypi.org] # beyond the default; recorded, not yet enforced llm: # L9: LiteLLM is the only LLM destination base_url: http://litellm.example:4000 @@ -45,10 +56,7 @@ matrix: # L5 gitea: # D4, D5 url: https://gitea.example token: ${NISSE_GITEA_TOKEN} - repo: owner/demo-repo operators: [your-gitea-login] - base_branch: main - push_key: ~/.ssh/push-demo-repo # a deploy key with write access (D4) # What each knob does, and how to choose a value: docs/Tuning-Guide.md. # These are the defaults; the section can be left out entirely. The file is diff --git a/nisse-dev/src/nisse_dev/agent.py b/nisse-dev/src/nisse_dev/agent.py index 935ba72..d7550c1 100644 --- a/nisse-dev/src/nisse_dev/agent.py +++ b/nisse-dev/src/nisse_dev/agent.py @@ -14,7 +14,7 @@ from dataclasses import dataclass from nisse_dev.approvals import PlanBoard from nisse_dev.cancel import Cancel from nisse_dev.catalog import ModelCatalog, select_client -from nisse_dev.config import AgentConfig +from nisse_dev.config import AgentConfig, Project from nisse_dev.context import BudgetedClient from nisse_dev.control import Orchestrator from nisse_dev.delegation import Caps, Delegation @@ -165,30 +165,34 @@ def make_llm_clients( return normalized, passthrough -def build_host(config: AgentConfig) -> SSHHost: - """The tool VM the workspace lives on (L13). Its known-hosts file and - connection socket live beside the state store.""" +def build_host(config: AgentConfig, project: Project | None = None) -> SSHHost: + """A project's build VM (L13; the only project's, if none is named). Its + known-hosts file and connection socket live beside the state store.""" + host = (project or config.only_project()).build_host return SSHHost( - config.build_host.address, - key=config.build_host.key, - user=config.build_host.user, - port=config.build_host.port, + host.address, + key=host.key, + user=host.user, + port=host.port, state_dir=config.state_db.parent / "ssh", ) -def push_credential(config: AgentConfig) -> PushCredential | None: - """What pushes go by: a push-only account's token over HTTPS, or a - deploy key over SSH (D4).""" - if config.push_user and config.push_token: - return HTTPSToken(config.push_user, config.push_token) - if config.push_key is None: +def push_credential( + config: AgentConfig, project: Project | None = None +) -> PushCredential | None: + """What a project's pushes go by: a push-only account's token over + HTTPS, or a deploy key over SSH (D4).""" + project = project or config.only_project() + if project.push_user and project.push_token: + return HTTPSToken(project.push_user, project.push_token) + if project.push_key is None: return None try: - return SSHKey(config.push_key.read_text()) + return SSHKey(project.push_key.read_text()) except OSError as exc: raise RuntimeError( - f"cannot read the push key {config.push_key}: {exc}" + f"cannot read the push key {project.push_key}: {exc}" ) from exc @@ -205,6 +209,7 @@ def prepare_workspace( account: str, remote: str, credential: PushCredential | None, + project: Project | None = None, ) -> GitRepo: """Ensure the workspace on the build VM is a clone of the project repo. @@ -213,7 +218,7 @@ def prepare_workspace( Cloning here rather than when a work item starts is the single-project shortcut that M1.4 and D3's fresh workspace replace. """ - repo = GitRepo(host, config.workspace) + repo = GitRepo(host, (project or config.only_project()).workspace) if not repo.exists(): repo.clone(remote, credential=credential) repo.configure_identity(account, f"{account}@users.noreply.local") diff --git a/nisse-dev/src/nisse_dev/config.py b/nisse-dev/src/nisse_dev/config.py index 8d9216f..576d7d3 100644 --- a/nisse-dev/src/nisse_dev/config.py +++ b/nisse-dev/src/nisse_dev/config.py @@ -17,7 +17,7 @@ silently ignored would look exactly like a knob that has no effect. import os import re from collections.abc import Mapping -from dataclasses import dataclass, field, fields +from dataclasses import dataclass, field, fields, replace from pathlib import Path from typing import Any @@ -30,8 +30,6 @@ DEFAULT_CONFIG_FILE = "nisse-dev.yaml" REQUIRED_KEYS = ( "state_db", - "workspace", - "project", "llm.base_url", "llm.token", "roles.executor", @@ -45,22 +43,31 @@ REQUIRED_KEYS = ( "matrix.operators", "gitea.url", "gitea.token", - "gitea.repo", "gitea.operators", - "build_host.address", - "build_host.key", ) OPTIONAL_KEYS = ( "llm.max_output_tokens", "llm.request_timeout_s", - "gitea.base_branch", - "gitea.push_key", - "gitea.push_user", - "gitea.push_token", - "build_host.user", - "build_host.port", "loop", ) +# A project entry (D8), under `projects: {name: {...}}`. +PROJECT_REQUIRED = ("repo", "workspace", "build_host.address", "build_host.key") +PROJECT_OPTIONAL = ( + "base_branch", + "push_key", + "push_user", + "push_token", + "build_host.user", + "build_host.port", + "labels", + "secrets", + "egress", +) +# Who may see a project secret (D7, D8): the model's commands get it as an +# environment variable, redacted from what comes back; harness-only ones are +# never in a command the model chose. +VISIBILITY = ("commands", "harness") +_PROJECT_NAME = re.compile(r"[a-z0-9][a-z0-9-]*") _REFERENCE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") @@ -225,6 +232,136 @@ def _read_file(path: Path, errors: list[str]) -> dict[str, Any]: return loaded +def _parse_projects( + raw: Any, env: Mapping[str, str], errors: list[str] +) -> tuple["Project", ...]: + """`projects:`, a mapping of name to entry (D8). Every problem in every + entry is reported, like the rest of the file.""" + if not isinstance(raw, dict) or not raw: + errors.append("projects: must name at least one project") + return () + projects: list[Project] = [] + repos: dict[str, str] = {} + for name, entry in raw.items(): + where = f"projects.{name}" + if not isinstance(name, str) or not _PROJECT_NAME.fullmatch(name): + errors.append(f"{where}: a project name is lowercase letters, digits, -") + continue + if not isinstance(entry, dict): + errors.append(f"{where}: must be a mapping") + continue + project = _parse_project(name, entry, env, errors) + if project is None: + continue + if project.repo in repos: + errors.append( + f"{where}.repo: {project.repo} is also project {repos[project.repo]}'s" + ) + repos[project.repo] = name + projects.append(project) + return tuple(projects) + + +def _parse_project( + name: str, entry: Mapping[str, Any], env: Mapping[str, str], errors: list[str] +) -> "Project | None": + where = f"projects.{name}" + # labels, secrets and egress are structured; the rest flattens. + flat = _flatten({k: v for k, v in entry.items() if k not in ("secrets",)}) + known = set(PROJECT_REQUIRED) | set(PROJECT_OPTIONAL) + for key in sorted(set(flat) - known): + errors.append(f"{where}.{key}: unknown setting") + values = { + key: _resolve(value, f"{where}.{key}", env, errors) + for key, value in flat.items() + if key in known + } + missing = [ + key + for key in PROJECT_REQUIRED + if values.get(key) is None + or (isinstance(values[key], str) and not values[key].strip()) + ] + if missing: + errors.append(f"{where}: unset or empty — " + ", ".join(missing)) + return None + port = values.get("build_host.port", 22) + try: + port = int(port) + if not 0 < port < 65536: + raise ValueError + except (TypeError, ValueError): + errors.append(f"{where}.build_host.port: must be a TCP port number") + push_user = values.get("push_user") + push_token = values.get("push_token") + push_key = values.get("push_key") + if bool(push_user) != bool(push_token): + errors.append(f"{where}: push_user and push_token: set both or neither") + if push_token and push_key: + errors.append(f"{where}: set a push token or a push key, not both") + labels = _strings(values.get("labels", []), f"{where}.labels", errors) + egress = _strings(values.get("egress", []), f"{where}.egress", errors) + secrets = _parse_secrets(entry.get("secrets"), where, env, errors) + return Project( + name=name, + repo=str(values["repo"]), + workspace=Path(values["workspace"]).expanduser(), + build_host=BuildHost( + address=str(values["build_host.address"]), + key=Path(values["build_host.key"]).expanduser(), + user=str(values.get("build_host.user") or "root"), + port=port if isinstance(port, int) else 22, + ), + base_branch=str(values.get("base_branch") or "main"), + push_key=Path(push_key).expanduser() if push_key else None, + push_user=str(push_user) if push_user else None, + push_token=str(push_token) if push_token else None, + labels=labels, + secrets=secrets, + egress=egress, + ) + + +def _strings(raw: Any, where: str, errors: list[str]) -> tuple[str, ...]: + if not isinstance(raw, list) or not all( + isinstance(x, str) and x.strip() for x in raw + ): + errors.append(f"{where}: must be a list of names") + return () + return tuple(x.strip() for x in raw) + + +def _parse_secrets( + raw: Any, where: str, env: Mapping[str, str], errors: list[str] +) -> dict[str, "Secret"]: + """name → {value: ${VAR}, visible: commands | harness}. A value must be + a reference: secrets live in secret stores, never in this file.""" + if raw is None: + return {} + if not isinstance(raw, dict): + errors.append(f"{where}.secrets: must be a mapping of name to secret") + return {} + secrets: dict[str, Secret] = {} + for name, spec in raw.items(): + at = f"{where}.secrets.{name}" + if not isinstance(name, str) or not re.fullmatch(r"[A-Z_][A-Z0-9_]*", name): + errors.append(f"{at}: a secret's name is an environment variable name") + continue + if not isinstance(spec, dict) or set(spec) - {"value", "visible"}: + errors.append(f"{at}: give value and visible, nothing else") + continue + visible = spec.get("visible") + if visible not in VISIBILITY: + errors.append(f"{at}.visible: must be one of {', '.join(VISIBILITY)}") + continue + value = spec.get("value") + if not isinstance(value, str) or not _REFERENCE.fullmatch(value.strip()): + errors.append(f"{at}.value: must be a ${{VAR}} reference, never inline") + continue + secrets[name] = Secret(_resolve(value.strip(), at, env, errors), visible) + return secrets + + @dataclass(frozen=True) class BuildHost: """The tool VM the workspace lives on (L13, D3). ``key`` is the path to @@ -236,11 +373,42 @@ class BuildHost: port: int = 22 +@dataclass(frozen=True) +class Secret: + value: str = field(repr=False) + visible: str = "harness" # one of VISIBILITY + + +@dataclass(frozen=True) +class Project: + """A project entry (D8): what makes a project known to the agent, and + everything per-project the rest of the agent refers to.""" + + name: str + repo: str + workspace: Path # on the build VM + build_host: BuildHost + base_branch: str = "main" + # What may push to the project repository, handed to each push on the + # build VM and to nothing else (D4): a push-only account's HTTPS token + # (push_user, push_token), or the path to a deploy key (push_key). + push_key: Path | None = None + push_user: str | None = None + push_token: str | None = field(default=None, repr=False) + labels: tuple[str, ...] = () # added to every issue's own (D1) + secrets: Mapping[str, Secret] = field(default_factory=dict) + egress: tuple[str, ...] = () # beyond the default; recorded, not yet enforced + + def command_secrets(self) -> dict[str, str]: + """The secrets the model's commands may see, by name.""" + return { + name: s.value for name, s in self.secrets.items() if s.visible == "commands" + } + + @dataclass(frozen=True) class AgentConfig: state_db: Path - workspace: Path - project: str llm_base_url: str # Credentials never appear in a repr: a failing test or a log line # would print them. @@ -250,16 +418,8 @@ class AgentConfig: operators: frozenset[str] gitea_url: str gitea_token: str = field(repr=False) - gitea_repo: str gitea_operators: frozenset[str] - build_host: BuildHost - base_branch: str = "main" - # What may push to the project repository, handed to each push on the - # build VM and to nothing else (D4): a push-only account's HTTPS token - # (push_user, push_token), or the path to a deploy key (push_key). - push_key: Path | None = None - push_user: str | None = None - push_token: str | None = field(default=None, repr=False) + projects: tuple[Project, ...] max_output_tokens: int = DEFAULT_MAX_OUTPUT_TOKENS request_timeout_s: float = DEFAULT_REQUEST_TIMEOUT_S tuning: Tuning = field(default_factory=Tuning) @@ -268,6 +428,55 @@ class AgentConfig: loop: str = "old" config_file: Path | None = None + # -- one project, for the old loop ---------------------------------------- + # control.py (until M2.8) knows one project; these read it, and refuse a + # configuration with more. + + def only_project(self) -> Project: + if len(self.projects) != 1: + names = ", ".join(p.name for p in self.projects) + raise ConfigError( + f"cannot start: the old loop runs exactly one project, but " + f"{len(self.projects)} are configured ({names}); use `loop: new`" + ) + return self.projects[0] + + def with_project(self, **changes: Any) -> "AgentConfig": + """This configuration with its one project changed (for tests).""" + return replace(self, projects=(replace(self.only_project(), **changes),)) + + @property + def project(self) -> str: + return self.only_project().name + + @property + def gitea_repo(self) -> str: + return self.only_project().repo + + @property + def workspace(self) -> Path: + return self.only_project().workspace + + @property + def build_host(self) -> BuildHost: + return self.only_project().build_host + + @property + def base_branch(self) -> str: + return self.only_project().base_branch + + @property + def push_key(self) -> Path | None: + return self.only_project().push_key + + @property + def push_user(self) -> str | None: + return self.only_project().push_user + + @property + def push_token(self) -> str | None: + return self.only_project().push_token + @classmethod def load(cls, env: Mapping[str, str] = os.environ) -> "AgentConfig": """The file named by NISSE_CONFIG, or ./nisse-dev.yaml.""" @@ -304,7 +513,8 @@ class AgentConfig: config_file: Path | None, ) -> "AgentConfig": tuning = _parse_tuning(tree.get("tuning"), errors) - flat = _flatten(tree) + projects = _parse_projects(tree.get("projects"), env, errors) + flat = _flatten({k: v for k, v in tree.items() if k != "projects"}) flat.pop("tuning", None) known = set(REQUIRED_KEYS) | set(OPTIONAL_KEYS) @@ -343,30 +553,14 @@ class AgentConfig: raise ValueError except (TypeError, ValueError): errors.append("llm.request_timeout_s: must be a positive number of seconds") - port = values.get("build_host.port", 22) - try: - port = int(port) - if not 0 < port < 65536: - raise ValueError - except (TypeError, ValueError): - errors.append("build_host.port: must be a TCP port number") loop = str(values.get("loop") or "old") if loop not in ("old", "new"): errors.append("loop: must be old or new") - push_user = values.get("gitea.push_user") - push_token = values.get("gitea.push_token") - if bool(push_user) != bool(push_token): - errors.append("gitea.push_user and gitea.push_token: set both or neither") - if push_token and values.get("gitea.push_key"): - errors.append("gitea: set a push token or a push key, not both") if errors: raise ConfigError("cannot start: " + "; ".join(errors)) - push_key = values.get("gitea.push_key") config = cls( state_db=Path(values["state_db"]).expanduser(), - workspace=Path(values["workspace"]).expanduser(), - project=str(values["project"]), llm_base_url=str(values["llm.base_url"]), llm_token=str(values["llm.token"]), roles=RoleMap( @@ -384,18 +578,8 @@ class AgentConfig: operators=_people(values["matrix.operators"]), gitea_url=str(values["gitea.url"]), gitea_token=str(values["gitea.token"]), - gitea_repo=str(values["gitea.repo"]), gitea_operators=_people(values["gitea.operators"]), - build_host=BuildHost( - address=str(values["build_host.address"]), - key=Path(values["build_host.key"]).expanduser(), - user=str(values.get("build_host.user") or "root"), - port=port, - ), - push_key=Path(push_key).expanduser() if push_key else None, - push_user=str(push_user) if push_user else None, - push_token=str(push_token) if push_token else None, - base_branch=str(values.get("gitea.base_branch") or "main"), + projects=projects, max_output_tokens=max_output, request_timeout_s=request_timeout, tuning=tuning, @@ -416,31 +600,34 @@ class AgentConfig: from and how the agent is deployed, which is not the project's business either. """ - workspace = self.workspace.expanduser().resolve() candidates = [ ("state_db", self.state_db), ("NISSE_ENV", Path(env.get("NISSE_ENV", ".env"))), ] if self.config_file is not None: candidates.append(("NISSE_CONFIG", self.config_file)) - offenders = [] - for label, path in candidates: - resolved = path.expanduser().resolve() - if resolved == workspace or workspace in resolved.parents: - offenders.append(f"{label} ({resolved})") - if offenders: - raise ConfigError( - "refusing to start: these sit inside the workspace clone at " - f"{workspace} and would be committed and pushed to the project " - "repository — " + ", ".join(offenders) - ) + for project in self.projects: + workspace = project.workspace.expanduser().resolve() + offenders = [] + for label, path in candidates: + resolved = path.expanduser().resolve() + if resolved == workspace or workspace in resolved.parents: + offenders.append(f"{label} ({resolved})") + if offenders: + raise ConfigError( + "refusing to start: these sit inside the workspace clone at " + f"{workspace} (project {project.name}) and would be " + "committed and pushed to the project repository — " + + ", ".join(offenders) + ) def redacted(self) -> dict[str, str]: """What the agent may safely announce about how it is configured.""" return { - "project": self.project, - "workspace": f"{self.build_host.address}:{self.workspace}", - "repo": self.gitea_repo, + "projects": ", ".join( + f"{p.name} ({p.repo}, {p.build_host.address}:{p.workspace})" + for p in self.projects + ), "planner": self.roles.planner, "executor": self.roles.executor, "helper": self.roles.helper, diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index 0669a9a..28928da 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/nisse-dev/src/nisse_dev/loop/agent.py @@ -19,8 +19,8 @@ import re import threading import time import uuid -from collections.abc import Callable, Iterable -from dataclasses import replace +from collections.abc import Callable, Iterable, Sequence +from dataclasses import dataclass, replace from nisse_dev.cancel import Cancel from nisse_dev.canonical import Fact, PlanSubmitted, UserMessage @@ -61,9 +61,11 @@ from nisse_dev.worker import Worker INTAKE_INTERVAL_S = 30.0 _GO = frozenset({"go", "approve"}) -_NEXT = re.compile(r"(?:do|work on) #?(\d+) (?:next|first|now)") +# An issue, named: "#14", "14", or with its project, "sandbox#14" (D8). +_ISSUE = r"(?:([a-z0-9][a-z0-9-]*)#|#)?(\d+)" +_NEXT = re.compile(rf"(?:do|work on) {_ISSUE} (?:next|first|now)") # A command that names its item applies to it, wherever it is said (L5). -_NAMED = re.compile(r"(go|approve|abandon|stop) #(\d+)") +_NAMED = re.compile(r"(go|approve|abandon|stop) (?:([a-z0-9][a-z0-9-]*))?#(\d+)") # At top level these need an item; without one they are ambiguous (L5). _NEEDS_ITEM = frozenset({"go", "approve", "abandon"}) BUG_GRACE_S = 600.0 @@ -125,6 +127,33 @@ LABELS = { } +@dataclass(frozen=True) +class Project: + """What the loop needs of one project (D8): its repository, label + defaults, and its own workspace and milestones on its build VM.""" + + name: str + repo: str + workspace: Workspace | None = None + milestones: Milestones | None = None + labels: tuple[str, ...] = () + + +def _restatement_problem(args: dict[str, object]) -> str | None: + """L5, D1: a restatement is agreed to as it stands, so it asks nothing. + One that ends in a question is sent back: questions go by ask_human.""" + text = str(args.get("text", "")).strip() + if not text: + return "a restatement needs text: what you take the request to mean" + if is_question(text): + return ( + "not posted: a restatement asks no questions, so the operator can " + "agree to it as it stands. If you need an answer first, use " + "ask_human; otherwise restate it as a statement." + ) + return None + + class LoopAgent: def __init__( self, @@ -132,16 +161,18 @@ class LoopAgent: transport: Transport, gitea: GiteaClient, *, - repo: str, - project: str, account: str, operators: Iterable[str], gitea_operators: Iterable[str], driver: Driver, - workspace: Workspace | None, worker: Worker, cancel: Cancel, oplog: OpLog, + projects: Sequence[Project] = (), + # One project, given directly: the same as projects=[Project(...)]. + repo: str = "", + project: str = "", + workspace: Workspace | None = None, roles: dict[str, str] | None = None, milestones: Milestones | None = None, reader: GoAheadReader | None = None, @@ -152,32 +183,56 @@ class LoopAgent: self._store = store self._transport = transport self._gitea = gitea - self._repo = repo - self._project = project + if not projects: + projects = [Project(project, repo, workspace, milestones)] + self._projects = {p.name: p for p in projects} self._account = account self._operators = frozenset(operators) self._gitea_operators = frozenset(gitea_operators) self._driver = driver - self._workspace = workspace self._worker = worker self._cancel = cancel self._oplog = oplog self._roles = roles or {} - self._milestones = milestones self._reader = reader self._grace = bug_grace_s self._clock = clock self._interval = intake_interval_s # thread root -> revision on screen there, read on the transport's - # thread to stamp replies on arrival (L3); issue -> thread root, for - # a command that names its item. + # thread to stamp replies on arrival (L3); (project, issue) -> thread + # root, for a command that names its item. self._shown: dict[str, int] = {} - self._threads: dict[int, str] = {} + self._threads: dict[tuple[str, int], str] = {} self._active_thread = "" + # The project whose workspace was last worked on: switching to + # another parks it (L1). None after a boot: unknown, so all are. + self._last_project: str | None = None self._running = False self._stop = threading.Event() self._intake_thread: threading.Thread | None = None + # -- projects (D8) ---------------------------------------------------------- + + def _at(self, item: WorkItem) -> Project: + return self._projects[item.scope or ""] + + def _label(self, item: WorkItem) -> str: + """How an item's issue is named to the operator: #14 with one + project, sandbox#14 with more, where #14 alone could be either.""" + issue = loop_of(item).issue + return f"#{issue}" if len(self._projects) == 1 else f"{item.scope}#{issue}" + + def _named(self, project: str | None, issue: int) -> list[WorkItem]: + """Open items for ``issue``, in ``project`` if it is given.""" + return [ + i + for i in self._store.work_items("open") + if loop_of(i).issue == issue and (project is None or i.scope == project) + ] + + def _repos(self) -> str: + return ", ".join(p.repo for p in self._projects.values()) + # -- lifecycle ---------------------------------------------------------- def start(self) -> None: @@ -220,7 +275,7 @@ class LoopAgent: self._reconcile_publishing() for item in self._store.work_items("open"): loop = loop_of(item) - self._threads[loop.issue] = loop.thread + self._threads[(item.scope or "", loop.issue)] = loop.thread if item.position == RUNNING: note = "I restarted while working on this. Reply to continue." self._wait(item, replace(loop, wait=ANSWER, note=note), note) @@ -247,8 +302,11 @@ class LoopAgent: continue item = self._store.work_item(intent.work_item) branch = str(intent.payload.get("branch", "")) + if item is None or item.scope not in self._projects: + self._store.abort_intent(intent.id, cause="item or project unknown") + continue try: - pr = self._gitea.find_pull_request(self._repo, branch) + pr = self._gitea.find_pull_request(self._at(item).repo, branch) except GiteaError: continue # unverifiable: left pending, asked about below if item is None or item.status != "open": @@ -272,33 +330,39 @@ class LoopAgent: # -- intake (D1) --------------------------------------------------------- def intake(self) -> None: - """Issues assigned to the agent become work items, once each.""" + """Issues assigned to the agent, in every project's repository, + become work items, once each.""" + for project in self._projects.values(): + self._intake(project) + self.watch() + self.check_blocked() + self.check_grace() + self._schedule() + + def _intake(self, project: Project) -> None: try: - issues = self._gitea.assigned_issues(self._repo, self._account) + issues = self._gitea.assigned_issues(project.repo, self._account) except GiteaError as exc: # L5: an outage delays intake, never fails anything. self._oplog.emit("loop.intake_unavailable", cause=str(exc)) return for issue in issues: - if item_for_issue(self._store, issue.number) is not None: + if item_for_issue(self._store, issue.number, project.name) is not None: continue - if not self._from_an_operator(issue): + if not self._from_an_operator(project, issue): self._oplog.emit( "loop.intake_ignored", cause="not filed or assigned by an operator", issue=issue.number, + repo=project.repo, ) continue try: - blocked_by = self._gitea.open_dependencies(self._repo, issue.number) + blocked_by = self._gitea.open_dependencies(project.repo, issue.number) except GiteaError as exc: self._oplog.emit("loop.intake_unavailable", cause=str(exc)) continue # taken on the next round - self._open(issue, tuple(blocked_by)) - self.watch() - self.check_blocked() - self.check_grace() - self._schedule() + self._open(project, issue, tuple(blocked_by)) # -- PR review and merge (D4, D5) ------------------------------------------- @@ -309,10 +373,11 @@ class LoopAgent: loop = loop_of(item) if not loop.pr: continue + repo = self._at(item).repo try: - pr = self._gitea.pull_request(self._repo, loop.pr) - comments = self._gitea.list_comments(self._repo, loop.pr) - notes = self._gitea.review_notes(self._repo, loop.pr) + pr = self._gitea.pull_request(repo, loop.pr) + comments = self._gitea.list_comments(repo, loop.pr) + notes = self._gitea.review_notes(repo, loop.pr) except GiteaError as exc: self._oplog.emit("loop.watch_unavailable", cause=str(exc)) continue @@ -379,19 +444,20 @@ class LoopAgent: if item.position not in (READY, BLOCKED): continue loop = loop_of(item) - issue = loop.issue + repo = self._at(item).repo try: - blocked_by = tuple(self._gitea.open_dependencies(self._repo, issue)) + blocked_by = tuple(self._gitea.open_dependencies(repo, loop.issue)) except GiteaError as exc: self._oplog.emit("loop.watch_unavailable", cause=str(exc)) continue # as it was, until Gitea answers position = BLOCKED if blocked_by else READY if position == item.position and blocked_by == loop.blocked_by: continue + name = self._label(item) if position == READY: - said = f"#{issue} is unblocked: its dependencies have closed." + said = f"{name} is unblocked: its dependencies have closed." else: - said = f"#{issue} is blocked by {', '.join(blocked_by)}." + said = f"{name} is blocked by {', '.join(blocked_by)}." self._fact(item, said) if position != item.position: self._transport.post(said, thread=loop.thread) @@ -421,38 +487,42 @@ class LoopAgent: ) self._go(item, loop, "the bug grace period") - def _from_an_operator(self, issue: Issue) -> bool: + def _from_an_operator(self, project: Project, issue: Issue) -> bool: """L11: only an operator's issue, or one an operator assigned, is an instruction.""" if issue.author in self._gitea_operators: return True try: - who = self._gitea.assigned_by(self._repo, issue.number, self._account) + who = self._gitea.assigned_by(project.repo, issue.number, self._account) except GiteaError: return False return who in self._gitea_operators - def _open(self, issue: Issue, blocked_by: tuple[str, ...]) -> None: - item_id = f"{self._project}-{issue.number}-{uuid.uuid4().hex[:6]}" - cause = f"assigned {self._repo}#{issue.number}" + def _open( + self, project: Project, issue: Issue, blocked_by: tuple[str, ...] + ) -> None: + item_id = f"{project.name}-{issue.number}-{uuid.uuid4().hex[:6]}" + cause = f"assigned {project.repo}#{issue.number}" ahead = len(self._store.work_items("open")) where = f", {ahead} ahead" if ahead else "" if blocked_by: where += f"; blocked by {', '.join(blocked_by)}" root = self._transport.post( - f"Queued {self._repo}#{issue.number}: {issue.title}{where}" + f"Queued {project.repo}#{issue.number}: {issue.title}{where}" ) + # D8: the project's label defaults steer as if the issue had them. + labels = tuple(dict.fromkeys((*issue.labels, *project.labels))) loop = Loop( thread=root, issue=issue.number, blocked_by=blocked_by, - labels=tuple(issue.labels), + labels=labels, ) - self._threads[issue.number] = root + self._threads[(project.name, issue.number)] = root self._store.open_work_item( item_id, title=issue.title, - scope=self._project, + scope=project.name, position=BLOCKED if blocked_by else READY, data={"loop": loop.to_data(), "roles": self._roles}, cause=cause, @@ -460,9 +530,14 @@ class LoopAgent: log = self._store.transcript(item_id, "orchestrator") log.append( Fact( - text=f"work item opened from {self._repo}#{issue.number} " + text=f"work item opened from {project.repo}#{issue.number} " f"({issue.title}), filed by {issue.author}" - + (f", labels: {', '.join(issue.labels)}" if issue.labels else "") + + (f", labels: {', '.join(labels)}" if labels else "") + + ( + f" ({', '.join(project.labels)} by the project's default)" + if project.labels + else "" + ) ) ) text = f"{issue.title}\n\n{issue.body}".strip() @@ -486,7 +561,14 @@ class LoopAgent: named = _NAMED.fullmatch(command) if named is not None: command = named.group(1) - thread = self._threads.get(int(named.group(2)), thread) + project, issue = named.group(2), int(named.group(3)) + threads = [ + t + for (p, n), t in list(self._threads.items()) + if n == issue and project in (None, p) + ] + # Ambiguous across projects: no stamp; handle asks which. + thread = threads[0] if len(threads) == 1 else thread if ( command == "stop" and self._running @@ -510,30 +592,27 @@ class LoopAgent: UserMessage(text=text, source=f"transport:{msg.sender}") ) self._transport.post( - self._put_first(int(order.group(1))), thread=msg.thread + self._put_first(order.group(1), int(order.group(2))), + thread=msg.thread, ) self._schedule() return named = _NAMED.fullmatch(command) if named is not None: # L5: a command that names its item applies to it. - command, issue = named.group(1), int(named.group(2)) - item = next( - ( - i - for i in self._store.work_items("open") - if loop_of(i).issue == issue - ), - None, - ) - if item is None: + command = named.group(1) + project, issue = named.group(2), int(named.group(3)) + found = self._named(project, issue) + if len(found) != 1: self._store.transcript(None, "operator").append( UserMessage(text=text, source=f"transport:{msg.sender}") ) self._transport.post( - f"#{issue} isn't one of my open work items.", thread=msg.thread + self._not_one(found, project, issue, f"{command} {{}}"), + thread=msg.thread, ) return + [item] = found if item is None: self._store.transcript(None, "operator").append( UserMessage(text=text, source=f"transport:{msg.sender}") @@ -547,7 +626,7 @@ class LoopAgent: self._transport.post("Nothing is running.", thread=msg.thread) else: self._transport.post( - f"I take work from issues assigned to me in {self._repo}: " + f"I take work from issues assigned to me in {self._repos()}: " "assign one and I'll pick it up. Reply in its thread to " "talk about it.", thread=msg.thread, @@ -574,12 +653,12 @@ class LoopAgent: # Stopping a run took effect on arrival; a stop is never an answer. if not loop.note.startswith("stopped ("): self._transport.post( - f"#{loop.issue} isn't running; nothing to stop.", + f"{self._label(item)} isn't running; nothing to stop.", thread=loop.thread, ) elif item.position == BLOCKED: self._transport.post( - f"#{loop.issue} is blocked by {', '.join(loop.blocked_by)}; " + f"{self._label(item)} is blocked by {', '.join(loop.blocked_by)}; " "I'll start on it when they close, or when you remove them.", thread=loop.thread, ) @@ -642,31 +721,44 @@ class LoopAgent: items = self._store.work_items("open") if not items: return f"{command} what? I have no open work items." - listed = ", ".join(f"#{loop_of(i).issue} ({i.title})" for i in items) + listed = ", ".join(f"{self._label(i)} ({i.title})" for i in items) return ( f"Which item? Say it in the item's thread, or name it " - f'("{command} #{loop_of(items[0]).issue}"). Open: {listed}.' + f'("{command} {self._label(items[0])}"). Open: {listed}.' ) - def _put_first(self, issue: int) -> str: + def _not_one( + self, found: list[WorkItem], project: str | None, issue: int, say: str + ) -> str: + """A named item that is none, or more than one (the same number in + two projects). ``say`` is the command, with {} for the item.""" + name = f"{project}#{issue}" if project else f"#{issue}" + if not found: + return f"{name} isn't one of my open work items." + options = " or ".join(say.format(self._label(i)) for i in found) + return f"{name} is open in more than one project: say {options}." + + def _put_first(self, project: str | None, issue: int) -> str: """The operator's "do #N next" (L1): it runs before every other ready item, the latest such order first. It doesn't unblock.""" + found = self._named(project, issue) + if len(found) != 1: + return self._not_one(found, project, issue, "do {} next") + [item] = found + name = self._label(item) items = self._store.work_items("open") - item = next((i for i in items if loop_of(i).issue == issue), None) - if item is None: - return f"#{issue} isn't one of my open work items." first = max(loop_of(i).first for i in items) + 1 loop = replace(loop_of(item), first=first) - save(self._store, item.id, item.position, loop, cause=f"#{issue} put first") + save(self._store, item.id, item.position, loop, cause=f"{name} put first") self._fact(item, "the operator put this item first in the queue") if item.position == BLOCKED: return ( - f"#{issue} goes first once it's unblocked " + f"{name} goes first once it's unblocked " f"(blocked by {', '.join(loop.blocked_by)})." ) if item.position == WAITING: - return f"#{issue} goes first, but it's waiting for {loop.wait}: {loop.note}" - return f"#{issue} is next." + return f"{name} goes first, but it's waiting for {loop.wait}: {loop.note}" + return f"{name} is next." def _queue(self) -> list[WorkItem]: """Ready items in the order they run (L1): the operator's order @@ -678,7 +770,10 @@ class LoopAgent: def status(self) -> str: items = self._store.work_items("open") if not items: - return f"idle: no open work items. I take issues assigned to me in {self._repo}." + return ( + "idle: no open work items. I take issues assigned to me in " + f"{self._repos()}." + ) queue = [i.id for i in self._queue()] lines = [] for item in items: @@ -693,7 +788,7 @@ class LoopAgent: what = f"blocked by {', '.join(loop.blocked_by)}" elif item.position == READY: what = f"ready, {queue.index(item.id) + 1} of {len(queue)} in line" - lines.append(f"#{loop.issue} {item.title}: {what}") + lines.append(f"{self._label(item)} {item.title}: {what}") return "\n".join(lines) # -- running (L1, L2) ------------------------------------------------------ @@ -709,20 +804,46 @@ class LoopAgent: return self._run(ready[0]) + def _keep(self, project: str) -> tuple[str, ...]: + """Branch prefixes of a project's open items: parked or not, they + are never set aside.""" + return tuple( + f"{i.id}-" for i in self._store.work_items("open") if i.scope == project + ) + + def _park_elsewhere(self, here: str) -> None: + """L1: switching to another project's item parks the one on the + project worked on last, so nothing lives only on its build VM. After + a boot nobody knows which that was, so every other is parked.""" + last = self._last_project + others = [p for p in self._projects.values() if p.name != here] + for other in others if last is None else [self._projects[last]]: + if other.name == here or other.milestones is None: + continue + try: + other.milestones.park(keep=self._keep(other.name)) + except Exception as exc: # noqa: BLE001 -- reported; the run goes on + self._oplog.emit( + "loop.park_failed", cause=repr(exc), project=other.name + ) + self._last_project = here + def _run(self, item: WorkItem) -> None: loop = loop_of(item) + project = self._at(item) + milestones = project.milestones self._store.set_active(item.id, cause="ready") - if self._milestones is not None: - # Branches of open items, parked or not, are never set aside. - keep = tuple(f"{i.id}-" for i in self._store.work_items("open")) + self._park_elsewhere(project.name) + if milestones is not None: + keep = self._keep(project.name) try: if not loop.prepared: # D3: a fresh workspace when a work item starts. - notes = self._milestones.prepare(f"{item.id}-work", keep=keep) + notes = milestones.prepare(f"{item.id}-work", keep=keep) else: # D4: back to its branch, parking whoever was there. notes = [] - self._milestones.resume( + milestones.resume( loop.branch or f"{item.id}-work", keep=keep, mine=f"{item.id}-" ) except Exception as exc: # noqa: BLE001 -- the item waits, not the loop @@ -755,12 +876,13 @@ class LoopAgent: ] ), ] + workspace = project.workspace system = SYSTEM.format( - repo=self._repo, + repo=project.repo, issue=loop.issue, title=item.title, labels="".join(f"{LABELS[x]}\n\n" for x in loop.labels if x in LABELS), - workspace=self._workspace.describe() if self._workspace else "", + workspace=workspace.describe() if workspace else "", ) self._active_thread = loop.thread self._running = True @@ -769,8 +891,9 @@ class LoopAgent: self._store.transcript(item.id, "driver"), system=system, tools=tools, - workspace=self._workspace, + workspace=workspace, handlers={"start_milestone": lambda args: self._start(item.id, args)}, + checks={"restate": _restatement_problem}, ) finally: self._running = False @@ -805,12 +928,12 @@ class LoopAgent: ) elif outcome.kind == "wait" and outcome.tool == "ask_human": question = str(args.get("question", "")).strip() - self._answer_where_asked(loop, question) + self._answer_where_asked(item, loop, question) self._wait(item, replace(loop, wait=ANSWER, note=question), None) elif outcome.kind == "wait" and outcome.tool == "wait_for_review": note = str(args.get("note", "")).strip() or f"waiting on PR #{loop.pr}" if loop.pr: - self._answer_where_asked(loop, note) + self._answer_where_asked(item, loop, note) self._wait(item, replace(loop, wait=MERGE, note=f"PR #{loop.pr}"), None) else: say = f"{note} (there's no open PR, so I need you to tell me what next)" @@ -855,11 +978,12 @@ class LoopAgent: item = self._store.work_item(item_id) assert item is not None loop = loop_of(item) + project = self._at(item) if loop.pr: # D4: the next milestone waits for the merge, read from Gitea now, # not from the last watch round. try: - pr = self._gitea.pull_request(self._repo, loop.pr) + pr = self._gitea.pull_request(project.repo, loop.pr) except GiteaError as exc: return f"couldn't check PR #{loop.pr}: {exc}; try again", False if pr.state == "open": @@ -875,12 +999,12 @@ class LoopAgent: if loop.branch: busy = f"milestone {loop.milestone} is still open; finish it first" return busy, False - if self._milestones is None: + if project.milestones is None: return "milestones aren't available here", False number = loop.milestone + 1 branch = f"{item.id}-milestone-{number}-{uuid.uuid4().hex[:4]}" try: - self._milestones.start(branch) + project.milestones.start(branch) except GitError as exc: return f"couldn't start the milestone: {exc}", False name = str(args.get("name", "")).strip() @@ -894,7 +1018,7 @@ class LoopAgent: ) return f"started milestone {number} on branch {branch}", True - def _answer_where_asked(self, loop: Loop, text: str) -> None: + def _answer_where_asked(self, item: WorkItem, loop: Loop, text: str) -> None: """D5: answer where the operator asked. A PR question is answered on the PR, quoting the line it was about (an inline thread can't be replied to through the API), with a pointer in the Matrix thread.""" @@ -905,7 +1029,7 @@ class LoopAgent: quoted += [f"> {line}" for line in loop.asked_text.splitlines()] body = "\n".join([*quoted, "", text]) try: - self._gitea.post_comment(self._repo, loop.asked_on_pr, body) + self._gitea.post_comment(self._at(item).repo, loop.asked_on_pr, body) except GiteaError: self._transport.post(text, thread=loop.thread) # the PR is unreachable return @@ -924,12 +1048,14 @@ class LoopAgent: final: bool = True, ) -> None: """finish_milestone: publish the milestone, or push a review fix.""" - if self._milestones is None: + project = self._at(item) + milestones = project.milestones + if milestones is None: note = "milestones aren't available here" self._wait(item, replace(loop, wait=ANSWER, note=note), note) return if loop.pr: - fixed = self._milestones.push_fix( + fixed = milestones.push_fix( item_id=item.id, branch=loop.branch, pr=loop.pr, @@ -938,7 +1064,7 @@ class LoopAgent: ) self._fact(item, f"review fix: {fixed.note}") if fixed.kind in ("pushed", "nothing"): - self._gitea_comment(loop.pr, fixed.note) + self._gitea_comment(project.repo, loop.pr, fixed.note) self._wait( item, replace(loop, wait=MERGE, note=f"PR #{loop.pr}"), @@ -971,13 +1097,13 @@ class LoopAgent: number = loop.milestone + 1 branch = f"{item.id}-milestone-{number}-{uuid.uuid4().hex[:4]}" try: - self._milestones.start(branch) + milestones.start(branch) except GitError as exc: note = f"I couldn't start milestone {number}: {exc}" self._wait(item, replace(loop, wait=ANSWER, note=note), note) return loop = replace(loop, milestone=number, branch=branch) - published = self._milestones.publish( + published = milestones.publish( item_id=item.id, branch=loop.branch, title=title or f"{item.title} (milestone {loop.milestone})", @@ -1018,9 +1144,9 @@ class LoopAgent: f"Milestone {loop.milestone}: {published.note}. What next?", ) - def _gitea_comment(self, pr: int, text: str) -> None: + def _gitea_comment(self, repo: str, pr: int, text: str) -> None: try: - self._gitea.post_comment(self._repo, pr, text) + self._gitea.post_comment(repo, pr, text) except GiteaError: pass # courtesy only; the pushed commit is the answer @@ -1031,26 +1157,27 @@ class LoopAgent: def _close(self, item: WorkItem, loop: Loop, outcome: str, note: str) -> None: self._fact(item, f"work item {outcome}: {note}") + project = self._at(item) if loop.pr and outcome == "abandoned": # L2: abandoning closes the item's open PR with a note. try: - self._gitea.post_comment(self._repo, loop.pr, f"abandoned: {note}") - self._gitea.close_pull_request(self._repo, loop.pr) + self._gitea.post_comment(project.repo, loop.pr, f"abandoned: {note}") + self._gitea.close_pull_request(project.repo, loop.pr) except GiteaError as exc: self._transport.post( f"could not close PR #{loop.pr}: {exc}", thread=loop.thread ) save(self._store, item.id, CLOSED, loop, cause=outcome) self._store.close_work_item(item.id, outcome=outcome, cause=note) - if self._milestones is not None: + if project.milestones is not None: try: - self._milestones.drop_parked(f"{item.id}-") + project.milestones.drop_parked(f"{item.id}-") except GitError: pass # a leftover side branch; nothing depends on it if loop.issue: try: - self._gitea.post_comment(self._repo, loop.issue, f"{outcome}: {note}") - self._gitea.close_issue(self._repo, loop.issue) + self._gitea.post_comment(project.repo, loop.issue, f"{outcome}: {note}") + self._gitea.close_issue(project.repo, loop.issue) except GiteaError as exc: self._transport.post( f"closed here, but could not close #{loop.issue}: {exc}", diff --git a/nisse-dev/src/nisse_dev/loop/build.py b/nisse-dev/src/nisse_dev/loop/build.py index 345adcc..09f0aec 100644 --- a/nisse-dev/src/nisse_dev/loop/build.py +++ b/nisse-dev/src/nisse_dev/loop/build.py @@ -16,7 +16,7 @@ from nisse_dev.agent import ( from nisse_dev.cancel import Cancel from nisse_dev.config import AgentConfig from nisse_dev.gitea import GiteaClient -from nisse_dev.loop.agent import LoopAgent +from nisse_dev.loop.agent import LoopAgent, Project from nisse_dev.loop.driver import Driver from nisse_dev.loop.milestones import Milestones from nisse_dev.loop.reader import GoAheadReader @@ -41,7 +41,8 @@ class RunningLoop: def start(self) -> None: self.loop.start() self.transport.post( - f"nisse-dev up as {self.account} (new loop) on {self.config.gitea_repo}; " + f"nisse-dev up as {self.account} (new loop) on " + f"{', '.join(p.repo for p in self.config.projects)}; " f"{self.loop_status()}" ) @@ -75,16 +76,37 @@ def build_loop_agent(config: AgentConfig) -> RunningLoop: catalog = discover_models(config, oplog) gitea = GiteaClient(config.gitea_url, config.gitea_token) account = gitea.whoami() - host = build_host(config) - credential = push_credential(config) - remote = remote_url(gitea, config.gitea_repo, credential) - git = prepare_workspace(host, config, account, remote, credential) - workspace = Workspace( - host, - config.workspace, - timeout_s=config.tuning.tools.command_timeout_s, - max_output_chars=config.tuning.tools.max_output_chars, - ) + # D8: each project its own build VM, clone, push credential and milestones. + projects = [] + for entry in config.projects: + host = build_host(config, entry) + credential = push_credential(config, entry) + remote = remote_url(gitea, entry.repo, credential) + git = prepare_workspace(host, config, account, remote, credential, entry) + projects.append( + Project( + name=entry.name, + repo=entry.repo, + labels=entry.labels, + workspace=Workspace( + host, + entry.workspace, + timeout_s=config.tuning.tools.command_timeout_s, + max_output_chars=config.tuning.tools.max_output_chars, + # D7: the secrets its commands may see, redacted on return. + secrets=entry.command_secrets(), + ), + milestones=Milestones( + store, + git, + gitea, + repo=entry.repo, + remote=remote, + credential=credential, + base=entry.base_branch, + ), + ) + ) normalized, _passthrough = make_llm_clients(config, catalog) cancel = Cancel() # The driver is the planner's model for now (M2.6 brings the driver role). @@ -99,13 +121,11 @@ def build_loop_agent(config: AgentConfig) -> RunningLoop: store, transport, gitea, - repo=config.gitea_repo, - project=config.project, + projects=projects, account=account, operators=config.operators, gitea_operators=config.gitea_operators, driver=driver, - workspace=workspace, worker=worker, cancel=cancel, oplog=oplog, @@ -114,15 +134,6 @@ def build_loop_agent(config: AgentConfig) -> RunningLoop: # classifier role arrives (M2.6). reader=GoAheadReader(normalized, config.roles.resolve("planner")), bug_grace_s=config.tuning.loop.bug_grace_s, - milestones=Milestones( - store, - git, - gitea, - repo=config.gitea_repo, - remote=remote, - credential=credential, - base=config.base_branch, - ), ) return RunningLoop( config=config, diff --git a/nisse-dev/src/nisse_dev/loop/driver.py b/nisse-dev/src/nisse_dev/loop/driver.py index cad9841..f0fef5a 100644 --- a/nisse-dev/src/nisse_dev/loop/driver.py +++ b/nisse-dev/src/nisse_dev/loop/driver.py @@ -58,10 +58,14 @@ class Driver: workspace: Workspace | None, handlers: Mapping[str, Callable[[dict[str, Any]], tuple[str, bool]]] | None = None, + checks: Mapping[str, Callable[[dict[str, Any]], str | None]] | None = None, ) -> RunOutcome: """``handlers`` answer control tools that don't end the run (e.g. - start_milestone): each returns a result and whether it succeeded.""" + start_milestone): each returns a result and whether it succeeded. + ``checks`` vet a waiting tool's arguments: a problem is the call's + failed result and the run goes on, so the model puts it right.""" handlers = handlers or {} + checks = checks or {} offered = names(tools) nudged = False for turn in range(self._max_turns): @@ -89,6 +93,19 @@ class Driver: if self._cancel.requested: skip_calls(transcript, calls[index:]) return RunOutcome("stopped", note=f"during turn {turn + 1}") + problem = ( + checks[call.tool](dict(call.arguments)) + if call.tool in checks and call.tool in offered + else None + ) + if problem is not None: + transcript.append( + ToolResult(call_id=call.call_id, result=problem, success=False) + ) + skip_calls( + transcript, calls[index + 1 :], "not run: fix the call above" + ) + break if call.tool in WAITING and call.tool in offered: transcript.append( ToolResult( diff --git a/nisse-dev/src/nisse_dev/loop/items.py b/nisse-dev/src/nisse_dev/loop/items.py index b15aee2..5cb1543 100644 --- a/nisse-dev/src/nisse_dev/loop/items.py +++ b/nisse-dev/src/nisse_dev/loop/items.py @@ -82,10 +82,12 @@ def item_for_thread(store: StateStore, thread: str) -> WorkItem | None: return None -def item_for_issue(store: StateStore, issue: int) -> WorkItem | None: - """Any item, open or closed, recorded by this issue: intake never takes - an issue twice (D1).""" +def item_for_issue( + store: StateStore, issue: int, project: str | None = None +) -> WorkItem | None: + """Any item, open or closed, recorded by this issue (in ``project``, + when given): intake never takes an issue twice (D1).""" for item in store.work_items(): - if loop_of(item).issue == issue: + if loop_of(item).issue == issue and project in (None, item.scope): return item return None diff --git a/nisse-dev/tests/test_agent_build.py b/nisse-dev/tests/test_agent_build.py index e457171..d11a050 100644 --- a/nisse-dev/tests/test_agent_build.py +++ b/nisse-dev/tests/test_agent_build.py @@ -9,7 +9,7 @@ from typing import Any import httpx import pytest -from test_config import SECRETS, tree +from test_config import SECRETS, P, tree from nisse_dev.agent import build_agent, discover_models, prepare_workspace from nisse_dev.catalog import ModelCatalog, UnknownModel @@ -51,11 +51,13 @@ def config_for(tmp_path: Path, **changes: str) -> AgentConfig: return AgentConfig.from_tree( tree( state_db=str(tmp_path / "state.db"), - workspace=str(tmp_path / "work"), - build_host__address=HOST.address, - build_host__user=HOST.user, - build_host__port=str(HOST.port), - build_host__key=str(HOST._key), + **{ + f"{P}workspace": str(tmp_path / "work"), + f"{P}build_host__address": HOST.address, + f"{P}build_host__user": HOST.user, + f"{P}build_host__port": str(HOST.port), + f"{P}build_host__key": str(HOST._key), + }, **changes, ), SECRETS, @@ -397,8 +399,63 @@ def test_the_new_loop_assembles(tmp_path: Path, origin: Path, monkeypatch) -> No running = build_loop_agent(config) try: assert running.account == "nisse-dev-testing" - assert running.loop._repo == config.gitea_repo + [project] = running.loop._projects.values() + assert project.repo == config.gitea_repo assert (config.workspace / "README.md").exists() # cloned on the build host finally: running.worker.call(running.store.close) running.worker.stop(timeout=5) + + +def test_each_project_gets_its_own_clone_and_secrets( + tmp_path: Path, origin: Path, monkeypatch +) -> None: + """D8: a second project is only a second entry.""" + from nisse_dev.loop.build import build_loop_agent + + second = tmp_path / "second" + config = config_for( + tmp_path, + loop="new", + projects__other={ + "repo": "don/other", + "workspace": str(tmp_path / "other-work"), + "build_host": { + "address": HOST.address, + "user": HOST.user, + "port": str(HOST.port), + "key": str(HOST._key), + }, + "labels": ["discuss"], + "secrets": { + "API_KEY": {"value": "${TEST_LLM_TOKEN}", "visible": "commands"} + }, + }, + ) + git("clone", "-q", "--bare", str(origin), str(second)) + origins = {"don/demo": str(origin), "don/other": str(second)} + monkeypatch.setattr( + "nisse_dev.loop.build.GiteaClient.whoami", lambda self: "nisse-dev-testing" + ) + monkeypatch.setattr( + "nisse_dev.loop.build.GiteaClient.clone_url", lambda self, repo: origins[repo] + ) + monkeypatch.setattr( + "nisse_dev.agent.ModelCatalog.discover", + classmethod(lambda cls, *a, **kw: TEST_CATALOG), + ) + monkeypatch.setattr( + "nisse_dev.loop.build.MatrixTransport", lambda cfg: FakeTransport() + ) + running = build_loop_agent(config) + try: + projects = running.loop._projects + assert set(projects) == {"greet", "other"} + assert (tmp_path / "work" / "README.md").exists() + assert (tmp_path / "other-work" / "README.md").exists() + assert projects["other"].labels == ("discuss",) + assert projects["other"].workspace._secrets == {"API_KEY": "sk-x"} + assert projects["greet"].workspace._secrets == {} + finally: + running.worker.call(running.store.close) + running.worker.stop(timeout=5) diff --git a/nisse-dev/tests/test_config.py b/nisse-dev/tests/test_config.py index da1b1a8..45f24e0 100644 --- a/nisse-dev/tests/test_config.py +++ b/nisse-dev/tests/test_config.py @@ -10,8 +10,13 @@ from nisse_dev.llm import DEFAULT_REQUEST_TIMEOUT_S # A complete config as the YAML file would give it, with secrets by reference. COMPLETE: dict[str, Any] = { "state_db": "~/demo/state.db", - "workspace": "~/demo/work", - "project": "greet", + "projects": { + "greet": { + "repo": "don/demo", + "workspace": "~/demo/work", + "build_host": {"address": "build.test", "key": "~/.ssh/nisse-dev"}, + } + }, "llm": {"base_url": "https://litellm.test", "token": "${TEST_LLM_TOKEN}"}, "roles": { "executor": "claude-haiku-4-5", @@ -29,11 +34,10 @@ COMPLETE: dict[str, Any] = { "gitea": { "url": "https://gitea.test", "token": "${TEST_GITEA_TOKEN}", - "repo": "don/demo", "operators": ["don"], }, - "build_host": {"address": "build.test", "key": "~/.ssh/nisse-dev"}, } +P = "projects__greet__" # the one project's keys, for tree() SECRETS = { "TEST_LLM_TOKEN": "sk-x", "TEST_MATRIX_TOKEN": "syt_x", @@ -83,15 +87,15 @@ def test_every_missing_setting_is_reported_at_once() -> None: # Failing at first use would surface a missing token twenty minutes into # a milestone, on the call that mattered. with pytest.raises(ConfigError) as excinfo: - build(matrix__token=None, gitea__repo=None) + build(matrix__token=None, **{f"{P}repo": None}) message = str(excinfo.value) assert "matrix.token" in message - assert "gitea.repo" in message + assert "projects.greet: unset or empty — repo" in message def test_empty_is_treated_as_missing() -> None: - with pytest.raises(ConfigError, match="gitea.repo"): - build(gitea__repo=" ") + with pytest.raises(ConfigError, match="projects.greet: unset or empty — repo"): + build(**{f"{P}repo": " "}) def test_a_reference_to_an_unset_variable_is_an_error() -> None: @@ -115,7 +119,7 @@ def test_an_operator_list_of_only_blanks_is_missing() -> None: def test_base_branch_is_configurable() -> None: - assert build(gitea__base_branch="trunk").base_branch == "trunk" + assert build(**{f"{P}base_branch": "trunk"}).base_branch == "trunk" def test_max_output_tokens_is_configurable_and_checked() -> None: @@ -148,7 +152,7 @@ def test_redacted_summary_carries_no_secrets() -> None: flat = " ".join(summary.values()) for secret in SECRETS.values(): assert secret not in flat - assert summary["project"] == "greet" + assert "greet (don/demo, build.test:" in summary["projects"] def test_tuning_is_read() -> None: @@ -178,7 +182,7 @@ def test_a_partly_given_tuning_section_keeps_the_other_defaults() -> None: def test_every_problem_is_reported_at_once() -> None: # A misspelt knob that was silently ignored would look exactly like a knob # with no effect, so unknown keys are errors. - bad = tree(project=None, llm_extra=1) + bad = tree(llm_extra=1, **{f"{P}workspace": None}) bad["tuning"] = { "executor": {"max_turn": 40, "failure_limit": 0}, "expert": {"consult_budget": True}, @@ -188,7 +192,7 @@ def test_every_problem_is_reported_at_once() -> None: AgentConfig.from_tree(bad, SECRETS) message = str(excinfo.value) for expected in ( - "project", + "projects.greet: unset or empty — workspace", "llm_extra: unknown setting", "tuning.executor.max_turn: unknown setting", "tuning.executor.failure_limit: must be a positive number", @@ -207,8 +211,13 @@ def test_a_whole_number_is_required_where_one_is_meant() -> None: FILE = """ state_db: ~/demo/state.db -workspace: ~/demo/work -project: greet +projects: + greet: + repo: don/demo + workspace: ~/demo/work + build_host: + address: build.test + key: ~/.ssh/nisse-dev llm: {base_url: "https://litellm.test", token: "${TEST_LLM_TOKEN}"} roles: executor: local-fast @@ -224,11 +233,7 @@ matrix: gitea: url: https://gitea.test token: ${TEST_GITEA_TOKEN} - repo: don/demo operators: [don] -build_host: - address: build.test - key: ~/.ssh/nisse-dev """ @@ -297,7 +302,7 @@ def test_state_db_inside_the_clone_is_refused(tmp_path: Path) -> None: # own state alongside the project. work = tmp_path / "project" with pytest.raises(ConfigError, match="inside the workspace"): - build(workspace=str(work), state_db=str(work / "state.db")) + build(state_db=str(work / "state.db"), **{f"{P}workspace": str(work)}) def test_env_file_inside_the_clone_is_refused(tmp_path: Path) -> None: @@ -306,7 +311,7 @@ def test_env_file_inside_the_clone_is_refused(tmp_path: Path) -> None: work = tmp_path / "project" with pytest.raises(ConfigError, match="NISSE_ENV"): AgentConfig.from_tree( - tree(workspace=str(work), state_db=str(tmp_path / "state.db")), + tree(state_db=str(tmp_path / "state.db"), **{f"{P}workspace": str(work)}), {**SECRETS, "NISSE_ENV": str(work / ".env")}, ) @@ -315,13 +320,17 @@ def test_a_config_file_inside_the_clone_is_refused(tmp_path: Path) -> None: work = tmp_path / "project" work.mkdir() text = FILE.replace("workspace: ~/demo/work", f"workspace: {work}") + assert text != FILE with pytest.raises(ConfigError, match="NISSE_CONFIG"): AgentConfig.from_file(write(work, text), SECRETS) def test_sibling_paths_are_fine(tmp_path: Path) -> None: config = AgentConfig.from_tree( - tree(workspace=str(tmp_path / "project"), state_db=str(tmp_path / "state.db")), + tree( + state_db=str(tmp_path / "state.db"), + **{f"{P}workspace": str(tmp_path / "project")}, + ), {**SECRETS, "NISSE_ENV": str(tmp_path / ".env")}, ) assert config.workspace.name == "project" @@ -342,30 +351,36 @@ def test_the_build_host_is_required_and_defaults_to_root_on_22() -> None: assert config.build_host.port == 22 assert config.build_host.key.is_absolute() with pytest.raises(ConfigError, match="build_host.address"): - build(build_host__address=None) + build(**{f"{P}build_host__address": None}) def test_a_bad_build_host_port_is_reported() -> None: with pytest.raises(ConfigError, match="build_host.port"): - build(build_host__port="ssh") + build(**{f"{P}build_host__port": "ssh"}) def test_the_push_key_is_optional_and_a_path() -> None: assert build().push_key is None - assert build(gitea__push_key="~/k").push_key == Path("~/k").expanduser() + assert build(**{f"{P}push_key": "~/k"}).push_key == Path("~/k").expanduser() def test_a_push_token_needs_its_user_and_excludes_a_key() -> None: - config = build(gitea__push_user="pusher", gitea__push_token="${TEST_GITEA_TOKEN}") + token = {f"{P}push_user": "pusher", f"{P}push_token": "${TEST_GITEA_TOKEN}"} + config = build(**token) assert (config.push_user, config.push_token) == ("pusher", "gt") with pytest.raises(ConfigError, match="set both or neither"): - build(gitea__push_user="pusher") + build(**{f"{P}push_user": "pusher"}) with pytest.raises(ConfigError, match="not both"): - build(gitea__push_user="p", gitea__push_token="t", gitea__push_key="~/k") + build(**token, **{f"{P}push_key": "~/k"}) def test_no_credential_appears_in_the_config_repr() -> None: - config = build(gitea__push_user="pusher", gitea__push_token="${TEST_GITEA_TOKEN}") + config = build( + **{f"{P}push_user": "pusher", f"{P}push_token": "${TEST_GITEA_TOKEN}"}, + **{ + f"{P}secrets": {"DB": {"value": "${TEST_LLM_TOKEN}", "visible": "commands"}} + }, + ) text = repr(config) for secret in SECRETS.values(): assert secret not in text @@ -376,3 +391,90 @@ def test_the_loop_defaults_to_old_and_takes_new() -> None: assert build(loop="new").loop == "new" with pytest.raises(ConfigError, match="loop: must be old or new"): build(loop="newer") + + +# --- projects (D8) ----------------------------------------------------------- + +SECOND = { + "repo": "don/other", + "workspace": "~/demo/other", + "build_host": {"address": "build2.test", "key": "~/.ssh/nisse-dev"}, +} + + +def test_a_second_project_is_a_second_entry() -> None: + config = build(projects__other=SECOND) + assert [p.name for p in config.projects] == ["greet", "other"] + other = config.projects[1] + assert (other.repo, other.build_host.address) == ("don/other", "build2.test") + assert other.base_branch == "main" and other.labels == () and other.secrets == {} + + +def test_the_old_loop_refuses_more_than_one_project() -> None: + config = build(projects__other=SECOND) + with pytest.raises(ConfigError, match="exactly one project"): + config.only_project() + with pytest.raises(ConfigError, match="exactly one project"): + _ = config.gitea_repo + + +def test_at_least_one_project_is_needed() -> None: + with pytest.raises(ConfigError, match="projects: must name at least one"): + build(projects={}) + + +def test_project_problems_are_named_by_project() -> None: + bad = tree(projects__other={**SECOND, "repo": "don/demo", "colour": "red"}) + with pytest.raises(ConfigError) as excinfo: + AgentConfig.from_tree(bad, SECRETS) + message = str(excinfo.value) + assert "projects.other.colour: unknown setting" in message + assert "projects.other.repo: don/demo is also project greet's" in message + + +def test_a_project_name_is_a_slug() -> None: + with pytest.raises(ConfigError, match="lowercase letters, digits"): + build(projects__Other_Project=SECOND) + + +def test_labels_and_egress_are_lists_of_names() -> None: + config = build(**{f"{P}labels": ["discuss"], f"{P}egress": ["pypi.org"]}) + assert config.only_project().labels == ("discuss",) + assert config.only_project().egress == ("pypi.org",) + with pytest.raises(ConfigError, match="projects.greet.labels: must be a list"): + build(**{f"{P}labels": "discuss"}) + + +def test_secrets_are_by_reference_with_a_visibility() -> None: + config = build( + **{ + f"{P}secrets": { + "DB_URL": {"value": "${TEST_LLM_TOKEN}", "visible": "commands"}, + "DEPLOY": {"value": "${TEST_GITEA_TOKEN}", "visible": "harness"}, + } + } + ) + project = config.only_project() + assert project.command_secrets() == {"DB_URL": "sk-x"} + assert project.secrets["DEPLOY"].visible == "harness" + + +def test_a_secret_written_inline_is_refused() -> None: + with pytest.raises(ConfigError, match="must be a \\$\\{VAR\\} reference"): + build(**{f"{P}secrets": {"DB": {"value": "hunter2", "visible": "commands"}}}) + + +def test_a_secret_needs_a_known_visibility() -> None: + with pytest.raises(ConfigError, match="visible: must be one of commands, harness"): + build( + **{f"{P}secrets": {"DB": {"value": "${TEST_LLM_TOKEN}", "visible": "all"}}} + ) + + +def test_a_workspace_of_any_project_inside_is_refused(tmp_path: Path) -> None: + work = tmp_path / "other" + with pytest.raises(ConfigError, match="project other"): + build( + state_db=str(work / "state.db"), + projects__other={**SECOND, "workspace": str(work)}, + ) diff --git a/nisse-dev/tests/test_loop_labels.py b/nisse-dev/tests/test_loop_labels.py index 44c39b8..aebf638 100644 --- a/nisse-dev/tests/test_loop_labels.py +++ b/nisse-dev/tests/test_loop_labels.py @@ -280,3 +280,32 @@ def _msg(text: str, thread: str) -> Any: from nisse_dev.transport import InboundMessage return InboundMessage(DON, text, thread=thread) + + +def test_a_restatement_that_asks_is_sent_back(tmp_path: Path) -> None: + """A restatement is agreed to as it stands; one ending in a question + (models add "Is this right?") never goes up.""" + h = Harness( + tmp_path, + ScriptedModel( + [call("restate", text="I'll add count_words. Is this right?")], + [call("restate", text="I'll add count_words to text.py, with tests.")], + ), + FakeGitea(issue()), + ) + h.agent.intake() + loop = loop_of(h.item()) + assert (loop.wait, loop.rev) == (GO_AHEAD, 1) + posted = [p for p in h.posted() if p.startswith("rev-")] + assert posted == [ + ( + "rev-1: I'll add count_words to text.py, with tests.\n\n" + "Say go to start, or reply with changes." + ) + ] + results = [ + e.event.result + for e in h.store.transcript(h.item().id, "driver").read() + if hasattr(e.event, "result") + ] + assert any("a restatement asks no questions" in r for r in results) diff --git a/nisse-dev/tests/test_loop_milestones.py b/nisse-dev/tests/test_loop_milestones.py index 4e32e0a..16d9888 100644 --- a/nisse-dev/tests/test_loop_milestones.py +++ b/nisse-dev/tests/test_loop_milestones.py @@ -157,6 +157,18 @@ class Forge: @pytest.fixture def world(ssh_host: SSHHost, tmp_path: Path) -> dict[str, Any]: + return make_world(ssh_host, tmp_path) + + +def make_world( + ssh_host: SSHHost, + tmp_path: Path, + store: StateStore | None = None, + repo_name: str = "don/sandbox", +) -> dict[str, Any]: + """A project's forge (a bare repository and a fake Gitea over it), its + clone and milestones; a second one can share the first's store.""" + tmp_path.mkdir(parents=True, exist_ok=True) bare = tmp_path / "forge.git" git("init", "-q", "--bare", "-b", "main", str(bare)) seed = tmp_path / "seed" @@ -179,13 +191,16 @@ def world(ssh_host: SSHHost, tmp_path: Path) -> dict[str, Any]: repo.clone(str(bare)) repo.configure_identity("nisse-dev", "n@t") repo.install_excludes() - store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())) + if store is None: + store = StateStore( + tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO()) + ) forge = Forge(bare) milestones = Milestones( store, repo, - forge, - repo="don/sandbox", # type: ignore[arg-type] + forge, # type: ignore[arg-type] + repo=repo_name, remote=str(bare), credential=None, ) @@ -197,6 +212,7 @@ def world(ssh_host: SSHHost, tmp_path: Path) -> dict[str, Any]: "host": ssh_host, "bare": bare, "seed": seed, + "repo_name": repo_name, } diff --git a/nisse-dev/tests/test_loop_projects.py b/nisse-dev/tests/test_loop_projects.py new file mode 100644 index 0000000..883a33a --- /dev/null +++ b/nisse-dev/tests/test_loop_projects.py @@ -0,0 +1,188 @@ +"""Several projects on the new loop (M1.4, D8): intake from each project's +repository, each item working in its own project's workspace and publishing +to its own repository, issue numbers told apart, label defaults, and parking +across projects. Real git on the throwaway build host; fake Gitea per repo.""" + +import io +from pathlib import Path +from typing import Any + +import pytest +from test_loop_milestones import DON, Forge, ScriptedModel, call, git, make_world + +from nisse_dev.cancel import Cancel +from nisse_dev.execution import SSHHost +from nisse_dev.loop.agent import LoopAgent, Project +from nisse_dev.loop.driver import Driver +from nisse_dev.loop.items import ANSWER, GO_AHEAD, MERGE, item_for_issue, loop_of +from nisse_dev.oplog import OpLog +from nisse_dev.tools import Workspace +from nisse_dev.transport import InMemoryTransport +from nisse_dev.worker import InlineWorker + +REPOS = {"alpha": "don/alpha", "beta": "don/beta"} + + +class Router: + """One Gitea for the agent, routing each call to its repository's fake.""" + + def __init__(self, forges: dict[str, Forge]) -> None: + self.forges = forges + + def __getattr__(self, name: str) -> Any: + def call(repo: str, *args: Any, **kwargs: Any) -> Any: + return getattr(self.forges[repo], name)(repo, *args, **kwargs) + + return call + + +@pytest.fixture +def worlds(ssh_host: SSHHost, tmp_path: Path) -> dict[str, dict[str, Any]]: + alpha = make_world(ssh_host, tmp_path / "alpha", repo_name=REPOS["alpha"]) + beta = make_world( + ssh_host, tmp_path / "beta", store=alpha["store"], repo_name=REPOS["beta"] + ) + return {"alpha": alpha, "beta": beta} + + +def agent_for( + worlds: dict[str, dict[str, Any]], + model: ScriptedModel, + labels: dict[str, tuple[str, ...]] | None = None, +) -> tuple[LoopAgent, InMemoryTransport]: + transport = InMemoryTransport() + cancel = Cancel() + router = Router({REPOS[n]: w["forge"] for n, w in worlds.items()}) + projects = [ + Project( + name=n, + repo=REPOS[n], + workspace=Workspace(w["host"], w["repo"].root, timeout_s=30), + milestones=w["milestones"], + labels=(labels or {}).get(n, ()), + ) + for n, w in worlds.items() + ] + for w in worlds.values(): + w["milestones"]._gitea = router # publishing goes through the router too + agent = LoopAgent( + worlds["alpha"]["store"], + transport, + router, # type: ignore[arg-type] + projects=projects, + account="nisse-dev", + operators={DON}, + gitea_operators={"don"}, + driver=Driver(model, "m", cancel=cancel), # type: ignore[arg-type] + worker=InlineWorker(), + cancel=cancel, + oplog=OpLog("nisse-dev", io.StringIO()), + ) + transport.on_message(agent.on_message) + return agent, transport + + +def item(worlds: dict[str, Any], project: str) -> Any: + found = item_for_issue(worlds["alpha"]["store"], 14, project) + assert found is not None + return found + + +def say(transport: InMemoryTransport, worlds: Any, project: str, text: str) -> None: + transport.receive(DON, text, thread=loop_of(item(worlds, project)).thread) + + +def restate(name: str) -> list[Any]: + return [call("restate", text=f"{name}: add count_words")] + + +def work(path: str) -> list[Any]: + return [ + call("start_milestone", name=path), + call("write_file", path=path, content=f"# {path}\n"), + call("finish_milestone", summary=path, final="yes"), + ] + + +def test_both_projects_issues_are_taken_and_told_apart(worlds: Any) -> None: + # Both forges have an issue #14. + agent, transport = agent_for(worlds, ScriptedModel(restate("a"), restate("b"))) + agent.intake() + a, b = item(worlds, "alpha"), item(worlds, "beta") + assert (a.scope, b.scope) == ("alpha", "beta") + said = [m.text for m in transport.outbound] + assert any(t.startswith("Queued don/alpha#14") for t in said) + assert any(t.startswith("Queued don/beta#14") for t in said) + status = agent.status() + assert "alpha#14 Add count_words" in status and "beta#14 Add count_words" in status + + +def test_each_item_works_and_publishes_in_its_own_project(worlds: Any) -> None: + model = ScriptedModel(restate("a"), restate("b"), work("a.py"), work("b.py")) + agent, transport = agent_for(worlds, model) + agent.intake() + say(transport, worlds, "alpha", "go") + say(transport, worlds, "beta", "go") + fa, fb = worlds["alpha"]["forge"], worlds["beta"]["forge"] + [pa], [pb] = fa.prs, fb.prs + assert fa.files(pa) == ["a.py"] and fb.files(pb) == ["b.py"] + assert fa.prs[pa]["head"].startswith(item(worlds, "alpha").id) + assert fb.prs[pb]["head"].startswith(item(worlds, "beta").id) + # Each workspace has only its own project's work. + assert not (worlds["alpha"]["repo"].root / "b.py").exists() + assert loop_of(item(worlds, "beta")).wait == MERGE + + +def test_a_bare_number_in_two_projects_is_asked_about(worlds: Any) -> None: + model = ScriptedModel(restate("a"), restate("b"), work("b.py")) + agent, transport = agent_for(worlds, model) + agent.intake() + transport.receive(DON, "go #14") + asked = transport.outbound[-1].text + assert "open in more than one project" in asked + assert "go alpha#14" in asked and "go beta#14" in asked + assert not loop_of(item(worlds, "alpha")).started + transport.receive(DON, "go beta#14") + assert loop_of(item(worlds, "beta")).started + assert not loop_of(item(worlds, "alpha")).started + + +def test_do_next_names_a_project(worlds: Any) -> None: + agent, transport = agent_for(worlds, ScriptedModel(restate("a"), restate("b"))) + agent.intake() + transport.receive(DON, "do #14 next") + assert "do alpha#14 next or do beta#14 next" in transport.outbound[-1].text + transport.receive(DON, "do beta#14 next") + assert "beta#14 goes first" in transport.outbound[-1].text + + +def test_a_projects_label_defaults_steer_its_items(worlds: Any) -> None: + model = ScriptedModel( + [call("ask_human", question="what's it for?")], # alpha: discuss first + restate("b"), + ) + agent, _ = agent_for(worlds, model, labels={"alpha": ("discuss",)}) + agent.intake() + assert loop_of(item(worlds, "alpha")).labels == ("discuss",) + assert loop_of(item(worlds, "alpha")).wait == ANSWER + assert loop_of(item(worlds, "beta")).wait == GO_AHEAD + + +def test_switching_projects_parks_the_other_projects_work(worlds: Any) -> None: + model = ScriptedModel( + restate("a"), + restate("b"), + [ # alpha: half done, then a question + call("start_milestone", name="a"), + call("write_file", path="half.py", content="H = 1\n"), + call("ask_human", question="one more thing?"), + ], + work("b.py"), # beta runs meanwhile + ) + agent, transport = agent_for(worlds, model) + agent.intake() + say(transport, worlds, "alpha", "go") + say(transport, worlds, "beta", "go") + bare = ["--git-dir", str(worlds["alpha"]["bare"])] + parked = f"refs/heads/{item(worlds, 'alpha').id}-parked" + assert git(*bare, "show", f"{parked}^3:half.py") == "H = 1" diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index d47930f..2e072fa 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/nisse-dev/tests/test_loop_properties.py @@ -15,13 +15,15 @@ from typing import Any import pytest from model_contract import assert_renderable -from test_loop_milestones import DON, Forge, call +from test_loop_milestones import DON, Forge, call, make_world from test_loop_milestones import world as world # noqa: PLC0414 -- the fixture +from test_loop_projects import Router from nisse_dev.cancel import Cancel from nisse_dev.canonical import Fact, PlanSubmitted, ToolCall, UserMessage +from nisse_dev.execution import SSHHost from nisse_dev.gitea import Comment, Issue -from nisse_dev.loop.agent import LoopAgent +from nisse_dev.loop.agent import LoopAgent, Project from nisse_dev.loop.driver import Driver from nisse_dev.loop.items import ( ANSWER, @@ -38,6 +40,7 @@ from nisse_dev.transport import InboundMessage, InMemoryTransport from nisse_dev.worker import InlineWorker SEEDS = range(32) +TWO_PROJECT_SEEDS = range(100, 116) STEPS = 30 @@ -88,13 +91,17 @@ class PolicyModel: class Walk: - def __init__(self, world: dict[str, Any], seed: int) -> None: - self.world = world + """``worlds``: project name -> its world (forge, bare repo, clone, + milestones), all over one store.""" + + def __init__(self, worlds: dict[str, dict[str, Any]], seed: int) -> None: + self.worlds = worlds + self.world = next(iter(worlds.values())) # the store's home self.rng = random.Random(seed) self.seed = seed self.steps: list[str] = [] self.shown_at_go: list[str] = [] # item:revision on screen at each go - self.next_issue = 15 + self.next_issue = dict.fromkeys(worlds, 15) # numbers repeat across projects self.now = 1000.0 self.agent, self.transport = self._agent() @@ -106,22 +113,29 @@ class Walk: def _agent(self) -> tuple[LoopAgent, InMemoryTransport]: transport = InMemoryTransport() cancel = Cancel() - w = self.world + router = Router({w["repo_name"]: w["forge"] for w in self.worlds.values()}) + for w in self.worlds.values(): + w["milestones"]._gitea = router agent = LoopAgent( - w["store"], + self.world["store"], transport, - w["forge"], - repo="don/sandbox", - project="sandbox", + router, # type: ignore[arg-type] + projects=[ + Project( + name=name, + repo=w["repo_name"], + workspace=Workspace(w["host"], w["repo"].root, timeout_s=30), + milestones=w["milestones"], + ) + for name, w in self.worlds.items() + ], account="nisse-dev", operators={DON}, gitea_operators={"don"}, driver=Driver(PolicyModel(self.rng), "m", cancel=cancel), # type: ignore[arg-type] - workspace=Workspace(w["host"], w["repo"].root, timeout_s=30), worker=InlineWorker(), cancel=cancel, oplog=OpLog("nisse-dev", _Null()), - milestones=w["milestones"], reader=self, # type: ignore[arg-type] bug_grace_s=600, clock=lambda: self.now, @@ -135,14 +149,21 @@ class Walk: def items(self) -> list[Any]: return self.world["store"].work_items() + def name(self, item: Any) -> str: + """How the operator names an item: by project when there are two.""" + issue = loop_of(item).issue + return f"#{issue}" if len(self.worlds) == 1 else f"{item.scope}#{issue}" + def step(self) -> None: - forge: Forge = self.world["forge"] open_items = [i for i in self.items() if i.status == "open"] # Weighted towards what an item can actually do, so review paths # are walked often, not drowned out by intake and crashes. weights = {"intake": 2 if not open_items else 1, "crash": 1, "issue": 1} item = self.rng.choice(open_items) if open_items else None loop = loop_of(item) if item else None + # The forge an item's project lives on; a new issue picks one. + project = item.scope if item else self.rng.choice(list(self.worlds)) + forge: Forge = self.worlds[project]["forge"] if item and loop: weights.update({"go": 3, "answer": 2, "abandon": 1, "go-stale": 1}) weights.update({"first": 1, "plain-yes": 2, "named-go": 1, "wait": 2}) @@ -151,12 +172,15 @@ class Walk: {"comment": 4, "merge": 2, "close-pr": 1, "comment-merge": 2} ) action = self.rng.choices(list(weights), list(weights.values()))[0] - self.steps.append(f"{action}#{loop.issue}" if loop else action) + self.steps.append(f"{action}{self.name(item)}" if item else action) if action == "intake": self.agent.intake() elif action == "issue": # A new issue, sometimes depending on one that is still open. - n, self.next_issue = self.next_issue, self.next_issue + 1 + project = self.rng.choice(list(self.worlds)) + forge = self.worlds[project]["forge"] + n = self.next_issue[project] + self.next_issue[project] += 1 labels = self.rng.choice([(), (), ("bug",), ("discuss",), ("design",)]) forge.issues.append(Issue(n, f"task {n}", f"do task {n}", "don", labels)) live = [i.number for i in forge.assigned_issues("", "") if i.number != n] @@ -171,7 +195,7 @@ class Walk: elif action == "named-go" and loop: if item.position == WAITING and loop.wait == GO_AHEAD: self.shown_at_go.append(f"{item.id}:{loop.rev}") - self.transport.receive(DON, f"go #{loop.issue}") + self.transport.receive(DON, f"go {self.name(item)}") elif action == "wait": # Time passes: bug countdowns may run out. self.now += self.rng.choice([60, 700]) @@ -185,7 +209,7 @@ class Walk: self.shown_at_go.append(f"{other.id}:{o.rev}") self.agent.intake() elif action == "first" and loop: - self.agent.handle(InboundMessage(DON, f"do #{loop.issue} next"), None) + self.agent.handle(InboundMessage(DON, f"do {self.name(item)} next"), None) elif action == "crash": # Volatile state is lost: a new agent over the same store. self.agent, self.transport = self._agent() @@ -231,12 +255,14 @@ class Walk: def check(self) -> None: where = f"seed {self.seed}, steps {self.steps}" - store, forge = self.world["store"], self.world["forge"] + store = self.world["store"] items = self.items() - issues = [loop_of(i).issue for i in items if loop_of(i).issue] + issues = [(i.scope, loop_of(i).issue) for i in items if loop_of(i).issue] assert len(issues) == len(set(issues)), f"one_item_per_issue: {where}" for item in items: loop = loop_of(item) + forge = self.worlds[item.scope]["forge"] + repo = self.worlds[item.scope]["repo_name"] if item.status == "open" and item.position == WAITING: explained = ( (loop.wait == GO_AHEAD and loop.rev > 0) @@ -257,7 +283,7 @@ class Walk: ) # Blocked comes from external truth (L1): an item whose issue # had an open dependency from the start never ran before it closed. - if forge.open_dependencies("don/sandbox", loop.issue): + if forge.open_dependencies(repo, loop.issue): assert item.position == BLOCKED or item.status == "closed", ( f"blocked_never_runs: #{loop.issue} {item.position}: {where}" ) @@ -311,11 +337,30 @@ class Walk: assert not [i for i in items if i.position == RUNNING], ( f"one_active (nothing left running): {where}" ) - self._parked_work_safe(where) - self._milestones_start_fresh(where) + for name, w in self.worlds.items(): + self._own_repository_only(name, w, where) + self._parked_work_safe(w, name, where) + self._milestones_start_fresh(w, where) + self._review_fixes_land(w, where) + + def _own_repository_only(self, name: str, world: Any, where: str) -> None: + """D8: an item's branches and PRs reach only its own project's + repository.""" + others = [i.id for i in self.items() if i.scope != name] + heads = [p["head"] for p in world["forge"].prs.values()] + branches = _git( + world["bare"], "for-each-ref", "--format=%(refname:short)", "refs/heads" + ).split() + for ref in [*heads, *branches]: + assert not any(ref.startswith(f"{i}-") for i in others), ( + f"own_repository_only: {ref} in {name}: {where}" + ) + + def _review_fixes_land(self, world: Any, where: str) -> None: + forge: Forge = world["forge"] heads = [p["head"] for p in forge.prs.values()] assert len(heads) == len(set(heads)), f"one_pr_per_milestone: {where}" - bare = self.world["bare"] + bare = world["bare"] for n, pr in forge.prs.items(): if not pr["merged"]: continue @@ -330,11 +375,11 @@ class Walk: ) assert landed, f"review_fixes_land (PR {n}): {where}" - def _milestones_start_fresh(self, where: str) -> None: + def _milestones_start_fresh(self, world: Any, where: str) -> None: """D4: a milestone starts from the latest default branch, so one redone after a closed PR never carries the rejected work.""" - forge: Forge = self.world["forge"] - bare = self.world["bare"] + forge: Forge = world["forge"] + bare = world["bare"] for item in self.items(): mine = sorted( (n, p) @@ -353,13 +398,13 @@ class Walk: bare, rejected, f"refs/heads/{later['head']}" ), f"milestones_start_fresh: PR {m} carries PR {n}: {where}" - def _parked_work_safe(self, where: str) -> None: + def _parked_work_safe(self, world: Any, name: str, where: str) -> None: """L1, D4: nothing of a parked item lives only on the build VM: what every branch of an open item that isn't on the workspace points at is - in the forge.""" - root, bare = self.world["repo"].root, self.world["bare"] + in its project's forge.""" + root, bare = world["repo"].root, world["bare"] current = _git(root, "rev-parse", "--abbrev-ref", "HEAD") - ids = [i.id for i in self.items() if i.status == "open"] + ids = [i.id for i in self.items() if i.status == "open" and i.scope == name] for line in _git( root, "for-each-ref", "--format=%(refname:short)", "refs/heads" ).splitlines(): @@ -384,7 +429,7 @@ class Walk: # And no write is lost: every file an open item's driver wrote is on # the workspace while it's that item's, or in the forge. for item in self.items(): - if item.status != "open": + if item.status != "open" or item.scope != name: continue written = { str(e.event.arguments.get("path")) @@ -444,7 +489,22 @@ def _is_ancestor(bare: Path, commit: str, ref: str) -> bool: @pytest.mark.parametrize("seed", SEEDS) def test_the_loop_keeps_its_properties(world: dict[str, Any], seed: int) -> None: - walk = Walk(world, seed) + walk = Walk({"sandbox": world}, seed) + for _ in range(STEPS): + walk.step() + walk.check() + + +@pytest.mark.parametrize("seed", TWO_PROJECT_SEEDS) +def test_two_projects_keep_the_properties( + ssh_host: SSHHost, tmp_path: Path, seed: int +) -> None: + """D8: the same walk over two projects, whose issue numbers collide.""" + sandbox = make_world(ssh_host, tmp_path / "sandbox") + other = make_world( + ssh_host, tmp_path / "other", store=sandbox["store"], repo_name="don/other" + ) + walk = Walk({"sandbox": sandbox, "other": other}, seed) for _ in range(STEPS): walk.step() walk.check() diff --git a/tests/integration/nisse-test.yaml b/tests/integration/nisse-test.yaml index 03e515e..ea0d2e5 100644 --- a/tests/integration/nisse-test.yaml +++ b/tests/integration/nisse-test.yaml @@ -7,15 +7,23 @@ # the live tests that need it, rather than skipping them, so a broken # credential cannot hide as a skip. -# Unused by the live tests, which work in pytest's temporary directories on a -# throwaway local sshd (the `ssh_host` fixture); present because every agent -# config has them. +# Unused by the live tests, which work in pytest's temporary directories; +# present because every agent config has it. state_db: /tmp/nisse-test/state.db -workspace: /tmp/nisse-test/work -project: nisse-dev-testing -build_host: - address: 127.0.0.1 - key: /tmp/nisse-test/unused-key + +projects: + nisse-dev-testing: + repo: dongar/nisse-dev-testing-sandbox # the sandbox, never a real project + # The live tests replace these with a throwaway local sshd (the `ssh_host` + # fixture) or the bring-up build VM, in temporary directories. + workspace: /tmp/nisse-test/work + build_host: + address: 127.0.0.1 + key: /tmp/nisse-test/unused-key + # What pushes go by (D4): the sandbox's deploy key, whose private half CI + # writes to this path from the NISSE_PUSH_KEY secret (locally: copy it + # there). An HTTPS push token works too: push_user plus push_token. + push_key: /tmp/nisse-test/push_key llm: base_url: https://litellm.dongar.net @@ -40,11 +48,6 @@ matrix: gitea: url: https://gitea.dongar.net token: ${NISSE_GITEA_TOKEN} - repo: dongar/nisse-dev-testing-sandbox # the sandbox, never a real project # nisse-testing-operator plays the operator in the end-to-end tests # (NISSE_OPERATOR_TOKEN); an operator here and nowhere else. operators: [dongar, nisse-testing-operator] - # What pushes go by (D4): the sandbox's deploy key, whose private half CI - # writes to this path from the NISSE_PUSH_KEY secret (locally: copy it - # there). An HTTPS push token works too: push_user plus push_token. - push_key: /tmp/nisse-test/push_key diff --git a/tests/integration/test_end_to_end.py b/tests/integration/test_end_to_end.py index 22abe53..9185fa1 100644 --- a/tests/integration/test_end_to_end.py +++ b/tests/integration/test_end_to_end.py @@ -74,11 +74,12 @@ def agent( operator = operator_gitea(live_config.gitea_url) base = stack.enter_context(base_branch(operator, live_config.gitea_repo, run)) config = replace( - live_config, - base_branch=base, + live_config.with_project( + base_branch=base, + workspace=Path(f"/tmp/nisse-e2e-{run}/sandbox"), + build_host=build_host(ssh_host), + ), state_db=tmp_path / "state.db", - workspace=Path(f"/tmp/nisse-e2e-{run}/sandbox"), - build_host=build_host(ssh_host), ) transport = InMemoryTransport() monkeypatch.setattr("nisse_dev.agent.MatrixTransport", lambda cfg: transport) diff --git a/tests/integration/test_end_to_end_loop.py b/tests/integration/test_end_to_end_loop.py index 91c6c0f..0440942 100644 --- a/tests/integration/test_end_to_end_loop.py +++ b/tests/integration/test_end_to_end_loop.py @@ -65,12 +65,13 @@ def running( operator = operator_gitea(live_config.gitea_url) base = stack.enter_context(base_branch(operator, live_config.gitea_repo, run)) config = replace( - live_config, + live_config.with_project( + base_branch=base, + workspace=Path(f"/tmp/nisse-e2e-{run}/sandbox"), + build_host=build_host(ssh_host), + ), loop="new", - base_branch=base, state_db=keep / "state.db", - workspace=Path(f"/tmp/nisse-e2e-{run}/sandbox"), - build_host=build_host(ssh_host), # Short, so a bug's grace period runs out within the test. tuning=replace(live_config.tuning, loop=LoopTuning(bug_grace_s=BUG_GRACE_S)), )