fix: phase 1 pre-1.0 API cleanup - #1
Conversation
- Return ErrNotFound from Client.Get so errors.Is(err, ErrNotFound) works; Store.Get maps sql.ErrNoRows to internal.ErrEntryNotFound. - Open() returns Getwd errors instead of logging and os.Exit; join cwd with filepath.Join for the default store path. - Rename DEFAULT_NAMESPACE to DefaultNamespace. - LoadOrCreateKey returns (key, created, err); move new-key stderr notices to the CLI via loadKey(). BREAKING CHANGE: DEFAULT_NAMESPACE renamed to DefaultNamespace. BREAKING CHANGE: internal.LoadOrCreateKey now returns (key []byte, created bool, err error). Co-authored-by: Cursor <cursoragent@cursor.com>
| func loadKey() ([]byte, error) { | ||
| key, created, err := internal.LoadOrCreateKey() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if created { | ||
| notifyNewEncryptionKey(key) | ||
| } | ||
| return key, nil | ||
| } | ||
|
|
||
| func notifyNewEncryptionKey(key []byte) { | ||
| path, err := internal.KeyPath() | ||
| if err != nil { | ||
| return | ||
| } | ||
| fmt.Fprintf(os.Stderr, "envmagic: generated new encryption key at %s\n", path) | ||
| fmt.Fprintf(os.Stderr, "envmagic: key (base64): %s\n", base64.StdEncoding.EncodeToString(key)) | ||
| fmt.Fprintf(os.Stderr, "envmagic: You can display the key again later by running `envmagic key`.\n") | ||
| fmt.Fprintln(os.Stderr, "envmagic: BACK THIS FILE UP - without it, stored values cannot be decrypted.") | ||
| } |
There was a problem hiding this comment.
The critical "BACK THIS FILE UP" backup notification is silently suppressed if
KeyPath() returns an error inside notifyNewEncryptionKey. In practice the call is redundant (it was already invoked successfully inside LoadOrCreateKey), but structuring the function this way means a transient failure would swallow the warning entirely — right after a new, unbackedup key was written to disk. Passing the path from loadKey into notifyNewEncryptionKey eliminates the silent failure path.
| func loadKey() ([]byte, error) { | |
| key, created, err := internal.LoadOrCreateKey() | |
| if err != nil { | |
| return nil, err | |
| } | |
| if created { | |
| notifyNewEncryptionKey(key) | |
| } | |
| return key, nil | |
| } | |
| func notifyNewEncryptionKey(key []byte) { | |
| path, err := internal.KeyPath() | |
| if err != nil { | |
| return | |
| } | |
| fmt.Fprintf(os.Stderr, "envmagic: generated new encryption key at %s\n", path) | |
| fmt.Fprintf(os.Stderr, "envmagic: key (base64): %s\n", base64.StdEncoding.EncodeToString(key)) | |
| fmt.Fprintf(os.Stderr, "envmagic: You can display the key again later by running `envmagic key`.\n") | |
| fmt.Fprintln(os.Stderr, "envmagic: BACK THIS FILE UP - without it, stored values cannot be decrypted.") | |
| } | |
| func loadKey() ([]byte, error) { | |
| key, created, err := internal.LoadOrCreateKey() | |
| if err != nil { | |
| return nil, err | |
| } | |
| if created { | |
| p, _ := internal.KeyPath() // already succeeded inside LoadOrCreateKey | |
| notifyNewEncryptionKey(key, p) | |
| } | |
| return key, nil | |
| } | |
| func notifyNewEncryptionKey(key []byte, path string) { | |
| fmt.Fprintf(os.Stderr, "envmagic: generated new encryption key at %s\n", path) | |
| fmt.Fprintf(os.Stderr, "envmagic: key (base64): %s\n", base64.StdEncoding.EncodeToString(key)) | |
| fmt.Fprintf(os.Stderr, "envmagic: You can display the key again later by running `envmagic key`.\n") | |
| fmt.Fprintln(os.Stderr, "envmagic: BACK THIS FILE UP - without it, stored values cannot be decrypted.") | |
| } |
- Introduced keyCreated boolean in Client struct to indicate if a new key file was created during store opening. - Updated OpenWithPath to return the keyCreated status. - Added KeyCreated method to Client for external access to the key creation status. - Implemented a test to verify the keyCreated behavior during multiple store openings.
- Changed version variable to be set at link time, defaulting to "dev" for go run. - Added `--yes` flag to create a new .envmagic file without prompting, supporting non-interactive usage. - Updated findOrCreateStorePath to accept a command context for better integration with flags. - Modified shell initialization scripts to include new commands for improved usability. - Enhanced tests to verify the creation of .envmagic files in non-interactive scenarios.
| @@ -27,7 +31,7 @@ version: | |||
| fi | |||
|
|
|||
| tag: version | |||
| @VERSION=$$(go run ./cmd/envmagic --version | awk '{print $$NF}'); \ | |||
| @VERSION=$$(go run -ldflags "$(GO_LD_FLAGS)" ./cmd/envmagic --version | awk '{print $$NF}'); \ | |||
| git tag "$$VERSION" && echo "Tagged $$VERSION" | |||
There was a problem hiding this comment.
make tag / make release are broken by the switch to git describe
The version guard and tag target both derive VERSION from VERSION_LD = git describe --tags --always --dirty. This creates a catch-22 for every release:
- Before tagging —
git describereturns something likev0.4.0-3-gabcdef(orv0.4.0-3-gabcdef-dirty). Theversionguard passes ("tag not yet created"), andmake tagthen creates a garbage tagv0.4.0-3-gabcdefinstead of the intendedv0.5.0. - After tagging — if the developer manually creates
git tag v0.5.0first,git describereturnsv0.5.0, but theversionguard immediately fails with "Error: version v0.5.0 already exists", blockingmake tagentirely.
The old const version = "v0.4.0" approach worked because the developer bumped that constant to v0.5.0 before running make tag, and git describe was never involved. With git describe as the sole version source, there is no clean state where make tag / make release can produce a correct semver tag. GoReleaser itself is unaffected (it uses {{.Version}} from an existing tag), but the local make release workflow is currently unusable.
BREAKING CHANGE: DEFAULT_NAMESPACE renamed to DefaultNamespace.
BREAKING CHANGE: internal.LoadOrCreateKey now returns (key []byte, created bool, err error).
Greptile Summary
This PR performs a pre-1.0 API cleanup across the library and CLI: error handling is tightened (
ErrNotFoundnow works witherrors.Is),Open()returns errors instead of callingos.Exit,DEFAULT_NAMESPACEis renamed toDefaultNamespace, andLoadOrCreateKeygains acreatedreturn value so backup notices can be surfaced at the right layer.Store.Getmapssql.ErrNoRowstointernal.ErrEntryNotFound;Client.Getmaps that to the publicErrNotFound.Client.KeyCreated()is added so library callers can detect new-key events.--yes/ENVMAGIC_NONINTERACTIVE=1flag letssetandimportcreate a missing.envmagicwithout a prompt; the shell-init bypass list is updated.versionchanges from a hardcodedconstto a link-time injectedvarsourced fromgit describe, which breaks themake tag/make releaseworkflow (see inline comment).Confidence Score: 4/5
Safe to merge for all library and CLI changes; the Makefile release targets need attention before running make tag or make release.
The library and CLI changes are clean and well-tested. The broken path is the local make tag / make release workflow: VERSION_LD is derived from git describe, creating an unresolvable catch-22 where either git describe returns a commit-hash string (wrong tag gets created) or a clean tag (version guard rejects it as already existing). GoReleaser builds are unaffected.
Makefile - the version and tag targets need updating to account for the switch from a hardcoded version constant to git describe-derived versioning.
Important Files Changed
Reviews (3): Last reviewed commit: "feat: enhance envmagic with non-interact..." | Re-trigger Greptile