Problem
In server/routes/update.js:175, POST /api/update/execute acquires the atomic update lock via await updateChecker.setUpdateInProgress(true). At line 227–242, it launches the update asynchronously:
executeUpdate(tag, emit, { forceCleanWorkspaces }).then(result => {
if (io) {
if (result.success) {
io.emit('portos:update:complete', { success: true, newVersion: result.version || tag.replace(/^v/, ''), versionKnown: !!result.version });
} else {
io.emit('portos:update:error', { message: result.errorMessage ?? 'Update failed', step: result.failedStep ?? 'unknown' });
}
}
}).catch(err => {
if (io) {
io.emit('portos:update:error', { message: err.message, step: 'unknown' });
}
});
If executeUpdate rejects (for example, when server/services/updateExecutor.js:98-102 rejects await spawnDetached(...) due to filesystem permissions or missing binaries before child event listeners can be attached), the .catch handler emits portos:update:error but never calls updateChecker.setUpdateInProgress(false).
Contrast this with server/services/appUpdater.js:240-243, which explicitly handles this failure mode:
const outcome = await executeUpdate(version, emit).catch(async (err) => {
await setUpdateInProgress(false);
throw err;
});
Additionally, server/routes/update.test.js completely lacks test coverage for:
setUpdateInProgress(true) resolving to false (the 409 UPDATE_IN_PROGRESS guard at line 177).
- Invalid release tag formats (the 400
INVALID_TAG guard at line 171).
executeUpdate rejection leading to lock release and socket error emission.
- Socket event emissions (
portos:update:step, portos:update:complete, portos:update:error) because makeApp() never attaches an io instance to the Express app.
Trigger
A user or automation requests a PortOS update via POST /api/update/execute. spawnDetached throws an error during launch (or executeUpdate fails before child process setup).
Impact
The atomic update lock updateInProgress remains stuck at true in data/update.json, and isUpdateInProgress() stays true in memory:
- Every subsequent call to
POST /api/update/execute returns HTTP 409 UPDATE_IN_PROGRESS.
- All CoS agent spawns across the entire PortOS installation are blocked by
isUpdateInProgress() until the 30-minute stale timeout elapses or the server process is restarted.
- The failure to release the lock is completely undetected by CI because
server/routes/update.test.js only checks that executeUpdate was called and does not verify async settlement.
Fix
- In
server/routes/update.js:238-243, update the .catch handler to await updateChecker.setUpdateInProgress(false) when handling the error.
- In
server/routes/update.test.js:
- Provide an
io mock in makeApp() (app.set('io', mockIo)) to record socket emissions.
- Add a test asserting that when
updateChecker.setUpdateInProgress.mockResolvedValue(false), POST /api/update/execute returns 409 UPDATE_IN_PROGRESS.
- Add a test asserting that invalid tag strings (e.g.
"not-semver", "; rm -rf /") return 400 INVALID_TAG without acquiring the lock.
- Add a test asserting that when
executeUpdate rejects, updateChecker.setUpdateInProgress(false) is invoked and io.emit('portos:update:error', ...) is sent.
- Add tests asserting that successful
executeUpdate emits portos:update:complete and result.success === false emits portos:update:error.
Rejected alternative: Releasing the lock inside updateExecutor.js instead of the route was rejected because executeUpdate is a low-level launcher that does not own the pre-lock acquisition performed in update.js.
Acceptance criteria
Problem
In
server/routes/update.js:175,POST /api/update/executeacquires the atomic update lock viaawait updateChecker.setUpdateInProgress(true). At line 227–242, it launches the update asynchronously:If
executeUpdaterejects (for example, whenserver/services/updateExecutor.js:98-102rejectsawait spawnDetached(...)due to filesystem permissions or missing binaries before child event listeners can be attached), the.catchhandler emitsportos:update:errorbut never callsupdateChecker.setUpdateInProgress(false).Contrast this with
server/services/appUpdater.js:240-243, which explicitly handles this failure mode:Additionally,
server/routes/update.test.jscompletely lacks test coverage for:setUpdateInProgress(true)resolving tofalse(the 409UPDATE_IN_PROGRESSguard at line 177).INVALID_TAGguard at line 171).executeUpdaterejection leading to lock release and socket error emission.portos:update:step,portos:update:complete,portos:update:error) becausemakeApp()never attaches anioinstance to the Express app.Trigger
A user or automation requests a PortOS update via
POST /api/update/execute.spawnDetachedthrows an error during launch (orexecuteUpdatefails before child process setup).Impact
The atomic update lock
updateInProgressremains stuck attrueindata/update.json, andisUpdateInProgress()staystruein memory:POST /api/update/executereturns HTTP 409UPDATE_IN_PROGRESS.isUpdateInProgress()until the 30-minute stale timeout elapses or the server process is restarted.server/routes/update.test.jsonly checks thatexecuteUpdatewas called and does not verify async settlement.Fix
server/routes/update.js:238-243, update the.catchhandler to awaitupdateChecker.setUpdateInProgress(false)when handling the error.server/routes/update.test.js:iomock inmakeApp()(app.set('io', mockIo)) to record socket emissions.updateChecker.setUpdateInProgress.mockResolvedValue(false),POST /api/update/executereturns 409UPDATE_IN_PROGRESS."not-semver","; rm -rf /") return 400INVALID_TAGwithout acquiring the lock.executeUpdaterejects,updateChecker.setUpdateInProgress(false)is invoked andio.emit('portos:update:error', ...)is sent.executeUpdateemitsportos:update:completeandresult.success === falseemitsportos:update:error.Rejected alternative: Releasing the lock inside
updateExecutor.jsinstead of the route was rejected becauseexecuteUpdateis a low-level launcher that does not own the pre-lock acquisition performed inupdate.js.Acceptance criteria
executeUpdaterejects inserver/routes/update.js,updateChecker.setUpdateInProgress(false)is called andportos:update:erroris emitted.server/routes/update.test.jstests thatPOST /api/update/executereturns 409UPDATE_IN_PROGRESSwhen the update lock is already held.server/routes/update.test.jstests thatPOST /api/update/executereturns 400INVALID_TAGfor non-semver tag formats.server/routes/update.test.jsverifiesportos:update:step,portos:update:complete, andportos:update:errorsocket emissions via a mockioinstance.server/routes/update.test.jspass withcd server && npm test routes/update.test.js.