Phase 3: Extract test suite from gateway-sidecar - #22
Merged
Conversation
Extract and adapt ~2,500 lines of tests from james-in-a-box gateway-sidecar: - test_git_validation.py: Path validation, git arg sanitization, gh API allowlist - test_rate_limiter.py: Sliding window rate limiting, thread safety - test_repo_parser.py: GitHub URL parsing, owner/repo extraction - test_repo_visibility.py: Visibility checking, caching, multi-token fallback - test_session_manager.py: Session lifecycle, persistence, thread safety - test_token_refresher.py: Token refresh, caching, failure handling - test_private_repo_policy.py: Private/public mode repo access control - test_worktree_manager.py: Worktree operations, path traversal prevention - test_proxy_security.py: Network lockdown, IP blocking patterns Enhanced conftest.py with: - Test environment setup (launcher secret, repo config) - Common fixtures (mock_session, mock_token_refresher, etc.) - Automatic cleanup of global state between tests Tests: 398 passed, 10 skipped, 10 xfailed (xfailed tests document gh API endpoints not yet in egg's allowlist) Issue: #17 Authored-by: jib
The tests work fine with standard unittest.mock patching - the skip markers were overly cautious. This brings the test count from 398 passed + 10 skipped to 408 passed. Authored-by: jib
Add 10 new GitHub API endpoints to GH_API_ALLOWED_PATHS to support additional operations that were previously blocked: PR review comments, issue events/timeline, commit comments, and releases. Remove all xfail markers from test_git_validation.py since these endpoints are now allowed. Closes #23 Authored-by: jib
This was referenced Feb 3, 2026
Contributor
Author
PR Review: Phase 3 Test Suite ExtractionThanks for extracting this substantial test suite (~3,300 lines). The test quality is good - well-structured with docstrings, thread safety coverage, and proper graceful skipping in sandbox environments. Issue #18 Implementation Status: PartialThis PR references closing issue #17 (test extraction) and #23 (API allowlist), but issue #18 (restore coverage threshold to 80%) remains unaddressed:
To close issue #18, the threshold needs to be updated and coverage verified. Missing Test Files (~2,266 lines, ~42% of source)Compared to
The first two are particularly important:
Alignment with PR #693 (Implementation Plan)The proposal specifies 90%+ code coverage for RecommendationsOption A: Expand this PR
Option B: Merge as-is with follow-ups
What's Good
Question: Should we expand this PR to include the missing files, or merge as-is and create a follow-up issue? — Authored by jib |
Contributor
Author
Follow-up: Issues #17 and #23 StatusIssue #23 (Expand gh API allowlist): ✅ Properly AddressedThe PR adds all 10 endpoints listed in issue #23:
Issue #23 can remain closed. Issue #17 (Phase 3: Test suite extraction):
|
| File | Status |
|---|---|
conftest.py |
✅ Extracted (enhanced) |
test_git_validation.py |
✅ Extracted |
test_rate_limiter.py |
✅ Extracted |
test_repo_parser.py |
✅ Extracted |
test_repo_visibility.py |
✅ Extracted |
test_session_manager.py |
✅ Extracted |
test_token_refresher.py |
✅ Extracted |
test_private_repo_policy.py |
✅ Extracted |
test_worktree_manager.py |
✅ Extracted |
test_proxy_security.py |
✅ Extracted |
test_gateway.py |
❌ Not extracted (997 lines) |
test_policy.py |
❌ Not extracted (570 lines) |
test_gateway_integration.py |
❌ Not extracted (355 lines) |
test_git_client.py |
❌ Not extracted (344 lines) |
Issue #17 acceptance criteria states: "All 14 test files extracted and adapted"
Since 4 files are missing, issue #17 should be reopened or a new issue created to track the remaining extraction work.
Recommended Action
- Reopen issue Phase 3: Test suite extraction #17 with updated checklist showing 10/14 complete, OR
- Create new issue for the 4 missing test files (test_gateway.py, test_policy.py, test_gateway_integration.py, test_git_client.py)
— Authored by jib
Extract comprehensive test files from james-in-a-box gateway-sidecar to increase test coverage: - tests/unit/test_gateway.py: Complete REST API tests for gateway endpoints including health, auth, git push, gh PR operations, blocked commands, and private mode enforcement (~1000 lines) - tests/integration/test_gateway_integration.py: Integration tests for git execute, worktree, and path validation endpoints (~350 lines) Coverage increased from ~20% to 66%. Updated threshold in test.yml to 65% (target 80% noted for follow-up work). Issue: #17, #18 Authored-by: jib
Contributor
Author
Option A ImplementedPer your feedback, I've expanded this PR with the 4 missing test files: Added/Updated Files
Coverage Status
Why Not 80% YetThe gateway module (
RecommendationThis PR raises coverage from 20% → 66%, which is a significant improvement. I recommend:
— Authored by jib |
Address PR review comment #3839323046 by adding missing tests: test_policy.py additions: - TestCachedPRInfo: Tests for PR info cache staleness detection - test_non_github_url: Verify non-GitHub URLs return None - test_full_refspec: Test complex refspec parsing test_git_client.py additions: - TestIsReposParentDirectory: 6 tests for parent directory detection - TestNormalizeFlag: 4 tests for flag normalization - TestBlockedGitFlags: Test dangerous flags are blocked - TestValidateRepoPath: 2 additional tests (None path, outside allowed) This brings coverage from 66% to 68% and completes the test extraction for these files (test_git_client.py now exceeds source at 355 vs 345 lines). Issue: #17 Authored-by: jib
Contributor
Author
Addressed: Missing test extraction completedPer this comment, I've added the remaining tests from test_policy.py (421 → 462 lines)
test_git_client.py (264 → 355 lines, now exceeds source's 345)
Coverage
Remaining gap for 80% coverage (Issue #18)The main gaps are in modules not directly covered by the source tests:
These require new tests rather than extraction from james-in-a-box. — Authored by jib |
Add comprehensive tests for: - proxy_monitor: BlockedRequest, ProxyStats, log functions (0% -> 77%) - config_validator: validate_config, network lockdown mode (16% -> 72%) - egg_config/loader: YAML loading, env var expansion (20% -> 100%) - egg_config/validators: ValidationResult, config validation (22% -> 100%) - fork_policy: ForkPolicyResult, ForkPolicy class (36% -> 100%) - error_messages: error formatting, hints (47% -> 100%) - worktree_manager: list/cleanup operations (48% -> 71%) - repo_parser: URL normalization, path parsing (61% -> 96%) - egg_logging: formatters, logger methods (74% -> 99%) - github_client: API path validation tests Total: 250+ new test cases added. Authored-by: jib
jwbron
approved these changes
Feb 3, 2026
james-in-a-box Bot
pushed a commit
that referenced
this pull request
Apr 20, 2026
- Add lock protection for _pod_states dict access in KubernetesMonitor to prevent data corruption from concurrent thread access (#7) - Fix exit_code=None incorrectly treated as clean exit — only exit_code==0 is a clean exit now (#18) - Prune _clean_exit_skipped when pods are removed to prevent unbounded memory growth (#21) - Remove pods/create from RBAC — orchestrator creates Jobs, not bare pods (#17) - Add sandbox image build and import to test-integration.yml to prevent ImagePullBackOff on agent pod spawns (#20) - Set KUBECONFIG default in Makefile deploy target so it works independently of k3s-setup subshell (#22)
jwbron
added a commit
that referenced
this pull request
Apr 21, 2026
* Initialize SDLC contract for issue #1553 * Add Kubernetes migration documentation and update existing docs Create docs/architecture/kubernetes-migration.md covering the Docker to k8s migration architecture, design decisions, component mapping, network isolation model, storage model, RBAC, developer workflow, and CI/CD changes. Update existing docs to reflect the k8s migration: - docs/guides/deployment.md: Replace Docker Compose with k3s deployment - docs/architecture/orchestrator.md: Update network architecture for k8s - docs/architecture/network-isolation.md: Add Kubernetes NetworkPolicy section - orchestrator/README.md: Update file listing for new k8s modules - docs/development/STRUCTURE.md: Add k8s/ directory, update orchestrator - docs/index.md: Add kubernetes-migration.md to doc index - CONTRIBUTING.md: Update integration test prereq from Docker to k3s * Update remaining docs for Docker-to-Kubernetes terminology - docs/architecture/README.md: Update system overview for k8s components - docs/architecture/git-isolation.md: Update storage/network comparison table - docs/guides/deploy-migration.md: Add deprecation note pointing to k8s - docs/guides/pipeline-health-monitoring.md: Update log reference terminology - docs/guides/concurrent-execution.md: Update worktree isolation for pod/Job naming * Add ContainerBackend protocol, KubernetesClient, and k8s manifests Phase 1: Define ContainerBackend Protocol with runtime_checkable interface that both DockerClient and KubernetesClient satisfy. Implement KubernetesClient wrapping the kubernetes Python client with Job/Pod lifecycle management, custom exception hierarchy, and singleton accessor. Add k8s-native fields (pod_name, namespace, job_name) to ContainerInfo. Phase 2: Create Kustomize manifests with base + local overlay structure. Base includes orchestrator/gateway Deployments and Services, RBAC for Job management, agent Job template with init container for .git shadow mount, and Calico NetworkPolicies enforcing default-deny with gateway-only egress for agent pods. * Update orchestrator architecture doc for k8s terminology Replace Docker-specific references with Kubernetes equivalents throughout: - ContainerMonitor → KubernetesMonitor - container_monitor.py → kubernetes_monitor.py - container_spawner.py → kubernetes_spawner.py - Docker container set → Kubernetes pod set - Docker queries → Kubernetes API queries - container ID → Job name for worktree keying - bind mounts → hostPath volumes - Docker host → host machine * Update orchestrator README for k8s terminology Replace remaining Docker-specific references: state volume, health checks, PATCH behavior, host path translation. * Update docs with accurate implementation details from coder Align migration docs with actual implementation: - NetworkPolicies: add DNS egress policy, correct label selectors (app.kubernetes.io/component, kubernetes.io/metadata.name) - ContainerBackend protocol: match actual method signatures - RBAC: document both ClusterRole and namespace-scoped Role - KubernetesClient: document label scheme (egg.pipeline.id, etc.) * Add tests for ContainerBackend protocol and KubernetesClient - test_container_backend.py: Protocol conformance (Docker, K8s, minimal, incomplete), exception hierarchy, ContainerInfo k8s fields, runtime checkability. - test_kubernetes_client.py: 101 tests covering create/start/stop/remove container, get_container_info, list_containers, logs, wait, cleanup, k8s-native methods (create_job, delete_job, list_jobs, get_pod_for_job, get_pod_logs, get_pod_status), _resolve_job_name, helper functions, singleton accessor, constants. - conftest.py: Mock kubernetes SDK (V1Container, V1Job, etc.) with attribute-storing data classes so tests work without the kubernetes package installed. * Migrate gateway to token-only auth, add KubernetesSpawner and Monitor Gateway auth: Remove IP-based session validation enforcement. Pod IPs are ephemeral in Kubernetes so sessions now authenticate by token only. IP is still recorded for audit logging. container_ip made optional in session registration. KubernetesSpawner: New spawner that creates k8s Jobs instead of Docker containers. Uses label-based identification, token-only gateway sessions, and the same SpawnedContainer interface. Supports agent and overseer job spawning, concurrent spawn functions, pipeline cleanup, and restart tracking. KubernetesMonitor: Replacement for ContainerMonitor using k8s pod polling. Detects pod state transitions, fires event callbacks, and handles orphan cleanup via label-based job listing. Routes updated to support both Docker and k8s backends via EGG_RUNTIME environment variable, defaulting to Docker for backward compatibility. * Add tests for KubernetesSpawner and KubernetesMonitor * Complete k8s migration: CLI runtime, CI/CD, Docker removal Phase 4 - CLI Runtime Migration: - Add to_k8s_job_kwargs() and build_sandbox_job_spec() to shared/egg_container/ for converting SandboxContainerConfig to k8s Job specs with proper volume, env, and security mapping. - Update sandbox/egg_lib/runtime.py with dual Docker/k8s path selected by EGG_RUNTIME env var. K8s path uses Service DNS for gateway resolution. Phase 5 - CI/CD and Docker Removal: - Add Makefile targets: k3s-setup, deploy, k3s-import, k3s-teardown. - Update CI workflows to set up k3s, import images, and deploy. - Replace Docker SDK code with backward-compat shims that re-export from kubernetes equivalents (DockerClient→KubernetesClient, etc.). - Remove docker-compose.yml files. - Replace docker>=7.0.0 with kubernetes>=31.0.0 in dependencies. - Update integration test fixtures for k3s-based test environment. - Add consensus stall recovery methods to KubernetesMonitor for backward compatibility with existing health check infrastructure. * Fix DockerClient test for k8s migration (DockerClient is now alias) * Fix 5 reviewer NACK issues: RBAC, labels, naming, singleton, list 1. SECURITY: Remove ClusterRole/ClusterRoleBinding from rbac.yaml; namespace-scoped Role+RoleBinding in egg-agents is sufficient. 2. CORRECTNESS: Add app.kubernetes.io/component:agent label in spawn_agent_job() so NetworkPolicies apply to agent pods. 3. CORRECTNESS: Prevent JOB_PREFIX double-prepending in create_container() and use correct prefixed name in spawner pre-cleanup. Add backward-compat method aliases and kwargs (docker_client, timeout, spawn_agent_container, etc.). 4. CORRECTNESS: Validate explicit namespace in singleton accessor get_kubernetes_client() using sentinel pattern. 5. CORRECTNESS: Guard against double-prefix in list_containers() when LABEL_CONTAINER_NAME is missing from pod labels. * Fix ruff violations and add _validate_container_id shim - Remove 7 unused imports (F401) from kubernetes_monitor.py and kubernetes_spawner.py via ruff check --fix. - Apply ruff format to all 3 source files. - Add _validate_container_id to docker_client.py shim so test_docker_client.py can collect without import errors. * Fix checks: apply automated formatting fixes * Fix lint: remove unused imports, fix hardcoded ports - Remove unused KubernetesClient, get_kubernetes_client, KubernetesSpawner imports from orchestrator/routes/pipelines.py (ruff F401) - Import GATEWAY_PORT/GATEWAY_PROXY_PORT from egg_config in kubernetes_spawner.py instead of hardcoding 9848/3129 - Add # noqa: EGG002 to k8s YAML manifests where port constants cannot be imported (infrastructure files require literal values) * Fix lint: sort imports in kubernetes_spawner, add raise-from in runtime * Fix mypy errors in runtime.py for kubernetes migration * Fix container_monitor tests for Kubernetes migration * Rewrite docker_client tests for Kubernetes shim layer * Update container_spawner tests for Kubernetes migration * Fix remaining test failures for Kubernetes migration * Fix kubernetes_spawner test assertions * Fix lint formatting in test files * Fix checks: align tests with Docker-to-Kubernetes migration * Address review feedback: fix all blocking issues in k8s migration Fix all 11 remaining blocking issues from the review: 1. Add resource limits (500m/512Mi req, 2CPU/2Gi limits), activeDeadlineSeconds (4h), and ttlSecondsAfterFinished (10m) to programmatic Job specs 2. Remove dead agent-job-template.yaml ConfigMap (never loaded by Python code) 3. Add allow-agent-to-orchestrator egress NetworkPolicy on port 9849 4. Fix namespace default in sandbox/egg_lib/runtime.py from egg-system to egg-agents 5. Forward timeout parameter to delete_job via grace_period_seconds 6. Add set_health_check_runner() method to KubernetesMonitor for cli.py compat 8. Add securityContext (runAsNonRoot, drop ALL caps, no privilege escalation) to gateway and orchestrator deployments 9. Add input validation on container_id/job names in KubernetesClient (_validate_name for create, _resolve_job_name for all other operations) 10. Add SHA256 checksum verification to install-calico.sh 11. Remove || true from CI Calico install and deploy steps 12. Fix EGG_REPO_PATH to include repo name derived from repos list Contract verification gaps addressed: - Add 23 unit tests for to_k8s_job_kwargs() and build_sandbox_job_spec() - Add k8s-based code paths to integration test fixtures (egg_stack, local_pipeline_stack) with test namespace creation/cleanup * Address re-review feedback: fix remaining blocking issues - B2: Add emptyDir volumes for /home/egg/.egg-state and /tmp to orchestrator deployment so it can write state with readOnlyRootFilesystem - B1: Wire health check runner into KubernetesMonitor._check_pod so RUNTIME_TICK checks fire on pod state transitions - B3: Add denylist for security-critical env vars (EGG_SESSION_TOKEN, GATEWAY_URL, HTTP_PROXY, etc.) that extra_env cannot override - N1: Move _UID_RE regex to module scope to avoid recompilation * Address non-blocking review feedback: fix stale comment, move constant to module scope - Fix stale comment in test_health_check_integration.py that incorrectly stated set_health_check_runner and _run_runtime_tick_checks were not carried over to KubernetesMonitor (they were, in da297cb) - Move _PROTECTED_ENV_KEYS from local variable to module-level constant to avoid re-creating the frozenset on every call * Add missing V1ResourceRequirements mock to fix 12 test failures * Port restart improvements from main to kubernetes_spawner: concurrency locks, pre-spawn count increment, mode validation * Fix restart count lock protection in KubernetesSpawner Match ContainerSpawner's thread-safety pattern: - get_restart_count() now acquires per-key lock before reading - reset_restart_counts() holds _restart_locks_lock while modifying both _restart_counts and _restart_locks atomically, using pop() to safely handle already-held locks * Address re-review feedback: namespace default, restart lock timeout, stale template references * Address review feedback: thread safety, correctness, and CI fixes - Add lock protection for _pod_states dict access in KubernetesMonitor to prevent data corruption from concurrent thread access (#7) - Fix exit_code=None incorrectly treated as clean exit — only exit_code==0 is a clean exit now (#18) - Prune _clean_exit_skipped when pods are removed to prevent unbounded memory growth (#21) - Remove pods/create from RBAC — orchestrator creates Jobs, not bare pods (#17) - Add sandbox image build and import to test-integration.yml to prevent ImagePullBackOff on agent pod spawns (#20) - Set KUBECONFIG default in Makefile deploy target so it works independently of k3s-setup subshell (#22) * Fix k3s deploy gaps: orchestrator image, Calico bump, sandbox context Found while testing #1692 on a fresh Fedora aarch64 machine: - `make build` and `make k3s-import` didn't include the orchestrator image, leaving the orchestrator deployment in ImagePullBackOff. - Calico v3.27.2 arm64 image ships without libpcap.so.0.8, so calico-node CrashLoopBackOffs on arm64 hosts (upstream bug, fixed in later patches). Bumped pin to v3.31.5. - The v3.27.2 SHA256 in install-calico.sh never matched the actual upstream manifest. Recomputed and pinned v3.31.5's hash. - Sandbox build fails without a `repo-deps/` directory in the build context, normally assembled by the egg Python build flow. Added a minimal marker bootstrap so `make build` works standalone. - Updated three doc references from v3.27.0 to v3.31.5. - Added `repo-deps/` to .gitignore. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address review feedback: fix leaky abstraction and VersionConflictError handling - Add .backend property to KubernetesSpawner as runtime-agnostic accessor for the container backend client (Issue #14). This eliminates scattered `spawner.k8s if _RUNTIME == "kubernetes" else spawner.docker` patterns. - Replace all spawner.docker and if/else runtime checks in routes/pipelines.py with spawner.backend - Remove dead code: overseer stop used identical methods on both branches (stop_agent_job == stop_agent_container) — collapsed to single call - Fix VersionConflictError handling in consensus stall recovery (Issue #13): explicit catch with pipeline reload and state verification instead of generic except Exception - Update test fixtures to set mock.backend alongside mock.docker * Fix k3s deploy: gateway/orchestrator actually start end-to-end Continued validation of #1692 on a fresh machine. Prior commit addressed build and Calico install; this one makes the deployments come up. Gateway: - Rewrite base deployment to match what gateway/entrypoint.sh actually reads: /secrets (Secret mount) with launcher-secret + secrets.env + github-app.pem + repositories.yaml, /shared/certs (emptyDir where the entrypoint writes the CA cert), /home/egg emptyDir, /home/egg/.egg-state emptyDir. The previous base mounted /etc/egg-gateway/certs and /var/lib/egg-gateway — paths nothing in the code touches. - Remove runAsNonRoot: the entrypoint is designed to start as root, chown squid dirs + /home/egg, then gosu-drop to HOST_UID. Running as UID 1000 directly hit /run/squid.pid EACCES plus a dozen other issues. Preserve fsGroup: 1000 so emptyDirs are writable post-gosu. - Fix health probe path: /api/v1/health (port 9851), not /healthz. - Add EGG_CONFIG_DIR, EGG_SECRETS_PATH, EGG_REPO_CONFIG env vars so gateway/repo_config resolve their file paths to the mounted Secret. - enableServiceLinks: false to stop the auto-injected GATEWAY_PORT/etc from colliding with the entrypoint's own vars. - Chown squid dirs to egg:egg in the Dockerfile (was proxy:proxy). - Source /secrets/secrets.env in the entrypoint so GITHUB_USER_TOKEN et al. are available (Compose got them from shell env). - Make the chown-everything block in the entrypoint tolerant of read-only bind mounts (k8s hostPath readOnly returns EROFS). Orchestrator: - enableServiceLinks: false (ORCHESTRATOR_PORT was being overwritten by the auto-injected tcp://<ip>:9849 value, breaking --port parsing). - Add emptyDir at /home/egg so .gitconfig / .egg-worktrees writes don't hit the read-only rootfs. - Source /secrets/secrets.env in its entrypoint too. Local overlay: - Replace the invented .egg-gateway hostPaths with strategic-merge additions for /home/egg/repos and /home/egg/.egg-worktrees hostPaths, on both deployments. Local-dev only; paths hardcoded to /home/jwies since kustomize has no env-var substitution. - New make target `k3s-secrets` that creates gateway-secrets from all files under ~/.config/egg/; `make deploy` depends on it. Fix wait targets to match actual deployment names (orchestrator/gateway, not egg-orchestrator/egg-gateway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add 'timed out' to RESTARTABLE_PATTERNS for restart detection 'timeout' does not match 'timed out' as a substring, causing error messages like 'Agent timed out waiting for response' to miss the restartable keyword check and escalate to HITL unnecessarily. * Fix restart lock race: retain per-key locks in reset_restart_counts Addresses review feedback B1/B2: reset_restart_counts() was deleting per-key locks from _restart_locks, which races with restart_agent_job holding those locks. If a lock is deleted while held, _get_restart_lock creates a new lock for the same key — breaking mutual exclusion. Fix: only clear counter entries in reset_restart_counts(), retain locks. Locks are lightweight and bounded by (pipeline, role) pairs. * Wire orchestrator → gateway connectivity and auth With the previous commit both deployments started, but the orchestrator still couldn't talk to the gateway: - gateway_client.py reads GATEWAY_HOST/GATEWAY_PORT (not GATEWAY_URL). Its default GATEWAY_HOST is "egg-gateway", the old Compose container name — no such name resolves in k8s. Set it to the Service FQDN. - Gateway rejected requests with "Missing or invalid Authorization header" because the orchestrator had no EGG_LAUNCHER_SECRET. Inject it via secretKeyRef from the same gateway-secrets Secret that the gateway mounts at /secrets/launcher-secret. With these in, the orchestrator registers sessions with the gateway and /api/v1/pipelines returns an empty list cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix pipeline submit + expose MCP + block lowercase proxy overrides Pipeline submission tripped three more issues on top of the stack: - state_store._ensure_worktree called logger.warning with structured kwargs (worktree=..., returncode=...) but logger is a stdlib logging.Logger, not a structlog wrapper. submit_task raised TypeError in the warning path when the state worktree needed recreation. Rewrite as a printf-style format. - Local repo mounts on both deployments were readOnly, but the orchestrator creates per-pipeline worktrees inside each repo's .git/worktrees/ and the gateway runs `git worktree prune` on startup. Drop readOnly on both repos mounts. - Nothing exposed the orchestrator MCP port on the host. Added hostPort: 9850 to the local overlay so Claude Code's MCP config (http://localhost:9850/mcp) connects without a port-forward. Also changed the orchestrator Deployment strategy to Recreate because hostPort is singleton per node — a rolling update gets stuck Pending waiting for the port to free up. Also addresses review N3 (flagged 3x): _PROTECTED_ENV_KEYS in kubernetes_spawner.py now blocks the lowercase http_proxy / https_proxy / no_proxy variants too, since curl/libcurl/requests all honor either case and leaving the lowercase forms unblocked is a defense-in-depth gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire agent Jobs end-to-end: images, mounts, creds, naming Validation of #1692 kept surfacing infrastructure gaps between pipeline submit and the point where agents actually do work. This fixes the remaining ones needed to get all four phase-0 agents (refiner, reviewer_refine, reviewer_agent_design, overseer) spawning with the right mounts, credentials, and names. Agent image resolution - `kubernetes_spawner.DEFAULT_SANDBOX_IMAGE` defaulted to `egg:latest`; `make build` produces `egg-sandbox:latest`, and there is no public `docker.io/library/egg`, so every agent pod ImagePullBackOff'd. Set `EGG_SANDBOX_IMAGE=egg-sandbox:latest` on the orchestrator. - Agent `V1Container` had no `imagePullPolicy`. Default for `:latest` is `Always`, which fails for locally-imported images that only live in containerd's cache. Force `IfNotPresent`. Also added to `shared/egg_container.to_k8s_job_kwargs` for the other code path. Pod security / credentials - Agent pods had no pod-level securityContext so they ran as root. Claude CLI's `--dangerously-skip-permissions` refuses to run as root. Set `runAsUser/Group/fsGroup=1000` (the `egg` user in the sandbox image) on the pod spec built by `kubernetes_client.create_container`. - Agent env had no Anthropic credentials and no proxy routing, so the CLI hit `Not logged in · Please run /login`. Set the same two env vars that `sandbox/entrypoint.py` sets in the Compose flow: `ANTHROPIC_BASE_URL` pointing at the gateway, plus a deliberately- invalid placeholder `CLAUDE_CODE_OAUTH_TOKEN` that satisfies local validation. The gateway strips the placeholder and injects the real credential server-side — real secrets still never enter the sandbox. Volume mounts - `kubernetes_client.create_container` previously dropped volume specs on the floor ("not currently translated to k8s volume mounts"). Add a `host_path_mounts` parameter and translate each entry to a matched `V1Volume`/`V1VolumeMount` pair (hostPath, DirectoryOrCreate). - `kubernetes_spawner.spawn_agent_job` now builds those mounts from `repo_volumes` (owner/repo → host path, one mount per repo) plus a single `worktrees` mount backed by `EGG_HOST_WORKTREES_PATH`. Without these, agents couldn't see the code they were supposed to edit — they tried `gh repo clone` into an empty `/home/egg/repos`. - Added `EGG_HOST_WORKTREES_PATH=/home/jwies/.egg-worktrees` to the local overlay's orchestrator patch. Naming - Job names longer than 63 chars (k8s RFC-1123 limit) failed validation outright. Long pipeline IDs + long role names like `reviewer_agent_design` overflow deterministically. Truncate to 54 chars of readable prefix and append an 8-char SHA1 suffix so uniqueness is preserved. Surfaced by submitting with qualifier `k3s-retry` which pushed the composed name to 64 chars. Other - Gateway `limits.memory: 256Mi` was OOMKilling the pod under normal load (Squid + waitress + git operations). Bumped to 1Gi/512Mi limits. - Orchestrator deployment strategy set to `Recreate` because the local overlay binds a singleton hostPort (9850 for MCP); the default RollingUpdate deadlocks waiting for the port to free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix B324: mark SHA1 hash as not used for security * Address PR #1692 review: container hardening, volume-name collision Two blocking items from the re-review of `be617281`: 1. Agent V1Container was missing container-level securityContext. The old ConfigMap-based Job template had allowPrivilegeEscalation: false capabilities: drop: [ALL] These disappeared in the switch to programmatic Job specs. Agents already run as UID 1000 via the pod securityContext so there's no reason for them to gain new privs or hold any Linux caps. Added. 2. `kubernetes_spawner.spawn_agent_job` built volume names from the repo basename alone (`repo-{short}`). Two repos from different orgs with the same basename (e.g. `Khan/webapp` + `other-org/webapp`, plausible as the repo list grows) would collide on the volume name and k8s would reject the Job. Include the owner in the name, normalize to RFC-1123, and hash-truncate if the composed name exceeds 63 chars. Non-blocking: strengthened the comment on the local-dev orchestrator overlay patch explaining that every `/home/jwies/...` path and the EGG_HOST_REPO_MAP entries are this developer's layout and must be edited before anyone else can `make deploy`. Portability is tracked as a follow-up in #1760. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix CI: mark sha1 usedforsecurity=False, sort test_cli imports CI's bandit job flagged the new sha1 hash in kubernetes_spawner (introduced in the previous commit's volume-name collision fix) as B324 — weak hash for security. It isn't a security hash (used to pick a unique-per-name suffix); add `usedforsecurity=False` to match the identical treatment already applied in kubernetes_client. Also auto-sorted the import block in orchestrator/tests/test_cli.py that was tripping ruff's I001 (unrelated to our changes, surfaced because `make lint` runs the full tree). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix checks: apply automated formatting fixes --------- Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
james-in-a-box Bot
added a commit
that referenced
this pull request
Apr 22, 2026
* Initialize SDLC contract for issue #1553 * Add Kubernetes migration documentation and update existing docs Create docs/architecture/kubernetes-migration.md covering the Docker to k8s migration architecture, design decisions, component mapping, network isolation model, storage model, RBAC, developer workflow, and CI/CD changes. Update existing docs to reflect the k8s migration: - docs/guides/deployment.md: Replace Docker Compose with k3s deployment - docs/architecture/orchestrator.md: Update network architecture for k8s - docs/architecture/network-isolation.md: Add Kubernetes NetworkPolicy section - orchestrator/README.md: Update file listing for new k8s modules - docs/development/STRUCTURE.md: Add k8s/ directory, update orchestrator - docs/index.md: Add kubernetes-migration.md to doc index - CONTRIBUTING.md: Update integration test prereq from Docker to k3s * Update remaining docs for Docker-to-Kubernetes terminology - docs/architecture/README.md: Update system overview for k8s components - docs/architecture/git-isolation.md: Update storage/network comparison table - docs/guides/deploy-migration.md: Add deprecation note pointing to k8s - docs/guides/pipeline-health-monitoring.md: Update log reference terminology - docs/guides/concurrent-execution.md: Update worktree isolation for pod/Job naming * Add ContainerBackend protocol, KubernetesClient, and k8s manifests Phase 1: Define ContainerBackend Protocol with runtime_checkable interface that both DockerClient and KubernetesClient satisfy. Implement KubernetesClient wrapping the kubernetes Python client with Job/Pod lifecycle management, custom exception hierarchy, and singleton accessor. Add k8s-native fields (pod_name, namespace, job_name) to ContainerInfo. Phase 2: Create Kustomize manifests with base + local overlay structure. Base includes orchestrator/gateway Deployments and Services, RBAC for Job management, agent Job template with init container for .git shadow mount, and Calico NetworkPolicies enforcing default-deny with gateway-only egress for agent pods. * Update orchestrator architecture doc for k8s terminology Replace Docker-specific references with Kubernetes equivalents throughout: - ContainerMonitor → KubernetesMonitor - container_monitor.py → kubernetes_monitor.py - container_spawner.py → kubernetes_spawner.py - Docker container set → Kubernetes pod set - Docker queries → Kubernetes API queries - container ID → Job name for worktree keying - bind mounts → hostPath volumes - Docker host → host machine * Update orchestrator README for k8s terminology Replace remaining Docker-specific references: state volume, health checks, PATCH behavior, host path translation. * Update docs with accurate implementation details from coder Align migration docs with actual implementation: - NetworkPolicies: add DNS egress policy, correct label selectors (app.kubernetes.io/component, kubernetes.io/metadata.name) - ContainerBackend protocol: match actual method signatures - RBAC: document both ClusterRole and namespace-scoped Role - KubernetesClient: document label scheme (egg.pipeline.id, etc.) * Add tests for ContainerBackend protocol and KubernetesClient - test_container_backend.py: Protocol conformance (Docker, K8s, minimal, incomplete), exception hierarchy, ContainerInfo k8s fields, runtime checkability. - test_kubernetes_client.py: 101 tests covering create/start/stop/remove container, get_container_info, list_containers, logs, wait, cleanup, k8s-native methods (create_job, delete_job, list_jobs, get_pod_for_job, get_pod_logs, get_pod_status), _resolve_job_name, helper functions, singleton accessor, constants. - conftest.py: Mock kubernetes SDK (V1Container, V1Job, etc.) with attribute-storing data classes so tests work without the kubernetes package installed. * Migrate gateway to token-only auth, add KubernetesSpawner and Monitor Gateway auth: Remove IP-based session validation enforcement. Pod IPs are ephemeral in Kubernetes so sessions now authenticate by token only. IP is still recorded for audit logging. container_ip made optional in session registration. KubernetesSpawner: New spawner that creates k8s Jobs instead of Docker containers. Uses label-based identification, token-only gateway sessions, and the same SpawnedContainer interface. Supports agent and overseer job spawning, concurrent spawn functions, pipeline cleanup, and restart tracking. KubernetesMonitor: Replacement for ContainerMonitor using k8s pod polling. Detects pod state transitions, fires event callbacks, and handles orphan cleanup via label-based job listing. Routes updated to support both Docker and k8s backends via EGG_RUNTIME environment variable, defaulting to Docker for backward compatibility. * Add tests for KubernetesSpawner and KubernetesMonitor * Complete k8s migration: CLI runtime, CI/CD, Docker removal Phase 4 - CLI Runtime Migration: - Add to_k8s_job_kwargs() and build_sandbox_job_spec() to shared/egg_container/ for converting SandboxContainerConfig to k8s Job specs with proper volume, env, and security mapping. - Update sandbox/egg_lib/runtime.py with dual Docker/k8s path selected by EGG_RUNTIME env var. K8s path uses Service DNS for gateway resolution. Phase 5 - CI/CD and Docker Removal: - Add Makefile targets: k3s-setup, deploy, k3s-import, k3s-teardown. - Update CI workflows to set up k3s, import images, and deploy. - Replace Docker SDK code with backward-compat shims that re-export from kubernetes equivalents (DockerClient→KubernetesClient, etc.). - Remove docker-compose.yml files. - Replace docker>=7.0.0 with kubernetes>=31.0.0 in dependencies. - Update integration test fixtures for k3s-based test environment. - Add consensus stall recovery methods to KubernetesMonitor for backward compatibility with existing health check infrastructure. * Fix DockerClient test for k8s migration (DockerClient is now alias) * Fix 5 reviewer NACK issues: RBAC, labels, naming, singleton, list 1. SECURITY: Remove ClusterRole/ClusterRoleBinding from rbac.yaml; namespace-scoped Role+RoleBinding in egg-agents is sufficient. 2. CORRECTNESS: Add app.kubernetes.io/component:agent label in spawn_agent_job() so NetworkPolicies apply to agent pods. 3. CORRECTNESS: Prevent JOB_PREFIX double-prepending in create_container() and use correct prefixed name in spawner pre-cleanup. Add backward-compat method aliases and kwargs (docker_client, timeout, spawn_agent_container, etc.). 4. CORRECTNESS: Validate explicit namespace in singleton accessor get_kubernetes_client() using sentinel pattern. 5. CORRECTNESS: Guard against double-prefix in list_containers() when LABEL_CONTAINER_NAME is missing from pod labels. * Fix ruff violations and add _validate_container_id shim - Remove 7 unused imports (F401) from kubernetes_monitor.py and kubernetes_spawner.py via ruff check --fix. - Apply ruff format to all 3 source files. - Add _validate_container_id to docker_client.py shim so test_docker_client.py can collect without import errors. * Fix checks: apply automated formatting fixes * Fix lint: remove unused imports, fix hardcoded ports - Remove unused KubernetesClient, get_kubernetes_client, KubernetesSpawner imports from orchestrator/routes/pipelines.py (ruff F401) - Import GATEWAY_PORT/GATEWAY_PROXY_PORT from egg_config in kubernetes_spawner.py instead of hardcoding 9848/3129 - Add # noqa: EGG002 to k8s YAML manifests where port constants cannot be imported (infrastructure files require literal values) * Fix lint: sort imports in kubernetes_spawner, add raise-from in runtime * Fix mypy errors in runtime.py for kubernetes migration * Fix container_monitor tests for Kubernetes migration * Rewrite docker_client tests for Kubernetes shim layer * Update container_spawner tests for Kubernetes migration * Fix remaining test failures for Kubernetes migration * Fix kubernetes_spawner test assertions * Fix lint formatting in test files * Fix checks: align tests with Docker-to-Kubernetes migration * Address review feedback: fix all blocking issues in k8s migration Fix all 11 remaining blocking issues from the review: 1. Add resource limits (500m/512Mi req, 2CPU/2Gi limits), activeDeadlineSeconds (4h), and ttlSecondsAfterFinished (10m) to programmatic Job specs 2. Remove dead agent-job-template.yaml ConfigMap (never loaded by Python code) 3. Add allow-agent-to-orchestrator egress NetworkPolicy on port 9849 4. Fix namespace default in sandbox/egg_lib/runtime.py from egg-system to egg-agents 5. Forward timeout parameter to delete_job via grace_period_seconds 6. Add set_health_check_runner() method to KubernetesMonitor for cli.py compat 8. Add securityContext (runAsNonRoot, drop ALL caps, no privilege escalation) to gateway and orchestrator deployments 9. Add input validation on container_id/job names in KubernetesClient (_validate_name for create, _resolve_job_name for all other operations) 10. Add SHA256 checksum verification to install-calico.sh 11. Remove || true from CI Calico install and deploy steps 12. Fix EGG_REPO_PATH to include repo name derived from repos list Contract verification gaps addressed: - Add 23 unit tests for to_k8s_job_kwargs() and build_sandbox_job_spec() - Add k8s-based code paths to integration test fixtures (egg_stack, local_pipeline_stack) with test namespace creation/cleanup * Address re-review feedback: fix remaining blocking issues - B2: Add emptyDir volumes for /home/egg/.egg-state and /tmp to orchestrator deployment so it can write state with readOnlyRootFilesystem - B1: Wire health check runner into KubernetesMonitor._check_pod so RUNTIME_TICK checks fire on pod state transitions - B3: Add denylist for security-critical env vars (EGG_SESSION_TOKEN, GATEWAY_URL, HTTP_PROXY, etc.) that extra_env cannot override - N1: Move _UID_RE regex to module scope to avoid recompilation * Address non-blocking review feedback: fix stale comment, move constant to module scope - Fix stale comment in test_health_check_integration.py that incorrectly stated set_health_check_runner and _run_runtime_tick_checks were not carried over to KubernetesMonitor (they were, in da297cb) - Move _PROTECTED_ENV_KEYS from local variable to module-level constant to avoid re-creating the frozenset on every call * Add missing V1ResourceRequirements mock to fix 12 test failures * Port restart improvements from main to kubernetes_spawner: concurrency locks, pre-spawn count increment, mode validation * Fix restart count lock protection in KubernetesSpawner Match ContainerSpawner's thread-safety pattern: - get_restart_count() now acquires per-key lock before reading - reset_restart_counts() holds _restart_locks_lock while modifying both _restart_counts and _restart_locks atomically, using pop() to safely handle already-held locks * Address re-review feedback: namespace default, restart lock timeout, stale template references * Address review feedback: thread safety, correctness, and CI fixes - Add lock protection for _pod_states dict access in KubernetesMonitor to prevent data corruption from concurrent thread access (#7) - Fix exit_code=None incorrectly treated as clean exit — only exit_code==0 is a clean exit now (#18) - Prune _clean_exit_skipped when pods are removed to prevent unbounded memory growth (#21) - Remove pods/create from RBAC — orchestrator creates Jobs, not bare pods (#17) - Add sandbox image build and import to test-integration.yml to prevent ImagePullBackOff on agent pod spawns (#20) - Set KUBECONFIG default in Makefile deploy target so it works independently of k3s-setup subshell (#22) * Fix k3s deploy gaps: orchestrator image, Calico bump, sandbox context Found while testing #1692 on a fresh Fedora aarch64 machine: - `make build` and `make k3s-import` didn't include the orchestrator image, leaving the orchestrator deployment in ImagePullBackOff. - Calico v3.27.2 arm64 image ships without libpcap.so.0.8, so calico-node CrashLoopBackOffs on arm64 hosts (upstream bug, fixed in later patches). Bumped pin to v3.31.5. - The v3.27.2 SHA256 in install-calico.sh never matched the actual upstream manifest. Recomputed and pinned v3.31.5's hash. - Sandbox build fails without a `repo-deps/` directory in the build context, normally assembled by the egg Python build flow. Added a minimal marker bootstrap so `make build` works standalone. - Updated three doc references from v3.27.0 to v3.31.5. - Added `repo-deps/` to .gitignore. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address review feedback: fix leaky abstraction and VersionConflictError handling - Add .backend property to KubernetesSpawner as runtime-agnostic accessor for the container backend client (Issue #14). This eliminates scattered `spawner.k8s if _RUNTIME == "kubernetes" else spawner.docker` patterns. - Replace all spawner.docker and if/else runtime checks in routes/pipelines.py with spawner.backend - Remove dead code: overseer stop used identical methods on both branches (stop_agent_job == stop_agent_container) — collapsed to single call - Fix VersionConflictError handling in consensus stall recovery (Issue #13): explicit catch with pipeline reload and state verification instead of generic except Exception - Update test fixtures to set mock.backend alongside mock.docker * Fix k3s deploy: gateway/orchestrator actually start end-to-end Continued validation of #1692 on a fresh machine. Prior commit addressed build and Calico install; this one makes the deployments come up. Gateway: - Rewrite base deployment to match what gateway/entrypoint.sh actually reads: /secrets (Secret mount) with launcher-secret + secrets.env + github-app.pem + repositories.yaml, /shared/certs (emptyDir where the entrypoint writes the CA cert), /home/egg emptyDir, /home/egg/.egg-state emptyDir. The previous base mounted /etc/egg-gateway/certs and /var/lib/egg-gateway — paths nothing in the code touches. - Remove runAsNonRoot: the entrypoint is designed to start as root, chown squid dirs + /home/egg, then gosu-drop to HOST_UID. Running as UID 1000 directly hit /run/squid.pid EACCES plus a dozen other issues. Preserve fsGroup: 1000 so emptyDirs are writable post-gosu. - Fix health probe path: /api/v1/health (port 9851), not /healthz. - Add EGG_CONFIG_DIR, EGG_SECRETS_PATH, EGG_REPO_CONFIG env vars so gateway/repo_config resolve their file paths to the mounted Secret. - enableServiceLinks: false to stop the auto-injected GATEWAY_PORT/etc from colliding with the entrypoint's own vars. - Chown squid dirs to egg:egg in the Dockerfile (was proxy:proxy). - Source /secrets/secrets.env in the entrypoint so GITHUB_USER_TOKEN et al. are available (Compose got them from shell env). - Make the chown-everything block in the entrypoint tolerant of read-only bind mounts (k8s hostPath readOnly returns EROFS). Orchestrator: - enableServiceLinks: false (ORCHESTRATOR_PORT was being overwritten by the auto-injected tcp://<ip>:9849 value, breaking --port parsing). - Add emptyDir at /home/egg so .gitconfig / .egg-worktrees writes don't hit the read-only rootfs. - Source /secrets/secrets.env in its entrypoint too. Local overlay: - Replace the invented .egg-gateway hostPaths with strategic-merge additions for /home/egg/repos and /home/egg/.egg-worktrees hostPaths, on both deployments. Local-dev only; paths hardcoded to /home/jwies since kustomize has no env-var substitution. - New make target `k3s-secrets` that creates gateway-secrets from all files under ~/.config/egg/; `make deploy` depends on it. Fix wait targets to match actual deployment names (orchestrator/gateway, not egg-orchestrator/egg-gateway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add 'timed out' to RESTARTABLE_PATTERNS for restart detection 'timeout' does not match 'timed out' as a substring, causing error messages like 'Agent timed out waiting for response' to miss the restartable keyword check and escalate to HITL unnecessarily. * Fix restart lock race: retain per-key locks in reset_restart_counts Addresses review feedback B1/B2: reset_restart_counts() was deleting per-key locks from _restart_locks, which races with restart_agent_job holding those locks. If a lock is deleted while held, _get_restart_lock creates a new lock for the same key — breaking mutual exclusion. Fix: only clear counter entries in reset_restart_counts(), retain locks. Locks are lightweight and bounded by (pipeline, role) pairs. * Wire orchestrator → gateway connectivity and auth With the previous commit both deployments started, but the orchestrator still couldn't talk to the gateway: - gateway_client.py reads GATEWAY_HOST/GATEWAY_PORT (not GATEWAY_URL). Its default GATEWAY_HOST is "egg-gateway", the old Compose container name — no such name resolves in k8s. Set it to the Service FQDN. - Gateway rejected requests with "Missing or invalid Authorization header" because the orchestrator had no EGG_LAUNCHER_SECRET. Inject it via secretKeyRef from the same gateway-secrets Secret that the gateway mounts at /secrets/launcher-secret. With these in, the orchestrator registers sessions with the gateway and /api/v1/pipelines returns an empty list cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix pipeline submit + expose MCP + block lowercase proxy overrides Pipeline submission tripped three more issues on top of the stack: - state_store._ensure_worktree called logger.warning with structured kwargs (worktree=..., returncode=...) but logger is a stdlib logging.Logger, not a structlog wrapper. submit_task raised TypeError in the warning path when the state worktree needed recreation. Rewrite as a printf-style format. - Local repo mounts on both deployments were readOnly, but the orchestrator creates per-pipeline worktrees inside each repo's .git/worktrees/ and the gateway runs `git worktree prune` on startup. Drop readOnly on both repos mounts. - Nothing exposed the orchestrator MCP port on the host. Added hostPort: 9850 to the local overlay so Claude Code's MCP config (http://localhost:9850/mcp) connects without a port-forward. Also changed the orchestrator Deployment strategy to Recreate because hostPort is singleton per node — a rolling update gets stuck Pending waiting for the port to free up. Also addresses review N3 (flagged 3x): _PROTECTED_ENV_KEYS in kubernetes_spawner.py now blocks the lowercase http_proxy / https_proxy / no_proxy variants too, since curl/libcurl/requests all honor either case and leaving the lowercase forms unblocked is a defense-in-depth gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire agent Jobs end-to-end: images, mounts, creds, naming Validation of #1692 kept surfacing infrastructure gaps between pipeline submit and the point where agents actually do work. This fixes the remaining ones needed to get all four phase-0 agents (refiner, reviewer_refine, reviewer_agent_design, overseer) spawning with the right mounts, credentials, and names. Agent image resolution - `kubernetes_spawner.DEFAULT_SANDBOX_IMAGE` defaulted to `egg:latest`; `make build` produces `egg-sandbox:latest`, and there is no public `docker.io/library/egg`, so every agent pod ImagePullBackOff'd. Set `EGG_SANDBOX_IMAGE=egg-sandbox:latest` on the orchestrator. - Agent `V1Container` had no `imagePullPolicy`. Default for `:latest` is `Always`, which fails for locally-imported images that only live in containerd's cache. Force `IfNotPresent`. Also added to `shared/egg_container.to_k8s_job_kwargs` for the other code path. Pod security / credentials - Agent pods had no pod-level securityContext so they ran as root. Claude CLI's `--dangerously-skip-permissions` refuses to run as root. Set `runAsUser/Group/fsGroup=1000` (the `egg` user in the sandbox image) on the pod spec built by `kubernetes_client.create_container`. - Agent env had no Anthropic credentials and no proxy routing, so the CLI hit `Not logged in · Please run /login`. Set the same two env vars that `sandbox/entrypoint.py` sets in the Compose flow: `ANTHROPIC_BASE_URL` pointing at the gateway, plus a deliberately- invalid placeholder `CLAUDE_CODE_OAUTH_TOKEN` that satisfies local validation. The gateway strips the placeholder and injects the real credential server-side — real secrets still never enter the sandbox. Volume mounts - `kubernetes_client.create_container` previously dropped volume specs on the floor ("not currently translated to k8s volume mounts"). Add a `host_path_mounts` parameter and translate each entry to a matched `V1Volume`/`V1VolumeMount` pair (hostPath, DirectoryOrCreate). - `kubernetes_spawner.spawn_agent_job` now builds those mounts from `repo_volumes` (owner/repo → host path, one mount per repo) plus a single `worktrees` mount backed by `EGG_HOST_WORKTREES_PATH`. Without these, agents couldn't see the code they were supposed to edit — they tried `gh repo clone` into an empty `/home/egg/repos`. - Added `EGG_HOST_WORKTREES_PATH=/home/jwies/.egg-worktrees` to the local overlay's orchestrator patch. Naming - Job names longer than 63 chars (k8s RFC-1123 limit) failed validation outright. Long pipeline IDs + long role names like `reviewer_agent_design` overflow deterministically. Truncate to 54 chars of readable prefix and append an 8-char SHA1 suffix so uniqueness is preserved. Surfaced by submitting with qualifier `k3s-retry` which pushed the composed name to 64 chars. Other - Gateway `limits.memory: 256Mi` was OOMKilling the pod under normal load (Squid + waitress + git operations). Bumped to 1Gi/512Mi limits. - Orchestrator deployment strategy set to `Recreate` because the local overlay binds a singleton hostPort (9850 for MCP); the default RollingUpdate deadlocks waiting for the port to free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix B324: mark SHA1 hash as not used for security * Address PR #1692 review: container hardening, volume-name collision Two blocking items from the re-review of `be617281`: 1. Agent V1Container was missing container-level securityContext. The old ConfigMap-based Job template had allowPrivilegeEscalation: false capabilities: drop: [ALL] These disappeared in the switch to programmatic Job specs. Agents already run as UID 1000 via the pod securityContext so there's no reason for them to gain new privs or hold any Linux caps. Added. 2. `kubernetes_spawner.spawn_agent_job` built volume names from the repo basename alone (`repo-{short}`). Two repos from different orgs with the same basename (e.g. `Khan/webapp` + `other-org/webapp`, plausible as the repo list grows) would collide on the volume name and k8s would reject the Job. Include the owner in the name, normalize to RFC-1123, and hash-truncate if the composed name exceeds 63 chars. Non-blocking: strengthened the comment on the local-dev orchestrator overlay patch explaining that every `/home/jwies/...` path and the EGG_HOST_REPO_MAP entries are this developer's layout and must be edited before anyone else can `make deploy`. Portability is tracked as a follow-up in #1760. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix CI: mark sha1 usedforsecurity=False, sort test_cli imports CI's bandit job flagged the new sha1 hash in kubernetes_spawner (introduced in the previous commit's volume-name collision fix) as B324 — weak hash for security. It isn't a security hash (used to pick a unique-per-name suffix); add `usedforsecurity=False` to match the identical treatment already applied in kubernetes_client. Also auto-sorted the import block in orchestrator/tests/test_cli.py that was tripping ruff's I001 (unrelated to our changes, surfaced because `make lint` runs the full tree). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix checks: apply automated formatting fixes --------- Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
james-in-a-box Bot
added a commit
that referenced
this pull request
Apr 22, 2026
* Initialize SDLC contract for issue #1553 * Add Kubernetes migration documentation and update existing docs Create docs/architecture/kubernetes-migration.md covering the Docker to k8s migration architecture, design decisions, component mapping, network isolation model, storage model, RBAC, developer workflow, and CI/CD changes. Update existing docs to reflect the k8s migration: - docs/guides/deployment.md: Replace Docker Compose with k3s deployment - docs/architecture/orchestrator.md: Update network architecture for k8s - docs/architecture/network-isolation.md: Add Kubernetes NetworkPolicy section - orchestrator/README.md: Update file listing for new k8s modules - docs/development/STRUCTURE.md: Add k8s/ directory, update orchestrator - docs/index.md: Add kubernetes-migration.md to doc index - CONTRIBUTING.md: Update integration test prereq from Docker to k3s * Update remaining docs for Docker-to-Kubernetes terminology - docs/architecture/README.md: Update system overview for k8s components - docs/architecture/git-isolation.md: Update storage/network comparison table - docs/guides/deploy-migration.md: Add deprecation note pointing to k8s - docs/guides/pipeline-health-monitoring.md: Update log reference terminology - docs/guides/concurrent-execution.md: Update worktree isolation for pod/Job naming * Add ContainerBackend protocol, KubernetesClient, and k8s manifests Phase 1: Define ContainerBackend Protocol with runtime_checkable interface that both DockerClient and KubernetesClient satisfy. Implement KubernetesClient wrapping the kubernetes Python client with Job/Pod lifecycle management, custom exception hierarchy, and singleton accessor. Add k8s-native fields (pod_name, namespace, job_name) to ContainerInfo. Phase 2: Create Kustomize manifests with base + local overlay structure. Base includes orchestrator/gateway Deployments and Services, RBAC for Job management, agent Job template with init container for .git shadow mount, and Calico NetworkPolicies enforcing default-deny with gateway-only egress for agent pods. * Update orchestrator architecture doc for k8s terminology Replace Docker-specific references with Kubernetes equivalents throughout: - ContainerMonitor → KubernetesMonitor - container_monitor.py → kubernetes_monitor.py - container_spawner.py → kubernetes_spawner.py - Docker container set → Kubernetes pod set - Docker queries → Kubernetes API queries - container ID → Job name for worktree keying - bind mounts → hostPath volumes - Docker host → host machine * Update orchestrator README for k8s terminology Replace remaining Docker-specific references: state volume, health checks, PATCH behavior, host path translation. * Update docs with accurate implementation details from coder Align migration docs with actual implementation: - NetworkPolicies: add DNS egress policy, correct label selectors (app.kubernetes.io/component, kubernetes.io/metadata.name) - ContainerBackend protocol: match actual method signatures - RBAC: document both ClusterRole and namespace-scoped Role - KubernetesClient: document label scheme (egg.pipeline.id, etc.) * Add tests for ContainerBackend protocol and KubernetesClient - test_container_backend.py: Protocol conformance (Docker, K8s, minimal, incomplete), exception hierarchy, ContainerInfo k8s fields, runtime checkability. - test_kubernetes_client.py: 101 tests covering create/start/stop/remove container, get_container_info, list_containers, logs, wait, cleanup, k8s-native methods (create_job, delete_job, list_jobs, get_pod_for_job, get_pod_logs, get_pod_status), _resolve_job_name, helper functions, singleton accessor, constants. - conftest.py: Mock kubernetes SDK (V1Container, V1Job, etc.) with attribute-storing data classes so tests work without the kubernetes package installed. * Migrate gateway to token-only auth, add KubernetesSpawner and Monitor Gateway auth: Remove IP-based session validation enforcement. Pod IPs are ephemeral in Kubernetes so sessions now authenticate by token only. IP is still recorded for audit logging. container_ip made optional in session registration. KubernetesSpawner: New spawner that creates k8s Jobs instead of Docker containers. Uses label-based identification, token-only gateway sessions, and the same SpawnedContainer interface. Supports agent and overseer job spawning, concurrent spawn functions, pipeline cleanup, and restart tracking. KubernetesMonitor: Replacement for ContainerMonitor using k8s pod polling. Detects pod state transitions, fires event callbacks, and handles orphan cleanup via label-based job listing. Routes updated to support both Docker and k8s backends via EGG_RUNTIME environment variable, defaulting to Docker for backward compatibility. * Add tests for KubernetesSpawner and KubernetesMonitor * Complete k8s migration: CLI runtime, CI/CD, Docker removal Phase 4 - CLI Runtime Migration: - Add to_k8s_job_kwargs() and build_sandbox_job_spec() to shared/egg_container/ for converting SandboxContainerConfig to k8s Job specs with proper volume, env, and security mapping. - Update sandbox/egg_lib/runtime.py with dual Docker/k8s path selected by EGG_RUNTIME env var. K8s path uses Service DNS for gateway resolution. Phase 5 - CI/CD and Docker Removal: - Add Makefile targets: k3s-setup, deploy, k3s-import, k3s-teardown. - Update CI workflows to set up k3s, import images, and deploy. - Replace Docker SDK code with backward-compat shims that re-export from kubernetes equivalents (DockerClient→KubernetesClient, etc.). - Remove docker-compose.yml files. - Replace docker>=7.0.0 with kubernetes>=31.0.0 in dependencies. - Update integration test fixtures for k3s-based test environment. - Add consensus stall recovery methods to KubernetesMonitor for backward compatibility with existing health check infrastructure. * Fix DockerClient test for k8s migration (DockerClient is now alias) * Fix 5 reviewer NACK issues: RBAC, labels, naming, singleton, list 1. SECURITY: Remove ClusterRole/ClusterRoleBinding from rbac.yaml; namespace-scoped Role+RoleBinding in egg-agents is sufficient. 2. CORRECTNESS: Add app.kubernetes.io/component:agent label in spawn_agent_job() so NetworkPolicies apply to agent pods. 3. CORRECTNESS: Prevent JOB_PREFIX double-prepending in create_container() and use correct prefixed name in spawner pre-cleanup. Add backward-compat method aliases and kwargs (docker_client, timeout, spawn_agent_container, etc.). 4. CORRECTNESS: Validate explicit namespace in singleton accessor get_kubernetes_client() using sentinel pattern. 5. CORRECTNESS: Guard against double-prefix in list_containers() when LABEL_CONTAINER_NAME is missing from pod labels. * Fix ruff violations and add _validate_container_id shim - Remove 7 unused imports (F401) from kubernetes_monitor.py and kubernetes_spawner.py via ruff check --fix. - Apply ruff format to all 3 source files. - Add _validate_container_id to docker_client.py shim so test_docker_client.py can collect without import errors. * Fix checks: apply automated formatting fixes * Fix lint: remove unused imports, fix hardcoded ports - Remove unused KubernetesClient, get_kubernetes_client, KubernetesSpawner imports from orchestrator/routes/pipelines.py (ruff F401) - Import GATEWAY_PORT/GATEWAY_PROXY_PORT from egg_config in kubernetes_spawner.py instead of hardcoding 9848/3129 - Add # noqa: EGG002 to k8s YAML manifests where port constants cannot be imported (infrastructure files require literal values) * Fix lint: sort imports in kubernetes_spawner, add raise-from in runtime * Fix mypy errors in runtime.py for kubernetes migration * Fix container_monitor tests for Kubernetes migration * Rewrite docker_client tests for Kubernetes shim layer * Update container_spawner tests for Kubernetes migration * Fix remaining test failures for Kubernetes migration * Fix kubernetes_spawner test assertions * Fix lint formatting in test files * Fix checks: align tests with Docker-to-Kubernetes migration * Address review feedback: fix all blocking issues in k8s migration Fix all 11 remaining blocking issues from the review: 1. Add resource limits (500m/512Mi req, 2CPU/2Gi limits), activeDeadlineSeconds (4h), and ttlSecondsAfterFinished (10m) to programmatic Job specs 2. Remove dead agent-job-template.yaml ConfigMap (never loaded by Python code) 3. Add allow-agent-to-orchestrator egress NetworkPolicy on port 9849 4. Fix namespace default in sandbox/egg_lib/runtime.py from egg-system to egg-agents 5. Forward timeout parameter to delete_job via grace_period_seconds 6. Add set_health_check_runner() method to KubernetesMonitor for cli.py compat 8. Add securityContext (runAsNonRoot, drop ALL caps, no privilege escalation) to gateway and orchestrator deployments 9. Add input validation on container_id/job names in KubernetesClient (_validate_name for create, _resolve_job_name for all other operations) 10. Add SHA256 checksum verification to install-calico.sh 11. Remove || true from CI Calico install and deploy steps 12. Fix EGG_REPO_PATH to include repo name derived from repos list Contract verification gaps addressed: - Add 23 unit tests for to_k8s_job_kwargs() and build_sandbox_job_spec() - Add k8s-based code paths to integration test fixtures (egg_stack, local_pipeline_stack) with test namespace creation/cleanup * Address re-review feedback: fix remaining blocking issues - B2: Add emptyDir volumes for /home/egg/.egg-state and /tmp to orchestrator deployment so it can write state with readOnlyRootFilesystem - B1: Wire health check runner into KubernetesMonitor._check_pod so RUNTIME_TICK checks fire on pod state transitions - B3: Add denylist for security-critical env vars (EGG_SESSION_TOKEN, GATEWAY_URL, HTTP_PROXY, etc.) that extra_env cannot override - N1: Move _UID_RE regex to module scope to avoid recompilation * Address non-blocking review feedback: fix stale comment, move constant to module scope - Fix stale comment in test_health_check_integration.py that incorrectly stated set_health_check_runner and _run_runtime_tick_checks were not carried over to KubernetesMonitor (they were, in da297cb) - Move _PROTECTED_ENV_KEYS from local variable to module-level constant to avoid re-creating the frozenset on every call * Add missing V1ResourceRequirements mock to fix 12 test failures * Port restart improvements from main to kubernetes_spawner: concurrency locks, pre-spawn count increment, mode validation * Fix restart count lock protection in KubernetesSpawner Match ContainerSpawner's thread-safety pattern: - get_restart_count() now acquires per-key lock before reading - reset_restart_counts() holds _restart_locks_lock while modifying both _restart_counts and _restart_locks atomically, using pop() to safely handle already-held locks * Address re-review feedback: namespace default, restart lock timeout, stale template references * Address review feedback: thread safety, correctness, and CI fixes - Add lock protection for _pod_states dict access in KubernetesMonitor to prevent data corruption from concurrent thread access (#7) - Fix exit_code=None incorrectly treated as clean exit — only exit_code==0 is a clean exit now (#18) - Prune _clean_exit_skipped when pods are removed to prevent unbounded memory growth (#21) - Remove pods/create from RBAC — orchestrator creates Jobs, not bare pods (#17) - Add sandbox image build and import to test-integration.yml to prevent ImagePullBackOff on agent pod spawns (#20) - Set KUBECONFIG default in Makefile deploy target so it works independently of k3s-setup subshell (#22) * Fix k3s deploy gaps: orchestrator image, Calico bump, sandbox context Found while testing #1692 on a fresh Fedora aarch64 machine: - `make build` and `make k3s-import` didn't include the orchestrator image, leaving the orchestrator deployment in ImagePullBackOff. - Calico v3.27.2 arm64 image ships without libpcap.so.0.8, so calico-node CrashLoopBackOffs on arm64 hosts (upstream bug, fixed in later patches). Bumped pin to v3.31.5. - The v3.27.2 SHA256 in install-calico.sh never matched the actual upstream manifest. Recomputed and pinned v3.31.5's hash. - Sandbox build fails without a `repo-deps/` directory in the build context, normally assembled by the egg Python build flow. Added a minimal marker bootstrap so `make build` works standalone. - Updated three doc references from v3.27.0 to v3.31.5. - Added `repo-deps/` to .gitignore. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address review feedback: fix leaky abstraction and VersionConflictError handling - Add .backend property to KubernetesSpawner as runtime-agnostic accessor for the container backend client (Issue #14). This eliminates scattered `spawner.k8s if _RUNTIME == "kubernetes" else spawner.docker` patterns. - Replace all spawner.docker and if/else runtime checks in routes/pipelines.py with spawner.backend - Remove dead code: overseer stop used identical methods on both branches (stop_agent_job == stop_agent_container) — collapsed to single call - Fix VersionConflictError handling in consensus stall recovery (Issue #13): explicit catch with pipeline reload and state verification instead of generic except Exception - Update test fixtures to set mock.backend alongside mock.docker * Fix k3s deploy: gateway/orchestrator actually start end-to-end Continued validation of #1692 on a fresh machine. Prior commit addressed build and Calico install; this one makes the deployments come up. Gateway: - Rewrite base deployment to match what gateway/entrypoint.sh actually reads: /secrets (Secret mount) with launcher-secret + secrets.env + github-app.pem + repositories.yaml, /shared/certs (emptyDir where the entrypoint writes the CA cert), /home/egg emptyDir, /home/egg/.egg-state emptyDir. The previous base mounted /etc/egg-gateway/certs and /var/lib/egg-gateway — paths nothing in the code touches. - Remove runAsNonRoot: the entrypoint is designed to start as root, chown squid dirs + /home/egg, then gosu-drop to HOST_UID. Running as UID 1000 directly hit /run/squid.pid EACCES plus a dozen other issues. Preserve fsGroup: 1000 so emptyDirs are writable post-gosu. - Fix health probe path: /api/v1/health (port 9851), not /healthz. - Add EGG_CONFIG_DIR, EGG_SECRETS_PATH, EGG_REPO_CONFIG env vars so gateway/repo_config resolve their file paths to the mounted Secret. - enableServiceLinks: false to stop the auto-injected GATEWAY_PORT/etc from colliding with the entrypoint's own vars. - Chown squid dirs to egg:egg in the Dockerfile (was proxy:proxy). - Source /secrets/secrets.env in the entrypoint so GITHUB_USER_TOKEN et al. are available (Compose got them from shell env). - Make the chown-everything block in the entrypoint tolerant of read-only bind mounts (k8s hostPath readOnly returns EROFS). Orchestrator: - enableServiceLinks: false (ORCHESTRATOR_PORT was being overwritten by the auto-injected tcp://<ip>:9849 value, breaking --port parsing). - Add emptyDir at /home/egg so .gitconfig / .egg-worktrees writes don't hit the read-only rootfs. - Source /secrets/secrets.env in its entrypoint too. Local overlay: - Replace the invented .egg-gateway hostPaths with strategic-merge additions for /home/egg/repos and /home/egg/.egg-worktrees hostPaths, on both deployments. Local-dev only; paths hardcoded to /home/jwies since kustomize has no env-var substitution. - New make target `k3s-secrets` that creates gateway-secrets from all files under ~/.config/egg/; `make deploy` depends on it. Fix wait targets to match actual deployment names (orchestrator/gateway, not egg-orchestrator/egg-gateway). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add 'timed out' to RESTARTABLE_PATTERNS for restart detection 'timeout' does not match 'timed out' as a substring, causing error messages like 'Agent timed out waiting for response' to miss the restartable keyword check and escalate to HITL unnecessarily. * Fix restart lock race: retain per-key locks in reset_restart_counts Addresses review feedback B1/B2: reset_restart_counts() was deleting per-key locks from _restart_locks, which races with restart_agent_job holding those locks. If a lock is deleted while held, _get_restart_lock creates a new lock for the same key — breaking mutual exclusion. Fix: only clear counter entries in reset_restart_counts(), retain locks. Locks are lightweight and bounded by (pipeline, role) pairs. * Wire orchestrator → gateway connectivity and auth With the previous commit both deployments started, but the orchestrator still couldn't talk to the gateway: - gateway_client.py reads GATEWAY_HOST/GATEWAY_PORT (not GATEWAY_URL). Its default GATEWAY_HOST is "egg-gateway", the old Compose container name — no such name resolves in k8s. Set it to the Service FQDN. - Gateway rejected requests with "Missing or invalid Authorization header" because the orchestrator had no EGG_LAUNCHER_SECRET. Inject it via secretKeyRef from the same gateway-secrets Secret that the gateway mounts at /secrets/launcher-secret. With these in, the orchestrator registers sessions with the gateway and /api/v1/pipelines returns an empty list cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix pipeline submit + expose MCP + block lowercase proxy overrides Pipeline submission tripped three more issues on top of the stack: - state_store._ensure_worktree called logger.warning with structured kwargs (worktree=..., returncode=...) but logger is a stdlib logging.Logger, not a structlog wrapper. submit_task raised TypeError in the warning path when the state worktree needed recreation. Rewrite as a printf-style format. - Local repo mounts on both deployments were readOnly, but the orchestrator creates per-pipeline worktrees inside each repo's .git/worktrees/ and the gateway runs `git worktree prune` on startup. Drop readOnly on both repos mounts. - Nothing exposed the orchestrator MCP port on the host. Added hostPort: 9850 to the local overlay so Claude Code's MCP config (http://localhost:9850/mcp) connects without a port-forward. Also changed the orchestrator Deployment strategy to Recreate because hostPort is singleton per node — a rolling update gets stuck Pending waiting for the port to free up. Also addresses review N3 (flagged 3x): _PROTECTED_ENV_KEYS in kubernetes_spawner.py now blocks the lowercase http_proxy / https_proxy / no_proxy variants too, since curl/libcurl/requests all honor either case and leaving the lowercase forms unblocked is a defense-in-depth gap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Wire agent Jobs end-to-end: images, mounts, creds, naming Validation of #1692 kept surfacing infrastructure gaps between pipeline submit and the point where agents actually do work. This fixes the remaining ones needed to get all four phase-0 agents (refiner, reviewer_refine, reviewer_agent_design, overseer) spawning with the right mounts, credentials, and names. Agent image resolution - `kubernetes_spawner.DEFAULT_SANDBOX_IMAGE` defaulted to `egg:latest`; `make build` produces `egg-sandbox:latest`, and there is no public `docker.io/library/egg`, so every agent pod ImagePullBackOff'd. Set `EGG_SANDBOX_IMAGE=egg-sandbox:latest` on the orchestrator. - Agent `V1Container` had no `imagePullPolicy`. Default for `:latest` is `Always`, which fails for locally-imported images that only live in containerd's cache. Force `IfNotPresent`. Also added to `shared/egg_container.to_k8s_job_kwargs` for the other code path. Pod security / credentials - Agent pods had no pod-level securityContext so they ran as root. Claude CLI's `--dangerously-skip-permissions` refuses to run as root. Set `runAsUser/Group/fsGroup=1000` (the `egg` user in the sandbox image) on the pod spec built by `kubernetes_client.create_container`. - Agent env had no Anthropic credentials and no proxy routing, so the CLI hit `Not logged in · Please run /login`. Set the same two env vars that `sandbox/entrypoint.py` sets in the Compose flow: `ANTHROPIC_BASE_URL` pointing at the gateway, plus a deliberately- invalid placeholder `CLAUDE_CODE_OAUTH_TOKEN` that satisfies local validation. The gateway strips the placeholder and injects the real credential server-side — real secrets still never enter the sandbox. Volume mounts - `kubernetes_client.create_container` previously dropped volume specs on the floor ("not currently translated to k8s volume mounts"). Add a `host_path_mounts` parameter and translate each entry to a matched `V1Volume`/`V1VolumeMount` pair (hostPath, DirectoryOrCreate). - `kubernetes_spawner.spawn_agent_job` now builds those mounts from `repo_volumes` (owner/repo → host path, one mount per repo) plus a single `worktrees` mount backed by `EGG_HOST_WORKTREES_PATH`. Without these, agents couldn't see the code they were supposed to edit — they tried `gh repo clone` into an empty `/home/egg/repos`. - Added `EGG_HOST_WORKTREES_PATH=/home/jwies/.egg-worktrees` to the local overlay's orchestrator patch. Naming - Job names longer than 63 chars (k8s RFC-1123 limit) failed validation outright. Long pipeline IDs + long role names like `reviewer_agent_design` overflow deterministically. Truncate to 54 chars of readable prefix and append an 8-char SHA1 suffix so uniqueness is preserved. Surfaced by submitting with qualifier `k3s-retry` which pushed the composed name to 64 chars. Other - Gateway `limits.memory: 256Mi` was OOMKilling the pod under normal load (Squid + waitress + git operations). Bumped to 1Gi/512Mi limits. - Orchestrator deployment strategy set to `Recreate` because the local overlay binds a singleton hostPort (9850 for MCP); the default RollingUpdate deadlocks waiting for the port to free. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix B324: mark SHA1 hash as not used for security * Address PR #1692 review: container hardening, volume-name collision Two blocking items from the re-review of `be617281`: 1. Agent V1Container was missing container-level securityContext. The old ConfigMap-based Job template had allowPrivilegeEscalation: false capabilities: drop: [ALL] These disappeared in the switch to programmatic Job specs. Agents already run as UID 1000 via the pod securityContext so there's no reason for them to gain new privs or hold any Linux caps. Added. 2. `kubernetes_spawner.spawn_agent_job` built volume names from the repo basename alone (`repo-{short}`). Two repos from different orgs with the same basename (e.g. `Khan/webapp` + `other-org/webapp`, plausible as the repo list grows) would collide on the volume name and k8s would reject the Job. Include the owner in the name, normalize to RFC-1123, and hash-truncate if the composed name exceeds 63 chars. Non-blocking: strengthened the comment on the local-dev orchestrator overlay patch explaining that every `/home/jwies/...` path and the EGG_HOST_REPO_MAP entries are this developer's layout and must be edited before anyone else can `make deploy`. Portability is tracked as a follow-up in #1760. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix CI: mark sha1 usedforsecurity=False, sort test_cli imports CI's bandit job flagged the new sha1 hash in kubernetes_spawner (introduced in the previous commit's volume-name collision fix) as B324 — weak hash for security. It isn't a security hash (used to pick a unique-per-name suffix); add `usedforsecurity=False` to match the identical treatment already applied in kubernetes_client. Also auto-sorted the import block in orchestrator/tests/test_cli.py that was tripping ruff's I001 (unrelated to our changes, surfaced because `make lint` runs the full tree). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix checks: apply automated formatting fixes --------- Co-authored-by: egg-orchestrator <egg@localhost> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract comprehensive test suite (~2,500 lines) from james-in-a-box gateway-sidecar per issue #17. This brings the test count from 174 to 788 (all passing, 19 skipped for sandbox environment).
Coverage improved from 68% to 80% with new tests for:
test_proxy_monitor.py- Proxy monitoring and audit logging (0% -> 77%)test_config_validator.py- Config validation and network lockdown (16% -> 72%)test_egg_config.py- YAML loading, env var expansion, validators (20-22% -> 100%)test_fork_policy.py- Fork operations in Private Repo Mode (36% -> 100%)test_error_messages.py- User-friendly error messages (47% -> 100%)test_logging.py- Structured logging formatters (74% -> 99%)Test files extracted:
test_git_validation.py- Path validation, git arg sanitization, gh API allowlisttest_rate_limiter.py- Sliding window rate limiting, thread safetytest_repo_parser.py- GitHub URL parsing, owner/repo extraction (61% -> 96%)test_repo_visibility.py- Visibility checking, caching, multi-token fallbacktest_session_manager.py- Session lifecycle, persistence, thread safetytest_token_refresher.py- Token refresh, caching, failure handlingtest_private_repo_policy.py- Private/public mode repo access controltest_worktree_manager.py- Worktree operations, path traversal prevention (48% -> 71%)test_proxy_security.py- Network lockdown, IP blocking patterns (skips gracefully if proxy config not present)Improvements:
conftest.pywith test environment setup and common fixturesGH_API_ALLOWED_PATHSwith 10 new endpoints: PR review comments, issue events/timeline, commit comments, and releases (Expand gh API allowlist with additional endpoints #23)Issue: #17
Also closes: #23
Test plan:
./dev native test- 788 passed, 19 skippedAuthored-by: jib