Skip to content

fix: phase 1 pre-1.0 API cleanup - #1

Open
peteraba wants to merge 3 commits into
mainfrom
chore/phase1-pre1-cleanup
Open

fix: phase 1 pre-1.0 API cleanup#1
peteraba wants to merge 3 commits into
mainfrom
chore/phase1-pre1-cleanup

Conversation

@peteraba

@peteraba peteraba commented May 14, 2026

Copy link
Copy Markdown
Owner
  • 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).

Greptile Summary

This PR performs a pre-1.0 API cleanup across the library and CLI: error handling is tightened (ErrNotFound now works with errors.Is), Open() returns errors instead of calling os.Exit, DEFAULT_NAMESPACE is renamed to DefaultNamespace, and LoadOrCreateKey gains a created return value so backup notices can be surfaced at the right layer.

  • API contracts fixed: Store.Get maps sql.ErrNoRows to internal.ErrEntryNotFound; Client.Get maps that to the public ErrNotFound. Client.KeyCreated() is added so library callers can detect new-key events.
  • Non-interactive mode: A new --yes / ENVMAGIC_NONINTERACTIVE=1 flag lets set and import create a missing .envmagic without a prompt; the shell-init bypass list is updated.
  • Release tooling: version changes from a hardcoded const to a link-time injected var sourced from git describe, which breaks the make tag / make release workflow (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

Filename Overview
Makefile Adds VERSION_LD from git describe and injects it via -ldflags; breaks make tag / make release because the version guard and tag creation form an unresolvable catch-22.
client.go Renames DEFAULT_NAMESPACE to DefaultNamespace, exposes KeyCreated(), maps internal.ErrEntryNotFound to ErrNotFound, and fixes Open() to use filepath.Join and return Getwd errors.
internal/key.go Changes LoadOrCreateKey to return (key, created, err) and removes the embedded stderr notice.
internal/store.go Introduces ErrEntryNotFound sentinel and maps sql.ErrNoRows to it, enabling errors.Is propagation through the stack.
cmd/envmagic/main.go Adds --yes/ENVMAGIC_NONINTERACTIVE flag, moves key-creation notification to notifyNewEncryptionKey, changes version from const to injected var.
cmd/envmagic/envfile.go Updates cmdImport to pass cmd into findOrCreateStorePath and use loadKey() instead of raw internal.LoadOrCreateKey.
cmd/envmagic/shellinit.go Adds import, rm, remove, and delete to the shell-init bypass list.
client_test.go New test file covering KeyCreated signal, ErrNotFound propagation, and Open() using the cwd store path.
cmd/envmagic/cli_test.go Adds setupBare helper and new tests for --yes / ENVMAGIC_NONINTERACTIVE create-on-first-use flows.
.goreleaser.yaml Adds -s -w stripping and -X=main.version injection; GoReleaser release builds will now report correct semver.

Fix All in Claude Code Fix All in Cursor

Reviews (3): Last reviewed commit: "feat: enhance envmagic with non-interact..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

- 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>
Comment thread cmd/envmagic/main.go
Comment on lines +451 to +471
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.")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Suggested change
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.")
}

Fix in Claude Code Fix in Cursor

Comment thread client.go Outdated
peteraba added 2 commits May 14, 2026 17:05
- 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.
Comment thread Makefile
Comment on lines 24 to 35
@@ -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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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 tagginggit describe returns something like v0.4.0-3-gabcdef (or v0.4.0-3-gabcdef-dirty). The version guard passes ("tag not yet created"), and make tag then creates a garbage tag v0.4.0-3-gabcdef instead of the intended v0.5.0.
  • After tagging — if the developer manually creates git tag v0.5.0 first, git describe returns v0.5.0, but the version guard immediately fails with "Error: version v0.5.0 already exists", blocking make tag entirely.

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.

Fix in Claude Code Fix in Cursor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant