From 823c9fe511bf9a7b93d1cdd02ef562e14c41d7de Mon Sep 17 00:00:00 2001 From: Debug Agent Date: Mon, 27 Apr 2026 15:48:59 -0700 Subject: [PATCH 1/4] feat(acp): inject secrets into ACP subprocess env via agent_context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously AgentContext.secrets was rejected for ACP agents with `acp_compatible: False` because ACP subprocesses did not use OpenHands secret injection. Now that user secrets are explicitly plumbed to ACP agents, this restriction is lifted and the SDK wires up both halves: **Prompt awareness** — `to_acp_prompt_context()` now includes the `` block (rendered by `get_system_message_suffix()`) when `agent_context.secrets` is set, so the ACP subprocess sees the names and descriptions of available environment variables in its first prompt, exactly as the regular OpenHands agent does. **Subprocess availability** — `_start_acp_server` resolves each `SecretSource` in `agent_context.secrets` to a plain string and injects it into the subprocess env via `setdefault`-style logic, so `acp_env` entries always take precedence. `SecretSource.get_value()` is called synchronously here, which is safe because `_start_acp_server` is a regular (non-async) method. `SecretSource` is now a top-level import in `acp_agent.py`. Tests added in `TestACPSecretsEnvInjection`: - StaticSecret value lands in subprocess env - acp_env wins over agent_context secret for the same key - None-valued secrets are excluded Existing tests that asserted secrets were *rejected* are updated to reflect the new behavior. Co-Authored-By: Claude Sonnet 4.6 --- .../openhands/sdk/agent/acp_agent.py | 15 ++ .../openhands/sdk/context/agent_context.py | 16 +- tests/sdk/agent/test_acp_agent.py | 159 +++++++++++++++++- 3 files changed, 177 insertions(+), 13 deletions(-) diff --git a/openhands-sdk/openhands/sdk/agent/acp_agent.py b/openhands-sdk/openhands/sdk/agent/acp_agent.py index 43bda63e8c..4818f9652e 100644 --- a/openhands-sdk/openhands/sdk/agent/acp_agent.py +++ b/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) diff --git a/openhands-sdk/openhands/sdk/context/agent_context.py b/openhands-sdk/openhands/sdk/context/agent_context.py index 6da4f65c95..967513b43a 100644 --- a/openhands-sdk/openhands/sdk/context/agent_context.py +++ b/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 + ```` 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 block from secrets. return self.get_system_message_suffix() def get_user_message_suffix( diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index d63170bd78..4e1fd9acd5 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/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 .""" 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,30 @@ def test_agent_context_to_acp_prompt_context_emits_datetime_by_default(self): assert prompt is not None assert "" 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 block so the + ACP subprocess knows which environment variables are available.""" + from openhands.sdk.secret import StaticSecret + from pydantic import SecretStr + + context = AgentContext( + secrets={ + "GITHUB_TOKEN": StaticSecret( + value=SecretStr("ghp_secret"), + description="GitHub authentication token", + ), + "MY_API_KEY": StaticSecret(value=SecretStr("key123")), + }, + current_datetime=None, + ) + + prompt = context.to_acp_prompt_context() - with pytest.raises(NotImplementedError, match="secrets"): - context.to_acp_prompt_context() + assert prompt is not None + assert "" 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 +3312,127 @@ 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 openhands.sdk.secret import StaticSecret + from pydantic import SecretStr + + 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 openhands.sdk.secret import StaticSecret + from pydantic import SecretStr + + 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 From 10a23ce369e637bd4c32a99eb3db76b06828837b Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 27 Apr 2026 22:55:30 +0000 Subject: [PATCH 2/4] test(acp): add test for empty string secret not injected into subprocess env Document that the `if value:` guard in `_start_acp_server` also filters out empty-string secrets (same as None), matching the existing test for None-valued secrets. Co-authored-by: openhands --- tests/sdk/agent/test_acp_agent.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/sdk/agent/test_acp_agent.py b/tests/sdk/agent/test_acp_agent.py index 4e1fd9acd5..c9d4d27be5 100644 --- a/tests/sdk/agent/test_acp_agent.py +++ b/tests/sdk/agent/test_acp_agent.py @@ -266,9 +266,10 @@ def test_agent_context_to_acp_prompt_context_emits_datetime_by_default(self): def test_agent_context_to_acp_prompt_context_includes_secrets(self): """Secrets appear in the ACP prompt as a block so the ACP subprocess knows which environment variables are available.""" - from openhands.sdk.secret import StaticSecret from pydantic import SecretStr + from openhands.sdk.secret import StaticSecret + context = AgentContext( secrets={ "GITHUB_TOKEN": StaticSecret( @@ -3345,6 +3346,7 @@ def _make_conn(): 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 = {} @@ -3395,9 +3397,10 @@ async def _fake_filter(_src, _dst): def test_static_secret_injected_into_subprocess_env(self, tmp_path): """A StaticSecret in agent_context.secrets lands in the subprocess env.""" - from openhands.sdk.secret import StaticSecret from pydantic import SecretStr + from openhands.sdk.secret import StaticSecret + agent = _make_agent( agent_context=AgentContext( secrets={ @@ -3413,9 +3416,10 @@ def test_static_secret_injected_into_subprocess_env(self, tmp_path): 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 openhands.sdk.secret import StaticSecret from pydantic import SecretStr + from openhands.sdk.secret import StaticSecret + agent = _make_agent( acp_env={"MY_TOKEN": "acp-env-wins"}, agent_context=AgentContext( @@ -3436,3 +3440,17 @@ def test_none_value_secret_not_injected(self, tmp_path): ) 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 From 3930f9dfdba7094f30170a3daab7e073711ef7bf Mon Sep 17 00:00:00 2001 From: Debug Agent Date: Mon, 27 Apr 2026 16:09:40 -0700 Subject: [PATCH 3/4] feat(acp): thread agent_context through ACPAgentSettings.create_agent() Adds ``agent_context: AgentContext | None`` to ``ACPAgentSettings`` and passes it to ``ACPAgent`` in ``create_agent()``. This is required by the app-server refactor (OpenHands/OpenHands#14171 follow-up): the app-server sets ``agent_context.secrets`` on an ``ACPAgentSettings`` copy and relies on ``create_agent()`` forwarding it to the resulting ``ACPAgent``. Co-Authored-By: Claude Sonnet 4.6 --- openhands-sdk/openhands/sdk/settings/model.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/openhands-sdk/openhands/sdk/settings/model.py b/openhands-sdk/openhands/sdk/settings/model.py index 0378785919..57e4a573bc 100644 --- a/openhands-sdk/openhands/sdk/settings/model.py +++ b/openhands-sdk/openhands/sdk/settings/model.py @@ -934,6 +934,14 @@ class ACPAgentSettings(BaseModel): ).model_dump(), }, ) + agent_context: AgentContext | None = Field( + default=None, + description=( + "Optional prompt-only context injected into each ACP user turn. " + "Secrets in agent_context are resolved to plain strings and injected " + "into the subprocess environment at start time." + ), + ) llm: LLM = Field( default_factory=_default_llm_settings, description=( @@ -994,6 +1002,7 @@ def create_agent(self) -> ACPAgent: acp_model=self.acp_model, acp_session_mode=self.acp_session_mode, acp_prompt_timeout=self.acp_prompt_timeout, + agent_context=self.agent_context, ) From 272dda6b9a0fbb03c379976e983ad8c1db156362 Mon Sep 17 00:00:00 2001 From: Debug Agent Date: Mon, 27 Apr 2026 18:19:17 -0700 Subject: [PATCH 4/4] refactor(acp): drop agent_context from ACPAgentSettings ACPAgent inherits agent_context from AgentBase already. The caller (OpenHands app server) can set it directly on the returned agent after create_agent(), so there's no need to surface it in the settings model. Co-Authored-By: Claude Sonnet 4.6 --- openhands-sdk/openhands/sdk/settings/model.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/openhands-sdk/openhands/sdk/settings/model.py b/openhands-sdk/openhands/sdk/settings/model.py index 57e4a573bc..0378785919 100644 --- a/openhands-sdk/openhands/sdk/settings/model.py +++ b/openhands-sdk/openhands/sdk/settings/model.py @@ -934,14 +934,6 @@ class ACPAgentSettings(BaseModel): ).model_dump(), }, ) - agent_context: AgentContext | None = Field( - default=None, - description=( - "Optional prompt-only context injected into each ACP user turn. " - "Secrets in agent_context are resolved to plain strings and injected " - "into the subprocess environment at start time." - ), - ) llm: LLM = Field( default_factory=_default_llm_settings, description=( @@ -1002,7 +994,6 @@ def create_agent(self) -> ACPAgent: acp_model=self.acp_model, acp_session_mode=self.acp_session_mode, acp_prompt_timeout=self.acp_prompt_timeout, - agent_context=self.agent_context, )