fix(wfctl): preserve required_secrets through install path#748
Merged
Conversation
`wfctl secrets setup --plugin <name>` reads required_secrets[] from the on-disk plugin.json. Two struct gaps were dropping the field along the install path: 1. cmd/wfctl/plugin_registry.go: RegistryManifest had no RequiredSecrets field, so json.Unmarshal silently discarded the upstream block at fetch time. 2. cmd/wfctl/plugin_install.go: installedPluginJSON likewise had no RequiredSecrets field, so writeInstalledManifest couldn't carry it through even if (1) was fixed. Both filled in with the existing PluginRequiredSecret type from secrets_setup_plugin.go. writeInstalledManifest now copies m.RequiredSecrets → pj.RequiredSecrets so the on-disk plugin.json preserves the block. Observed in practice via workflow-plugin-hover v0.2.0: - registry manifest at plugins/hover/manifest.json had required_secrets[3] - installed plugin.json at data/plugins/hover/plugin.json had no required_secrets field - wfctl secrets setup --plugin hover --scope repo reported "declares no required_secrets[]; nothing to do" Regression tests: - TestRegistryManifest_UnmarshalPreservesRequiredSecrets - TestWriteInstalledManifest_PreservesRequiredSecrets Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression in wfctl where plugin required_secrets[] declared in registry manifest.json were silently dropped during fetch/install, causing wfctl secrets setup --plugin <name> to incorrectly report there are no required secrets.
Changes:
- Add
RequiredSecrets []PluginRequiredSecrettoRegistryManifestso registry manifest unmarshalling preservesrequired_secrets. - Add
RequiredSecrets []PluginRequiredSecretto the installedplugin.jsonshape and copy it inwriteInstalledManifest. - Add regression tests covering both the unmarshal path and the installed-manifest write path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/wfctl/plugin_registry.go | Extends RegistryManifest to retain required_secrets from registry manifest.json. |
| cmd/wfctl/plugin_install.go | Extends installed plugin.json struct + ensures writeInstalledManifest copies RequiredSecrets. |
| cmd/wfctl/plugin_install_test.go | Adds regression tests ensuring required_secrets survives both unmarshal and disk-write paths. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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
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
`wfctl secrets setup --plugin ` reads `required_secrets[]` from the on-disk `plugin.json`. Two struct gaps were dropping the field along the install path:
Both filled in with the existing `PluginRequiredSecret` type from `secrets_setup_plugin.go`. `writeInstalledManifest` now copies `m.RequiredSecrets → pj.RequiredSecrets`.
How this surfaced
`workflow-plugin-hover@v0.2.0` declares 3 required secrets (`HOVER_USERNAME`, `HOVER_PASSWORD`, `HOVER_TOTP_SECRET`). After:
```
wfctl plugin install
wfctl secrets setup --plugin hover --scope repo
```
→ `plugin "workflow-plugin-hover" declares no required_secrets[]; nothing to do`
`jq .required_secrets data/plugins/hover/plugin.json` returned `null`, even though `raw.githubusercontent.com/.../plugins/hover/manifest.json` had the full array.
Tests
Both fail on `main` without these struct changes.
Test plan
🤖 Generated with Claude Code