Scope build GitHub reviewer credentials to evaluation

backend/druks/agents.py

@@ -353,7 +353,7 @@ async def _run(
             mcp_servers, mcp_secret_refs = (), []
             if self.include_mcp:
                 mcp_servers, mcp_secret_refs = await workspace_class.get_all_mcp_servers(
-                    session, subject, workflow.account_id
+                    session, subject, workflow.account_id, agent=self.id
                 )
             if lent_browser:
                 server, secret = lent_browser

backend/druks/contrib/software_factory/workflows.py

@@ -66,13 +66,16 @@ async def get_secrets(cls, subject: Any) -> list[SandboxSecret]:
         ]
 
     @classmethod
-    async def get_mcp_servers(cls, subject: Any) -> tuple[SandboxMcpServer, ...]:
-        # GitHub MCP acts as the review actor. The clone acts as the operator.
-        actor = await get_review_actor()
+    async def get_mcp_servers(
+        cls, subject: Any, *, agent: str = ""
+    ) -> tuple[SandboxMcpServer, ...]:
+        service = cls.github
+        if agent == SoftwareFactory.evaluate_implementation.id:
+            service = (await get_review_actor()).service
         github = SandboxMcpServer(
             name=GITHUB_MCP_NAME,
             url=GITHUB_MCP_URL,
-            secret_id=(await actor.service.get()).id,
+            secret_id=(await service.get()).id,
             resources=await get_project_repos(cls.get_repo(subject)),
         )
         if (await SoftwareFactory.settings()).tracker == "druks":
@@ -321,6 +324,7 @@ async def _implement_phase(self) -> None:
         await self._create_branch()
         while True:
             await self.implement()
+            await self._check_review_access()
             evaluation = await SoftwareFactory.evaluate_implementation()
             if evaluation.verdict == EvaluationVerdict.FAIL and (
                 self.journal.implementation_revision < self._settings.max_implementation_revisions
@@ -385,6 +389,13 @@ async def implement(self) -> ImplementationOutput:
             raise FatalError(f"implementation needs clarification: {delivery.summary}")
         return delivery
 
+    @step
+    async def _check_review_access(self) -> None:
+        # Drukbox fetches the evaluator's MCP entry on the agent's first call, not at
+        # creation, so a missing reviewer installation would reach only the agent.
+        client = await (await get_review_actor()).service.get_app_client()
+        await client.get_installation_id((await self.subject).repo)
+
     @step
     async def _create_branch(self) -> None:
         # Replay keeps the branch: GitHub holds it, and a second pass would create the
@@ -507,7 +518,9 @@ async def get_secrets(cls, subject: Any) -> list[SandboxSecret]:
         ]
 
     @classmethod
-    async def get_mcp_servers(cls, subject: Any) -> tuple[SandboxMcpServer, ...]:
+    async def get_mcp_servers(
+        cls, subject: Any, *, agent: str = ""
+    ) -> tuple[SandboxMcpServer, ...]:
         actor = await get_review_actor()
         return (
             SandboxMcpServer(

backend/druks/workflows.py

@@ -1007,6 +1007,9 @@ async def _lease_host(
         # only the host-id matters across steps — held-across-steps never fights replay.
         if not self.steps_reuse_sandbox:
             return
+        secrets_hash = hashlib.sha256(
+            json.dumps([config.secrets_hash, sorted(ref.key for ref in refs)]).encode()
+        ).hexdigest()
         # A crashed process left its box behind. Its identity finds it again.
         if not self._host and refs:
             identity = await SandboxIdentity.lookup(
@@ -1018,8 +1021,8 @@ async def _lease_host(
             )
             if identity:
                 self._host = await sandbox_client.reattach(host_id=identity.host_id)
-                self._host_secrets_hash = config.secrets_hash
-        if self._host and self._host_secrets_hash != config.secrets_hash:
+                self._host_secrets_hash = secrets_hash
+        if self._host and self._host_secrets_hash != secrets_hash:
             # Drukbox binds entries at creation.
             await self._reap_run()
         if not self._host:
@@ -1048,7 +1051,7 @@ async def _lease_host(
                 identity=identity,
                 lease_seconds=WORKFLOW_HOST_LEASE_SECONDS,
             )
-            self._host_secrets_hash = config.secrets_hash
+            self._host_secrets_hash = secrets_hash
         return self._host.id
 
     async def _reap_run(self) -> None:

backend/druks/workspaces.py

@@ -62,7 +62,9 @@ def get_env(self) -> dict[str, str]:
         return {}
 
     @classmethod
-    async def get_mcp_servers(cls, subject: Any) -> tuple[SandboxMcpServer, ...]:
+    async def get_mcp_servers(
+        cls, subject: Any, *, agent: str = ""
+    ) -> tuple[SandboxMcpServer, ...]:
         # Override to declare this workspace's servers and the vault row each
         # one issues through. Read before the box exists. Base: none.
         return ()
@@ -214,6 +216,7 @@ async def get_all_mcp_servers(
         account_id: str | None,
         *,
         skip_unauthenticated: bool = False,
+        agent: str = "",
     ) -> tuple[tuple[McpServer, ...], list[SecretRef]]:
         """The MCP servers a box of this workspace reaches, as the harness names
         them, and the secret refs for the box's entries, one per bearer and per
@@ -222,7 +225,7 @@ async def get_all_mcp_servers(
         server that cannot authenticate fails here, before the box, unless each
         account keys or connects it for itself or ``skip_unauthenticated`` leaves
         it out."""
-        workspace_servers = await cls.get_mcp_servers(subject)
+        workspace_servers = await cls.get_mcp_servers(subject, agent=agent)
         workspace_names = {server.name for server in workspace_servers}
         if len(workspace_names) != len(workspace_servers):
             # One config key per name in the emitted harness config — a dupe

backend/tests/software_factory/test_build_workspace.py

@@ -69,7 +69,7 @@ async def base_run_agent(self: Any, **kwargs: Any) -> str:
     assert execs == [["mkdir", "-p", get_related_root("exedev")], ["run_agent"]]
 
 
-async def test_build_workspace_declares_its_github_mcp_as_the_review_actor(druks_db):
+async def test_build_workspace_defaults_to_operator_mcp(druks_db):
     operator = await connect_service(
         "github", identity={"app_id": "1", "slug": "druks-operator"}, secrets={"private_key": "pem"}
     )
@@ -84,6 +84,10 @@ async def test_build_workspace_declares_its_github_mcp_as_the_review_actor(druks
 
     assert github.url == GITHUB_MCP_URL
     assert github.bearer_token_env_var == get_bearer_token_env_var(GITHUB_MCP_NAME)
+    assert ref.key == ("mcp_github_token", operator.id, ("o/main",), "api.githubcopilot.com")
+    _, [ref] = await BuildWorkspace.get_all_mcp_servers(
+        druks_db, subject, None, agent=SoftwareFactory.evaluate_implementation.id
+    )
     assert ref.key == ("mcp_github_token", reviewer.id, ("o/main",), "api.githubcopilot.com")
     [clone] = await BuildWorkspace.get_secrets(subject)
     assert clone == SandboxSecret(

backend/tests/software_factory/test_review.py

@@ -16,9 +16,11 @@
 from druks.contrib.software_factory.github import get_review_actor
 from druks.contrib.software_factory.models import Project, ProjectRepo
 from druks.contrib.software_factory.services import GithubReviewer
-from druks.contrib.software_factory.workflows import PullRequestReview
-from druks.core.apis.github import GITHUB_AUTHORITY
+from druks.contrib.software_factory.workflows import Build, PullRequestReview
+from druks.core.apis.exceptions import GitHubAppNotInstalledError
+from druks.core.apis.github import GITHUB_AUTHORITY, GitHubAppClient
 from druks.core.services import Github
+from druks.durable.engine import configure_engine
 from druks.prompts import render_prompt
 from druks.secrets.datastructures import Audience
 from druks.secrets.models import VaultSecret
@@ -278,3 +280,27 @@ async def _start(cls, **kwargs):
     assert run_id == "run-review"
     [start] = started
     assert (start["account_id"], start["note"]) == (dev.id, "Focus on the upgrade path.")
+
+
+async def test_build_evaluation_needs_the_reviewer_on_the_repo(druks_db, monkeypatch):
+    await _connect_operator()
+    await _connect_reviewer()
+    configure_engine(await druks_db.connection())
+
+    async def run_step(_options, fn):
+        return await fn()
+
+    async def get_installation_id(self, repo):
+        raise GitHubAppNotInstalledError(repo, app=self._slug)
+
+    monkeypatch.setattr("druks.workflows.DBOS.run_step_async", run_step)
+    monkeypatch.setattr(GitHubAppClient, "get_installation_id", get_installation_id)
+    build = Build()
+    build.input = Build._run_input_model()
+    build.subject = SimpleNamespace(repo="acme/app")
+
+    try:
+        with pytest.raises(GitHubAppNotInstalledError, match="druks-reviewer has no access"):
+            await build._check_review_access()
+    finally:
+        configure_engine(None)

backend/tests/test_agents.py

@@ -1340,3 +1340,16 @@ async def test_event_requires_an_artifact(druks_db, tmp_path, monkeypatch, curre
         await DUMMY_AGENT._run(db_session(), workflow_id="wf-9")
 
     assert (await AgentCall.list_for_run(db_session(), "wf-9"))[0].status == "failed"
+
+
+async def test_workspace_mcp_selection_receives_agent_id(
+    druks_db, tmp_path, monkeypatch, current_run
+):
+    sandbox = _patch_runtime(monkeypatch, tmp_path, {"ok": True})
+    _patch_ephemeral(monkeypatch, sandbox)
+    servers = AsyncMock(return_value=())
+    monkeypatch.setattr(Workspace, "get_mcp_servers", servers)
+
+    await DUMMY_AGENT._run(db_session(), workflow_id="wf-9")
+
+    servers.assert_awaited_once_with(None, agent="dummy")

backend/tests/test_mcp_servers.py

@@ -64,7 +64,7 @@ def _declaring(*servers: SandboxMcpServer) -> type[Workspace]:
     # through, as SoftwareFactory does.
     class _Declaring(Workspace):
         @classmethod
-        async def get_mcp_servers(cls, subject) -> tuple[SandboxMcpServer, ...]:
+        async def get_mcp_servers(cls, subject, *, agent="") -> tuple[SandboxMcpServer, ...]:
             return servers
 
     return _Declaring

backend/tests/test_warm_host_rotation.py

@@ -4,11 +4,16 @@
 
 import druks.workflows as sdk
 import pytest
+from conftest import connect_provider, connect_service
 from drukbox_sdk import Secret
 from druks.accounts.models import Account
 from druks.db import db_session
+from druks.harnesses.providers import AnthropicProvider
 from druks.sandbox.constants import WORKFLOW_HOST_LEASE_SECONDS
+from druks.sandbox.models import SandboxIdentity, SecretRef
+from druks.testing import seed_run
 from druks.workflows import Workflow
+from druks_field_notes.workflows import Summarize
 
 
 def _config(secrets: dict[str, Secret], secrets_hash: str = "") -> SimpleNamespace:
@@ -53,10 +58,11 @@ async def provision(
     ) -> _FakeSandbox:
         assert lease_seconds == WORKFLOW_HOST_LEASE_SECONDS
         assert template is None
-        assert identity is None
         self.provisions.append(idempotency_key)
         self.secrets.append(secrets)
         host_id = f"host-{len(self.provisions)}"
+        if identity:
+            await identity.bind(host_id)
         return _FakeSandbox(id=host_id, expires_at=datetime.now(UTC) + self.lease)
 
     async def release(self, *, host_id: str) -> None:
@@ -178,12 +184,6 @@ async def test_no_warm_host_when_reuse_disabled(monkeypatch):
 async def test_a_replay_finds_the_warm_box_through_its_identity(
     monkeypatch: pytest.MonkeyPatch, druks_db
 ) -> None:
-    from conftest import connect_provider
-    from druks.harnesses.providers import AnthropicProvider
-    from druks.sandbox.models import SandboxIdentity, SecretRef
-    from druks.testing import seed_run
-    from druks_field_notes.workflows import Summarize
-
     await seed_run(db_session(), kind=Summarize.kind, run_id="wf-1")
     subscription = await connect_provider(
         AnthropicProvider, {"claudeAiOauth": {"accessToken": "test-token"}}
@@ -208,3 +208,57 @@ async def test_a_replay_finds_the_warm_box_through_its_identity(
 
     assert client.reattached == ["host-crashed"]
     assert client.provisions == []
+
+
+async def test_warm_box_switches_mcp_identity_and_switches_back(monkeypatch, druks_db):
+    await seed_run(db_session(), kind=Summarize.kind, run_id="wf-1")
+    operator = await connect_service(
+        "github",
+        identity={"app_id": "1", "slug": "operator"},
+        secrets={"private_key": "operator-pem"},
+    )
+    reviewer = await connect_service(
+        "github_reviewer",
+        identity={"app_id": "2", "slug": "reviewer"},
+        secrets={"private_key": "reviewer-pem"},
+    )
+    operator_refs = [
+        SecretRef(
+            name="mcp_github_token",
+            secret_id=operator.id,
+            resources=["acme/app"],
+            host="api.githubcopilot.com",
+        )
+    ]
+    reviewer_refs = [
+        SecretRef(
+            name="mcp_github_token",
+            secret_id=reviewer.id,
+            resources=["acme/app"],
+            host="api.githubcopilot.com",
+        )
+    ]
+    fake = _FakeSandboxClient()
+    monkeypatch.setattr(sdk, "sandbox_client", fake)
+    flow = _warm_workflow()
+    flow.account_id = (await Account.get_for_run(db_session(), None)).id
+
+    assert await flow._lease_host(db_session(), _NONE, operator_refs) == "host-1"
+    assert await flow._lease_host(db_session(), _NONE, operator_refs) == "host-1"
+    assert await flow._lease_host(db_session(), _NONE, reviewer_refs) == "host-2"
+    resumed = _warm_workflow()
+    resumed.account_id = flow.account_id
+    assert await resumed._lease_host(db_session(), _NONE, reviewer_refs) == "host-2"
+    assert await resumed._lease_host(db_session(), _NONE, operator_refs) == "host-3"
+
+    assert fake.released == ["host-1", "host-2"]
+    assert fake.reattached == ["host-2"]
+    assert len(set(fake.provisions)) == 3
+    identity = await SandboxIdentity.lookup(
+        db_session(),
+        account_id=flow.account_id,
+        run_id="wf-1",
+        scoped_to="workflow",
+        secret_refs=operator_refs,
+    )
+    assert identity.host_id == "host-3"

docs/configuration.md

@@ -488,9 +488,9 @@ sandbox's repo, with the expiry GitHub gives it, and fetches a new one before
 it expires. The token reaches that repo and the project's other repos that the
 same installation covers, and no other repo. The `software_factory` build
 clones and pushes as the operator App. A review clones as the reviewer App when
-one is connected. Builds and reviews
-also get the GitHub MCP server for `api.githubcopilot.com`. It acts as the
-review identity through a second entry.
+one is connected. Builds and reviews also get the GitHub MCP server for
+`api.githubcopilot.com`. A review and a build's evaluation use it as the review
+identity. The other build steps use it as the operator App.
 
 **To upgrade an existing installation**, paste the credentials one time on each
 active host. Open **Settings → Connections → Services**. Connect GitHub with the
@@ -532,7 +532,8 @@ Connect a second App to let reviews approve pull requests and request changes.
 Open **Settings → Connections → Services → Github Reviewer** and select
 **Create App**. The flow is the same as for the operator App. Install the
 reviewer App on every repository that has the operator App, because builds also
-post their reviews through it.
+post their reviews through it. Without that installation, a build's evaluation
+fails, and the error names the App and the repository.
 
 To register the reviewer App manually:
 

docs/writing-an-app.md

@@ -857,7 +857,7 @@ A vault row with one secret issues it under any name. For the secret of a
 service that does not depend on the subject,
 [list it on the agent](#give-an-agent-a-service-secret) instead.
 
-Override `get_mcp_servers(subject)` to give the sandbox an MCP server with its
+Override `get_mcp_servers(subject, *, agent="")` to give the sandbox an MCP server with its
 own vault row:
 
 ```python
@@ -866,18 +866,24 @@ from druks.sandbox import SandboxMcpServer
 
 class BuildWorkspace(RepoWorkspace):
     @classmethod
-    async def get_mcp_servers(cls, subject) -> tuple[SandboxMcpServer, ...]:
-        actor = await get_review_actor()
+    async def get_mcp_servers(cls, subject, *, agent="") -> tuple[SandboxMcpServer, ...]:
+        service = Github
+        if agent == SoftwareFactory.evaluate_implementation.id:
+            service = (await get_review_actor()).service
         return (
             SandboxMcpServer(
                 name="github",
                 url="https://api.githubcopilot.com/mcp/",
-                secret_id=(await actor.service.get()).id,
+                secret_id=(await service.get()).id,
                 resources=(cls.get_repo(subject),),
             ),
         )
 ```
 
+The `agent` keyword is the current agent ID, or an empty string outside an agent
+call. Overrides must accept it. Use it to select a credential for a particular
+agent. Warm sandboxes rotate when their secret references change.
+
 The server names the vault row the issuer answers from and what the token
 covers: here a connected GitHub service and its repo. Druks binds the server's
 host and the variable `MCP_GITHUB_TOKEN` to the entry when it creates the