feat(acp): secrets in agent_context — prompt awareness + subprocess injection

openhands-sdk/openhands/sdk/agent/acp_agent.py

@@ -56,6 +56,7 @@
 from openhands.sdk.llm import LLM, Message, MessageToolCall, TextContent
 from openhands.sdk.logger import get_logger
 from openhands.sdk.observability.laminar import maybe_init_laminar, observe
+from openhands.sdk.secret import SecretSource
 from openhands.sdk.tool import Tool  # noqa: TC002
 from openhands.sdk.tool.builtins.finish import FinishAction, FinishObservation
 
@@ -885,6 +886,20 @@ def _start_acp_server(self, state: ConversationState) -> None:
         env = default_environment()
         env.update(os.environ)
         env.update(self.acp_env)
+        # Inject secrets from agent_context. acp_env entries take precedence
+        # (already set above), so we only fill keys not already present.
+        # SecretSource.get_value() is synchronous; calling it here is safe
+        # because _start_acp_server is a regular (non-async) method.
+        if self.agent_context and self.agent_context.secrets:
+            for name, secret in self.agent_context.secrets.items():
+                if name not in env:
+                    value = (
+                        secret.get_value()
+                        if isinstance(secret, SecretSource)
+                        else str(secret)
+                    )
+                    if value:
+                        env[name] = value
         # Strip CLAUDECODE so nested Claude Code instances don't refuse to start
         env.pop("CLAUDECODE", None)
 

openhands-sdk/openhands/sdk/context/agent_context.py

@@ -95,7 +95,7 @@ class AgentContext(BaseModel):
             "Values can be either strings or SecretSource instances "
             "(str | SecretSource)."
         ),
-        json_schema_extra={"acp_compatible": False},
+        json_schema_extra={"acp_compatible": True},
     )
     current_datetime: datetime | str | None = Field(
         default_factory=datetime.now,
@@ -316,20 +316,24 @@ def to_acp_prompt_context(self) -> str | None:
 
         ACP servers own their tools, MCP servers, hooks, and execution model, so
         this adapter only emits prompt-only context.  Unsupported AgentContext
-        semantics (e.g. secrets) are rejected by
-        :meth:`validate_acp_compatibility`.
+        fields are rejected by :meth:`validate_acp_compatibility`.
 
         The rendering reuses :meth:`get_system_message_suffix` with the same
         ``system_message_suffix.j2`` template so that ACP agents receive the
-        identical prompt layout as the general agent (minus secrets).
+        identical prompt layout as the regular agent.  This includes the
+        ``<CUSTOM_SECRETS>`` block when secrets are present, informing the ACP
+        subprocess which environment variables are available.  The actual secret
+        values are injected into the subprocess environment by
+        ``ACPAgent._start_acp_server``; the prompt block only advertises their
+        names so the agent knows to use them.
 
         ``user_message_suffix`` is a compatible field but is not emitted here
         because ``LocalConversation`` already applies it through
         ``event.to_llm_message()``; including it would duplicate it.
         """
         self.validate_acp_compatibility()
-        # ACP doesn't support secrets (enforced above) and has no model-specific
-        # skill filtering, so we delegate to the shared renderer with no extras.
+        # No model-specific skill filtering for ACP — delegate to the shared
+        # renderer which also renders the <CUSTOM_SECRETS> block from secrets.
         return self.get_system_message_suffix()
 
     def get_user_message_suffix(

tests/sdk/agent/test_acp_agent.py

@@ -211,13 +211,15 @@ def test_allows_agent_context_for_prompt_extensions(self, tmp_path):
 
         self._init_with_patches(agent, tmp_path)
 
-    def test_rejects_unsupported_agent_context_at_init(self, tmp_path):
+    def test_allows_agent_context_with_secrets(self, tmp_path):
+        """Secrets are now ACP-compatible: they are injected into the subprocess
+        env by _start_acp_server and advertised in the prompt via <CUSTOM_SECRETS>."""
         agent = ACPAgent(
             acp_command=["echo"],
             agent_context=AgentContext(secrets={"GITHUB_TOKEN": "ghp_secret"}),
         )
-        with pytest.raises(NotImplementedError, match="secrets"):
-            self._init_with_patches(agent, tmp_path)
+        # Should not raise
+        self._init_with_patches(agent, tmp_path)
 
     def test_agent_context_to_acp_prompt_context(self):
         context = AgentContext(
@@ -261,11 +263,31 @@ def test_agent_context_to_acp_prompt_context_emits_datetime_by_default(self):
         assert prompt is not None
         assert "<CURRENT_DATETIME>" in prompt
 
-    def test_agent_context_to_acp_prompt_context_rejects_secrets(self):
-        context = AgentContext(secrets={"GITHUB_TOKEN": "ghp_secret"})
+    def test_agent_context_to_acp_prompt_context_includes_secrets(self):
+        """Secrets appear in the ACP prompt as a <CUSTOM_SECRETS> block so the
+        ACP subprocess knows which environment variables are available."""
+        from pydantic import SecretStr
+
+        from openhands.sdk.secret import StaticSecret
+
+        context = AgentContext(
+            secrets={
+                "GITHUB_TOKEN": StaticSecret(
+                    value=SecretStr("ghp_secret"),
+                    description="GitHub authentication token",
+                ),
+                "MY_API_KEY": StaticSecret(value=SecretStr("key123")),
+            },
+            current_datetime=None,
+        )
 
-        with pytest.raises(NotImplementedError, match="secrets"):
-            context.to_acp_prompt_context()
+        prompt = context.to_acp_prompt_context()
+
+        assert prompt is not None
+        assert "<CUSTOM_SECRETS>" in prompt
+        assert "$GITHUB_TOKEN" in prompt
+        assert "GitHub authentication token" in prompt
+        assert "$MY_API_KEY" in prompt
 
     def test_agent_context_to_acp_prompt_context_includes_legacy_repo_skills(self):
         context = AgentContext(
@@ -3291,3 +3313,144 @@ def test_roundtrip_via_conversation_state_persistence(self, tmp_path):
         assert kwargs["cwd"] == str(workspace)
         conn2.new_session.assert_not_awaited()
         assert agent2._session_id == "roundtrip-sess"
+
+
+class TestACPSecretsEnvInjection:
+    """Tests for secret injection into the ACP subprocess environment.
+
+    Secrets passed via ``agent_context.secrets`` must land in the subprocess
+    env so the ACP server (Claude Code, Codex CLI, etc.) can use them.
+    ``acp_env`` entries take precedence over agent_context secrets.
+    """
+
+    @staticmethod
+    def _make_conn():
+        conn = MagicMock()
+        init_response = MagicMock()
+        init_response.agent_info = MagicMock()
+        init_response.agent_info.name = "claude-agent-acp"
+        init_response.agent_info.version = "1.0"
+        init_response.auth_methods = []
+        conn.initialize = AsyncMock(return_value=init_response)
+        new_response = MagicMock()
+        new_response.session_id = "sess-1"
+        conn.new_session = AsyncMock(return_value=new_response)
+        conn.load_session = AsyncMock(return_value=MagicMock())
+        conn.set_session_mode = AsyncMock()
+        conn.set_session_model = AsyncMock()
+        conn.authenticate = AsyncMock()
+        conn.close = AsyncMock()
+        return conn
+
+    @staticmethod
+    def _run_start_capturing_env(agent, tmp_path) -> dict:
+        """Run _start_acp_server and return the env dict passed to the subprocess."""
+        from contextlib import ExitStack
+
+        from openhands.sdk.utils.async_executor import AsyncExecutor
+
+        captured: dict = {}
+        conn = TestACPSecretsEnvInjection._make_conn()
+
+        mock_process = MagicMock()
+        mock_process.stdin = MagicMock()
+        mock_process.stdout = MagicMock()
+
+        async def _fake_create_subprocess_exec(*_args, env=None, **_kwargs):
+            captured.update(env or {})
+            return mock_process
+
+        async def _fake_filter(_src, _dst):
+            return None
+
+        state = _make_state(tmp_path)
+        agent._executor = AsyncExecutor()
+
+        with ExitStack() as stack:
+            stack.enter_context(
+                patch(
+                    "openhands.sdk.agent.acp_agent.asyncio.create_subprocess_exec",
+                    new=_fake_create_subprocess_exec,
+                )
+            )
+            stack.enter_context(
+                patch(
+                    "openhands.sdk.agent.acp_agent.ClientSideConnection",
+                    return_value=conn,
+                )
+            )
+            stack.enter_context(
+                patch(
+                    "openhands.sdk.agent.acp_agent._filter_jsonrpc_lines",
+                    new=_fake_filter,
+                )
+            )
+            stack.enter_context(
+                patch(
+                    "openhands.sdk.agent.acp_agent.asyncio.StreamReader",
+                    return_value=MagicMock(),
+                )
+            )
+            agent._start_acp_server(state)
+
+        return captured
+
+    def test_static_secret_injected_into_subprocess_env(self, tmp_path):
+        """A StaticSecret in agent_context.secrets lands in the subprocess env."""
+        from pydantic import SecretStr
+
+        from openhands.sdk.secret import StaticSecret
+
+        agent = _make_agent(
+            agent_context=AgentContext(
+                secrets={
+                    "GITHUB_TOKEN": StaticSecret(
+                        value=SecretStr("ghp_test123"),
+                        description="GitHub token",
+                    )
+                }
+            )
+        )
+        env = self._run_start_capturing_env(agent, tmp_path)
+        assert env.get("GITHUB_TOKEN") == "ghp_test123"
+
+    def test_acp_env_takes_precedence_over_agent_context_secret(self, tmp_path):
+        """An explicit acp_env entry wins over the same key in agent_context.secrets."""
+        from pydantic import SecretStr
+
+        from openhands.sdk.secret import StaticSecret
+
+        agent = _make_agent(
+            acp_env={"MY_TOKEN": "acp-env-wins"},
+            agent_context=AgentContext(
+                secrets={"MY_TOKEN": StaticSecret(value=SecretStr("secret-panel"))}
+            ),
+        )
+        env = self._run_start_capturing_env(agent, tmp_path)
+        assert env.get("MY_TOKEN") == "acp-env-wins"
+
+    def test_none_value_secret_not_injected(self, tmp_path):
+        """A StaticSecret with value=None is not added to the subprocess env."""
+        from openhands.sdk.secret import StaticSecret
+
+        agent = _make_agent(
+            agent_context=AgentContext(
+                secrets={"ABSENT_SECRET": StaticSecret(value=None)}
+            )
+        )
+        env = self._run_start_capturing_env(agent, tmp_path)
+        assert "ABSENT_SECRET" not in env
+
+    def test_empty_string_secret_not_injected(self, tmp_path):
+        """Empty string secrets are not injected into the subprocess env."""
+        from pydantic import SecretStr
+
+        from openhands.sdk.secret import StaticSecret
+
+        agent = _make_agent(
+            agent_context=AgentContext(
+                secrets={"EMPTY_SECRET": StaticSecret(value=SecretStr(""))}
+            )
+        )
+        env = self._run_start_capturing_env(agent, tmp_path)
+        assert "EMPTY_SECRET" not in env