Skip to content

Add socket unit test suite for App Management handlers (server/sockets/apps.js) covering in-flight collision, step buffer replay, and update failure recovery #6037

Description

@atomantic

Problem

server/sockets/apps.js manages long-running, multi-step operations (app:update, app:standardize, app:deploy, detect:start, standardize:start) and tracks active operations via an in-memory Map (activeAppOperations:21).

Despite being the critical socket backbone for App Management, no dedicated test suite exists for server/sockets/apps.js. The only test that imports registerAppHandlers is server/services/updatePreflightParity.test.js, which only tests PortOS-specific preflight refusals.

Consequently, several core mechanisms have zero test coverage:

  1. Conflict detection (findConflictingOperation:30-31): A duplicate update or standardize on the same appId, or on a different app record sharing the same repoPath, must be refused with { appId, duplicate: true, message } on app:update:error or app:standardize:error.
  2. Active operations buffer and client reconnect (activeOperationsPayload:24-26, recordOperationStep:47-51, socket.on('app:operations:list')): Operations must sanitize internal fields (repoPath stripped), deduplicate step frames by step ID, and replay active operations on connect or demand.
  3. Failure handling during app:update: When appUpdater.updateApp rejects, the socket handler must catch the error, emit app:update:error, log to the audit ledger via logAction('update', ..., success: false, failure.message), broadcast notifyAppsChanged('update', appId), and clean up via endAppOperation.
  4. Failure handling during app:standardize: When pm2Standardizer.analyzeApp or applyStandardization fails, the error must be emitted, steps recorded, and endAppOperation executed in finally.

Trigger

An operator triggers multiple simultaneous update/standardize actions from the App Management UI, navigates away and re-mounts the page during an active update, or encounters a failure in appUpdater.updateApp.

Impact

A regression in findConflictingOperation would allow two concurrent git pull or standardize runs to mutate the same git repository concurrently, corrupting working trees or PM2 configurations. A regression in activeAppOperations causes in-flight progress cards to vanish or desync upon page reload. Because there is no unit test suite for server/sockets/apps.js, regressions in socket data validation, audit logging, or error emission go completely unnoticed in CI.

Fix

Create server/sockets/apps.test.js using a socket/io test harness (adapting the pattern from server/services/updatePreflightParity.test.js:69-82) to assert the complete contract of server/sockets/apps.js:

  1. Collision guard tests:
    • Dispatch app:update when an operation is already active for the same app ID -> emits app:update:error with duplicate: true.
    • Dispatch app:update for App B pointing to the same repoPath as active App A -> emits duplicate: true error.
    • Dispatch app:standardize while app:update is running on the same app -> emits duplicate: true.
  2. In-flight operations buffer tests:
    • Dispatching an update emits app:operations:active containing the active operation with repoPath excluded.
    • Incoming step events update the buffer via recordOperationStep.
    • Emitting app:operations:list returns the current active operations list.
  3. Failure and cleanup tests:
    • When appUpdater.updateApp rejects, verify logAction is called with success: false and the error message, notifyAppsChanged('update', appId) is called, and endAppOperation emits app:operations:active without the app.
    • When appStandardizeSchema validation fails or app is missing, appropriate error events are emitted.

Rejected alternative: Testing socket handlers exclusively through client-side React integration tests (client/src/pages/Apps.test.jsx) was rejected because client tests mock socket events rather than testing the server's Socket.IO event handler implementations.

Acceptance criteria

  • server/sockets/apps.test.js is created and imports registerAppHandlers.
  • Tests verify findConflictingOperation rejects concurrent operations on the same app ID or shared repoPath with duplicate: true.
  • Tests verify app:operations:list and initial socket connection emit app:operations:active with repoPath stripped.
  • Tests verify app:update failure logs to logAction with success: false, calls notifyAppsChanged, and cleans up active operations.
  • Tests verify app:standardize analyze and apply errors emit structured error events and clean up active operations.
  • cd server && npm test sockets/apps.test.js passes deterministically.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions