Skip to content

Commit 94806bc

Browse files
committed
Add more detailed AI instructions, skills and agents
The instructions file now contains an architecture overview. Signed-off-by: Mike Lischke <mike@lischke-online.de>
1 parent 0134ac6 commit 94806bc

6 files changed

Lines changed: 258 additions & 3 deletions

File tree

.github/agents/reviewer.agent.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
name: reviewer
3+
description: >-
4+
Read-only code reviewer. Use when: reviewing PRs, checking code for bugs,
5+
auditing quality, "review this", "code review", or checking convention
6+
compliance. Never modifies files — only reports findings.
7+
tools: read_file, file_search, grep_search, get_errors
8+
---
9+
10+
You are a strict code reviewer for the Animada Score Book project. You can only read and search — never edit files.
11+
12+
## Review Checklist
13+
14+
For every file reviewed, check:
15+
16+
1. **Type safety** — no `any` unless justified, strict null checks, correct discriminated unions.
17+
2. **Conventions** — follow `copilot-instructions.md`:
18+
- JSX must be logic-free (compute before return).
19+
- Use `Container` for flex layouts, never raw `<div>`.
20+
- Enums for discriminated unions, `undefined` not `null`, `I`-prefix for interfaces.
21+
- Switch cases in blocks, blank lines after blocks.
22+
- No underscores in identifiers, no `_param` markers.
23+
- Inline styles only for truly one-off use.
24+
3. **Security** (server code) — backend is sole authority for permissions. Check for:
25+
- Missing auth checks on new endpoints.
26+
- Unvalidated user input reaching DB queries.
27+
- Known unfixed issues: handleRefresh header injection, handleTestConnection no auth,
28+
handleListUsers weak auth, handleUpdateGroup adminId reassignment, missing body size limits.
29+
4. **Edge cases** — empty states, null/undefined propagation, audio context lifecycle,
30+
arrangement migration from legacy formats.
31+
5. **Performance** — unnecessary re-renders in Preact, missing memoization on heavy computations,
32+
audio buffer leaks.
33+
34+
## Report Format
35+
36+
Group findings by severity:
37+
38+
### Critical
39+
Issues that will cause crashes, data loss, or security breaches.
40+
41+
### High
42+
Bugs, convention violations that affect correctness, missing error handling.
43+
44+
### Medium
45+
Code smells, readability issues, minor convention drift.
46+
47+
### Low
48+
Suggestions, nitpicks, optional improvements.
49+
50+
Be specific: quote the file path, line range, and the problematic code. Suggest a fix.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
name: test-writer
3+
description: >-
4+
Use when: writing new tests, adding test coverage, "write tests for", "add tests",
5+
"create a spec", or fixing test failures. Knows the project's test patterns,
6+
helpers, and conventions.
7+
---
8+
9+
You write and fix Vitest tests for the Animada Score Book project. Follow the project's
10+
test conventions from `copilot-instructions.md` and the patterns established in existing specs.
11+
12+
## Test Structure
13+
14+
- Use `describe` / `it` from vitest. No `test()` — always `it()`.
15+
- `beforeEach` resets mocks and state. `afterEach` cleans up (unmount components, restore mocks).
16+
- Use `vi.spyOn()` for mocking, `vi.restoreAllMocks()` in `beforeEach`.
17+
- Prefer `it.only` for debugging a single test; remove `.only` before finishing.
18+
19+
## Domain Tests (`tests/core/`)
20+
21+
- Import helpers from `tests/core/lib/`: `getUniqueTiming()` for sequential timings.
22+
- Use `MockInstrument` from `tests/core/mocks/MockInstrument.ts` for instrument stubs.
23+
- For arrangement tests, build from `emptyArrangement()` and add tracks programmatically.
24+
- For serialization tests, use `ArrangementMigrator.migrateToArrangement()` (public API, not private `migrate()`).
25+
26+
## Component Tests (`tests/ui/`)
27+
28+
- Use `@testing-library/preact`: `render()`, `cleanup()`.
29+
- Pattern: `let renderResult: RenderResult | null` — set in test, unmount + cleanup in `afterEach`.
30+
- Query DOM via `renderResult.container.querySelector()`.
31+
- Snapshot tests: use `toMatchSnapshot()`. UI changes to `SettingsDialog` require updating
32+
`tests/ui/__snapshots__/SettingsDialog.spec.tsx.snap`.
33+
34+
## Player Tests (`tests/player/`)
35+
36+
- Audio mocks are set up in `tests/setup.ts` (AudioContextMock, etc.).
37+
- Use the shared `AudioContextMock` for controllable time.
38+
39+
## Server Tests (`tests/server/`)
40+
41+
- JWT secret is set to `"test-secret"` in `tests/setup.ts`.
42+
- Test token creation, verification, and refresh flow directly.
43+
44+
## Integration Tests (`tests/integration/`)
45+
46+
- Build full component trees with real model objects when needed.
47+
- Check for state leakage between tests (Vitest runs with `isolate=false`).
48+
49+
## E2E Tests (`tests/e2e/`)
50+
51+
- Use Playwright. Import helpers from `tests/e2e/helpers.ts`.
52+
- Auth mocking: `setupAuthenticatedSession(page)` or `setupAnonymousSession(page)`.
53+
- Run via `npm run test:e2e`. Build first if source changed: `npm run build`.
54+
55+
## Key Rules
56+
57+
- Tests must pass with `isolate=false` (shared module state). If mocking singletons, isolate state.
58+
- Never skip tests with `.skip` without a documented reason.
59+
- Fix the actual bug first; only fix tests after the bug is confirmed fixed.
60+
- Systematically fix ALL tests in a file — work through every failure, don't leave any.

.github/copilot-instructions.md

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,59 @@ description: Project-wide coding guidelines and conventions for the Animada Scor
44

55
# Animada Score Book — Project Instructions
66

7-
Score management and arrangement app.
7+
Score management and arrangement app for Samba/Bateria groups.
88
Stack: TypeScript + Preact (Vite) frontend, Node.js backend (MySQL/PostgreSQL), Vitest + Playwright tests.
99

1010
---
1111

12+
## Architecture
13+
14+
### Entry Point & App Lifecycle
15+
16+
`src/main.tsx` mounts `<App />` into `#app`. There is **no client-side router** — the app uses an `AppPhase` state machine:
17+
18+
```
19+
Checking → Setup → AdminSetup → Login → Running
20+
```
21+
22+
`App.render()` switches on `phase` to show the appropriate splash dialog or the full app layout. `App` owns the top-level singletons: `ScoreBookDataModel`, `ArrangementPlayer`, `UndoManager`, and `services` (SelectionManager + ModeManager).
23+
24+
### Key Directories
25+
26+
| Directory | Purpose |
27+
|-----------|---------|
28+
| `src/core/` | Domain model: `ScoreBookDataModel` (central data), `Arrangement`, `Track`, `Instrument`, `TimeParams`, `edit.ts` (single edit dispatcher with discriminated unions), `UndoManager`/`UndoRedoStack` |
29+
| `src/core/serialisation/` | Snapshot serialization, packing for URL/localStorage, `ArrangementMigrator` (legacy v1/v2 + BananaDrum import) |
30+
| `src/core/types/` | Core domain types: `general.ts` (IAudioData, IArrangementSnapshot, IFraction, etc.), `edit_commands.ts` (discriminated union of all edit commands) |
31+
| `src/player/` | Audio playback engine (Web Audio API): `ArrangementPlayer` orchestrates `TrackPlayer`s + `Metronome` via `TimeCoordinator` (score-time ↔ real-time math) |
32+
| `src/supplement/` | Utilities: `Requisitions` (typed pub/sub event bus — all cross-component communication), `EscapeStack`, `Stack`, `Semaphore`, `MP3Export` |
33+
| `src/components/ui/` | Feature components: `Arrangement/`, `Bar/` (Grid + Staff views), `Note/`, `Track/`, `Minimap/`, `GuideRail/`, `InstrumentBrowser/`, `NotificationCenter/`, `Statusbar/`, `Print/`, `composites/` |
34+
| `src/components/ui/framework/` | Custom UI component library: `UIComponent` (base class), `Container` (flex layout), `Dialog`, `Button`, `Menu/`, `TreeGrid`, `Tabview/`, `Popup`, `Tooltip`, etc. |
35+
| `src/ui/` | Top-level UI modules: `ScoreLibrary` (lazy-loaded tree-grid), `SettingsDialog`, `LoginDialog`, `SelectionManager`, `ModeManager`, `MouseHandler`, `AnimationEngine`, admin editors |
36+
| `src/server/` | Node.js backend: `backend.ts` (entry), `Router.ts` (flat `?action=` dispatch), `Auth.ts` + `AuthRoutes.ts` (JWT, scrypt, refresh tokens), `ScoreRoutes.ts`, `AdminRoutes.ts`, `StaticRoutes.ts`, `mysql-adapter.ts`/`postgres-adapter.ts` (canonical schema in `createTablesSQL`), `config.ts` |
37+
| `tests/` | `tests/core/` (domain unit tests), `tests/ui/` (component tests via `@testing-library/preact`), `tests/player/` (audio engine), `tests/server/` (auth unit tests), `tests/integration/` (cross-layer), `tests/e2e/` (Playwright), `tests/temp/` (throwaway debug tests) |
38+
39+
### Data Flow
40+
41+
- **Single source of truth:** `ScoreBookDataModel` holds `arrangement`, `instruments`, `user`, and `scoreBookTree`.
42+
- **All mutations** go through `edit.ts``UndoManager.edit(command)` — every edit is a discriminated union `EditCommand`.
43+
- **Cross-component communication** uses `Requisitions` (typed pub/sub): components `register`/`unregister` for topics like `settingsChanged`, `playbackStateChanged`, `selectionChanged`, `authChanged`, `backendDisconnected`.
44+
- **Persistence:** `AppStorage` (localStorage/sessionStorage) for UI settings; backend API for scores/users/groups.
45+
46+
### Dual View System
47+
48+
The score renders in **grid mode** (matrix-style, each bar a column) or **staff mode** (vertical notation, each bar a column of track rows). Toggled via `trackViewMode` in `ArrangementViewer`. Both modes implement `ISelectionHitTester` for hit-testing. `SelectionManager` maintains two parallel selection structures: legacy per-track `currentTrackSelections` and new granular `currentSelection` (Map with `SelectionGranularity` from Track down to Note).
49+
50+
### Custom UI Framework
51+
52+
All UI components extend `UIComponent<P, S>` (not raw `Component`). Key patterns:
53+
- `Container` for all flex layouts — never raw `<div>` with `display: flex`.
54+
- `generateFinalClassName(["base-class", this.props.className])` merges classes.
55+
- `data-*` props are auto-forwarded to DOM via `ICommonUIProperties`.
56+
- Styles are per-component SCSS files under `src/components/ui/framework/styles/`, imported centrally via `styles/index.scss`.
57+
58+
---
59+
1260
## Persona
1361
- You are a senior frontend and audio engineer.
1462
- You are a TypeScript expert familiar with advanced concepts like discriminated unions, generics, decorators, conditional types and type inference.
@@ -127,7 +175,7 @@ The backend server does NOT auto-reload. Whenever files in `src/server/` change,
127175

128176
### CSS Styles
129177

130-
- CSS styles for UI framework components go into `src/components/ui/framework/<component-name>.scss`.
178+
- CSS styles for UI framework components go into `src/components/ui/framework/styles/<component-name>.scss`.
131179
- Component CSS files are imported in `src/components/ui/framework/styles/index.scss` only.
132180

133181
---

.github/skills/fix-all/SKILL.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
name: fix-all
3+
description: >-
4+
Use when: the user says "fix all errors", "make everything green", "run all checks",
5+
"fix everything", or "green build". Runs the full CI pipeline — TypeScript check,
6+
ESLint, Vitest unit tests, and Playwright e2e tests — fixing errors at each stage
7+
before moving to the next.
8+
---
9+
10+
# Fix All — Full CI Pipeline
11+
12+
Run each stage in order. Do not proceed to the next stage until the current one is fully green.
13+
Fix errors at each stage — do not skip, ignore, or work around them.
14+
15+
## Stage 1: TypeScript (`npm run check`)
16+
17+
- Run `npm run check` and collect all errors.
18+
- Fix every TS error across src/ and tests/. Common patterns:
19+
- Missing imports, wrong types, unused variables, strict null checks.
20+
- Tests use `tests/tsconfig.json` — check both tsconfigs.
21+
- Re-run until zero errors.
22+
23+
## Stage 2: ESLint (`npm run lint`)
24+
25+
- Run `npm run lint` and collect all errors and warnings.
26+
- Fix every lint issue. Do not disable rules with inline comments unless the rule is genuinely wrong for that line and the user approves.
27+
- Re-run until zero errors and zero warnings.
28+
29+
## Stage 3: Unit Tests (`npm run test`)
30+
31+
- Run `npm run test` and collect all failures.
32+
- Fix failing tests one at a time. Use `it.only` to focus on a single test while debugging, then remove `.only` and run the full suite.
33+
- Tests may share module state (Vitest runs with `isolate=false`) — if a fix causes cascading failures, check for state leakage.
34+
- Re-run until all tests pass.
35+
36+
## Stage 4: E2E Tests (`npm run test:e2e`)
37+
38+
- Build must be up to date: `npm run build` if source files changed.
39+
- Run `npm run test:e2e` and collect all failures.
40+
- E2E tests run against the production build (`dist/`), not the dev server.
41+
- Re-run until all tests pass.
42+
43+
## Final Report
44+
45+
After all four stages pass, report:
46+
- Number of errors fixed per stage
47+
- Any files that needed non-obvious changes (explain why)
48+
- Total time if available
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
name: server-change
3+
description: >-
4+
Use when: the user says they changed server files, "I modified the backend",
5+
"check server changes", "what after server changes", or after editing any file
6+
under src/server/. Runs a post-change checklist: restart reminders, schema
7+
migration checks, and relevant server tests.
8+
---
9+
10+
# Server Change Checklist
11+
12+
Run this after modifying any file under `src/server/`.
13+
14+
## 1. Check for Schema Changes
15+
16+
Review the git diff (or recent edits) for changes to `createTablesSQL` in `mysql-adapter.ts` or `postgres-adapter.ts`.
17+
If any new columns, tables, or constraints were added:
18+
19+
- **Tell the user** they need to run a manual SQL migration.
20+
- Quote the exact `ALTER TABLE` or `CREATE TABLE` statement needed.
21+
- Reminder: MySQL does not support `IF NOT EXISTS` for `ALTER TABLE ADD COLUMN` — tell the user to check with `SHOW COLUMNS FROM <table>` first.
22+
- Note that `postgres-adapter.ts` must stay in sync with `mysql-adapter.ts` — flag any drift.
23+
24+
## 2. Restart Reminder
25+
26+
The backend server does NOT auto-reload. **Always remind the user:**
27+
> Restart the server: `npm run start-debug`
28+
29+
## 3. Run Server Tests
30+
31+
- Run `npm run test -- tests/server/` to verify auth, token handling, and route logic.
32+
- If any integration/e2e tests exercise the changed endpoints, suggest running those too.
33+
34+
## 4. Security Check (if modifying auth or routes)
35+
36+
Reference the known unfixed issues from the security audit (2025-06-30):
37+
- `handleRefresh`: x-auth-type and x-group-id headers are client-controlled — do not trust them.
38+
- `handleTestConnection`: must be authenticated.
39+
- `handleListUsers`: must be admin-only.
40+
- `handleUpdateGroup`: adminId must not be reassignable by non-admins.
41+
- Request body size limits: `readJsonBody`/`readRawBody` should have a maxSize.
42+
- `handleSetup`: auth check required when users exist.
43+
44+
If the change touches any of these areas, flag the relevant issue.
45+
46+
## 5. Config Changes
47+
48+
If `backend-config.json` format or `config.ts` defaults changed, remind the user to update their local `backend-config.json`.
49+
Note which env vars (`ALLOWED_ORIGINS`, `TRUST_PROXY`, `DB_*`, `JWT_SECRET`) are affected.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"generate-themes": "tsx build/generate-daisyui-themes.ts",
2626
"fix-svgs": "tsx build/fix-svg-attributes.ts",
2727
"start": "tsx src/server/backend.ts",
28-
"start-with-dummy-secret": "JWT_SECRET=\"for-debugging\" tsx src/server/backend.ts"
28+
"start-debug": "JWT_SECRET=\"for-debugging\" tsx src/server/backend.ts"
2929
},
3030
"dependencies": {
3131
"@mediabunny/mp3-encoder": "1.39.1",

0 commit comments

Comments
 (0)