feat: Phase 1a — Map Editor by countercheck · Pull Request #1 · countercheck/triplanetary

countercheck · 2026-04-11T00:16:04Z

Summary

Gravity lib (@triplanetary/shared): pure computeGravity / hexRing functions (CJS-compatible, no external deps); 9 unit tests
Maps API (@triplanetary/server): maps Postgres table (jsonb), full CRUD routes + export endpoint, requireAdmin middleware; 9 integration tests
Map Editor UI (@triplanetary/client): /admin/map-editor route behind AdminGuard; SVG hex grid with honeycomb-grid + pan/zoom; useMapEditor hook (all editor modes, immutable state, auto-gravity); toolbar, body/base pickers, hex inspector, save/export flow via TanStack Query
Seed script: canonical Triplanetary Inner Solar System map (50 hexes, 40 gravity) seeded from db:seed

Test Plan

pnpm --filter @triplanetary/shared test → 9 gravity unit tests pass
docker compose up -d && pnpm db:migrate → maps table created
pnpm --filter @triplanetary/server test → 16 integration tests pass (auth + maps)
pnpm --filter @triplanetary/client test → 17 tests pass (AdminGuard, HexGrid, useMapEditor, LobbyPage)
pnpm db:seed → "Seeded canonical map with 50 hexes (40 gravity)"
Log in as admin, navigate to /admin/map-editor, place hexes, save, reload (persists), export JSON
Non-admin user hitting /admin/map-editor redirects to /

…avity arrows

…body placement

Copilot

Pull request overview

Implements Phase 1a of the Map Editor feature across the monorepo: shared gravity computation utilities, a Postgres-backed Maps API with CRUD/export, and an admin-only client UI for editing and persisting hex maps.

Changes:

Added @triplanetary/shared gravity utilities (computeGravity, hexRing) with Vitest unit coverage and exported them from the package.
Introduced server-side maps persistence (Drizzle schema + migration), CRUD/export API routes, admin middleware, integration tests, and a canonical seed script.
Added client-side admin map editor UI (route guarded by AdminGuard), hex grid rendering with pan/zoom, map editor state hook, and TanStack Query hooks for saving/loading maps.

Reviewed changes

Copilot reviewed 31 out of 32 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
pnpm-lock.yaml	Locks new client deps (honeycomb-grid, react-zoom-pan-pinch) and Vitest usage.
packages/shared/vitest.config.ts	Adds Vitest config for shared package tests.
packages/shared/src/types/map.ts	Extends map/hex types (weak gravity flags) and adjusts literal unions.
packages/shared/src/lib/gravity.ts	Adds gravity computation + hex ring helpers.
packages/shared/src/lib/tests/gravity.test.ts	Unit tests for `hexRing` and `computeGravity`.
packages/shared/src/index.ts	Exports new gravity utilities from shared entrypoint.
packages/shared/package.json	Adds `vitest` and a `test` script for shared package.
packages/server/src/index.ts	Mounts new `/api/maps` router into the server.
packages/server/src/db/seed/canonicalMap.ts	Seed script to insert canonical inner-solar-system map into DB.
packages/server/src/db/schema.ts	Adds `maps` table with `jsonb` map data.
packages/server/src/db/migrations/meta/0001_snapshot.json	Drizzle migration snapshot updated for `maps` table.
packages/server/src/db/migrations/meta/_journal.json	Adds new migration entry for maps table.
packages/server/src/db/migrations/0001_little_wildside.sql	SQL migration creating the `maps` table.
packages/server/src/api/routes/maps.ts	New authenticated maps list/detail + admin create/update + export endpoint.
packages/server/src/api/middleware/requireAdmin.ts	New middleware enforcing admin-only access.
packages/server/src/tests/maps.test.ts	Integration tests covering maps CRUD/export auth behavior.
packages/server/package.json	Adds `db:seed` script to run canonical map seeding.
packages/client/src/pages/admin/MapEditorPage.tsx	Main admin map editor page: grid, toolbars, save/export, recompute gravity.
packages/client/src/pages/admin/HexInspector.tsx	Hex inspector UI including gravity override controls.
packages/client/src/pages/admin/EditorToolbar.tsx	Editor mode selection + recompute gravity action.
packages/client/src/pages/admin/BodyPicker.tsx	Modal picker for placing bodies/planets.
packages/client/src/pages/admin/BasePicker.tsx	Modal picker for toggling base sides on a body.
packages/client/src/lib/apiClient.ts	Refactors API client request helper (`apiRequest`) and wiring for get/post.
packages/client/src/hooks/useMaps.ts	Adds TanStack Query hooks for listing/loading/creating/updating maps.
packages/client/src/hooks/useMapEditor.ts	Adds immutable map editor state machine + auto-gravity recomputation.
packages/client/src/components/map/HexGrid.tsx	Adds SVG hex grid rendering using honeycomb-grid + pan/zoom + gravity arrows.
packages/client/src/components/AdminGuard.tsx	Adds admin-only route guard based on `/auth/me`.
packages/client/src/App.tsx	Registers `/admin/map-editor` route behind `AdminGuard`.
packages/client/src/tests/useMapEditor.test.ts	Unit tests for editor hook behavior.
packages/client/src/tests/HexGrid.test.tsx	Component tests for basic hex grid rendering and clicks.
packages/client/src/tests/AdminGuard.test.tsx	Tests admin guard behavior using MSW.
packages/client/package.json	Adds map editor UI deps (honeycomb-grid, react-zoom-pan-pinch).

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-11T00:33:21Z

+        case 'space':
+          applyHexChange(key, { type: 'space' });
+          break;
+        case 'asteroid':
+          applyHexChange(key, { type: 'asteroid' });
+          break;
+        case 'erase':
+          applyHexChange(key, { type: 'space' });
+          break;


The editor stores explicit { type: 'space' } entries for both space and erase modes. Since rendering already treats missing keys as space, this will bloat saved map JSON as users click around and makes export/DB storage larger than necessary. Consider deleting the key from hexes for empty space (and leaving absence to mean space) instead of persisting type: 'space'.

Copilot · 2026-04-11T00:33:21Z

+export const HEX_SIZE = 48;
+
+// Hex type → fill color
+const HEX_COLORS: Record<string, string> = {
+  space: '#0a0a1a',
+  gravity: '#0d1a3a',
+  planet: '#4a7c59',
+  asteroid: '#5a4a2a',
+  clandestine: '#3a0a3a',
+};
+
+const ProtoHex = defineHex({
+  dimensions: HEX_SIZE,
+  orientation: 'pointy',
+  origin: 'topLeft',
+});


HEX_SIZE and orientation are hardcoded, ignoring data.meta.hexSize / data.meta.orientation. This will render incorrectly if a map is saved with a different hex size or flat-top orientation. Derive size/orientation from data.meta (and rebuild the ProtoHex/grid when they change).

@copilot apply changes based on this feedback

Copilot · 2026-04-11T00:33:22Z

+    a.href = url;
+    a.download = `${editor.hexData.meta.name.replace(/\s+/g, '-').toLowerCase()}.json`;
+    a.click();
+    URL.revokeObjectURL(url);


URL.revokeObjectURL(url) is called immediately after triggering the download click. Some browsers may cancel/produce a zero-byte download if the URL is revoked too early. Revoke it after a tick (e.g., setTimeout) or on a/window events.

Suggested change

URL.revokeObjectURL(url);

setTimeout(() => URL.revokeObjectURL(url), 0);

Copilot · 2026-04-11T00:33:22Z

+  const filename = `${row.name.replace(/\s+/g, '-').toLowerCase()}-v${row.version}.json`;
+  ctx.set('Content-Disposition', `attachment; filename="${filename}"`);
+  ctx.set('Content-Type', 'application/json');
+  ctx.body = JSON.stringify(row.data, null, 2);


The Content-Disposition filename is built from row.name without escaping quotes/control characters. A map name containing " or CR/LF could break the header or enable response-splitting. Sanitize the filename more defensively (strip/escape quotes and control chars) before interpolating it into the header.

@copilot apply changes based on this feedback

Copilot · 2026-04-11T00:33:22Z

+/**
+ * Computes gravity hexes for all bodies. Existing manual overrides are preserved.
+ * Non-manual gravity hexes are dropped and recomputed from scratch.
+ *
+ * @param bodies   Body definitions with center coordinates and gravityRings radius.
+ * @param existing Existing hex entries (may include manual gravity overrides).
+ * @returns        New hex record containing only gravity hexes (caller merges with non-gravity hexes).
+ */
+export function computeGravity(
+  bodies: Record<string, BodyEntry>,
+  existing: Record<string, HexEntry> = {},
+): Record<string, HexEntry> {
+  // Carry forward manual overrides only; drop non-manual gravity (will recompute)
+  const result: Record<string, HexEntry> = Object.fromEntries(
+    Object.entries(existing).filter(
+      ([, h]) => !(h.type === 'gravity' && !h.manual),
+    ),
+  );


The JSDoc says computeGravity “returns … containing only gravity hexes (caller merges with non-gravity hexes)”, but the implementation preserves all non-gravity entries in existing (it only filters out non-manual gravity). Either update the docstring to match behavior or change the function to actually return only gravity entries to avoid misleading callers.

@copilot apply changes based on this feedback

- Add eslint.config.js (flat config) with rules for server/shared (Node) and client (React + browser) - Add lint and typecheck scripts to root and turbo pipeline - Add .github/workflows/ci.yml: runs lint, typecheck, test, and build on PRs and pushes to main; includes Postgres 17 service container for server integration tests - Fix bgio/server.ts TS2742/TS4023 with explicit ReturnType<typeof Server> - Fix session.ts and auth.ts ESLint/typecheck issues

…ithub-actions

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…tions ci: ESLint + typecheck + GitHub Actions CI workflow

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-04-11T05:08:11Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2026-04-11T05:09:35Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

Copilot · 2026-04-11T05:09:54Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

- lefthook pre-commit: runs pnpm lint + pnpm test before every git commit - Claude Code PostToolUse hook: lints any .ts/.tsx file immediately after each Edit/Write/MultiEdit tool call via scripts/post-edit-lint.sh - Add "prepare": "lefthook install" so hooks self-install on pnpm install

…ons in settings.local.json

… Postgres so CI-only

…utes mapId

…t handleHexClick

…hParams to deps

…nd client pages - Fix pgStore expire column bug: use NOW() + interval instead of passing a JS Date, avoiding timestamp-without-timezone serialization issues - Add vitest fork isolation (pool: forks) to prevent pool.end() cross-test contamination - New server tests: session pgStore get/set/destroy (8), auth input validation (7), maps export filename sanitization (6) - New client tests: apiClient (8), useAuth (4), useMaps (6), LoginPage (4), RegisterPage (5), MapEditorPage (6) - Update Claude Code project settings with PostToolUse lint hooks

Add TCP socket poll loop in CI workflow to ensure Postgres is accepting connections before pnpm db:migrate runs. Also add select-1 retry loop in migrate.ts for local robustness.

Turbo v2 does not forward env vars to task subprocesses unless declared in turbo.json. When pnpm db:migrate ran through turbo, the Pool received connectionString=undefined and fell back to port 5432 (default), causing ECONNREFUSED. Fix: CI runs pnpm --filter @triplanetary/server db:migrate directly, bypassing turbo entirely. Also declare DATABASE_URL in turbo.json env for the db:migrate task so 'pnpm db:migrate' works correctly locally.

- HexGrid: map 'pointy'/'flat' strings to Orientation enum (honeycomb-grid uses POINTY/FLAT, not raw strings) - apiClient: conditionally spread body to satisfy exactOptionalPropertyTypes (body: undefined is not assignable to BodyInit | null) - apiClient.test: use rejects.toMatchObject instead of catch-and-cast to avoid TS18046 'err is of type unknown' - lefthook: add typecheck command to pre-commit hook

0001_add_session_table.sql was created outside Drizzle and never added to _journal.json, so the migrator silently skipped it in CI. Added as 0002_add_session_table with IF NOT EXISTS (idempotent) and timestamptz for the expire column to match the NOW() + interval store logic. Removed the orphaned 0001_add_session_table.sql file.

Koa processes middleware in registration order. Both maps.test.ts and maps-export.test.ts were mounting routes before session/passport, so ctx.isAuthenticated() always returned false and every authenticated request got 401. Also run the build job in parallel with lint-typecheck and test.

Wrapping the /api router inside a plain Router() caused @koa/router to drop matched params (e.g. :id) when the child routes ran, producing params: undefined and a failing Drizzle query. Match production layout: mount the /api prefixed router directly on app.

Vite was resolving @triplanetary/shared to the CJS dist/ during production build. Rollup can't statically analyse CJS __exportStar, so named exports like SCENARIOS appeared missing. Add the same resolve alias used in vitest.config.ts so Vite always compiles directly from the TypeScript source.

…op pool.end

Copilot

Pull request overview

Copilot reviewed 58 out of 60 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T01:42:03Z

+import path from "node:path";
+import { config } from "dotenv";

 // Load .env from monorepo root (src/ → server/ → packages/ → root)
-config({ path: path.resolve(__dirname, '../../../.env') });
+config({ path: path.resolve(__dirname, "../../../.env") });


dotenv.config() here won’t reliably populate process.env for modules imported below when the server is compiled as CommonJS: TypeScript emits all require(...) calls before this config(...) line, and createSessionMiddleware imports pool from db/client, which reads process.env.DATABASE_URL at module-eval time. This can result in Pool({ connectionString: undefined }) unless the env var is set externally. Consider moving dotenv loading into src/db/client.ts (before Pool construction), or using a tiny bootstrap entrypoint that calls config() and then require('./index'), or switching to a top-level import 'dotenv/config' + DOTENV_CONFIG_PATH so env is loaded before other imports.

@copilot apply changes based on this feedback

Fixed via the bootstrap entrypoint approach. Added src/bootstrap.ts which imports only path and dotenv (no env-reading side effects), calls config(), then dynamically require('./index').

The key insight: because require('./index') is a body statement in bootstrap.ts, it executes after config() has populated process.env. Every module in index.ts's import tree — including bgio/server.ts reading BGIO_ALLOWED_ORIGINS and db/client.ts constructing new Pool({ connectionString: process.env.DATABASE_URL }) — is now evaluated after dotenv has loaded.

Updated dev and start scripts to use bootstrap.ts/bootstrap.js. Tests are unaffected — vitest.config.ts injects env vars directly and never relies on .env.

Generated by Claude Code

Copilot · 2026-04-13T01:42:03Z

+  it("calls POST /api/maps on save for new map and updates URL", async () => {
+    let posted = false;
+    mswServer.use(
+      http.post("/api/maps", async () => {
+        posted = true;


This test is named as if it verifies a Save flow (POST + URL update), but it never makes the editor dirty, never clicks the Save button, and never asserts posted === true or that the URL/search params changed. As written, it will pass even if the POST/save logic is broken. Update the test to drive UI interactions (e.g., switch to a mode that mutates state, click a hex to set dirty, click Save) and then assert the POST handler was called and the search param updates.

@copilot apply changes based on this feedback

Copilot · 2026-04-13T01:42:04Z

+  it("calls PUT /api/maps/:id on save for existing map", async () => {
+    let putCalled = false;
+    mswServer.use(
+      http.get("/api/maps/map-1", () => HttpResponse.json(SAVED_MAP)),
+      http.put("/api/maps/map-1", async () => {


This test name claims it "calls PUT ... on save", but the test only loads the map and asserts there was no spurious PUT during load; it never makes the editor dirty or clicks Save, so it doesn’t validate the update flow. Either rename it to reflect what it checks, or extend it to perform a save interaction and assert the PUT handler was invoked.

@copilot apply changes based on this feedback

Copilot · 2026-04-13T01:42:04Z

+beforeAll(async () => {
+  await pool.query("DELETE FROM users WHERE email = 'export-test@test.com'");
+  await pool.query("DELETE FROM maps WHERE name LIKE 'Export%'");
+


This test file inserts rows into users / maps (including multiple Export* maps) but only cleans them up in beforeAll. If the file is re-run locally, or if a test fails mid-run, these rows can accumulate and potentially interfere with other integration tests. Add an afterAll cleanup for the export-test@test.com user and maps WHERE name LIKE 'Export%' (and consider ending the pool only if each test file has its own isolated pool).

@copilot apply changes based on this feedback

Copilot · 2026-04-13T01:42:04Z

  "permissions": {
    "allow": [
-      "Bash(pnpm --filter @triplanetary/server db:migrate)",
-      "Bash(DATABASE_URL=postgresql://triplanetary:triplanetary@localhost:5432/triplanetary pnpm db:migrate)",
-      "Bash(DATABASE_URL=postgresql://triplanetary:triplanetary@localhost:5432/triplanetary ./node_modules/.bin/drizzle-kit migrate)",
-      "Bash(docker exec:*)",
-      "Bash(pnpm --filter @triplanetary/server test)",
-      "Bash(pnpm view:*)",
-      "Bash(node -e \"import\\('boardgame.io/server'\\).then\\(m => console.log\\(Object.keys\\(m\\)\\)\\)\")",
-      "Bash(node -e \"import\\('boardgame.io/server'\\).then\\(m => { const s = m.Server\\({ games: [] }\\); console.log\\(Object.keys\\(s\\)\\); }\\)\")",
-      "Bash(node -e \"const m = require\\('boardgame.io/server'\\); const s = m.Server\\({ games: [] }\\); console.log\\(Object.keys\\(s\\)\\);\")",
-      "Bash(node -e \"const m = require\\('boardgame.io/server'\\); const s = m.Server\\({ games: [], origins: ['http://localhost:3000'] }\\); console.log\\(s.app.constructor.name\\); console.log\\(typeof s.app.use\\);\")",
-      "Bash(node -e \"const m = require\\('./packages/server/node_modules/boardgame.io/dist/cjs/server.js'\\); const s = m.Server\\({ games: [], origins: ['http://localhost:3000'] }\\); console.log\\(s.app.constructor.name\\); console.log\\(typeof s.app.use\\); console.log\\(typeof s.router\\);\")",
-      "Bash(node -e \"const m = require\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/boardgame.io@0.50.2/node_modules/boardgame.io/dist/cjs/server.js'\\); const s = m.Server\\({ games: [], origins: ['http://localhost:3000'] }\\); console.log\\('app type:', s.app.constructor.name\\); console.log\\('router type:', typeof s.router\\); console.log\\('router keys:', s.router && Object.keys\\(s.router\\).slice\\(0,5\\)\\);\")",
-      "Bash(timeout 8 tsx src/index.ts)",
-      "Bash(pnpm --filter @triplanetary/server exec timeout 8 tsx src/index.ts)",
-      "Bash(curl -s localhost:8000/api/health)",
-      "Bash(curl -s localhost:8000/games)",
-      "Bash(node -e \"const p = require\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/boardgame.io@0.50.2/node_modules/boardgame.io/package.json'\\); console.log\\(JSON.stringify\\(p.exports, null, 2\\)\\)\")",
-      "Bash(node -e \"const p = require\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/boardgame.io@0.50.2/node_modules/boardgame.io/package.json'\\); console.log\\(p.main, p.module, p.types\\)\")",
-      "Read(//private/tmp/**)",
-      "Bash(node -e \"const f = require\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/is-generator-function@1.1.2/node_modules/generator-function/index.js'\\); console.log\\(typeof f, f\\(\\)\\)\")",
-      "Bash(node -e \"const f = require\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/is-generator-function@1.1.2/node_modules/is-generator-function/index.js'\\); console.log\\(f\\(async function\\(\\){}\\), f\\(function*\\(\\){}\\)\\)\")",
-      "Bash(node -e \"const cjs = require\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/boardgame.io@0.50.2/node_modules/boardgame.io/dist/cjs/server.js'\\); const s = cjs.Server\\({ games: [], origins: ['http://localhost:3000'] }\\); s.app.use\\(function\\(ctx, next\\) { return next\\(\\); }\\); console.log\\('koa app.use works'\\)\")",
-      "Bash(node -e ':*)",
-      "Bash(node -e \"const src = require\\('fs'\\).readFileSync\\('/Users/sam/Code/triplanetary/node_modules/.pnpm/boardgame.io@0.50.2/node_modules/boardgame.io/dist/cjs/server.js', 'utf8'\\); const idx = src.indexOf\\('run\\('\\); console.log\\(src.slice\\(idx, idx+600\\)\\);\")",
-      "Bash(pnpm add:*)",
-      "Bash(pnpm --filter @triplanetary/client test)"
+      "Bash(ls /Users/sam/Code/triplanetary/node_modules/.pnpm/honeycomb-grid*)"
    ]


The permissions.allow entry hard-codes a developer-specific absolute path (/Users/sam/...). Committing this makes the config non-portable and can cause confusing behavior for other contributors. Prefer removing the machine-specific allow rule, or replacing it with a repo-relative command/pattern that works across environments.

@copilot apply changes based on this feedback

Copilot · 2026-04-13T01:56:17Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

Copilot · 2026-04-13T01:59:31Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

Copilot · 2026-04-13T02:00:04Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

Copilot · 2026-04-13T02:00:15Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

Copilot · 2026-04-13T02:00:48Z

@countercheck I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

In CJS, TypeScript compiles static import statements to top-level require() calls that all run before the importing module's body. Placing dotenv.config() in index.ts body meant it always fired after db/client.ts created its Pool (reading DATABASE_URL as undefined) and after bgio/server.ts read BGIO_ALLOWED_ORIGINS — both from .env. Add src/bootstrap.ts as the new entry point: it calls config() then dynamically require('./index'). Because require('./index') executes as a body statement (after config()), every module in the index.ts import tree sees the populated process.env when their bodies run. Update dev and start scripts to use bootstrap.ts / bootstrap.js. Tests are unaffected — vitest.config.ts injects env vars directly. https://claude.ai/code/session_01DeLbhCjrgM6A71jTPZCrsn

- maps-export.test.ts: add afterAll DB cleanup for export-test rows. Previously only the server was closed; re-runs or mid-run failures could leave Export* maps and the export-test user in the DB. - MapEditorPage.test.tsx: fix two tests that claimed to test the save flow but never clicked Save. Both tests now switch to asteroid mode, click a hex to dirty the map, click Save, and assert the expected HTTP handler was invoked. The PUT test retains its existing "no spurious PUT on load" assertion before the save interaction. - .claude/settings.json: remove machine-specific absolute path (/Users/sam/...) from permissions.allow — non-portable across contributor environments. https://claude.ai/code/session_01DeLbhCjrgM6A71jTPZCrsn

- maps-export.test.ts: wrap server.close() in a Promise so the async afterAll actually waits for the server to shut down before Vitest exits, preventing open-handle warnings and teardown races. - MapEditorPage.test.tsx: add a waitFor assertion after the POST save that verifies "Saved Map" appears. After setSearchParams updates the URL to ?id=new-map-id, useMap fires GET /api/maps/new-map-id and loads the returned map — confirming the URL update actually happened. https://claude.ai/code/session_01DeLbhCjrgM6A71jTPZCrsn

Copilot

Pull request overview

Copilot reviewed 59 out of 61 changed files in this pull request and generated 8 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T16:39:41Z

+  const [row] = await db
+    .select()
+    .from(maps)
+    .where(eq(maps.id, ctx.params["id"] ?? ""));
+  if (!row) {


These queries pass ctx.params["id"] directly into eq(maps.id, ...) where maps.id is a uuid column. If a client sends a non-UUID id, Postgres will throw an invalid input syntax for type uuid error and this route will likely return 500. Consider validating :id as a UUID up front and returning 400 for invalid ids (same applies to the PUT and export endpoints).

Copilot · 2026-04-13T16:39:41Z

+  const id = ctx.params["id"] ?? "";
+  const [existing] = await db
+    .select({ id: maps.id })
+    .from(maps)
+    .where(eq(maps.id, id));
+  if (!existing) {


Same UUID-validation issue as the GET route: where(eq(maps.id, id)) will throw (and likely 500) for non-UUID :id values. Consider rejecting invalid UUIDs with a 400 before querying.

Copilot · 2026-04-13T16:39:41Z

+  const [row] = await db
+    .select()
+    .from(maps)
+    .where(eq(maps.id, ctx.params["id"] ?? ""));
+  if (!row) {


Same UUID-validation issue here: where(eq(maps.id, ctx.params["id"] ?? "")) can throw and return 500 when :id is not a valid UUID. Consider validating :id and returning 400 before executing the query.

Copilot · 2026-04-13T16:39:42Z

+        // Do not overwrite planet, asteroid, or clandestine hexes with gravity
+        const existingHex = result[key];
+        if (existingHex && existingHex.type !== "gravity") continue;


computeGravity currently treats any existing non-gravity hex as blocking (if (existingHex && existingHex.type !== "gravity") continue). Since HexType includes "space" and the Maps API tests already create maps containing { type: "space" } entries, those “space” entries will prevent gravity from being generated on those coordinates. Consider only blocking on truly-occupied types (e.g. planet/asteroid/clandestine), or explicitly allowing space to be overwritten by gravity.

Copilot · 2026-04-13T16:39:42Z

+    editor.loadHexData({
+      ...editor.hexData,
+      hexes: { ...nonGravity, ...gravityHexes },
+    });


handleRecomputeGravity calls editor.loadHexData(...), but loadHexData resets dirty to false and clears selectedHex. That means clicking “Recompute All Gravity” can silently lose selection and disable Save even though the map was modified. Prefer updating hexData through a method that preserves selection and sets dirty=true (or add a dedicated setHexData/replaceHexes API on the hook for this).

Suggested change

editor.loadHexData({

...editor.hexData,

hexes: { ...nonGravity, ...gravityHexes },

});

editor.replaceHexes({ ...nonGravity, ...gravityHexes });

Copilot · 2026-04-13T16:39:42Z

+        case "gravity":
+          applyHexChange(key, {
+            type: "gravity",
+            offset: [0, 1],
+            manual: false,
+          });


In gravity mode you create { type: "gravity", offset: [0,1], manual: false } without a body. Because computeGravity drops non-manual gravity entries on recompute, any user-placed gravity hex will be deleted the next time gravity is recomputed (e.g. when placing a planet) and it can’t be associated to a source body. Consider marking user-placed gravity as manual: true and/or requiring/deriving the body field so the saved map data stays consistent.

Copilot · 2026-04-13T16:39:42Z

+  const svgWidth = (qMax - qMin + 2) * hexSize * Math.sqrt(3);
+  const svgHeight = (rMax - rMin + 2) * hexSize * 1.5;


svgWidth/svgHeight are computed using pointy-top spacing constants (sqrt(3) for width and 1.5 for height). Since the grid can be data.meta.orientation === "flat", flat-top maps can end up with incorrect SVG bounds/clipping. Consider branching these formulas based on orientation.

Suggested change

const svgWidth = (qMax - qMin + 2) * hexSize * Math.sqrt(3);

const svgHeight = (rMax - rMin + 2) * hexSize * 1.5;

const svgWidth =

(qMax - qMin + 2) *

hexSize *

(orientation === "pointy" ? Math.sqrt(3) : 1.5);

const svgHeight =

(rMax - rMin + 2) *

hexSize *

(orientation === "pointy" ? 1.5 : Math.sqrt(3));

Copilot · 2026-04-13T16:39:43Z

+                      const dx =
+                        hexSize * (Math.sqrt(3) * dq + (Math.sqrt(3) / 2) * dr);
+                      const dy = hexSize * (1.5 * dr);
+                      const len = Math.sqrt(dx * dx + dy * dy);


The gravity arrow’s axial→pixel conversion uses pointy-top geometry (dx = sqrt(3)*dq + sqrt(3)/2*dr, dy = 1.5*dr). If orientation is "flat", these constants should change, otherwise arrows will point/render incorrectly. Consider computing dx/dy based on the active orientation.

Suggested change

const dx =

hexSize * (Math.sqrt(3) * dq + (Math.sqrt(3) / 2) * dr);

const dy = hexSize * (1.5 * dr);

const len = Math.sqrt(dx * dx + dy * dy);

const isFlatOrientation =

orientation === Orientation.FLAT || orientation === "flat";

const dx = isFlatOrientation

? hexSize * (1.5 * dq)

: hexSize *

(Math.sqrt(3) * dq + (Math.sqrt(3) / 2) * dr);

const dy = isFlatOrientation

? hexSize *

((Math.sqrt(3) / 2) * dq + Math.sqrt(3) * dr)

: hexSize * (1.5 * dr);

const len = Math.sqrt(dx * dx + dy * dy);

if (len === 0) {

return null;

}

countercheck added 9 commits April 10, 2026 18:26

feat(shared): gravity auto-computation lib with unit tests

ae30214

feat(server): maps table, CRUD API routes, requireAdmin middleware

fe9c6f0

feat(client): AdminGuard component and /admin/map-editor route

a09828e

feat(client): HexGrid SVG component with honeycomb-grid, pan/zoom, gr…

2b34219

…avity arrows

feat(client): useMapEditor hook — immutable state, all editor modes, …

1b51bc4

…body placement

feat(client): map editor UI — toolbar, body/base pickers, hex inspector

5ee38ea

feat(client): useMaps hook and save/export flow in map editor

36a96a6

feat(server): canonical Triplanetary Inner Solar System map seed

fd93e93

chore: add honeycomb-grid and react-zoom-pan-pinch dependencies

fc0fa74

countercheck requested a review from Copilot April 11, 2026 00:28

Copilot started reviewing on behalf of countercheck April 11, 2026 00:29 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

countercheck and others added 7 commits April 10, 2026 21:24

docs: add phase-1a map editor implementation plan

e3fa9e8

Merge branch 'feature/phase-1a-map-editor' into ci/eslint-typecheck-g…

59ff472

…ithub-actions

Update .github/workflows/ci.yml

2448f8e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge pull request #2 from countercheck/ci/eslint-typecheck-github-ac…

b8d0ea6

…tions ci: ESLint + typecheck + GitHub Actions CI workflow

Update packages/client/src/components/AdminGuard.tsx

ad9d0cc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update packages/client/src/hooks/useMapEditor.ts

eb75310

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update packages/server/src/api/middleware/requireAdmin.ts

cac0959

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

countercheck added 7 commits April 11, 2026 12:08

fix(ci): remove explicit pnpm version, read from packageManager field

1cc3cc9

fix(hooks): propagate eslint exit code so Claude sees lint errors

0af0cd0

fix: guard lefthook install behind .git check

e67cf33

chore: split Claude settings — hooks in settings.json, local permissi…

dc1a038

…ons in settings.local.json

fix(hooks): pass -- before file path to prevent flag injection in eslint

07e3f40

fix(hooks): pre-commit tests only shared+client; server tests require…

ef803d2

… Postgres so CI-only

countercheck added 15 commits April 12, 2026 16:07

fix(editor): use setSearchParams after create so React Router re-comp…

5ed4108

…utes mapId

fix(maps): guard useMap query against empty string id

3d46bb0

fix(editor): wire HexInspector Erase button to dedicated eraseHex, no…

49667fe

…t handleHexClick

fix(lint): suppress unused destructure var in deleteHex, add setSearc…

31e5600

…hParams to deps

fix(ci): wait for Postgres before running migrations

2df1c39

Add TCP socket poll loop in CI workflow to ensure Postgres is accepting connections before pnpm db:migrate runs. Also add select-1 retry loop in migrate.ts for local robustness.

ci: run lint+typecheck and test in parallel jobs

baaae41

fix(tests): scope auth cleanup to own email, fix middleware order, dr…

8fc52aa

…op pool.end

docs: add Decision Log section to CLAUDE.md

bcabba3

countercheck requested a review from Copilot April 13, 2026 01:36

Copilot started reviewing on behalf of countercheck April 13, 2026 01:36 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

claude added 3 commits April 13, 2026 11:11

countercheck requested a review from Copilot April 13, 2026 16:26

Copilot started reviewing on behalf of countercheck April 13, 2026 16:27 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

countercheck merged commit 1c25179 into main Apr 13, 2026
7 checks passed

	URL.revokeObjectURL(url);
	setTimeout(() => URL.revokeObjectURL(url), 0);

		const svgWidth = (qMax - qMin + 2) * hexSize * Math.sqrt(3);
		const svgHeight = (rMax - rMin + 2) * hexSize * 1.5;

-                      const dx =
-                        hexSize * (Math.sqrt(3) * dq + (Math.sqrt(3) / 2) * dr);
-                      const dy = hexSize * (1.5 * dr);
-                      const len = Math.sqrt(dx * dx + dy * dy);
+                      const isFlatOrientation =
+                        orientation === Orientation.FLAT || orientation === "flat";
+                      const dx = isFlatOrientation
+                        ? hexSize * (1.5 * dq)
+                        : hexSize *
+                            (Math.sqrt(3) * dq + (Math.sqrt(3) / 2) * dr);
+                      const dy = isFlatOrientation
+                        ? hexSize *
+                            ((Math.sqrt(3) / 2) * dq + Math.sqrt(3) * dr)
+                        : hexSize * (1.5 * dr);
+                      const len = Math.sqrt(dx * dx + dy * dy);
+                      if (len === 0) {
+                        return null;
+                      }

Conversation

countercheck commented Apr 11, 2026

Summary

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Apr 11, 2026

Uh oh!

Copilot AI commented Apr 11, 2026

Uh oh!

Copilot AI commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Apr 13, 2026

Uh oh!

Copilot AI commented Apr 13, 2026

Uh oh!

Copilot AI commented Apr 13, 2026

Uh oh!

Copilot AI commented Apr 13, 2026

Uh oh!

Copilot AI commented Apr 13, 2026