Compare commits

..
Author SHA1 Message Date
logan b9e60bdb0a fix(shell): dismiss the wizard when a library exists
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m58s
CI / e2e (pull_request) Successful in 10m25s
The first-run wizard dismissed itself as a step in its own flow, so a
library appearing by any other route left a full-screen modal up over
an app that was already set up — intercepting every pointer event,
which is also what keeps Settings out of reach while it is there.

AddLibrary emits LibraryAdded whoever calls it, so one subscription
makes the dismissal follow the state the wizard exists to wait for
rather than the button being pressed. It is registered before the
initial read, or a library arriving while that call is in flight is
answered with a stale empty list; both routes out now end in one
dismiss(), which sets `finished` before asking the dialog to close
because preventClose cancels the hide otherwise.

Closes #175
2026-08-30 06:40:32 -04:00
6 changed files with 174 additions and 35 deletions
+1 -1
View File
@@ -161,7 +161,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 a doc names a make target that doesn't exist
make skill-check # Fail if .pi/ documents 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
+1 -1
View File
@@ -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 docs name a missing make target, or AGENTS.md is not a symlink
skill-check: ## Fail if the agent 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
@@ -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,
@@ -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);
});
});
+3 -7
View File
@@ -38,14 +38,10 @@ pre-commit:
glob: "*.go"
run: ./scripts/bindings-check.sh
# 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.
# .pi/ documents make targets; a stale one sends an agent off a
# cliff with total confidence. Instant.
skill-check:
glob: "{Makefile,.pi/**/*.md,AGENTS.md,CLAUDE.md,README.md,CONTRIBUTING.md}"
glob: "{Makefile,.pi/**/*.md}"
run: ./scripts/skill-check.sh
frontend-typecheck:
+4 -21
View File
@@ -14,11 +14,6 @@
# 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
@@ -48,19 +43,7 @@ if [ -e AGENTS.md ] || [ -L AGENTS.md ]; then
fi
fi
# 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
[ -d .pi ] || 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
@@ -85,7 +68,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="$(printf '%s\n' "$docs" |
mentioned="$({ find .pi -name '*.md' 2>/dev/null; echo CLAUDE.md; } |
xargs awk '
FNR == 1 { fence = 0 }
/^```/ { fence = !fence; next }
@@ -110,10 +93,10 @@ for t in $mentioned; do
done
if [ -n "$missing" ]; then
echo "skill-check: the docs name make targets that do not exist:" >&2
echo "skill-check: the agent docs name make targets that do not exist:" >&2
for t in $missing; do
echo " make $t" >&2
printf '%s\n' "$docs" | xargs grep -ln "make $t" | sed 's/^/ /' >&2
grep -rln "make $t" .pi CLAUDE.md --include='*.md' | sed 's/^/ /' >&2
done
echo "Fix the docs, or restore the target." >&2
exit 1