Merge pull request 'v1.2.0' (#10) from dev into main

Reviewed-on: #10

.gitea/workflows/ci.yml

@@ -6,6 +6,9 @@ on:
   pull_request:
     branches: [dev, main]
 
+env:
+  IMAGE: ${{ vars.REGISTRY_HOST }}/${{ gitea.repository_owner }}/catalyst
+
 jobs:
   test:
     runs-on: ubuntu-latest
@@ -16,8 +19,35 @@ jobs:
       - uses: actions/setup-node@v4
         with:
           node-version: 'lts/*'
-          cache: npm
 
       - run: npm ci
 
       - run: npm test
+
+  build-dev:
+    runs-on: ubuntu-latest
+    needs: test
+    if: github.event_name == 'push' && github.ref == 'refs/heads/dev'
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Log in to registry
+        uses: docker/login-action@v3
+        with:
+          registry: ${{ vars.REGISTRY_HOST }}
+          username: ${{ gitea.actor }}
+          password: ${{ secrets.TOKEN }}
+
+      - name: Compute short SHA
+        run: echo "SHORT_SHA=$(git rev-parse --short HEAD)" >> $GITEA_ENV
+
+      - name: Build and push
+        uses: docker/build-push-action@v5
+        with:
+          context: .
+          push: true
+          build-args: BUILD_VERSION=dev-${{ env.SHORT_SHA }}
+          tags: |
+            ${{ env.IMAGE }}:dev
+            ${{ env.IMAGE }}:dev-${{ gitea.sha }}

Dockerfile

@@ -7,10 +7,15 @@ COPY package*.json ./
 RUN npm ci --omit=dev
 
 COPY . .
-RUN awk -F'"' '/"version"/{printf "const VERSION = \"%s\";\n", $4; exit}' \
+ARG BUILD_VERSION=""
-    package.json > js/version.js
+RUN if [ -n "$BUILD_VERSION" ]; then \
+      printf 'const VERSION = "%s";\n' "$BUILD_VERSION" > js/version.js; \
+    else \
+      awk -F'"' '/"version"/{printf "const VERSION = \"%s\";\n", $4; exit}' \
+          package.json > js/version.js; \
+    fi
 
-RUN chown -R app:app /app
+RUN mkdir -p /app/data && chown -R app:app /app
 USER app
 
 EXPOSE 3000

css/app.css

@@ -289,6 +289,7 @@ select:focus { border-color: var(--accent); }
   border-radius: 3px;
   letter-spacing: 0.08em;
   text-transform: uppercase;
+  text-align: center;
 }
 
 .badge.deployed    { background: var(--accent2); color: var(--accent); }

js/app.js

@@ -38,6 +38,9 @@ window.addEventListener('popstate', e => {
 
 // ── Bootstrap ─────────────────────────────────────────────────────────────────
 
-if (VERSION) document.getElementById('nav-version').textContent = `v${VERSION}`;
+if (VERSION) {
+  const label = /^\d/.test(VERSION) ? `v${VERSION}` : VERSION;
+  document.getElementById('nav-version').textContent = label;
+}
 
 handleRoute();

package.json

@@ -1,6 +1,6 @@
 {
   "name": "catalyst",
-  "version": "1.1.2",
+  "version": "1.2.0",
   "type": "module",
   "scripts": {
     "start": "node server/server.js",

server/db.js

@@ -133,5 +133,18 @@ export function _resetForTest() {
 }
 
 // ── Boot ──────────────────────────────────────────────────────────────────────
+// Skipped in test environment — parallel Vitest workers would race to open
+// the same file, causing "database is locked". _resetForTest() in beforeEach
+// handles initialisation for every test worker using :memory: instead.
 
-init(process.env.DB_PATH ?? DEFAULT_PATH);
+if (process.env.NODE_ENV !== 'test') {
+  const DB_PATH = process.env.DB_PATH ?? DEFAULT_PATH;
+  try {
+    init(DB_PATH);
+  } catch (e) {
+    console.error('[catalyst] fatal: could not open database at', DB_PATH);
+    console.error('[catalyst] ensure the data directory exists and is writable by the server process.');
+    console.error(e);
+    process.exit(1);
+  }
+}

server/routes.js

@@ -78,7 +78,8 @@ router.post('/instances', (req, res) => {
   } catch (e) {
     if (e.message.includes('UNIQUE')) return res.status(409).json({ error: 'vmid already exists' });
     if (e.message.includes('CHECK'))  return res.status(400).json({ error: 'invalid field value' });
-    throw e;
+    console.error('POST /api/instances', e);
+    res.status(500).json({ error: 'internal server error' });
   }
 });
 
@@ -98,7 +99,8 @@ router.put('/instances/:vmid', (req, res) => {
   } catch (e) {
     if (e.message.includes('UNIQUE')) return res.status(409).json({ error: 'vmid already exists' });
     if (e.message.includes('CHECK'))  return res.status(400).json({ error: 'invalid field value' });
-    throw e;
+    console.error('PUT /api/instances/:vmid', e);
+    res.status(500).json({ error: 'internal server error' });
   }
 });
 
@@ -112,6 +114,11 @@ router.delete('/instances/:vmid', (req, res) => {
   if (instance.stack !== 'development')
     return res.status(422).json({ error: 'only development instances can be deleted' });
 
-  deleteInstance(vmid);
+  try {
-  res.status(204).end();
+    deleteInstance(vmid);
+    res.status(204).end();
+  } catch (e) {
+    console.error('DELETE /api/instances/:vmid', e);
+    res.status(500).json({ error: 'internal server error' });
+  }
 });

tests/api.test.js

@@ -1,7 +1,8 @@
-import { describe, it, expect, beforeEach } from 'vitest'
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'
 import request from 'supertest'
 import { app } from '../server/server.js'
 import { _resetForTest } from '../server/db.js'
+import * as dbModule from '../server/db.js'
 
 beforeEach(() => _resetForTest())
 
@@ -277,3 +278,74 @@ describe('static assets and SPA routing', () => {
     expect(res.text).toContain('<base href="/">')
   })
 })
+
+// ── Error handling — unexpected DB failures ───────────────────────────────────
+
+const dbError = () => Object.assign(
+  new Error('attempt to write a readonly database'),
+  { code: 'ERR_SQLITE_ERROR', errcode: 8 }
+)
+
+describe('error handling — unexpected DB failures', () => {
+  let consoleSpy
+
+  beforeEach(() => {
+    consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
+  })
+
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
+  it('POST returns 500 with friendly message when DB throws unexpectedly', async () => {
+    vi.spyOn(dbModule, 'createInstance').mockImplementationOnce(() => { throw dbError() })
+    const res = await request(app).post('/api/instances').send(base)
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'internal server error' })
+  })
+
+  it('POST logs the error with route context when DB throws unexpectedly', async () => {
+    vi.spyOn(dbModule, 'createInstance').mockImplementationOnce(() => { throw dbError() })
+    await request(app).post('/api/instances').send(base)
+    expect(consoleSpy).toHaveBeenCalledWith(
+      expect.stringContaining('POST /api/instances'),
+      expect.any(Error)
+    )
+  })
+
+  it('PUT returns 500 with friendly message when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send(base)
+    vi.spyOn(dbModule, 'updateInstance').mockImplementationOnce(() => { throw dbError() })
+    const res = await request(app).put('/api/instances/100').send(base)
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'internal server error' })
+  })
+
+  it('PUT logs the error with route context when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send(base)
+    vi.spyOn(dbModule, 'updateInstance').mockImplementationOnce(() => { throw dbError() })
+    await request(app).put('/api/instances/100').send(base)
+    expect(consoleSpy).toHaveBeenCalledWith(
+      expect.stringContaining('PUT /api/instances/:vmid'),
+      expect.any(Error)
+    )
+  })
+
+  it('DELETE returns 500 with friendly message when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send({ ...base, stack: 'development', state: 'testing' })
+    vi.spyOn(dbModule, 'deleteInstance').mockImplementationOnce(() => { throw dbError() })
+    const res = await request(app).delete('/api/instances/100')
+    expect(res.status).toBe(500)
+    expect(res.body).toEqual({ error: 'internal server error' })
+  })
+
+  it('DELETE logs the error with route context when DB throws unexpectedly', async () => {
+    await request(app).post('/api/instances').send({ ...base, stack: 'development', state: 'testing' })
+    vi.spyOn(dbModule, 'deleteInstance').mockImplementationOnce(() => { throw dbError() })
+    await request(app).delete('/api/instances/100')
+    expect(consoleSpy).toHaveBeenCalledWith(
+      expect.stringContaining('DELETE /api/instances/:vmid'),
+      expect.any(Error)
+    )
+  })
+})

tests/db.test.js

@@ -165,3 +165,23 @@ describe('deleteInstance', () => {
     expect(getInstance(2)).not.toBeNull();
   });
 });
+
+// ── Test environment boot isolation ───────────────────────────────────────────
+
+describe('test environment boot isolation', () => {
+  it('vitest runs with NODE_ENV=test', () => {
+    // Vitest sets NODE_ENV=test automatically. This is the guard condition
+    // that prevents the boot init() from opening the real database file.
+    expect(process.env.NODE_ENV).toBe('test');
+  });
+
+  it('db module loads cleanly in parallel workers without locking the real db file', () => {
+    // Regression: the module-level init(DEFAULT_PATH) used to run unconditionally,
+    // causing "database is locked" when multiple test workers imported db.js at
+    // the same time. process.exit(1) then killed the worker mid-suite.
+    // Fix: boot init is skipped when NODE_ENV=test. _resetForTest() handles setup.
+    // Reaching this line proves the module loaded without calling process.exit.
+    expect(() => _resetForTest()).not.toThrow();
+    expect(getInstances()).toEqual([]);
+  });
+});

tests/helpers.test.js

@@ -1,5 +1,7 @@
 // @vitest-environment jsdom
 import { describe, it, expect } from 'vitest'
+import { readFileSync } from 'fs'
+import { join } from 'path'
 
 // ── esc() ─────────────────────────────────────────────────────────────────────
 // Mirrors the implementation in ui.js exactly (DOM-based).
@@ -112,3 +114,53 @@ describe('fmtDateFull', () => {
     expect(fmtDateFull('')).toBe('—')
   })
 })
+
+// ── versionLabel() ───────────────────────────────────────────────────────────
+// Mirrors the logic in app.js — semver strings get a v prefix, dev strings don't.
+
+function versionLabel(v) {
+  return /^\d/.test(v) ? `v${v}` : v
+}
+
+describe('version label formatting', () => {
+  it('prepends v for semver strings', () => {
+    expect(versionLabel('1.1.2')).toBe('v1.1.2')
+    expect(versionLabel('2.0.0')).toBe('v2.0.0')
+  })
+
+  it('does not prepend v for dev build strings', () => {
+    expect(versionLabel('dev-abc1234')).toBe('dev-abc1234')
+  })
+})
+
+// ── CSS regressions ───────────────────────────────────────────────────────────
+
+const css = readFileSync(join(__dirname, '../css/app.css'), 'utf8')
+
+describe('CSS regressions', () => {
+  it('.badge has text-align: center so state labels are not left-skewed on cards', () => {
+    // Regression: badges rendered left-aligned inside the card's flex-end column.
+    // Without text-align: center, short labels (e.g. "deployed") appear
+    // left-justified inside their pill rather than centred.
+    expect(css).toMatch(/\.badge\s*\{[^}]*text-align\s*:\s*center/s)
+  })
+})
+
+// ── CI workflow regressions ───────────────────────────────────────────────────
+
+const ciYml = readFileSync(join(__dirname, '../.gitea/workflows/ci.yml'), 'utf8')
+
+describe('CI workflow regressions', () => {
+  it('build-dev job passes BUILD_VERSION build arg', () => {
+    // Regression: dev image showed semver instead of dev-<sha> because
+    // BUILD_VERSION was never passed to docker build.
+    expect(ciYml).toContain('BUILD_VERSION')
+  })
+
+  it('short SHA is computed with git rev-parse, not $GITEA_SHA (which is empty)', () => {
+    // Regression: ${GITEA_SHA::7} expands to "" on Gitea runners — nav showed "dev-".
+    // git rev-parse --short HEAD works regardless of which env vars the runner sets.
+    expect(ciYml).toContain('git rev-parse --short HEAD')
+    expect(ciYml).not.toContain('GITEA_SHA')
+  })
+})