test: pin the App Management socket contract (server/sockets/apps.js) - #6137
Merged
Conversation
#6037) server/sockets/apps.js owns three pieces of state nothing else can observe — the re-entrancy guard that stops two concurrent git pull / PM2 standardize runs against the same checkout, the resumable progress buffer a remounting client rehydrates from, and the ledger/broadcast/cleanup epilogue that must run even when the underlying service throws. The only suite importing registerAppHandlers covered just the PortOS-app preflight refusals, so all three were unpinned. Adds server/sockets/apps.test.js, driving the real handlers over a fake socket/io bus at the event boundary: - a duplicate update, an update on a different app record sharing the same repoPath, and a standardize racing an update are each refused with duplicate: true, while an unrelated checkout still proceeds - a reconnecting client's connect-time push and its app:operations:list reply both carry the in-flight operation with repoPath stripped and step frames deduplicated last-write-wins per step id - a rejecting appUpdater.updateApp still writes a success:false ledger row with the error message, broadcasts notifyAppsChanged, emits no completion frame, and clears the operation - a standardize failing at analyze never reaches apply, and one failing at apply leaves the app record untouched — pm2ProcessNames must not be persisted from a run that never wrote the config those names describe In-flight cases park inside appUpdater.updateApp on a deferred promise rather than sleeping, so "operation is active" is deterministic with no timers, and the parked run is released from onTestFinished because activeAppOperations is module state that outlives a single test. Claude-Session: https://claude.ai/code/session_01RA3pD5YM2dukQwbZ3pC6WA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
server/sockets/apps.jsowns three pieces of state nothing else can observe — the re-entrancy guard that stops two concurrentgit pull/ PM2 standardize runs against the same checkout, the resumable progress buffer a remounting client rehydrates from, and the ledger/broadcast/cleanup epilogue that must run even when the underlying service throws. Until now the only suite importingregisterAppHandlerswasserver/services/updatePreflightParity.test.js, which covers just the PortOS-app preflight refusals, so all three were unpinned.This adds
server/sockets/apps.test.js— a test-only change, no production code touched.Seven tests, driving the real handlers over a fake socket/io bus at the event boundary:
Collision guard
app:updatefor the same app while one is in flight is refused withduplicate: true, andappUpdater.updateAppis not called twice.app:updatefor a different app record that points at the samerepoPathis refused too (the half offindConflictingOperationthat an app-id lookup alone would miss), while an unrelated checkout is unaffected.app:standardizeis refused while an update runs on the same checkout, and analysis never starts.Active-operations buffer
app:operations:listreply both carry the in-flight operation withrepoPathstripped, and with step frames deduplicated last-write-wins per step id rather than one frame appended per emit.Failure recovery
appUpdater.updateAppstill emitsapp:update:error, writes asuccess: falseledger row carrying the error message, broadcastsnotifyAppsChanged('update', appId), emits noapp:update:complete, and clears the operation.analyze/errorstep and the error event, never reaches apply, and clears the operation.pm2ProcessNamesmust not be persisted from a standardization that never wrote the config those names describe.Notes
appUpdater.updateAppon a deferred promise rather than sleeping, so "operation is active" is deterministic with no timers.activeAppOperationsis module state that outlives an individual test, sostartParkedUpdatereleases its parked run fromonTestFinished. Without that, one failing assertion would leave the guard armed and cascade into every later test in the file — verified by mutating the source and watching failures stay attributed to the one behavior broken.Test plan
cd server && npx vitest run sockets/apps.test.js— 7 passed.cd server && npx vitest run sockets/ services/updatePreflightParity.test.js— 5 files, 77 passed.routes/settings.secretsStrip.test.js, 10s timeout) is an unrelated contention flake from parallel suite runs on the same machine — it passes in isolation and touches no socket code.repoPathhalf offindConflictingOperation, un-strippingrepoPathfrom the payload, removing the step dedupe, dropping the failure ledger row, removing thefinallycleanup, and removing either standardize early-return each fail exactly the test(s) claiming that behavior, with the source restored afterward.Closes #6037
https://claude.ai/code/session_01RA3pD5YM2dukQwbZ3pC6WA