fix: address Codex review findings on PR #28

- WithOptions copies the Options slice (constructor isolation regression)
- WithService auto-discovers service name from package path via reflect
- WithService auto-registers HandleIPCEvents if present (v0.3.3 parity)
- Add test for failing option short-circuit in New()

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-24 16:59:33 +00:00
parent 9f6caa3c90
commit 2d017980dd
2 changed files with 55 additions and 3 deletions

View file

@ -6,6 +6,7 @@ package core
import (
"context"
"reflect"
)
// Message is the type for IPC broadcasts (fire-and-forget).
@ -120,8 +121,10 @@ func New(opts ...CoreOption) Result {
// core.WithOptions(core.Options{{Key: "name", Value: "myapp"}})
func WithOptions(opts Options) CoreOption {
return func(c *Core) Result {
c.options = &opts
if name := opts.String("name"); name != "" {
cp := make(Options, len(opts))
copy(cp, opts)
c.options = &cp
if name := cp.String("name"); name != "" {
c.app.Name = name
}
return Result{OK: true}
@ -138,7 +141,41 @@ func WithOptions(opts Options) CoreOption {
// core.WithService(display.Register(nil))
func WithService(factory func(*Core) Result) CoreOption {
return func(c *Core) Result {
return factory(c)
r := factory(c)
if !r.OK {
return r
}
// If the factory returned a service instance, auto-discover and register
if r.Value != nil {
instance := r.Value
// Service name discovery from package path
typeOf := reflect.TypeOf(instance)
if typeOf.Kind() == reflect.Ptr {
typeOf = typeOf.Elem()
}
pkgPath := typeOf.PkgPath()
parts := Split(pkgPath, "/")
name := Lower(parts[len(parts)-1])
if name != "" {
// IPC handler discovery
instanceValue := reflect.ValueOf(instance)
handlerMethod := instanceValue.MethodByName("HandleIPCEvents")
if handlerMethod.IsValid() {
if handler, ok := handlerMethod.Interface().(func(*Core, Message) Result); ok {
c.RegisterAction(handler)
}
}
// Register the service if not already registered by the factory
if sr := c.Service(name); !sr.OK {
c.Service(name, Service{})
}
}
}
return Result{OK: true}
}
}

View file

@ -64,6 +64,21 @@ func TestNew_WithServiceLock_Good(t *testing.T) {
assert.False(t, reg.OK)
}
func TestNew_WithService_Bad_FailingOption(t *testing.T) {
secondCalled := false
r := New(
WithService(func(c *Core) Result {
return Result{Value: E("test", "intentional failure", nil), OK: false}
}),
WithService(func(c *Core) Result {
secondCalled = true
return Result{OK: true}
}),
)
assert.False(t, r.OK)
assert.False(t, secondCalled, "second option should not run after first fails")
}
// --- Accessors ---
func TestAccessors_Good(t *testing.T) {