Merge pull request 'M7.6: nisse-review's image' (#171) from review-image into main

.gitea/workflows/release.yml

@@ -1,7 +1,11 @@
 name: Release
-# A published release builds the agent's image and pushes it to Gitea's
-# registry (Development Plan M1.5, X4): gitea.dongar.net/dev/nisse-dev
-# tagged with the version (the release tag without its v) and latest. Cut a
+# A published release builds every agent's image and pushes it to Gitea's
+# registry (Development Plan M1.5, M7.6, X4): gitea.dongar.net/dev/nisse-dev
+# and gitea.dongar.net/dev/nisse-review, each tagged with the version (the
+# release tag without its v) and latest. One release, one version for all:
+# the agents share nisse-core and were tested together. Older versions are
+# removed by the dev organization's package cleanup rule (keep the 10 most
+# recent, and latest), set in Gitea, not here. Cut a
 # release in Gitea (Releases → New release, tag vX.Y.Z); a tag pushed for any
 # other reason builds nothing. The full test set runs first: no image without
 # it passing.
@@ -22,6 +26,7 @@ env:
   REGISTRY: gitea.dongar.net
   REGISTRY_USER: registry-push  # the token's account
   IMAGE: gitea.dongar.net/dev/nisse-dev
+  REVIEW_IMAGE: gitea.dongar.net/dev/nisse-review
 
 jobs:
   tests:
@@ -87,7 +92,7 @@ jobs:
       - name: Set up Docker Buildx
         uses: docker/setup-buildx-action@v3
 
-      - name: Build and push the agent
+      - name: Build and push nisse-dev
         uses: docker/build-push-action@v5
         with:
           context: .
@@ -101,3 +106,18 @@ jobs:
           tags: |
             ${{ env.IMAGE }}:${{ steps.vars.outputs.version }}
             ${{ env.IMAGE }}:latest
+
+      - name: Build and push nisse-review
+        uses: docker/build-push-action@v5
+        with:
+          context: .
+          target: review
+          push: true
+          build-args: |
+            VERSION=${{ steps.vars.outputs.version }}
+          labels: |
+            org.opencontainers.image.version=${{ steps.vars.outputs.version }}
+            org.opencontainers.image.revision=${{ github.sha }}
+          tags: |
+            ${{ env.REVIEW_IMAGE }}:${{ steps.vars.outputs.version }}
+            ${{ env.REVIEW_IMAGE }}:latest

Dockerfile

@@ -1,23 +1,27 @@
-# The nisse-dev agent as a container image (Development Plan M1.5, X4).
+# The agents as container images, one each (Development Plan §3; M1.5
+# nisse-dev, M7.6 nisse-review, X4).
 #
-# Stateless: the state store and SSH known-hosts live in a volume
-# (/var/lib/nisse-dev); configuration is mounted read-only
-# (/etc/nisse-dev/nisse-dev.yaml); secrets come as environment variables
-# (docker --env-file); SSH and deploy keys are mounted read-only wherever the
-# configuration says. The agent runs no model-chosen command here (L13):
-# those go to build VMs over SSH.
+# Each is stateless, runs as its own user, and keeps its state in a volume
+# (/var/lib/<agent>); its configuration is mounted read-only
+# (/etc/<agent>/<agent>.yaml) and its secrets come as environment variables
+# (docker --env-file). nisse-dev's SSH and deploy keys are mounted read-only
+# wherever its configuration says; it runs no model-chosen command here
+# (L13): those go to build VMs over SSH. nisse-review needs only git, for
+# its read-only clones, kept in its volume (scratch/) between restarts.
 #
-#   docker build -t nisse-dev .                  the agent
-#   docker build --target test -t nisse-dev:test .   plus the tests, for
-#                                                   scripts/e2e.sh --image
-#   docker build --target test --build-arg TEST_BASE=<published image> \
-#       -t nisse-dev:test .                      the tests over a release as
-#                                                published (M1.5 acceptance)
+#   docker build -t nisse-dev .                       nisse-dev
+#   docker build --target review -t nisse-review .    nisse-review
+#   docker build --target test -t nisse-dev:test .    plus the tests, for
+#                                                     scripts/e2e.sh --image
+#   docker build --target test --build-arg TEST_BASE=<image> -t <tag> .
+#       the tests over another image: nisse-review's (TEST_BASE=review), or
+#       a release as published (M1.5, M7.6 acceptance)
 
 # What the test stage stacks on: this build's agent, or a published image.
 ARG TEST_BASE=runtime
 
-FROM python:3.12-slim AS build
+# The workspace's source, from which each agent's environment is installed.
+FROM python:3.12-slim AS source
 COPY --from=ghcr.io/astral-sh/uv:0.12.10 /uv /usr/local/bin/uv
 ENV UV_PROJECT_ENVIRONMENT=/opt/nisse UV_COMPILE_BYTECODE=1 UV_LINK_MODE=copy
 WORKDIR /src
@@ -26,10 +30,33 @@ COPY nisse-core nisse-core
 COPY nisse-dev nisse-dev
 COPY nisse-ops nisse-ops
 COPY nisse-review nisse-review
-# The agent and what it needs, installed (not editable): the source tree
+
+# nisse-dev and what it needs, installed (not editable): the source tree
 # isn't part of the image.
+FROM source AS build
 RUN uv sync --frozen --no-dev --package nisse-dev --no-editable
 
+# nisse-review and what it needs, in an environment of its own.
+FROM source AS build-review
+RUN uv sync --frozen --no-dev --package nisse-review --no-editable
+
+FROM python:3.12-slim AS review
+ARG VERSION=dev
+LABEL org.opencontainers.image.title=nisse-review org.opencontainers.image.version=$VERSION
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends git \
+    && rm -rf /var/lib/apt/lists/* \
+    && useradd --create-home --uid 10002 nisse-review \
+    && mkdir -p /var/lib/nisse-review /etc/nisse-review \
+    && chown nisse-review /var/lib/nisse-review
+COPY --from=build-review /opt/nisse /opt/nisse
+ENV PATH=/opt/nisse/bin:$PATH \
+    NISSE_CONFIG=/etc/nisse-review/nisse-review.yaml \
+    NISSE_ENV=/run/nisse-review/.env
+VOLUME /var/lib/nisse-review
+USER nisse-review
+ENTRYPOINT ["nisse-review"]
+
 FROM python:3.12-slim AS runtime
 ARG VERSION=dev
 LABEL org.opencontainers.image.title=nisse-dev org.opencontainers.image.version=$VERSION
@@ -50,6 +77,9 @@ ENTRYPOINT ["nisse-dev"]
 # The tests over an image's own package (this build's, or a published one,
 # TEST_BASE): only the test tools are added, never the agent reinstalled, so
 # the end-to-end suite runs against what was built, not the source beside it.
+# The suite's own code imports every agent's package (nisse-dev's config,
+# for one), so a package the base lacks is added without its dependencies:
+# what the image installed stays as it was.
 FROM ${TEST_BASE} AS test
 USER root
 RUN apt-get update \
@@ -68,7 +98,12 @@ COPY scripts scripts
 COPY conftest.py ./
 ENV UV_PROJECT_ENVIRONMENT=/opt/nisse UV_NO_SYNC=1
 RUN uv export --frozen --only-group dev --no-hashes --no-emit-project > /tmp/dev.txt \
-    && uv pip install --quiet --python /opt/nisse/bin/python -r /tmp/dev.txt
+    && uv pip install --quiet --python /opt/nisse/bin/python -r /tmp/dev.txt \
+    && for package in nisse_dev nisse_review; do \
+         /opt/nisse/bin/python -c "import $package" 2>/dev/null \
+         || uv pip install --quiet --no-deps --python /opt/nisse/bin/python \
+              "./$(echo "$package" | tr _ -)"; \
+       done
 ENTRYPOINT []
 
 # The default target: the agent itself.

docs/Development-Plan.md

@@ -39,8 +39,8 @@ 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 | partial | Bring-up runs `nisse-dev`'s image under systemd on this box, `nisse-review` from a venv; project entries in config (M1.4); egress not enforced |
-| **O**, **X** | partial | `nisse-dev` has its image (M1.5); `nisse-review` doesn't yet (M7.6); nothing runs on the agent host |
+| **D7–D8** Deployment, projects | partial | Bring-up runs `nisse-dev`'s and `nisse-review`'s images under systemd on this box; project entries in config (M1.4); egress not enforced |
+| **O**, **X** | partial | `nisse-dev` has its image (M1.5), `nisse-review` its own (M7.6, acceptance after merge); nothing runs on the agent host |
 | `specs/` | done for now | `loop.qnt` (the architecture, #91); `orchestrator.qnt` went with `control.py` (M2.8) |
 
 ---
@@ -763,6 +763,19 @@ publishes `gitea.dongar.net/dev/nisse-review` beside `nisse-dev`, and
 `review-bringup.sh` runs the image instead of a virtualenv.
 - Acceptance: a release publishes both images, and the review end-to-end tests
   run over the published `nisse-review` image.
+- The `review` target: user `nisse-review` (uid 10002), state and its clones
+  (`scratch/`) in `/var/lib/nisse-review`, configuration at
+  `/etc/nisse-review/nisse-review.yaml`, only its own agent installed. One
+  release builds every agent's image at one version: they share `nisse-core`
+  and were tested together. Older versions go by the `dev` organization's
+  package cleanup rule (keep the 10 most recent, and `latest`), set in Gitea.
+- The `test` stage adds an agent package its base lacks without its
+  dependencies, since the suite's own code imports every agent's: over
+  `nisse-review`'s image, `scripts/e2e.sh --image REF -k test_review_live`.
+  `tests/deploy/test_images.py` (needs Docker) checks that what the base
+  installed stays as it was.
+- The bring-up runs the image as root with `$REVIEW` at its own path, as
+  `bringup.sh` runs `nisse-dev`, so its config and state stay where they were.
 
 **M7.7 Deployment.** *Waits for the agent host (Phase 6).* Its container on the
 agent host, Gitea account and secrets (R5).

scripts/review-bringup.sh

@@ -1,45 +1,72 @@
 #!/usr/bin/env bash
 # Bring-up runs of nisse-review on this development VM (Development Plan M7.3).
 #
-# It runs from the exported code under test, in a virtualenv of its own, as a
-# transient systemd unit, so it outlives the shell that started it. It runs
-# from its image after M7.6, and on the agent host after M7.7.
+# It runs as its container image (M7.6), built from the code under test, as
+# a transient systemd unit, so it outlives the shell that started it. It runs
+# on the agent host after M7.7.
 #
-#   scripts/review-bringup.sh start [REF]   export REF (default HEAD), install
-#                                           it, and start the reviewer
+# $REVIEW is mounted at the same path inside the container, so the config's
+# paths (state_db, the vault) mean the same there; for that it runs as root
+# here, as bringup.sh runs nisse-dev. In production the image runs as its
+# own user with a state volume (Dockerfile).
+#
+#   scripts/review-bringup.sh start [REF]   export REF (default HEAD), build it
+#                                           into an image, start the reviewer
+#   scripts/review-bringup.sh plan [REF]    print what start would build and run
 #   scripts/review-bringup.sh stop          stop it cleanly (SIGTERM)
 #   scripts/review-bringup.sh status        the unit, and its items
 #   scripts/review-bringup.sh logs          follow its log
 #
 # Layout, under $REVIEW (default /root/nisse-review):
 #   nisse-review.yaml, .env   the config and its secrets (made by hand once)
-#   code/, venv/              the exported code and its install
-#   state.db, scratch/        its state, and repository trees while reviewing
+#   code/                     the exported code, built into the image
+#   state.db, scratch/        its state, and repository clones kept between runs
 #   vault/                    its memory (memory/nisse-review)
 set -euo pipefail
 
 REVIEW=${NISSE_REVIEW:-/root/nisse-review}
 SRC=${NISSE_SRC:-/root/nisse}
 UNIT=nisse-review-bringup
-export PATH="$HOME/.local/bin:$PATH"
+IMAGE=nisse-review:bringup
 
 die() { echo "review-bringup: $*" >&2; exit 1; }
 running() { systemctl is-active --quiet "$UNIT"; }
 
+# The container the unit runs: host networking (Gitea, LiteLLM as here),
+# $REVIEW at its own path, the config and secrets from it.
+docker_run() {
+    echo docker run --rm --name "$UNIT" --network host --user 0 \
+        -v "$REVIEW:$REVIEW" -w "$REVIEW" \
+        -e NISSE_CONFIG="$REVIEW/nisse-review.yaml" -e NISSE_ENV="$REVIEW/.env" \
+        "$IMAGE"
+}
+
+cmd_plan() {
+    local ref=${1:-HEAD} sha
+    sha=$(git -C "$SRC" rev-parse --verify "$ref^{commit}") || die "unknown ref $ref"
+    echo "git -C $SRC archive $sha | tar -x -C $REVIEW/code"
+    echo "docker build --target review -t $IMAGE $REVIEW/code"
+    echo "systemd-run --unit=$UNIT ... $(docker_run)"
+}
+
 cmd_start() {
     local ref=${1:-HEAD} sha
     [ -f "$REVIEW/nisse-review.yaml" ] && [ -f "$REVIEW/.env" ] || die "no config in $REVIEW"
     running && die "already running; stop it first"
     sha=$(git -C "$SRC" rev-parse --verify "$ref^{commit}") || die "unknown ref $ref"
+    # Export the code under test, so edits to the working tree can't change a
+    # running reviewer. Uncommitted changes are not part of a run.
     rm -rf "$REVIEW/code" && mkdir -p "$REVIEW/code" "$REVIEW/vault/memory/nisse-review"
     git -C "$SRC" archive "$sha" | tar -x -C "$REVIEW/code"
-    UV_PROJECT_ENVIRONMENT="$REVIEW/venv" uv sync -q --frozen --no-dev \
-        --package nisse-review --no-editable --project "$REVIEW/code"
+    docker build --quiet --target review -t "$IMAGE" "$REVIEW/code" > /dev/null
     echo "$sha" > "$REVIEW/run-info"
-    systemd-run --unit="$UNIT" --collect \
-        -p WorkingDirectory="$REVIEW" \
-        -E NISSE_CONFIG="$REVIEW/nisse-review.yaml" -E NISSE_ENV="$REVIEW/.env" \
-        "$REVIEW/venv/bin/nisse-review"
+    systemctl reset-failed "$UNIT" 2>/dev/null || true
+    # SIGTERM reaches the reviewer through docker run's signal proxy; a stop
+    # breaks off its run, which is redone on the next start.
+    # shellcheck disable=SC2046
+    systemd-run --quiet --unit="$UNIT" --collect \
+        --property=KillSignal=SIGTERM --property=TimeoutStopSec=120 \
+        $(docker_run)
     echo "started $UNIT at $sha"
 }
 
@@ -55,8 +82,9 @@ cmd_logs() { journalctl -u "$UNIT" -f; }
 
 case "${1:-}" in
     start) shift; cmd_start "$@" ;;
+    plan) shift; cmd_plan "$@" ;;
     stop) cmd_stop ;;
     status) cmd_status ;;
     logs) cmd_logs ;;
-    *) die "usage: $0 start [REF] | stop | status | logs" ;;
+    *) die "usage: $0 start [REF] | plan [REF] | stop | status | logs" ;;
 esac

tests/deploy/test_images.py

@@ -0,0 +1,96 @@
+"""Every agent is its own image (Development Plan §3, M7.6): nisse-review's
+runs as its own user with git and only its own agent, and the test stage
+over it keeps what it installed. Builds the images, so it needs Docker."""
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).parent.parent.parent
+pytestmark = [
+    pytest.mark.integration,
+    pytest.mark.skipif(
+        shutil.which("docker") is None
+        or subprocess.run(
+            ["docker", "info"], capture_output=True, check=False
+        ).returncode,
+        reason="needs Docker",
+    ),
+]
+
+# The installed packages' files and their times, hashed: the same in an
+# image and the test stage over it means the tests ran against what was
+# built, not a reinstall (even one from the same source).
+HASH = (
+    "import hashlib, pathlib, nisse_review, nisse_core; h = hashlib.sha256(); "
+    "[h.update(p.read_bytes() + str(p.stat().st_mtime_ns).encode()) for m in (nisse_review, nisse_core) "
+    "for p in sorted(pathlib.Path(m.__file__).parent.rglob('*.py'))]; "
+    "print(h.hexdigest())"
+)
+
+
+def build(tag: str, *args: str) -> str:
+    subprocess.run(
+        ["docker", "build", "--quiet", "-t", tag, *args, str(ROOT)],
+        check=True,
+        capture_output=True,
+        timeout=1200,
+    )
+    return tag
+
+
+def sh(image: str, script: str) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(
+        ["docker", "run", "--rm", "--entrypoint", "sh", image, "-c", script],
+        capture_output=True,
+        text=True,
+        timeout=120,
+        check=False,
+    )
+
+
+@pytest.fixture(scope="module")
+def review() -> str:
+    return build("nisse-review:image-test", "--target", "review")
+
+
+def test_the_reviewer_runs_as_its_own_user_with_git(review: str) -> None:
+    done = sh(review, "id -un; git --version; command -v nisse-review")
+    user, git, entry = done.stdout.splitlines()
+    assert user == "nisse-review" and git.startswith("git version")
+    assert entry == "/opt/nisse/bin/nisse-review"
+    inspected = subprocess.run(
+        ["docker", "inspect", "-f", "{{json .Config.Volumes}}", review],
+        capture_output=True,
+        text=True,
+        check=True,
+    ).stdout
+    assert "/var/lib/nisse-review" in inspected
+
+
+def test_the_reviewer_image_holds_no_other_agent(review: str) -> None:
+    assert sh(review, "python -c 'import nisse_review'").returncode == 0
+    assert sh(review, "python -c 'import nisse_dev'").returncode != 0
+
+
+def test_its_state_is_writable_and_its_config_folder_is_not(review: str) -> None:
+    assert sh(review, "touch /var/lib/nisse-review/x").returncode == 0
+    assert sh(review, "touch /etc/nisse-review/x").returncode != 0
+
+
+def test_the_test_stage_over_it_keeps_what_it_installed(review: str) -> None:
+    tests = build(
+        "nisse-review:image-test-tests",
+        "--target",
+        "test",
+        "--build-arg",
+        f"TEST_BASE={review}",
+    )
+    both = sh(tests, "python -c 'import nisse_dev, nisse_review, pytest'")
+    assert both.returncode == 0, both.stderr  # the suite's imports resolve
+    assert (
+        sh(review, f'python -c "{HASH}"').stdout
+        == sh(tests, f'python -c "{HASH}"').stdout
+    )

tests/deploy/test_review_bringup.py

@@ -0,0 +1,97 @@
+"""scripts/review-bringup.sh runs nisse-review from its image (M7.6): built
+from the code under test, with the reviewer's folder at its own path."""
+
+import subprocess
+from pathlib import Path
+
+SCRIPT = Path(__file__).parent.parent.parent / "scripts" / "review-bringup.sh"
+
+
+def _setup(tmp_path: Path, *, running: bool = False) -> dict[str, str]:
+    """A source repository with one commit, the reviewer's folder with its
+    config, and stand-ins for docker, systemd-run and systemctl that log."""
+    src = tmp_path / "src"
+    src.mkdir()
+    (src / "Dockerfile").write_text("FROM scratch\n")
+    for args in (
+        ["init", "-q"],
+        ["add", "."],
+        ["-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", "c"],
+    ):
+        subprocess.run(["git", "-C", str(src), *args], check=True)
+    review = tmp_path / "review"
+    review.mkdir()
+    (review / "nisse-review.yaml").write_text("state_db: x\n")
+    (review / ".env").write_text("")
+    bin_ = tmp_path / "bin"
+    bin_.mkdir()
+    log = tmp_path / "calls"
+    stubs = {
+        "docker": f'echo "docker $*" >> {log}',
+        "systemd-run": f'echo "systemd-run $*" >> {log}',
+        "systemctl": f'echo "systemctl $*" >> {log}; '
+        f'[ "$1" = is-active ] && exit {0 if running else 3}; exit 0',
+    }
+    for name, body in stubs.items():
+        path = bin_ / name
+        path.write_text(f"#!/usr/bin/env bash\n{body}\n")
+        path.chmod(0o755)
+    return {
+        "PATH": f"{bin_}:/usr/bin:/bin",
+        "HOME": str(tmp_path),
+        "NISSE_REVIEW": str(review),
+        "NISSE_SRC": str(src),
+        "LOG": str(log),
+    }
+
+
+def _run(env: dict[str, str], *args: str) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(
+        ["bash", str(SCRIPT), *args],
+        env=env,
+        capture_output=True,
+        text=True,
+        timeout=30,
+        check=False,
+    )
+
+
+def test_start_builds_the_review_image_and_runs_it(tmp_path: Path) -> None:
+    env = _setup(tmp_path)
+    review = env["NISSE_REVIEW"]
+    done = _run(env, "start")
+    assert done.returncode == 0, done.stderr
+    calls = Path(env["LOG"]).read_text().splitlines()
+    [build] = [c for c in calls if c.startswith("docker build")]
+    assert "--target review" in build and build.endswith(f"{review}/code")
+    assert (Path(review) / "code" / "Dockerfile").exists()  # the code exported
+    [run] = [c for c in calls if c.startswith("systemd-run")]
+    assert "--unit=nisse-review-bringup" in run
+    assert "--property=KillSignal=SIGTERM" in run
+    assert "docker run --rm --name nisse-review-bringup" in run
+    assert f"-v {review}:{review}" in run
+    assert f"-e NISSE_CONFIG={review}/nisse-review.yaml" in run
+    assert f"-e NISSE_ENV={review}/.env" in run
+    assert run.endswith("nisse-review:bringup")
+    sha = subprocess.run(
+        ["git", "-C", env["NISSE_SRC"], "rev-parse", "HEAD"],
+        capture_output=True,
+        text=True,
+        check=True,
+    ).stdout.strip()
+    assert (Path(review) / "run-info").read_text().strip() == sha
+
+
+def test_it_wont_start_a_second_reviewer(tmp_path: Path) -> None:
+    env = _setup(tmp_path, running=True)
+    done = _run(env, "start")
+    assert done.returncode != 0 and "already running" in done.stderr
+    assert "docker" not in Path(env["LOG"]).read_text()
+
+
+def test_plan_shows_what_start_would_run(tmp_path: Path) -> None:
+    env = _setup(tmp_path)
+    done = _run(env, "plan")
+    assert done.returncode == 0, done.stderr
+    assert "docker build --target review -t nisse-review:bringup" in done.stdout
+    assert not Path(env["LOG"]).exists()  # nothing run