test: address Copilot review comments on app worker unit tests

- worker.spec.ts: snapshot/restore process.env instead of deleting keys
- worker.spec.ts: replace meaningless expect(true) with not.to.throw()
- worker.spec.ts: assert fakeWatcher.close() in 'closes watchers' test
- static-mirroring-worker.spec.ts: snapshot/restore process.env.SECRET
- static-mirroring-worker.spec.ts: implement 'rejects events from relay itself' test
- static-mirroring-worker.spec.ts: assert true (not just boolean) for valid event
- static-mirroring-worker.spec.ts: add real assertions to onMessage tests with mock client
- static-mirroring-worker.spec.ts: assert WebSocket.terminate() in close test
- app.spec.ts: use deterministic counter instead of Math.random() for worker IDs
- app.spec.ts: advance fake timers and assert cluster.fork in restart test
- src/app/static-mirroring-worker.ts: validate this.config after path() lookup
- src/app/app.ts: use this.process.env consistently for all env reads in run()

Author: phoenix-server

src/app/app.ts

@@ -47,8 +47,8 @@ export class App implements IRunnable {
    ░   ░ ░ ░ ░ ░ ▒  ░  ░  ░    ░        ░░   ░    ░    ░   ▒   ░      ░
          ░     ░ ░        ░              ░        ░  ░     ░  ░       ░`)
     const width = 74
-    const torHiddenServicePort = process.env.HIDDEN_SERVICE_PORT ? Number(process.env.HIDDEN_SERVICE_PORT) : 80
-    const port = process.env.RELAY_PORT ? Number(process.env.RELAY_PORT) : 8008
+    const torHiddenServicePort = this.process.env.HIDDEN_SERVICE_PORT ? Number(this.process.env.HIDDEN_SERVICE_PORT) : 80
+    const port = this.process.env.RELAY_PORT ? Number(this.process.env.RELAY_PORT) : 8008
 
     const logCentered = (input: string, width: number) => {
       const start = (width - input.length) >> 1

src/app/static-mirroring-worker.ts

@@ -56,6 +56,9 @@ export class StaticMirroringWorker implements IRunnable {
     logger.info('mirroring', currentSettings.mirroring)
 
     this.config = path(['mirroring', 'static', this.process.env.MIRROR_INDEX], currentSettings) as Mirror
+    if (!this.config) {
+      throw new Error(`Mirror configuration not found for index ${this.process.env.MIRROR_INDEX}`)
+    }
 
     let since = Math.floor(Date.now() / 1000) - 60 * 10
 

test/unit/app/app.spec.ts

@@ -37,14 +37,20 @@ describe('App', () => {
     } as any,
   })
 
-  const createFakeWorker = (): any => ({
-    id: Math.floor(Math.random() * 10000),
-    process: { pid: Math.floor(Math.random() * 100000) },
-    send: sandbox.stub(),
-  })
+  let workerIdCounter = 0
+
+  const createFakeWorker = (): any => {
+    const id = ++workerIdCounter
+    return {
+      id,
+      process: { pid: id + 100000 },
+      send: sandbox.stub(),
+    }
+  }
 
   beforeEach(() => {
     sandbox = Sinon.createSandbox()
+    workerIdCounter = 0
 
     fakeProcess = Object.assign(new EventEmitter(), {
       exit: sandbox.stub(),
@@ -290,9 +296,10 @@ describe('App', () => {
   describe('onClusterExit', () => {
     let worker: any
     let deadWorker: any
+    let clock: Sinon.SinonFakeTimers
 
     beforeEach(() => {
-      sandbox.useFakeTimers()
+      clock = sandbox.useFakeTimers()
 
       worker = createFakeWorker()
       deadWorker = createFakeWorker()
@@ -319,11 +326,18 @@ describe('App', () => {
     })
 
     it('schedules worker restart on unexpected exit', () => {
-      // When a worker exits unexpectedly, the app schedules a restart
-      // We verify that exit handling doesn't throw
-      expect(() => {
-        fakeCluster.emit('exit', deadWorker, 1, '')
-      }).not.to.throw()
+      settingsState.workers = { count: 1 }
+      settingsState.mirroring = { static: [] }
+
+      app.run()
+
+      const registeredWorker = (fakeCluster.fork as Sinon.SinonStub).firstCall.returnValue
+      fakeCluster.fork.resetHistory()
+
+      fakeCluster.emit('exit', registeredWorker, 1, '')
+      clock.tick(10001)
+
+      expect(fakeCluster.fork).to.have.been.called
     })
   })
 

test/unit/app/static-mirroring-worker.spec.ts

@@ -8,6 +8,8 @@ import { StaticMirroringWorker } from '../../../src/app/static-mirroring-worker'
 import { Event } from '../../../src/@types/event'
 import { Settings } from '../../../src/@types/settings'
 import { IEventRepository, IUserRepository } from '../../../src/@types/repositories'
+import { getPublicKey, getRelayPrivateKey } from '../../../src/utils/event'
+import { WebSocketServerAdapterEvent } from '../../../src/constants/adapter'
 
 chai.use(sinonChai)
 
@@ -54,8 +56,11 @@ describe('StaticMirroringWorker', () => {
     ...overrides,
   })
 
+  let savedSecret: string | undefined
+
   beforeEach(() => {
     sandbox = Sinon.createSandbox()
+    savedSecret = process.env.SECRET
     process.env.SECRET = 'test-secret-for-unit-tests'
 
     fakeProcess = Object.assign(new EventEmitter(), {
@@ -84,7 +89,8 @@ describe('StaticMirroringWorker', () => {
   })
 
   afterEach(() => {
-    delete process.env.SECRET
+    if (savedSecret === undefined) delete process.env.SECRET
+    else process.env.SECRET = savedSecret
     sandbox.restore()
   })
 
@@ -124,15 +130,20 @@ describe('StaticMirroringWorker', () => {
 
   describe('canAcceptEvent', () => {
     it('rejects events from the relay itself', () => {
-      // This tests the private canAcceptEvent method indirectly through the worker behavior
-      // For now, we focus on testing the public interface
+      const relayPrivkey = getRelayPrivateKey(settingsState.info!.relay_url)
+      const relayPubkey = getPublicKey(relayPrivkey)
+
+      const event = createEvent({ pubkey: relayPubkey })
+      const result = (worker as any).canAcceptEvent(event)
+
+      expect(result).to.equal(false)
     })
 
     it('accepts valid events within limits', () => {
       const event = createEvent({ pubkey: 'd'.repeat(64) })
       const result = (worker as any).canAcceptEvent(event)
 
-      expect(result).to.be.a('boolean')
+      expect(result).to.equal(true)
     })
 
     it('rejects events with content exceeding limits', () => {
@@ -357,29 +368,42 @@ describe('StaticMirroringWorker', () => {
   })
 
   describe('onMessage', () => {
+    let mockClient: { send: Sinon.SinonStub; readyState: number }
+
+    beforeEach(() => {
+      mockClient = { send: sandbox.stub(), readyState: 1 /* WebSocket.OPEN */ }
+      ;(worker as any).config = settingsState.mirroring!.static![0]
+      ;(worker as any).client = mockClient
+    })
+
     it('relays broadcast messages to connected mirror', () => {
-      const testMessage = {
-        eventName: 'Broadcast',
+      fakeProcess.emit('message', {
+        eventName: WebSocketServerAdapterEvent.Broadcast,
         event: createEvent(),
         source: 'local',
-      }
-
-      // Simulate message reception
-      fakeProcess.emit('message', testMessage)
+      })
 
-      // The message handler should attempt to forward if client is open
+      expect(mockClient.send).to.have.been.called
     })
 
-    it('ignores messages from the same source', () => {
-      const testMessage = {
-        eventName: 'Broadcast',
+    it('ignores messages from the same source as the mirror', () => {
+      fakeProcess.emit('message', {
+        eventName: WebSocketServerAdapterEvent.Broadcast,
         event: createEvent(),
         source: 'ws://source-relay.com',
-      }
+      })
+
+      expect(mockClient.send).not.to.have.been.called
+    })
 
-      fakeProcess.emit('message', testMessage)
+    it('ignores non-broadcast messages', () => {
+      fakeProcess.emit('message', {
+        eventName: 'other-event',
+        event: createEvent(),
+        source: 'local',
+      })
 
-      // Should not forward to same source
+      expect(mockClient.send).not.to.have.been.called
     })
   })
 
@@ -415,9 +439,12 @@ describe('StaticMirroringWorker', () => {
 
   describe('close', () => {
     it('terminates the WebSocket client', () => {
+      const mockClient = { terminate: sandbox.stub() }
+      ;(worker as any).client = mockClient
+
       worker.close()
 
-      // Verify close completes without error
+      expect(mockClient.terminate).to.have.been.called
     })
 
     it('invokes the callback when provided', () => {

test/unit/app/worker.spec.ts

@@ -18,9 +18,13 @@ describe('AppWorker', () => {
   let fakeAdapter: any
   let watchSettingsStub: Sinon.SinonStub
 
+  let savedEnv: { PORT: string | undefined; RELAY_PORT: string | undefined }
+
   beforeEach(() => {
     sandbox = Sinon.createSandbox()
 
+    savedEnv = { PORT: process.env.PORT, RELAY_PORT: process.env.RELAY_PORT }
+
     fakeProcess = Object.assign(new EventEmitter(), {
       exit: sandbox.stub(),
       env: process.env,
@@ -46,9 +50,10 @@ describe('AppWorker', () => {
   })
 
   afterEach(() => {
-    // Clean up env vars
-    delete process.env.PORT
-    delete process.env.RELAY_PORT
+    if (savedEnv.PORT === undefined) delete process.env.PORT
+    else process.env.PORT = savedEnv.PORT
+    if (savedEnv.RELAY_PORT === undefined) delete process.env.RELAY_PORT
+    else process.env.RELAY_PORT = savedEnv.RELAY_PORT
     sandbox.restore()
   })
 
@@ -147,21 +152,13 @@ describe('AppWorker', () => {
     it('handles TypeError about database connection without throwing', () => {
       const error = new TypeError("Cannot read properties of undefined (reading '__knexUid')")
 
-      // This should not throw because onError logs and returns for this specific error
-      fakeProcess.emit('uncaughtException', error)
-
-      // Verify error was handled gracefully
-      expect(true).to.be.true
+      expect(() => fakeProcess.emit('uncaughtException', error)).not.to.throw()
     })
 
-    it('logs other errors', () => {
+    it('logs other errors without throwing', () => {
       const error = new Error('test error')
 
-      // onError logs the error
-      fakeProcess.emit('uncaughtException', error)
-
-      // Verify the handler was called
-      expect(true).to.be.true
+      expect(() => fakeProcess.emit('uncaughtException', error)).not.to.throw()
     })
   })
 
@@ -233,6 +230,8 @@ describe('AppWorker', () => {
       worker.close()
 
       expect(fakeAdapter.close).to.have.been.called
+      expect(fakeWatcher1.close).to.have.been.called
+      expect(fakeWatcher2.close).to.have.been.called
     })
 
     it('handles no watchers gracefully', () => {