From 1da05935172180b296019e2a4021a3694963d3d0 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 24 Apr 2026 09:03:25 -0700 Subject: [PATCH 1/9] Fix #1986: auto-discover host worktree path; remove jwies hardcoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Producers in every phase stall on EACCES when trying to write draft artifacts into their worktree. Root cause: the gateway's `translate_to_host_path()` relied on `HOST_HOME=/home/egg` (the container-side path) as an identity map, so when the overlay actually backed the worktrees volume with a different host path, the orchestrator ended up passing the in-pod path to kubelet as a hostPath source. `DirectoryOrCreate` then created an empty root-owned directory at the wrong host location, and every agent mounted that empty dir instead of its real worktree. Fix in three parts: 1. gateway/gateway.py — auto-discover the host path for any in-pod path from `/proc/self/mountinfo`. Kubelet records the bind source as the mount's `root` field for every hostPath volume, so the gateway can translate reliably without any env-var configuration. `HOST_HOME` is preserved as an explicit fallback for test environments. Verified live in the cluster: the pod's mountinfo yields `/home/egg/.egg-worktrees/...` → `/home/jwies/.egg-worktrees/...`, which is the correct host path. 2. k8s/overlays/local/patches/*.yaml — replace every hardcoded `/home/jwies/...` with `${EGG_HOST_HOME}` / `${EGG_HOST_REPOS_DIR}` placeholders. The overlay no longer needs per-developer edits to the YAML. 3. Makefile — pipe kustomize output through envsubst with those two variables in the deploy target. Defaults: `EGG_HOST_HOME=$HOME` and `EGG_HOST_REPOS_DIR=$HOME/khan`. Override at invocation: `make deploy EGG_HOST_HOME=/data/egg EGG_HOST_REPOS_DIR=/srv/repos`. Secondary gaps the issue calls out (overseer silence on this failure mode, slow orchestrator stall escalation) are left for separate PRs. Tests: new `gateway/tests/test_translate_host_path.py` covers longest-prefix selection, sibling-path safety, HOST_HOME fallback precedence, and mountinfo parsing edge cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- Makefile | 9 + gateway/gateway.py | 61 +++++-- gateway/tests/test_translate_host_path.py | 165 ++++++++++++++++++ .../local/patches/gateway-volumes.yaml | 12 +- .../local/patches/orchestrator-volumes.yaml | 23 +-- 5 files changed, 245 insertions(+), 25 deletions(-) create mode 100644 gateway/tests/test_translate_host_path.py diff --git a/Makefile b/Makefile index b387e6faa0..bb73333a11 100644 --- a/Makefile +++ b/Makefile @@ -391,8 +391,17 @@ k3s-secrets: ## Create gateway secrets from ~/.config/egg/ deploy: k3s-secrets ## Deploy egg to k3s @echo "Deploying to k3s with tag $(EGG_IMAGE_TAG)..." + @command -v envsubst >/dev/null 2>&1 || { \ + echo "ERROR: envsubst not found. Install GNU gettext: 'dnf install gettext' or 'brew install gettext'." >&2; \ + exit 1; \ + } export KUBECONFIG=$${KUBECONFIG:-/etc/rancher/k3s/k3s.yaml} && \ + export EGG_HOST_HOME="$${EGG_HOST_HOME:-$$HOME}" && \ + export EGG_HOST_REPOS_DIR="$${EGG_HOST_REPOS_DIR:-$$EGG_HOST_HOME/khan}" && \ + echo " EGG_HOST_HOME=$$EGG_HOST_HOME" && \ + echo " EGG_HOST_REPOS_DIR=$$EGG_HOST_REPOS_DIR" && \ kubectl kustomize k8s/overlays/local/ | \ + envsubst '$$EGG_HOST_HOME $$EGG_HOST_REPOS_DIR' | \ sed -e "s|egg-orchestrator:latest|egg-orchestrator:$(EGG_IMAGE_TAG)|g" \ -e "s|egg-gateway:latest|egg-gateway:$(EGG_IMAGE_TAG)|g" \ -e "s|egg-sandbox:latest|egg-sandbox:$(EGG_IMAGE_TAG)|g" | \ diff --git a/gateway/gateway.py b/gateway/gateway.py index 6ccc67d14a..ab4cca0dd6 100644 --- a/gateway/gateway.py +++ b/gateway/gateway.py @@ -416,32 +416,73 @@ def handle_unhandled_exception(e: Exception) -> tuple[Response, int]: DEFAULT_THREADS = int(os.environ.get("GATEWAY_THREADS", "32")) HEALTH_CHECK_PORT = int(os.environ.get("GATEWAY_HEALTH_PORT", "9851")) -# Host home directory for path translation +# Host home directory for path translation (explicit override). # The gateway container uses /home/egg internally, but needs to return -# host paths to the egg launcher for Docker mount sources +# host paths to the orchestrator because those paths become the +# ``hostPath.path`` source of agent-pod mounts — if the gateway returns +# its in-pod path and the host layout doesn't match, kubelet +# ``DirectoryOrCreate``s an empty root-owned dir and the agent lands in +# an unwritable worktree (#1986). +# +# Normally we discover the host path directly from /proc/self/mountinfo +# (see ``translate_to_host_path``) so no env-var configuration is +# required. ``HOST_HOME`` remains as an explicit escape hatch for test +# environments and unusual setups where mountinfo doesn't reflect the +# real mapping. HOST_HOME = os.environ.get("HOST_HOME", "") CONTAINER_HOME = "/home/egg" +def _load_bind_mount_mapping() -> list[tuple[str, str]]: + """Read /proc/self/mountinfo and return a list of (mount_point, host_root) tuples. + + For every bind mount visible to this process, ``mount_point`` is the + path in this process's mount namespace and ``host_root`` is the path + the kernel recorded as the bind source — for kubelet-managed + ``hostPath`` volumes that's the actual host path. The list is sorted + longest-first so prefix lookup picks the most specific mount. + """ + entries: list[tuple[str, str]] = [] + try: + with open("/proc/self/mountinfo") as fh: + for line in fh: + # Format: mount_id parent_id major:minor root mount_point ... + fields = line.split() + if len(fields) < 5: + continue + entries.append((fields[4], fields[3])) + except OSError: + return [] + entries.sort(key=lambda p: len(p[0]), reverse=True) + return entries + + +_BIND_MOUNT_MAPPING: list[tuple[str, str]] = _load_bind_mount_mapping() + + def translate_to_host_path(container_path: str) -> str: """ Translate a container path to the corresponding host path. - The gateway runs with paths like /home/egg/.egg-worktrees/... - but the egg launcher needs host paths like /home/user/.egg-worktrees/... - for Docker mount sources. + Tries in order: + 1. /proc/self/mountinfo — find the longest mount_point that is a + prefix of ``container_path`` and substitute with its host root. + This works for any hostPath volume without configuration. + 2. ``HOST_HOME`` env var — explicit override, used when mountinfo is + not available or needs to be bypassed (tests, unusual setups). Args: container_path: Path inside the gateway container Returns: - The corresponding host path, or original path if translation not possible + The corresponding host path, or the original path if no + translation is possible. """ - if not HOST_HOME: - # No host home configured - return as-is (may cause mount issues) - return container_path + for mount_point, host_root in _BIND_MOUNT_MAPPING: + if container_path == mount_point or container_path.startswith(mount_point + "/"): + return host_root + container_path[len(mount_point) :] - if container_path.startswith(CONTAINER_HOME): + if HOST_HOME and container_path.startswith(CONTAINER_HOME): return container_path.replace(CONTAINER_HOME, HOST_HOME, 1) return container_path diff --git a/gateway/tests/test_translate_host_path.py b/gateway/tests/test_translate_host_path.py new file mode 100644 index 0000000000..c53099292f --- /dev/null +++ b/gateway/tests/test_translate_host_path.py @@ -0,0 +1,165 @@ +"""Tests for ``gateway.translate_to_host_path``. + +The gateway returns host paths to the orchestrator to use as +``hostPath.path`` sources for agent-pod mounts. If the translation is +wrong, kubelet ``DirectoryOrCreate``s an empty root-owned dir and the +agent lands in an unwritable worktree (#1986). These tests cover the +two translation strategies: mountinfo-based auto-discovery (primary) +and the ``HOST_HOME`` env var (fallback). +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +import gateway as gateway_module # noqa: E402 + + +@pytest.fixture +def override_bind_mounts(): + """Replace the module-level mount table with a test fixture.""" + original = gateway_module._BIND_MOUNT_MAPPING + + def _set(entries: list[tuple[str, str]]) -> None: + # Sort longest-first to match _load_bind_mount_mapping's contract. + gateway_module._BIND_MOUNT_MAPPING = sorted(entries, key=lambda p: len(p[0]), reverse=True) + + yield _set + gateway_module._BIND_MOUNT_MAPPING = original + + +@pytest.fixture +def override_host_home(): + """Temporarily set HOST_HOME at module scope.""" + original = gateway_module.HOST_HOME + + def _set(value: str) -> None: + gateway_module.HOST_HOME = value + + yield _set + gateway_module.HOST_HOME = original + + +class TestMountinfoTranslation: + """Auto-discovered translation via /proc/self/mountinfo.""" + + def test_longest_prefix_wins(self, override_bind_mounts, override_host_home): + # A nested hostPath mount must beat its parent emptyDir mount. + override_bind_mounts( + [ + ("/home/egg", "/var/lib/kubelet/pods/abc/emptydir/home"), + ("/home/egg/.egg-worktrees", "/home/user/.egg-worktrees"), + ] + ) + override_host_home("") + + result = gateway_module.translate_to_host_path("/home/egg/.egg-worktrees/pipeline-1/repo") + + assert result == "/home/user/.egg-worktrees/pipeline-1/repo" + + def test_exact_mount_point_match(self, override_bind_mounts, override_host_home): + override_bind_mounts([("/home/egg/.egg-worktrees", "/home/user/.egg-worktrees")]) + override_host_home("") + + assert ( + gateway_module.translate_to_host_path("/home/egg/.egg-worktrees") + == "/home/user/.egg-worktrees" + ) + + def test_sibling_paths_not_conflated(self, override_bind_mounts, override_host_home): + # /home/egg-other must not match the /home/egg mount_point. + override_bind_mounts([("/home/egg", "/host/egg")]) + override_host_home("") + + assert gateway_module.translate_to_host_path("/home/egg-other/x") == "/home/egg-other/x" + + def test_no_mountinfo_match_falls_through(self, override_bind_mounts, override_host_home): + override_bind_mounts([("/home/egg", "/host/egg")]) + override_host_home("") + + assert gateway_module.translate_to_host_path("/other/path") == "/other/path" + + +class TestHostHomeFallback: + """Explicit HOST_HOME env var, used when mountinfo lookup misses.""" + + def test_host_home_used_when_mountinfo_empty(self, override_bind_mounts, override_host_home): + override_bind_mounts([]) + override_host_home("/home/user") + + assert ( + gateway_module.translate_to_host_path("/home/egg/.egg-worktrees/x") + == "/home/user/.egg-worktrees/x" + ) + + def test_mountinfo_takes_precedence_over_host_home( + self, override_bind_mounts, override_host_home + ): + # mountinfo disagrees with HOST_HOME — trust mountinfo because it + # reflects what the kernel actually set up. + override_bind_mounts([("/home/egg/.egg-worktrees", "/real/host/path")]) + override_host_home("/stale/home") + + assert ( + gateway_module.translate_to_host_path("/home/egg/.egg-worktrees/x") + == "/real/host/path/x" + ) + + def test_no_translation_when_both_unavailable(self, override_bind_mounts, override_host_home): + override_bind_mounts([]) + override_host_home("") + + assert ( + gateway_module.translate_to_host_path("/home/egg/.egg-worktrees/x") + == "/home/egg/.egg-worktrees/x" + ) + + def test_host_home_ignored_for_non_container_path( + self, override_bind_mounts, override_host_home + ): + override_bind_mounts([]) + override_host_home("/home/user") + + assert gateway_module.translate_to_host_path("/other/path") == "/other/path" + + +class TestLoadBindMountMapping: + """``_load_bind_mount_mapping`` parses /proc/self/mountinfo.""" + + def test_parses_mount_point_and_root(self, tmp_path): + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text("1 0 0:1 /host/root /pod/mount rw,relatime - tmpfs tmpfs rw\n") + with patch("builtins.open", lambda *a, **kw: mountinfo.open()): + result = gateway_module._load_bind_mount_mapping() + assert ("/pod/mount", "/host/root") in result + + def test_sorts_longest_first(self, tmp_path): + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text( + "1 0 0:1 /r1 /a rw,relatime - tmpfs tmpfs rw\n" + "2 0 0:2 /r2 /a/b rw,relatime - tmpfs tmpfs rw\n" + ) + with patch("builtins.open", lambda *a, **kw: mountinfo.open()): + result = gateway_module._load_bind_mount_mapping() + assert result[0] == ("/a/b", "/r2") + assert result[1] == ("/a", "/r1") + + def test_skips_malformed_lines(self, tmp_path): + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text("garbage\n1 0 0:1 /r /m rw,relatime - tmpfs tmpfs rw\n") + with patch("builtins.open", lambda *a, **kw: mountinfo.open()): + result = gateway_module._load_bind_mount_mapping() + assert result == [("/m", "/r")] + + def test_returns_empty_when_mountinfo_missing(self): + def _raise(*_a, **_kw): + raise OSError(2, "No such file or directory") + + with patch("builtins.open", _raise): + assert gateway_module._load_bind_mount_mapping() == [] diff --git a/k8s/overlays/local/patches/gateway-volumes.yaml b/k8s/overlays/local/patches/gateway-volumes.yaml index d9d90e6066..f4503312c8 100644 --- a/k8s/overlays/local/patches/gateway-volumes.yaml +++ b/k8s/overlays/local/patches/gateway-volumes.yaml @@ -2,9 +2,11 @@ # worktrees. These append to the base's volumes/volumeMounts by name # (no conflicts, so strategic merge does the right thing here). # -# Paths are hardcoded to /home/jwies/ because kustomize has no native -# env var substitution. For a different host user, edit these paths or -# use envsubst before `kubectl apply`. +# ``${EGG_HOST_HOME}`` is expanded by ``envsubst`` in the Makefile's +# ``deploy`` target — defaults to ``$HOME`` on the machine running the +# deploy, so these hostPaths resolve to the current user's home +# without per-developer edits. Override via +# ``make deploy EGG_HOST_HOME=/data/egg`` for a different host layout. apiVersion: apps/v1 kind: Deployment metadata: @@ -25,9 +27,9 @@ spec: volumes: - name: repos hostPath: - path: /home/jwies/repos + path: ${EGG_HOST_HOME}/repos type: Directory - name: worktrees hostPath: - path: /home/jwies/.egg-worktrees + path: ${EGG_HOST_HOME}/.egg-worktrees type: DirectoryOrCreate diff --git a/k8s/overlays/local/patches/orchestrator-volumes.yaml b/k8s/overlays/local/patches/orchestrator-volumes.yaml index 9c0c6a673b..c1f76698bd 100644 --- a/k8s/overlays/local/patches/orchestrator-volumes.yaml +++ b/k8s/overlays/local/patches/orchestrator-volumes.yaml @@ -1,13 +1,16 @@ # Strategic merge patch: add local-dev-only host mounts for the orchestrator. # Appends to base volumes/volumeMounts by name. # -# ⚠ DEVELOPER-SPECIFIC PATHS — edit before use ⚠ -# Every `/home/jwies/...` below and the owner/repo → host path map in -# EGG_HOST_REPO_MAP point at this PR author's layout. Kustomize has no -# env-var substitution, so other contributors must replace these with -# their own $HOME and repo paths before `make deploy`. Tracked as a -# follow-up in #1760: make this portable (envsubst wrapper, Helm chart, -# or render from ~/.config/egg/repositories.yaml). +# ``${EGG_HOST_HOME}`` and ``${EGG_HOST_REPOS_DIR}`` are expanded by +# ``envsubst`` in the Makefile's ``deploy`` target. Defaults: +# ``EGG_HOST_HOME=$HOME``, ``EGG_HOST_REPOS_DIR=$HOME/khan``. Override +# either at deploy time for a different layout: +# make deploy EGG_HOST_HOME=/data/egg EGG_HOST_REPOS_DIR=/srv/repos +# +# EGG_HOST_REPO_MAP is the owner/repo → host-path map the orchestrator +# uses when building hostPath mounts for spawned agent pods. The keys +# listed below are an example set from one contributor's layout; edit +# to match your own repos (or drop entries you don't use). apiVersion: apps/v1 kind: Deployment metadata: @@ -35,7 +38,7 @@ spec: # mounts when spawning sandbox Jobs. Local-dev only; production # k8s would use a PV-backed repo store instead. - name: EGG_HOST_REPO_MAP - value: '{"jwbron/testing":"/home/jwies/khan/testing","jwbron/egg":"/home/jwies/khan/egg","Khan/webapp":"/home/jwies/khan/webapp","Khan/internal-services":"/home/jwies/khan/internal-services","Khan/jenkins-jobs":"/home/jwies/khan/jenkins-jobs","Khan/buildmaster2":"/home/jwies/khan/buildmaster2"}' + value: '{"jwbron/testing":"${EGG_HOST_REPOS_DIR}/testing","jwbron/egg":"${EGG_HOST_REPOS_DIR}/egg","Khan/webapp":"${EGG_HOST_REPOS_DIR}/webapp","Khan/internal-services":"${EGG_HOST_REPOS_DIR}/internal-services","Khan/jenkins-jobs":"${EGG_HOST_REPOS_DIR}/jenkins-jobs","Khan/buildmaster2":"${EGG_HOST_REPOS_DIR}/buildmaster2"}' volumeMounts: # repos mount is read-write because the orchestrator creates # per-pipeline worktrees inside each repo's .git/worktrees/. @@ -49,11 +52,11 @@ spec: volumes: - name: repos hostPath: - path: /home/jwies/repos + path: ${EGG_HOST_HOME}/repos type: Directory - name: worktrees hostPath: - path: /home/jwies/.egg-worktrees + path: ${EGG_HOST_HOME}/.egg-worktrees type: DirectoryOrCreate - name: secrets secret: From e5703d12510575e47dd7c670e7336bb7a34b5e89 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 24 Apr 2026 09:16:08 -0700 Subject: [PATCH 2/9] Auto-derive EGG_HOST_REPO_MAP from repositories.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay previously hand-maintained a JSON map of `owner/repo → host_path` with specific Khan/* and jwbron/* entries pinned to one contributor's ``/home/jwies/khan/*`` layout. Another developer would have had to edit the YAML before `make deploy` worked. Replace the hardcoded map with a deploy-time lookup: - ``scripts/build-host-repo-map.py`` reads ``local_repos.paths`` from ``~/.config/egg/repositories.yaml``, runs ``git config --get remote.origin.url`` on each, parses ``owner/repo`` from the remote URL (SCP, ssh://, https:// forms all handled), and emits the JSON mapping to stdout. Missing config / missing dirs / broken remotes are skipped with a stderr warning. - The orchestrator overlay now uses ``${EGG_HOST_REPO_MAP}`` as a placeholder. A follow-up ``sed`` in the Makefile adds single quotes around the expanded JSON value — kustomize strips quotes from the unexpanded placeholder, so the YAML parser would otherwise mistake the brace-delimited JSON for a flow-style mapping literal. - ``Makefile`` computes ``EGG_HOST_REPO_MAP`` from the helper (with the standard env-var override), echoes the resolved value, and pipes through envsubst + the JSON-quoting sed. PyYAML is already a project dependency. Tests: ``scripts/tests/test_build_host_repo_map.py`` covers every remote URL shape the parser accepts (plus rejected forms), missing config, missing directories, missing origin, empty/missing ``local_repos`` section, and sorted JSON output from the CLI entry point. Co-Authored-By: Claude Opus 4.7 (1M context) --- Makefile | 7 +- .../local/patches/orchestrator-volumes.yaml | 22 +-- scripts/build-host-repo-map.py | 120 +++++++++++++++ scripts/tests/test_build_host_repo_map.py | 137 ++++++++++++++++++ 4 files changed, 272 insertions(+), 14 deletions(-) create mode 100755 scripts/build-host-repo-map.py create mode 100644 scripts/tests/test_build_host_repo_map.py diff --git a/Makefile b/Makefile index bb73333a11..3a60b2471c 100644 --- a/Makefile +++ b/Makefile @@ -397,11 +397,12 @@ deploy: k3s-secrets ## Deploy egg to k3s } export KUBECONFIG=$${KUBECONFIG:-/etc/rancher/k3s/k3s.yaml} && \ export EGG_HOST_HOME="$${EGG_HOST_HOME:-$$HOME}" && \ - export EGG_HOST_REPOS_DIR="$${EGG_HOST_REPOS_DIR:-$$EGG_HOST_HOME/khan}" && \ + export EGG_HOST_REPO_MAP="$${EGG_HOST_REPO_MAP:-$$(scripts/build-host-repo-map.py)}" && \ echo " EGG_HOST_HOME=$$EGG_HOST_HOME" && \ - echo " EGG_HOST_REPOS_DIR=$$EGG_HOST_REPOS_DIR" && \ + echo " EGG_HOST_REPO_MAP=$$EGG_HOST_REPO_MAP" && \ kubectl kustomize k8s/overlays/local/ | \ - envsubst '$$EGG_HOST_HOME $$EGG_HOST_REPOS_DIR' | \ + envsubst '$$EGG_HOST_HOME $$EGG_HOST_REPO_MAP' | \ + sed -E "s|^(\s*value: )(\{.*\})$$|\1'\2'|" | \ sed -e "s|egg-orchestrator:latest|egg-orchestrator:$(EGG_IMAGE_TAG)|g" \ -e "s|egg-gateway:latest|egg-gateway:$(EGG_IMAGE_TAG)|g" \ -e "s|egg-sandbox:latest|egg-sandbox:$(EGG_IMAGE_TAG)|g" | \ diff --git a/k8s/overlays/local/patches/orchestrator-volumes.yaml b/k8s/overlays/local/patches/orchestrator-volumes.yaml index c1f76698bd..8447331454 100644 --- a/k8s/overlays/local/patches/orchestrator-volumes.yaml +++ b/k8s/overlays/local/patches/orchestrator-volumes.yaml @@ -1,16 +1,16 @@ # Strategic merge patch: add local-dev-only host mounts for the orchestrator. # Appends to base volumes/volumeMounts by name. # -# ``${EGG_HOST_HOME}`` and ``${EGG_HOST_REPOS_DIR}`` are expanded by -# ``envsubst`` in the Makefile's ``deploy`` target. Defaults: -# ``EGG_HOST_HOME=$HOME``, ``EGG_HOST_REPOS_DIR=$HOME/khan``. Override -# either at deploy time for a different layout: -# make deploy EGG_HOST_HOME=/data/egg EGG_HOST_REPOS_DIR=/srv/repos -# -# EGG_HOST_REPO_MAP is the owner/repo → host-path map the orchestrator -# uses when building hostPath mounts for spawned agent pods. The keys -# listed below are an example set from one contributor's layout; edit -# to match your own repos (or drop entries you don't use). +# ``${EGG_HOST_HOME}`` and ``${EGG_HOST_REPO_MAP}`` are expanded by +# ``envsubst`` in the Makefile's ``deploy`` target: +# - ``EGG_HOST_HOME`` defaults to ``$HOME``. +# - ``EGG_HOST_REPO_MAP`` is a JSON map ``owner/repo → host_path`` +# auto-derived by ``scripts/build-host-repo-map.py`` from +# ``~/.config/egg/repositories.yaml`` (``local_repos.paths`` with +# the ``origin`` remote URL parsed to recover ``owner/repo``). +# Override either at deploy time: +# make deploy EGG_HOST_HOME=/data/egg +# EGG_HOST_REPO_MAP='{"owner/repo":"/path"}' make deploy apiVersion: apps/v1 kind: Deployment metadata: @@ -38,7 +38,7 @@ spec: # mounts when spawning sandbox Jobs. Local-dev only; production # k8s would use a PV-backed repo store instead. - name: EGG_HOST_REPO_MAP - value: '{"jwbron/testing":"${EGG_HOST_REPOS_DIR}/testing","jwbron/egg":"${EGG_HOST_REPOS_DIR}/egg","Khan/webapp":"${EGG_HOST_REPOS_DIR}/webapp","Khan/internal-services":"${EGG_HOST_REPOS_DIR}/internal-services","Khan/jenkins-jobs":"${EGG_HOST_REPOS_DIR}/jenkins-jobs","Khan/buildmaster2":"${EGG_HOST_REPOS_DIR}/buildmaster2"}' + value: '${EGG_HOST_REPO_MAP}' volumeMounts: # repos mount is read-write because the orchestrator creates # per-pipeline worktrees inside each repo's .git/worktrees/. diff --git a/scripts/build-host-repo-map.py b/scripts/build-host-repo-map.py new file mode 100755 index 0000000000..ef9d3c6315 --- /dev/null +++ b/scripts/build-host-repo-map.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""Build EGG_HOST_REPO_MAP from ~/.config/egg/repositories.yaml. + +The orchestrator reads ``EGG_HOST_REPO_MAP`` — a JSON ``owner/repo → +host_path`` mapping — to know where each repo lives on the host when it +builds ``hostPath`` mounts for spawned agent pods. + +This script derives the map from ``local_repos.paths`` in +``repositories.yaml``: for each path, it reads the repo's ``origin`` +remote URL to recover the ``owner/repo`` identifier, pairs that with the +host path, and prints the resulting JSON to stdout. + +Usage: + scripts/build-host-repo-map.py # uses ~/.config/egg/repositories.yaml + scripts/build-host-repo-map.py # explicit config path + +Emits ``{}`` (and exits 0) when the config is missing or lists no repos +— callers that want to fail on empty input must check the output. +""" + +from __future__ import annotations + +import json +import re +import subprocess +import sys +from pathlib import Path + +try: + import yaml +except ImportError: + print("ERROR: PyYAML not installed", file=sys.stderr) + sys.exit(1) + + +# Supported remote URL formats: +# git@host:owner/repo[.git] (scp-like SSH) +# ssh://[user@]host[:port]/owner/repo[.git] +# https://host[:port]/owner/repo[.git][/] +_SCP_SSH_REMOTE = re.compile(r"^[^@]+@[^:]+:(?P[^/]+)/(?P[^/]+?)(?:\.git)?$") +_URL_REMOTE = re.compile(r"^(?:ssh|https?)://[^/]+/(?P[^/]+)/(?P[^/]+?)(?:\.git)?/?$") + + +def parse_owner_repo(remote_url: str) -> str | None: + """Extract ``owner/repo`` from a git remote URL, or None if unparseable.""" + for pattern in (_SCP_SSH_REMOTE, _URL_REMOTE): + match = pattern.match(remote_url.strip()) + if match: + return f"{match.group('owner')}/{match.group('repo')}" + return None + + +def get_origin_url(repo_path: Path) -> str | None: + """Return the ``origin`` remote URL for a git repo, or None.""" + try: + result = subprocess.run( + ["git", "-C", str(repo_path), "config", "--get", "remote.origin.url"], + capture_output=True, + text=True, + check=False, + timeout=5, + ) + except (OSError, subprocess.SubprocessError): + return None + if result.returncode != 0: + return None + return result.stdout.strip() or None + + +def build_map(config_path: Path) -> dict[str, str]: + """Read ``local_repos.paths`` and build the owner/repo → host_path map.""" + if not config_path.exists(): + return {} + + with config_path.open() as fh: + config = yaml.safe_load(fh) or {} + + local_repos = config.get("local_repos") or {} + paths = local_repos.get("paths") or [] if isinstance(local_repos, dict) else [] + + mapping: dict[str, str] = {} + for raw_path in paths: + path = Path(raw_path).expanduser() + if not path.is_dir(): + continue + remote_url = get_origin_url(path) + if not remote_url: + print( + f"WARN: no origin remote for {path}, skipping", + file=sys.stderr, + ) + continue + owner_repo = parse_owner_repo(remote_url) + if not owner_repo: + print( + f"WARN: could not parse owner/repo from {remote_url!r}, skipping {path}", + file=sys.stderr, + ) + continue + mapping[owner_repo] = str(path) + + return mapping + + +def main() -> None: + if len(sys.argv) > 2: + print(f"Usage: {sys.argv[0]} [config_path]", file=sys.stderr) + sys.exit(2) + + if len(sys.argv) == 2: + config_path = Path(sys.argv[1]).expanduser() + else: + config_path = Path.home() / ".config" / "egg" / "repositories.yaml" + + mapping = build_map(config_path) + print(json.dumps(mapping, separators=(",", ":"), sort_keys=True)) + + +if __name__ == "__main__": + main() diff --git a/scripts/tests/test_build_host_repo_map.py b/scripts/tests/test_build_host_repo_map.py new file mode 100644 index 0000000000..5b80f8c2eb --- /dev/null +++ b/scripts/tests/test_build_host_repo_map.py @@ -0,0 +1,137 @@ +"""Tests for ``scripts/build-host-repo-map.py``. + +The orchestrator consumes ``EGG_HOST_REPO_MAP`` (owner/repo → host_path) +to build hostPath mounts for spawned agent pods. Before this script +existed that map was hand-maintained in the k8s overlay, hardcoding +specific owner/repo pairs to one developer's filesystem (#1986). These +tests cover parsing every remote-URL shape that can appear in +``~/.config/egg/repositories.yaml`` and the edge cases the builder has +to survive (missing config, missing dirs, broken remotes). +""" + +from __future__ import annotations + +import importlib.util +import json +import subprocess +from pathlib import Path + +import pytest + +_BUILDER_PATH = Path(__file__).resolve().parent.parent / "build-host-repo-map.py" +_spec = importlib.util.spec_from_file_location("build_host_repo_map", _BUILDER_PATH) +assert _spec and _spec.loader +build_host_repo_map = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(build_host_repo_map) + + +class TestParseOwnerRepo: + @pytest.mark.parametrize( + "url,expected", + [ + ("git@github.com:Khan/webapp.git", "Khan/webapp"), + ("git@github.com:Khan/webapp", "Khan/webapp"), + ("git@gitlab.internal:team/repo.git", "team/repo"), + ("ssh://git@github.com/Khan/webapp.git", "Khan/webapp"), + ("ssh://git@github.com:22/Khan/webapp.git", "Khan/webapp"), + ("https://github.com/Khan/webapp.git", "Khan/webapp"), + ("https://github.com/Khan/webapp", "Khan/webapp"), + ("http://example.com/o/r.git", "o/r"), + ("https://github.com/Khan/webapp/", "Khan/webapp"), + (" git@github.com:Khan/webapp.git\n", "Khan/webapp"), + ], + ) + def test_parses_common_forms(self, url, expected): + assert build_host_repo_map.parse_owner_repo(url) == expected + + @pytest.mark.parametrize( + "url", + [ + "", + "not-a-url", + "file:///local/path.git", + "https://github.com/only-one-segment", + ], + ) + def test_unparseable_returns_none(self, url): + assert build_host_repo_map.parse_owner_repo(url) is None + + +def _init_repo_with_remote(path: Path, remote_url: str) -> None: + """Initialize a bare-bones git repo with an origin remote for testing.""" + path.mkdir(parents=True, exist_ok=True) + subprocess.run(["git", "init", "-q", str(path)], check=True) + subprocess.run(["git", "-C", str(path), "remote", "add", "origin", remote_url], check=True) + + +class TestBuildMap: + def test_builds_map_from_configured_paths(self, tmp_path): + repo_a = tmp_path / "webapp" + repo_b = tmp_path / "egg" + _init_repo_with_remote(repo_a, "git@github.com:Khan/webapp.git") + _init_repo_with_remote(repo_b, "ssh://git@github.com/owner/egg.git") + + config_path = tmp_path / "repositories.yaml" + config_path.write_text(f"local_repos:\n paths:\n - {repo_a}\n - {repo_b}\n") + + result = build_host_repo_map.build_map(config_path) + + assert result == { + "Khan/webapp": str(repo_a), + "owner/egg": str(repo_b), + } + + def test_missing_config_returns_empty(self, tmp_path): + assert build_host_repo_map.build_map(tmp_path / "does-not-exist.yaml") == {} + + def test_skips_nonexistent_paths(self, tmp_path): + config_path = tmp_path / "repositories.yaml" + config_path.write_text("local_repos:\n paths:\n - /definitely/not/real/path\n") + + assert build_host_repo_map.build_map(config_path) == {} + + def test_skips_paths_without_git_remote(self, tmp_path): + repo = tmp_path / "no-remote" + repo.mkdir() + subprocess.run(["git", "init", "-q", str(repo)], check=True) + # no `git remote add` — origin is absent + + config_path = tmp_path / "repositories.yaml" + config_path.write_text(f"local_repos:\n paths:\n - {repo}\n") + + assert build_host_repo_map.build_map(config_path) == {} + + def test_handles_missing_local_repos_section(self, tmp_path): + config_path = tmp_path / "repositories.yaml" + config_path.write_text("github_username: someone\n") + + assert build_host_repo_map.build_map(config_path) == {} + + def test_empty_local_repos_returns_empty(self, tmp_path): + config_path = tmp_path / "repositories.yaml" + config_path.write_text("local_repos:\n paths: []\n") + + assert build_host_repo_map.build_map(config_path) == {} + + +class TestScriptEntryPoint: + def test_cli_emits_sorted_json(self, tmp_path): + repo_a = tmp_path / "zeta" + repo_b = tmp_path / "alpha" + _init_repo_with_remote(repo_a, "git@github.com:owner/zeta.git") + _init_repo_with_remote(repo_b, "git@github.com:owner/alpha.git") + + config_path = tmp_path / "repositories.yaml" + config_path.write_text(f"local_repos:\n paths:\n - {repo_a}\n - {repo_b}\n") + + proc = subprocess.run( + [str(_BUILDER_PATH), str(config_path)], + capture_output=True, + text=True, + check=True, + ) + + payload = json.loads(proc.stdout) + assert list(payload.keys()) == ["owner/alpha", "owner/zeta"] + assert payload["owner/alpha"] == str(repo_b) + assert payload["owner/zeta"] == str(repo_a) From c538393b541800ffabd2435a53baf32b42cd5ffb Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 24 Apr 2026 09:19:09 -0700 Subject: [PATCH 3/9] Replace user-specific example values in docstrings and test fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several docstrings and test mocks used /home/jwies and jwbron as arbitrary example values — not runtime hardcoding, but they suggest the code assumes a specific developer's setup. Swap to neutral placeholders (/home/user, my-org) so other contributors reading the code don't have to pattern-match around one person's username. Canonical references to the jwbron/egg project (schema $ids, GitHub Action refs, release image names, issue links) are intentionally left alone — those point at the actual project and would be wrong as placeholders. Co-Authored-By: Claude Opus 4.7 (1M context) --- config/repo_config.py | 2 +- docs/architecture/orchestrator.md | 4 ++-- gateway/gateway.py | 2 +- orchestrator/kubernetes_spawner.py | 2 +- orchestrator/tests/test_container_spawner.py | 24 +++++++++---------- .../shared/egg_container/test_phase_mounts.py | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/config/repo_config.py b/config/repo_config.py index 23ac674bc8..087e62e6e2 100644 --- a/config/repo_config.py +++ b/config/repo_config.py @@ -411,7 +411,7 @@ def is_checkpoint_repo(owner: str, repo: str) -> bool: """Check if a repository is configured as a checkpoint destination. Args: - owner: Repository owner (e.g. "jwbron") + owner: Repository owner (e.g. "my-org") repo: Repository name (e.g. "egg-checkpoints") Returns: diff --git a/docs/architecture/orchestrator.md b/docs/architecture/orchestrator.md index 9c48175c27..a77c3d9818 100644 --- a/docs/architecture/orchestrator.md +++ b/docs/architecture/orchestrator.md @@ -219,7 +219,7 @@ During the `implement` phase, certain `.egg-state/` subdirectories are mounted r The orchestrator calls `ensure_egg_state_dirs()` before spawning containers to create the required directories (bind mounts require existing source paths) and place `.egg-readonly` marker files explaining the restriction and current phase. Reviewer agents do not receive the `.egg-readonly` marker in the `reviews/` directory. Then `phase_readonly_mounts()` generates the readonly `MountSpec` entries, which are added alongside the existing `.git` shadow mounts. Only directories that exist on the host are mounted (missing directories are skipped). See `shared/egg_container/__init__.py` and `orchestrator/container_spawner.py`. -**Host path translation:** The gateway returns worktree paths relative to the host (e.g., `/home/jwies/.egg-worktrees/...`), but the orchestrator pod only sees these via `/home/egg/...` hostPath mounts. The spawner uses the `HOST_HOME` env var to translate host paths to orchestrator-accessible local paths for `is_dir()` checks and `ensure_egg_state_dirs()`. hostPath mount sources still use the original host paths unchanged. +**Host path translation:** The gateway returns worktree paths relative to the host (e.g., `/home/user/.egg-worktrees/...`), but the orchestrator pod only sees these via `/home/egg/...` hostPath mounts. The spawner uses the `HOST_HOME` env var to translate host paths to orchestrator-accessible local paths for `is_dir()` checks and `ensure_egg_state_dirs()`. hostPath mount sources still use the original host paths unchanged. **Worktree state synchronization:** The orchestrator maintains bidirectional synchronization between local worktree branches and their remote counterparts: @@ -559,7 +559,7 @@ if is_orchestrator_mode(): | `EGG_AGENT_ROLE` | Agent role for multi-agent mode | None | | `EGG_BRANCH` | Target branch for the agent's worktree | `egg/{pipeline_id}/work` | | `EGG_PRIVATE_MODE` | Private network mode (set by host wrapper, detected by `egg-sdlc`) | None | -| `HOST_HOME` | Host machine's home directory (e.g., `/home/jwies`); used to translate host worktree paths to orchestrator-accessible paths | None | +| `HOST_HOME` | Host machine's home directory (e.g., `/home/user`); used to translate host worktree paths to orchestrator-accessible paths | None | ### Constants diff --git a/gateway/gateway.py b/gateway/gateway.py index ab4cca0dd6..d75022235f 100644 --- a/gateway/gateway.py +++ b/gateway/gateway.py @@ -340,7 +340,7 @@ def _is_checkpoint_repo_for_request(owner: str, repo: str) -> bool: but the orchestrator passed ``EGG_CHECKPOINT_REPO``). Args: - owner: Repository owner (e.g. "jwbron") + owner: Repository owner (e.g. "my-org") repo: Repository name (e.g. "checkpoints") Returns: diff --git a/orchestrator/kubernetes_spawner.py b/orchestrator/kubernetes_spawner.py index cfc2fcf8eb..6f45039573 100644 --- a/orchestrator/kubernetes_spawner.py +++ b/orchestrator/kubernetes_spawner.py @@ -206,7 +206,7 @@ def _host_to_local_volumes(repo_volumes: dict[str, str]) -> dict[str, str]: """Translate host paths to orchestrator-local paths for filesystem ops. The gateway returns worktree paths relative to the Docker host - (e.g. ``/home/jwies/.egg-worktrees/...``), but the orchestrator + (e.g. ``/home/user/.egg-worktrees/...``), but the orchestrator container only sees these via a volume mount at ``/home/egg/...``. Uses the ``HOST_HOME`` env var to perform the translation. """ diff --git a/orchestrator/tests/test_container_spawner.py b/orchestrator/tests/test_container_spawner.py index 05eab957dd..4ad2889752 100644 --- a/orchestrator/tests/test_container_spawner.py +++ b/orchestrator/tests/test_container_spawner.py @@ -498,8 +498,8 @@ class TestHostToLocalVolumes: def test_translates_host_home_to_container_home(self): """HOST_HOME prefix is replaced with /home/egg.""" - repo_volumes = {"repo": "/home/jwies/.egg-worktrees/repo"} - with patch.dict("os.environ", {"HOST_HOME": "/home/jwies"}): + repo_volumes = {"repo": "/home/user/.egg-worktrees/repo"} + with patch.dict("os.environ", {"HOST_HOME": "/home/user"}): result = _host_to_local_volumes(repo_volumes) assert result == {"repo": "/home/egg/.egg-worktrees/repo"} @@ -519,18 +519,18 @@ def test_passthrough_when_host_home_empty(self): def test_only_replaces_prefix(self): """Only the first occurrence of HOST_HOME at the start is replaced.""" - repo_volumes = {"repo": "/home/jwies/repos/home/jwies/nested"} - with patch.dict("os.environ", {"HOST_HOME": "/home/jwies"}): + repo_volumes = {"repo": "/home/user/repos/home/user/nested"} + with patch.dict("os.environ", {"HOST_HOME": "/home/user"}): result = _host_to_local_volumes(repo_volumes) - assert result == {"repo": "/home/egg/repos/home/jwies/nested"} + assert result == {"repo": "/home/egg/repos/home/user/nested"} def test_multiple_repos(self): """All repos in the mapping are translated.""" repo_volumes = { - "a": "/home/jwies/.egg-worktrees/a", - "b": "/home/jwies/.egg-worktrees/b", + "a": "/home/user/.egg-worktrees/a", + "b": "/home/user/.egg-worktrees/b", } - with patch.dict("os.environ", {"HOST_HOME": "/home/jwies"}): + with patch.dict("os.environ", {"HOST_HOME": "/home/user"}): result = _host_to_local_volumes(repo_volumes) assert result == { "a": "/home/egg/.egg-worktrees/a", @@ -540,10 +540,10 @@ def test_multiple_repos(self): def test_non_matching_paths_unchanged(self): """Paths not starting with HOST_HOME are left unchanged.""" repo_volumes = { - "a": "/home/jwies/.egg-worktrees/a", + "a": "/home/user/.egg-worktrees/a", "b": "/other/path/b", } - with patch.dict("os.environ", {"HOST_HOME": "/home/jwies"}): + with patch.dict("os.environ", {"HOST_HOME": "/home/user"}): result = _host_to_local_volumes(repo_volumes) assert result == { "a": "/home/egg/.egg-worktrees/a", @@ -552,8 +552,8 @@ def test_non_matching_paths_unchanged(self): def test_trailing_slash_on_host_home(self): """HOST_HOME with trailing slash does not produce double slashes.""" - repo_volumes = {"repo": "/home/jwies/.egg-worktrees/repo"} - with patch.dict("os.environ", {"HOST_HOME": "/home/jwies/"}): + repo_volumes = {"repo": "/home/user/.egg-worktrees/repo"} + with patch.dict("os.environ", {"HOST_HOME": "/home/user/"}): result = _host_to_local_volumes(repo_volumes) assert result == {"repo": "/home/egg/.egg-worktrees/repo"} diff --git a/tests/shared/egg_container/test_phase_mounts.py b/tests/shared/egg_container/test_phase_mounts.py index 2155fde409..4f78c73ea3 100644 --- a/tests/shared/egg_container/test_phase_mounts.py +++ b/tests/shared/egg_container/test_phase_mounts.py @@ -292,7 +292,7 @@ def test_local_volumes_used_for_existence_check(self, tmp_path): for dirname in _IMPLEMENT_READONLY_DIRS: (tmp_path / ".egg-state" / dirname).mkdir(parents=True) - host_path = "/home/jwies/.egg-worktrees/some-repo" + host_path = "/home/user/.egg-worktrees/some-repo" repo_volumes = {"myrepo": host_path} local_volumes = {"myrepo": str(tmp_path)} From 3094ed7628d229ef49c2b43d0f78b80afaf06caf Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 16:37:28 +0000 Subject: [PATCH 4/9] Address review feedback: rename mount mapping, scope mocks, handle YAML errors - Rename _load_bind_mount_mapping -> _load_mount_mapping (and _BIND_MOUNT_MAPPING -> _MOUNT_MAPPING) since the function collects all mount types, not just bind mounts - Document mountinfo root-field assumption (single-partition) and octal escape limitation in the docstring - Scope test mocks to gateway.open instead of builtins.open to prevent intercepting unrelated file opens - Handle yaml.YAMLError in build_map so a corrupted repositories.yaml emits a warning and returns {} instead of crashing - Document last-wins behavior for duplicate owner/repo entries - Add test for corrupted YAML graceful degradation --- gateway/gateway.py | 28 ++++++++++++++++------- gateway/tests/test_translate_host_path.py | 28 +++++++++++------------ scripts/build-host-repo-map.py | 11 +++++++-- scripts/tests/test_build_host_repo_map.py | 6 +++++ 4 files changed, 49 insertions(+), 24 deletions(-) diff --git a/gateway/gateway.py b/gateway/gateway.py index d75022235f..ff449b8c7e 100644 --- a/gateway/gateway.py +++ b/gateway/gateway.py @@ -433,14 +433,26 @@ def handle_unhandled_exception(e: Exception) -> tuple[Response, int]: CONTAINER_HOME = "/home/egg" -def _load_bind_mount_mapping() -> list[tuple[str, str]]: +def _load_mount_mapping() -> list[tuple[str, str]]: """Read /proc/self/mountinfo and return a list of (mount_point, host_root) tuples. - For every bind mount visible to this process, ``mount_point`` is the - path in this process's mount namespace and ``host_root`` is the path - the kernel recorded as the bind source — for kubelet-managed - ``hostPath`` volumes that's the actual host path. The list is sorted - longest-first so prefix lookup picks the most specific mount. + For every mount visible to this process, ``mount_point`` is the path + in this process's mount namespace and ``host_root`` is the path the + kernel recorded as the mount root — for kubelet-managed ``hostPath`` + volumes that's the actual host path. The list includes *all* mount + types (not just bind mounts); longest-prefix matching in + ``translate_to_host_path`` ensures the most specific entry wins. + + Note: ``host_root`` (``fields[3]``, the mountinfo *root* field) is + the path relative to the filesystem's root. On single-partition + systems this equals the absolute host path; on multi-partition setups + it may be relative to the partition root. The ``HOST_HOME`` env var + is the escape hatch for those configurations. + + Note: mountinfo uses octal escapes for special characters in paths + (``\\040`` for space, ``\\011`` for tab, ``\\134`` for backslash). + We don't decode them — unlikely to matter for ``/home/...`` paths + but worth knowing if paths ever contain whitespace. """ entries: list[tuple[str, str]] = [] try: @@ -457,7 +469,7 @@ def _load_bind_mount_mapping() -> list[tuple[str, str]]: return entries -_BIND_MOUNT_MAPPING: list[tuple[str, str]] = _load_bind_mount_mapping() +_MOUNT_MAPPING: list[tuple[str, str]] = _load_mount_mapping() def translate_to_host_path(container_path: str) -> str: @@ -478,7 +490,7 @@ def translate_to_host_path(container_path: str) -> str: The corresponding host path, or the original path if no translation is possible. """ - for mount_point, host_root in _BIND_MOUNT_MAPPING: + for mount_point, host_root in _MOUNT_MAPPING: if container_path == mount_point or container_path.startswith(mount_point + "/"): return host_root + container_path[len(mount_point) :] diff --git a/gateway/tests/test_translate_host_path.py b/gateway/tests/test_translate_host_path.py index c53099292f..b53fb6f993 100644 --- a/gateway/tests/test_translate_host_path.py +++ b/gateway/tests/test_translate_host_path.py @@ -24,14 +24,14 @@ @pytest.fixture def override_bind_mounts(): """Replace the module-level mount table with a test fixture.""" - original = gateway_module._BIND_MOUNT_MAPPING + original = gateway_module._MOUNT_MAPPING def _set(entries: list[tuple[str, str]]) -> None: - # Sort longest-first to match _load_bind_mount_mapping's contract. - gateway_module._BIND_MOUNT_MAPPING = sorted(entries, key=lambda p: len(p[0]), reverse=True) + # Sort longest-first to match _load_mount_mapping's contract. + gateway_module._MOUNT_MAPPING = sorted(entries, key=lambda p: len(p[0]), reverse=True) yield _set - gateway_module._BIND_MOUNT_MAPPING = original + gateway_module._MOUNT_MAPPING = original @pytest.fixture @@ -129,14 +129,14 @@ def test_host_home_ignored_for_non_container_path( assert gateway_module.translate_to_host_path("/other/path") == "/other/path" -class TestLoadBindMountMapping: - """``_load_bind_mount_mapping`` parses /proc/self/mountinfo.""" +class TestLoadMountMapping: + """``_load_mount_mapping`` parses /proc/self/mountinfo.""" def test_parses_mount_point_and_root(self, tmp_path): mountinfo = tmp_path / "mountinfo" mountinfo.write_text("1 0 0:1 /host/root /pod/mount rw,relatime - tmpfs tmpfs rw\n") - with patch("builtins.open", lambda *a, **kw: mountinfo.open()): - result = gateway_module._load_bind_mount_mapping() + with patch("gateway.open", lambda *a, **kw: mountinfo.open()): + result = gateway_module._load_mount_mapping() assert ("/pod/mount", "/host/root") in result def test_sorts_longest_first(self, tmp_path): @@ -145,21 +145,21 @@ def test_sorts_longest_first(self, tmp_path): "1 0 0:1 /r1 /a rw,relatime - tmpfs tmpfs rw\n" "2 0 0:2 /r2 /a/b rw,relatime - tmpfs tmpfs rw\n" ) - with patch("builtins.open", lambda *a, **kw: mountinfo.open()): - result = gateway_module._load_bind_mount_mapping() + with patch("gateway.open", lambda *a, **kw: mountinfo.open()): + result = gateway_module._load_mount_mapping() assert result[0] == ("/a/b", "/r2") assert result[1] == ("/a", "/r1") def test_skips_malformed_lines(self, tmp_path): mountinfo = tmp_path / "mountinfo" mountinfo.write_text("garbage\n1 0 0:1 /r /m rw,relatime - tmpfs tmpfs rw\n") - with patch("builtins.open", lambda *a, **kw: mountinfo.open()): - result = gateway_module._load_bind_mount_mapping() + with patch("gateway.open", lambda *a, **kw: mountinfo.open()): + result = gateway_module._load_mount_mapping() assert result == [("/m", "/r")] def test_returns_empty_when_mountinfo_missing(self): def _raise(*_a, **_kw): raise OSError(2, "No such file or directory") - with patch("builtins.open", _raise): - assert gateway_module._load_bind_mount_mapping() == [] + with patch("gateway.open", _raise): + assert gateway_module._load_mount_mapping() == [] diff --git a/scripts/build-host-repo-map.py b/scripts/build-host-repo-map.py index ef9d3c6315..608c513009 100755 --- a/scripts/build-host-repo-map.py +++ b/scripts/build-host-repo-map.py @@ -72,8 +72,12 @@ def build_map(config_path: Path) -> dict[str, str]: if not config_path.exists(): return {} - with config_path.open() as fh: - config = yaml.safe_load(fh) or {} + try: + with config_path.open() as fh: + config = yaml.safe_load(fh) or {} + except yaml.YAMLError as exc: + print(f"WARN: failed to parse {config_path}: {exc}", file=sys.stderr) + return {} local_repos = config.get("local_repos") or {} paths = local_repos.get("paths") or [] if isinstance(local_repos, dict) else [] @@ -97,6 +101,9 @@ def build_map(config_path: Path) -> dict[str, str]: file=sys.stderr, ) continue + # Last-wins if two paths resolve to the same owner/repo (e.g. two + # checkouts of the same repo). The later entry in paths silently + # shadows the earlier one. mapping[owner_repo] = str(path) return mapping diff --git a/scripts/tests/test_build_host_repo_map.py b/scripts/tests/test_build_host_repo_map.py index 5b80f8c2eb..16728f41d4 100644 --- a/scripts/tests/test_build_host_repo_map.py +++ b/scripts/tests/test_build_host_repo_map.py @@ -101,6 +101,12 @@ def test_skips_paths_without_git_remote(self, tmp_path): assert build_host_repo_map.build_map(config_path) == {} + def test_corrupted_yaml_returns_empty(self, tmp_path): + config_path = tmp_path / "repositories.yaml" + config_path.write_text("local_repos:\n paths:\n - valid\n bad: [unterminated\n") + + assert build_host_repo_map.build_map(config_path) == {} + def test_handles_missing_local_repos_section(self, tmp_path): config_path = tmp_path / "repositories.yaml" config_path.write_text("github_username: someone\n") From 5f283bcec31300de137baa14d8c510203244f300 Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 16:50:45 +0000 Subject: [PATCH 5/9] Rename override_bind_mounts fixture to override_mounts for consistency --- gateway/tests/test_translate_host_path.py | 34 +++++++++++------------ 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/gateway/tests/test_translate_host_path.py b/gateway/tests/test_translate_host_path.py index b53fb6f993..4ac7e6d668 100644 --- a/gateway/tests/test_translate_host_path.py +++ b/gateway/tests/test_translate_host_path.py @@ -22,7 +22,7 @@ @pytest.fixture -def override_bind_mounts(): +def override_mounts(): """Replace the module-level mount table with a test fixture.""" original = gateway_module._MOUNT_MAPPING @@ -49,9 +49,9 @@ def _set(value: str) -> None: class TestMountinfoTranslation: """Auto-discovered translation via /proc/self/mountinfo.""" - def test_longest_prefix_wins(self, override_bind_mounts, override_host_home): + def test_longest_prefix_wins(self, override_mounts, override_host_home): # A nested hostPath mount must beat its parent emptyDir mount. - override_bind_mounts( + override_mounts( [ ("/home/egg", "/var/lib/kubelet/pods/abc/emptydir/home"), ("/home/egg/.egg-worktrees", "/home/user/.egg-worktrees"), @@ -63,8 +63,8 @@ def test_longest_prefix_wins(self, override_bind_mounts, override_host_home): assert result == "/home/user/.egg-worktrees/pipeline-1/repo" - def test_exact_mount_point_match(self, override_bind_mounts, override_host_home): - override_bind_mounts([("/home/egg/.egg-worktrees", "/home/user/.egg-worktrees")]) + def test_exact_mount_point_match(self, override_mounts, override_host_home): + override_mounts([("/home/egg/.egg-worktrees", "/home/user/.egg-worktrees")]) override_host_home("") assert ( @@ -72,15 +72,15 @@ def test_exact_mount_point_match(self, override_bind_mounts, override_host_home) == "/home/user/.egg-worktrees" ) - def test_sibling_paths_not_conflated(self, override_bind_mounts, override_host_home): + def test_sibling_paths_not_conflated(self, override_mounts, override_host_home): # /home/egg-other must not match the /home/egg mount_point. - override_bind_mounts([("/home/egg", "/host/egg")]) + override_mounts([("/home/egg", "/host/egg")]) override_host_home("") assert gateway_module.translate_to_host_path("/home/egg-other/x") == "/home/egg-other/x" - def test_no_mountinfo_match_falls_through(self, override_bind_mounts, override_host_home): - override_bind_mounts([("/home/egg", "/host/egg")]) + def test_no_mountinfo_match_falls_through(self, override_mounts, override_host_home): + override_mounts([("/home/egg", "/host/egg")]) override_host_home("") assert gateway_module.translate_to_host_path("/other/path") == "/other/path" @@ -89,8 +89,8 @@ def test_no_mountinfo_match_falls_through(self, override_bind_mounts, override_h class TestHostHomeFallback: """Explicit HOST_HOME env var, used when mountinfo lookup misses.""" - def test_host_home_used_when_mountinfo_empty(self, override_bind_mounts, override_host_home): - override_bind_mounts([]) + def test_host_home_used_when_mountinfo_empty(self, override_mounts, override_host_home): + override_mounts([]) override_host_home("/home/user") assert ( @@ -99,11 +99,11 @@ def test_host_home_used_when_mountinfo_empty(self, override_bind_mounts, overrid ) def test_mountinfo_takes_precedence_over_host_home( - self, override_bind_mounts, override_host_home + self, override_mounts, override_host_home ): # mountinfo disagrees with HOST_HOME — trust mountinfo because it # reflects what the kernel actually set up. - override_bind_mounts([("/home/egg/.egg-worktrees", "/real/host/path")]) + override_mounts([("/home/egg/.egg-worktrees", "/real/host/path")]) override_host_home("/stale/home") assert ( @@ -111,8 +111,8 @@ def test_mountinfo_takes_precedence_over_host_home( == "/real/host/path/x" ) - def test_no_translation_when_both_unavailable(self, override_bind_mounts, override_host_home): - override_bind_mounts([]) + def test_no_translation_when_both_unavailable(self, override_mounts, override_host_home): + override_mounts([]) override_host_home("") assert ( @@ -121,9 +121,9 @@ def test_no_translation_when_both_unavailable(self, override_bind_mounts, overri ) def test_host_home_ignored_for_non_container_path( - self, override_bind_mounts, override_host_home + self, override_mounts, override_host_home ): - override_bind_mounts([]) + override_mounts([]) override_host_home("/home/user") assert gateway_module.translate_to_host_path("/other/path") == "/other/path" From ad50a667b074643723b806f7dbb2d67dc1c02780 Mon Sep 17 00:00:00 2001 From: egg Date: Fri, 24 Apr 2026 16:52:15 +0000 Subject: [PATCH 6/9] Fix checks: apply automated formatting fixes --- gateway/tests/test_translate_host_path.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gateway/tests/test_translate_host_path.py b/gateway/tests/test_translate_host_path.py index 4ac7e6d668..ecec4db993 100644 --- a/gateway/tests/test_translate_host_path.py +++ b/gateway/tests/test_translate_host_path.py @@ -98,9 +98,7 @@ def test_host_home_used_when_mountinfo_empty(self, override_mounts, override_hos == "/home/user/.egg-worktrees/x" ) - def test_mountinfo_takes_precedence_over_host_home( - self, override_mounts, override_host_home - ): + def test_mountinfo_takes_precedence_over_host_home(self, override_mounts, override_host_home): # mountinfo disagrees with HOST_HOME — trust mountinfo because it # reflects what the kernel actually set up. override_mounts([("/home/egg/.egg-worktrees", "/real/host/path")]) @@ -120,9 +118,7 @@ def test_no_translation_when_both_unavailable(self, override_mounts, override_ho == "/home/egg/.egg-worktrees/x" ) - def test_host_home_ignored_for_non_container_path( - self, override_mounts, override_host_home - ): + def test_host_home_ignored_for_non_container_path(self, override_mounts, override_host_home): override_mounts([]) override_host_home("/home/user") From 2d8e363813c2868522d1ffde967c590dd5ef072a Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 24 Apr 2026 10:17:02 -0700 Subject: [PATCH 7/9] Require authorized_users; remove repo-owner default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `on-review-feedback.yml` previously defaulted `authorized_users` (both the reusable input and the `EGG_AUTHORIZED_USERS` repo variable) to `jwbron`. That made the canonical repo owner an implicit ambient authorizer for any fork that didn't set the variable. Make the setting explicit: - `authorized_users` input is now `required: true` with no default. - `EGG_AUTHORIZED_USERS` joins `EGG_BOT_USERNAME` in the validate-config step — event-triggered runs fail fast with a clear error listing the missing variable(s) rather than silently authorising the canonical owner. - The resolve-inputs step no longer inlines the `|| 'jwbron'` fallback. - Docs in `github-automation.md` and `reusable-workflows.md` reflect the new required-and-no-default contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/on-review-feedback.yml | 22 +++++++++++++++------- docs/guides/github-automation.md | 13 ++++--------- docs/guides/reusable-workflows.md | 12 +++--------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/.github/workflows/on-review-feedback.yml b/.github/workflows/on-review-feedback.yml index 133339bc8e..564a790ca9 100644 --- a/.github/workflows/on-review-feedback.yml +++ b/.github/workflows/on-review-feedback.yml @@ -39,10 +39,9 @@ on: type: string default: "jwbron/egg/action@main" authorized_users: - description: 'Comma-separated list of GitHub usernames authorized to trigger the bot (via review or @mention)' - required: false + description: 'Comma-separated list of GitHub usernames authorized to trigger the bot (via review or @mention). Required — there is no default.' + required: true type: string - default: "jwbron" reviewer_username: description: 'GitHub username of the reviewer bot (for review trigger detection)' required: false @@ -88,9 +87,18 @@ jobs: steps: - name: Validate required repository variables run: | + missing=() if [[ -z "${{ vars.EGG_BOT_USERNAME }}" ]]; then - echo "::error::Required repository variable 'EGG_BOT_USERNAME' is not set" - echo "::error::Set it in Settings > Secrets and variables > Actions > Variables" + missing+=("EGG_BOT_USERNAME") + fi + if [[ -z "${{ vars.EGG_AUTHORIZED_USERS }}" ]]; then + missing+=("EGG_AUTHORIZED_USERS") + fi + if (( ${#missing[@]} > 0 )); then + for var in "${missing[@]}"; do + echo "::error::Required repository variable '$var' is not set" + done + echo "::error::Set them in Settings > Secrets and variables > Actions > Variables" exit 1 fi echo "All required repository variables are configured" @@ -120,12 +128,12 @@ jobs: echo "bot_username=${{ inputs.bot_username }}" echo "branch_prefix=${{ inputs.branch_prefix }}" echo "reviewer_username=${{ inputs.reviewer_username || '' }}" - echo "authorized_users=${{ inputs.authorized_users || 'jwbron' }}" + echo "authorized_users=${{ inputs.authorized_users }}" else echo "bot_username=${{ vars.EGG_BOT_USERNAME }}" echo "branch_prefix=${{ vars.EGG_BRANCH_PREFIX }}" echo "reviewer_username=${{ vars.EGG_REVIEWER_USERNAME || '' }}" - echo "authorized_users=${{ vars.EGG_AUTHORIZED_USERS || 'jwbron' }}" + echo "authorized_users=${{ vars.EGG_AUTHORIZED_USERS }}" fi echo "max_feedback_rounds=${{ inputs.max_feedback_rounds || '5' }}" diff --git a/docs/guides/github-automation.md b/docs/guides/github-automation.md index b029874f79..3507886fbd 100644 --- a/docs/guides/github-automation.md +++ b/docs/guides/github-automation.md @@ -151,7 +151,7 @@ Without it, the system falls back to posting reviews as comments (self-review mo 1. **Trigger authorization** — For event-triggered runs, verifies the triggering user is authorized: - Bot reviews always trigger (the bot can review its own PRs) - Human reviews and @mentions require the user to be in the `authorized_users` list - - Configured via `EGG_AUTHORIZED_USERS` repository variable (defaults to `jwbron`) + - Configured via the `EGG_AUTHORIZED_USERS` repository variable (required — the workflow fails fast if the variable is unset, so there is no implicit default) - Manual/workflow_call triggers bypass authorization 2. **Filter checks** — Only runs when: @@ -593,16 +593,11 @@ Event-triggered workflows require these repository variables (Settings → Secre |----------|---------|---------| | `EGG_BOT_USERNAME` | Bot's GitHub username for self-trigger prevention | `james-in-a-box[bot]` | | `EGG_BRANCH_PREFIX` | Branch prefix for bot-owned branches | `egg` | +| `EGG_AUTHORIZED_USERS` | Comma-separated list of GitHub users authorized to trigger review feedback via reviews or @mentions | `alice,bob` | -Reusable workflows called via `workflow_call` receive these values as inputs from the caller instead. - -### Optional Repository Variables +`EGG_AUTHORIZED_USERS` controls who can trigger the Address Review Feedback workflow through human reviews or @mentions — the bot itself is always authorized to trigger via automated reviews. The workflow fails fast at the validation step if any required variable is unset, so there is no implicit default. -| Variable | Purpose | Default | -|----------|---------|---------| -| `EGG_AUTHORIZED_USERS` | Comma-separated list of GitHub users authorized to trigger review feedback via reviews or @mentions | `jwbron` | - -This variable controls who can trigger the Address Review Feedback workflow through human reviews or @mentions. The bot itself is always authorized to trigger via automated reviews. +Reusable workflows called via `workflow_call` receive these values as inputs from the caller instead. ### Per-Repository Customization diff --git a/docs/guides/reusable-workflows.md b/docs/guides/reusable-workflows.md index f0d06b0488..968a0220cc 100644 --- a/docs/guides/reusable-workflows.md +++ b/docs/guides/reusable-workflows.md @@ -120,7 +120,7 @@ jobs: | `bot_username` | GitHub username of your bot | Yes | | `branch_prefix` | Prefix for bot-owned branches | Yes | | `action_ref` | Reference to egg action (documentation only; see note) | No | -| `authorized_users` | Comma-separated list of authorized users | No (default: `jwbron`) | +| `authorized_users` | Comma-separated list of authorized users (review-feedback workflow only) | Yes (no default) | | `timeout` | Timeout in minutes | No (varies by workflow) | ## Repository Variables @@ -131,20 +131,14 @@ jobs: |----------|---------|---------| | `EGG_BOT_USERNAME` | Bot's GitHub username | `james-in-a-box[bot]` | | `EGG_BRANCH_PREFIX` | Branch prefix for bot-owned branches | `egg` | - -**OPTIONAL**: Additional variables for customization: - -| Variable | Purpose | Default | -|----------|---------|---------| -| `EGG_AUTHORIZED_USERS` | Comma-separated list of users authorized to trigger review feedback | `jwbron` | +| `EGG_AUTHORIZED_USERS` | Comma-separated list of users authorized to trigger review feedback via reviews or @mentions | `alice,bob` | ### Setting Up Repository Variables 1. Go to your repository's **Settings** → **Secrets and variables** → **Actions** 2. Click the **Variables** tab 3. Click **New repository variable** -4. Add required variables (`EGG_BOT_USERNAME` and `EGG_BRANCH_PREFIX`) -5. Optionally add `EGG_AUTHORIZED_USERS` to control who can trigger feedback via reviews or @mentions +4. Add the required variables (`EGG_BOT_USERNAME`, `EGG_BRANCH_PREFIX`, `EGG_AUTHORIZED_USERS`) **Note**: Workflows will fail with a validation error if required variables are not set. From 4b9516802b642caaf4feadcd6ceed3f5e50bc271 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 24 Apr 2026 10:17:24 -0700 Subject: [PATCH 8/9] Genericize example values in tests, docs, and code comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo was dotted with illustrative references to this project's owner (`jwbron/egg`), the author's home path (`/home/jwies`), and a specific downstream org (`Khan/webapp`). They were never runtime dependencies — just arbitrary examples inside docstrings, CLI usage tables, schema descriptions, agent-output log snippets, test fixtures, and code comments — but they implied the code assumed one developer's setup. Swap them to neutral placeholders (`owner/repo`, `owner/repo-checkpoints`, `/home/user`, `my-org`, `owner--repo`) across: - Test fixture data and assertions (gateway, orchestrator, sandbox, shared) — retained the canonical string only where tests exercise the `EGG_REPO` runtime gate itself (`reviewer_agent_design` filter, `test_git_remote_takes_precedence_over_egg_repo`). - Code-comment examples in `orchestrator/kubernetes_spawner.py` and `sandbox/entrypoint.py`. - Docstring examples (`gateway/gateway.py`, `config/repo_config.py`, `shared/egg_contracts/checkpoints.py`). - MCP tool schema descriptions (`orchestrator/mcp_tools.py`). - JSON Schema `description` fields under `.egg/schemas/`. - Arch/guide/reference docs (logging, orchestrator, checkpoint-access, custom-phase, mcp-deployment-tools, checkpoint-browser, sdlc-pipeline). - Skill files (`skills/sdlc/SKILL.md`, `skills/babysit-pr/SKILL.md`). - `action/generate-config.sh` comment. Canonical references to the `jwbron/egg` project are intentionally left alone: - GitHub issue / PR URLs - Schema `$id` URLs - Reusable-workflow invocation paths (`jwbron/egg/.github/workflows/…@…`) - GitHub Action refs (`jwbron/egg/action@main`, `jwbron/egg@main`) - GHCR image names (`ghcr.io/jwbron/egg-*`) - The `EGG_REPO` runtime constant that gates the `reviewer_agent_design` role — `docs/reference/agent-roles.md` now calls out that this is a hardcoded scope tied to this project's identity. Co-Authored-By: Claude Opus 4.7 (1M context) --- .egg/schemas/checkpoint.schema.json | 2 +- action/generate-config.sh | 2 +- config/secrets.template.env | 2 +- docs/architecture/logging.md | 2 +- docs/architecture/orchestrator.md | 2 +- docs/architecture/sdlc-pipeline.md | 2 +- docs/guides/checkpoint-access.md | 20 ++++++------ docs/guides/custom-phase.md | 16 +++++----- docs/guides/sdlc-pipeline.md | 2 +- docs/reference/agent-roles.md | 2 +- docs/reference/checkpoint-browser.md | 16 +++++----- docs/reference/mcp-deployment-tools.md | 2 +- gateway/tests/test_checkpoint_handler.py | 32 +++++++++---------- gateway/tests/test_checkpoint_read.py | 6 ++-- gateway/tests/test_commit_observer.py | 18 +++++------ gateway/tests/test_commit_registry_client.py | 4 +-- gateway/tests/test_execute_filtered_push.py | 6 ++-- gateway/tests/test_session_manager.py | 12 +++---- orchestrator/README.md | 2 +- orchestrator/kubernetes_spawner.py | 2 +- orchestrator/mcp_tools.py | 4 +-- .../tests/test_commit_authorship_store.py | 6 ++-- orchestrator/tests/test_mcp_tools.py | 10 +++--- .../tests/test_mcp_tools_enrichment.py | 2 +- sandbox/egg_lib/sdlc_cli.py | 2 +- sandbox/entrypoint.py | 4 +-- .../test_cli_push_scope_filter_removed.py | 2 +- shared/egg_contracts/checkpoints.py | 2 +- skills/babysit-pr/SKILL.md | 2 +- skills/sdlc/SKILL.md | 8 ++--- tests/config/test_repo_config_checkpoint.py | 12 +++---- tests/sandbox/test_gh_wrapper.py | 10 +++--- .../egg_contracts/test_checkpoint_cli.py | 20 ++++++------ .../egg_contracts/test_checkpoint_cli_http.py | 20 ++++++------ .../egg_contracts/test_checkpoint_loader.py | 10 +++--- .../shared/egg_contracts/test_checkpoints.py | 12 +++---- tests/shared/egg_contracts/test_models.py | 4 +-- 37 files changed, 141 insertions(+), 141 deletions(-) diff --git a/.egg/schemas/checkpoint.schema.json b/.egg/schemas/checkpoint.schema.json index fff3793206..da879f834f 100644 --- a/.egg/schemas/checkpoint.schema.json +++ b/.egg/schemas/checkpoint.schema.json @@ -80,7 +80,7 @@ }, "repo": { "type": ["string", "null"], - "description": "Source repository in owner/repo format (e.g. 'jwbron/egg')", + "description": "Source repository in owner/repo format (e.g. 'owner/repo')", "pattern": "^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$", "default": null }, diff --git a/action/generate-config.sh b/action/generate-config.sh index 6b62a7ff19..21347b39e8 100755 --- a/action/generate-config.sh +++ b/action/generate-config.sh @@ -7,7 +7,7 @@ # - launcher-secret (auth token for launcher API calls) # # Required environment variables: -# GITHUB_REPOSITORY — owner/repo (e.g., "jwbron/egg") +# GITHUB_REPOSITORY — owner/repo (e.g., "owner/repo") # GITHUB_ACTOR — GitHub username triggering the workflow # GITHUB_ACTOR_ID — Numeric ID for noreply email # INPUT_ANTHROPIC_OAUTH_TOKEN — Anthropic OAuth token diff --git a/config/secrets.template.env b/config/secrets.template.env index 4a02eb5af5..cf31fd503f 100644 --- a/config/secrets.template.env +++ b/config/secrets.template.env @@ -85,7 +85,7 @@ GATEWAY_BOT_BRANCH_PREFIX="" # Trusted GitHub usernames whose branches the bot can push to (optional) # Comma-separated list of usernames (case-insensitive) -# Example: GATEWAY_TRUSTED_USERS="jwbron,octocat" +# Example: GATEWAY_TRUSTED_USERS="alice,bob" GATEWAY_TRUSTED_USERS="" # ============================================================================= diff --git a/docs/architecture/logging.md b/docs/architecture/logging.md index 86d0deec28..3ef6ee82d4 100644 --- a/docs/architecture/logging.md +++ b/docs/architecture/logging.md @@ -69,7 +69,7 @@ All log entries include these fields: "traceFlags": "01", "context": { "task_id": "bd-xyz789", - "repository": "jwbron/egg", + "repository": "owner/repo", "pr_number": 123 } } diff --git a/docs/architecture/orchestrator.md b/docs/architecture/orchestrator.md index a77c3d9818..8d4ad15ae6 100644 --- a/docs/architecture/orchestrator.md +++ b/docs/architecture/orchestrator.md @@ -470,7 +470,7 @@ The `wait_for_status_change` tool is the event-triggered sibling of `get_status` Available MCP tools (gateway-backed, requires `gateway_url`): `list_checkpoints`, `search_checkpoints`, `get_contract` -The gateway-backed checkpoint tools (`list_checkpoints`, `search_checkpoints`) accept an optional `repo` parameter to specify the checkpoint repository in `owner/repo` format (e.g., `jwbron/egg-checkpoints`). When provided, this is forwarded as the `source_repo` query parameter to the gateway checkpoint endpoint. The `get_contract` tool also uses the gateway session but does not require the `repo` parameter. +The gateway-backed checkpoint tools (`list_checkpoints`, `search_checkpoints`) accept an optional `repo` parameter to specify the checkpoint repository in `owner/repo` format (e.g., `owner/repo-checkpoints`). When provided, this is forwarded as the `source_repo` query parameter to the gateway checkpoint endpoint. The `get_contract` tool also uses the gateway session but does not require the `repo` parameter. **CLI Access:** The `egg-orch` CLI (`sandbox/bin/egg-orch`) provides command-line access to all orchestrator API endpoints. Available in sandbox containers for agent use, or can be run from the host with appropriate environment variables. See the [README CLI Reference](../../README.md#egg-orch-cli) for command details. diff --git a/docs/architecture/sdlc-pipeline.md b/docs/architecture/sdlc-pipeline.md index b7ecbdf61d..0aadbed366 100644 --- a/docs/architecture/sdlc-pipeline.md +++ b/docs/architecture/sdlc-pipeline.md @@ -98,7 +98,7 @@ The contract is a JSON document tracking the complete state of an issue through "review_cycles": 1 }] }], - "workflow_owner": "jwbron", + "workflow_owner": "my-org", "audit_log": [...] } ``` diff --git a/docs/guides/checkpoint-access.md b/docs/guides/checkpoint-access.md index f596b11fa3..cf11d64c49 100644 --- a/docs/guides/checkpoint-access.md +++ b/docs/guides/checkpoint-access.md @@ -14,8 +14,8 @@ Both `--repo-path` and `--checkpoint-repo` can be placed **before or after** the ```bash # These are equivalent: -egg-checkpoint --checkpoint-repo jwbron/egg-checkpoints list --issue 42 -egg-checkpoint list --checkpoint-repo jwbron/egg-checkpoints --issue 42 +egg-checkpoint --checkpoint-repo owner/repo-checkpoints list --issue 42 +egg-checkpoint list --checkpoint-repo owner/repo-checkpoints --issue 42 ``` If the flag is supplied in both positions, the last value wins. @@ -151,7 +151,7 @@ All list/context filters use AND logic (all must match). Filters available: | Issue | `--issue N` | `--issue 530` | | PR | `--pr N` | `--pr 42` | | Pipeline | `--pipeline ID` | `--pipeline issue-530` | -| Repo | `--repo OWNER/REPO` | `--repo jwbron/egg` | +| Repo | `--repo OWNER/REPO` | `--repo owner/repo` | | Session | `--session ID` | `--session container-abc` | | Branch | `--branch NAME` | `--branch egg/feature` | | Trigger | `--trigger TYPE` | `--trigger commit` or `--trigger session_end` | @@ -184,7 +184,7 @@ Supported composite role names: `reviewer_code`, `reviewer_contract`, `reviewer_ When a query matches no checkpoints, the CLI now prints the repository and branch it searched to stderr: ``` -Searched jwbron/egg branch egg/checkpoints/v2 +Searched owner/repo branch egg/checkpoints/v2 No checkpoints found matching filters ``` @@ -269,7 +269,7 @@ egg-checkpoint cost --issue $EGG_ISSUE_NUMBER ### "No checkpoints found" -The CLI now shows which repository and branch it searched when no results are found (e.g., `Searched jwbron/egg branch egg/checkpoints/v2`). Check the displayed repo/branch — if it's unexpected: +The CLI now shows which repository and branch it searched when no results are found (e.g., `Searched owner/repo branch egg/checkpoints/v2`). Check the displayed repo/branch — if it's unexpected: 1. **Checkpoints in a separate repo**: Some projects store checkpoints in a dedicated repo (e.g., `owner/project-checkpoints`). Set the `EGG_CHECKPOINT_REPO` env var or use `--checkpoint-repo`. 2. **Missing `repositories.yaml`**: Auto-detection relies on a config file that may not exist in the sandbox. Set the env var instead. @@ -314,7 +314,7 @@ List checkpoints with optional filters. ``` list_checkpoints(issue=1489, phase="implement") list_checkpoints(pipeline="issue-1489", agent_type="coder") -list_checkpoints(issue=1489, repo="jwbron/egg-checkpoints") +list_checkpoints(issue=1489, repo="owner/repo-checkpoints") ``` **Parameters:** `issue` (int), `pipeline` (string), `agent_type` (string), `phase` (string), `status` (string), `repo` (string, `owner/repo` format), `limit` (int, default 20) @@ -325,18 +325,18 @@ Search checkpoint metadata for matching text (searches agent_type, pipeline_phas ``` search_checkpoints(text="coder", pipeline="issue-1489") -search_checkpoints(text="reviewer", repo="jwbron/egg-checkpoints") +search_checkpoints(text="reviewer", repo="owner/repo-checkpoints") ``` **Parameters:** `text` (string, required), `issue` (int), `pipeline` (string), `agent_type` (string), `repo` (string, `owner/repo` format), `limit` (int, default 10) ### Specifying the checkpoint repository -When checkpoints are stored in a separate repository (e.g., `jwbron/egg-checkpoints`), use the `repo` parameter to target it: +When checkpoints are stored in a separate repository (e.g., `owner/repo-checkpoints`), use the `repo` parameter to target it: ``` -list_checkpoints(issue=1489, repo="jwbron/egg-checkpoints") -search_checkpoints(text="error", repo="jwbron/egg-checkpoints") +list_checkpoints(issue=1489, repo="owner/repo-checkpoints") +search_checkpoints(text="error", repo="owner/repo-checkpoints") ``` The `repo` value is forwarded as `source_repo` to the gateway checkpoint endpoint. diff --git a/docs/guides/custom-phase.md b/docs/guides/custom-phase.md index 4559c4aa6d..450785fb3d 100644 --- a/docs/guides/custom-phase.md +++ b/docs/guides/custom-phase.md @@ -73,7 +73,7 @@ a local `kubectl port-forward`): run_agent_task( phase = "refine", roles = ["refiner"], - repo = "jwbron/egg", + repo = "owner/repo", description = "Investigate how concurrent_executor filters the review graph" ) ``` @@ -83,7 +83,7 @@ Minimal form — default roster, no branch, no PR, no upstream contract: ``` run_agent_task( phase = "implement", - repo = "jwbron/egg", + repo = "owner/repo", description = "Fix typo in README.md under ## Quickstart" ) ``` @@ -250,7 +250,7 @@ would be." run_agent_task( phase = "refine", roles = ["refiner"], - repo = "jwbron/egg", + repo = "owner/repo", description = "Evaluate cost of migrating integration tests off compose" ) ``` @@ -271,7 +271,7 @@ overkill (no tester needed, no documenter needed): run_agent_task( phase = "implement", roles = ["coder"], - repo = "jwbron/egg", + repo = "owner/repo", description = "Fix log-level typo in orchestrator/routes/pipelines.py line 42" ) ``` @@ -289,7 +289,7 @@ documenter churn: run_agent_task( phase = "implement", roles = ["coder", "reviewer_code"], - repo = "jwbron/egg", + repo = "owner/repo", description = "Refactor _handle_submit_task to share a common validator helper with _handle_babysit_pr" ) ``` @@ -305,12 +305,12 @@ branches. This is the **subsumption path** (decision-2): the underlying run_agent_task( phase = "implement", pr_number = 1234, - repo = "jwbron/egg", + repo = "owner/repo", description = "Improve test coverage on the PR's new validator helper" ) ``` -This is equivalent to `babysit_pr(pr_number=1234, repo="jwbron/egg")` +This is equivalent to `babysit_pr(pr_number=1234, repo="owner/repo")` end-to-end — use `babysit_pr` for the canonical PR-improvement flow, and `run_agent_task` when you want a non-default roster on a PR. @@ -323,7 +323,7 @@ do): ``` run_agent_task( phase = "plan", - repo = "jwbron/egg", + repo = "owner/repo", description = "Plan the refactor of _run_concurrent_phase", analysis = "" ) diff --git a/docs/guides/sdlc-pipeline.md b/docs/guides/sdlc-pipeline.md index 891469b389..54dd503ed8 100644 --- a/docs/guides/sdlc-pipeline.md +++ b/docs/guides/sdlc-pipeline.md @@ -476,7 +476,7 @@ The local orchestrator handles concurrent contract updates through `orchestrator } ], "decisions": [], - "workflow_owner": "jwbron", + "workflow_owner": "my-org", "audit_log": [] } ``` diff --git a/docs/reference/agent-roles.md b/docs/reference/agent-roles.md index 7764a0d5c9..16df59c5a8 100644 --- a/docs/reference/agent-roles.md +++ b/docs/reference/agent-roles.md @@ -68,7 +68,7 @@ All agents within a phase run concurrently via BRC consensus. Concurrency is ena ### `reviewer_agent_design` -**Scope**: Egg repo only (`jwbron/egg`). Not spawned for pipelines on other repos. +**Scope**: Egg repo only (`jwbron/egg`). Not spawned for pipelines on other repos. The canonical repo string is hardcoded in `shared/egg_contracts/agent_roles.py` (`EGG_REPO`). **Purpose**: Review the analysis for agent-mode alignment and anti-patterns (e.g., correct use of egg's structural enforcement model). diff --git a/docs/reference/checkpoint-browser.md b/docs/reference/checkpoint-browser.md index e87239a009..e09b441626 100644 --- a/docs/reference/checkpoint-browser.md +++ b/docs/reference/checkpoint-browser.md @@ -24,8 +24,8 @@ The `--checkpoint-repo` and `--repo-path` flags can be placed **before or after* ```bash # Both of these are equivalent: -egg-checkpoint --checkpoint-repo jwbron/egg-checkpoints list --issue 42 -egg-checkpoint list --checkpoint-repo jwbron/egg-checkpoints --issue 42 +egg-checkpoint --checkpoint-repo owner/repo-checkpoints list --issue 42 +egg-checkpoint list --checkpoint-repo owner/repo-checkpoints --issue 42 ``` If the flag is supplied in both positions, the last value wins (standard argparse behavior). @@ -52,7 +52,7 @@ All list/context filters use AND logic. Common filters: | Issue | `--issue N` | `--issue 530` | | PR | `--pr N` | `--pr 42` | | Pipeline | `--pipeline ID` | `--pipeline issue-530` | -| Repo | `--repo OWNER/REPO` | `--repo jwbron/egg` | +| Repo | `--repo OWNER/REPO` | `--repo owner/repo` | | Agent | `--agent-type TYPE` | `--agent-type coder` or `--agent-type reviewer_code` | | Phase | `--phase PHASE` | `--phase implement` | | Status | `--status STATUS` | `--status failed` | @@ -88,7 +88,7 @@ egg-checkpoint list --agent-type reviewer --pipeline $EGG_PIPELINE_ID When a query matches no checkpoints, the CLI prints the repository and branch that were searched to stderr, helping diagnose whether the correct checkpoint source was used: ``` -Searched jwbron/egg branch egg/checkpoints/v2 +Searched owner/repo branch egg/checkpoints/v2 No checkpoints found matching filters ``` @@ -133,7 +133,7 @@ egg-checkpoint cost --issue $EGG_ISSUE_NUMBER ## Troubleshooting -**"No checkpoints found"**: The CLI now prints which repository and branch it searched (e.g., `Searched jwbron/egg branch egg/checkpoints/v2`) to stderr, making it easier to diagnose configuration issues. If the repo/branch shown is unexpected: +**"No checkpoints found"**: The CLI now prints which repository and branch it searched (e.g., `Searched owner/repo branch egg/checkpoints/v2`) to stderr, making it easier to diagnose configuration issues. If the repo/branch shown is unexpected: 1. Check if checkpoints are in a separate repo — set `EGG_CHECKPOINT_REPO=OWNER/REPO` or use `--checkpoint-repo` 2. Check if `repositories.yaml` exists (it may not be present in the sandbox) 3. Try listing without metadata filters — some checkpoints (ad-hoc sessions) have no issue/pipeline metadata @@ -153,13 +153,13 @@ The orchestrator MCP server (port 9850) exposes checkpoint data via two tools: | `list_checkpoints` | List checkpoints with filters (issue, pipeline, agent_type, phase, status, repo) | | `search_checkpoints` | Search checkpoint metadata by text with filters (issue, pipeline, agent_type, repo) | -Both tools accept an optional `repo` parameter to specify the checkpoint repository in `owner/repo` format (e.g., `jwbron/egg-checkpoints`). This is useful when checkpoints are stored in a separate repository from the main codebase. +Both tools accept an optional `repo` parameter to specify the checkpoint repository in `owner/repo` format (e.g., `owner/repo-checkpoints`). This is useful when checkpoints are stored in a separate repository from the main codebase. **Examples:** ``` list_checkpoints(issue=1489, phase="implement") -list_checkpoints(issue=1489, repo="jwbron/egg-checkpoints") -search_checkpoints(text="coder", pipeline="issue-1489", repo="jwbron/egg-checkpoints") +list_checkpoints(issue=1489, repo="owner/repo-checkpoints") +search_checkpoints(text="coder", pipeline="issue-1489", repo="owner/repo-checkpoints") ``` ## Related CLIs diff --git a/docs/reference/mcp-deployment-tools.md b/docs/reference/mcp-deployment-tools.md index 82c9bc3329..cc07999bc3 100644 --- a/docs/reference/mcp-deployment-tools.md +++ b/docs/reference/mcp-deployment-tools.md @@ -255,7 +255,7 @@ configured repo — there is no per-repo scope parameter. ```json { "git_worktree_prune": { - "jwbron/egg": ["/home/egg/.egg-worktrees/stale-1", "/home/egg/.egg-worktrees/stale-2"] + "owner/repo": ["/home/egg/.egg-worktrees/stale-1", "/home/egg/.egg-worktrees/stale-2"] }, "orphan_dirs": ["/home/egg/.egg-worktrees/orphan-dir-3"], "dry_run": true diff --git a/gateway/tests/test_checkpoint_handler.py b/gateway/tests/test_checkpoint_handler.py index 664bc80dd7..f3668da3e9 100644 --- a/gateway/tests/test_checkpoint_handler.py +++ b/gateway/tests/test_checkpoint_handler.py @@ -431,7 +431,7 @@ def test_explicit_checkpoint_repo_skips_auto_detection( session=session, session_status=SessionStatus.COMPLETED, repo_path="/home/egg/repos/test-repo", - checkpoint_repo="jwbron/egg-checkpoints", + checkpoint_repo="owner/repo-checkpoints", async_store=False, ) @@ -440,8 +440,8 @@ def test_explicit_checkpoint_repo_skips_auto_detection( mock_auto_detect.assert_not_called() # store_checkpoint_v2 should receive the explicit checkpoint_repo call_kwargs = mock_handler.store_checkpoint_v2.call_args - assert call_kwargs[1].get("checkpoint_repo") == "jwbron/egg-checkpoints" or ( - len(call_kwargs[0]) > 2 and call_kwargs[0][2] == "jwbron/egg-checkpoints" + assert call_kwargs[1].get("checkpoint_repo") == "owner/repo-checkpoints" or ( + len(call_kwargs[0]) > 2 and call_kwargs[0][2] == "owner/repo-checkpoints" ) @patch("checkpoint_handler._get_checkpoint_repo_for_path") @@ -457,7 +457,7 @@ def test_auto_detection_used_when_checkpoint_repo_is_none( TriggerType, ) - mock_auto_detect.return_value = "jwbron/egg-checkpoints" + mock_auto_detect.return_value = "owner/repo-checkpoints" now = datetime.now(UTC) mock_handler = MagicMock() @@ -486,7 +486,7 @@ def test_auto_detection_used_when_checkpoint_repo_is_none( mock_auto_detect.assert_called_once_with("/home/egg/repos/test-repo") # store_checkpoint_v2 should receive the auto-detected checkpoint_repo call_kwargs = mock_handler.store_checkpoint_v2.call_args - assert call_kwargs[1].get("checkpoint_repo") == "jwbron/egg-checkpoints" + assert call_kwargs[1].get("checkpoint_repo") == "owner/repo-checkpoints" class TestAutoCommitShaInCheckpoint: @@ -621,7 +621,7 @@ def track_run_git(cwd, args, **kwargs): try: handler.store_checkpoint_v2( - checkpoint, "/fake/repo", checkpoint_repo="jwbron/egg-checkpoints" + checkpoint, "/fake/repo", checkpoint_repo="owner/repo-checkpoints" ) except Exception: pass @@ -973,27 +973,27 @@ def test_https_url(self, mock_run): """Extracts owner/repo from HTTPS remote URL.""" mock_run.return_value = MagicMock( returncode=0, - stdout="https://github.com/jwbron/egg.git\n", + stdout="https://github.com/owner/repo.git\n", ) - assert _extract_repo_from_remote("/some/repo") == "jwbron/egg" + assert _extract_repo_from_remote("/some/repo") == "owner/repo" @patch("checkpoint_handler.subprocess.run") def test_https_url_without_git_suffix(self, mock_run): """Extracts owner/repo from HTTPS URL without .git suffix.""" mock_run.return_value = MagicMock( returncode=0, - stdout="https://github.com/jwbron/egg\n", + stdout="https://github.com/owner/repo\n", ) - assert _extract_repo_from_remote("/some/repo") == "jwbron/egg" + assert _extract_repo_from_remote("/some/repo") == "owner/repo" @patch("checkpoint_handler.subprocess.run") def test_ssh_url(self, mock_run): """Extracts owner/repo from SSH remote URL.""" mock_run.return_value = MagicMock( returncode=0, - stdout="git@github.com:jwbron/egg.git\n", + stdout="git@github.com:owner/repo.git\n", ) - assert _extract_repo_from_remote("/some/repo") == "jwbron/egg" + assert _extract_repo_from_remote("/some/repo") == "owner/repo" @patch("checkpoint_handler.subprocess.run") def test_ssh_url_without_git_suffix(self, mock_run): @@ -1081,10 +1081,10 @@ def test_from_repo_path(self, mock_extract): """Resolves repo from repo_path.""" from checkpoint_handler import CheckpointHandler - mock_extract.return_value = "jwbron/egg" + mock_extract.return_value = "owner/repo" handler = CheckpointHandler() result = handler._resolve_repo("/home/egg/repos/egg", None) - assert result == "jwbron/egg" + assert result == "owner/repo" mock_extract.assert_called_once_with("/home/egg/repos/egg") @patch("checkpoint_handler._extract_repo_from_remote") @@ -1119,12 +1119,12 @@ def test_none_repo_path_with_session(self, mock_extract): """Uses session.last_repo_path when repo_path is None.""" from checkpoint_handler import CheckpointHandler - mock_extract.return_value = "jwbron/egg" + mock_extract.return_value = "owner/repo" handler = CheckpointHandler() session = _make_test_session() session.last_repo_path = "/home/egg/repos/egg" result = handler._resolve_repo(None, session) - assert result == "jwbron/egg" + assert result == "owner/repo" mock_extract.assert_called_once_with("/home/egg/repos/egg") @patch("checkpoint_handler._extract_repo_from_remote") diff --git a/gateway/tests/test_checkpoint_read.py b/gateway/tests/test_checkpoint_read.py index d73f674824..7af01401aa 100644 --- a/gateway/tests/test_checkpoint_read.py +++ b/gateway/tests/test_checkpoint_read.py @@ -440,7 +440,7 @@ def test_source_repo_used_when_auto_detection_fails(self, app, client, auth_head mock_get_handler.return_value = mock_handler response = client.get( - "/api/v1/checkpoints?issue=738&source_repo=jwbron/egg", + "/api/v1/checkpoints?issue=738&source_repo=owner/repo", headers=auth_headers, ) assert response.status_code == 200 @@ -566,7 +566,7 @@ def test_uses_source_repo_when_repo_path_has_no_remote(self, app): """Falls back to source_repo param for token resolution.""" import gateway as gw - with app.test_request_context("/?source_repo=jwbron/egg"): + with app.test_request_context("/?source_repo=owner/repo"): with ( patch("checkpoint_handler._resolve_github_token", return_value=None), patch.object(gw, "get_token_for_repo", return_value=("ghp_test", "bot", "")), @@ -578,7 +578,7 @@ def test_returns_token_from_repo_path_when_available(self, app): """Uses standard token resolution when repo_path has a remote.""" import gateway as gw - with app.test_request_context("/?source_repo=jwbron/egg"): + with app.test_request_context("/?source_repo=owner/repo"): with patch("checkpoint_handler._resolve_github_token", return_value="ghp_from_remote"): result = gw._resolve_checkpoint_token("/repo") assert result == "ghp_from_remote" diff --git a/gateway/tests/test_commit_observer.py b/gateway/tests/test_commit_observer.py index 42ac790956..baccc14389 100644 --- a/gateway/tests/test_commit_observer.py +++ b/gateway/tests/test_commit_observer.py @@ -73,7 +73,7 @@ def test_observe_with_no_role_returns_empty(self): branch="main", session_role=None, pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == [] @@ -89,7 +89,7 @@ def test_observe_with_empty_role_returns_empty(self): branch="main", session_role="", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == [] @@ -125,7 +125,7 @@ def test_before_equals_after_is_noop(self): branch="main", session_role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == [] @@ -140,7 +140,7 @@ def test_missing_after_is_noop(self): branch="main", session_role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == [] @@ -175,7 +175,7 @@ def fake_rev_list(*args, **kwargs): branch="egg/issue-1882", session_role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == [after] @@ -198,7 +198,7 @@ def test_unborn_branch_registers_after_head_only(self): branch="egg/issue-1882", session_role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) # No rev-list walk → no subprocess → falls back to [after_head]. @@ -235,7 +235,7 @@ def fake_rev_list(*args, **kwargs): branch="egg/issue-1882", session_role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == new_shas @@ -247,7 +247,7 @@ def fake_rev_list(*args, **kwargs): for item in items: assert item["role"] == "coder" assert item["pipeline_id"] == "issue-1882" - assert item["repo"] == "jwbron/egg" + assert item["repo"] == "owner/repo" def test_rev_list_nonzero_falls_back_to_after_head(self, monkeypatch): """If rev-list fails (e.g. rewritten history), register just the tip.""" @@ -271,7 +271,7 @@ def fake_rev_list(*args, **kwargs): branch="egg/issue-1882", session_role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", registry_client=client, ) assert result == [after] diff --git a/gateway/tests/test_commit_registry_client.py b/gateway/tests/test_commit_registry_client.py index 7caf66cb1c..e02cafd7e1 100644 --- a/gateway/tests/test_commit_registry_client.py +++ b/gateway/tests/test_commit_registry_client.py @@ -63,7 +63,7 @@ def fake_post(path, payload, *, timeout): sha=_VALID_SHA, role="coder", pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", branch="egg/issue-1882", ) is True @@ -74,7 +74,7 @@ def fake_post(path, payload, *, timeout): assert payload["sha"] == _VALID_SHA assert payload["role"] == "coder" assert payload["pipeline_id"] == "issue-1882" - assert payload["repo"] == "jwbron/egg" + assert payload["repo"] == "owner/repo" assert payload["branch"] == "egg/issue-1882" def test_register_409_is_benign_success(self, client, monkeypatch): diff --git a/gateway/tests/test_execute_filtered_push.py b/gateway/tests/test_execute_filtered_push.py index a94b4e254e..22045edc9a 100644 --- a/gateway/tests/test_execute_filtered_push.py +++ b/gateway/tests/test_execute_filtered_push.py @@ -204,7 +204,7 @@ def test_rewrite_strips_blocked_paths(self, repo: Path): push_fn=_PushStub(), registry_register=_RegistryStub(), pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", ) assert result.success is True assert "docs/README.md" in result.excluded_files @@ -616,14 +616,14 @@ def test_registers_rewritten_own_commit(self, repo: Path): push_fn=_PushStub(), registry_register=registry, pipeline_id="issue-1882", - repo="jwbron/egg", + repo="owner/repo", ) assert result.success assert len(registry.calls) == 1 call = registry.calls[0] assert call["role"] == "coder" assert call["pipeline_id"] == "issue-1882" - assert call["repo"] == "jwbron/egg" + assert call["repo"] == "owner/repo" assert call["sha"] == result.rewritten_commits[0]["new_sha"] def test_registry_exception_swallowed(self, repo: Path): diff --git a/gateway/tests/test_session_manager.py b/gateway/tests/test_session_manager.py index 8cb99b0efe..05007eae3c 100644 --- a/gateway/tests/test_session_manager.py +++ b/gateway/tests/test_session_manager.py @@ -1470,10 +1470,10 @@ def test_session_with_checkpoint_fields(self): created_at=now, last_seen=now, expires_at=now + timedelta(hours=24), - checkpoint_repo="jwbron/egg-checkpoints", + checkpoint_repo="owner/repo-checkpoints", last_repo_path="/home/egg/repos/egg", ) - assert session.checkpoint_repo == "jwbron/egg-checkpoints" + assert session.checkpoint_repo == "owner/repo-checkpoints" assert session.last_repo_path == "/home/egg/repos/egg" def test_to_dict_includes_checkpoint_fields(self): @@ -1488,11 +1488,11 @@ def test_to_dict_includes_checkpoint_fields(self): created_at=now, last_seen=now, expires_at=now + timedelta(hours=24), - checkpoint_repo="jwbron/egg-checkpoints", + checkpoint_repo="owner/repo-checkpoints", last_repo_path="/home/egg/repos/egg", ) d = session.to_dict_for_persistence() - assert d["checkpoint_repo"] == "jwbron/egg-checkpoints" + assert d["checkpoint_repo"] == "owner/repo-checkpoints" assert d["last_repo_path"] == "/home/egg/repos/egg" def test_to_dict_excludes_none_checkpoint_fields(self): @@ -1524,12 +1524,12 @@ def test_roundtrip_with_checkpoint_fields(self): created_at=now, last_seen=now, expires_at=now + timedelta(hours=24), - checkpoint_repo="jwbron/egg-checkpoints", + checkpoint_repo="owner/repo-checkpoints", last_repo_path="/home/egg/repos/egg", ) d = session.to_dict_for_persistence() restored = Session.from_persistence(d) - assert restored.checkpoint_repo == "jwbron/egg-checkpoints" + assert restored.checkpoint_repo == "owner/repo-checkpoints" assert restored.last_repo_path == "/home/egg/repos/egg" def test_backward_compatibility_without_checkpoint_fields(self): diff --git a/orchestrator/README.md b/orchestrator/README.md index a7359bf2d7..28cbdaf8a5 100644 --- a/orchestrator/README.md +++ b/orchestrator/README.md @@ -322,7 +322,7 @@ These tools require a `gateway_url` and authenticate via a gateway session. The | `search_checkpoints` | Search checkpoint metadata by text with filters (issue, pipeline, agent_type, repo, limit) | | `get_contract` | Get SDLC contract state by issue number or task ID | -Both checkpoint tools accept an optional `repo` parameter (string, `owner/repo` format) to specify the checkpoint repository when checkpoints are stored separately (e.g., `jwbron/egg-checkpoints`). The value is forwarded as `source_repo` to the gateway. +Both checkpoint tools accept an optional `repo` parameter (string, `owner/repo` format) to specify the checkpoint repository when checkpoints are stored separately (e.g., `owner/repo-checkpoints`). The value is forwarded as `source_repo` to the gateway. ### Orchestrator-Backed Tools diff --git a/orchestrator/kubernetes_spawner.py b/orchestrator/kubernetes_spawner.py index 6f45039573..71c8fec81b 100644 --- a/orchestrator/kubernetes_spawner.py +++ b/orchestrator/kubernetes_spawner.py @@ -676,7 +676,7 @@ def spawn_agent_job( for owner_repo, host_path in (repo_volumes or {}).items(): # Include the owner in the k8s volume name so two repos # with the same basename from different orgs don't collide - # (e.g. "Khan/webapp" and "other-org/webapp" both produce + # (e.g. "my-org/webapp" and "other-org/webapp" both produce # container path /home/egg/repos/webapp, but need distinct # volume names). Normalize to RFC-1123 (lowercase, hyphens) # and truncate to fit the 63-char name limit. diff --git a/orchestrator/mcp_tools.py b/orchestrator/mcp_tools.py index 40388730e8..59b9937f42 100644 --- a/orchestrator/mcp_tools.py +++ b/orchestrator/mcp_tools.py @@ -621,7 +621,7 @@ def _is_timeout_error(exc: BaseException) -> bool: }, "repo": { "type": "string", - "description": "Checkpoint repository in owner/repo format, e.g. jwbron/egg-checkpoints", + "description": "Checkpoint repository in owner/repo format, e.g. owner/repo-checkpoints", }, }, }, @@ -659,7 +659,7 @@ def _is_timeout_error(exc: BaseException) -> bool: }, "repo": { "type": "string", - "description": "Checkpoint repository in owner/repo format, e.g. jwbron/egg-checkpoints", + "description": "Checkpoint repository in owner/repo format, e.g. owner/repo-checkpoints", }, }, "required": ["text"], diff --git a/orchestrator/tests/test_commit_authorship_store.py b/orchestrator/tests/test_commit_authorship_store.py index 99ea5a8336..30f24241c6 100644 --- a/orchestrator/tests/test_commit_authorship_store.py +++ b/orchestrator/tests/test_commit_authorship_store.py @@ -103,12 +103,12 @@ def test_register_captures_metadata(self, store: CommitAuthorshipStore): _VALID_SHA, "coder", "issue-1882", - repo="jwbron/egg", + repo="owner/repo", branch="egg/issue-1882", ) shard = store.worktree / SUBSTORE_DIR / "issue-1882.json" entry = json.loads(shard.read_text())["entries"][_VALID_SHA] - assert entry["repo"] == "jwbron/egg" + assert entry["repo"] == "owner/repo" assert entry["branch"] == "egg/issue-1882" assert entry["registered_at"] # non-empty ISO8601 @@ -202,7 +202,7 @@ def test_collision_preserves_original_metadata( _VALID_SHA, "coder", "issue-1882", - repo="jwbron/egg", + repo="owner/repo", branch="egg/issue-1882", ) shard = worktree / SUBSTORE_DIR / "issue-1882.json" diff --git a/orchestrator/tests/test_mcp_tools.py b/orchestrator/tests/test_mcp_tools.py index 4022927c42..013d06aaf0 100644 --- a/orchestrator/tests/test_mcp_tools.py +++ b/orchestrator/tests/test_mcp_tools.py @@ -532,11 +532,11 @@ def test_with_repo_param(self, handler): mock_gw.return_value = {"success": True, "data": {"checkpoints": []}} handler.handle_tool_call( "list_checkpoints", - {"repo": "jwbron/egg-checkpoints", "issue": 42}, + {"repo": "owner/repo-checkpoints", "issue": 42}, ) call_url = mock_gw.call_args[0][0] - assert "source_repo=jwbron%2Fegg-checkpoints" in call_url + assert "source_repo=owner%2Frepo-checkpoints" in call_url assert "issue=42" in call_url def test_without_repo_param_no_source_repo(self, handler): @@ -609,11 +609,11 @@ def test_with_repo_param(self, handler): mock_gw.return_value = {"data": {"checkpoints": []}} handler.handle_tool_call( "search_checkpoints", - {"text": "coder", "repo": "jwbron/egg-checkpoints"}, + {"text": "coder", "repo": "owner/repo-checkpoints"}, ) call_url = mock_gw.call_args[0][0] - assert "source_repo=jwbron%2Fegg-checkpoints" in call_url + assert "source_repo=owner%2Frepo-checkpoints" in call_url def test_without_repo_param_no_source_repo(self, handler): """Verify source_repo is not added when repo is not provided.""" @@ -2535,7 +2535,7 @@ def test_repo_argument_is_silently_ignored(self, handler): return_value={"success": True, "data": {}}, ) as mock_req: handler.handle_tool_call( - "prune_stale_worktrees", {"dry_run": True, "repo": "jwbron/egg"} + "prune_stale_worktrees", {"dry_run": True, "repo": "owner/repo"} ) kwargs = mock_req.call_args.kwargs assert "repo" not in kwargs["data"], ( diff --git a/orchestrator/tests/test_mcp_tools_enrichment.py b/orchestrator/tests/test_mcp_tools_enrichment.py index 50a5b34865..aa1472867d 100644 --- a/orchestrator/tests/test_mcp_tools_enrichment.py +++ b/orchestrator/tests/test_mcp_tools_enrichment.py @@ -42,7 +42,7 @@ def _make_status(decisions: list[dict], current_phase: str = "refine") -> dict: } -def _make_pipeline_data(repo: str = "jwbron/egg", issue_number: int = 42) -> dict: +def _make_pipeline_data(repo: str = "owner/repo", issue_number: int = 42) -> dict: return {"repo": repo, "issue_number": issue_number} diff --git a/sandbox/egg_lib/sdlc_cli.py b/sandbox/egg_lib/sdlc_cli.py index 86631b5617..a2ec155c62 100644 --- a/sandbox/egg_lib/sdlc_cli.py +++ b/sandbox/egg_lib/sdlc_cli.py @@ -86,7 +86,7 @@ def _clear_screen() -> None: def _resolve_repo_dir(repo_dir: str) -> str | None: """Resolve a repo directory name to owner/repo format. - Looks up the directory name in EGG_REPOS (e.g. "egg" matches "jwbron/egg") + Looks up the directory name in EGG_REPOS (e.g. "repo" matches "owner/repo") and changes to the repo directory if it exists. Side effect: calls os.chdir() to the repo directory on success, so that diff --git a/sandbox/entrypoint.py b/sandbox/entrypoint.py index 9bc7eeef54..08c81e514d 100644 --- a/sandbox/entrypoint.py +++ b/sandbox/entrypoint.py @@ -802,10 +802,10 @@ def restore_prebuilt_deps( # already restored by the Dockerfile; skip it here. if repo_dir.name == "__egg_system_dirs__": continue - # repo_dir is like /opt/prebuilt-deps/Khan--webapp + # repo_dir is like /opt/prebuilt-deps/owner--repo # Convert back to repo name to find mount point # Try each mounted repo to find a match - repo_dir_name = repo_dir.name # e.g. "Khan--webapp" + repo_dir_name = repo_dir.name # e.g. "owner--repo" # Find the matching mounted repo directory target_repo = None diff --git a/sandbox/tests/test_cli_push_scope_filter_removed.py b/sandbox/tests/test_cli_push_scope_filter_removed.py index 666d866e2d..7c6f6d27ba 100644 --- a/sandbox/tests/test_cli_push_scope_filter_removed.py +++ b/sandbox/tests/test_cli_push_scope_filter_removed.py @@ -131,7 +131,7 @@ def test_get_agent_env_does_not_include_egg_agent_file_patterns(self): pipeline.branch = "egg/issue-1882" pipeline.current_phase = MagicMock() pipeline.current_phase.value = "implement" - pipeline.repo = "jwbron/egg" + pipeline.repo = "owner/repo" pipeline.id = "issue-1882" def fake_spawn(**_kwargs): diff --git a/shared/egg_contracts/checkpoints.py b/shared/egg_contracts/checkpoints.py index f882c13c20..0949ad29b9 100644 --- a/shared/egg_contracts/checkpoints.py +++ b/shared/egg_contracts/checkpoints.py @@ -261,7 +261,7 @@ class CheckpointV2(BaseModel): repo: str | None = Field( default=None, pattern=r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$", - description="Source repository in owner/repo format (e.g. 'jwbron/egg')", + description="Source repository in owner/repo format (e.g. 'owner/repo')", ) # Session details diff --git a/skills/babysit-pr/SKILL.md b/skills/babysit-pr/SKILL.md index 1811245578..0a8d6ea403 100644 --- a/skills/babysit-pr/SKILL.md +++ b/skills/babysit-pr/SKILL.md @@ -45,7 +45,7 @@ If a bare PR number is supplied, auto-detect the repo the same way 1. Run `git -C "$EGG_REPO_PATH" remote get-url origin 2>/dev/null` (or fall back to `git remote -v` from the working directory). 2. Parse the `owner/name` from the URL (e.g. `https://github.com/jwbron/egg.git` - → `jwbron/egg`). + → `owner/repo`). 3. If a `--repo` flag was passed, use that instead. Only ask for the repo if detection fails AND no `--repo` flag was provided. diff --git a/skills/sdlc/SKILL.md b/skills/sdlc/SKILL.md index cecfeaf6c6..045f9b4950 100644 --- a/skills/sdlc/SKILL.md +++ b/skills/sdlc/SKILL.md @@ -56,9 +56,9 @@ If the user provided arguments after `/sdlc`, parse them: | `/sdlc #1059` | Issue number (with hash) | | `/sdlc KORE-1234` | JIRA ticket (matches `-` pattern) | | `/sdlc Add retry logic for API calls` | Free-text task description | -| `/sdlc --repo jwbron/egg 1059` | Repo override + issue number | +| `/sdlc --repo owner/repo 1059` | Repo override + issue number | | `/sdlc --issue 1059` | Issue number (legacy flag, same as bare integer) | -| `/sdlc --repo jwbron/egg KORE-1234` | Repo override + JIRA ticket | +| `/sdlc --repo owner/repo KORE-1234` | Repo override + JIRA ticket | | `/sdlc KORE-1234 --qualifier backend` | JIRA ticket + qualifier (pipeline: `KORE-1234-backend`, branch: `egg/KORE-1234-backend`) | | `/sdlc 1059 --qualifier frontend` | Issue number + qualifier (pipeline: `issue-1059-frontend`, branch: `egg/issue-1059-frontend`) | @@ -1015,9 +1015,9 @@ After stripping the `--short` flag, parse remaining arguments: | Input | Interpretation | |-------|---------------| | `/sdlc --short Add retry logic to the API client` | Free-text task description | -| `/sdlc --short --repo jwbron/egg Fix flaky test` | Repo override + task description | +| `/sdlc --short --repo owner/repo Fix flaky test` | Repo override + task description | | `/sdlc --short KORE-1234` | JIRA ticket (matches `-` pattern) | -| `/sdlc --short --repo jwbron/egg ENG-42` | Repo override + JIRA ticket | +| `/sdlc --short --repo owner/repo ENG-42` | Repo override + JIRA ticket | | `/sdlc --short KORE-1234 --qualifier backend` | JIRA ticket + qualifier | When a JIRA ticket ID is detected, run the [JIRA & Confluence Context Gathering](#jira--confluence-context-gathering) procedure and use the enriched description as the task description. If `--qualifier` is provided, store it as `pipeline_qualifier`. Proceed directly to Phase S2. diff --git a/tests/config/test_repo_config_checkpoint.py b/tests/config/test_repo_config_checkpoint.py index b050e3f2d1..aa54de0263 100644 --- a/tests/config/test_repo_config_checkpoint.py +++ b/tests/config/test_repo_config_checkpoint.py @@ -148,10 +148,10 @@ def test_env_var_checkpoint_repo(self, temp_dir, monkeypatch): """EGG_CHECKPOINT_REPO env var is included in the result set.""" monkeypatch.setenv("EGG_REPO_CONFIG", str(temp_dir / "nonexistent.yaml")) monkeypatch.setenv("HOME", str(temp_dir)) - monkeypatch.setenv("EGG_CHECKPOINT_REPO", "jwbron/checkpoints") + monkeypatch.setenv("EGG_CHECKPOINT_REPO", "owner/checkpoints") result = get_all_checkpoint_repos() - assert result == {"jwbron/checkpoints"} + assert result == {"owner/checkpoints"} def test_env_var_merged_with_config(self, temp_dir, monkeypatch): """EGG_CHECKPOINT_REPO is merged with config-based checkpoint repos.""" @@ -163,19 +163,19 @@ def test_env_var_merged_with_config(self, temp_dir, monkeypatch): " checkpoint_repo: testuser/my-checkpoints\n" ) monkeypatch.setenv("EGG_REPO_CONFIG", str(config_file)) - monkeypatch.setenv("EGG_CHECKPOINT_REPO", "jwbron/checkpoints") + monkeypatch.setenv("EGG_CHECKPOINT_REPO", "owner/checkpoints") result = get_all_checkpoint_repos() - assert result == {"testuser/my-checkpoints", "jwbron/checkpoints"} + assert result == {"testuser/my-checkpoints", "owner/checkpoints"} def test_env_var_case_insensitive(self, temp_dir, monkeypatch): """EGG_CHECKPOINT_REPO is lowercased for comparison.""" monkeypatch.setenv("EGG_REPO_CONFIG", str(temp_dir / "nonexistent.yaml")) monkeypatch.setenv("HOME", str(temp_dir)) - monkeypatch.setenv("EGG_CHECKPOINT_REPO", "Jwbron/Checkpoints") + monkeypatch.setenv("EGG_CHECKPOINT_REPO", "Owner/Checkpoints") result = get_all_checkpoint_repos() - assert "jwbron/checkpoints" in result + assert "owner/checkpoints" in result def test_env_var_empty_ignored(self, temp_dir, monkeypatch): """Empty EGG_CHECKPOINT_REPO is ignored.""" diff --git a/tests/sandbox/test_gh_wrapper.py b/tests/sandbox/test_gh_wrapper.py index d641603b2b..369f6d2668 100644 --- a/tests/sandbox/test_gh_wrapper.py +++ b/tests/sandbox/test_gh_wrapper.py @@ -2441,19 +2441,19 @@ def test_no_repo_flag(self): def test_short_repo_flag_before_command(self): """-R owner/repo before the command should be skipped.""" - main, sub = self._parse(["-R", "jwbron/egg", "issue", "create", "--title", "test"]) + main, sub = self._parse(["-R", "owner/repo", "issue", "create", "--title", "test"]) assert main == "issue" assert sub == "create" def test_long_repo_flag_before_command(self): """--repo owner/repo before the command should be skipped.""" - main, sub = self._parse(["--repo", "jwbron/egg", "pr", "comment", "42", "--body", "hi"]) + main, sub = self._parse(["--repo", "owner/repo", "pr", "comment", "42", "--body", "hi"]) assert main == "pr" assert sub == "comment" def test_repo_flag_equals_syntax(self): """--repo=owner/repo (equals syntax) should be skipped.""" - main, sub = self._parse(["--repo=jwbron/egg", "issue", "create"]) + main, sub = self._parse(["--repo=owner/repo", "issue", "create"]) assert main == "issue" assert sub == "create" @@ -2471,13 +2471,13 @@ def test_hostname_equals_syntax(self): def test_repo_and_hostname_together(self): """Both -R and -H before the command should be skipped.""" - main, sub = self._parse(["-R", "jwbron/egg", "-H", "github.com", "pr", "close", "10"]) + main, sub = self._parse(["-R", "owner/repo", "-H", "github.com", "pr", "close", "10"]) assert main == "pr" assert sub == "close" def test_repo_flag_after_command(self): """-R after the command should not affect dispatch (positionals already found).""" - main, sub = self._parse(["issue", "create", "-R", "jwbron/egg", "--title", "test"]) + main, sub = self._parse(["issue", "create", "-R", "owner/repo", "--title", "test"]) assert main == "issue" assert sub == "create" diff --git a/tests/shared/egg_contracts/test_checkpoint_cli.py b/tests/shared/egg_contracts/test_checkpoint_cli.py index 1e1adfe3b8..5dcdbe7c0d 100644 --- a/tests/shared/egg_contracts/test_checkpoint_cli.py +++ b/tests/shared/egg_contracts/test_checkpoint_cli.py @@ -359,37 +359,37 @@ def test_explicit_flag_takes_priority(self): assert checkpoint_repo == "owner/explicit-repo" assert source_repo is None - @patch("config.repo_config.get_checkpoint_repo", return_value="jwbron/egg-checkpoints") + @patch("config.repo_config.get_checkpoint_repo", return_value="owner/repo-checkpoints") @patch("egg_contracts.checkpoint_cli.run_git") def test_auto_detects_from_https_remote(self, mock_git, mock_config): """Auto-detects checkpoint_repo from HTTPS remote URL.""" mock_git.return_value = subprocess.CompletedProcess( args=[], returncode=0, - stdout="https://github.com/jwbron/egg.git\n", + stdout="https://github.com/owner/repo.git\n", stderr="", ) args = self._make_args() checkpoint_repo, source_repo = _get_checkpoint_repo_from_args(args) - assert checkpoint_repo == "jwbron/egg-checkpoints" - assert source_repo == "jwbron/egg" - mock_config.assert_called_once_with("jwbron/egg") + assert checkpoint_repo == "owner/repo-checkpoints" + assert source_repo == "owner/repo" + mock_config.assert_called_once_with("owner/repo") - @patch("config.repo_config.get_checkpoint_repo", return_value="jwbron/egg-checkpoints") + @patch("config.repo_config.get_checkpoint_repo", return_value="owner/repo-checkpoints") @patch("egg_contracts.checkpoint_cli.run_git") def test_auto_detects_from_ssh_remote(self, mock_git, mock_config): """Auto-detects checkpoint_repo from SSH remote URL.""" mock_git.return_value = subprocess.CompletedProcess( args=[], returncode=0, - stdout="git@github.com:jwbron/egg.git\n", + stdout="git@github.com:owner/repo.git\n", stderr="", ) args = self._make_args() checkpoint_repo, source_repo = _get_checkpoint_repo_from_args(args) - assert checkpoint_repo == "jwbron/egg-checkpoints" - assert source_repo == "jwbron/egg" - mock_config.assert_called_once_with("jwbron/egg") + assert checkpoint_repo == "owner/repo-checkpoints" + assert source_repo == "owner/repo" + mock_config.assert_called_once_with("owner/repo") @patch("config.repo_config.get_checkpoint_repo", return_value=None) @patch("egg_contracts.checkpoint_cli.run_git") diff --git a/tests/shared/egg_contracts/test_checkpoint_cli_http.py b/tests/shared/egg_contracts/test_checkpoint_cli_http.py index e6e01a441d..bf5493553c 100644 --- a/tests/shared/egg_contracts/test_checkpoint_cli_http.py +++ b/tests/shared/egg_contracts/test_checkpoint_cli_http.py @@ -338,16 +338,16 @@ class TestGetSourceRepo: @patch("egg_contracts.checkpoint_cli.run_git") def test_extracts_from_https_remote(self, mock_git): mock_git.return_value = subprocess.CompletedProcess( - args=[], returncode=0, stdout="https://github.com/jwbron/egg.git\n", stderr="" + args=[], returncode=0, stdout="https://github.com/owner/repo.git\n", stderr="" ) - assert _get_source_repo("/repo") == "jwbron/egg" + assert _get_source_repo("/repo") == "owner/repo" @patch("egg_contracts.checkpoint_cli.run_git") def test_extracts_from_ssh_remote(self, mock_git): mock_git.return_value = subprocess.CompletedProcess( - args=[], returncode=0, stdout="git@github.com:jwbron/egg.git\n", stderr="" + args=[], returncode=0, stdout="git@github.com:owner/repo.git\n", stderr="" ) - assert _get_source_repo("/repo") == "jwbron/egg" + assert _get_source_repo("/repo") == "owner/repo" @patch("egg_contracts.checkpoint_cli.run_git") def test_returns_none_when_git_fails(self, mock_git): @@ -366,9 +366,9 @@ def test_returns_none_for_non_github_url(self, mock_git): @patch("egg_contracts.checkpoint_cli.run_git") def test_handles_trailing_slash(self, mock_git): mock_git.return_value = subprocess.CompletedProcess( - args=[], returncode=0, stdout="https://github.com/jwbron/egg/\n", stderr="" + args=[], returncode=0, stdout="https://github.com/owner/repo/\n", stderr="" ) - assert _get_source_repo("/repo") == "jwbron/egg" + assert _get_source_repo("/repo") == "owner/repo" @patch("egg_contracts.checkpoint_cli.run_git", side_effect=Exception("timeout")) def test_returns_none_on_exception(self, mock_git): @@ -389,12 +389,12 @@ def test_passes_checkpoint_repo_when_available(self, mock_get_ckpt): @patch("egg_contracts.checkpoint_cli._get_checkpoint_repo_from_args") def test_passes_source_repo_when_checkpoint_repo_unavailable(self, mock_get_ckpt): - mock_get_ckpt.return_value = (None, "jwbron/egg") + mock_get_ckpt.return_value = (None, "owner/repo") args = argparse.Namespace(repo_path="/repo", checkpoint_repo=None) params: dict = {"repo_path": "/repo"} _add_checkpoint_resolution_params(params, args) assert "checkpoint_repo" not in params - assert params["source_repo"] == "jwbron/egg" + assert params["source_repo"] == "owner/repo" @patch("egg_contracts.checkpoint_cli._get_checkpoint_repo_from_args") def test_passes_neither_when_both_unavailable(self, mock_get_ckpt): @@ -411,7 +411,7 @@ class TestBuildListParamsSourceRepo: @patch("egg_contracts.checkpoint_cli._get_checkpoint_repo_from_args") def test_includes_source_repo_when_checkpoint_repo_unavailable(self, mock_get_ckpt): - mock_get_ckpt.return_value = (None, "jwbron/egg") + mock_get_ckpt.return_value = (None, "owner/repo") args = argparse.Namespace( limit=50, issue=None, @@ -429,4 +429,4 @@ def test_includes_source_repo_when_checkpoint_repo_unavailable(self, mock_get_ck ) params = _build_list_params(args) assert "checkpoint_repo" not in params - assert params["source_repo"] == "jwbron/egg" + assert params["source_repo"] == "owner/repo" diff --git a/tests/shared/egg_contracts/test_checkpoint_loader.py b/tests/shared/egg_contracts/test_checkpoint_loader.py index 277c108125..57428f605c 100644 --- a/tests/shared/egg_contracts/test_checkpoint_loader.py +++ b/tests/shared/egg_contracts/test_checkpoint_loader.py @@ -436,14 +436,14 @@ def test_by_repo_populated(self): checkpoint_id="ckpt-aa00000001", session_id="session-1", commit_sha="aaa1234567890", - repo="jwbron/egg", + repo="owner/repo", now=now, ) cp2 = _make_v2_checkpoint( checkpoint_id="ckpt-bb00000002", session_id="session-2", commit_sha="bbb1234567890", - repo="jwbron/egg", + repo="owner/repo", now=now + timedelta(seconds=1), ) cp3 = _make_v2_checkpoint( @@ -458,7 +458,7 @@ def test_by_repo_populated(self): add_checkpoint_to_index_v2(cp2, index_path) index = add_checkpoint_to_index_v2(cp3, index_path) - assert len(index.get_by_repo("jwbron/egg")) == 2 + assert len(index.get_by_repo("owner/repo")) == 2 assert len(index.get_by_repo("entireio/cli")) == 1 assert index.get_by_repo("nonexistent/repo") == [] @@ -792,7 +792,7 @@ def test_filter_by_repo(self): checkpoint_id="ckpt-aa00000001", session_id="session-1", commit_sha="aaa1234567890", - repo="jwbron/egg", + repo="owner/repo", now=now, ) cp2 = _make_v2_checkpoint( @@ -808,7 +808,7 @@ def test_filter_by_repo(self): add_checkpoint_to_index_v2(cp1, index_path) add_checkpoint_to_index_v2(cp2, index_path) - results = list_checkpoints_v2(checkpoints_dir, index_path, repo="jwbron/egg") + results = list_checkpoints_v2(checkpoints_dir, index_path, repo="owner/repo") assert len(results) == 1 assert results[0].id == "ckpt-aa00000001" diff --git a/tests/shared/egg_contracts/test_checkpoints.py b/tests/shared/egg_contracts/test_checkpoints.py index fac2faaf24..b0f632c01a 100644 --- a/tests/shared/egg_contracts/test_checkpoints.py +++ b/tests/shared/egg_contracts/test_checkpoints.py @@ -518,9 +518,9 @@ def test_repo_set(self): session=session, created_at=now, session_started_at=now, - repo="jwbron/egg", + repo="owner/repo", ) - assert checkpoint.repo == "jwbron/egg" + assert checkpoint.repo == "owner/repo" def test_empty_commit_sha_becomes_none(self): """Test that empty string commit_sha is converted to None.""" @@ -619,10 +619,10 @@ def test_from_checkpoint_with_repo(self): session=session, created_at=now, session_started_at=now, - repo="jwbron/egg", + repo="owner/repo", ) summary = CheckpointSummaryV2.from_checkpoint(checkpoint) - assert summary.repo == "jwbron/egg" + assert summary.repo == "owner/repo" def test_from_checkpoint_files_touched_count(self): """Test that files_touched_count is computed from checkpoint.""" @@ -818,10 +818,10 @@ def test_get_by_repo(self): index = CheckpointIndexV2( last_updated=now, by_repo={ - "jwbron/egg": ["ckpt-aa00000001", "ckpt-bb00000002"], + "owner/repo": ["ckpt-aa00000001", "ckpt-bb00000002"], "entireio/cli": ["ckpt-cc00000003"], }, ) - assert index.get_by_repo("jwbron/egg") == ["ckpt-aa00000001", "ckpt-bb00000002"] + assert index.get_by_repo("owner/repo") == ["ckpt-aa00000001", "ckpt-bb00000002"] assert index.get_by_repo("entireio/cli") == ["ckpt-cc00000003"] assert index.get_by_repo("nonexistent/repo") == [] diff --git a/tests/shared/egg_contracts/test_models.py b/tests/shared/egg_contracts/test_models.py index 0400f694a1..209e3ac522 100644 --- a/tests/shared/egg_contracts/test_models.py +++ b/tests/shared/egg_contracts/test_models.py @@ -298,9 +298,9 @@ def test_contract_with_workflow_owner(self): title="Test issue", url="https://github.com/owner/repo/issues/133", ), - workflow_owner="jwbron", + workflow_owner="my-org", ) - assert contract.workflow_owner == "jwbron" + assert contract.workflow_owner == "my-org" def test_contract_workflow_owner_null(self): """Test that workflow_owner can be explicitly set to None.""" From abeceac7383303cd1af4642d2d6a7aee307aa10c Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Fri, 24 Apr 2026 10:56:01 -0700 Subject: [PATCH 9/9] Address review feedback (PR #1991 round 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Make the HOST_HOME escape hatch actually reachable. Real Linux containers expose a rootfs ``/ → /`` entry in ``/proc/self/mountinfo`` that matches every path under longest-prefix lookup as an identity translation, so the mountinfo branch never falls through to the env-var fallback in production. Add ``EGG_DISABLE_MOUNTINFO=1`` to short-circuit the mountinfo read at module import — the flag lets operators force the explicit ``HOST_HOME`` path in environments where mountinfo doesn't reflect the real layout (multi-partition hosts, exotic bind-mount namespaces). 2. Fix mismatched example in ``skills/babysit-pr/SKILL.md``. The repo detection section showed a canonical input URL (``github.com/jwbron/egg.git``) paired with a generic output (``owner/repo``) — parsing the former yields ``jwbron/egg``, not ``owner/repo``. Make both sides of the example use the same placeholder so the transformation is accurate. 3. Scope the Makefile JSON-quoting sed to ``EGG_HOST_REPO_MAP`` only. The previous regex matched any YAML value line of the shape ``value: {...}``, which would silently re-quote any future env var whose value happens to be a flow-style mapping. Anchor on the preceding ``- name: EGG_HOST_REPO_MAP`` line via ``N;s|…|…|`` so the substitution only triggers for the one entry we intend. Tests: ``test_translate_host_path.py`` gains coverage for the disable flag (honored, accepts standard truthy spellings, lets ``HOST_HOME`` take over), bringing the suite to 25 tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- Makefile | 2 +- gateway/gateway.py | 31 +++++++++++++---- gateway/tests/test_translate_host_path.py | 41 +++++++++++++++++++++++ skills/babysit-pr/SKILL.md | 4 +-- 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 3a60b2471c..6785f9b19e 100644 --- a/Makefile +++ b/Makefile @@ -402,7 +402,7 @@ deploy: k3s-secrets ## Deploy egg to k3s echo " EGG_HOST_REPO_MAP=$$EGG_HOST_REPO_MAP" && \ kubectl kustomize k8s/overlays/local/ | \ envsubst '$$EGG_HOST_HOME $$EGG_HOST_REPO_MAP' | \ - sed -E "s|^(\s*value: )(\{.*\})$$|\1'\2'|" | \ + sed -E "/name: EGG_HOST_REPO_MAP$$/{N;s|^(\s*- name: EGG_HOST_REPO_MAP\s*\n\s*value: )(\{.*\})$$|\1'\2'|}" | \ sed -e "s|egg-orchestrator:latest|egg-orchestrator:$(EGG_IMAGE_TAG)|g" \ -e "s|egg-gateway:latest|egg-gateway:$(EGG_IMAGE_TAG)|g" \ -e "s|egg-sandbox:latest|egg-sandbox:$(EGG_IMAGE_TAG)|g" | \ diff --git a/gateway/gateway.py b/gateway/gateway.py index ff449b8c7e..424528d325 100644 --- a/gateway/gateway.py +++ b/gateway/gateway.py @@ -426,13 +426,27 @@ def handle_unhandled_exception(e: Exception) -> tuple[Response, int]: # # Normally we discover the host path directly from /proc/self/mountinfo # (see ``translate_to_host_path``) so no env-var configuration is -# required. ``HOST_HOME`` remains as an explicit escape hatch for test -# environments and unusual setups where mountinfo doesn't reflect the -# real mapping. +# required. ``HOST_HOME`` is the escape hatch for environments where +# mountinfo doesn't reflect the real host layout (e.g. multi-partition +# setups, or an operator who wants to override the discovered value). +# Set ``EGG_DISABLE_MOUNTINFO=1`` to skip mountinfo entirely and force +# the ``HOST_HOME`` path — needed because real Linux containers always +# expose a rootfs ``/ → /`` entry that matches every path under longest- +# prefix lookup, so without the disable flag the env-var fallback is +# unreachable. HOST_HOME = os.environ.get("HOST_HOME", "") CONTAINER_HOME = "/home/egg" +def _mountinfo_disabled() -> bool: + return os.environ.get("EGG_DISABLE_MOUNTINFO", "").strip().lower() in { + "1", + "true", + "yes", + "on", + } + + def _load_mount_mapping() -> list[tuple[str, str]]: """Read /proc/self/mountinfo and return a list of (mount_point, host_root) tuples. @@ -455,6 +469,8 @@ def _load_mount_mapping() -> list[tuple[str, str]]: but worth knowing if paths ever contain whitespace. """ entries: list[tuple[str, str]] = [] + if _mountinfo_disabled(): + return entries try: with open("/proc/self/mountinfo") as fh: for line in fh: @@ -479,9 +495,12 @@ def translate_to_host_path(container_path: str) -> str: Tries in order: 1. /proc/self/mountinfo — find the longest mount_point that is a prefix of ``container_path`` and substitute with its host root. - This works for any hostPath volume without configuration. - 2. ``HOST_HOME`` env var — explicit override, used when mountinfo is - not available or needs to be bypassed (tests, unusual setups). + This works for any hostPath volume without configuration. Real + Linux containers always include a rootfs ``/ → /`` entry, so + this strategy is reachable unless explicitly disabled. + 2. ``HOST_HOME`` env var — explicit override. To reach this branch + on Linux, set ``EGG_DISABLE_MOUNTINFO=1`` to skip the mountinfo + lookup (otherwise the ``/`` entry always matches first). Args: container_path: Path inside the gateway container diff --git a/gateway/tests/test_translate_host_path.py b/gateway/tests/test_translate_host_path.py index ecec4db993..4571d21ef1 100644 --- a/gateway/tests/test_translate_host_path.py +++ b/gateway/tests/test_translate_host_path.py @@ -159,3 +159,44 @@ def _raise(*_a, **_kw): with patch("gateway.open", _raise): assert gateway_module._load_mount_mapping() == [] + + def test_returns_empty_when_disable_flag_set(self, tmp_path): + """``EGG_DISABLE_MOUNTINFO=1`` skips the read entirely.""" + # mountinfo exists and is parseable — but the env flag should + # short-circuit before opening it. + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text("1 0 0:1 /r /m rw,relatime - tmpfs tmpfs rw\n") + + def _fail_if_opened(*_a, **_kw): + raise AssertionError("should not open mountinfo when disabled") + + with patch.dict("os.environ", {"EGG_DISABLE_MOUNTINFO": "1"}): + with patch("gateway.open", _fail_if_opened): + assert gateway_module._load_mount_mapping() == [] + + @pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "on"]) + def test_disable_flag_accepts_truthy_values(self, value): + with patch.dict("os.environ", {"EGG_DISABLE_MOUNTINFO": value}): + assert gateway_module._mountinfo_disabled() is True + + @pytest.mark.parametrize("value", ["", "0", "false", "no", "off", "anything-else"]) + def test_disable_flag_rejects_falsy_values(self, value): + with patch.dict("os.environ", {"EGG_DISABLE_MOUNTINFO": value}): + assert gateway_module._mountinfo_disabled() is False + + +class TestDisableMountinfoWithHostHome: + """When mountinfo is disabled, ``HOST_HOME`` takes over.""" + + def test_disable_flag_lets_host_home_translate(self, override_mounts, override_host_home): + # Simulate a container with a realistic rootfs ``/ → /`` entry + # that would otherwise short-circuit every lookup as an identity + # translation. With the disable flag and HOST_HOME set, the + # fallback path is the only active strategy. + override_mounts([]) # Simulate EGG_DISABLE_MOUNTINFO having cleared the table. + override_host_home("/home/user") + + assert ( + gateway_module.translate_to_host_path("/home/egg/.egg-worktrees/x") + == "/home/user/.egg-worktrees/x" + ) diff --git a/skills/babysit-pr/SKILL.md b/skills/babysit-pr/SKILL.md index 0a8d6ea403..aae7830a79 100644 --- a/skills/babysit-pr/SKILL.md +++ b/skills/babysit-pr/SKILL.md @@ -44,8 +44,8 @@ If a bare PR number is supplied, auto-detect the repo the same way 1. Run `git -C "$EGG_REPO_PATH" remote get-url origin 2>/dev/null` (or fall back to `git remote -v` from the working directory). -2. Parse the `owner/name` from the URL (e.g. `https://github.com/jwbron/egg.git` - → `owner/repo`). +2. Parse the `owner/name` from the URL (e.g. `https://github.com/my-org/my-repo.git` + → `my-org/my-repo`). 3. If a `--repo` flag was passed, use that instead. Only ask for the repo if detection fails AND no `--repo` flag was provided.