fix(server): update client lastActivityAt and lastPingAt on keepalive ping packets (#17)

[ghzhost]

Problema resuelto (#17)

Cuando un cliente/jugador pasa la aplicación a segundo plano o bloquea la pantalla en dispositivos móviles, el cliente sigue enviando paquetes de periódicamente cada 10 segundos para mantener vivo el socket WebSocket. Sin embargo, en (), la lógica retornaba antes de actualizar al recibir paquetes de ping.

Como resultado, al cabo de 15 minutos (), el sweep de inactividad consideraba al jugador inactivo y lo expulsaba () aunque la conexión seguía viva.

Cambios realizados

1. En , actualizamos , y con el timestamp actual al procesar .
2. En , añadimos el campo opcional a la estructura .
3. Verificamos que el build del servidor () compila limpiamente sin errores.

Closes #17.

[gitar-bot]

⚠️ Bug: Setting lastPacketAt early zeroes out packet-interval metrics

The new ws.lastPacketAt = now; at server.ts:370 runs before line 378 reads previousPacketAt = Number(ws.lastPacketAt ?? 0). Since it was just overwritten with now, previousPacketAt === now, so intervalMs = Math.max(0, now - previousPacketAt) is always 0. This silently breaks lastPacketIntervalMs, minPacketIntervalMs, and recentPacketIntervalsMs for every non-ping packet — metrics consumed by the packet-usage ranking/anti-bot diagnostics in commands.ts (sendPacketUsageRanking). Fix by not overwriting lastPacketAt before the interval is computed: set it only inside the ping branch (the non-ping path already sets it at line 423).

Move lastPacketAt assignment into the ping branch so the non-ping interval calc still reads the previous packet's timestamp.:

ws.packetCount = Number(ws.packetCount ?? 0) + 1;

if (isPingPacket) {
    ws.lastPacketAt = now;
    ws.lastPingAt = now;
    ws.lastActivityAt = now;
    return;
}

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

CI failed: Integration tests failed due to missing seed files (like npcs.json) causing runtime file-not-found errors and cascading 500 status code test failures.

Overview

1 test failure pattern found across 1 job, causing the test suite to fail due to missing seed data files at runtime.

Failures

Missing Seed Files / Database Seeding Failure (confidence: high)

• Type: test
• Affected jobs: 96640274444
• Related to change: unclear
• Root cause: The test suite encountered an ENOENT: no such file or directory error when trying to open src/jsons/npcs.json, causing NPC seeding to fail and dependent integration/public endpoint tests to return 500 Internal Server Errors.
• Suggested fix: Ensure that required seed data files (such as src/jsons/npcs.json) are present in the repository and correctly placed or copied into the expected runtime directory.

Summary

• Change-related failures: 0 (Unclear if related to PR changes, but involves missing runtime assets)
• Infrastructure/flaky failures: 0
• Recommended action: Verify that all required JSON asset and seed files are included in version control or correctly generated during the test setup/build phase.

Code Review ⚠️ Changes requested 0 resolved / 1 findings

Updates client lastActivityAt and lastPingAt on keepalive ping packets to prevent premature idle disconnects, but setting lastPacketAt early zeroes out packet-interval metrics.

⚠️ Bug: Setting lastPacketAt early zeroes out packet-interval metrics

📄 server/src/server.ts:370 📄 server/src/server.ts:378 📄 server/src/server.ts:406-420 📄 server/src/server.ts:423

The new ws.lastPacketAt = now; at server.ts:370 runs before line 378 reads previousPacketAt = Number(ws.lastPacketAt ?? 0). Since it was just overwritten with now, previousPacketAt === now, so intervalMs = Math.max(0, now - previousPacketAt) is always 0. This silently breaks lastPacketIntervalMs, minPacketIntervalMs, and recentPacketIntervalsMs for every non-ping packet — metrics consumed by the packet-usage ranking/anti-bot diagnostics in commands.ts (sendPacketUsageRanking). Fix by not overwriting lastPacketAt before the interval is computed: set it only inside the ping branch (the non-ping path already sets it at line 423).

Move lastPacketAt assignment into the ping branch so the non-ping interval calc still reads the previous packet's timestamp.

ws.packetCount = Number(ws.packetCount ?? 0) + 1;

if (isPingPacket) {
    ws.lastPacketAt = now;
    ws.lastPingAt = now;
    ws.lastActivityAt = now;
    return;
}

🤖 Prompt for agents

Code Review: Updates client lastActivityAt and lastPingAt on keepalive ping packets to prevent premature idle disconnects, but setting lastPacketAt early zeroes out packet-interval metrics.

1. ⚠️ Bug: Setting lastPacketAt early zeroes out packet-interval metrics
   Files: server/src/server.ts:370, server/src/server.ts:378, server/src/server.ts:406-420, server/src/server.ts:423

   The new `ws.lastPacketAt = now;` at server.ts:370 runs before line 378 reads `previousPacketAt = Number(ws.lastPacketAt ?? 0)`. Since it was just overwritten with `now`, `previousPacketAt === now`, so `intervalMs = Math.max(0, now - previousPacketAt)` is always 0. This silently breaks `lastPacketIntervalMs`, `minPacketIntervalMs`, and `recentPacketIntervalsMs` for every non-ping packet — metrics consumed by the packet-usage ranking/anti-bot diagnostics in commands.ts (sendPacketUsageRanking). Fix by not overwriting lastPacketAt before the interval is computed: set it only inside the ping branch (the non-ping path already sets it at line 423).

   Fix (Move lastPacketAt assignment into the ping branch so the non-ping interval calc still reads the previous packet's timestamp.):
   ws.packetCount = Number(ws.packetCount ?? 0) + 1;
   
   if (isPingPacket) {
       ws.lastPacketAt = now;
       ws.lastPingAt = now;
       ws.lastActivityAt = now;
       return;
   }

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[ghzhost]

Estado del CI

El check API (typecheck, test, build) falla con dos errores que ya existían en main antes de este PR:

1. ENOENT: npcs.json not found — archivo de seed ausente (relacionado con Falta api/src/jsons/npcs.json en el repositorio y el test de market no puede correr #83)
2. wiki endpoint retorna 500 — bug en el endpoint de wiki

Ambos fallos se reproducen en el historial de CI de main (run 2026-08-18T21:14:03Z). Este PR no introduce ninguna regresión en los tests; los cambios se limitan al manejo de keepalive ping en server/src/server.ts.

Puede verificarse corriendo el suite de tests en main directamente para confirmar.

[leocagli]

Cierro esta porque la #17 ya quedo resuelta en main. El diagnostico era correcto: los pings salian de trackClientActivity sin tocar nada, asi que un cliente que solo manda keepalive contaba como inactivo.

Que paso

La #17 junto cuatro PRs: #34, #50, #121 y #123. Se mergeo #121, de @franklincg, que tenia la issue asignada, y ademas resuelve el problema de una forma que las otras no.

La solucion directa es hacer que el ping toque lastActivityAt. Funciona para no expulsar al jugador, pero rompe cualquier medicion de AFK: un cliente con la pantalla bloqueada manda pings solo, asi que pasa a figurar activo para siempre y el timeout de inactividad deja de existir para todos.

#121 separa las dos cosas:

if (isPingPacket) {
    ws.lastPingAt = now;   // la conexion esta viva
    return;                // pero el jugador no hizo nada
}

y despues, al decidir si expulsar, toma el maximo entre lastActivityAt, lastPingAt y connectedAt. Asi el keepalive mantiene la sesion sin contaminar la actividad real. Trae ademas server/src/scripts/testClientActivityPolicy.ts, que deja la politica escrita y verificable.

Contexto que no se veia desde afuera

Los PRs que vienen de un fork quedan en action_required y el workflow no corre hasta que un maintainer lo aprueba. Nadie lo estaba aprobando, asi que el CI de esta PR nunca dijo nada, ni bueno ni malo.

Y main estaba en rojo desde el dia que se agrego el workflow, por dos archivos de seed que faltaban en el repositorio. Eso tambien se arreglo hoy (#109), asi que a partir de ahora el CI sirve como senal de verdad.

Gracias por el laburo. Hay issues abiertas sin asignar: comenta con un plan concreto (que archivo, que funcion, como lo vas a verificar) y te la asigno.