86 lines
5.7 KiB
Markdown
86 lines
5.7 KiB
Markdown
Project TODO and context
|
|
|
|
This TODO collects remaining tasks and diagnostic steps for the "peddlers-of-ketran" project,
|
|
especially focused on making headless harness runs deterministic and actionable.
|
|
|
|
High-level goals
|
|
- Fix UI bugs (House Rules read-only on new-game load).
|
|
- Make startup deterministic: server should send a consolidated attach-time "initial-game" snapshot and
|
|
the client should treat that as a full update so headless tests can start at a known state.
|
|
- Add an instrumented headless harness that captures lifecycle artifacts (DOMContentLoaded snapshot,
|
|
DOM when inputs first appear, screenshots, and intercepted WebSocket sends). Use this harness in CI.
|
|
|
|
Immediate tasks (priority order)
|
|
1) Add an assertion in the Puppeteer harness to fail early if the client doesn't send a player-name
|
|
message with the expected name (Automaton). This prevents flakey runs from masking regressions.
|
|
- Location: tools/puppeteer-test/test.js
|
|
- Implementation: after saving /workspace/tmp-ws-sends-<ts>.json, parse each intercepted send and
|
|
verify there exists an entry where JSON.parse(entry.data).type === 'player-name' and
|
|
extracted name === 'Automaton'. Fail the test if not present.
|
|
|
|
2) Confirm server-side handler accepts normalized payloads everywhere.
|
|
- Location: server/routes/games.js
|
|
- Implementation: Add a small normalizeIncoming(msg) helper used at the top of the ws.on('message')
|
|
handler that returns a consistent object shape: { type, data } where data may contain name, etc.
|
|
- Rationale: reduces repeated defensive parsing and prevents future undefined values.
|
|
|
|
3) Investigate duplicate player-name sends from the client.
|
|
- Symptom: harness observed two identical player-name sends per run.
|
|
- Approach: add client-side debounce or guard after the first send; or only send when the name actually changes.
|
|
- Files: client/src/PlayerName.tsx or client/src/App.tsx (where normalizedSend is invoked).
|
|
|
|
4) Verify persistence of name change in DB / game state.
|
|
- Query: after setPlayerName completes, check game's session object and the games DB to ensure changes persisted.
|
|
- Tools: sqlite3 queries against db/users.db or db/games.db, or add a short admin HTTP endpoint for inspection.
|
|
|
|
5) Harden the harness reporting and CI integration.
|
|
- Add explicit exit codes and artifact names so CI can surface failures.
|
|
- Add a GH Action that spins up the compose stack and runs the peddlers-test service inside the compose network.
|
|
|
|
New / follow-up actions (from recent session)
|
|
|
|
6) Re-run the instrumented harness while streaming server logs to capture exact server-side handling.
|
|
- Goal: get a single, reproducible run that writes `/workspace/tmp-ws-sends-<ts>.json` and produce
|
|
a server log snippet showing setPlayerName processing for the same timestamp.
|
|
- Acceptance criteria:
|
|
- Harness exits 0 (or the expected non-zero on assert failure) and artifacts are present.
|
|
- Server logs include a `setPlayerName` line mentioning the same name (Automaton) and a timestamp
|
|
that can be correlated to the harness-run (or the harness writes the server log lines into an artifact).
|
|
- Quick command (dev):
|
|
```bash
|
|
# run the harness and tail server logs in parallel (dev machine)
|
|
docker compose -f docker-compose.yml logs --no-color --follow server &
|
|
docker compose -f docker-compose.yml run --rm -e TEST_MAX_MS=20000 peddlers-test
|
|
```
|
|
- Notes: If you want, I can run this now and capture the correlated logs/artifacts.
|
|
|
|
7) Consolidate server-side message normalization helper.
|
|
- Goal: add a single `normalizeIncoming(raw)` helper in `server/routes/games.js` (or a small util) that
|
|
returns { type, data } and is used at the top of the websocket message handler so each case can assume
|
|
the canonical shape.
|
|
- Acceptance criteria:
|
|
- No functional changes to handlers beyond switching to the normalized shape.
|
|
- Unit/regression smoke: a quick local run validates `player-name` and a couple other common message types
|
|
still work (server logs unchanged behavior).
|
|
- Implementation sketch:
|
|
- add `function normalizeIncoming(msg) { try { const parsed = typeof msg === 'string' ? JSON.parse(msg) : msg; return { type: parsed.type, data: parsed.data || { ...parsed, ...(parsed.data||{}) } }; } catch(e){ return { type: null, data: null }; } }`
|
|
- call at the top of `ws.on('message', (m) => { const incoming = normalizeIncoming(m); switch(incoming.type) { ... }})`
|
|
|
|
8) Add a CI workflow that runs the harness inside the project's canonical container environment.
|
|
- Goal: run the harness as part of PR checks so regressions are caught early.
|
|
- Acceptance criteria:
|
|
- A GitHub Actions workflow `ci/harness.yml` is present that uses the project's Dockerfile/docker-compose
|
|
to run services and executes the `peddlers-test` service; artifacts (tmp-ws-sends-*.json, screenshots) are
|
|
uploaded as job artifacts on failure.
|
|
- Minimal approach:
|
|
- Spin up services via docker compose in the action runner (use the repo's Dockerfile images to avoid host npm installs),
|
|
run `docker compose run --rm peddlers-test`, collect artifacts, and fail the job on non-zero exit.
|
|
- Notes: Follow the repo-level guidance in `.github/copilot-instructions.md` — do not run `npm install` on the host; use the provided Docker workflows.
|
|
|
|
Lower priority / follow-ups
|
|
- Add server unit tests for message parsing and setPlayerName behavior.
|
|
- Consolidate log markers and timestamps to make correlation between client sends and server logs easier.
|
|
- Add a small README for the harness explaining how to run it locally with Docker Compose.
|
|
|
|
If you want, I can implement item #1 now (fail-fast assertion in the harness) and re-run the harness to demonstrate.
|