From fe230dda2c75944cb19f2e9b440816d384e33802 Mon Sep 17 00:00:00 2001 From: cstns Date: Wed, 14 Oct 2026 13:06:52 +0300 Subject: [PATCH] Track starting and suspending states for the team broker agent A suspend arriving while start was still launching the agent found nothing to stop, then deleted the record, leaving the agent running untracked. The agent now stays starting until the driver has launched it and suspend replies 409 meanwhile; suspend marks it suspending first so a start in that window gets a 409 too. A failed launch removes the record instead of leaving it stuck in starting. --- forge/ee/routes/teamBroker/3rdPartyBroker.js | 28 +++++- .../routes/teamBroker/3rdPartyBroker_spec.js | 89 ++++++++++++++++++- 2 files changed, 112 insertions(+), 5 deletions(-) diff --git a/forge/ee/routes/teamBroker/3rdPartyBroker.js b/forge/ee/routes/teamBroker/3rdPartyBroker.js index 481d9616c6..a1a65f7194 100644 --- a/forge/ee/routes/teamBroker/3rdPartyBroker.js +++ b/forge/ee/routes/teamBroker/3rdPartyBroker.js @@ -445,16 +445,30 @@ module.exports = async function (app) { }, async (request, reply) => { if (request.params.brokerId === 'team-broker') { const agent = await app.db.models.TeamBrokerAgent.byTeam(request.team.hashid) - if (agent && agent.state !== 'running') { + if (agent?.state === 'suspending') { + reply.status(409).send({ code: 'agent_suspending', error: 'The Team Broker agent is still suspending, try again shortly' }) + } else if (agent?.state === 'starting') { + // Another start is already launching the agent: starting is a no-op + reply.status(200).send({}) + } else if (agent && agent.state !== 'running') { await app.containers.sendBrokerAgentCommand(agent, 'start') reply.status(200).send({}) } else if (!agent) { + // Stays 'starting' until the driver has launched the agent, so a suspend + // arriving meanwhile is refused rather than missing a process it can't stop yet const agent = await app.db.models.TeamBrokerAgent.create({ - state: 'running', + state: 'starting', TeamId: request.team.id }) await agent.reload({ include: [{ model: app.db.models.Team }] }) - await app.containers.startBrokerAgent(agent) + try { + await app.containers.startBrokerAgent(agent) + } catch (err) { + await agent.destroy() + throw err + } + agent.state = 'running' + await agent.save() reply.status(200).send({}) } else { // Agent already running: starting is a no-op @@ -509,7 +523,13 @@ module.exports = async function (app) { }, async (request, reply) => { if (request.params.brokerId === 'team-broker') { const agent = await app.db.models.TeamBrokerAgent.byTeam(request.team.id) - if (agent) { + if (agent?.state === 'starting') { + reply.status(409).send({ code: 'agent_starting', error: 'The Team Broker agent is still starting, try again shortly' }) + } else if (agent) { + // Marked before stopping, so a start arriving before the row is removed is refused + // rather than treating the agent as already running + agent.state = 'suspending' + await agent.save() await app.containers.stopBrokerAgent(agent) setTimeout(async () => { await agent.destroy() diff --git a/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js b/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js index 9b3c3ae421..2cf80a3579 100644 --- a/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js +++ b/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js @@ -1,5 +1,7 @@ const { Op } = require('sequelize') -const should = require('should') // eslint-disable-line +const should = require('should') +const sinon = require('sinon') + const setup = require('../../setup') const FF_UTIL = require('flowforge-test-utils') @@ -806,6 +808,91 @@ describe('3rd Party Broker API', function () { response.statusCode.should.equal(200) }) + it('start keeps a new team-broker agent starting until the driver has launched it', async function () { + let stateDuringLaunch + const startBrokerAgent = sinon.stub(app.containers, 'startBrokerAgent').callsFake(async () => { + stateDuringLaunch = (await app.db.models.TeamBrokerAgent.byTeam(app.team.id)).state + }) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/start`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + stateDuringLaunch.should.equal('starting') + const agent = await app.db.models.TeamBrokerAgent.byTeam(app.team.id) + agent.state.should.equal('running') + } finally { + startBrokerAgent.restore() + } + }) + + it('start removes the team-broker agent if the driver fails to launch it', async function () { + const startBrokerAgent = sinon.stub(app.containers, 'startBrokerAgent').rejects(new Error('launch failed')) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/start`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(500) + const agent = await app.db.models.TeamBrokerAgent.byTeam(app.team.id) + should(agent?.state).be.undefined() + } finally { + startBrokerAgent.restore() + } + }) + + it('start replies 409 while the team-broker agent is suspending', async function () { + await app.db.models.TeamBrokerAgent.create({ state: 'suspending', TeamId: app.team.id }) + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/start`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(409) + response.json().should.have.property('code', 'agent_suspending') + }) + + it('suspend replies 409 while the team-broker agent is starting', async function () { + await app.db.models.TeamBrokerAgent.create({ state: 'starting', TeamId: app.team.id }) + const stopBrokerAgent = sinon.spy(app.containers, 'stopBrokerAgent') + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/suspend`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(409) + response.json().should.have.property('code', 'agent_starting') + stopBrokerAgent.called.should.be.false() + const agent = await app.db.models.TeamBrokerAgent.byTeam(app.team.id) + agent.state.should.equal('starting') + } finally { + stopBrokerAgent.restore() + } + }) + + it('suspend marks the team-broker agent suspending before stopping it', async function () { + await app.db.models.TeamBrokerAgent.create({ state: 'running', TeamId: app.team.id }) + let stateDuringStop + const stopBrokerAgent = sinon.stub(app.containers, 'stopBrokerAgent').callsFake(async () => { + stateDuringStop = (await app.db.models.TeamBrokerAgent.byTeam(app.team.id)).state + }) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/suspend`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + stateDuringStop.should.equal('suspending') + } finally { + stopBrokerAgent.restore() + } + }) + it('suspend replies not_found when there is no team-broker agent', async function () { const response = await app.inject({ method: 'POST',