feat(wfctl): lockfile dep tracking + always-track plain installs (workflow#771)#772
Merged
Conversation
Extends resolveDependencies to call updateLockfileWithChecksum after each dep install (was: only parent tracked). Also drops the name@version gate so plain 'install <name>' captures resolved registry version. Closes #771 ACs.
Cycle 1 FAIL: 2 Critical (C1 early-return no-op on new-format dominant case; C2 dropped gate breaks installFromLockfile no-clobber contract). Cycle 2: - Refactor updateLockfileWithChecksum to fan out to BOTH legacy + new-format paths; preserve Platforms data on new-format merge. - Replace gate with installSkipLockfileUpdate package-level flag set+deferred by installFromLockfile (preserves no-clobber). - Document Repository field gap; defer fallback construction. - Test plan splits legacy + new-format scenarios; regression test for installFromLockfile invariant.
…tallPluginReqDirect) Cycle 2 FAIL: 1 Critical + 1 Important. Cycle 3: - C3: installSkipLockfileUpdate guard around installFromWfctlLockfile's runPluginInstall fallback call (mirror of cycle-2 fix for legacy installFromLockfile). - I4: append updateLockfileWithChecksum to installPluginReqDirect's parent install (closes user-intent asymmetry: --from-config deps were tracked, parents weren't). - Test plan §e + §f added for both regression coverage.
…URL call site)
…Checksum chokepoint) Cycle 4 FAIL: 1 Critical (gate at runPluginInstall:256 inert for line-86 installFromURL path which writes lockfile at 846). Cycle 5 adopts reviewer Option 1: gate INSIDE updateLockfileWithChecksum. Single chokepoint; all 5 call sites (256/846/934/dep-recursion/installPluginReqDirect) auto-respect. Resolves C1 + I6 together.
6 tasks, 1 PR. Chokepoint guard at top of updateLockfileWithChecksum + fan-out to both legacy + new-format lockfile + outer-frame guards in installFromLockfile/installFromWfctlLockfile + dep tracking in resolveDependencies + parent tracking in installPluginReqDirect.
Cycle 1 FAIL: 2 Critical. Cycle 2: - C1 OVERRIDDEN: line 846 is in installFromURL (760+), NOT installPluginFromManifest (325-431). installPluginFromManifest verified to NOT call updateLockfileWithChecksum. Tasks 4+5 ARE necessary; reviewer misread. Override documented inline. - C2 ACCEPTED: each task's Step 1.5 added — rewrite tests to invoke actual production entrypoints via httptest pattern (precedent at plugin_install_lockfile_test.go:38+). - Task 2 Step 4 wrong commentary corrected (test survives via mergeIntoNewFormatLockfile's preservation, not Task 3 guards). - Task 3 Step 3 anon-func wrap explicitly picked. - Task 1 var comment: t.Parallel() warning. - Task 5 variable names pre-resolved.
… normalized name) Cycle 2 FAIL: 2 Critical (test snippets use const assignment; anon-func wrapper breaks continue) + 4 Important. Cycle 3: - CYC2-C1: tests use os.Chdir(dir) pattern (precedent at existing tests:30/95/160) instead of reassigning const wfctlLockPath. - CYC2-C2: Task 3 installFromWfctlLockfile uses FUNCTION-SCOPE guard (set+defer at top of function), not anon-func-per-iter (illegal continue inside func literal). - CYC2-I1: Task 5 uses normalized pluginName (line 87) for both hash path AND lockfile key, not raw req.Name. - CYC2-I3: test name reference fixed to existing TestRunPluginInstall_DoesNotRewriteNewFormatLockfile.
…lization) Cycle 3 FAIL: 2 Critical (CYC3-C1: legacy Save wipes Platforms via PluginLockEntry lacking field; CYC3-C2: helper doesn't normalize keys → duplicate short/long-form entries). Cycle 4: - Mutually-exclusive format paths: if v1 lockfile present, write ONLY v1 (returns true; skip legacy). Legacy write only when v1 absent. Restores Platforms protection that the dropped early-return provided. - mergeIntoNewFormatLockfile returns bool + scans for normalized-equivalent existing key when exact match missing (handles real-world long-form lockfile keys). - Task 4 dep tracking normalizes depKey for both hash path AND lockfile key (CYC3-I1). - Task 3 installFromLockfile guard moved to function scope (CYC3-I2; mirrors installFromWfctlLockfile fix).
…d comment) Adversarial PASS cycle 4 with 3 micro-edit Importants. Apply inline: - I1 Option (b): Task 4 uses raw dep.Name for hash + lockfile key (matches un-normalized install at line 268). Reverts cycle-4's normalize-only-on-write asymmetry. - I3 Option (a): documentary note in installFromWfctlLockfile about dep auto-pin suppression under function-scope guard. - I2: dep entries always short-form (already documented behavior; existing runPluginInstall:257 pattern).
…ithChecksum (workflow#771 Task 1)
…te) (workflow#771 Task 2)
…(workflow#771 Task 3)
…s (workflow#771 Task 4)
…ReqDirect (workflow#771 Task 5)
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes workflow#771. Extends
wfctl plugin installto track transitively-installed deps in.wfctl-lock.yaml(both legacy + new-format) AND always-track plaininstall <name>(drops@versiongate). PreservesinstallFromLockfile/installFromWfctlLockfileno-clobber contracts via chokepoint guard insideupdateLockfileWithChecksum.Architecture
Per cycle-5 chokepoint pattern + cycle-4 mutually-exclusive format paths:
mergeIntoNewFormatLockfilehelper preservesPlatforms/Compatibility; handles real-world long-form keys via normalized-equivalent scan; returns bool to gate legacy fallback.updateLockfileWithChecksumchecksinstallSkipLockfileUpdateflag at TOP (single chokepoint covers all 5 call sites). V1 format takes precedence (returns after v1 write); legacy fallback only when v1 absent.installFromLockfile,installFromWfctlLockfile) set+defer-clear flag at function scope — protects against inner installs clobbering pinned entries before checksum verification.resolveDependencies+installPluginReqDirectnewly callupdateLockfileWithChecksum;installPluginFromManifest(the dep install path) doesn't write lockfile on its own.Test plan
GOWORK=off go test -count=1 -timeout 300s ./cmd/wfctl/...— PASS (113s)GOWORK=off go vet ./...— cleangolangci-lint run ./cmd/wfctl/...— 0 issueswfctl plugin install --help— help text correctDesign + plan
Iterated through 5 design cycles + 4 plan cycles + alignment-check + scope-lock:
docs/plans/2026-05-24-lockfile-deps-design.md(cycle 5 PASS adversarial)docs/plans/2026-05-24-lockfile-deps.md(cycle 4 PASS adversarial + alignment PASS + Locked 2026-05-24T08:57:59Z)Implementer deviations (minor)
TestInstallFromLockfile_NoClobberInvariantto FAIL pre-implementation; it PASSED becausemergeIntoNewFormatLockfile's preservation semantics (Source preserved when empty; Platforms never touched) make the inner write a no-op for the test's pinned data. Test remains as regression check; guard IS correctly installed.depainstead ofdepA(lowercase perverifyInstalledPluginname regex). No functional impact.Reviewer override (cycle 1 plan)
Plan-phase reviewer cycle 1 flagged Tasks 4+5 as redundant claiming line-846 is in
installPluginFromManifest. Verified factually wrong: line 846 lives insideinstallFromURL(lines 760+);installPluginFromManifestbody (325-431) contains zeroupdateLockfileWithChecksumcalls. Tasks 4+5 are necessary. Override documented in plan.Out of scope (deferred follow-ups)
wfctl plugin removelockfile cleanupRepositoryfallback URL construction for empty manifest entries--no-lockuser-facing opt-out flag