Skip to content

fix(server): count keepalive ping as client activity (#17) - #121

Merged
leocagli merged 3 commits into
Bitcoindefi:mainfrom
franklincg:fastmoney-openao17-final
Aug 27, 2026
Merged

fix(server): count keepalive ping as client activity (#17)#121
leocagli merged 3 commits into
Bitcoindefi:mainfrom
franklincg:fastmoney-openao17-final

Conversation

@franklincg

@franklincg franklincg commented Aug 24, 2026

Copy link
Copy Markdown

Fixes #17


Summary by Gitar

  • Server activity tracking:
    • Updated trackClientActivity in server.ts so keepalive ping packets refresh ws.lastActivityAt
  • Testing:
    • Added a test:client-activity script and assertion test suite in testClientActivityPolicy.ts
  • Workflow:
    • Added workflow for applying review fixes and validating client activity policy tests

This will update automatically on new commits.

Comment thread server/src/server.ts
@franklincg
franklincg force-pushed the fastmoney-openao17-final branch from 4c8740e to 1d62032 Compare August 24, 2026 10:52
@gitar-bot

gitar-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Keepalive ping packets now reset the client idle timer by updating lastActivityAt in trackClientActivity, preventing premature AFK disconnects. Added test coverage with testClientActivityPolicy.ts and a validation workflow. No issues found.

✅ 1 resolved
Bug: Keepalive ping resets idle timer, disabling AFK disconnect

📄 server/src/server.ts:371-375 📄 server/src/server.ts:699-713 📄 server/src/server.ts:733-735 📄 server/src/server.ts:749-755
The client sends an automatic keepalive ping every 10s (frontend/components/game/session/useGameSession.ts: setInterval(sendPing, 10000)), independent of user activity. By setting ws.lastActivityAt = now on every ping, processIdleCharactersTick's reference (client.lastActivityAt) is refreshed far more often than the 15-min idleCharacterTimeoutMs, so normal single-connection users are effectively never disconnected for inactivity — the idle-disconnect feature only remains meaningful for duplicate-IP scouts (which use getScoutIdleReferenceAt on real movement/combat). If disconnecting AFK-but-connected users is still intended, track socket liveness in a separate field (e.g. ws.lastPingAt) and keep lastActivityAt driven only by real user activity.

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         

Important

Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@franklincg

Copy link
Copy Markdown
Author

Thanks for the review.

I separated transport liveness from real player activity in commit c8b7b0f:

  • keepalive ping now updates lastPingAt
  • non-ping gameplay packets continue to update lastActivityAt
  • ping still does not touch lastPacketAt, PPS counters, packet intervals, or non-ping metrics

This matches issue #17 acceptance criteria, which explicitly require:

  • a client sending only pings must not be disconnected for inactivity
  • a truly silent client must still be cleaned up
  • returning from background after 15 minutes must preserve the session

So lastPingAt is intentionally part of connection liveness, while lastActivityAt remains reserved for real player activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bloquear la pantalla del celular expulsa al jugador: los pings no cuentan como actividad

2 participants