Compare commits
15
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e897364a73 | ||
|
|
5fa68fcf43 | ||
|
|
b1368bbc7e | ||
|
|
9432f68c8b | ||
|
|
4c921ed1ba | ||
|
|
f8800ca1f8 | ||
|
|
dddc8aaf55 | ||
|
|
f6e9df2f68 | ||
|
|
47f65dad89 | ||
|
|
f5dae71050 | ||
|
|
b0bda625e0 | ||
|
|
b9e60bdb0a | ||
|
|
bcf3856b6f | ||
|
|
d225f922fb | ||
|
|
26251badda |
@@ -168,8 +168,12 @@ Merge when, and only when, **all** hold:
|
||||
`block_on_outdated_branch: true` refuses it anyway; never
|
||||
`force_manually_merged` around it.
|
||||
|
||||
**Refresh before every merge.** In the loop worktree: fetch, then
|
||||
`git merge origin/main` on the PR branch, push. A textual conflict
|
||||
**Refresh before every merge.** In the loop worktree: `git fetch origin`
|
||||
in the same breath, then `git merge origin/main` on the PR branch,
|
||||
push. The fetch must be immediate — a cached `origin/main` merges
|
||||
against the wrong base, CI goes green on it, and the merge comes back
|
||||
405 "behind base", one whole CI cycle wasted (measured on the adoption
|
||||
wave). A textual conflict
|
||||
stops the leg there — as diff text, not as a failed merge click: hunks
|
||||
the loop authored are resolved by the loop; anything else is left with
|
||||
`⟦loop⟧` comment for a human, never forced. After any refresh push,
|
||||
@@ -247,7 +251,10 @@ AVD), then `make android-emulator` per session.
|
||||
|
||||
- **Worktree:** `git worktree add ~/.paseo/worktrees/loop/jumpy-hound
|
||||
origin/main` (from any clone; branch from origin/main in the loop
|
||||
tree, never `git checkout main`).
|
||||
tree, never `git checkout main`). **Provision it once before the
|
||||
first push:** `make build-frontend` and `make testdata` — the pre-push
|
||||
`go-test` hook needs `frontend/dist` (the `//go:embed` in `main.go`)
|
||||
and the fixture library, and refuses the push without them.
|
||||
- **Session:** pi in that worktree, `/name loop`. Add the job via
|
||||
`/schedule-prompt` (name `yj-loop`, cron
|
||||
`0 0 10-18 * * 1-5`, prompt: "Read `.pi/skills/yj-loop/SKILL.md` and
|
||||
@@ -270,4 +277,12 @@ AVD), then `make android-emulator` per session.
|
||||
which leg it really is.
|
||||
- The job did not fire — the scheduler fires only while a session is
|
||||
open in its directory (documented); "the loop is off" is the correct
|
||||
reading, not a bug.
|
||||
reading, not a bug.
|
||||
- `error: object file … is empty` / `unpack-objects failed` / `bad
|
||||
object refs/heads/…` during a fetch or checkout — the shared object
|
||||
store was corrupted (a killed fetch leaves 0-byte object files, and a
|
||||
local ref can end up pointing at the dead sha1). **Halt and report**;
|
||||
do not retry, the churn only deepens it. Human repair: delete the
|
||||
0-byte objects, `git fetch origin --prune`, delete any ref that
|
||||
still dangles (`git update-ref -d refs/heads/<b>`), re-checkout the
|
||||
worktree at `origin/main`, then `git fsck --full`.
|
||||
@@ -112,7 +112,9 @@ in the job's directory — that limitation is the switch:
|
||||
|
||||
- **Worktree:** `git worktree add` a dedicated clone at
|
||||
`~/.paseo/worktrees/loop/jumpy-hound`. Loop edits happen only there; a
|
||||
dirty tree there is the loop's business and nobody else's.
|
||||
dirty tree there is the loop's business and nobody else's. **Provision
|
||||
it once before its first push:** `make build-frontend` + `make testdata`
|
||||
— the pre-push `go-test` hook needs both and refuses without them.
|
||||
- **Session:** pi in that worktree, `/name loop`. The job is bound to that
|
||||
session, so another pi elsewhere in the same directory does not
|
||||
double-fire it.
|
||||
|
||||
@@ -172,7 +172,7 @@ make ui-test # Vitest component/store suite in a real browser (no app)
|
||||
make ui-visual # Same, including toMatchScreenshot comparisons
|
||||
make ui-setup # Install the Vitest provider's own Chromium (once)
|
||||
make bindings-check # Fail if frontend/bindings is stale vs the Go bindings
|
||||
make skill-check # Fail if .pi/ documents a make target that doesn't exist
|
||||
make skill-check # Fail if a doc names a make target that doesn't exist
|
||||
make commit-check # Fail if a commit subject is not a Conventional Commit
|
||||
make lint # golangci-lint v2 (strict), all three build configurations
|
||||
make test # All tests with race detector, all three build configurations
|
||||
@@ -673,6 +673,28 @@ rather than renaming them.
|
||||
one of the shell's rows, which is what the skip link is absolutely
|
||||
positioned to avoid.
|
||||
- `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments.
|
||||
|
||||
**A setter that can reject its argument puts the old value back**, and
|
||||
that is a correctness rule rather than hygiene (#231). `Save()`
|
||||
validates the *whole* config, so a value left behind by a failed write
|
||||
does not merely fail its own call: it fails every later save, of every
|
||||
unrelated setting — theme, launch page, shortcuts, libraries — for the
|
||||
rest of the session. Nothing reaches disk, so a restart clears it,
|
||||
which is exactly what makes the fault invisible and unreportable. One
|
||||
rejected track-list column list was enough to stop the app saving
|
||||
anything at all.
|
||||
|
||||
Two shapes are safe and a third is the trap. A setter that assigns and
|
||||
*then* validates snapshots the field first and restores it on the
|
||||
error path — seven do. `SetLibraryDirectory` is the better shape where
|
||||
the value can be built on its own: it validates a candidate *before*
|
||||
assigning, so there is nothing to undo. And a setter whose argument no
|
||||
validation inspects needs neither — the bools, the favourites playlist
|
||||
id and the shortcut bindings, plus `SetViewVisible`, which refuses an
|
||||
unknown, non-hideable or launch-page view up front so
|
||||
`GeneralConfig.Validate` never sees one it would fail on. Which set a
|
||||
new setter joins is decided by whether its own `Validate` can reject
|
||||
it, not by preference.
|
||||
- `playlist` / `smartplaylist` — Playlist CRUD and rule-based smart playlists.
|
||||
- `mediacontrols` — OS media controls behind one `Handler`: MPRIS over
|
||||
D-Bus on desktop Linux, a MediaSession on Android, a no-op stub
|
||||
@@ -3286,6 +3308,27 @@ its own duplicates apart) — and changing either is invisible against an
|
||||
existing `YJ_HOME`, whose `config.toml` already holds the old list, so
|
||||
`make sandbox-seed NAME=default` before believing the app.
|
||||
|
||||
**And the *valid* columns are declared twice too, which is the pair
|
||||
that drifted.** `tracklist.AllColumnIDs` is what the backend accepts;
|
||||
`COLUMN_DEFS` is what the frontend knows how to draw, and they are not
|
||||
the same set — `titleArtist` is a definition and not a choice, since it
|
||||
is the phone's stacked column and is picked by width in
|
||||
`PHONE_COLUMN_IDS`. Settings built its list from `Object.keys(
|
||||
COLUMN_DEFS)` and so offered it: **two rows both called "Track Name"**
|
||||
(#197), the second unselectable, because ticking it sends a column set
|
||||
Go rejects with `unknown track-list column ID` and `config-page`
|
||||
swallows that into a `console.error`. `CONFIGURABLE_COLUMN_IDS` is what
|
||||
the configurator reads now, derived from a `configurable` flag on the
|
||||
definition, and `settings-column-list.test.ts` reads Go's own list out
|
||||
of the source rather than writing it down a third time — the rule being
|
||||
about every column, so checking one checks nothing.
|
||||
|
||||
One thing it does **not** fix, because it is reachable from any invalid
|
||||
input rather than from that row: `SetTrackListColumns` assigns before it
|
||||
validates, so a rejected list stays in memory and `Save()` validates the
|
||||
whole config — one tick and **no setting saves for the rest of the
|
||||
session**, silently. That is #231.
|
||||
|
||||
**Event-driven communication**: Backend emits events via Wails runtime; frontend stores subscribe to them. Event names are constants in `backend/events/`.
|
||||
|
||||
`frontend/src/events.ts` is **generated** from `backend/events/events.go`
|
||||
|
||||
@@ -192,7 +192,7 @@ css-check: ## Fail on a css`` literal ended early by a backtick, or a nested rul
|
||||
# Every command in them is a make target on purpose, so this is
|
||||
# checkable. It also asserts AGENTS.md is a symlink to CLAUDE.md, so the
|
||||
# two harnesses cannot drift onto two descriptions of one project.
|
||||
skill-check: ## Fail if the agent docs name a missing make target, or AGENTS.md is not a symlink
|
||||
skill-check: ## Fail if the docs name a missing make target, or AGENTS.md is not a symlink
|
||||
@./scripts/skill-check.sh
|
||||
|
||||
# Conventional Commits, which CLAUDE.md claimed CI enforced for a long
|
||||
|
||||
@@ -303,6 +303,24 @@ func (c *Config) GetLibraryDirectory() string {
|
||||
return string(c.Library.DirectoryPath)
|
||||
}
|
||||
|
||||
// A rejected setter puts the old value back, and that is not tidiness
|
||||
// (#231). Save validates the *whole* config, so a value left behind by
|
||||
// a failed write does not merely fail its own call: it fails every
|
||||
// later save, of every unrelated setting, silently and for the rest of
|
||||
// the session. Nothing reaches disk, so a restart clears it -- which
|
||||
// is exactly what makes the fault hard to see and impossible to report.
|
||||
//
|
||||
// The setters below that assign and then validate therefore snapshot
|
||||
// the field first and restore it on the error path. SetLibraryDirectory
|
||||
// is the other safe shape and the better one where the value can be
|
||||
// built on its own: it validates a candidate *before* assigning
|
||||
// anything, so there is nothing to undo.
|
||||
//
|
||||
// Not every setter needs either. A bool, an int64 and the shortcut
|
||||
// bindings pass through no validation that can reject them, and
|
||||
// SetViewVisible refuses an unknown, non-hideable or launch-page view
|
||||
// up front, so GeneralConfig.Validate never sees one it would fail on.
|
||||
|
||||
// SetLibraryDirectory validates and saves a new library directory,
|
||||
// then emits the LibraryConfigChanged event so listeners (e.g. the
|
||||
// Library scanner) can react.
|
||||
@@ -360,11 +378,14 @@ func (c *Config) SetScanConcurrency(mode string) error {
|
||||
c.Library.ApplyDefaults()
|
||||
}
|
||||
|
||||
previous := c.Library.ScanConcurrency
|
||||
c.Library.ScanConcurrency = library.ScanConcurrency(
|
||||
mode,
|
||||
)
|
||||
|
||||
if err := c.Library.Validate(); err != nil {
|
||||
c.Library.ScanConcurrency = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid scan concurrency mode: %w", err,
|
||||
)
|
||||
@@ -455,9 +476,12 @@ func (c *Config) SetThemeAccentColor(
|
||||
c.Theme.ApplyDefaults()
|
||||
}
|
||||
|
||||
previous := c.Theme.AccentColor
|
||||
c.Theme.AccentColor = color
|
||||
|
||||
if err := c.Theme.Validate(); err != nil {
|
||||
c.Theme.AccentColor = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid theme accent color: %w", err,
|
||||
)
|
||||
@@ -488,9 +512,12 @@ func (c *Config) SetThemeBackgroundShade(
|
||||
c.Theme.ApplyDefaults()
|
||||
}
|
||||
|
||||
previous := c.Theme.BackgroundShade
|
||||
c.Theme.BackgroundShade = theme.BackgroundShade(shade)
|
||||
|
||||
if err := c.Theme.Validate(); err != nil {
|
||||
c.Theme.BackgroundShade = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid theme background shade: %w", err,
|
||||
)
|
||||
@@ -544,9 +571,12 @@ func (c *Config) SetDefaultPage(page string) error {
|
||||
c.General.ApplyDefaults()
|
||||
}
|
||||
|
||||
previous := c.General.DefaultPage
|
||||
c.General.DefaultPage = View(page)
|
||||
|
||||
if err := c.General.Validate(); err != nil {
|
||||
c.General.DefaultPage = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid default page: %w", err,
|
||||
)
|
||||
@@ -591,9 +621,12 @@ func (c *Config) SetQueueFallback(mode string) error {
|
||||
c.General.ApplyDefaults()
|
||||
}
|
||||
|
||||
previous := c.General.QueueFallback
|
||||
c.General.QueueFallback = QueueFallback(mode)
|
||||
|
||||
if err := c.General.Validate(); err != nil {
|
||||
c.General.QueueFallback = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid queue fallback: %w", err,
|
||||
)
|
||||
@@ -801,9 +834,12 @@ func (c *Config) SetTrackListColumns(
|
||||
c.TrackList = &tracklist.Config{}
|
||||
}
|
||||
|
||||
previous := c.TrackList.Columns
|
||||
c.TrackList.Columns = columns
|
||||
|
||||
if err := c.TrackList.Validate(); err != nil {
|
||||
c.TrackList.Columns = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid track-list columns: %w", err,
|
||||
)
|
||||
@@ -901,9 +937,12 @@ func (c *Config) SetFavoritesIconStyle(
|
||||
c.Favorites.ApplyDefaults()
|
||||
}
|
||||
|
||||
previous := c.Favorites.IconStyle
|
||||
c.Favorites.IconStyle = favorites.IconStyle(style)
|
||||
|
||||
if err := c.Favorites.Validate(); err != nil {
|
||||
c.Favorites.IconStyle = previous
|
||||
|
||||
return fmt.Errorf(
|
||||
"invalid favorites icon style: %w", err,
|
||||
)
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"yellowjacket/backend/library"
|
||||
"yellowjacket/backend/tracklist"
|
||||
)
|
||||
|
||||
// newSavableConfig builds a loaded, valid config in a temp directory,
|
||||
// so Save() writes rather than refusing with errSaveBeforeLoad.
|
||||
//
|
||||
// The library directory is real and set, because Config.Validate only
|
||||
// validates the Library section when DirectoryPath is non-empty -- an
|
||||
// empty one would hide a poisoned ScanConcurrency from the whole-config
|
||||
// save that is the symptom under test.
|
||||
func newSavableConfig(t *testing.T) *Config {
|
||||
t.Helper()
|
||||
|
||||
c := &Config{
|
||||
logger: slog.Default(),
|
||||
filePath: filepath.Join(t.TempDir(), "config.toml"),
|
||||
Library: &library.Config{
|
||||
DirectoryPath: library.Directory(t.TempDir()),
|
||||
},
|
||||
}
|
||||
|
||||
c.applyDefaults()
|
||||
|
||||
if err := c.Load(); err != nil {
|
||||
t.Fatalf("Load() error: %v", err)
|
||||
}
|
||||
|
||||
if err := c.Save(); err != nil {
|
||||
t.Fatalf("Save() on a fresh config error: %v", err)
|
||||
}
|
||||
|
||||
return c
|
||||
}
|
||||
|
||||
// TestSetterRejectionDoesNotPoisonTheConfig is the whole of #231.
|
||||
//
|
||||
// Every setter here assigns to the in-memory config and then validates.
|
||||
// When the validation rejects the argument, the rejected value has to go
|
||||
// back -- not because the caller sees it (it gets an error either way),
|
||||
// but because Config.Save() validates the *whole* config. A value left
|
||||
// behind by a failed setter therefore fails every later save, of every
|
||||
// unrelated setting, silently and for the rest of the session.
|
||||
//
|
||||
// So each case asserts three things in order: the setter reports the
|
||||
// error, the getter still reports the old value, and an unrelated save
|
||||
// still works. The third is the one the user feels.
|
||||
func TestSetterRejectionDoesNotPoisonTheConfig(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
// reject calls the setter with an argument its own Validate
|
||||
// refuses.
|
||||
reject func(*Config) error
|
||||
// read reports the value the setter writes, so the rollback is
|
||||
// asserted on the config rather than only on the save.
|
||||
read func(*Config) string
|
||||
}{
|
||||
{
|
||||
name: "scan concurrency",
|
||||
reject: func(c *Config) error {
|
||||
return c.SetScanConcurrency("telepathy")
|
||||
},
|
||||
read: (*Config).GetScanConcurrency,
|
||||
},
|
||||
{
|
||||
name: "theme accent colour",
|
||||
reject: func(c *Config) error {
|
||||
return c.SetThemeAccentColor("not-a-hex")
|
||||
},
|
||||
read: (*Config).GetThemeAccentColor,
|
||||
},
|
||||
{
|
||||
name: "theme background shade",
|
||||
reject: func(c *Config) error {
|
||||
return c.SetThemeBackgroundShade("chartreuse")
|
||||
},
|
||||
read: (*Config).GetThemeBackgroundShade,
|
||||
},
|
||||
{
|
||||
name: "default page",
|
||||
reject: func(c *Config) error {
|
||||
return c.SetDefaultPage("nowhere")
|
||||
},
|
||||
read: (*Config).GetDefaultPage,
|
||||
},
|
||||
{
|
||||
name: "queue fallback",
|
||||
reject: func(c *Config) error {
|
||||
return c.SetQueueFallback("improvise")
|
||||
},
|
||||
read: (*Config).GetQueueFallback,
|
||||
},
|
||||
{
|
||||
name: "favorites icon style",
|
||||
reject: func(c *Config) error {
|
||||
return c.SetFavoritesIconStyle("asterisk")
|
||||
},
|
||||
read: (*Config).GetFavoritesIconStyle,
|
||||
},
|
||||
{
|
||||
name: "track-list columns",
|
||||
reject: func(c *Config) error {
|
||||
// titleArtist is a drawing definition, not a
|
||||
// configurable column (#197), so it is exactly what
|
||||
// the frontend used to be able to send.
|
||||
return c.SetTrackListColumns([]tracklist.Column{
|
||||
{ID: "titleArtist"},
|
||||
})
|
||||
},
|
||||
read: func(c *Config) string {
|
||||
return columnIDs(c.GetTrackListColumns())
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "track-list columns, duplicated",
|
||||
reject: func(c *Config) error {
|
||||
// The route #197 closed was one invalid id; a
|
||||
// duplicate is the one still reachable from a client
|
||||
// that assembles the list itself.
|
||||
return c.SetTrackListColumns([]tracklist.Column{
|
||||
{ID: tracklist.ColTrackName},
|
||||
{ID: tracklist.ColTrackName},
|
||||
})
|
||||
},
|
||||
read: func(c *Config) string {
|
||||
return columnIDs(c.GetTrackListColumns())
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
c := newSavableConfig(t)
|
||||
before := tc.read(c)
|
||||
|
||||
if err := tc.reject(c); err == nil {
|
||||
t.Fatal("setter accepted an invalid value, want an error")
|
||||
}
|
||||
|
||||
if after := tc.read(c); after != before {
|
||||
t.Errorf(
|
||||
"value after a rejected write = %q, want the previous %q",
|
||||
after, before,
|
||||
)
|
||||
}
|
||||
|
||||
// The symptom: an unrelated setting can no longer be saved.
|
||||
if err := c.SetPopupVolume(true); err != nil {
|
||||
t.Errorf("an unrelated setter failed after a rejected write: %v", err)
|
||||
}
|
||||
|
||||
if err := c.Save(); err != nil {
|
||||
t.Errorf("Save() failed after a rejected write: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRejectedSetterLeavesNothingOnDisk pairs with the sweep above: the
|
||||
// rollback must not be undone by what the file already holds, so a
|
||||
// config reloaded from disk after a rejected write agrees with memory.
|
||||
func TestRejectedSetterLeavesNothingOnDisk(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
c := newSavableConfig(t)
|
||||
|
||||
if err := c.SetThemeAccentColor("#123456"); err != nil {
|
||||
t.Fatalf("SetThemeAccentColor() error: %v", err)
|
||||
}
|
||||
|
||||
if err := c.SetThemeAccentColor("not-a-hex"); err == nil {
|
||||
t.Fatal("SetThemeAccentColor accepted a non-colour, want an error")
|
||||
}
|
||||
|
||||
reloaded := &Config{logger: slog.Default(), filePath: c.filePath}
|
||||
reloaded.applyDefaults()
|
||||
|
||||
if err := reloaded.Load(); err != nil {
|
||||
t.Fatalf("Load() error: %v", err)
|
||||
}
|
||||
|
||||
if got := reloaded.GetThemeAccentColor(); got != "#123456" {
|
||||
t.Errorf("accent colour on disk = %q, want %q", got, "#123456")
|
||||
}
|
||||
|
||||
if c.GetThemeAccentColor() != reloaded.GetThemeAccentColor() {
|
||||
t.Errorf(
|
||||
"in-memory accent %q disagrees with disk %q after a rejected write",
|
||||
c.GetThemeAccentColor(), reloaded.GetThemeAccentColor(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSetLibraryDirectoryValidatesBeforeAssigning pins the precedent the
|
||||
// seven rolled-back setters follow: this one has always built and
|
||||
// validated a candidate before assigning, so a bad path never reaches
|
||||
// the config at all.
|
||||
func TestSetLibraryDirectoryValidatesBeforeAssigning(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
c := newSavableConfig(t)
|
||||
before := c.GetLibraryDirectory()
|
||||
|
||||
if err := c.SetLibraryDirectory(filepath.Join(t.TempDir(), "no-such-dir")); err == nil {
|
||||
t.Fatal("SetLibraryDirectory accepted a missing directory, want an error")
|
||||
}
|
||||
|
||||
if after := c.GetLibraryDirectory(); after != before {
|
||||
t.Errorf("library directory = %q, want the previous %q", after, before)
|
||||
}
|
||||
|
||||
if err := c.Save(); err != nil {
|
||||
t.Errorf("Save() failed after a rejected library directory: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSetViewVisibleRefusesBeforeAssigning covers the other setter left
|
||||
// out of the rollback pass: it guards its own argument up front, so
|
||||
// GeneralConfig.Validate never sees a view it would reject.
|
||||
func TestSetViewVisibleRefusesBeforeAssigning(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
c := newSavableConfig(t)
|
||||
|
||||
if err := c.SetViewVisible("no-such-view", false); err == nil {
|
||||
t.Fatal("SetViewVisible accepted an unknown view, want an error")
|
||||
}
|
||||
|
||||
if err := c.SetViewVisible(c.GetDefaultPage(), false); err == nil {
|
||||
t.Fatal("SetViewVisible hid the launch page, want an error")
|
||||
}
|
||||
|
||||
if err := c.Save(); err != nil {
|
||||
t.Errorf("Save() failed after a refused view visibility change: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// columnIDs renders a column list for comparison in the table above.
|
||||
func columnIDs(cols []tracklist.Column) string {
|
||||
ids := make([]byte, 0, len(cols)*8)
|
||||
|
||||
for i, col := range cols {
|
||||
if i > 0 {
|
||||
ids = append(ids, ',')
|
||||
}
|
||||
|
||||
ids = append(ids, col.ID...)
|
||||
}
|
||||
|
||||
return string(ids)
|
||||
}
|
||||
@@ -51,7 +51,7 @@ import type { BackgroundShade } from '@store/theme-store';
|
||||
import type { IconStyle } from '@store/favorites-store';
|
||||
import {
|
||||
COLUMN_DEFS,
|
||||
ALL_COLUMN_IDS,
|
||||
CONFIGURABLE_COLUMN_IDS,
|
||||
} from '@components/track-list/columns';
|
||||
|
||||
import './config-field';
|
||||
@@ -1640,7 +1640,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
...this.trackListCtrl.columnIds,
|
||||
];
|
||||
|
||||
const disabledIds = ALL_COLUMN_IDS.filter(
|
||||
const disabledIds = CONFIGURABLE_COLUMN_IDS.filter(
|
||||
(id) => !enabledIds.includes(id),
|
||||
);
|
||||
|
||||
|
||||
@@ -2,12 +2,14 @@ import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, state, query } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/dialog/dialog.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import { EventsOn } from '@runtime/runtime';
|
||||
import {
|
||||
AddLibrary,
|
||||
GetAllLibrariesWithTrackCounts,
|
||||
} from '@go/library/library.js';
|
||||
import { describeError, explainError } from '@utils/describe-error';
|
||||
import { nameDialogsIn } from '@utils/name-dialog';
|
||||
import { Events } from '../../events';
|
||||
import { pickDirectory } from '../../utils/pick-directory';
|
||||
|
||||
/**
|
||||
@@ -19,6 +21,13 @@ import { pickDirectory } from '../../utils/pick-directory';
|
||||
* prompting the user to pick their music folder, registers it through the
|
||||
* library CRUD API, and dismisses itself. AddLibrary emits LibraryAdded
|
||||
* and kicks off the initial scan automatically.
|
||||
*
|
||||
* **The dismissal follows the library existing, not the button being
|
||||
* pressed.** `AddLibrary` emits `LibraryAdded` whoever calls it, so the
|
||||
* wizard waits on the state it exists to wait for rather than on a step
|
||||
* in its own flow — a library arriving by any other route (Settings, a
|
||||
* direct call) leaves a full-screen modal up otherwise, intercepting
|
||||
* every pointer event.
|
||||
*/
|
||||
@customElement('first-run-wizard')
|
||||
export class FirstRunWizard extends LitElement {
|
||||
@@ -37,9 +46,18 @@ export class FirstRunWizard extends LitElement {
|
||||
/** Error message from a failed pick/save, if any. */
|
||||
@state() private errorMessage = '';
|
||||
|
||||
/** Unsubscribe from LibraryAdded, while this element is connected. */
|
||||
private cancelLibraryAdded?: () => void;
|
||||
|
||||
override async connectedCallback(): Promise<void> {
|
||||
super.connectedCallback();
|
||||
|
||||
// Subscribed before the read below, so a library arriving while
|
||||
// that call is in flight is not answered with a stale empty list.
|
||||
this.cancelLibraryAdded = EventsOn(Events.LibraryAdded, () => {
|
||||
this.dismiss();
|
||||
});
|
||||
|
||||
try {
|
||||
const existing = await GetAllLibrariesWithTrackCounts();
|
||||
|
||||
@@ -54,6 +72,8 @@ export class FirstRunWizard extends LitElement {
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.finished) return;
|
||||
|
||||
this.active = true;
|
||||
|
||||
await this.updateComplete;
|
||||
@@ -61,6 +81,13 @@ export class FirstRunWizard extends LitElement {
|
||||
if (this.dialog) this.dialog.open = true;
|
||||
}
|
||||
|
||||
override disconnectedCallback(): void {
|
||||
this.cancelLibraryAdded?.();
|
||||
this.cancelLibraryAdded = undefined;
|
||||
|
||||
super.disconnectedCallback();
|
||||
}
|
||||
|
||||
static override styles = css`
|
||||
wa-dialog {
|
||||
--width: 480px;
|
||||
@@ -239,6 +266,20 @@ export class FirstRunWizard extends LitElement {
|
||||
if (!this.finished) e.preventDefault();
|
||||
};
|
||||
|
||||
/**
|
||||
* Close, and stay closed: a library exists, so setup is over.
|
||||
*
|
||||
* `finished` is set first, or `preventClose` cancels the hide this
|
||||
* asks for.
|
||||
*/
|
||||
private dismiss(): void {
|
||||
this.finished = true;
|
||||
|
||||
if (this.dialog) this.dialog.open = false;
|
||||
|
||||
this.active = false;
|
||||
}
|
||||
|
||||
private handleChoose = async (): Promise<void> => {
|
||||
this.errorMessage = '';
|
||||
|
||||
@@ -264,11 +305,7 @@ export class FirstRunWizard extends LitElement {
|
||||
try {
|
||||
await AddLibrary(this.selectedDirectory);
|
||||
|
||||
this.finished = true;
|
||||
|
||||
if (this.dialog) this.dialog.open = false;
|
||||
|
||||
this.active = false;
|
||||
this.dismiss();
|
||||
} catch (err) {
|
||||
this.errorMessage = explainError(
|
||||
err,
|
||||
|
||||
@@ -32,6 +32,18 @@ export interface ColumnDef {
|
||||
id: string;
|
||||
/** Human-readable header label. */
|
||||
label: string;
|
||||
/**
|
||||
* Whether Settings may offer this column. Defaults to true.
|
||||
*
|
||||
* A definition is not the same thing as a *choice*. `titleArtist`
|
||||
* is the phone's stacked column, picked by width in
|
||||
* `PHONE_COLUMN_IDS`, and `tracklist.AllColumnIDs` in Go does not
|
||||
* list it — so a tick in the configurator sends a column set the
|
||||
* backend rejects with `unknown track-list column ID`, the tick
|
||||
* reverts on the next render, and the only trace is a
|
||||
* `console.error` (#197).
|
||||
*/
|
||||
configurable?: boolean;
|
||||
/** Extracts the display value from a track. */
|
||||
accessor: (track: library.Track) => string;
|
||||
/** Default CSS width (used when no saved width exists). */
|
||||
@@ -98,11 +110,16 @@ export const COLUMN_DEFS: Record<string, ColumnDef> = {
|
||||
},
|
||||
titleArtist: {
|
||||
id: 'titleArtist',
|
||||
// Named for what it sorts by, since that is the only place the
|
||||
// label is user-visible: the phone has no column headers, and
|
||||
// the page header's sort list is built from the *configured*
|
||||
// columns rather than the drawn ones.
|
||||
// Named for what it sorts by. That label is drawn nowhere
|
||||
// today: the phone has no column headers, and the page header's
|
||||
// sort list is built from the *configured* columns, which this
|
||||
// one can never be — see `configurable` below.
|
||||
label: 'Track Name',
|
||||
// Chosen by width, never by the user, and rejected by the
|
||||
// backend if it ever were. #197: Settings listed it anyway, so
|
||||
// there were two rows called "Track Name" and the second one
|
||||
// could not be selected.
|
||||
configurable: false,
|
||||
accessor: (t) => t.TrackName,
|
||||
defaultWidth: '1fr',
|
||||
comparator: (a, b) => compareStr(a.TrackName, b.TrackName),
|
||||
@@ -266,10 +283,15 @@ export const COLUMN_DEFS: Record<string, ColumnDef> = {
|
||||
};
|
||||
|
||||
/**
|
||||
* All column IDs in default display order.
|
||||
* Used by the settings UI to list available columns.
|
||||
* The column IDs Settings may offer, in default display order.
|
||||
*
|
||||
* Not every definition is one: a column the user cannot choose has no
|
||||
* row in the configurator, because a checkbox that cannot change
|
||||
* anything is worse than an absent one — see `ColumnDef.configurable`.
|
||||
*/
|
||||
export const ALL_COLUMN_IDS: string[] = Object.keys(COLUMN_DEFS);
|
||||
export const CONFIGURABLE_COLUMN_IDS: string[] = Object.keys(
|
||||
COLUMN_DEFS,
|
||||
).filter((id) => COLUMN_DEFS[id]?.configurable !== false);
|
||||
|
||||
/**
|
||||
* Column IDs that are always searched regardless of visibility.
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
/**
|
||||
* #175: the first-run wizard's dismissal follows the library existing,
|
||||
* not its own button being pressed.
|
||||
*
|
||||
* The wizard is a modal that blocks every pointer event, so a library
|
||||
* arriving by another route — Settings, a direct call — used to leave
|
||||
* it up over an app that was already set up. `LibraryAdded` is emitted
|
||||
* by `AddLibrary` whoever calls it, which is what makes one
|
||||
* subscription the whole fix.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, stub } from '../support/harness';
|
||||
import { fixture, shadow, shadowAll } from '../support/render';
|
||||
import { wails } from '../support/wails-fake';
|
||||
|
||||
import '@components/first-run-wizard/first-run-wizard';
|
||||
|
||||
import type { FirstRunWizard } from '@components/first-run-wizard/first-run-wizard';
|
||||
|
||||
/** A library row, as `GetAllLibrariesWithTrackCounts` returns one. */
|
||||
const aLibrary = {
|
||||
id: 1,
|
||||
name: 'Music',
|
||||
path: '/home/logan/Music',
|
||||
trackCount: 9,
|
||||
};
|
||||
|
||||
/** Mount the wizard on a fresh install: no libraries yet. */
|
||||
async function wizardOnAFreshInstall(): Promise<FirstRunWizard> {
|
||||
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
|
||||
|
||||
return fixture<FirstRunWizard>('first-run-wizard');
|
||||
}
|
||||
|
||||
/** Whether the wizard is rendering its modal at all. */
|
||||
function isShowing(el: FirstRunWizard): boolean {
|
||||
return shadow(el, 'wa-dialog') !== null;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
stub('library.Library.AddLibrary', aLibrary);
|
||||
});
|
||||
|
||||
describe('first-run-wizard', () => {
|
||||
it('shows on a fresh install and stays up until a library exists', async () => {
|
||||
const el = await wizardOnAFreshInstall();
|
||||
|
||||
expect(isShowing(el)).toBe(true);
|
||||
});
|
||||
|
||||
it('stays hidden when a library is already configured', async () => {
|
||||
stub('library.Library.GetAllLibrariesWithTrackCounts', [aLibrary]);
|
||||
|
||||
const el = await fixture<FirstRunWizard>('first-run-wizard');
|
||||
|
||||
expect(isShowing(el)).toBe(false);
|
||||
});
|
||||
|
||||
it('dismisses when a library appears by another route', async () => {
|
||||
const el = await wizardOnAFreshInstall();
|
||||
|
||||
expect(isShowing(el)).toBe(true);
|
||||
|
||||
emit(Events.LibraryAdded, aLibrary);
|
||||
await el.updateComplete;
|
||||
|
||||
expect(isShowing(el)).toBe(false);
|
||||
});
|
||||
|
||||
it('does not raise itself when a library arrives while it is asking', async () => {
|
||||
// The read is still in flight when the event lands, so its
|
||||
// answer — an empty list — is stale by the time it returns.
|
||||
let answer: (libraries: unknown[]) => void = () => {};
|
||||
|
||||
stub(
|
||||
'library.Library.GetAllLibrariesWithTrackCounts',
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
answer = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
const el = await fixture<FirstRunWizard>('first-run-wizard');
|
||||
|
||||
emit(Events.LibraryAdded, aLibrary);
|
||||
answer([]);
|
||||
|
||||
await el.updateComplete;
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
await el.updateComplete;
|
||||
|
||||
expect(isShowing(el)).toBe(false);
|
||||
});
|
||||
|
||||
it('still dismisses through its own Get Started button', async () => {
|
||||
stub('frontendutil.FrontendUtil.HasNativeDirectoryPicker', true);
|
||||
stub('frontendutil.FrontendUtil.DirectoryPicker', '/home/logan/Music');
|
||||
|
||||
const { resetDirectoryPickerCache } = await import(
|
||||
'@utils/pick-directory'
|
||||
);
|
||||
|
||||
resetDirectoryPickerCache();
|
||||
|
||||
const el = await wizardOnAFreshInstall();
|
||||
const [choose, finish] = shadowAll<HTMLButtonElement>(el, '.btn');
|
||||
|
||||
choose?.click();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
await el.updateComplete;
|
||||
|
||||
finish?.click();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
await el.updateComplete;
|
||||
|
||||
expect(
|
||||
wails.calls.filter((c) => c.path === 'library.Library.AddLibrary'),
|
||||
).toHaveLength(1);
|
||||
expect(isShowing(el)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,190 @@
|
||||
/**
|
||||
* Settings offers the columns the backend will accept, and no others.
|
||||
*
|
||||
* The list is built from `COLUMN_DEFS`, which is the *drawing* table:
|
||||
* every definition the track list knows how to render, including
|
||||
* `titleArtist` — the phone's stacked column, chosen by width in
|
||||
* `PHONE_COLUMN_IDS` and never by a person. `tracklist.AllColumnIDs` in
|
||||
* Go does not list that id, so the configurator offered a nineteenth
|
||||
* row that could not be ticked:
|
||||
*
|
||||
* ```
|
||||
* validate = unknown track-list column ID: "titleArtist"
|
||||
* titleArtist valid = false
|
||||
* ```
|
||||
*
|
||||
* What a user saw was **two rows both called "Track Name"** (#197), one
|
||||
* of which did nothing — and a screen reader heard "Show the Track Name
|
||||
* column" twice with nothing to tell them apart, which is `a11y.32`'s
|
||||
* complaint inside the list that was fixed for exactly that.
|
||||
*
|
||||
* It is worse than an inert control, which is why the duplicate name
|
||||
* was not the thing to fix. `SetTrackListColumns` assigns before it
|
||||
* validates, so a rejected list stays in memory and `Save()` validates
|
||||
* the whole config:
|
||||
*
|
||||
* ```
|
||||
* later, unrelated SetThemeAccentColor = could not save config: invalid
|
||||
* config: ... unknown track-list column ID: "titleArtist"
|
||||
* ```
|
||||
*
|
||||
* — one tick and no setting saves for the rest of the session. That
|
||||
* half is filed separately; this file keeps the row from being offered.
|
||||
*
|
||||
* The last test is the one that would have caught it when the column
|
||||
* was added: the two lists are in different languages, so nothing but a
|
||||
* sweep can hold them together.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/config-page/config-page';
|
||||
|
||||
import {
|
||||
COLUMN_DEFS,
|
||||
CONFIGURABLE_COLUMN_IDS,
|
||||
} from '@components/track-list/columns';
|
||||
import { flush, stub } from '@test/support/harness';
|
||||
import { fixture, shadowAll } from '@test/support/render';
|
||||
|
||||
/** Go's own list of column ids, as text. */
|
||||
const GO_CONFIG = Object.values(
|
||||
import.meta.glob<string>('../../../backend/tracklist/config.go', {
|
||||
eager: true,
|
||||
query: '?raw',
|
||||
import: 'default',
|
||||
}),
|
||||
)[0];
|
||||
|
||||
/**
|
||||
* The ids `tracklist.AllColumnIDs` actually contains.
|
||||
*
|
||||
* Read out of the source rather than written down here, because a
|
||||
* third copy of this list is a third thing to forget — which is the
|
||||
* defect, one copy earlier.
|
||||
*/
|
||||
function goColumnIDs(source: string): string[] {
|
||||
const constants = new Map<string, string>();
|
||||
const constBlock = /const \(([\s\S]*?)\n\)/.exec(source)?.[1] ?? '';
|
||||
|
||||
for (const [, name, id] of constBlock.matchAll(
|
||||
/(\w+)\s+ColumnID\s*=\s*"([^"]+)"/g,
|
||||
)) {
|
||||
constants.set(name!, id!);
|
||||
}
|
||||
|
||||
const listBlock =
|
||||
/var AllColumnIDs = \[\]ColumnID\{([\s\S]*?)\n\}/.exec(source)?.[1] ?? '';
|
||||
|
||||
return [...listBlock.matchAll(/(\w+),/g)]
|
||||
.map(([, name]) => constants.get(name!))
|
||||
.filter((id): id is string => id !== undefined);
|
||||
}
|
||||
|
||||
/**
|
||||
* The column rows, and only those.
|
||||
*
|
||||
* Settings’ view-visibility list (#25) is drawn with the same two
|
||||
* classes, so a bare `.column-label` sweeps 29 rows across two
|
||||
* sections — and "Albums" the destination sitting beside "Album" the
|
||||
* column is not the fault this file is about. The `for`/`id` prefix is
|
||||
* what tells them apart.
|
||||
*/
|
||||
const COLUMN_ROW_LABEL = 'label.column-label[for^="column-"]';
|
||||
const COLUMN_ROW_BOX = 'input.column-toggle[id^="column-"]';
|
||||
|
||||
/** The rows the configurator draws, by their visible name. */
|
||||
async function columnRowNames(): Promise<string[]> {
|
||||
const page = await fixture('config-page');
|
||||
|
||||
await flush();
|
||||
await page.updateComplete;
|
||||
|
||||
// Every section renders collapsed, and a collapsed body is `hidden`.
|
||||
for (const section of shadowAll<HTMLElement>(page, 'config-section')) {
|
||||
section.shadowRoot
|
||||
?.querySelector<HTMLButtonElement>('button[aria-expanded="false"]')
|
||||
?.click();
|
||||
}
|
||||
|
||||
await flush();
|
||||
await page.updateComplete;
|
||||
|
||||
return shadowAll<HTMLElement>(page, COLUMN_ROW_LABEL).map(
|
||||
(label) => label.textContent?.trim() ?? '',
|
||||
);
|
||||
}
|
||||
|
||||
describe('the Settings column list', () => {
|
||||
beforeEach(() => {
|
||||
for (const path of [
|
||||
'library.Library.GetAllLibrariesWithTrackCounts',
|
||||
'jobs.Service.GetJobs',
|
||||
'download.Service.ListProviders',
|
||||
'download.Service.ProviderKinds',
|
||||
]) {
|
||||
stub(path, []);
|
||||
}
|
||||
|
||||
stub('config.Config.GetShortcuts', {});
|
||||
stub('config.Config.GetDownloadPreferences', {});
|
||||
stub('config.Config.GetThemeAccentColor', '#ffd43b');
|
||||
stub('config.Config.GetThemeBackgroundShade', 'dark');
|
||||
});
|
||||
|
||||
it('names each row once', async () => {
|
||||
const names = await columnRowNames();
|
||||
|
||||
// A sweep over nothing passes.
|
||||
expect(names.length, 'the page draws column rows').toBeGreaterThan(5);
|
||||
|
||||
const seen = new Set<string>();
|
||||
const duplicated = names.filter((name) => !seen.add(name));
|
||||
|
||||
expect(duplicated).toEqual([]);
|
||||
expect(names.filter((n) => n === 'Track Name')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('gives each checkbox a name that identifies it', async () => {
|
||||
// The visible half above is what was reported; this is the half a
|
||||
// screen reader gets, and it is the one `config-page` computes
|
||||
// from the same string.
|
||||
const page = await fixture('config-page');
|
||||
|
||||
await flush();
|
||||
await page.updateComplete;
|
||||
|
||||
const labels = shadowAll<HTMLInputElement>(page, COLUMN_ROW_BOX).map(
|
||||
(box) => box.getAttribute('aria-label') ?? '',
|
||||
);
|
||||
|
||||
expect(labels.length, 'the page draws column checkboxes').toBeGreaterThan(5);
|
||||
expect(new Set(labels).size).toBe(labels.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the column table', () => {
|
||||
it('offers no column the backend would reject', async () => {
|
||||
const accepted = goColumnIDs(GO_CONFIG ?? '');
|
||||
|
||||
// Two non-vacuity guards: a glob that stopped matching, and a
|
||||
// parse that stopped finding the list it names.
|
||||
expect(GO_CONFIG, 'backend/tracklist/config.go is readable').toBeTruthy();
|
||||
expect(accepted.length, 'AllColumnIDs was parsed').toBeGreaterThan(10);
|
||||
|
||||
expect(
|
||||
CONFIGURABLE_COLUMN_IDS.filter((id) => !accepted.includes(id)),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('still knows how to draw every column it offers', async () => {
|
||||
// The filter must not have taken a column *out* of the drawing
|
||||
// table: `configurable` says what Settings may list, not what the
|
||||
// list may render.
|
||||
expect(
|
||||
CONFIGURABLE_COLUMN_IDS.filter((id) => COLUMN_DEFS[id] === undefined),
|
||||
).toEqual([]);
|
||||
expect(CONFIGURABLE_COLUMN_IDS).not.toContain('titleArtist');
|
||||
expect(COLUMN_DEFS['titleArtist'], 'the phone still has its column')
|
||||
.toBeTruthy();
|
||||
});
|
||||
});
|
||||
+7
-3
@@ -38,10 +38,14 @@ pre-commit:
|
||||
glob: "*.go"
|
||||
run: ./scripts/bindings-check.sh
|
||||
|
||||
# .pi/ documents make targets; a stale one sends an agent off a
|
||||
# cliff with total confidence. Instant.
|
||||
# The docs document make targets; a stale one sends an agent — or a
|
||||
# contributor reading CONTRIBUTING.md — off a cliff with total
|
||||
# confidence. The glob is the script's own scanned set, because a
|
||||
# hook that does not fire on a file the check reads is the drift the
|
||||
# check exists to prevent: it was `{Makefile,.pi/**/*.md}` while the
|
||||
# script already read CLAUDE.md. Instant.
|
||||
skill-check:
|
||||
glob: "{Makefile,.pi/**/*.md}"
|
||||
glob: "{Makefile,.pi/**/*.md,AGENTS.md,CLAUDE.md,README.md,CONTRIBUTING.md}"
|
||||
run: ./scripts/skill-check.sh
|
||||
|
||||
frontend-typecheck:
|
||||
|
||||
+21
-4
@@ -14,6 +14,11 @@
|
||||
# missing: CLAUDE.md names 27 targets and nothing verified one of them,
|
||||
# so the file the agents trust most was the file least checked.
|
||||
#
|
||||
# README.md and CONTRIBUTING.md are in it too, and the header sentence
|
||||
# above is why: a person who has *not* read the Makefile goes looking in
|
||||
# the contributor-facing doc, so a renamed target sends them off the
|
||||
# same cliff it sends an agent off. CONTRIBUTING.md names 21 targets.
|
||||
#
|
||||
# **AGENTS.md is a symlink to CLAUDE.md.** This repo is worked on by
|
||||
# two agent harnesses that read different files by convention — Claude
|
||||
# Code reads CLAUDE.md, others read AGENTS.md — and two harnesses
|
||||
@@ -43,7 +48,19 @@ if [ -e AGENTS.md ] || [ -L AGENTS.md ]; then
|
||||
fi
|
||||
fi
|
||||
|
||||
[ -d .pi ] || exit 0
|
||||
# The scan is over the docs that are actually there: a checkout without
|
||||
# .pi/ still has README.md and CONTRIBUTING.md to check, and gating the
|
||||
# whole run on .pi/ would have made the human-facing half conditional on
|
||||
# the agent-facing one. This list is used twice — once to read the
|
||||
# mentions out and once to say which file a missing target came from —
|
||||
# because a second list is a second thing to forget.
|
||||
# `ls` exits non-zero when *any* of its arguments is missing while still
|
||||
# printing the ones that are there, and under `set -e` that would sink
|
||||
# the assignment rather than scanning what exists, so swallow it.
|
||||
docs="$({ find .pi -name '*.md' 2>/dev/null
|
||||
ls CLAUDE.md README.md CONTRIBUTING.md 2>/dev/null || true; })"
|
||||
|
||||
[ -n "$docs" ] || exit 0
|
||||
|
||||
# `make -pq` prints the database including every rule, without running
|
||||
# anything. It exits non-zero when a target is out of date, and under
|
||||
@@ -68,7 +85,7 @@ targets="$({ make -pqRr 2>/dev/null || true; } |
|
||||
# AGENTS.md is deliberately not in this list: it is a symlink to
|
||||
# CLAUDE.md, asserted above, so scanning it would report every failure
|
||||
# twice under two names.
|
||||
mentioned="$({ find .pi -name '*.md' 2>/dev/null; echo CLAUDE.md; } |
|
||||
mentioned="$(printf '%s\n' "$docs" |
|
||||
xargs awk '
|
||||
FNR == 1 { fence = 0 }
|
||||
/^```/ { fence = !fence; next }
|
||||
@@ -93,10 +110,10 @@ for t in $mentioned; do
|
||||
done
|
||||
|
||||
if [ -n "$missing" ]; then
|
||||
echo "skill-check: the agent docs name make targets that do not exist:" >&2
|
||||
echo "skill-check: the docs name make targets that do not exist:" >&2
|
||||
for t in $missing; do
|
||||
echo " make $t" >&2
|
||||
grep -rln "make $t" .pi CLAUDE.md --include='*.md' | sed 's/^/ /' >&2
|
||||
printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/ /' >&2
|
||||
done
|
||||
echo "Fix the docs, or restore the target." >&2
|
||||
exit 1
|
||||
|
||||
Reference in New Issue
Block a user