fix(critical): Codex review — 7 high-severity issues resolved

Critical:
- Result.Result() zero args returns receiver instead of panicking

High:
- i18n.SetLanguage: added mutex, forwards to translator
- embed.GetAsset: hold RLock through assets map read (race fix)
- cli.PrintHelp: safe type assertion on Translate result
- task.PerformAsync: guard nil task in reflect.TypeOf
- Service/Command registries initialised in New() (race fix)

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-20 17:20:08 +00:00
parent bc06480b58
commit f1bd36db2e
10 changed files with 59 additions and 42 deletions

View file

@ -121,8 +121,9 @@ func (cl *Cli) PrintHelp() {
if cmd.Hidden {
continue
}
desc := cl.core.I18n().Translate(cmd.I18nKey()).Value.(string)
if desc == cmd.I18nKey() {
tr := cl.core.I18n().Translate(cmd.I18nKey())
desc, _ := tr.Value.(string)
if desc == "" || desc == cmd.I18nKey() {
cl.Print(" %s", path)
} else {
cl.Print(" %-30s %s", path, desc)

View file

@ -130,10 +130,6 @@ type commandRegistry struct {
// c.Command("deploy", Command{Action: handler})
// r := c.Command("deploy")
func (c *Core) Command(path string, command ...Command) Result {
if c.commands == nil {
c.commands = &commandRegistry{commands: make(map[string]*Command)}
}
if len(command) == 0 {
c.commands.mu.RLock()
cmd, ok := c.commands.commands[path]

View file

@ -73,16 +73,18 @@ type ActionTaskCompleted struct {
// })
func New(opts ...Options) *Core {
c := &Core{
app: &App{},
data: &Data{},
drive: &Drive{},
fs: &Fs{root: "/"},
config: &Config{ConfigOptions: &ConfigOptions{}},
error: &ErrorPanic{},
log: &ErrorLog{log: defaultLog},
lock: &Lock{},
ipc: &Ipc{},
i18n: &I18n{},
app: &App{},
data: &Data{},
drive: &Drive{},
fs: &Fs{root: "/"},
config: &Config{ConfigOptions: &ConfigOptions{}},
error: &ErrorPanic{},
log: &ErrorLog{log: defaultLog},
lock: &Lock{},
ipc: &Ipc{},
i18n: &I18n{},
services: &serviceRegistry{services: make(map[string]*Service)},
commands: &commandRegistry{commands: make(map[string]*Command)},
}
if len(opts) > 0 {

View file

@ -70,11 +70,12 @@ func AddAsset(group, name, data string) {
func GetAsset(group, name string) Result {
assetGroupsMu.RLock()
g, ok := assetGroups[group]
assetGroupsMu.RUnlock()
if !ok {
assetGroupsMu.RUnlock()
return Result{}
}
data, ok := g.assets[name]
assetGroupsMu.RUnlock()
if !ok {
return Result{}
}

View file

@ -94,19 +94,30 @@ func (i *I18n) Translate(messageID string, args ...any) Result {
return Result{messageID, true}
}
// SetLanguage sets the active language. No-op if no translator is registered.
// SetLanguage sets the active language and forwards to the translator if registered.
func (i *I18n) SetLanguage(lang string) Result {
if lang != "" {
i.locale = lang
if lang == "" {
return Result{OK: true}
}
i.mu.Lock()
i.locale = lang
t := i.translator
i.mu.Unlock()
if t != nil {
if err := t.SetLanguage(lang); err != nil {
return Result{err, false}
}
}
return Result{OK: true}
}
// Language returns the current language code, or "en" if not set.
func (i *I18n) Language() string {
if i.locale != "" {
return i.locale
i.mu.RLock()
locale := i.locale
i.mu.RUnlock()
if locale != "" {
return locale
}
return "en"
}

View file

@ -40,9 +40,6 @@ func (c *Core) LockEnable(name ...string) {
}
c.Lock(n).Mutex.Lock()
defer c.Lock(n).Mutex.Unlock()
if c.services == nil {
c.services = &serviceRegistry{services: make(map[string]*Service)}
}
c.services.lockEnabled = true
}

View file

@ -50,21 +50,21 @@ type Result struct {
// r.Result(value) // OK = true, Value = value
// r.Result() // after set — returns the value
func (r Result) Result(args ...any) Result {
if len(args) == 0 {
return r
}
if len(args) == 1 {
return Result{args[0], true}
}
if len(args) >= 2 {
if err, ok := args[len(args)-1].(error); ok {
if err != nil {
return Result{err, false}
}
return Result{args[0], true}
if err, ok := args[len(args)-1].(error); ok {
if err != nil {
return Result{err, false}
}
return Result{args[0], true}
}
return Result{args[0], true}
}
// Option is a single key-value configuration pair.

View file

@ -38,10 +38,6 @@ type serviceRegistry struct {
// c.Service("auth", core.Service{OnStart: startFn})
// r := c.Service("auth")
func (c *Core) Service(name string, service ...Service) Result {
if c.services == nil {
c.services = &serviceRegistry{services: make(map[string]*Service)}
}
if len(service) == 0 {
c.Lock("srv").Mutex.RLock()
v, ok := c.services.services[name]

View file

@ -35,7 +35,12 @@ func (c *Core) PerformAsync(t Task) Result {
if e, ok := r.Value.(error); ok {
err = e
} else {
err = E("core.PerformAsync", Join(" ", "no handler found for task type", reflect.TypeOf(t).String()), nil)
taskType := reflect.TypeOf(t)
typeName := "<nil>"
if taskType != nil {
typeName = taskType.String()
}
err = E("core.PerformAsync", Join(" ", "no handler found for task type", typeName), nil)
}
}
c.ACTION(ActionTaskCompleted{TaskIdentifier: taskID, Task: t, Result: r.Value, Error: err})

View file

@ -202,8 +202,16 @@ func TestResult_Result_WithError_Bad(t *testing.T) {
assert.Equal(t, err, r.Value)
}
func TestResult_Result_ZeroArgs_Ugly(t *testing.T) {
assert.Panics(t, func() {
Result{}.Result()
})
func TestResult_Result_ZeroArgs_Good(t *testing.T) {
r := Result{"hello", true}
got := r.Result()
assert.Equal(t, "hello", got.Value)
assert.True(t, got.OK)
}
func TestResult_Result_ZeroArgs_Empty_Good(t *testing.T) {
r := Result{}
got := r.Result()
assert.Nil(t, got.Value)
assert.False(t, got.OK)
}