fix(workspace): preserve caller's AgentContext in load_skills_from_agent_server()

[vnktadithya]

HUMAN:

The base context, which is already present, is being overwritten by the load_skills_from_agent_server function. This function's job is to explicitly change skills and load_public_skills, and to preserve all the existing context. But it was creating a new AgentContext object and overwriting all the existing parameters.

---

AGENT:

Why

RemoteWorkspace.load_skills_from_agent_server() always constructed a brand-new AgentContext from scratch when returning, discarding every field of any AgentContext the caller already held except skills and load_public_skills. This silently reset load_user_skills, load_project_skills, disabled_skills, marketplace_path, both message suffixes, and — less obviously — current_datetime, which has a default_factory rather than a static default, so it got re-stamped to a fresh timestamp on every call rather than preserved.

OpenHandsCloudWorkspace.load_skills_from_agent_server() in openhands-workspace/openhands/workspace/cloud/workspace.py re-declares this method's full signature and forwards every argument to super().load_skills_from_agent_server(...) — done deliberately so griffe (this repo's API-compatibility checker) doesn't flag inherited-method removal from the subclass. That means the fix has to be mirrored there too, or OpenHandsCloudWorkspace callers would silently never get access to the new capability while its docstring still claims parity with RemoteWorkspace.

Originally flagged as an out-of-scope "optional companion change" in #4542 while fixing the load_memory propagation bug (#4566) — noted there as worth doing on its own merits but an independent code path from that fix.

Summary

• Added an optional base_context: AgentContext | None = None parameter to RemoteWorkspace.load_skills_from_agent_server() (openhands-sdk/openhands/sdk/workspace/remote/base.py). When provided, the method now returns base.model_copy(update={"skills": ..., "load_public_skills": ...}) instead of constructing a fresh AgentContext(...), so every other field the caller configured survives. When omitted (default), behavior is unchanged from today.
• Mirrored the identical signature and forwarding change in the OpenHandsCloudWorkspace override (openhands-workspace/openhands/workspace/cloud/workspace.py) so the fix isn't silently unavailable on that class.
• Added regression tests: 4 new tests in tests/sdk/workspace/remote/test_remote_workspace.py (base context preserved when skills are found, preserved on the empty-skills fallback path, no-base-context call matches legacy behavior, current_datetime specifically survives a round-trip) and 1 new test in tests/workspace/test_cloud_workspace_repos.py confirming the OpenHandsCloudWorkspace override actually forwards base_context through to the fix.

Issue Number

Fixes #4863

How to Test

Unit tests:

uv run pytest tests/sdk/workspace/remote/test_remote_workspace.py -v

Result: 71 passed (67 existing + 4 new, all unchanged existing tests still green since base_context defaults to None and preserves today's behavior when omitted).

uv run pytest tests/workspace/test_cloud_workspace_repos.py -v

Result: 65 passed (64 existing + 1 new).

Unit tests alone rely on mocked responses, so I also ran an end-to-end pass exercising real AgentContext construction and real Pydantic model_copy behavior — the only thing mocked is the outbound HTTP call to the agent-server, since no live sandbox is available here:

uv run python .pr/verify_base_context_preserved.py

Output:

[09/06/26 10:37:51] INFO Loading skills via agent-server... base.py:913
[09/06/26 10:37:51] INFO Loaded 1 skills base.py:930
[PASS] A: skills loaded
[PASS] A: marketplace_path preserved
[PASS] A: disabled_skills preserved
[PASS] A: user_message_suffix preserved
[PASS] A: current_datetime preserved exactly
[PASS] A: load_public_skills=False (skills were found)
[09/06/26 10:37:51] INFO Loading skills via agent-server... base.py:913
[09/06/26 10:37:51] INFO Loaded 0 skills base.py:930
[09/06/26 10:37:51] WARNING No skills loaded, falling back to public skills base.py:943
[PASS] B: no skills returned
[PASS] B: marketplace_path preserved on fallback
[PASS] B: disabled_skills preserved on fallback
[PASS] B: load_public_skills=True (fallback)
[09/06/26 10:37:51] INFO Loading skills via agent-server... base.py:913
[09/06/26 10:37:51] INFO Loaded 1 skills base.py:930
[PASS] C: default marketplace_path when no base_context
[PASS] C: default disabled_skills when no base_context
[PASS] C: default user_message_suffix when no base_context
[09/06/26 10:37:51] INFO Loading skills via agent-server... base.py:913
[09/06/26 10:37:51] INFO Loaded 1 skills base.py:930
[PASS] D: OpenHandsCloudWorkspace forwards base_context
All 14 checks passed.

Acceptance criteria (from #4863)

• load_skills_from_agent_server() accepts an optional base AgentContext instead of always constructing one from scratch — confirmed in the diff (new base_context parameter, openhands-sdk/openhands/sdk/workspace/remote/base.py)
• When a base context is provided, all of its fields survive the call except skills and load_public_skills — test_load_skills_from_agent_server_preserves_base_context_when_found
• current_datetime from the base context is preserved, not regenerated — test_load_skills_from_agent_server_preserves_current_datetime
• Calling the function without a base context (every existing call site today) produces output equivalent to current behavior — no breaking change — test_load_skills_from_agent_server_without_base_context_matches_legacy; all pre-existing tests in test_remote_workspace.py pass unchanged
• Test coverage for: (1) base context preserved when skills are found — test_load_skills_from_agent_server_preserves_base_context_when_found; (2) base context preserved on the empty-skills fallback path — test_load_skills_from_agent_server_preserves_base_context_on_fallback; (3) no-base-context call matches legacy behavior — test_load_skills_from_agent_server_without_base_context_matches_legacy; (4) current_datetime specifically survives a round-trip — test_load_skills_from_agent_server_preserves_current_datetime
• OpenHandsCloudWorkspace.load_skills_from_agent_server() accepts and forwards base_context to super().load_skills_from_agent_server(...), with test coverage confirming preserved fields survive through the override — test_load_skills_from_agent_server_preserves_base_context (tests/workspace/test_cloud_workspace_repos.py)

Video/Screenshots

No video — this is a backend-only change with no UI. The .pr/verify_base_context_preserved.py script above is the functional evidence: it drives real RemoteWorkspace and OpenHandsCloudWorkspace instances through real AgentContext/model_copy code, not pytest mocks of the fix itself.

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

User-facing: load_skills_from_agent_server() now accepts an optional base_context parameter on both RemoteWorkspace and OpenHandsCloudWorkspace. Omitting it (every existing call site today) leaves behavior unchanged — not a breaking change.

Other RemoteWorkspace subclasses — ApptainerWorkspace, APIRemoteWorkspace, DockerWorkspace, DockerDevWorkspace — inherit the method directly without overriding it, so they receive base_context support automatically with no changes needed. AsyncRemoteWorkspace doesn't implement this method at all and is unaffected. Confirmed via a repo-wide search for every call site and override of load_skills_from_agent_server before opening this PR.

Checked this change against .github/scripts/check_sdk_api_breakage.py (the griffe-based API-breakage gate this repo runs in CI): its scope, per its own docstring and its full test suite, is limited to detecting removed exports/members against the last published release. Adding an optional parameter with a default is purely additive and outside what that check inspects, so no MINOR version bump is required for this change alone.

model_copy(update={...}) performs a shallow copy, so list-valued fields (e.g. disabled_skills) on the returned context share the same underlying list object as the caller's base_context unless a fresh one is supplied per call. This is the same shallow-copy convention already relied on by _with_load_memory() in conversation_service.py from #4566, not a new risk introduced here.

Related: #4542 (root issue), #4566 (merged fix for the load_memory propagation half of that issue — this is the other half, called out there as independent and deferred).

[github-actions]

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. Because this is a fork PR, the directory will be automatically removed from main immediately after merge.

[Copilot]

🟡 Changes recommended

A couple of small convention/maintainability issues (change-narration comments and brittle default assertions) should be cleaned up before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes a workspace-context preservation bug by ensuring RemoteWorkspace.load_skills_from_agent_server() can update only skills / load_public_skills while preserving a caller-provided AgentContext (including current_datetime), and mirrors the signature/forwarding in OpenHandsCloudWorkspace to keep subclass behavior consistent.

Changes:

• Add optional base_context: AgentContext | None to RemoteWorkspace.load_skills_from_agent_server() and return base_context.model_copy(update=...) instead of constructing a fresh AgentContext.
• Mirror the base_context parameter and forwarding in OpenHandsCloudWorkspace.load_skills_from_agent_server().
• Add regression tests covering preservation behavior (including fallback and current_datetime) and subclass forwarding; include a .pr/ verification script.

File summaries

File	Description
openhands-sdk/openhands/sdk/workspace/remote/base.py	Adds base_context and preserves caller context via model_copy(update=...) while updating loaded skills + load_public_skills.
openhands-workspace/openhands/workspace/cloud/workspace.py	Mirrors signature and forwards base_context through the override to the base implementation.
tests/sdk/workspace/remote/test_remote_workspace.py	Adds unit tests validating base-context preservation, fallback behavior, legacy behavior when omitted, and current_datetime preservation.
tests/workspace/test_cloud_workspace_repos.py	Adds a regression test ensuring the cloud workspace override forwards base_context.
.pr/verify_base_context_preserved.py	Adds an end-to-end verification script demonstrating preservation on both workspace classes (PR artifact).

Review details

Suppressed comments (1)

openhands-workspace/openhands/workspace/cloud/workspace.py:943

• Remove the inline “# new” comment on the forwarded argument; it’s change-narration noise and will drift over time.

            base_context=base_context,  # new

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[all-hands-bot]

🚦 CI is currently failing on this PR's latest commit.

Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request @all-hands-bot as a reviewer to have it reviewed regardless of CI status.)

This is an automated check - no AI was used to generate this comment.

[vnktadithya]

tests/sdk/llm/test_runtime_metadata.py::test_effective_unchanged_before_resolution and tests/sdk/llm/test_model_features.py::test_reasoning_effort_support[openrouter/moonshotai/kimi-k2-thinking-False] are failing on main independent of this PR — reproduced on a clean checkout at the current tip with none of this PR's changes touching tests/sdk/llm/ or the LLM subsystem at all. Flagging rather than fixing, since it's outside this PR's scope.