Repository navigation
fix(metadata): confirm deletion before resolving client and token - #214
Merged
Merged
Conversation
The metadata defaults, group, instance, and peer delete commands created the metadata service client and handled the access token before asking "Really delete?". Without a base URI or token they failed with an unrelated error instead of prompting, unlike the other delete commands. Prompt first, then create the client and handle the token, the same order as the boot, bss, cloud-init, and smd delete commands. Add the four commands to TestDeleteConfirm_DeclineBeforeClient. Fixes OpenCHAMI#188 Signed-off-by: anyingiit <49945850+anyingiit@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
synackd
approved these changes
Oct 23, 2026
10 of 17 tasks
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.
Requested by winshyon · project thread
Fixes #188
The four
metadata * deletecommands (defaults,group,instance,peer) created the metadata-service client inRunEand calledrt.HandleTokenat the top ofrunCore*. Both happened before the "Really delete?" prompt. With no base URI or token configured, they failed (exit 3) without ever prompting.This PR moves the client creation and the token handling after the confirmation block. That matches
runCoreBootBmcDeleteand the other delete commands already covered byTestDeleteConfirm_DeclineBeforeClient. TherunCore*functions no longer take the client as a parameter, the same as the boot delete commands. Nothing else in the repo calls them.Testing
metadata defaults/group/instance/peercases toTestDeleteConfirm_DeclineBeforeClientincmd/prompts_test.go.go test ./...: all packages pass.golangci-lint run ./...(v2.15.0, as in CI): 0 issues.The commit is signed off (DCO).
This change was prepared with AI assistance (Claude Code). I reviewed the diff and ran the tests above myself.
🤖 Generated with Claude Code