Compare commits
12
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
bf0a53e64c | ||
|
|
7be4a02e31 | ||
|
|
ad9c25a5a2 | ||
|
|
a83a127e31 | ||
|
|
e049a71458 | ||
|
|
0c944f2382 | ||
|
|
75525b67e4 | ||
|
|
85768dc489 | ||
|
|
1a221a40d3 | ||
|
|
0821deb877 | ||
|
|
31ada14111 | ||
|
|
20139394f3 |
@@ -0,0 +1,107 @@
|
|||||||
|
name: Unclaim
|
||||||
|
|
||||||
|
# A `Closes #N` footer in a commit body closes the issue on merge — and
|
||||||
|
# leaves `Status/In Progress` on it, because Gitea's auto-close touches
|
||||||
|
# state and nothing else. So #100 was closed and simultaneously marked
|
||||||
|
# as being actively worked on, and `scripts/issue.sh close` (which does
|
||||||
|
# drop the label) is exactly the thing the footer exists to avoid
|
||||||
|
# calling.
|
||||||
|
#
|
||||||
|
# **This hooks the close, not the merge.** Stripping the label in the
|
||||||
|
# PR would work and would be a per-PR habit; habits are what the footer
|
||||||
|
# removed. `issues: [closed]` covers every path an issue can close by —
|
||||||
|
# the footer on merge, `issue.sh close`, someone clicking Close in the
|
||||||
|
# web UI — and asks nothing of anyone at any of them.
|
||||||
|
#
|
||||||
|
# **Reopening deliberately does not restore it.** Reopening says the
|
||||||
|
# work was not finished, not that somebody is at a keyboard doing it
|
||||||
|
# now; the claim gets re-made by whoever picks it up.
|
||||||
|
#
|
||||||
|
# **This is not instant, and should not be described as it.** The
|
||||||
|
# runner has capacity 1 and is shared with an index build that can hold
|
||||||
|
# it for three hours, so a label tweak can queue behind one. Stale for
|
||||||
|
# an afternoon beats stale forever, which is what it was.
|
||||||
|
#
|
||||||
|
# The audit that answers "is this still firing" stays in CLAUDE.md and
|
||||||
|
# is one command:
|
||||||
|
#
|
||||||
|
# ./scripts/issue.sh list --state closed --label "Status/In Progress"
|
||||||
|
#
|
||||||
|
# A workflow that silently stops working is the failure mode this whole
|
||||||
|
# area has already produced once.
|
||||||
|
|
||||||
|
on:
|
||||||
|
issues:
|
||||||
|
types: [closed]
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
unclaim:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
container:
|
||||||
|
image: ubuntu:24.04
|
||||||
|
|
||||||
|
steps:
|
||||||
|
- name: Drop the claim label
|
||||||
|
# **Inside a container the act runner selects `sh`, not bash**, so
|
||||||
|
# `set -o pipefail` fails the job on its second line with "Illegal
|
||||||
|
# option" and the step never reaches the API. `homebrew-formula.yml`
|
||||||
|
# carries the same `set -euo pipefail` without trouble because it
|
||||||
|
# runs with **no container**, on the host image where bash is the
|
||||||
|
# default — so "another workflow does it" is not evidence here.
|
||||||
|
shell: bash
|
||||||
|
env:
|
||||||
|
# The automatic Actions token, as release.yml uses for the
|
||||||
|
# floor tag. It needs no more than write access to this repo.
|
||||||
|
TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||||
|
API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
|
||||||
|
ISSUE: ${{ github.event.issue.number }}
|
||||||
|
run: |
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
# `ca-certificates` is named because `--no-install-recommends`
|
||||||
|
# skips it, and `ubuntu:24.04` ships no CA bundle of its own —
|
||||||
|
# so curl comes up unable to verify TLS against our own Gitea
|
||||||
|
# and fails with "error setting certificate file" (exit 77).
|
||||||
|
# Every other containerised workflow here spells it out for the
|
||||||
|
# same reason; this one did not, and cost a release cycle.
|
||||||
|
apt-get update -qq
|
||||||
|
apt-get install -y -qq --no-install-recommends \
|
||||||
|
ca-certificates curl jq >/dev/null
|
||||||
|
|
||||||
|
label_id=$(
|
||||||
|
curl -sSf -H "Authorization: token $TOKEN" "$API/labels?limit=100" |
|
||||||
|
jq -r '.[] | select(.name == "Status/In Progress") | .id'
|
||||||
|
)
|
||||||
|
|
||||||
|
# The label not existing is a repo somebody reorganised, not a
|
||||||
|
# failure of this run — say so and stop, rather than failing a
|
||||||
|
# job on every close from then on.
|
||||||
|
if [ -z "$label_id" ]; then
|
||||||
|
echo "unclaim: no 'Status/In Progress' label in this repo; nothing to do"
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
# DELETE is idempotent here: an issue that never carried the
|
||||||
|
# label answers the same as one that did, which is what makes
|
||||||
|
# this safe to run on *every* close rather than only the ones
|
||||||
|
# that were claimed.
|
||||||
|
# The body is captured, not discarded, so a refusal is
|
||||||
|
# diagnosable from this log alone. Whether the automatic
|
||||||
|
# token carries issue-write scope is still unproven, and
|
||||||
|
# "DELETE returned 403" without Gitea's own sentence costs
|
||||||
|
# another merge to find out which of the two it is.
|
||||||
|
body=$(mktemp)
|
||||||
|
code=$(
|
||||||
|
curl -sS -o "$body" -w '%{http_code}' -X DELETE \
|
||||||
|
-H "Authorization: token $TOKEN" \
|
||||||
|
"$API/issues/$ISSUE/labels/$label_id"
|
||||||
|
)
|
||||||
|
|
||||||
|
case "$code" in
|
||||||
|
204) echo "unclaim: #$ISSUE is closed and unclaimed" ;;
|
||||||
|
*)
|
||||||
|
echo "unclaim: DELETE returned $code for #$ISSUE" >&2
|
||||||
|
cat "$body" >&2
|
||||||
|
exit 1
|
||||||
|
;;
|
||||||
|
esac
|
||||||
-118
@@ -1,118 +0,0 @@
|
|||||||
# Work log
|
|
||||||
|
|
||||||
Temporal memory: what happened and what's next. Structure lives in
|
|
||||||
`CLAUDE.md`, operational instructions in `.pi/skills/yellowjacket-dev/`,
|
|
||||||
measured discoveries in `.planning/NOTES.md`. Don't duplicate those here.
|
|
||||||
|
|
||||||
## Current state
|
|
||||||
|
|
||||||
Plan 005 (agent development harness) is **complete — all seven
|
|
||||||
phases**. Everything from phase 1 onward is still **uncommitted**: one
|
|
||||||
large but coherent working-tree diff, nothing pushed.
|
|
||||||
|
|
||||||
All four tiers verified green from a cold, cleaned state:
|
|
||||||
`make ui-test` 313 passed, `make lint` 0 issues × 3 configurations,
|
|
||||||
`make test` green × 3 passes, `make e2e` 19 passed. Both CI jobs
|
|
||||||
verified green in a bare `ubuntu:24.04` container, including 19/19 on
|
|
||||||
WebKit.
|
|
||||||
|
|
||||||
**Committed and pushed** as `5ca6cad` (the harness) + `ccacd67` (a CI
|
|
||||||
fix), and **green on the real runner**: job `check` ~4 min, job `e2e`
|
|
||||||
~3 min with 19/19 chromium *and* 19/19 webkit. One commit rather than
|
|
||||||
seven because the working tree was the end state, not per-phase
|
|
||||||
snapshots — `Makefile`, `CLAUDE.md` and `lefthook.yml` are touched by
|
|
||||||
nearly every phase, so a split would have been fabricated history.
|
|
||||||
|
|
||||||
Still unverified, because no run has failed yet: the
|
|
||||||
`actions/upload-artifact` step (`continue-on-error`, so it cannot mask
|
|
||||||
a real failure) and whether pnpm honours `npm_config_store_dir` for
|
|
||||||
store caching. Worth checking the next time a spec legitimately fails.
|
|
||||||
|
|
||||||
- [ ] `gitea_ci`'s `job_logs` returns 404 on Gitea 1.27.1 — the endpoint
|
|
||||||
is not exposed. Logs come from the VPS instead: `zstdcat` the file
|
|
||||||
under `gitea/actions_log/<owner>/<repo>/<xx>/<task_id>.log.zst`,
|
|
||||||
and note `zstdcat` is not in the gitea container, so
|
|
||||||
`docker cp` it out first. Job status is `action_run_job.status`
|
|
||||||
(1 success, 2 failure, 4 skipped, 5 waiting, 6 running).
|
|
||||||
Probably belongs in the `gitea` skill, not here.
|
|
||||||
|
|
||||||
Open items deliberately not fixed: WAV tags are write-only
|
|
||||||
(`TestWAVTagsAreNotReadableYet`), `themeStore.loadFromBackend`'s failure
|
|
||||||
handler cannot recover, `backend/playlist` has no CRUD suite.
|
|
||||||
|
|
||||||
## Log
|
|
||||||
|
|
||||||
### 2026-08-11 — cold skill run, then phase 7 (CI)
|
|
||||||
|
|
||||||
- **Followed the skill cold first**, as the last session asked. It
|
|
||||||
works: app up from a wiped `.dev/`, an undocumented flow driven
|
|
||||||
(queue panel + shuffle, asserted on `QueueModeChanged`), stopped —
|
|
||||||
~1 minute, no dead ends. One real config bug: `outputDir` in
|
|
||||||
`.playwright/cli.config.json` resolves against **cwd**, not the
|
|
||||||
config file's directory (only `initScript` does that), so snapshots
|
|
||||||
were landing above the repo and a *stale* one from the previous
|
|
||||||
session answered `ls -t` instead. That cost a DOM walk to disprove a
|
|
||||||
regression that did not exist. Four smaller doc gaps fixed
|
|
||||||
(`sandbox-seed` already runs `testdata`; `ui-setup`/`e2e-setup` were
|
|
||||||
undocumented prerequisites; `snapshot` prints a path; `dev-stop`
|
|
||||||
leaves the browser open), plus `dev-headless.sh`'s own banner, which
|
|
||||||
was suggesting the bare `window.go` call its next paragraph warns
|
|
||||||
against.
|
|
||||||
- **Built both CI jobs as container scripts before writing any YAML**,
|
|
||||||
then transcribed the YAML back out and re-ran it to prove the
|
|
||||||
transcription. Push-and-see is a bad loop on a self-hosted runner.
|
|
||||||
- **It found a real bug immediately**: `make lint` omitted
|
|
||||||
`webkit2_41` on all three passes, so it was linting configurations
|
|
||||||
nothing builds. Invisible on Arch (which still ships
|
|
||||||
`webkit2gtk-4.0.pc`), fatal on Ubuntu 24.04. Tag sets now match
|
|
||||||
`make test`.
|
|
||||||
- **Both open decisions settled by measurement**: ALSA `null` PCM for
|
|
||||||
audio (no daemon; the elapsed clock really advances), dead-address
|
|
||||||
stub for the explore artifact (and setting it for the *app* run, not
|
|
||||||
just seeding, is worth 8x on suite wall clock). **WebKit is a
|
|
||||||
required step** — it had never been run anywhere, so one throwaway
|
|
||||||
container run replaced a coin flip with 19/19 at +11 s.
|
|
||||||
|
|
||||||
### 2026-08-10 — phase 6, pi affordances
|
|
||||||
|
|
||||||
- Added `.pi/skills/yellowjacket-dev/` as a directory rather than a flat
|
|
||||||
file: only the description is always in context, so `SKILL.md` stays
|
|
||||||
short enough that reading it whole is never a decision, and the deeper
|
|
||||||
material sits in `references/{harness,fixtures,ui-tier,schema-change}.md`.
|
|
||||||
- Settled the CLAUDE.md-vs-skill split **grammatically, not topically**,
|
|
||||||
because a topical split is what rots — every new fact gets two
|
|
||||||
plausible homes. Three docs, three tenses: NOTES.md is past
|
|
||||||
(measured, dated, append-only), CLAUDE.md is present (what the system
|
|
||||||
is), the skill is imperative (what to run). A new paragraph's tense
|
|
||||||
decides where it goes.
|
|
||||||
- The five gotchas (binding timeouts, first-run wizard, `pkill -f`,
|
|
||||||
seeds-by-running, WebKit-is-CI-only) went **inline in SKILL.md**, not
|
|
||||||
into a reference: you need them before the failure, not after.
|
|
||||||
- Trimmed CLAUDE.md's "Fixtures and the headless harness" section by
|
|
||||||
about half — the command sequences and gotchas it was carrying are now
|
|
||||||
the skill's, and leaving both would have created exactly the duplicate
|
|
||||||
description this repo has a standing rule against.
|
|
||||||
- Added `make skill-check` / `scripts/skill-check.sh` + a pre-commit
|
|
||||||
hook: every command in `.pi/**/*.md` must be a real `make` target, so
|
|
||||||
the Makefile stays the source of truth for invocation and a renamed
|
|
||||||
target fails a commit instead of misleading an agent later. Verified
|
|
||||||
it fails (it caught its own not-yet-created target) and passes.
|
|
||||||
- Added the `/e2e` prompt template: promoting a hand-driven
|
|
||||||
`playwright-cli` session into a spec is a transcription with four
|
|
||||||
fixed substitutions (refs → testids, sleeps → `waitForEvent`, raw
|
|
||||||
`window.go` → `callBinding`, short fixture → `LONG_TRACK`), plus three
|
|
||||||
runs — pass, pass again, pass after a DB restore — because the usual
|
|
||||||
failure is a spec depending on state the hand-driving left behind.
|
|
||||||
- One shell trap: under `set -euo pipefail`, `x="$(make -pqRr | …)"`
|
|
||||||
fails the whole assignment, because `make -q` exits non-zero when a
|
|
||||||
target is out of date and `pipefail` propagates it.
|
|
||||||
|
|
||||||
### Earlier
|
|
||||||
|
|
||||||
Phases 1–5 of plan 005: fixture generator and manifest, headless launch
|
|
||||||
and seeds, the event bridge + `data-testid` pass + `backend/testctl` +
|
|
||||||
`e2e/`, the Vitest component tier + `make bindings-check`, and the
|
|
||||||
`events.Emit` wrapper with its in-process service-event tests. Recaps
|
|
||||||
and the five "verified end to end" blocks are in
|
|
||||||
`.planning/plans/active/005-agent-development-harness.md`; the lessons
|
|
||||||
are in `.planning/NOTES.md`.
|
|
||||||
@@ -53,14 +53,50 @@ reinvention:
|
|||||||
- **Hard blockers are real Gitea dependencies**, which render on the
|
- **Hard blockers are real Gitea dependencies**, which render on the
|
||||||
issue itself, and the blocked issue carries `Status/Blocked`.
|
issue itself, and the blocked issue carries `Status/Blocked`.
|
||||||
- **A PR body carries a commit-to-issue table, the verification
|
- **A PR body carries a commit-to-issue table, the verification
|
||||||
actually run, and a `Closes` list** — PR #83 is the shape.
|
actually run, and a `Closes` list** — PR #83 is the shape. That list
|
||||||
|
is for whoever reads the PR; what actually closes an issue is the
|
||||||
|
footer below.
|
||||||
|
|
||||||
**And the `Closes` list does not reliably close anything.** #83 listed
|
**The closing keyword goes in the commit body, one issue per line.**
|
||||||
ten and five of them stayed open, shipped in `main`, for a fortnight.
|
|
||||||
So closing is a step you take and check, not a keyword you trust:
|
```
|
||||||
`./scripts/issue.sh close <n>` after the merge, with a comment naming
|
docs: delete four documents that contradict the code
|
||||||
the commit that shipped it. `close` also drops `Status/In Progress`,
|
|
||||||
because a claim outlives the work if nothing takes the label off.
|
<body>
|
||||||
|
|
||||||
|
Closes #98
|
||||||
|
```
|
||||||
|
|
||||||
|
**Gitea parses commit messages that reach `main`; it does not parse the
|
||||||
|
PR body**, which only closes anything if the merge happens to copy it
|
||||||
|
into the merge commit. Both halves of that were measured. #83's merge
|
||||||
|
commit carried `Closes #9, #13, #14, …` and closed **five of ten** — a
|
||||||
|
comma list is partially matched. #93's merge commit body was one
|
||||||
|
`Reviewed-on:` trailer, so #92 stayed open behind a perfectly correct
|
||||||
|
`Closes` line in the PR description.
|
||||||
|
|
||||||
|
A footer costs nothing elsewhere: Conventional Commits allows one,
|
||||||
|
`scripts/commit-check.sh` only regexes the subject, and
|
||||||
|
semantic-release reads the type from the subject — so this changes no
|
||||||
|
release decision. The rule that the issue number stays out of the
|
||||||
|
**subject** is unaffected, and was never about the body.
|
||||||
|
|
||||||
|
**Check it anyway.** A squash, or a merge message edited by hand,
|
||||||
|
still drops the footer. `./scripts/issue.sh list --state open` after a
|
||||||
|
merge, looking for what you just shipped; `./scripts/issue.sh close
|
||||||
|
<n>` for whatever did not take, with a comment naming the commit.
|
||||||
|
|
||||||
|
**Unclaiming is automatic, and it is hooked to the close rather than
|
||||||
|
to the merge.** Gitea's auto-close changes state and nothing else, so a
|
||||||
|
footer left `Status/In Progress` on a closed issue — #100 was closed
|
||||||
|
and marked as being actively worked on at the same time.
|
||||||
|
`.gitea/workflows/unclaim.yml` runs on `issues: [closed]`, which covers
|
||||||
|
the footer, `issue.sh close` and a click in the web UI alike; stripping
|
||||||
|
the label in the PR instead would have been a per-PR habit, and habits
|
||||||
|
are what the footer removed. It is not instant — the runner has
|
||||||
|
capacity 1 — and reopening deliberately does not restore the label.
|
||||||
|
`./scripts/issue.sh list --state closed --label "Status/In Progress"`
|
||||||
|
is how you find out it has stopped firing.
|
||||||
|
|
||||||
## Planning
|
## Planning
|
||||||
|
|
||||||
@@ -2150,10 +2186,11 @@ Pre-commit runs vet, lint, codegen check, and frontend typecheck in parallel. Pr
|
|||||||
|
|
||||||
## CI
|
## CI
|
||||||
|
|
||||||
Seven workflows in `.gitea/workflows/`. Five of them package and
|
Eight workflows in `.gitea/workflows/`. Five of them package and
|
||||||
publish (`arch-package`, `homebrew-formula`, `index-artifact`,
|
publish (`arch-package`, `homebrew-formula`, `index-artifact`,
|
||||||
`android-apk`, `desktop-assets`); `release.yml` decides *whether* four of
|
`android-apk`, `desktop-assets`); `release.yml` decides *whether* four of
|
||||||
those run at all; only `ci.yml` gates, and it is the one to look at when
|
those run at all; `unclaim.yml` is housekeeping on the tracker and
|
||||||
|
touches no code; only `ci.yml` gates, and it is the one to look at when
|
||||||
deciding whether a push was healthy.
|
deciding whether a push was healthy.
|
||||||
|
|
||||||
**`release.yml` is the entry point for all of it.** On every push to
|
**`release.yml` is the entry point for all of it.** On every push to
|
||||||
|
|||||||
@@ -106,5 +106,8 @@ make dev # run with hot-reload
|
|||||||
make build-prod # produce a release binary
|
make build-prod # produce a release binary
|
||||||
```
|
```
|
||||||
|
|
||||||
More detail for contributors lives in
|
More detail for contributors lives in [`CLAUDE.md`](./CLAUDE.md) — the
|
||||||
[`docs/dev/overview.md`](./docs/dev/overview.md) and [`CLAUDE.md`](./CLAUDE.md).
|
architecture, the conventions and the reasons behind them. What is
|
||||||
|
being worked on is [the issue
|
||||||
|
tracker](https://git.ljones.me/yonlu/yellowjacket/issues); #73 is the
|
||||||
|
roadmap.
|
||||||
|
|||||||
@@ -1,194 +0,0 @@
|
|||||||
# Config Improvement Suggestions
|
|
||||||
|
|
||||||
Remaining suggestions for improving the configuration system in YellowJacket.
|
|
||||||
|
|
||||||
## 2. Thread Safety Concerns
|
|
||||||
|
|
||||||
The current `Config` struct lacks synchronization:
|
|
||||||
- `Load()` and `Save()` can race with concurrent reads
|
|
||||||
- `handleConfigUpdate()` in library mutates `l.conf.DirectoryPath` without locks
|
|
||||||
|
|
||||||
**Suggestion:** Add a `sync.RWMutex` to protect config access, especially if config is read during scans.
|
|
||||||
|
|
||||||
```go
|
|
||||||
type Config struct {
|
|
||||||
mu sync.RWMutex
|
|
||||||
ctx context.Context
|
|
||||||
logger *slog.Logger
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
|
|
||||||
func (c *Config) Load() error {
|
|
||||||
c.mu.Lock()
|
|
||||||
defer c.mu.Unlock()
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 3. Nil Safety in Validation
|
|
||||||
|
|
||||||
In `config.go`, validation only runs if `c.Library != nil`, but `handleConfigPost` dereferences `postedConfig.Library` without checking for nil:
|
|
||||||
|
|
||||||
```go
|
|
||||||
if postedConfig.Library != nil {
|
|
||||||
c.Library = postedConfig.Library
|
|
||||||
// ...
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
**Status:** Partially addressed in the event refactor, but consider adding explicit nil checks in `Validate()` as well.
|
|
||||||
|
|
||||||
## 4. Inconsistent Error Handling on HTTP Responses
|
|
||||||
|
|
||||||
In `httphandler.go:28-31`, `WriteHeader` is called *after* rendering the error template, which won't work as expected (headers must be set before writing body):
|
|
||||||
|
|
||||||
```go
|
|
||||||
c.formSubmitError(err.Error()).Render(r.Context(), w)
|
|
||||||
w.WriteHeader(http.StatusInternalServerError) // Too late!
|
|
||||||
```
|
|
||||||
|
|
||||||
**Fix:** Set the status code before rendering:
|
|
||||||
|
|
||||||
```go
|
|
||||||
w.WriteHeader(http.StatusInternalServerError)
|
|
||||||
c.formSubmitError(err.Error()).Render(r.Context(), w)
|
|
||||||
```
|
|
||||||
|
|
||||||
## 5. Make `scanWorkerCount` Configurable
|
|
||||||
|
|
||||||
There's a TODO at `library.go:289`:
|
|
||||||
```go
|
|
||||||
// TODO: make configurable via Config.
|
|
||||||
var scanWorkerCount = goruntime.NumCPU()
|
|
||||||
```
|
|
||||||
|
|
||||||
**Suggestion:** Add this to `library.Config`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type Config struct {
|
|
||||||
DirectoryPath Directory `form:"Directory" schema:"directory,required"`
|
|
||||||
ScanWorkers int `form:"ScanWorkers" schema:"scan_workers"`
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Then in `NewLibrary()` or `Scan()`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
workers := l.conf.ScanWorkers
|
|
||||||
if workers <= 0 {
|
|
||||||
workers = goruntime.NumCPU()
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 6. Consider Config Defaults
|
|
||||||
|
|
||||||
Currently if no config exists, an empty one is saved. Consider providing sensible defaults (e.g., common music directories like `~/Music`).
|
|
||||||
|
|
||||||
```go
|
|
||||||
func (c *Config) setDefaults() {
|
|
||||||
if c.Library == nil {
|
|
||||||
c.Library = &library.Config{}
|
|
||||||
}
|
|
||||||
if c.Library.DirectoryPath == "" {
|
|
||||||
// Try common music directories
|
|
||||||
home, _ := os.UserHomeDir()
|
|
||||||
musicDir := filepath.Join(home, "Music")
|
|
||||||
if info, err := os.Stat(musicDir); err == nil && info.IsDir() {
|
|
||||||
c.Library.DirectoryPath = library.Directory(musicDir)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 7. Config Reload/Watch Capability
|
|
||||||
|
|
||||||
The config is only loaded at startup. Consider adding:
|
|
||||||
- File watcher for external config changes (using `fsnotify`)
|
|
||||||
- Explicit reload method callable from UI
|
|
||||||
|
|
||||||
```go
|
|
||||||
func (c *Config) Watch() error {
|
|
||||||
watcher, err := fsnotify.NewWatcher()
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
go func() {
|
|
||||||
for event := range watcher.Events {
|
|
||||||
if event.Op&fsnotify.Write == fsnotify.Write {
|
|
||||||
c.Load()
|
|
||||||
// Emit event for listeners
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}()
|
|
||||||
|
|
||||||
return watcher.Add(c.filePath)
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 8. Validation Should Return Structured Errors
|
|
||||||
|
|
||||||
Currently validation returns combined errors. Consider returning a structured validation result that the UI can map to specific fields for better user feedback.
|
|
||||||
|
|
||||||
```go
|
|
||||||
type ValidationError struct {
|
|
||||||
Field string
|
|
||||||
Message string
|
|
||||||
}
|
|
||||||
|
|
||||||
type ValidationResult struct {
|
|
||||||
Valid bool
|
|
||||||
Errors []ValidationError
|
|
||||||
}
|
|
||||||
|
|
||||||
func (c *Config) ValidateStructured() ValidationResult {
|
|
||||||
var result ValidationResult
|
|
||||||
result.Valid = true
|
|
||||||
|
|
||||||
if c.Library != nil {
|
|
||||||
if err := c.Library.Validate(); err != nil {
|
|
||||||
result.Valid = false
|
|
||||||
result.Errors = append(result.Errors, ValidationError{
|
|
||||||
Field: "Library.DirectoryPath",
|
|
||||||
Message: err.Error(),
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return result
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
## 9. Use Standard Library for Config Paths
|
|
||||||
|
|
||||||
The path construction in `system/userdata.go` doesn't respect `$XDG_CONFIG_HOME` on Linux or use the standard Go `os.UserConfigDir()`.
|
|
||||||
|
|
||||||
**Current implementation:**
|
|
||||||
```go
|
|
||||||
case "linux":
|
|
||||||
return fmt.Sprintf("/home/%s/%s/yellowjacket", username, unixSubdirs[dt]), nil
|
|
||||||
```
|
|
||||||
|
|
||||||
**Suggested improvement:**
|
|
||||||
```go
|
|
||||||
func GetUserConfigDirPath() (string, error) {
|
|
||||||
baseDir, err := os.UserConfigDir() // Respects XDG_CONFIG_HOME
|
|
||||||
if err != nil {
|
|
||||||
return "", fmt.Errorf("could not get user config directory: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
path := filepath.Join(baseDir, "yellowjacket")
|
|
||||||
|
|
||||||
if err := os.MkdirAll(path, 0o755); err != nil {
|
|
||||||
return "", fmt.Errorf("could not create config directory: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
return path, nil
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
This approach:
|
|
||||||
- Respects `$XDG_CONFIG_HOME` on Linux
|
|
||||||
- Uses proper macOS paths (`~/Library/Application Support`)
|
|
||||||
- Uses `%AppData%` on Windows
|
|
||||||
- Is more portable and follows platform conventions
|
|
||||||
@@ -1,53 +0,0 @@
|
|||||||
# Development Overview
|
|
||||||
|
|
||||||
YellowJacket is a moderately complex application. This document gives an overview of how development of it works.
|
|
||||||
|
|
||||||
## Logical Breakdown
|
|
||||||
|
|
||||||
YellowJacket can be thought about in a heirarchy of logical modules and components. The borders of these logical sections are mostly represented in the code and directory structure as well.
|
|
||||||
|
|
||||||
- Frontend
|
|
||||||
- UI Components (see [Lit](###lit-web-components))
|
|
||||||
- Backend
|
|
||||||
- App
|
|
||||||
- Asset Handler
|
|
||||||
- Logging
|
|
||||||
- System
|
|
||||||
- Player
|
|
||||||
- Library
|
|
||||||
- Config
|
|
||||||
- Database
|
|
||||||
- Queries (see [sqlc](###sqlc))
|
|
||||||
|
|
||||||
## Dependencies
|
|
||||||
|
|
||||||
YellowJacket uses many tools and libraries to provide its functionality.
|
|
||||||
This section lists each of these dependencies and explains how they are used.
|
|
||||||
|
|
||||||
### [Wails](https://wails.io)
|
|
||||||
|
|
||||||
Used to create desktop apps with Go and web technologies.
|
|
||||||
|
|
||||||
### [SQLite](https://github.com/mattn/go-sqlite3?tab=readme-ov-file#go-sqlite3)
|
|
||||||
|
|
||||||
Used for local database.
|
|
||||||
|
|
||||||
### [sqlc](https://sqlc.dev/)
|
|
||||||
|
|
||||||
Used to generate Go code from SQL.
|
|
||||||
|
|
||||||
### [Templ](https://templ.guide/)
|
|
||||||
|
|
||||||
Used to generate HTML templates with Go code.
|
|
||||||
|
|
||||||
### [Beep](https://github.com/gopxl/beep?tab=readme-ov-file#beep)
|
|
||||||
|
|
||||||
Used for audio playback.
|
|
||||||
|
|
||||||
### [Lit Web Components](https://lit.dev/)
|
|
||||||
|
|
||||||
Used for dynamic/reactive frontend components.
|
|
||||||
|
|
||||||
### [HTMX](https://htmx.org/)
|
|
||||||
|
|
||||||
Used for requesting HTML fragments from the backend and rendering them on the frontend.
|
|
||||||
-1648
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user