Files
yellowjacket/.planning/phases/02-backend-correctness/02-01-PLAN.md
T

221 lines
7.4 KiB
Markdown

---
phase: 02-backend-correctness
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
- backend/app.go
- backend/config/config.go
autonomous: true
requirements: [CORR-05, CORR-06, CORR-07]
must_haves:
truths:
- "Package-level startupErr variable no longer exists; startup errors are stored in a YellowJacketApp struct field"
- "Config files are written with 0o644 permissions"
- "MPRIS callback errors (Pause, Seek) appear in the application log instead of being silently discarded"
artifacts:
- path: "backend/app.go"
provides: "Startup error as struct field + MPRIS error logging"
contains: "startupErr error"
- path: "backend/config/config.go"
provides: "Secure config file permissions"
contains: "0o644"
key_links:
- from: "backend/app.go:OnStartup"
to: "backend/app.go:OnDomReady"
via: "yj.startupErr field (not package-level var)"
pattern: "yj\\.startupErr"
- from: "backend/app.go:MPRIS callbacks"
to: "yj.logger"
via: "Warn log on Pause/Seek error"
pattern: "yj\\.logger\\.Warn.*MPRIS"
---
<objective>
Fix three independent error handling gaps in the application shell and config layer: eliminate the package-level startupErr variable, secure config file permissions, and log MPRIS callback errors.
Purpose: Remove global mutable state (startupErr), prevent world-writable config files, and ensure MPRIS failures are observable in logs.
Output: Modified `backend/app.go` and `backend/config/config.go` with all three fixes applied.
</objective>
<execution_context>
@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md
@/home/caleb/.config/opencode/get-shit-done/templates/summary.md
</execution_context>
<context>
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/phases/02-backend-correctness/02-CONTEXT.md
@.planning/phases/02-backend-correctness/02-RESEARCH.md
@backend/app.go
@backend/config/config.go
<interfaces>
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
From backend/app.go:
```go
// YellowJacketApp is the main application struct for Wails.
type YellowJacketApp struct {
FEBindings []any
FrontendUtil *frontendutil.FrontendUtil
logger *slog.Logger
assetHandler *assets.Handler
database *database.DB
library *library.Library
player *player.Player
playlist *playlist.Service
queue *queue.Queue
mediaControls mediacontrols.Handler
appContext context.Context
appConfig *config.Config
}
var startupErr error // line 134 — TO BE REMOVED
func (yj *YellowJacketApp) OnStartup(ctx context.Context) // line 137 — uses startupErr
func (yj *YellowJacketApp) OnDomReady(ctx context.Context) // line 251 — checks startupErr
```
MPRIS callback closures at lines 181-203:
```go
OnPause: func() { _ = yj.player.Pause() },
OnPlayPause: func() {
if yj.player.IsPlaying() {
_ = yj.player.Pause()
} else {
yj.queue.Play()
}
},
OnStop: func() { _ = yj.player.Pause() },
OnSeek: func(positionSec int) {
_ = yj.player.Seek(positionSec)
},
```
</interfaces>
</context>
<tasks>
<task type="auto">
<name>Task 1: Move startupErr to struct field and fix config permissions</name>
<files>backend/app.go, backend/config/config.go</files>
<action>
**CORR-05 — Startup error struct field (backend/app.go):**
1. Add `startupErr error` field to the `YellowJacketApp` struct (after `appConfig`)
2. Delete the package-level `var startupErr error` declaration at line 134
3. In `OnStartup` (line 154-155): change `startupErr = errors.Join(startupErr, ...)` to `yj.startupErr = errors.Join(yj.startupErr, ...)`
4. In `OnDomReady` (line 252-254): change `if startupErr != nil` to `if yj.startupErr != nil`, and `startupErr.Error()` to `yj.startupErr.Error()`
5. Verify no other references to the package-level `startupErr` exist
**CORR-06 — Config permissions (backend/config/config.go):**
1. At line 152, change `os.FileMode(int(0o666))` to `0o644`
2. This is a single expression replacement — the `os.WriteFile` call signature stays the same
</action>
<verify>
<automated>cd /mnt/vault/dev/golang/yellowjacket && go vet ./backend/... && grep -q "startupErr error" backend/app.go && ! grep -q "^var startupErr" backend/app.go && grep -q "0o644" backend/config/config.go && ! grep -q "0o666" backend/config/config.go</automated>
</verify>
<done>Package-level startupErr is gone; YellowJacketApp has startupErr field; OnStartup and OnDomReady reference yj.startupErr; config.go writes with 0o644 permissions</done>
</task>
<task type="auto">
<name>Task 2: Log MPRIS callback errors</name>
<files>backend/app.go</files>
<action>
**CORR-07 — MPRIS callback error logging (backend/app.go):**
Replace the four MPRIS closures (lines 183-195) that discard errors with closures that log on failure. Use `Warn` level per research recommendation — these are non-fatal conditions. Keep inline closures (no named method extraction).
1. **OnPause** (line 183): Replace `func() { _ = yj.player.Pause() }` with:
```go
func() {
if err := yj.player.Pause(); err != nil {
yj.logger.Warn("MPRIS Pause failed", "err", err)
}
}
```
2. **OnPlayPause** (lines 184-189): Replace the `_ = yj.player.Pause()` inside the `if yj.player.IsPlaying()` branch:
```go
func() {
if yj.player.IsPlaying() {
if err := yj.player.Pause(); err != nil {
yj.logger.Warn("MPRIS PlayPause(pause) failed", "err", err)
}
} else {
yj.queue.Play()
}
}
```
3. **OnStop** (line 191): Replace `func() { _ = yj.player.Pause() }` with:
```go
func() {
if err := yj.player.Pause(); err != nil {
yj.logger.Warn("MPRIS Stop failed", "err", err)
}
}
```
4. **OnSeek** (lines 194-196): Replace `func(positionSec int) { _ = yj.player.Seek(positionSec) }` with:
```go
func(positionSec int) {
if err := yj.player.Seek(positionSec); err != nil {
yj.logger.Warn("MPRIS Seek failed", "err", err)
}
}
```
Ensure all four closures no longer use `_ =` to discard errors.
</action>
<verify>
<automated>cd /mnt/vault/dev/golang/yellowjacket && go vet ./backend/... && ! grep -q '_ = yj.player.Pause()' backend/app.go && ! grep -q '_ = yj.player.Seek' backend/app.go && grep -c 'MPRIS.*failed' backend/app.go | grep -q '^4$'</automated>
</verify>
<done>All four MPRIS callbacks (OnPause, OnPlayPause, OnStop, OnSeek) check errors and log at Warn level; no discarded errors remain in MPRIS closures</done>
</task>
</tasks>
<verification>
```bash
# All backend packages compile and pass vet
go vet ./backend/...
# No package-level startupErr
! grep -q "^var startupErr" backend/app.go
# Struct field exists
grep -q "startupErr error" backend/app.go
# Config permissions fixed
grep -q "0o644" backend/config/config.go
! grep -q "0o666" backend/config/config.go
# MPRIS errors logged (4 occurrences)
test "$(grep -c 'MPRIS.*failed' backend/app.go)" -eq 4
# No discarded player errors in MPRIS closures
! grep -q '_ = yj.player' backend/app.go
# Linting passes
golangci-lint run ./backend/...
```
</verification>
<success_criteria>
- `go vet ./backend/...` passes
- `golangci-lint run ./backend/...` passes
- Package-level `startupErr` variable eliminated
- Config file written with 0o644 permissions
- All four MPRIS callbacks log errors at Warn level
</success_criteria>
<output>
After completion, create `.planning/phases/02-backend-correctness/02-01-SUMMARY.md`
</output>