Skip to content

Add plugin lock-file upgrade - #6317

Open
samuv wants to merge 6 commits into
plugins-lock/04-syncfrom
plugins-lock/05-upgrade
Open

Add plugin lock-file upgrade#6317
samuv wants to merge 6 commits into
plugins-lock/04-syncfrom
plugins-lock/05-upgrade

Conversation

@samuv

@samuv samuv commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Why: Sync restores a pin; teams also need a reviewed path to advance that pin when a mutable source has newer content.
  • What:
    • thv ai-plugin upgrade and POST /plugins/upgrade re-resolve each plugins: lock entry's source and install newer content when the digest moved (--preview / --allow-ref-change / --fail-on-changes).
    • Immutable sources (OCI digest or full git commit hash) are reported not-upgradable. Source is never rewritten.
    • A repository move is blocked unless --allow-ref-change is passed. Signer-change guarding is Stack 2 / PR9 — --allow-signer-change is not exposed yet.
    • Preview and --fail-on-changes still fetch OCI artifacts to compare digests (RFC: preview is not side-effect-free) but do not write the lock or install.
    • Gated by TOOLHIVE_PLUGINS_LOCK_ENABLED (403 when off).

Part of #6300. Stack 5/5 — schema → lock-service → install-hooks → sync → upgrade.

Type of change

  • New feature

Test plan

  • Unit tests (./pkg/plugins/pluginsvc upgrade tests and ./pkg/api/v1 upgrade endpoint tests, with the Taskfile race/ldflags flags)
  • Linting (task lint-fix)

Does this introduce a user-facing change?

No by default — the feature is inert unless TOOLHIVE_PLUGINS_LOCK_ENABLED=true. With the gate on, thv ai-plugin upgrade re-resolves plugins: lock entries.

Special notes for reviewers

  • No signer-change guard in this PR (PR9). The options type already aliases AllowSignerChange from skills; this PR does not enforce it and does not add the CLI flag.
  • resolveLatestState mirrors Install's dispatch (git → OCI → registry name) but stops short of extraction / DB / lock writes.
  • Git resolve clones to read HEAD; there is no lighter digest-only primitive, matching skills' "preview is not side-effect-free" note for OCI.

@github-actions github-actions Bot added the size/XL Extra large PR: 1000+ lines changed label Aug 13, 2026
@samuv samuv self-assigned this Aug 13, 2026
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Aug 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.73981% with 138 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.92%. Comparing base (f63ae52) to head (d97c97e).

Files with missing lines Patch % Lines
pkg/plugins/pluginsvc/upgrade.go 57.53% 67 Missing and 26 partials ⚠️
pkg/plugins/pluginsvc/sync.go 45.28% 23 Missing and 6 partials ⚠️
pkg/plugins/client/client.go 0.00% 12 Missing ⚠️
pkg/api/v1/plugins.go 90.90% 1 Missing and 1 partial ⚠️
pkg/plugins/pluginsvc/install.go 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           plugins-lock/04-sync    #6317      +/-   ##
========================================================
- Coverage                 73.01%   72.92%   -0.10%     
========================================================
  Files                       747      748       +1     
  Lines                     79256    79571     +315     
========================================================
+ Hits                      57870    58027     +157     
- Misses                    17278    17409     +131     
- Partials                   4108     4135      +27     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX JAORMX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel review found inconsistent plain-name resolution, ambiguous preview API status, and missing required CLI coverage/completion. Please address the inline findings before merge. The transaction fixes requested on the earlier stack PRs also apply to this upgrade path.

Comment thread pkg/plugins/pluginsvc/upgrade.go Outdated
Comment thread pkg/plugins/pluginsvc/upgrade.go
Comment thread cmd/thv/app/ai_plugin_upgrade.go
Comment thread cmd/thv/app/ai_plugin_upgrade.go
@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from 9459007 to eb5e5b4 Compare August 14, 2026 08:16
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Aug 14, 2026
@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from eb5e5b4 to d071d5a Compare August 14, 2026 08:46
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Aug 14, 2026
@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from d071d5a to f51bf78 Compare August 14, 2026 08:57
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Aug 14, 2026

@JAORMX JAORMX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local-store resolution, E2E coverage, and completion were addressed. The local apply path still drops the artifact/reference needed by persisted state and later sync.

Comment thread pkg/plugins/pluginsvc/upgrade.go
@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from f51bf78 to f5b5f83 Compare August 14, 2026 12:52
@github-actions github-actions Bot removed the size/XL Extra large PR: 1000+ lines changed label Aug 14, 2026
@github-actions github-actions Bot added the size/XL Extra large PR: 1000+ lines changed label Aug 14, 2026

@JAORMX JAORMX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One merge-blocking stale-plan race remains in the current head.

Comment thread pkg/plugins/pluginsvc/upgrade.go Outdated

@JAORMX JAORMX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed commit ace63c0 against plugins-lock/04-sync. No new line-level findings in this commit. Current failing checks are mostly GitHub action-download 429/503 failures; the stack also inherits blockers from earlier PRs.

@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from ace63c0 to 6eaf5cb Compare August 17, 2026 17:26
@samuv

samuv commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Update

Rebased onto the sync locking fixes and added local-store restorability in 6eaf5cb4c:

  • Local upgrades leave ResolvedReference empty (no stale remote pin beside a local digest)
  • Sync restores plain-source + empty-reference + digest pins from the local OCI store by exact digest
  • Missing digest → digest-missing without mutating DB/lock
  • Round-trip tests included

env -u TOOLHIVE_DEV go test ./pkg/plugins/pluginsvc/ -count=1 passes on this branch.

@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from 6eaf5cb to 90543bf Compare August 17, 2026 17:31
@samuv

samuv commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (8343851e8). All previously unanswered review threads have replies and are marked resolved — please take another look when convenient.

@JAORMX JAORMX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed commit 90543bf against the declared stacked base. No new line-level findings in this commit. The PR remains dependent on unresolved blockers in earlier stack PRs, and several current CI checks are failing.

@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from 90543bf to 60d1ea0 Compare August 18, 2026 08:20
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Aug 18, 2026
@samuv

samuv commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main over the lower-stack fixes; no code changes needed here this round.

samuv added 6 commits August 18, 2026 10:57
Re-resolve plugins: lock entries and install newer content via
thv ai-plugin upgrade and POST /plugins/upgrade.
Plain-name lock entries now resolve the same way Install does, so a
local rebuild is visible to upgrade. Also complete upgrade args from
lock entries and cover fail-on-changes in the plugin CLI e2e.
A bare local-store tag must not be rewritten as a Docker Hub
digest reference; apply the resolved layer bytes instead.
The DB should keep the local tag while the lock file keeps
the previous restorable pin instead of a Docker Hub rewrite.
A stale plan must not resurrect an uninstall or overwrite a
newer install of the same plugin.
Sync loads plain-source empty-reference pins by digest, and local
upgrades no longer keep a stale remote resolved reference.
@samuv
samuv force-pushed the plugins-lock/05-upgrade branch from 60d1ea0 to d97c97e Compare August 18, 2026 09:06
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants