Skip to content

fix(sdk): preserve pinned requested_ref in InstallationManager.update() - #4448

Open
santhiprakash wants to merge 1 commit into
OpenHands:mainfrom
santhiprakash:fix/installation-update-preserve-requested-ref
Open

fix(sdk): preserve pinned requested_ref in InstallationManager.update()#4448
santhiprakash wants to merge 1 commit into
OpenHands:mainfrom
santhiprakash:fix/installation-update-preserve-requested-ref

Conversation

@santhiprakash

Copy link
Copy Markdown

HUMAN:

Santhi Prakash — small correctness fix for the extension update path; see AGENT section for evidence.


AGENT:

Why

InstallationManager.update() always re-fetched with ref=None, ignoring the requested_ref recorded at install time (added in #4349 / #4375). Calling update() on an extension pinned to a tag or commit silently un-pinned it and jumped to the source's current default-branch HEAD instead of just re-syncing to the pinned ref. This is reachable today via the public API for both Plugins and Skills:

  • plugins_router.py POST /installed/{plugin_name}/refreshservice_update_pluginInstallationManager.update()
  • skills_router.py POST /installed/{skill_name}/refreshservice_update_skillInstallationManager.update()

Reported in #4363.

Summary

  • update() now re-fetches at current_info.requested_ref instead of hardcoding ref=None. A pinned install (tag/commit) stays pinned (only its resolved SHA may move, e.g. a moved tag); a floating install (requested_ref is None) still resolves to latest, same as before.
  • Re-pinning to a different ref remains an explicit install(source, ref=new_ref, force=True) call — update() needs no new parameter.
  • This mirrors the reconcile-vs-repin split already used by the Canvas Extensions staged refresh (check_canvas_extension_update fetches with ref=current_info.requested_ref), so the shared InstallationManager now matches that established pattern.
  • Replaced test_update_clears_requested_ref_to_track_latest (which encoded the old un-pinning behavior) with test_update_preserves_pinned_requested_ref, and added test_update_floating_install_resolves_latest to guard the floating case.

Issue Number

Fixes #4363

How to Test

uv run pytest tests/sdk/extensions/installation/test_installation_manager.py -q
uv run pytest tests/sdk/extensions/ tests/agent_server/canvas_extensions/ tests/agent_server/test_skills_router.py tests/agent_server/test_plugins_router.py -q
uv run ruff check openhands-sdk/openhands/sdk/extensions/installation/manager.py tests/sdk/extensions/installation/test_installation_manager.py
uv run pre-commit run --files openhands-sdk/openhands/sdk/extensions/installation/manager.py tests/sdk/extensions/installation/test_installation_manager.py

Video/Screenshots

Not applicable — pure logic change with no UI/visual surface; behavior is covered by the unit tests above.

Design Doc

Not added — this is a small one-line correctness fix (a trivial PR per CONTRIBUTING.md), not a new API or behavior-change in the agent loop.

Type

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

Notes

This changes the (previously undocumented) behavior of update() and the /refresh endpoints for Plugins and Skills: a pinned extension now stays pinned on refresh instead of jumping to latest. The issue calls this out explicitly and the new behavior matches the reconcile-vs-repin distinction already shipped for Canvas Extensions in #4374. No persisted settings shape or REST contract changes — InstallationInfo.requested_ref is unchanged.

update() hardcoded ref=None when re-fetching, so refreshing an extension
pinned to a tag or commit silently un-pinned it and jumped to the source's
default-branch HEAD. Re-fetch at current_info.requested_ref instead: a
pinned install stays pinned (only its resolved SHA may move, e.g. a moved
tag), and a floating install (requested_ref is None) still resolves to
latest, same as before. Re-pinning to a different ref remains an explicit
install(source, ref=new_ref, force=True) call.

This mirrors the reconcile-vs-repin split already used by the Canvas
Extensions staged refresh (check_canvas_extension_update), and fixes the
public /refresh endpoints for Plugins and Skills that route through
update().

Fixes OpenHands#4363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InstallationManager.update() silently drops pinned refs (should reconcile, not force ref=None)

1 participant