fix(s3): mv create-bucket override + configure picker credentials-file (Codex findings)#47
Merged
Merged
Conversation
…ntials-file Two Codex-review findings in the wizard work: - mv (High): picking an existing destination bucket after backing out of "+ Create new bucket…" still created and moved to the typed name. The new-bucket step's Resetter was a no-op, so a stale st.newDstBucket survived the skip and overrode the chosen bucket in finalizeMove. The Resetter now clears st.newDstBucket — correct here because (unlike the profile wizard) the name lives in its own field, not the one the dest-bucket Select writes. - configure (Medium): the profile picker listed profiles from the default/env credentials file, ignoring --credentials-file, while the save honored it. The picker's Loader now resolves opts.CredentialsFile. - docs: the README claimed read commands take --profile; only configure/show have a local one — others use --auth.profile / VERDA_PROFILE / `auth use`. Tests: TestMoveStepNewDestBucket_ResetterClears, TestBuildMoveFlow_BackOutOfCreateBucket (end-to-end back-navigation), TestFinalizeMove_CreatesNewBucket, TestConfigureProfilePicker_HonorsCredentialsFile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the stale hand-rolled index-loop guidance (mv no longer uses it) with the shared wizard-engine pattern: Flow + NewEngine, dynamic Loaders, the Resetter/ShouldSkip gotcha, the "+ Create new…" sentinel pattern, and testing flows with WithTestResults. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Fixes from the Codex review of the merged S3 wizard work (#46).
mv — wrong destination bucket (High)
In
verda s3 mv, choosing "+ Create new bucket…", typing a name, then pressing Esc back and picking an existing bucket still created and moved to the typed name. The new-bucket step'sResetterwas a no-op, so a stalest.newDstBucketsurvived the skip and overrode the chosen bucket infinalizeMove.Fix: the new-bucket step's
Resetternow clearsst.newDstBucket. (Unlike the profile wizard — where the no-op is correct because the name shares the bound variable — here the name lives in its own field, so clearing it is right.)configure — picker ignored --credentials-file (Medium)
The profile picker listed profiles from the default/env credentials file while the save honored
--credentials-file, so existing profiles in the explicit file weren't shown. The picker'sLoadernow resolvesopts.CredentialsFile.docs (Low)
The README claimed read commands accept
--profile; onlyconfigure/showdefine a local one. Corrected to--auth.profile/VERDA_PROFILE/verda auth usefor the read commands.Also
.ai/skills/new-command.md(replacing the stale hand-rolled-loop guidance), incl. the Resetter/ShouldSkip gotcha and the "+ Create new…" sentinel pattern.Tests
TestMoveStepNewDestBucket_ResetterClears,TestBuildMoveFlow_BackOutOfCreateBucket(drives the full back-navigation viaBackResult()),TestFinalizeMove_CreatesNewBucket,TestConfigureProfilePicker_HonorsCredentialsFile.make build/make test/make lint(incl. CI's golangci-lint v2.5.0) all pass.🤖 Generated with Claude Code