From 14b0f5c2d611188ea1fc5a3bc37097831b7063bb Mon Sep 17 00:00:00 2001 From: q Date: Mon, 19 Oct 2026 05:26:01 +0000 Subject: [PATCH 1/2] Skip unchanged deployer-owned GitHub secrets with a lock-fenced Redis receipt A deploy digests the GitHub repository id and its six deployer-owned secret values. A matching receipt from an earlier complete write skips those six PUTs and their public-key reads; REGISTRY_* are still written on every deploy. The receipt is claimed (hit or deleted) and published in Lua scripts that compare the project deploy lock first, and published only after all nine secrets were set. Every secret PUT attempt, retries included, checks the lock through the shared request guard, and a refusal stops the batch as DeployFenceLost. --- docs/CHANGELOG.md | 2 + docs/ERROR_HANDLING.md | 2 +- docs/SECRETS.md | 7 +- services/langgraph/src/deploy_fence.py | 18 +- .../src/subgraphs/devops/deploy_secrets.py | 134 +++++++ .../src/subgraphs/devops/deployer.py | 87 +---- .../tests/service/_public_deploy_transport.py | 11 + .../service/test_deploy_secrets_receipt.py | 90 +++++ .../tests/unit/consumers/test_deploy_fence.py | 6 +- .../tests/unit/test_deploy_secrets.py | 339 ++++++++++++++++++ .../langgraph/tests/unit/test_deployer.py | 65 +++- .../test_imported_repository_merge_order.py | 2 +- shared/clients/github/_secrets.py | 14 +- .../clients/test_github_repository_secrets.py | 72 ++++ tests/live/test_deploy_infra.py | 4 +- 15 files changed, 762 insertions(+), 91 deletions(-) create mode 100644 services/langgraph/src/subgraphs/devops/deploy_secrets.py create mode 100644 services/langgraph/tests/service/test_deploy_secrets_receipt.py create mode 100644 services/langgraph/tests/unit/test_deploy_secrets.py create mode 100644 shared/tests/clients/test_github_repository_secrets.py diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3ee926e44..18e16082d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,8 @@ See the CHANGELOG rule in [AGENTS.md](../AGENTS.md). ## 2026-10-19 +- A redeploy with unchanged deployer-owned secrets skips their six GitHub writes: a lock-fenced Redis + digest receipt certifies the last complete write; `REGISTRY_*` are still written every deploy. - New products scaffold from kit tag `0.11.0` (commit `59a0752c`): parallel PR CI behind fail-closed `lint-and-test`, native SSH deploy with `up --wait`; existing products keep their workflow until updated. - The deployer rereads a pending deploy run and an accepted failed-job rerun every 3 s instead of diff --git a/docs/ERROR_HANDLING.md b/docs/ERROR_HANDLING.md index f54fb6f71..907a2725b 100644 --- a/docs/ERROR_HANDLING.md +++ b/docs/ERROR_HANDLING.md @@ -343,7 +343,7 @@ unknown failures use the bounded `RETRY` path. A future remediation agent may di asynchronously and propose a tested code fix outside the deploy path. ### Deploy Deduplication and the Deploy Fence -Atomic `SET NX EX` Redis lock per project (`deploy::lock`, TTL `deploy.deploy_lock_ttl`) prevents duplicate deploys. Its value is a token unique to the holder (`:`), and the claim is the deploy's `DeployFence` (`services/langgraph/src/deploy_fence.py`), threaded explicitly through the consumer, the DevOps subgraph state (`deploy_fence`) and the result handlers. Every deploy write calls `DeployFence.ensure_held()` — a Lua GET-compare — immediately before it is made: secret persistence and GitHub secret writes, the pin tag, stopping older runs, workflow dispatch and rerun, the deployment record, the post-deploy product writes (owner grant, settings seed), remote execution, and the deploy's Run and application state. Two rules govern the checks: nothing is awaited between a check and its effect (an API claim, read or wait in between gets its own check after it, so the GitHub dispatch and rerun check again after the dispatch claim), and a Run's terminal outcome is written last (a lifecycle action moves the application's status before it records its Run). A deploy whose lock expired or was replaced raises `DeployFenceLost`, makes no further write, and records its own Run once as `DeployOutcome.DEPLOY_LOCK_LOST` — never over a Run that already holds a terminal outcome, which the API refuses to rewrite; the supervisor redeploys the story under the retry bound. Release is a compare-and-delete in `finally`, so a deploy never deletes a lock another deploy now holds. The check narrows the window to one Redis round trip; it cannot stop a write already in flight when the key expires. +Atomic `SET NX EX` Redis lock per project (`deploy::lock`, TTL `deploy.deploy_lock_ttl`) prevents duplicate deploys. Its value is a token unique to the holder (`:`), and the claim is the deploy's `DeployFence` (`services/langgraph/src/deploy_fence.py`), threaded explicitly through the consumer, the DevOps subgraph state (`deploy_fence`) and the result handlers. Every deploy write calls `DeployFence.ensure_held()` — a Lua GET-compare — immediately before it is made: secret persistence, GitHub secret writes (each PUT attempt) and the deploy-secrets receipt (`docs/SECRETS.md`), the pin tag, stopping older runs, workflow dispatch and rerun, the deployment record, the post-deploy product writes (owner grant, settings seed), remote execution, and the deploy's Run and application state. Two rules govern the checks: nothing is awaited between a check and its effect (an API claim, read or wait in between gets its own check after it, so the GitHub dispatch and rerun check again after the dispatch claim), and a Run's terminal outcome is written last (a lifecycle action moves the application's status before it records its Run). A deploy whose lock expired or was replaced raises `DeployFenceLost`, makes no further write, and records its own Run once as `DeployOutcome.DEPLOY_LOCK_LOST` — never over a Run that already holds a terminal outcome, which the API refuses to rewrite; the supervisor redeploys the story under the retry bound. Release is a compare-and-delete in `finally`, so a deploy never deletes a lock another deploy now holds. The check narrows the window to one Redis round trip; it cannot stop a write already in flight when the key expires. ### Stale Worker Cleanup `_check_project_lock()` in the engineering consumer verifies `worker:status` in Redis. Workers in terminal states (`DEAD`/`FAILED`/`STOPPED`) get their Redis keys cleaned up automatically, unblocking new task dispatch without manual intervention. diff --git a/docs/SECRETS.md b/docs/SECRETS.md index f50e066ba..0f09ba4d8 100644 --- a/docs/SECRETS.md +++ b/docs/SECRETS.md @@ -273,9 +273,10 @@ Application deployment is fully delegated to GitHub Actions. This allows secure 2. **Secret Injection** (three stages): * **Scaffolder**: Sets `REGISTRY_URL`, `REGISTRY_USER`, `REGISTRY_PASSWORD` immediately after repo creation (before first CI push) * **scheduler-pipeline**: The PR poller rewrites the same three immediately before every merge, and on every tick a PR is still armed for GitHub auto-merge from an earlier release; it is the only automated merger (no GitHub auto-merge), so push-main CI never starts on stale or absent registry secrets; a failed write blocks the merge - * **DeployerNode**: Sets 9 secrets total — `DOTENV`, `DEPLOY_HOST`, `DEPLOY_USER`, `DEPLOY_SSH_KEY`, `DEPLOY_PORT`, `PROJECT_NAME`, `REGISTRY_URL`, `REGISTRY_USER`, `REGISTRY_PASSWORD` -3. **CI workflow** (`ci.yml`, on push): lint → test → build images → push to self-hosted Docker registry -4. **Deploy workflow** (`deploy.yml`, on `workflow_dispatch` from Orchestrator): SCP compose files → write `.env` from DOTENV → pull images → `docker compose up` + * **DeployerNode** (`subgraphs/devops/deploy_secrets.py`): every deploy needs 9 secrets set before it dispatches `deploy.yml`. It writes the three `REGISTRY_*` on every deploy, and is the only writer of the other six: `DOTENV`, `DEPLOY_HOST`, `DEPLOY_USER`, `DEPLOY_SSH_KEY`, `DEPLOY_PORT`, `PROJECT_NAME`. Those six are skipped, with their public-key reads, while the deploy-secrets receipt vouches for them (below) +3. **Deploy-secrets receipt**: Redis key `deploy::github-secrets` holds one SHA-256 over the GitHub repository id (one `get_repo` read per deploy) and the six resolved names and values in canonical order; never a value, key or per-value digest. A deploy whose digest matches writes only `REGISTRY_*`. Any other receipt (absent, corrupt, another digest, a recreated repository) is a miss: the deploy deletes the receipt, writes all nine, and sets the new digest only after all nine succeeded. Reading, deleting and setting it are Lua scripts that first compare the project deploy lock with this deploy's token, and every secret PUT attempt checks the lock, so a deploy that lost the lock fails `deploy_lock_lost` and certifies nothing. A partial or interrupted write leaves no receipt, so the next deploy writes all nine again; a Redis or repository-read failure refuses the deploy rather than counting as a hit. Losing the receipt costs one full write. The digest attests what the orchestrator last wrote, not what GitHub holds: after a managed secret is edited or deleted outside the orchestrator, delete the receipt before the next deploy with `docker compose exec -T redis redis-cli DEL deploy::github-secrets`. As with every deploy write, a PUT already in flight when the lock expires is not revoked. +4. **CI workflow** (`ci.yml`, on push): lint → test → build images → push to self-hosted Docker registry +5. **Deploy workflow** (`deploy.yml`, on `workflow_dispatch` from Orchestrator): SCP compose files → write `.env` from DOTENV → pull images → `docker compose up` **Privilege Separation:** * **Infra Service**: Can create/destroy servers (Root access via Ansible). Cannot see App Secrets. diff --git a/services/langgraph/src/deploy_fence.py b/services/langgraph/src/deploy_fence.py index 57fa485b1..d398557a2 100644 --- a/services/langgraph/src/deploy_fence.py +++ b/services/langgraph/src/deploy_fence.py @@ -121,13 +121,17 @@ async def ensure_held(self, write: DeployWrite) -> None: """Refuse `write` unless the lock still stores this deploy's token.""" held = await self.redis.eval(_HOLDS_DEPLOY_LOCK, 1, self.lock_key, self.token) if held != 1: - logger.warning( - "deploy_fence_lost", - project_id=self.project_id, - lock_key=self.lock_key, - refused_write=write.value, - ) - raise DeployFenceLost(self.project_id, write) + raise self.lost(write) + + def lost(self, write: DeployWrite) -> DeployFenceLost: + """The refusal of `write`, logged, for a check that found another holder.""" + logger.warning( + "deploy_fence_lost", + project_id=self.project_id, + lock_key=self.lock_key, + refused_write=write.value, + ) + return DeployFenceLost(self.project_id, write) async def release(self) -> None: """Delete the lock only while it is still this deploy's.""" diff --git a/services/langgraph/src/subgraphs/devops/deploy_secrets.py b/services/langgraph/src/subgraphs/devops/deploy_secrets.py new file mode 100644 index 000000000..00e0015f5 --- /dev/null +++ b/services/langgraph/src/subgraphs/devops/deploy_secrets.py @@ -0,0 +1,134 @@ +"""The GitHub Actions repository secrets a deploy needs, and the receipt that skips unchanged ones. + +The deployer is the only writer of six secrets (`DOTENV`, `DEPLOY_HOST`, +`DEPLOY_USER`, `DEPLOY_SSH_KEY`, `DEPLOY_PORT`, `PROJECT_NAME`). Redis key +`deploy::github-secrets` holds the SHA-256 of the last complete +successful write of all six, bound to the GitHub repository id, so a redeploy +with the same values to the same repository writes only the registry secrets. +The receipt is read, deleted and set only inside a Lua script that first +compares the project deploy lock with this deploy's token. +""" + +from collections.abc import Iterable +from functools import partial +import hashlib +import json + +import structlog + +from shared.clients.github import ( + GitHubAppClient, + RegistrySecretsNotRefreshedError, + registry_repository_secrets, +) +from shared.diagnostics import redact_diagnostic + +from ...deploy_fence import DeployFence, DeployFenceLost, DeployWrite + +logger = structlog.get_logger() + +_NOT_HELD = -1 + +# A hit leaves the receipt; anything else is deleted before the write starts, so +# an interrupted write can never leave a receipt for values it replaced. +_CLAIM_RECEIPT = """ +if redis.call('GET', KEYS[1]) ~= ARGV[1] then + return -1 +end +if redis.call('GET', KEYS[2]) == ARGV[2] then + return 1 +end +redis.call('DEL', KEYS[2]) +return 0 +""" + +_PUBLISH_RECEIPT = """ +if redis.call('GET', KEYS[1]) ~= ARGV[1] then + return -1 +end +redis.call('SET', KEYS[2], ARGV[2]) +return 1 +""" + + +def receipt_key(project_id: str) -> str: + return f"deploy:{project_id}:github-secrets" + + +def payload_digest(repository_id: int, secrets: dict[str, str]) -> str: + """SHA-256 of the repository id and every resolved secret name and value, order-free.""" + canonical = json.dumps( + {"repository_id": repository_id, "secrets": secrets}, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest() + + +async def _fenced_receipt(fence: DeployFence, script: str, digest: str) -> int: + result = await fence.redis.eval( + script, 2, fence.lock_key, receipt_key(fence.project_id), fence.token, digest + ) + if result == _NOT_HELD: + raise fence.lost(DeployWrite.GITHUB_SECRETS) + return result + + +async def write_deploy_secrets( + github: GitHubAppClient, + owner: str, + repo: str, + deployer_secrets: dict[str, str], + *, + deploy_fence: DeployFence, + diagnostic_secrets: Iterable[str] = (), +) -> bool: + """Make the deployer's six secrets and the three registry secrets current in `owner/repo`. + + True only when all nine are known set: written now, or the six certified by a + matching receipt. A lost deploy lock raises `DeployFenceLost`; every other + failure, Redis and the repository read included, is False and publishes nothing. + """ + # The scaffolder and the PR poller write the registry secrets too, so no + # deploy-only receipt can vouch for them; rewriting them on every deploy keeps + # an administrative or capability redeploy's repository current as well. + try: + registry_secrets = registry_repository_secrets() + except RegistrySecretsNotRefreshedError as error: + logger.error("registry_env_missing", reason=error.reason.value, detail=error.detail) + return False + + guard = partial(deploy_fence.ensure_held, DeployWrite.GITHUB_SECRETS) + try: + # A repository deleted and recreated under the same name gets a new id. + repository = await github.get_repo(owner, repo) + await guard() + digest = payload_digest(repository.id, deployer_secrets) + cached = await _fenced_receipt(deploy_fence, _CLAIM_RECEIPT, digest) == 1 + pending = registry_secrets if cached else {**deployer_secrets, **registry_secrets} + written = await github.set_repository_secrets(owner, repo, pending, guard=guard) + logger.info( + "deploy_secrets_configured", + owner=owner, + repo=repo, + secrets_count=written, + cached_count=len(deployer_secrets) if cached else 0, + total=len(deployer_secrets) + len(registry_secrets), + ) + if written != len(pending): + return False + if not cached: + await _fenced_receipt(deploy_fence, _PUBLISH_RECEIPT, digest) + return True + except DeployFenceLost: + raise + except Exception as e: + logger.error( + "deploy_secrets_setup_failed", + owner=owner, + repo=repo, + error=redact_diagnostic(e, secrets=diagnostic_secrets), + error_type=type(e).__name__, + ) + return False diff --git a/services/langgraph/src/subgraphs/devops/deployer.py b/services/langgraph/src/subgraphs/devops/deployer.py index a848dcf04..14fa7f5d5 100644 --- a/services/langgraph/src/subgraphs/devops/deployer.py +++ b/services/langgraph/src/subgraphs/devops/deployer.py @@ -8,13 +8,7 @@ from langchain_core.messages import AIMessage import structlog -from shared.clients.github import ( - GitHubAppClient, - RegistrySecretsNotRefreshedError, - RequestGuard, - deploy_pin_tag, - registry_repository_secrets, -) +from shared.clients.github import GitHubAppClient, RequestGuard, deploy_pin_tag from shared.clients.registry import RegistryError from shared.contracts.dto.application import ApplicationStatus from shared.contracts.dto.deploy_dispatch import DeployDispatchClaim @@ -26,6 +20,7 @@ from ...deploy_fence import DeployFence, DeployFenceLost, DeployWrite from ...nodes.base import FunctionalNode from ...runtime_identity import project_spec_runtime_slug +from .deploy_secrets import write_deploy_secrets from .deploy_workflow import require_backend_workflow from .dotenv_builder import build_dotenv, encode_dotenv from .image_gate import ImagesNotPublishedError, image_references, verify_published_images @@ -294,62 +289,6 @@ def _encode_deploy_payload( return dotenv_b64 -async def _write_deploy_secrets( # noqa: PLR0913 - github_client: GitHubAppClient, - owner: str, - repo: str, - server_ip: str, - port: int, - project_name: str, - dotenv_b64: str, - ssh_key: str, - ssh_user: str, - *, - deploy_fence: DeployFence, - diagnostic_secrets: Iterable[str] = (), -) -> bool: - """Write deployment secrets to GitHub repository for deploy.yml workflow.""" - # Registry credentials for CI docker push. The PR poller already wrote them - # before the merge that started this commit's build; rewriting them here keeps - # an administrative or capability redeploy's repository current as well. - try: - registry_secrets = registry_repository_secrets() - except RegistrySecretsNotRefreshedError as error: - logger.error("registry_env_missing", reason=error.reason.value, detail=error.detail) - return False - - secrets_map = { - "DOTENV": dotenv_b64, - "DEPLOY_HOST": server_ip, - "DEPLOY_USER": ssh_user, - "DEPLOY_SSH_KEY": ssh_key, - "DEPLOY_PORT": str(port), - "PROJECT_NAME": project_name, - **registry_secrets, - } - - await deploy_fence.ensure_held(DeployWrite.GITHUB_SECRETS) - try: - count = await github_client.set_repository_secrets(owner, repo, secrets_map) - logger.info( - "deploy_secrets_configured", - owner=owner, - repo=repo, - secrets_count=count, - total=len(secrets_map), - ) - return count == len(secrets_map) - except Exception as e: - logger.error( - "deploy_secrets_setup_failed", - owner=owner, - repo=repo, - error=redact_diagnostic(e, secrets=diagnostic_secrets), - error_type=type(e).__name__, - ) - return False - - class DeployerNode(FunctionalNode): """Deploy via GitHub Actions: write secrets, dispatch deploy.yml, wait for completion.""" @@ -768,16 +707,18 @@ async def _deploy(self, state: DevOpsState) -> dict: # noqa: PLR0911 repo=repo, dotenv_len=len(dotenv_b64), ) - secrets_ok = await _write_deploy_secrets( - github_client=github, - owner=owner, - repo=repo, - server_ip=server_ip, - port=port, - project_name=project_name, - dotenv_b64=dotenv_b64, - ssh_key=ssh_key, - ssh_user=server.ssh_user, + secrets_ok = await write_deploy_secrets( + github, + owner, + repo, + { + "DOTENV": dotenv_b64, + "DEPLOY_HOST": server_ip, + "DEPLOY_USER": server.ssh_user, + "DEPLOY_SSH_KEY": ssh_key, + "DEPLOY_PORT": str(port), + "PROJECT_NAME": project_name, + }, deploy_fence=deploy_fence, diagnostic_secrets=diagnostic_secrets, ) diff --git a/services/langgraph/tests/service/_public_deploy_transport.py b/services/langgraph/tests/service/_public_deploy_transport.py index 9e51a586c..848acd8a2 100644 --- a/services/langgraph/tests/service/_public_deploy_transport.py +++ b/services/langgraph/tests/service/_public_deploy_transport.py @@ -72,6 +72,17 @@ def respond(self, request): ) assert request.method == "GET" assert request.headers["Authorization"] == "token content-fixture-token" + if path == "/repos/fixture/public": + return httpx.Response( + 200, + json={ + "id": 1439, + "name": "public", + "full_name": "fixture/public", + "html_url": "https://github.com/fixture/public", + "clone_url": "https://github.com/fixture/public.git", + }, + ) assert request.headers["Accept"] == "application/vnd.github.raw+json" assert dict(request.url.params) == {"ref": self.built} assert path in { diff --git a/services/langgraph/tests/service/test_deploy_secrets_receipt.py b/services/langgraph/tests/service/test_deploy_secrets_receipt.py new file mode 100644 index 000000000..5c9746f9a --- /dev/null +++ b/services/langgraph/tests/service/test_deploy_secrets_receipt.py @@ -0,0 +1,90 @@ +"""The deploy secrets receipt and the deploy lock as real Redis Lua, across deploy jobs.""" + +import os +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch +import uuid + +import pytest + +from shared.redis import RedisStreamClient +from src.deploy_fence import DeployFence, DeployFenceLost +from src.subgraphs.devops.deploy_secrets import receipt_key, write_deploy_secrets + +REGISTRY_ENV = { + "ORCHESTRATOR_HOSTNAME": "registry.test", + "REGISTRY_USER": "fixture", + "REGISTRY_PASSWORD": "fixture-password", # noqa: S105 +} +REGISTRY_ENV_NAMES = ["REGISTRY_URL", "REGISTRY_USER", "REGISTRY_PASSWORD"] +VALUES = { + "DOTENV": "VE9LRU49c2VydmljZQo=", + "DEPLOY_HOST": "192.0.2.20", + "DEPLOY_USER": "deploy", + "DEPLOY_SSH_KEY": "synthetic-service-key", + "DEPLOY_PORT": "8080", + "PROJECT_NAME": "receipt-service", +} + + +@pytest.fixture +async def redis(): + stream = RedisStreamClient() + await stream.connect() + project_id = f"receipt-{uuid.uuid4().hex}" + with patch.dict(os.environ, REGISTRY_ENV): + yield stream.redis, project_id + await stream.redis.delete(f"deploy:{project_id}:lock", receipt_key(project_id)) + await stream.close() + + +def _github(written: list[list[str]], on_write=None) -> AsyncMock: + async def write(owner, repo, values, *, guard): + written.append(list(values)) + for _ in values: + await guard() + if on_write is not None: + await on_write() + return len(values) + + github = AsyncMock() + github.get_repo = AsyncMock(return_value=SimpleNamespace(id=1713)) + github.set_repository_secrets = AsyncMock(side_effect=write) + return github + + +async def _deploy(redis, project_id, task_id, values, github) -> bool: + fence = DeployFence.for_job(redis, project_id, task_id) + assert await fence.acquire(60) + try: + return await write_deploy_secrets(github, "fixture", "receipt", values, deploy_fence=fence) + finally: + await fence.release() + + +async def test_a_second_deploy_job_skips_the_six_certified_secrets(redis): + redis, project_id = redis + written: list[list[str]] = [] + + assert await _deploy(redis, project_id, "deploy-a", VALUES, _github(written)) + assert await _deploy(redis, project_id, "deploy-b", VALUES, _github(written)) + + assert written == [[*VALUES, *REGISTRY_ENV_NAMES], REGISTRY_ENV_NAMES] + + +async def test_a_holder_that_lost_the_lock_cannot_certify_its_payload(redis): + redis, project_id = redis + written: list[list[str]] = [] + assert await _deploy(redis, project_id, "deploy-a", VALUES, _github(written)) + changed = {**VALUES, "DOTENV": "VE9LRU49cm90YXRlZAo="} + + async def hand_over(): + await redis.set(f"deploy:{project_id}:lock", "deploy-c:other", ex=60) + + with pytest.raises(DeployFenceLost): + await _deploy(redis, project_id, "deploy-b", changed, _github(written, hand_over)) + + assert await redis.get(receipt_key(project_id)) is None + await redis.delete(f"deploy:{project_id}:lock") + assert await _deploy(redis, project_id, "deploy-d", changed, _github(written)) + assert written[-1] == [*VALUES, *REGISTRY_ENV_NAMES] diff --git a/services/langgraph/tests/unit/consumers/test_deploy_fence.py b/services/langgraph/tests/unit/consumers/test_deploy_fence.py index fe18d431c..64e4e610a 100644 --- a/services/langgraph/tests/unit/consumers/test_deploy_fence.py +++ b/services/langgraph/tests/unit/consumers/test_deploy_fence.py @@ -17,6 +17,7 @@ from contextlib import ExitStack from dataclasses import dataclass from datetime import UTC, datetime, timedelta +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import fakeredis @@ -225,7 +226,10 @@ async def patch_api(path: str, json: dict): lifecycle_api.get_server_ssh_key = AsyncMock(return_value="ssh-key") github = AsyncMock() - github.set_repository_secrets = AsyncMock(side_effect=lambda owner, repo, values: len(values)) + github.get_repo = AsyncMock(return_value=SimpleNamespace(id=1001)) + github.set_repository_secrets = AsyncMock( + side_effect=lambda owner, repo, values, **_: len(values) + ) github.fence_workflow = AsyncMock(return_value=[]) github.wait_for_workflow_completion = AsyncMock( return_value={ diff --git a/services/langgraph/tests/unit/test_deploy_secrets.py b/services/langgraph/tests/unit/test_deploy_secrets.py new file mode 100644 index 000000000..33fc027db --- /dev/null +++ b/services/langgraph/tests/unit/test_deploy_secrets.py @@ -0,0 +1,339 @@ +"""A deploy's repository secrets: written when they change, skipped while a receipt vouches. + +The GitHub client is the real one over controlled HTTP, sealing each value with +the repository key; the receipt and the deploy lock are the real Lua scripts on +an in-memory Redis shared by every deploy of the test. +""" + +import base64 +from datetime import UTC, datetime, timedelta +import json +import os +import re +from unittest.mock import AsyncMock, patch + +import fakeredis +import httpx +from nacl import public +import pytest +from structlog.testing import capture_logs + +from shared.clients.github import GitHubAppClient +from src.deploy_fence import DeployFence, DeployFenceLost +from src.subgraphs.devops.deploy_secrets import receipt_key, write_deploy_secrets + +OWNER, REPO = "my-org", "reminder-bot" +PROJECT_ID = "proj-receipt" +OTHER_HOLDER = "deploy-b:4f1c" +REGISTRY_ENV = { + "ORCHESTRATOR_HOSTNAME": "registry.example.com", + "REGISTRY_USER": "registry-user", + "REGISTRY_PASSWORD": "registry-password-value", # noqa: S105 +} +REGISTRY_SECRETS = { + "REGISTRY_URL": "registry.example.com", + "REGISTRY_USER": "registry-user", + "REGISTRY_PASSWORD": "registry-password-value", +} +REGISTRY_NAMES = list(REGISTRY_SECRETS) +DOTENV = base64.b64encode(b"BACKEND_IMAGE=registry.example.com/bot:sha-b\nTOKEN=canary\n").decode() +VALUES = { + "DOTENV": DOTENV, + "DEPLOY_HOST": "192.0.2.10", + "DEPLOY_USER": "deploy", + "DEPLOY_SSH_KEY": "-----BEGIN OPENSSH PRIVATE KEY-----\nsynthetic\n-----END-----", + "DEPLOY_PORT": "8080", + "PROJECT_NAME": "reminder-bot-0000", +} +CHANGED = { + "DOTENV": base64.b64encode(b"TOKEN=rotated\n").decode(), + "DEPLOY_HOST": "192.0.2.11", + "DEPLOY_USER": "ops", + "DEPLOY_SSH_KEY": "-----BEGIN OPENSSH PRIVATE KEY-----\nrotated\n-----END-----", + "DEPLOY_PORT": "8081", + "PROJECT_NAME": "reminder-bot-0001", +} + + +class Repository: + """The GitHub repository `OWNER/REPO` as the Actions secrets API exposes it.""" + + def __init__(self, server: fakeredis.FakeServer) -> None: + self.server = server + self.redis = fakeredis.FakeRedis(server=server, decode_responses=True) + self.id = 1001 + self._key = public.PrivateKey.generate() + self.secrets: dict[str, str] = {} + self.requests: list[tuple[str, str]] = [] + self.refuse: set[str] = set() + self.hand_over_on: tuple[str, str] | None = None + self.answer_first_put_with: int | None = None + self.unreachable = False + # One pool for every client: building an httpx client loads the TLS trust store. + self.http = httpx.AsyncClient(transport=httpx.MockTransport(self.handle)) + + def hand_over(self) -> None: + """The deploy's lock expires and another deploy claims the project.""" + self.redis.set(f"deploy:{PROJECT_ID}:lock", OTHER_HOLDER) + + def handle(self, request: httpx.Request) -> httpx.Response: + path = request.url.path.removeprefix(f"/repos/{OWNER}/{REPO}") + self.requests.append((request.method, path)) + if (request.method, path) == self.hand_over_on: + self.hand_over() + if path == "" and self.unreachable: + raise httpx.ConnectError(f"refused while sending {DOTENV}", request=request) + if path == "": + return httpx.Response( + 200, + json={ + "id": self.id, + "name": REPO, + "full_name": f"{OWNER}/{REPO}", + "html_url": f"https://github.com/{OWNER}/{REPO}", + "clone_url": f"https://github.com/{OWNER}/{REPO}.git", + }, + ) + if path == "/actions/secrets/public-key": + key = base64.b64encode(bytes(self._key.public_key)).decode() + return httpx.Response(200, json={"key": key, "key_id": "k1"}) + name = path.removeprefix("/actions/secrets/") + if self.answer_first_put_with is not None: + status, self.answer_first_put_with = self.answer_first_put_with, None + return httpx.Response(status) + if name in self.refuse: + return httpx.Response(422, json={"message": "Unprocessable"}) + sealed = base64.b64decode(json.loads(request.content)["encrypted_value"]) + self.secrets[name] = public.SealedBox(self._key).decrypt(sealed).decode() + return httpx.Response(204) + + def written(self) -> list[str]: + return [path.rsplit("/", 1)[1] for method, path in self.requests if method == "PUT"] + + def key_reads(self) -> int: + return self.requests.count(("GET", "/actions/secrets/public-key")) + + +@pytest.fixture +def server(): + return fakeredis.FakeServer() + + +@pytest.fixture +async def repository(server): + repository = Repository(server) + with ( + patch.dict(os.environ, REGISTRY_ENV), + patch("shared.clients.github._base.asyncio.sleep", new_callable=AsyncMock), + ): + yield repository + await repository.http.aclose() + + +def _client(repository: Repository) -> GitHubAppClient: + """A separate client per deploy, as each deploy job constructs its own.""" + with patch.dict(os.environ, {"GITHUB_APP_ID": "12345"}): + github = GitHubAppClient() + github._http_client = repository.http + github._token_cache[111] = ("token", datetime.now(UTC) + timedelta(hours=1)) + github.get_installation_id = AsyncMock(return_value=111) + return github + + +def _deploy(server: fakeredis.FakeServer, task_id: str) -> DeployFence: + """A new deploy job of the project, holding its lock.""" + fence = DeployFence.for_job( + fakeredis.aioredis.FakeRedis(server=server, decode_responses=True), PROJECT_ID, task_id + ) + fakeredis.FakeRedis(server=server, decode_responses=True).set(fence.lock_key, fence.token) + return fence + + +async def _write(repository, values, task_id="deploy-a", diagnostic_secrets=()) -> bool: + return await write_deploy_secrets( + _client(repository), + OWNER, + REPO, + values, + deploy_fence=_deploy(repository.server, task_id), + diagnostic_secrets=diagnostic_secrets, + ) + + +def _receipt(repository: Repository) -> str | None: + return repository.redis.get(receipt_key(PROJECT_ID)) + + +@pytest.mark.asyncio +async def test_a_redeploy_from_another_job_writes_only_the_registry_secrets(repository): + assert await _write(repository, VALUES, "deploy-a") + assert repository.written() == [*VALUES, *REGISTRY_NAMES] + assert repository.secrets == {**VALUES, **REGISTRY_SECRETS} + repository.requests.clear() + + with capture_logs() as logs: + assert await _write(repository, VALUES, "deploy-b") + + assert repository.written() == REGISTRY_NAMES + assert repository.key_reads() == len(REGISTRY_NAMES) + configured = [entry for entry in logs if entry["event"] == "deploy_secrets_configured"] + assert [(e["secrets_count"], e["cached_count"], e["total"]) for e in configured] == [(3, 6, 9)] + + +@pytest.mark.asyncio +async def test_the_receipt_is_one_digest_per_project_holding_no_value(repository): + assert await _write(repository, VALUES, "deploy-a") + assert await _write(repository, CHANGED, "deploy-b") + + keys = set(repository.redis.keys("*")) + assert keys == {f"deploy:{PROJECT_ID}:lock", receipt_key(PROJECT_ID)} + assert re.fullmatch(r"[0-9a-f]{64}", _receipt(repository)) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("name", list(VALUES)) +async def test_any_changed_value_writes_all_six_again(repository, name): + assert await _write(repository, VALUES, "deploy-a") + repository.requests.clear() + + assert await _write(repository, {**VALUES, name: CHANGED[name]}, "deploy-b") + + assert repository.written() == [*VALUES, *REGISTRY_NAMES] + assert repository.secrets[name] == CHANGED[name] + + +@pytest.mark.asyncio +async def test_the_same_values_in_another_order_are_the_same_payload(repository): + assert await _write(repository, VALUES, "deploy-a") + repository.requests.clear() + + assert await _write(repository, dict(reversed(VALUES.items())), "deploy-b") + + assert repository.written() == REGISTRY_NAMES + + +@pytest.mark.asyncio +async def test_a_repository_recreated_under_the_same_name_is_written_again(repository): + assert await _write(repository, VALUES, "deploy-a") + repository.id = 2002 + repository.secrets.clear() + repository.requests.clear() + + assert await _write(repository, VALUES, "deploy-b") + + assert repository.secrets == {**VALUES, **REGISTRY_SECRETS} + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "receipt", [None, "corrupt", "0" * 64], ids=["deleted", "corrupt", "other"] +) +async def test_a_missing_or_foreign_receipt_writes_and_repairs(repository, receipt): + assert await _write(repository, VALUES, "deploy-a") + repository.redis.delete(receipt_key(PROJECT_ID)) + if receipt is not None: + repository.redis.set(receipt_key(PROJECT_ID), receipt) + repository.requests.clear() + + assert await _write(repository, VALUES, "deploy-b") + assert repository.written() == [*VALUES, *REGISTRY_NAMES] + repository.requests.clear() + assert await _write(repository, VALUES, "deploy-c") + assert repository.written() == REGISTRY_NAMES + + +@pytest.mark.asyncio +async def test_a_partial_write_of_b_leaves_no_receipt_and_a_is_repaired(repository): + assert await _write(repository, VALUES, "deploy-a") + repository.refuse = {"DEPLOY_SSH_KEY"} + + assert not await _write(repository, CHANGED, "deploy-b") + + assert repository.secrets["DOTENV"] == CHANGED["DOTENV"] + assert _receipt(repository) is None + repository.refuse = set() + repository.requests.clear() + assert await _write(repository, VALUES, "deploy-c") + assert repository.written() == [*VALUES, *REGISTRY_NAMES] + assert {name: repository.secrets[name] for name in VALUES} == VALUES + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("values", "request_", "receipt_kept"), + [ + (CHANGED, ("GET", ""), True), + (CHANGED, ("GET", "/actions/secrets/public-key"), False), + (VALUES, ("GET", ""), True), + (VALUES, ("GET", "/actions/secrets/public-key"), True), + ], + ids=["identity-read", "key-read", "certified-identity-read", "certified-key-read"], +) +async def test_a_lock_lost_during_a_read_sends_no_secret( + repository, values, request_, receipt_kept +): + assert await _write(repository, VALUES, "deploy-a") + receipt = _receipt(repository) + repository.requests.clear() + repository.hand_over_on = request_ + + with pytest.raises(DeployFenceLost): + await _write(repository, values, "deploy-b") + + assert repository.written() == [] + assert _receipt(repository) == (receipt if receipt_kept else None) + assert repository.redis.get(f"deploy:{PROJECT_ID}:lock") == OTHER_HOLDER + + +@pytest.mark.asyncio +async def test_a_lock_lost_before_a_retry_stops_the_batch(repository): + repository.answer_first_put_with = 502 + repository.hand_over_on = ("PUT", "/actions/secrets/DOTENV") + + with pytest.raises(DeployFenceLost): + await _write(repository, VALUES, "deploy-a") + + assert repository.written() == ["DOTENV"] + assert _receipt(repository) is None + + +@pytest.mark.asyncio +async def test_a_lock_lost_after_the_last_write_publishes_no_receipt(repository): + repository.hand_over_on = ("PUT", "/actions/secrets/REGISTRY_PASSWORD") + + with pytest.raises(DeployFenceLost): + await _write(repository, VALUES, "deploy-a") + + assert repository.written() == [*VALUES, *REGISTRY_NAMES] + assert _receipt(repository) is None + repository.hand_over_on = None + repository.requests.clear() + assert await _write(repository, VALUES, "deploy-b") + assert repository.written() == [*VALUES, *REGISTRY_NAMES] + + +@pytest.mark.asyncio +async def test_an_unreadable_receipt_store_is_no_hit_and_no_setup(server, repository): + assert await _write(repository, VALUES, "deploy-a") + fence = _deploy(server, "deploy-b") + repository.requests.clear() + server.connected = False + + assert not await write_deploy_secrets( + _client(repository), OWNER, REPO, VALUES, deploy_fence=fence, diagnostic_secrets=[DOTENV] + ) + + assert repository.written() == [] + + +@pytest.mark.asyncio +async def test_a_failed_identity_read_writes_nothing_and_logs_no_value(repository): + repository.unreachable = True + + with capture_logs() as logs: + assert not await _write(repository, VALUES, "deploy-a", diagnostic_secrets=[DOTENV]) + + assert repository.written() == [] + assert _receipt(repository) is None + assert [entry["event"] for entry in logs] == ["deploy_secrets_setup_failed"] + assert DOTENV not in repr(logs) diff --git a/services/langgraph/tests/unit/test_deployer.py b/services/langgraph/tests/unit/test_deployer.py index 70f0ce715..50ba43a00 100644 --- a/services/langgraph/tests/unit/test_deployer.py +++ b/services/langgraph/tests/unit/test_deployer.py @@ -5,6 +5,7 @@ from datetime import UTC, datetime, timedelta from ipaddress import ip_address import os +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import httpx @@ -17,6 +18,7 @@ from shared.contracts.dto.deploy_dispatch import DeployDispatchClaim from shared.contracts.dto.run import RunStatus from shared.contracts.queues.deploy import DeployOutcome +from src.deploy_fence import DeployFence from src.subgraphs.devops.deployer import DeployerNode, _platform_services from src.subgraphs.devops.image_gate import ImagesNotPublishedError from tests.unit.deploy_fences import held_deploy_fence @@ -120,6 +122,7 @@ def _dotenv(repository_secrets: dict[str, str]) -> dict[str, str]: def _setup_happy_mocks(mock_api, mock_gh_cls): gh = AsyncMock() mock_gh_cls.return_value = gh + gh.get_repo.return_value = SimpleNamespace(id=1001) gh.set_repository_secrets.return_value = 9 gh.wait_for_workflow_completion.return_value = _SUCCESS_RUN gh.get_file_contents.return_value = ( @@ -187,7 +190,7 @@ def health(request): requests.append(request) return httpx.Response(200, json={"status": "ok"}) - async def write(owner, repo, values): + async def write(owner, repo, values, **_): written.update(values) return len(values) @@ -378,6 +381,31 @@ async def test_partial_secret_write_refuses_before_fence_or_dispatch( gh.trigger_workflow_dispatch.assert_not_awaited() mock_api.create_deployment.assert_not_awaited() + @pytest.mark.asyncio + @patch.dict( + os.environ, + { + "ORCHESTRATOR_HOSTNAME": "registry.example.com", + "REGISTRY_USER": "registry-user", + "REGISTRY_PASSWORD": "registry-password", + }, + ) + @patch("src.subgraphs.devops.deployer.GitHubAppClient") + @patch("src.subgraphs.devops.deployer.api_client") + async def test_an_unread_repository_identity_refuses_before_any_secret_or_dispatch( + self, mock_api, mock_gh_cls, deployer, base_state + ): + gh = _setup_happy_mocks(mock_api, mock_gh_cls) + gh.get_repo.side_effect = httpx.ConnectError("github unreachable") + + result = await deployer.run(base_state) + + assert result["deployment_result"]["status"] == "failed" + assert "GitHub Actions secrets are incomplete" in result["errors"][0] + gh.set_repository_secrets.assert_not_awaited() + gh.create_or_reset_tag.assert_not_awaited() + gh.trigger_workflow_dispatch.assert_not_awaited() + @pytest.mark.asyncio @patch("src.subgraphs.devops.deployer.logger") @patch("src.subgraphs.devops.deployer.GitHubAppClient") @@ -440,6 +468,37 @@ async def test_writes_deploy_secrets(self, mock_api, mock_gh_cls, deployer, base mock_api.get_server.assert_awaited_once_with("srv-1") mock_api.get_server_ssh_key.assert_awaited_once_with("srv-1") + @pytest.mark.asyncio + @patch("src.subgraphs.devops.deployer.GitHubAppClient") + @patch("src.subgraphs.devops.deployer.api_client") + async def test_a_redeploy_of_unchanged_values_writes_only_the_registry_secrets( + self, mock_api, mock_gh_cls, deployer, base_state + ): + gh = _setup_happy_mocks(mock_api, mock_gh_cls) + gh.set_repository_secrets.side_effect = lambda owner, repo, values, **_: len(values) + first = base_state["deploy_fence"] + await deployer.run(base_state) + second = DeployFence.for_job(first.redis, "proj-123", "deploy-2") + await first.redis.set(first.lock_key, second.token) + + result = await deployer.run({**base_state, "deploy_fence": second}) + + assert result["deployment_result"]["status"] == "success" + registry = ["REGISTRY_URL", "REGISTRY_USER", "REGISTRY_PASSWORD"] + deployer_owned = [ + "DOTENV", + "DEPLOY_HOST", + "DEPLOY_USER", + "DEPLOY_SSH_KEY", + "DEPLOY_PORT", + "PROJECT_NAME", + ] + assert [list(call.args[2]) for call in gh.set_repository_secrets.await_args_list] == [ + [*deployer_owned, *registry], + registry, + ] + assert gh.trigger_workflow_dispatch.await_count == 2 + @pytest.mark.asyncio @patch("src.subgraphs.devops.deployer.GitHubAppClient") @patch("src.subgraphs.devops.deployer.api_client") @@ -625,6 +684,7 @@ async def test_propagates_unproven_workflow_cancellation_without_rerun( async def test_handles_workflow_failure(self, mock_api, mock_gh_cls, deployer, base_state): gh = AsyncMock() mock_gh_cls.return_value = gh + gh.get_repo.return_value = SimpleNamespace(id=1001) gh.set_repository_secrets.return_value = 9 mock_api.get_server_ssh_key = AsyncMock(return_value="ssh-key-content") mock_api.get_server = AsyncMock(return_value=MagicMock(ssh_user="dev")) @@ -647,6 +707,7 @@ async def test_handles_workflow_failure(self, mock_api, mock_gh_cls, deployer, b async def test_handles_timeout(self, mock_api, mock_gh_cls, deployer, base_state): gh = AsyncMock() mock_gh_cls.return_value = gh + gh.get_repo.return_value = SimpleNamespace(id=1001) gh.set_repository_secrets.return_value = 9 mock_api.get_server_ssh_key = AsyncMock(return_value="ssh-key-content") mock_api.get_server = AsyncMock(return_value=MagicMock(ssh_user="dev")) @@ -1021,7 +1082,7 @@ async def test_the_payload_is_replaced_before_earlier_runs_are_stopped( order = [] gh.fence_workflow = AsyncMock(side_effect=lambda *a, **k: order.append("fence") or [7]) gh.set_repository_secrets = AsyncMock( - side_effect=lambda owner, repo, secrets: order.append("secrets") or len(secrets) + side_effect=lambda owner, repo, secrets, **_: order.append("secrets") or len(secrets) ) gh.trigger_workflow_dispatch = AsyncMock( side_effect=lambda *a, **k: order.append("dispatch") or True diff --git a/services/scheduler/tests/unit/test_imported_repository_merge_order.py b/services/scheduler/tests/unit/test_imported_repository_merge_order.py index 3e359bd2a..6b079c7e1 100644 --- a/services/scheduler/tests/unit/test_imported_repository_merge_order.py +++ b/services/scheduler/tests/unit/test_imported_repository_merge_order.py @@ -81,7 +81,7 @@ async def __aexit__(self, *exc: object) -> bool: return False async def set_repository_secret( # noqa: PLR0913 — the real method's signature - self, owner: str, repo: str, secret_name: str, secret_value: str, token=None + self, owner: str, repo: str, secret_name: str, secret_value: str, token=None, *, guard=None ) -> None: if secret_name == self.refused_secret: raise RuntimeError(f"403 Resource not accessible by integration: {secret_value}") diff --git a/shared/clients/github/_secrets.py b/shared/clients/github/_secrets.py index 053a531f1..c0ee1e2cf 100644 --- a/shared/clients/github/_secrets.py +++ b/shared/clients/github/_secrets.py @@ -6,6 +6,8 @@ from shared.diagnostics import redact_diagnostic from shared.log_config import get_logger +from ._base import RequestGuard, RequestGuardRefusedError + logger = get_logger(__name__) #: The repository secrets a generated product's push-main CI logs in to the @@ -61,6 +63,8 @@ async def set_repository_secret( secret_name: str, secret_value: str, token: str | None = None, + *, + guard: RequestGuard | None = None, ) -> None: """Set an encrypted repository secret for GitHub Actions. @@ -73,6 +77,7 @@ async def set_repository_secret( secret_name: Name of the secret (e.g., DEPLOY_HOST) secret_value: Plain text value to encrypt and store token: Optional pre-obtained token (e.g. org-level). Falls back to per-repo lookup. + guard: Awaited immediately before each attempt of the PUT; see `_make_request`. """ # Lazy import: pynacl only needed when this method is called from nacl import public @@ -106,6 +111,7 @@ async def set_repository_secret( "PUT", f"https://api.github.com/repos/{owner}/{repo}/actions/secrets/{secret_name}", headers=headers, + guard=guard, json={ "encrypted_value": encrypted_b64, "key_id": key_id, @@ -125,6 +131,8 @@ async def set_repository_secrets( repo: str, secrets: dict[str, str], token: str | None = None, + *, + guard: RequestGuard | None = None, ) -> int: """Set multiple repository secrets at once. @@ -133,6 +141,8 @@ async def set_repository_secrets( repo: Repository name secrets: Dictionary of secret_name -> secret_value token: Optional pre-obtained token (e.g. org-level). Falls back to per-repo lookup. + guard: Awaited immediately before each PUT attempt. Its own refusal is raised + unchanged and stops the batch, so no later secret is written. Returns: Number of secrets successfully set @@ -140,8 +150,10 @@ async def set_repository_secrets( count = 0 for name, value in secrets.items(): try: - await self.set_repository_secret(owner, repo, name, value, token=token) + await self.set_repository_secret(owner, repo, name, value, token=token, guard=guard) count += 1 + except RequestGuardRefusedError as refused: + raise refused.error from None except Exception as e: logger.error( "github_secret_set_failed", diff --git a/shared/tests/clients/test_github_repository_secrets.py b/shared/tests/clients/test_github_repository_secrets.py new file mode 100644 index 000000000..801b882b1 --- /dev/null +++ b/shared/tests/clients/test_github_repository_secrets.py @@ -0,0 +1,72 @@ +"""Repository secret writes under a caller's write guard.""" + +from base64 import b64encode +from datetime import UTC, datetime, timedelta +import os +from unittest.mock import AsyncMock, patch + +import httpx +from nacl import public +import pytest +from structlog.testing import capture_logs + +from shared.clients.github import GitHubAppClient + +OWNER, REPO = "my-org", "product" +SECRETS = {"FIRST": "first-value", "SECOND": "second-value", "THIRD": "third-value"} + + +class Refused(Exception): + pass + + +@pytest.fixture +async def github(): + key = public.PrivateKey.generate().public_key + puts: list[str] = [] + answers = {"FIRST": [502, 204], "SECOND": [204], "THIRD": [204]} + + def handle(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/public-key"): + return httpx.Response(200, json={"key": b64encode(bytes(key)).decode(), "key_id": "k1"}) + name = request.url.path.rsplit("/", 1)[1] + puts.append(name) + return httpx.Response(answers[name].pop(0)) + + with patch.dict(os.environ, {"GITHUB_APP_ID": "12345"}): + client = GitHubAppClient() + client._token_cache[111] = ("token", datetime.now(UTC) + timedelta(hours=1)) + client.get_installation_id = AsyncMock(return_value=111) + client._http_client = httpx.AsyncClient(transport=httpx.MockTransport(handle)) + with patch("shared.clients.github._base.asyncio.sleep", new_callable=AsyncMock): + yield client, puts + await client._http_client.aclose() + + +@pytest.mark.asyncio +async def test_the_guard_precedes_every_put_attempt_including_retries(github): + github, puts = github + checks: list[int] = [] + + async def guard() -> None: + checks.append(len(puts)) + + assert await github.set_repository_secrets(OWNER, REPO, SECRETS, guard=guard) == 3 + + assert puts == ["FIRST", "FIRST", "SECOND", "THIRD"] + assert checks == [0, 1, 2, 3] + + +@pytest.mark.asyncio +async def test_a_guard_refusal_stops_the_batch_and_is_raised_unchanged(github): + github, puts = github + + async def guard() -> None: + if puts: + raise Refused("lock lost") + + with capture_logs() as logs, pytest.raises(Refused): + await github.set_repository_secrets(OWNER, REPO, SECRETS, guard=guard) + + assert puts == ["FIRST"] + assert not [entry for entry in logs if entry["event"] == "github_secret_set_failed"] diff --git a/tests/live/test_deploy_infra.py b/tests/live/test_deploy_infra.py index 0208f8801..105bd447f 100644 --- a/tests/live/test_deploy_infra.py +++ b/tests/live/test_deploy_infra.py @@ -187,7 +187,7 @@ def test_deploy_workflow_template_exists(self): def test_langgraph_has_registry_env_vars(self, compose_exec): """langgraph container has ORCHESTRATOR_HOSTNAME + REGISTRY_* env vars. - _write_deploy_secrets() reads these to push to GitHub Actions secrets. + write_deploy_secrets() reads these to push to GitHub Actions secrets. """ env_vars = ["ORCHESTRATOR_HOSTNAME", "REGISTRY_USER", "REGISTRY_PASSWORD"] missing = [] @@ -202,7 +202,7 @@ def test_langgraph_has_registry_env_vars(self, compose_exec): assert not missing, ( f"langgraph container missing deploy env vars: {', '.join(missing)}. " - "Deployer node (_write_deploy_secrets) will fail." + "Deployer node (write_deploy_secrets) will fail." ) @pytest.mark.parametrize("var", ["ORCHESTRATOR_HOSTNAME", "REGISTRY_USER", "REGISTRY_PASSWORD"]) From ca51f9dbf13eb035f90d96f6874c47e4613103e5 Mon Sep 17 00:00:00 2001 From: q Date: Mon, 19 Oct 2026 05:47:19 +0000 Subject: [PATCH 2/2] Use a neutral repository id in the deploy-secrets service fixtures --- services/langgraph/tests/service/_public_deploy_transport.py | 2 +- services/langgraph/tests/service/test_deploy_secrets_receipt.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/langgraph/tests/service/_public_deploy_transport.py b/services/langgraph/tests/service/_public_deploy_transport.py index 848acd8a2..f1fda5aa8 100644 --- a/services/langgraph/tests/service/_public_deploy_transport.py +++ b/services/langgraph/tests/service/_public_deploy_transport.py @@ -76,7 +76,7 @@ def respond(self, request): return httpx.Response( 200, json={ - "id": 1439, + "id": 1001, "name": "public", "full_name": "fixture/public", "html_url": "https://github.com/fixture/public", diff --git a/services/langgraph/tests/service/test_deploy_secrets_receipt.py b/services/langgraph/tests/service/test_deploy_secrets_receipt.py index 5c9746f9a..0f0241775 100644 --- a/services/langgraph/tests/service/test_deploy_secrets_receipt.py +++ b/services/langgraph/tests/service/test_deploy_secrets_receipt.py @@ -48,7 +48,7 @@ async def write(owner, repo, values, *, guard): return len(values) github = AsyncMock() - github.get_repo = AsyncMock(return_value=SimpleNamespace(id=1713)) + github.get_repo = AsyncMock(return_value=SimpleNamespace(id=1001)) github.set_repository_secrets = AsyncMock(side_effect=write) return github