From b6adb11f7b6dde1ae06ff14b5a70f551e49010d8 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 09:35:05 -0700 Subject: [PATCH 1/8] feat(hosted): new hosted installs run on the native engine; a migration moves the existing ones (TASK-265) values-dev sets backend.env.hostedEngine "native" (HOSTED_ENGINE), so a new "Run it here" install runs in-process from creation (#2224). The chart value is empty by default, which keeps the Durable Object. scripts/migrate-hosted-to-native (dry run by default, --apply) moves the existing installs, per agent identity: stop its Durable Object, revoke its runtime tokens, mark each of its active hosted installs native (keeping whether it was provisioned, carrying the seat's persona sentence), run recent pending events in-process and close stale ones with a reason. One identity failing is reported and does not stop the others. Production today: 8 active hosted installs, all on the Durable Object, 0 pending events. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../services/hostedEngineMigration.test.js | 135 ++++++++++++++++ backend/scripts/migrate-hosted-to-native.ts | 31 ++++ backend/services/hostedEngineMigration.ts | 153 ++++++++++++++++++ .../templates/core/backend-deployment.yaml | 7 + k8s/helm/commonly/values-dev.yaml | 5 + k8s/helm/commonly/values.yaml | 3 + 6 files changed, 334 insertions(+) create mode 100644 backend/__tests__/unit/services/hostedEngineMigration.test.js create mode 100644 backend/scripts/migrate-hosted-to-native.ts create mode 100644 backend/services/hostedEngineMigration.ts diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js new file mode 100644 index 0000000000..bf845931ab --- /dev/null +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -0,0 +1,135 @@ +/** + * TASK-265: moving hosted installs from the Durable Object onto the native + * engine. Real installation, event and user models on an in-memory Mongo; the + * worker call, the token revoke and the in-process dispatcher are mocked. + */ +const { MongoMemoryServer } = require('mongodb-memory-server'); +const mongoose = require('mongoose'); + +const mockDeprovision = jest.fn(); +jest.mock('../../../services/hostedRuntimeService', () => ({ + ...jest.requireActual('../../../services/hostedRuntimeService'), + isConfigured: () => true, + deprovisionAgent: (...args) => mockDeprovision(...args), +})); +const mockRevoke = jest.fn(); +jest.mock('../../../routes/registry/tokens', () => ({ + revokeRuntimeTokensForAgent: (...args) => mockRevoke(...args), +})); +const mockDispatch = jest.fn(); +jest.mock('../../../services/agentEventService', () => ({ + dispatchInProcess: (...args) => mockDispatch(...args), +})); + +let mongod; let AgentInstallation; let AgentEvent; let User; let migrate; +const NOW = new Date('2026-10-15T17:00:00Z'); +const OWNER = new mongoose.Types.ObjectId(); + +beforeAll(async () => { + mongod = await MongoMemoryServer.create(); + await mongoose.connect(mongod.getUri()); + ({ AgentInstallation } = require('../../../models/AgentRegistry')); + AgentEvent = require('../../../models/AgentEvent'); + User = require('../../../models/User'); + ({ migrateHostedInstallsToNative: migrate } = require('../../../services/hostedEngineMigration')); +}); + +afterAll(async () => { await mongoose.disconnect(); await mongod.stop(); }); + +const install = (agentName, podId, hosted) => AgentInstallation.create({ + agentName, + instanceId: 'default', + podId, + version: '1.0.0', + status: 'active', + installedBy: OWNER, + config: { runtime: { runtimeType: 'hosted' }, ...(hosted ? { hosted } : {}) }, +}); + +const hostedOf = async (id) => { + const row = await AgentInstallation.findById(id).lean(); + return row.config.hosted; +}; + +let podA; let podB; let maya; let mayaB; let idle; let already; +beforeEach(async () => { + jest.clearAllMocks(); + await Promise.all([AgentInstallation.deleteMany({}), AgentEvent.deleteMany({}), User.deleteMany({})]); + mockDeprovision.mockResolvedValue(true); + mockRevoke.mockResolvedValue(['hash-1']); + podA = new mongoose.Types.ObjectId(); + podB = new mongoose.Types.ObjectId(); + // One identity installed in two pods, a stopped one, and one already native. + maya = await install('maya-agent', podA, { provisionedAt: new Date('2026-10-01') }); + mayaB = await install('maya-agent', podB, { provisionedAt: new Date('2026-10-02') }); + idle = await install('idle-agent', podA, { provisionedAt: null, deprovisionedAt: new Date('2026-10-03') }); + already = await install('native-agent', podA, { engine: 'native', provisionedAt: new Date('2026-10-04') }); + await User.create({ + username: 'maya-agent', + email: 'maya-agent@agents.commonly.local', + password: 'x'.repeat(12), + isBot: true, + botMetadata: { agentName: 'maya-agent', instanceId: 'default', description: ' Keeps the research log. ' }, + }); +}); + +test('a dry run reports per identity and changes nothing', async () => { + const report = await migrate({ apply: false, now: NOW }); + + expect(report.dryRun).toBe(true); + expect(report.identities.map((entry) => [entry.agentName, entry.installs, entry.provisioned]).sort()) + .toEqual([['idle-agent', 1, false], ['maya-agent', 2, true]]); + expect(mockDeprovision).not.toHaveBeenCalled(); + expect(mockRevoke).not.toHaveBeenCalled(); + expect((await hostedOf(maya._id)).engine).toBeUndefined(); +}); + +test('apply stops the Durable Object, revokes tokens and marks every install of the identity native', async () => { + const report = await migrate({ apply: true, now: NOW }); + + const entry = report.identities.find((row) => row.agentName === 'maya-agent'); + expect(entry).toMatchObject({ durableObjectStopped: true, tokensRevoked: 1 }); + expect(mockDeprovision).toHaveBeenCalledWith('maya-agent', 'default'); + for (const id of [maya._id, mayaB._id]) { + // eslint-disable-next-line no-await-in-loop + expect(await hostedOf(id)).toEqual(expect.objectContaining({ + engine: 'native', provisionedAt: NOW, about: 'Keeps the research log.', migratedFrom: 'durable-object', + })); + } +}); + +test('a stopped install stays stopped, gets no worker call, and an already-native one is untouched', async () => { + await migrate({ apply: true, now: NOW }); + + expect(await hostedOf(idle._id)).toEqual(expect.objectContaining({ engine: 'native', provisionedAt: null })); + expect(mockDeprovision).not.toHaveBeenCalledWith('idle-agent', 'default'); + const untouched = await hostedOf(already._id); + expect(untouched.migratedFrom).toBeUndefined(); +}); + +test('a recent pending event runs in-process; a stale one is closed with a reason', async () => { + const [recent, stale] = await AgentEvent.create([ + { agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', createdAt: new Date(NOW.getTime() - 60 * 1000) }, + { agentName: 'maya-agent', instanceId: 'default', podId: podB, type: 'chat.mention', payload: {}, status: 'pending', createdAt: new Date(NOW.getTime() - 60 * 60 * 1000) }, + ]); + + const report = await migrate({ apply: true, now: NOW }); + + expect(report.identities.find((row) => row.agentName === 'maya-agent')).toMatchObject({ pendingRun: 1, pendingClosed: 1 }); + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(String(mockDispatch.mock.calls[0][0]._id)).toBe(String(recent._id)); + expect((await AgentEvent.findById(recent._id).lean()).status).toBe('delivered'); + const closed = await AgentEvent.findById(stale._id).lean(); + expect(closed.status).toBe('failed'); + expect(closed.error).toMatch(/hosted engine migrated/); +}); + +test('one identity failing is reported and does not stop the others', async () => { + mockDeprovision.mockRejectedValueOnce(new Error('worker down')); + + const report = await migrate({ apply: true, now: NOW }); + + expect(report.identities.find((row) => row.agentName === 'maya-agent').error).toBe('worker down'); + expect((await hostedOf(maya._id)).engine).toBeUndefined(); + expect((await hostedOf(idle._id)).engine).toBe('native'); +}); diff --git a/backend/scripts/migrate-hosted-to-native.ts b/backend/scripts/migrate-hosted-to-native.ts new file mode 100644 index 0000000000..6d2e1d0c23 --- /dev/null +++ b/backend/scripts/migrate-hosted-to-native.ts @@ -0,0 +1,31 @@ +#!/usr/bin/env node +/* + * Move every active hosted install from the Durable Object onto the native + * engine (TASK-265). The rules live in services/hostedEngineMigration.ts. + * Dry run by default; `--apply` changes rows. + * + * Usage (inside the backend pod, where MONGO_URI and the hosted worker env are set): + * node dist/scripts/migrate-hosted-to-native.js # report + * node dist/scripts/migrate-hosted-to-native.js --apply # migrate + */ +import mongoose from 'mongoose'; + +const { migrateHostedInstallsToNative } = require('../services/hostedEngineMigration'); + +export const main = async (argv: string[]): Promise => { + await mongoose.connect(String(process.env.MONGO_URI)); + try { + const report = await migrateHostedInstallsToNative({ apply: argv.includes('--apply') }); + console.log(JSON.stringify(report, null, 2)); + return report.identities.some((entry: { error?: string }) => entry.error) ? 1 : 0; + } finally { + await mongoose.disconnect(); + } +}; + +if (require.main === module) { + main(process.argv.slice(2)).then((code) => process.exit(code)).catch((err) => { + console.error(err); + process.exit(1); + }); +} diff --git a/backend/services/hostedEngineMigration.ts b/backend/services/hostedEngineMigration.ts new file mode 100644 index 0000000000..bbf2de1ab6 --- /dev/null +++ b/backend/services/hostedEngineMigration.ts @@ -0,0 +1,153 @@ +/** + * TASK-265: move every active hosted install from the Durable Object onto the + * native engine (ADR-023 amendment A1). Dry run by default. + * + * Per agent identity (agentName, instanceId), because one agent can be + * installed in several pods: + * 1. stop its Durable Object, when one was provisioned and the worker is + * configured, so nothing outside the backend still polls as it; + * 2. revoke its runtime tokens: a native-engine agent holds none + * (routes/registry/tokens.ts refuses to mint one); + * 3. mark each of its active hosted installs native, keeping whether it was + * provisioned, and carrying the persona sentence the seat shows + * (botMetadata.description) into the hosted prompt; + * 4. events still pending for it, which nothing polls once the Durable + * Object stops, run in-process when recent and are closed with a reason + * when stale, so a person's question is answered or visibly dropped, + * never silently parked. + * One identity failing is reported and does not stop the others. + */ +import crypto from 'crypto'; +import { HOSTED_ENGINE_NATIVE, hostedEngineOf, readHostedConfig } from './hostedEngine'; + +// eslint-disable-next-line @typescript-eslint/no-require-imports +const { AgentInstallation } = require('../models/AgentRegistry'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const AgentEvent = require('../models/AgentEvent'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const User = require('../models/User'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const AgentIdentityService = require('./agentIdentityService'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const hostedRuntime = require('./hostedRuntimeService'); + +const STALE_PENDING_MS = 10 * 60 * 1000; +const ABOUT_LIMIT = 160; + +export interface IdentityReport { + agentName: string; + instanceId: string; + installs: number; + provisioned: boolean; + durableObjectStopped?: boolean; + tokensRevoked?: number; + pendingRun: number; + pendingClosed: number; + error?: string; +} + +export interface MigrationReport { + dryRun: boolean; + identities: IdentityReport[]; +} + +const aboutFrom = (description: unknown): string | undefined => { + const text = typeof description === 'string' ? description.trim().replace(/\s+/g, ' ').slice(0, ABOUT_LIMIT) : ''; + return text || undefined; +}; + +export const migrateHostedInstallsToNative = async ( + { apply = false, now = new Date() }: { apply?: boolean; now?: Date } = {}, +): Promise => { + const report: MigrationReport = { dryRun: !apply, identities: [] }; + const installs = await AgentInstallation.find({ status: 'active', 'config.runtime.runtimeType': 'hosted' }); + const byIdentity = new Map(); + for (const install of installs) { + if (hostedEngineOf(install) === HOSTED_ENGINE_NATIVE) continue; + const key = `${install.agentName}:${install.instanceId || 'default'}`; + byIdentity.set(key, [...(byIdentity.get(key) || []), install]); + } + + for (const rows of byIdentity.values()) { + const agentName = String(rows[0].agentName); + const instanceId = String(rows[0].instanceId || 'default'); + const provisioned = rows.some((row) => Boolean(readHostedConfig(row).provisionedAt)); + // eslint-disable-next-line no-await-in-loop + const pending = await AgentEvent.find({ agentName, instanceId, status: 'pending' }).lean(); + const isStale = (event: any) => now.getTime() - new Date(event.createdAt).getTime() >= STALE_PENDING_MS; + const entry: IdentityReport = { + agentName, + instanceId, + installs: rows.length, + provisioned, + pendingRun: pending.filter((event: any) => !isStale(event)).length, + pendingClosed: pending.filter(isStale).length, + }; + report.identities.push(entry); + if (!apply) continue; + + try { + if (provisioned && hostedRuntime.isConfigured()) { + // eslint-disable-next-line no-await-in-loop + await hostedRuntime.deprovisionAgent(agentName, instanceId); + entry.durableObjectStopped = true; + } + // eslint-disable-next-line no-await-in-loop + const agentUser = await User.findOne({ + username: AgentIdentityService.buildAgentUsername(agentName, instanceId), + isBot: true, + }); + if (agentUser) { + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { revokeRuntimeTokensForAgent } = require('../routes/registry/tokens'); + // eslint-disable-next-line no-await-in-loop + const revoked = await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation: rows[0] }); + entry.tokensRevoked = revoked.length; + } + const about = aboutFrom(agentUser?.botMetadata?.description); + for (const row of rows) { + const wasProvisioned = Boolean(readHostedConfig(row).provisionedAt); + row.config.set('hosted', { + engine: HOSTED_ENGINE_NATIVE, + provisionedAt: wasProvisioned ? now : null, + ...(about ? { about } : {}), + migratedFrom: 'durable-object', + migratedAt: now, + }); + // eslint-disable-next-line no-await-in-loop + await row.save(); + } + + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const AgentEventService = require('./agentEventService'); + for (const event of pending) { + if (isStale(event)) { + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne( + { _id: event._id, status: 'pending' }, + { $set: { status: 'failed', error: 'hosted engine migrated before this event ran (stale)' } }, + ); + continue; + } + // eslint-disable-next-line no-await-in-loop + const installation = rows.find((row) => String(row.podId) === String(event.podId)); + if (!installation || !readHostedConfig(installation).provisionedAt) continue; + // eslint-disable-next-line no-await-in-loop + const claimed = await AgentEvent.findOneAndUpdate( + { _id: event._id, status: 'pending' }, + { + $set: { status: 'delivered', deliveredAt: new Date(), deliveryNonce: crypto.randomBytes(16).toString('hex') }, + $inc: { attempts: 1 }, + }, + { new: true }, + ); + if (claimed) AgentEventService.dispatchInProcess(claimed, installation, agentName, instanceId, String(claimed.type || '')); + } + } catch (error) { + entry.error = (error as Error).message; + } + } + return report; +}; + +module.exports = { migrateHostedInstallsToNative }; diff --git a/k8s/helm/commonly/templates/core/backend-deployment.yaml b/k8s/helm/commonly/templates/core/backend-deployment.yaml index 1bba24f469..8d7a0329f2 100644 --- a/k8s/helm/commonly/templates/core/backend-deployment.yaml +++ b/k8s/helm/commonly/templates/core/backend-deployment.yaml @@ -174,6 +174,13 @@ spec: - name: HOSTED_AGENTS_PER_USER value: {{ .Values.backend.env.hostedAgentsPerUser | quote }} {{- end }} + {{- if .Values.backend.env.hostedEngine }} + # The engine NEW hosted installs land on (services/hostedEngine.ts): + # "native" runs their turns in-process; unset keeps the Durable Object. + # Existing installs move with scripts/migrate-hosted-to-native. + - name: HOSTED_ENGINE + value: {{ .Values.backend.env.hostedEngine | quote }} + {{- end }} {{- if .Values.backend.env.hostedTurnsPerDay }} - name: HOSTED_TURNS_PER_DAY value: {{ .Values.backend.env.hostedTurnsPerDay | quote }} diff --git a/k8s/helm/commonly/values-dev.yaml b/k8s/helm/commonly/values-dev.yaml index b02a6a9090..e99f4d8982 100644 --- a/k8s/helm/commonly/values-dev.yaml +++ b/k8s/helm/commonly/values-dev.yaml @@ -50,6 +50,11 @@ backend: # hosted-agent entitlement (redeemed at signup or later), not signup # itself. BYO/local agents are open to every account. registrationInviteOnly: "0" + # TASK-265: new "Run it here" installs run in-process on the native engine + # (AgentRun per turn, per-user ceiling, tool gate, connectors). Existing + # installs move with dist/scripts/migrate-hosted-to-native.js (dry run, + # then --apply). Empty would put new installs back on the Durable Object. + hostedEngine: "native" agentProvisionerK8s: "1" agentProvisionerDocker: "0" agentProvisionerNodePool: "dev" diff --git a/k8s/helm/commonly/values.yaml b/k8s/helm/commonly/values.yaml index fd9afeb0e1..27405366ef 100644 --- a/k8s/helm/commonly/values.yaml +++ b/k8s/helm/commonly/values.yaml @@ -136,6 +136,9 @@ backend: hostedRuntimeUrl: "" hostedAgentsPerUser: "" hostedTurnsPerDay: "" + # "native" to run new hosted installs in-process (TASK-261); empty keeps + # the Durable Object. + hostedEngine: "" clawdbotGatewayUrl: "" moltbotGatewayUrl: "" pgHost: "YOUR_PG_HOST" From 8581f044ac6c6ae9138d1ba596ad41bf316d115e Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 09:49:59 -0700 Subject: [PATCH 2/8] fix(hosted): the migration marks native first, restores on failure, and runs no turns itself Rhea's and sprint-review's HOLDs on #2236, per identity: - Refuse (nothing changed) when the Durable Object is provisioned but the worker is not configured: it could not be stopped (Rhea 3). - Mark every row native FIRST, so the token issuer refuses mints before the revoke; no window for a token to survive (Rhea 1). A failed row restores the rows already marked; a Durable Object that will not stop restores every row, so a failure leaves the identity as it was (Rhea 2). - A failed revoke leaves the rows native (minting refused) and the Durable Object stopped, and says to run again; every run revokes for already-migrated identities, so the re-run finishes it. - The script runs no turns (Rhea 4): recent pending events on a provisioned install are marked delivered for the in-process sweep (#2226) to run; recent ones on a stopped install, and stale ones, are closed with a reason (sprint-review). The dry run reports all three. - The bot username goes through resolveAgentType, like the runtime's lookup. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../services/hostedEngineMigration.test.js | 142 +++++++----- backend/services/hostedEngineMigration.ts | 206 ++++++++++++------ 2 files changed, 231 insertions(+), 117 deletions(-) diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index bf845931ab..1f0a8fe720 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -1,25 +1,23 @@ /** * TASK-265: moving hosted installs from the Durable Object onto the native * engine. Real installation, event and user models on an in-memory Mongo; the - * worker call, the token revoke and the in-process dispatcher are mocked. + * worker calls and the token revoke are mocked. Each arm below is one of the + * findings in Rhea's and sprint-review's HOLDs on #2236. */ const { MongoMemoryServer } = require('mongodb-memory-server'); const mongoose = require('mongoose'); +let mockConfigured = true; const mockDeprovision = jest.fn(); jest.mock('../../../services/hostedRuntimeService', () => ({ ...jest.requireActual('../../../services/hostedRuntimeService'), - isConfigured: () => true, + isConfigured: () => mockConfigured, deprovisionAgent: (...args) => mockDeprovision(...args), })); const mockRevoke = jest.fn(); jest.mock('../../../routes/registry/tokens', () => ({ revokeRuntimeTokensForAgent: (...args) => mockRevoke(...args), })); -const mockDispatch = jest.fn(); -jest.mock('../../../services/agentEventService', () => ({ - dispatchInProcess: (...args) => mockDispatch(...args), -})); let mongod; let AgentInstallation; let AgentEvent; let User; let migrate; const NOW = new Date('2026-10-15T17:00:00Z'); @@ -45,25 +43,24 @@ const install = (agentName, podId, hosted) => AgentInstallation.create({ installedBy: OWNER, config: { runtime: { runtimeType: 'hosted' }, ...(hosted ? { hosted } : {}) }, }); +const hostedOf = async (id) => (await AgentInstallation.findById(id).lean()).config.hosted; +const engines = async (ids) => Promise.all(ids.map(async (id) => (await hostedOf(id))?.engine)); +const entryFor = (report, name) => report.identities.find((row) => row.agentName === name); -const hostedOf = async (id) => { - const row = await AgentInstallation.findById(id).lean(); - return row.config.hosted; -}; - -let podA; let podB; let maya; let mayaB; let idle; let already; +let podA; let podB; let maya; let mayaB; let idle; let born; beforeEach(async () => { + jest.restoreAllMocks(); jest.clearAllMocks(); + mockConfigured = true; await Promise.all([AgentInstallation.deleteMany({}), AgentEvent.deleteMany({}), User.deleteMany({})]); mockDeprovision.mockResolvedValue(true); mockRevoke.mockResolvedValue(['hash-1']); podA = new mongoose.Types.ObjectId(); podB = new mongoose.Types.ObjectId(); - // One identity installed in two pods, a stopped one, and one already native. maya = await install('maya-agent', podA, { provisionedAt: new Date('2026-10-01') }); mayaB = await install('maya-agent', podB, { provisionedAt: new Date('2026-10-02') }); idle = await install('idle-agent', podA, { provisionedAt: null, deprovisionedAt: new Date('2026-10-03') }); - already = await install('native-agent', podA, { engine: 'native', provisionedAt: new Date('2026-10-04') }); + born = await install('native-agent', podA, { engine: 'native', provisionedAt: new Date('2026-10-04') }); await User.create({ username: 'maya-agent', email: 'maya-agent@agents.commonly.local', @@ -77,59 +74,106 @@ test('a dry run reports per identity and changes nothing', async () => { const report = await migrate({ apply: false, now: NOW }); expect(report.dryRun).toBe(true); - expect(report.identities.map((entry) => [entry.agentName, entry.installs, entry.provisioned]).sort()) + expect(report.identities.map((row) => [row.agentName, row.installs, row.provisioned]).sort()) .toEqual([['idle-agent', 1, false], ['maya-agent', 2, true]]); expect(mockDeprovision).not.toHaveBeenCalled(); expect(mockRevoke).not.toHaveBeenCalled(); - expect((await hostedOf(maya._id)).engine).toBeUndefined(); + expect(await engines([maya._id, mayaB._id, idle._id])).toEqual([undefined, undefined, undefined]); }); -test('apply stops the Durable Object, revokes tokens and marks every install of the identity native', async () => { +test('rows are native BEFORE the revoke, so no token can be minted in between (Rhea 1)', async () => { + let enginesAtRevoke; + mockRevoke.mockImplementation(async () => { + enginesAtRevoke = await engines([maya._id, mayaB._id]); + return ['hash-1']; + }); + const report = await migrate({ apply: true, now: NOW }); - const entry = report.identities.find((row) => row.agentName === 'maya-agent'); - expect(entry).toMatchObject({ durableObjectStopped: true, tokensRevoked: 1 }); - expect(mockDeprovision).toHaveBeenCalledWith('maya-agent', 'default'); - for (const id of [maya._id, mayaB._id]) { - // eslint-disable-next-line no-await-in-loop - expect(await hostedOf(id)).toEqual(expect.objectContaining({ - engine: 'native', provisionedAt: NOW, about: 'Keeps the research log.', migratedFrom: 'durable-object', - })); - } + expect(enginesAtRevoke).toEqual(['native', 'native']); + expect(entryFor(report, 'maya-agent')).toMatchObject({ durableObjectStopped: true, tokensRevoked: 1 }); + expect(await hostedOf(maya._id)).toEqual(expect.objectContaining({ + engine: 'native', provisionedAt: NOW, about: 'Keeps the research log.', migratedFrom: 'durable-object', + })); }); -test('a stopped install stays stopped, gets no worker call, and an already-native one is untouched', async () => { - await migrate({ apply: true, now: NOW }); +test('a row that fails to save restores the others; the worker and the tokens are untouched (Rhea 2)', async () => { + // Fail exactly the second row's native mark, after the first row of the + // same identity is already native, so the restore has real work to do. + const original = AgentInstallation.updateOne.bind(AgentInstallation); + jest.spyOn(AgentInstallation, 'updateOne').mockImplementation((filter, update, ...rest) => { + const marking = update?.$set?.['config.hosted']?.engine === 'native'; + if (marking && String(filter?._id) === String(mayaB._id)) return Promise.reject(new Error('write failed')); + return original(filter, update, ...rest); + }); - expect(await hostedOf(idle._id)).toEqual(expect.objectContaining({ engine: 'native', provisionedAt: null })); - expect(mockDeprovision).not.toHaveBeenCalledWith('idle-agent', 'default'); - const untouched = await hostedOf(already._id); - expect(untouched.migratedFrom).toBeUndefined(); + const report = await migrate({ apply: true, now: NOW }); + + expect(entryFor(report, 'maya-agent').error).toMatch(/restored, nothing changed/); + expect(await engines([maya._id, mayaB._id])).toEqual([undefined, undefined]); + expect((await hostedOf(maya._id)).provisionedAt).toEqual(new Date('2026-10-01')); + expect(mockDeprovision).not.toHaveBeenCalledWith('maya-agent', 'default'); + expect(mockRevoke).not.toHaveBeenCalled(); }); -test('a recent pending event runs in-process; a stale one is closed with a reason', async () => { - const [recent, stale] = await AgentEvent.create([ - { agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', createdAt: new Date(NOW.getTime() - 60 * 1000) }, - { agentName: 'maya-agent', instanceId: 'default', podId: podB, type: 'chat.mention', payload: {}, status: 'pending', createdAt: new Date(NOW.getTime() - 60 * 60 * 1000) }, - ]); +test('a Durable Object that will not stop restores every row (Rhea 2)', async () => { + mockDeprovision.mockRejectedValueOnce(new Error('worker down')); const report = await migrate({ apply: true, now: NOW }); - expect(report.identities.find((row) => row.agentName === 'maya-agent')).toMatchObject({ pendingRun: 1, pendingClosed: 1 }); - expect(mockDispatch).toHaveBeenCalledTimes(1); - expect(String(mockDispatch.mock.calls[0][0]._id)).toBe(String(recent._id)); - expect((await AgentEvent.findById(recent._id).lean()).status).toBe('delivered'); - const closed = await AgentEvent.findById(stale._id).lean(); - expect(closed.status).toBe('failed'); - expect(closed.error).toMatch(/hosted engine migrated/); + expect(entryFor(report, 'maya-agent').error).toMatch(/did not stop .*restored/); + expect(await engines([maya._id, mayaB._id])).toEqual([undefined, undefined]); + expect(mockRevoke).not.toHaveBeenCalled(); }); -test('one identity failing is reported and does not stop the others', async () => { - mockDeprovision.mockRejectedValueOnce(new Error('worker down')); +test('a failed revoke is reported, and the next run finishes it (Rhea 2)', async () => { + mockRevoke.mockRejectedValueOnce(new Error('db down')); + const first = await migrate({ apply: true, now: NOW }); + expect(entryFor(first, 'maya-agent').error).toMatch(/run again to finish/); + expect(await engines([maya._id, mayaB._id])).toEqual(['native', 'native']); + + const second = await migrate({ apply: true, now: NOW }); + expect(entryFor(second, 'maya-agent')).toMatchObject({ alreadyMigrated: true, tokensRevoked: 1 }); + expect(mockDeprovision).toHaveBeenCalledTimes(1); // not stopped twice +}); + +test('a provisioned identity is refused when the worker is not configured (Rhea 3)', async () => { + mockConfigured = false; const report = await migrate({ apply: true, now: NOW }); - expect(report.identities.find((row) => row.agentName === 'maya-agent').error).toBe('worker down'); - expect((await hostedOf(maya._id)).engine).toBeUndefined(); - expect((await hostedOf(idle._id)).engine).toBe('native'); + expect(entryFor(report, 'maya-agent').error).toMatch(/cannot be stopped; refused/); + expect(await engines([maya._id, mayaB._id])).toEqual([undefined, undefined]); + expect(mockRevoke).not.toHaveBeenCalledWith(expect.objectContaining({ agentName: 'maya-agent' })); + // A stopped identity needs no worker and still moves. + expect(await engines([idle._id])).toEqual(['native']); +}); + +test('pending events: recent on a running install go to the sweep; on a stopped one, or stale, they close (Rhea 4, sprint-review)', async () => { + const at = (ms) => new Date(NOW.getTime() - ms); + const [toSweep, stale, onStopped] = await AgentEvent.create([ + { agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', createdAt: at(60 * 1000) }, + { agentName: 'maya-agent', instanceId: 'default', podId: podB, type: 'chat.mention', payload: {}, status: 'pending', createdAt: at(60 * 60 * 1000) }, + { agentName: 'idle-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', createdAt: at(60 * 1000) }, + ]); + + const dry = await migrate({ apply: false, now: NOW }); + expect(entryFor(dry, 'maya-agent')).toMatchObject({ pendingToSweep: 1, pendingClosed: 1 }); + expect(entryFor(dry, 'idle-agent')).toMatchObject({ pendingToSweep: 0, pendingClosed: 1 }); + + await migrate({ apply: true, now: NOW }); + + const swept = await AgentEvent.findById(toSweep._id).lean(); + expect(swept).toMatchObject({ status: 'delivered', deliveryNonce: null }); + expect((await AgentEvent.findById(stale._id).lean())).toMatchObject({ status: 'failed', error: expect.stringMatching(/stale/) }); + expect((await AgentEvent.findById(onStopped._id).lean())).toMatchObject({ status: 'failed', error: expect.stringMatching(/stopped/) }); +}); + +test('a stopped install stays stopped with no worker call; one born native is not touched or reported', async () => { + const report = await migrate({ apply: true, now: NOW }); + + expect(await hostedOf(idle._id)).toEqual(expect.objectContaining({ engine: 'native', provisionedAt: null })); + expect(mockDeprovision).not.toHaveBeenCalledWith('idle-agent', 'default'); + expect((await hostedOf(born._id)).migratedFrom).toBeUndefined(); + expect(entryFor(report, 'native-agent')).toBeUndefined(); }); diff --git a/backend/services/hostedEngineMigration.ts b/backend/services/hostedEngineMigration.ts index bbf2de1ab6..e4f98647c0 100644 --- a/backend/services/hostedEngineMigration.ts +++ b/backend/services/hostedEngineMigration.ts @@ -2,22 +2,27 @@ * TASK-265: move every active hosted install from the Durable Object onto the * native engine (ADR-023 amendment A1). Dry run by default. * - * Per agent identity (agentName, instanceId), because one agent can be - * installed in several pods: - * 1. stop its Durable Object, when one was provisioned and the worker is - * configured, so nothing outside the backend still polls as it; - * 2. revoke its runtime tokens: a native-engine agent holds none - * (routes/registry/tokens.ts refuses to mint one); - * 3. mark each of its active hosted installs native, keeping whether it was - * provisioned, and carrying the persona sentence the seat shows - * (botMetadata.description) into the hosted prompt; - * 4. events still pending for it, which nothing polls once the Durable - * Object stops, run in-process when recent and are closed with a reason - * when stale, so a person's question is answered or visibly dropped, - * never silently parked. + * Per agent identity (agentName, instanceId), because one agent can sit in + * several pods, and in this order (Rhea and sprint-review on #2236): + * 0. Refuse the identity, changing nothing, when its Durable Object is + * provisioned but the hosted worker is not configured: it could not be + * stopped, and would keep polling as the agent. + * 1. Mark every row native FIRST. From that moment the token issuer refuses + * to mint for the identity (routes/registry/tokens.ts), so no token can + * be issued between the revoke and the mark. A failed row restores the + * rows already marked. + * 2. Stop the Durable Object. On failure every row is restored, and the + * identity is exactly as it was. + * 3. Revoke its runtime tokens. On failure the rows stay native (minting is + * refused) and the Durable Object is stopped; the report says to run + * again, and every run revokes for already-migrated identities. + * 4. Pending events, which nothing polls once the Durable Object stops: a + * recent one on a provisioned install is handed to the in-process sweep + * (marked delivered; agentEventService re-runs it), so this script never + * runs a turn itself. A recent one on a stopped install, and any stale + * one, is closed with a reason. * One identity failing is reported and does not stop the others. */ -import crypto from 'crypto'; import { HOSTED_ENGINE_NATIVE, hostedEngineOf, readHostedConfig } from './hostedEngine'; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -33,15 +38,19 @@ const hostedRuntime = require('./hostedRuntimeService'); const STALE_PENDING_MS = 10 * 60 * 1000; const ABOUT_LIMIT = 160; +const MIGRATED_FROM = 'durable-object'; export interface IdentityReport { agentName: string; instanceId: string; installs: number; provisioned: boolean; + alreadyMigrated?: boolean; durableObjectStopped?: boolean; tokensRevoked?: number; - pendingRun: number; + /** Recent events on a provisioned install, handed to the in-process sweep. */ + pendingToSweep: number; + /** Recent events on a stopped install, and stale ones, closed with a reason. */ pendingClosed: number; error?: string; } @@ -56,14 +65,32 @@ const aboutFrom = (description: unknown): string | undefined => { return text || undefined; }; +const setHosted = (id: unknown, hosted: Record | null) => AgentInstallation.updateOne( + { _id: id }, + hosted && Object.keys(hosted).length ? { $set: { 'config.hosted': hosted } } : { $unset: { 'config.hosted': '' } }, +); + +const findAgentUser = (agentName: string, instanceId: string) => User.findOne({ + username: AgentIdentityService.buildAgentUsername(AgentIdentityService.resolveAgentType(agentName), instanceId), + isBot: true, +}); + +const revokeFor = async (agentName: string, instanceId: string, installation: unknown): Promise => { + const agentUser = await findAgentUser(agentName, instanceId); + if (!agentUser) return 0; + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { revokeRuntimeTokensForAgent } = require('../routes/registry/tokens'); + const revoked = await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation }); + return revoked.length; +}; + export const migrateHostedInstallsToNative = async ( { apply = false, now = new Date() }: { apply?: boolean; now?: Date } = {}, ): Promise => { const report: MigrationReport = { dryRun: !apply, identities: [] }; - const installs = await AgentInstallation.find({ status: 'active', 'config.runtime.runtimeType': 'hosted' }); + const installs = await AgentInstallation.find({ status: 'active', 'config.runtime.runtimeType': 'hosted' }).lean(); const byIdentity = new Map(); for (const install of installs) { - if (hostedEngineOf(install) === HOSTED_ENGINE_NATIVE) continue; const key = `${install.agentName}:${install.instanceId || 'default'}`; byIdentity.set(key, [...(byIdentity.get(key) || []), install]); } @@ -71,80 +98,123 @@ export const migrateHostedInstallsToNative = async ( for (const rows of byIdentity.values()) { const agentName = String(rows[0].agentName); const instanceId = String(rows[0].instanceId || 'default'); - const provisioned = rows.some((row) => Boolean(readHostedConfig(row).provisionedAt)); + const todo = rows.filter((row) => hostedEngineOf(row) !== HOSTED_ENGINE_NATIVE); + const isProvisioned = (row: any) => Boolean(readHostedConfig(row).provisionedAt); + + if (!todo.length) { + // Already moved by an earlier run. Finish it: a migrated identity holds + // no runtime token, even if that run's revoke failed. + if (!rows.some((row) => readHostedConfig(row).migratedFrom === MIGRATED_FROM)) continue; + const entry: IdentityReport = { + agentName, instanceId, installs: rows.length, provisioned: rows.some(isProvisioned), + alreadyMigrated: true, pendingToSweep: 0, pendingClosed: 0, + }; + report.identities.push(entry); + if (apply) { + try { + // eslint-disable-next-line no-await-in-loop + entry.tokensRevoked = await revokeFor(agentName, instanceId, rows[0]); + } catch (error) { + entry.error = `tokens not revoked (${(error as Error).message}); run again`; + } + } + continue; + } + + const provisioned = todo.some(isProvisioned); // eslint-disable-next-line no-await-in-loop const pending = await AgentEvent.find({ agentName, instanceId, status: 'pending' }).lean(); - const isStale = (event: any) => now.getTime() - new Date(event.createdAt).getTime() >= STALE_PENDING_MS; + const podIsRunning = (podId: unknown) => todo.some((row) => String(row.podId) === String(podId) && isProvisioned(row)); + const recent = (event: any) => now.getTime() - new Date(event.createdAt).getTime() < STALE_PENDING_MS; + const toSweep = pending.filter((event: any) => recent(event) && podIsRunning(event.podId)); + const toClose = pending.filter((event: any) => !toSweep.includes(event)); const entry: IdentityReport = { agentName, instanceId, - installs: rows.length, + installs: todo.length, provisioned, - pendingRun: pending.filter((event: any) => !isStale(event)).length, - pendingClosed: pending.filter(isStale).length, + pendingToSweep: toSweep.length, + pendingClosed: toClose.length, }; report.identities.push(entry); + + if (provisioned && !hostedRuntime.isConfigured()) { + entry.error = 'its Durable Object is provisioned but the hosted worker is not configured, so it cannot be ' + + 'stopped; refused, nothing changed'; + continue; + } if (!apply) continue; - try { - if (provisioned && hostedRuntime.isConfigured()) { + // 1. Mark every row native first, so minting is refused from here on. + // eslint-disable-next-line no-await-in-loop + const agentUser = await findAgentUser(agentName, instanceId); + const about = aboutFrom(agentUser?.botMetadata?.description); + const previous = new Map(todo.map((row) => [String(row._id), readHostedConfig(row)])); + const restore = async (ids: string[]) => { + for (const id of ids) { // eslint-disable-next-line no-await-in-loop - await hostedRuntime.deprovisionAgent(agentName, instanceId); - entry.durableObjectStopped = true; + await setHosted(id, previous.get(id) || null); } - // eslint-disable-next-line no-await-in-loop - const agentUser = await User.findOne({ - username: AgentIdentityService.buildAgentUsername(agentName, instanceId), - isBot: true, - }); - if (agentUser) { - // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports - const { revokeRuntimeTokensForAgent } = require('../routes/registry/tokens'); + }; + const marked: string[] = []; + try { + for (const row of todo) { // eslint-disable-next-line no-await-in-loop - const revoked = await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation: rows[0] }); - entry.tokensRevoked = revoked.length; - } - const about = aboutFrom(agentUser?.botMetadata?.description); - for (const row of rows) { - const wasProvisioned = Boolean(readHostedConfig(row).provisionedAt); - row.config.set('hosted', { + await setHosted(row._id, { engine: HOSTED_ENGINE_NATIVE, - provisionedAt: wasProvisioned ? now : null, + provisionedAt: isProvisioned(row) ? now : null, ...(about ? { about } : {}), - migratedFrom: 'durable-object', + migratedFrom: MIGRATED_FROM, migratedAt: now, }); - // eslint-disable-next-line no-await-in-loop - await row.save(); + marked.push(String(row._id)); } + } catch (error) { + // eslint-disable-next-line no-await-in-loop + await restore(marked).catch(() => undefined); + entry.error = `marking failed (${(error as Error).message}); restored, nothing changed`; + continue; + } - // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports - const AgentEventService = require('./agentEventService'); - for (const event of pending) { - if (isStale(event)) { - // eslint-disable-next-line no-await-in-loop - await AgentEvent.updateOne( - { _id: event._id, status: 'pending' }, - { $set: { status: 'failed', error: 'hosted engine migrated before this event ran (stale)' } }, - ); - continue; - } + // 2. Stop the Durable Object; on failure, put every row back. + if (provisioned) { + try { // eslint-disable-next-line no-await-in-loop - const installation = rows.find((row) => String(row.podId) === String(event.podId)); - if (!installation || !readHostedConfig(installation).provisionedAt) continue; + await hostedRuntime.deprovisionAgent(agentName, instanceId); + entry.durableObjectStopped = true; + } catch (error) { // eslint-disable-next-line no-await-in-loop - const claimed = await AgentEvent.findOneAndUpdate( - { _id: event._id, status: 'pending' }, - { - $set: { status: 'delivered', deliveredAt: new Date(), deliveryNonce: crypto.randomBytes(16).toString('hex') }, - $inc: { attempts: 1 }, - }, - { new: true }, - ); - if (claimed) AgentEventService.dispatchInProcess(claimed, installation, agentName, instanceId, String(claimed.type || '')); + await restore(marked).catch(() => undefined); + entry.error = `the Durable Object did not stop (${(error as Error).message}); restored, nothing changed`; + continue; } + } + + // 3. Revoke. The rows are native (no new token can be minted) and the + // Durable Object is stopped, so a failure here is finished by a re-run. + try { + // eslint-disable-next-line no-await-in-loop + entry.tokensRevoked = await revokeFor(agentName, instanceId, todo[0]); } catch (error) { - entry.error = (error as Error).message; + entry.error = `tokens not revoked (${(error as Error).message}); the rows are native and the Durable ` + + 'Object is stopped; run again to finish'; + } + + // 4. Pending events: hand recent ones on a running install to the sweep, + // close the rest with a reason. This script never runs a turn itself. + for (const event of toSweep) { + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne( + { _id: event._id, status: 'pending' }, + { $set: { status: 'delivered', deliveredAt: now, deliveryNonce: null }, $inc: { attempts: 1 } }, + ); + } + for (const event of toClose) { + const reason = recent(event) + ? 'hosted engine migrated while this agent was stopped; it was not run' + : 'hosted engine migrated before this event ran (stale)'; + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne({ _id: event._id, status: 'pending' }, { $set: { status: 'failed', error: reason } }); } } return report; From 72df53576a3748de9b3e27c32456edb5cb5b0d50 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:04:31 -0700 Subject: [PATCH 3/8] fix(hosted): a forward-only, resumable migration, and an issuer that takes back an in-flight mint Rhea's third review of #2236 (5536866789): 1. An in-flight mint survived: the issuer checks in-process once, then saves. Both issuers now re-check after the save and, if the identity went in-process meanwhile, revoke the token they just stored and refuse. A save that lands before the migration's revoke is caught by that revoke, so no interleaving leaves a live token. 2. A native marker was taken for a completed shutdown. Each row now records config.hosted.migration { state: marked | stopped | done, doProvisioned }, so a run interrupted before the stop stops it on the next run. 3. A failed rollback was reported as a clean restore. There is no rollback now: the migration is forward-only, each step idempotent, and a failure stops that identity at the step reported. 4. Events were stranded after a write failure. Every unfinished identity is resumed from its recorded state, events included. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../routes/registry.runtime-tokens.test.js | 38 +++ .../services/hostedEngineMigration.test.js | 126 +++++---- backend/routes/registry/tokens.ts | 15 ++ backend/services/hostedEngineMigration.ts | 249 ++++++++++-------- 4 files changed, 259 insertions(+), 169 deletions(-) diff --git a/backend/__tests__/unit/routes/registry.runtime-tokens.test.js b/backend/__tests__/unit/routes/registry.runtime-tokens.test.js index 1335d4a8c6..6e27e63be1 100644 --- a/backend/__tests__/unit/routes/registry.runtime-tokens.test.js +++ b/backend/__tests__/unit/routes/registry.runtime-tokens.test.js @@ -141,6 +141,44 @@ describe('agent runtime tokens', () => { expect(agentUser.agentRuntimeTokens).toHaveLength(0); }); + // Rhea on #2236: a mint that passed the in-process check can still be + // saving when a migration marks the identity in-process. The issuer + // re-checks after its save and takes the token back. + it('a mint whose identity went in-process while it was saving takes its token back and refuses', async () => { + const User = require('../../../models/User'); + const AgentCredential = require('../../../models/AgentCredential'); + if (!User.updateMany) User.updateMany = jest.fn(); + User.updateMany.mockResolvedValue({ modifiedCount: 1 }); + Pod.findById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ _id: 'pod-1', createdBy: 'user-1', members: ['user-1'] }), + }); + const installation = { + agentName: 'maya-agent', podId: 'pod-1', instanceId: 'default', status: 'active', installedBy: 'user-1', + config: { runtime: { runtimeType: 'hosted' } }, runtimeTokens: [], save: jest.fn().mockResolvedValue(true), + }; + const agentUser = { agentRuntimeTokens: [], botMetadata: { agentName: 'maya-agent' }, save: jest.fn().mockResolvedValue(true) }; + AgentInstallation.findOne.mockResolvedValue(installation); + // Not in-process at the check; in-process by the time the save landed. + AgentInstallation.exists.mockResolvedValueOnce(null).mockResolvedValueOnce({ _id: 'inst-1' }); + AgentIdentityService.getOrCreateAgentUser.mockResolvedValue(agentUser); + AgentIdentityService.ensureAgentInPod.mockResolvedValue(true); + + const res = await request(app) + .post('/api/registry/pods/pod-1/agents/maya-agent/runtime-tokens') + .send({ label: 'racing a migration' }); + + expect(res.status).toBe(409); + expect(res.body.code).toBe('in_process_agent_no_token'); + expect(res.body.token).toBeUndefined(); + const savedHash = agentUser.agentRuntimeTokens[0].tokenHash; + expect(AgentCredential.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ tokenHash: { $in: [savedHash] } }), expect.anything(), + ); + expect(User.updateMany).toHaveBeenCalledWith( + { 'agentRuntimeTokens.tokenHash': savedHash }, { $pull: { agentRuntimeTokens: { tokenHash: savedHash } } }, + ); + }); + it('matches every in-process shape in any spelling, and nothing else', async () => { await mintFor('scout', { runtime: { runtimeType: 'native' } }); const [query] = AgentInstallation.exists.mock.calls[0]; diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index 1f0a8fe720..34c749080f 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -1,8 +1,9 @@ /** - * TASK-265: moving hosted installs from the Durable Object onto the native - * engine. Real installation, event and user models on an in-memory Mongo; the - * worker calls and the token revoke are mocked. Each arm below is one of the - * findings in Rhea's and sprint-review's HOLDs on #2236. + * TASK-265: the hosted-engine migration, forward-only and resumable. Real + * installation, event and user models on an in-memory Mongo; the worker + * calls and the token revoke are mocked. The arms are Rhea's and + * sprint-review's findings across three reviews of #2236: each step can fail, + * and the next run must finish the identity from where it stopped. */ const { MongoMemoryServer } = require('mongodb-memory-server'); const mongoose = require('mongoose'); @@ -44,7 +45,7 @@ const install = (agentName, podId, hosted) => AgentInstallation.create({ config: { runtime: { runtimeType: 'hosted' }, ...(hosted ? { hosted } : {}) }, }); const hostedOf = async (id) => (await AgentInstallation.findById(id).lean()).config.hosted; -const engines = async (ids) => Promise.all(ids.map(async (id) => (await hostedOf(id))?.engine)); +const field = async (ids, pick) => Promise.all(ids.map(async (id) => pick((await hostedOf(id)) || {}))); const entryFor = (report, name) => report.identities.find((row) => row.agentName === name); let podA; let podB; let maya; let mayaB; let idle; let born; @@ -74,82 +75,102 @@ test('a dry run reports per identity and changes nothing', async () => { const report = await migrate({ apply: false, now: NOW }); expect(report.dryRun).toBe(true); - expect(report.identities.map((row) => [row.agentName, row.installs, row.provisioned]).sort()) - .toEqual([['idle-agent', 1, false], ['maya-agent', 2, true]]); + expect(report.identities.map((row) => [row.agentName, row.installs, row.doProvisioned, row.from]).sort()) + .toEqual([['idle-agent', 1, false, 'new'], ['maya-agent', 2, true, 'new']]); expect(mockDeprovision).not.toHaveBeenCalled(); - expect(mockRevoke).not.toHaveBeenCalled(); - expect(await engines([maya._id, mayaB._id, idle._id])).toEqual([undefined, undefined, undefined]); + expect(await field([maya._id, mayaB._id], (h) => h.engine)).toEqual([undefined, undefined]); }); -test('rows are native BEFORE the revoke, so no token can be minted in between (Rhea 1)', async () => { +test('a clean run: marked native before the revoke, stopped, revoked, done', async () => { let enginesAtRevoke; mockRevoke.mockImplementation(async () => { - enginesAtRevoke = await engines([maya._id, mayaB._id]); + enginesAtRevoke = await field([maya._id, mayaB._id], (h) => h.engine); return ['hash-1']; }); const report = await migrate({ apply: true, now: NOW }); expect(enginesAtRevoke).toEqual(['native', 'native']); - expect(entryFor(report, 'maya-agent')).toMatchObject({ durableObjectStopped: true, tokensRevoked: 1 }); + expect(entryFor(report, 'maya-agent')).toMatchObject({ reached: 'done', durableObjectStopped: true, tokensRevoked: 1 }); expect(await hostedOf(maya._id)).toEqual(expect.objectContaining({ engine: 'native', provisionedAt: NOW, about: 'Keeps the research log.', migratedFrom: 'durable-object', + migration: { state: 'done', doProvisioned: true }, })); + // A second run has nothing left to do for a done identity. + jest.clearAllMocks(); + const again = await migrate({ apply: true, now: NOW }); + expect(entryFor(again, 'maya-agent')).toBeUndefined(); + expect(mockRevoke).not.toHaveBeenCalled(); }); -test('a row that fails to save restores the others; the worker and the tokens are untouched (Rhea 2)', async () => { - // Fail exactly the second row's native mark, after the first row of the - // same identity is already native, so the restore has real work to do. +test('marking interrupted part-way: the next run marks the rest and finishes', async () => { const original = AgentInstallation.updateOne.bind(AgentInstallation); - jest.spyOn(AgentInstallation, 'updateOne').mockImplementation((filter, update, ...rest) => { - const marking = update?.$set?.['config.hosted']?.engine === 'native'; - if (marking && String(filter?._id) === String(mayaB._id)) return Promise.reject(new Error('write failed')); + const spy = jest.spyOn(AgentInstallation, 'updateOne').mockImplementation((filter, update, ...rest) => { + if (String(filter?._id) === String(mayaB._id) && update?.$set?.['config.hosted']) return Promise.reject(new Error('write failed')); return original(filter, update, ...rest); }); - - const report = await migrate({ apply: true, now: NOW }); - - expect(entryFor(report, 'maya-agent').error).toMatch(/restored, nothing changed/); - expect(await engines([maya._id, mayaB._id])).toEqual([undefined, undefined]); - expect((await hostedOf(maya._id)).provisionedAt).toEqual(new Date('2026-10-01')); + const first = await migrate({ apply: true, now: NOW }); + expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: 'mark', error: expect.stringMatching(/run again/) }); expect(mockDeprovision).not.toHaveBeenCalledWith('maya-agent', 'default'); - expect(mockRevoke).not.toHaveBeenCalled(); + + spy.mockRestore(); + const second = await migrate({ apply: true, now: NOW }); + expect(entryFor(second, 'maya-agent')).toMatchObject({ reached: 'done', durableObjectStopped: true }); + expect(await field([maya._id, mayaB._id], (h) => h.migration?.state)).toEqual(['done', 'done']); }); -test('a Durable Object that will not stop restores every row (Rhea 2)', async () => { +test('a stop that failed is retried on the next run, not assumed (Rhea: marker is not shutdown)', async () => { mockDeprovision.mockRejectedValueOnce(new Error('worker down')); - - const report = await migrate({ apply: true, now: NOW }); - - expect(entryFor(report, 'maya-agent').error).toMatch(/did not stop .*restored/); - expect(await engines([maya._id, mayaB._id])).toEqual([undefined, undefined]); + const first = await migrate({ apply: true, now: NOW }); + expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: 'stop', error: expect.stringMatching(/run again to stop/) }); + expect(await field([maya._id, mayaB._id], (h) => h.migration?.state)).toEqual(['marked', 'marked']); expect(mockRevoke).not.toHaveBeenCalled(); + + const second = await migrate({ apply: true, now: NOW }); + expect(entryFor(second, 'maya-agent')).toMatchObject({ from: 'marked', reached: 'done', durableObjectStopped: true }); + expect(mockDeprovision).toHaveBeenCalledTimes(2); }); -test('a failed revoke is reported, and the next run finishes it (Rhea 2)', async () => { +test('a failed revoke resumes at the revoke, without stopping the worker twice', async () => { mockRevoke.mockRejectedValueOnce(new Error('db down')); const first = await migrate({ apply: true, now: NOW }); - expect(entryFor(first, 'maya-agent').error).toMatch(/run again to finish/); - expect(await engines([maya._id, mayaB._id])).toEqual(['native', 'native']); + expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: 'revoke' }); + + const second = await migrate({ apply: true, now: NOW }); + expect(entryFor(second, 'maya-agent')).toMatchObject({ from: 'stopped', reached: 'done', tokensRevoked: 1 }); + expect(mockDeprovision).toHaveBeenCalledTimes(1); +}); + +test('a failed event write is resumed: the next run handles the events and finishes (Rhea 4)', async () => { + const [event] = await AgentEvent.create([{ + agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', + createdAt: new Date(NOW.getTime() - 60 * 1000), + }]); + const original = AgentEvent.updateOne.bind(AgentEvent); + const spy = jest.spyOn(AgentEvent, 'updateOne').mockRejectedValueOnce(new Error('event write failed')); + const first = await migrate({ apply: true, now: NOW }); + expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: 'events' }); + expect((await AgentEvent.findById(event._id).lean()).status).toBe('pending'); + spy.mockImplementation((...args) => original(...args)); const second = await migrate({ apply: true, now: NOW }); - expect(entryFor(second, 'maya-agent')).toMatchObject({ alreadyMigrated: true, tokensRevoked: 1 }); - expect(mockDeprovision).toHaveBeenCalledTimes(1); // not stopped twice + expect(entryFor(second, 'maya-agent')).toMatchObject({ reached: 'done', pendingToSweep: 1 }); + expect((await AgentEvent.findById(event._id).lean()).status).toBe('delivered'); }); -test('a provisioned identity is refused when the worker is not configured (Rhea 3)', async () => { +test('a provisioned identity is refused when the worker is not configured; a stopped one still moves', async () => { mockConfigured = false; const report = await migrate({ apply: true, now: NOW }); - expect(entryFor(report, 'maya-agent').error).toMatch(/cannot be stopped; refused/); - expect(await engines([maya._id, mayaB._id])).toEqual([undefined, undefined]); - expect(mockRevoke).not.toHaveBeenCalledWith(expect.objectContaining({ agentName: 'maya-agent' })); - // A stopped identity needs no worker and still moves. - expect(await engines([idle._id])).toEqual(['native']); + expect(entryFor(report, 'maya-agent')).toMatchObject({ reached: 'refused' }); + expect(await field([maya._id, mayaB._id], (h) => h.engine)).toEqual([undefined, undefined]); + expect(entryFor(report, 'idle-agent')).toMatchObject({ reached: 'done' }); + expect(await hostedOf(idle._id)).toEqual(expect.objectContaining({ engine: 'native', provisionedAt: null })); + expect(mockDeprovision).not.toHaveBeenCalled(); }); -test('pending events: recent on a running install go to the sweep; on a stopped one, or stale, they close (Rhea 4, sprint-review)', async () => { +test('pending events: recent on a running install go to the sweep; on a stopped one, or stale, they close', async () => { const at = (ms) => new Date(NOW.getTime() - ms); const [toSweep, stale, onStopped] = await AgentEvent.create([ { agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', createdAt: at(60 * 1000) }, @@ -157,23 +178,18 @@ test('pending events: recent on a running install go to the sweep; on a stopped { agentName: 'idle-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', createdAt: at(60 * 1000) }, ]); - const dry = await migrate({ apply: false, now: NOW }); - expect(entryFor(dry, 'maya-agent')).toMatchObject({ pendingToSweep: 1, pendingClosed: 1 }); - expect(entryFor(dry, 'idle-agent')).toMatchObject({ pendingToSweep: 0, pendingClosed: 1 }); - await migrate({ apply: true, now: NOW }); - const swept = await AgentEvent.findById(toSweep._id).lean(); - expect(swept).toMatchObject({ status: 'delivered', deliveryNonce: null }); - expect((await AgentEvent.findById(stale._id).lean())).toMatchObject({ status: 'failed', error: expect.stringMatching(/stale/) }); - expect((await AgentEvent.findById(onStopped._id).lean())).toMatchObject({ status: 'failed', error: expect.stringMatching(/stopped/) }); + expect(await AgentEvent.findById(toSweep._id).lean()).toMatchObject({ status: 'delivered', deliveryNonce: null }); + expect(await AgentEvent.findById(stale._id).lean()).toMatchObject({ status: 'failed', error: expect.stringMatching(/stale/) }); + expect(await AgentEvent.findById(onStopped._id).lean()).toMatchObject({ status: 'failed', error: expect.stringMatching(/stopped/) }); }); -test('a stopped install stays stopped with no worker call; one born native is not touched or reported', async () => { +test('an install born native is never touched or reported', async () => { const report = await migrate({ apply: true, now: NOW }); - expect(await hostedOf(idle._id)).toEqual(expect.objectContaining({ engine: 'native', provisionedAt: null })); - expect(mockDeprovision).not.toHaveBeenCalledWith('idle-agent', 'default'); - expect((await hostedOf(born._id)).migratedFrom).toBeUndefined(); expect(entryFor(report, 'native-agent')).toBeUndefined(); + const hosted = await hostedOf(born._id); + expect(hosted.migratedFrom).toBeUndefined(); + expect(hosted.migration).toBeUndefined(); }); diff --git a/backend/routes/registry/tokens.ts b/backend/routes/registry/tokens.ts index 3e7f640c76..f0c5f28c02 100644 --- a/backend/routes/registry/tokens.ts +++ b/backend/routes/registry/tokens.ts @@ -312,6 +312,17 @@ const issueRuntimeTokenForAgent = async (agentUser: any, label: any, installatio await installation.save(); } + // The check above runs before the token exists. A migration can mark the + // identity in-process while this mint is still saving, and its revoke may + // run before the save lands (Rhea on #2236). Re-check now the token is + // stored: if the identity went in-process meanwhile, take the token back + // and refuse. A save that landed before the migration's revoke is caught + // by that revoke, so no ordering leaves a live token. + if (await runsInProcess(agentUser, installation)) { + await revokeRuntimeTokenEverywhere(tokenRecord.tokenHash); + throw new InProcessAgentTokenRefused(); + } + return { token: rawToken, label: label || 'Runtime token', @@ -334,6 +345,10 @@ const issueRuntimeTokenForInstallation = async (installation: any, label: any) = createdAt: new Date(), }); await installation.save(); + if (await runsInProcess(null, installation)) { + await revokeRuntimeTokenEverywhere(hash(rawToken)); + throw new InProcessAgentTokenRefused(); + } return { token: rawToken, label: label || 'Runtime token' }; }; diff --git a/backend/services/hostedEngineMigration.ts b/backend/services/hostedEngineMigration.ts index e4f98647c0..a652890201 100644 --- a/backend/services/hostedEngineMigration.ts +++ b/backend/services/hostedEngineMigration.ts @@ -2,26 +2,33 @@ * TASK-265: move every active hosted install from the Durable Object onto the * native engine (ADR-023 amendment A1). Dry run by default. * - * Per agent identity (agentName, instanceId), because one agent can sit in - * several pods, and in this order (Rhea and sprint-review on #2236): - * 0. Refuse the identity, changing nothing, when its Durable Object is - * provisioned but the hosted worker is not configured: it could not be - * stopped, and would keep polling as the agent. - * 1. Mark every row native FIRST. From that moment the token issuer refuses - * to mint for the identity (routes/registry/tokens.ts), so no token can - * be issued between the revoke and the mark. A failed row restores the - * rows already marked. - * 2. Stop the Durable Object. On failure every row is restored, and the - * identity is exactly as it was. - * 3. Revoke its runtime tokens. On failure the rows stay native (minting is - * refused) and the Durable Object is stopped; the report says to run - * again, and every run revokes for already-migrated identities. - * 4. Pending events, which nothing polls once the Durable Object stops: a - * recent one on a provisioned install is handed to the in-process sweep - * (marked delivered; agentEventService re-runs it), so this script never - * runs a turn itself. A recent one on a stopped install, and any stale - * one, is closed with a reason. - * One identity failing is reported and does not stop the others. + * Forward-only and resumable, with no rollback (Rhea's third review of + * #2236). Each row records how far its identity got, in + * `config.hosted.migration`: + * + * { state: 'marked' | 'stopped' | 'done', doProvisioned: boolean } + * + * Every run resumes each unfinished identity from its recorded state, per + * agent identity (agentName, instanceId), since one agent can sit in several + * pods: + * 0. Refuse, changing nothing, when a Durable Object that still needs + * stopping is provisioned but the hosted worker is not configured. + * 1. mark: every row native, state 'marked'. From here the token issuer + * refuses to mint for the identity, and re-checks after saving, + * so a mint already in flight takes its own token back + * (routes/registry/tokens.ts). + * 2. stop: the Durable Object, if one existed; state 'stopped'. Recorded + * apart from the mark, so a run interrupted before the stop + * stops it on the next run instead of assuming it happened. + * 3. revoke: the identity's runtime tokens. + * 4. events: pending events, which nothing polls once the Durable Object + * stops. A recent one on a provisioned install is handed to the + * in-process sweep (marked delivered; #2226 re-runs it), so this + * script never runs a turn. Others are closed with a reason. + * 5. done. + * Every step is idempotent. A step that fails stops that identity there; the + * report says which, and the next run resumes it. Nothing is reported done + * that is not. */ import { HOSTED_ENGINE_NATIVE, hostedEngineOf, readHostedConfig } from './hostedEngine'; @@ -40,17 +47,20 @@ const STALE_PENDING_MS = 10 * 60 * 1000; const ABOUT_LIMIT = 160; const MIGRATED_FROM = 'durable-object'; +type MigrationState = 'marked' | 'stopped' | 'done'; + export interface IdentityReport { agentName: string; instanceId: string; installs: number; - provisioned: boolean; - alreadyMigrated?: boolean; + /** Where this run found the identity: 'new' or the recorded state. */ + from: 'new' | MigrationState; + /** Where it ended: 'done', or the step that failed. */ + reached: 'done' | 'refused' | 'mark' | 'stop' | 'revoke' | 'events' | 'finish' | 'dry-run'; + doProvisioned: boolean; durableObjectStopped?: boolean; tokensRevoked?: number; - /** Recent events on a provisioned install, handed to the in-process sweep. */ pendingToSweep: number; - /** Recent events on a stopped install, and stale ones, closed with a reason. */ pendingClosed: number; error?: string; } @@ -65,9 +75,14 @@ const aboutFrom = (description: unknown): string | undefined => { return text || undefined; }; -const setHosted = (id: unknown, hosted: Record | null) => AgentInstallation.updateOne( - { _id: id }, - hosted && Object.keys(hosted).length ? { $set: { 'config.hosted': hosted } } : { $unset: { 'config.hosted': '' } }, +const migrationOf = (row: any): { state?: MigrationState; doProvisioned?: boolean } => { + const value = readHostedConfig(row).migration; + return value && typeof value === 'object' ? value as { state?: MigrationState; doProvisioned?: boolean } : {}; +}; + +const setState = (ids: unknown[], state: MigrationState) => AgentInstallation.updateMany( + { _id: { $in: ids } }, + { $set: { 'config.hosted.migration.state': state } }, ); const findAgentUser = (agentName: string, instanceId: string) => User.findOne({ @@ -75,15 +90,6 @@ const findAgentUser = (agentName: string, instanceId: string) => User.findOne({ isBot: true, }); -const revokeFor = async (agentName: string, instanceId: string, installation: unknown): Promise => { - const agentUser = await findAgentUser(agentName, instanceId); - if (!agentUser) return 0; - // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports - const { revokeRuntimeTokensForAgent } = require('../routes/registry/tokens'); - const revoked = await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation }); - return revoked.length; -}; - export const migrateHostedInstallsToNative = async ( { apply = false, now = new Date() }: { apply?: boolean; now?: Date } = {}, ): Promise => { @@ -98,123 +104,138 @@ export const migrateHostedInstallsToNative = async ( for (const rows of byIdentity.values()) { const agentName = String(rows[0].agentName); const instanceId = String(rows[0].instanceId || 'default'); - const todo = rows.filter((row) => hostedEngineOf(row) !== HOSTED_ENGINE_NATIVE); const isProvisioned = (row: any) => Boolean(readHostedConfig(row).provisionedAt); - - if (!todo.length) { - // Already moved by an earlier run. Finish it: a migrated identity holds - // no runtime token, even if that run's revoke failed. - if (!rows.some((row) => readHostedConfig(row).migratedFrom === MIGRATED_FROM)) continue; - const entry: IdentityReport = { - agentName, instanceId, installs: rows.length, provisioned: rows.some(isProvisioned), - alreadyMigrated: true, pendingToSweep: 0, pendingClosed: 0, - }; - report.identities.push(entry); - if (apply) { - try { - // eslint-disable-next-line no-await-in-loop - entry.tokensRevoked = await revokeFor(agentName, instanceId, rows[0]); - } catch (error) { - entry.error = `tokens not revoked (${(error as Error).message}); run again`; - } - } - continue; - } - - const provisioned = todo.some(isProvisioned); + const unmarked = rows.filter((row) => hostedEngineOf(row) !== HOSTED_ENGINE_NATIVE); + const migrated = rows.filter((row) => readHostedConfig(row).migratedFrom === MIGRATED_FROM); + const unfinished = migrated.filter((row) => migrationOf(row).state !== 'done'); + // Done, or born native: nothing to do. + if (!unmarked.length && !unfinished.length) continue; + + const states = unfinished.map((row) => migrationOf(row).state || 'marked'); + const from: IdentityReport['from'] = unmarked.length ? 'new' : (states.includes('marked') ? 'marked' : 'stopped'); + const doProvisioned = unmarked.some(isProvisioned) || migrated.some((row) => migrationOf(row).doProvisioned === true); + // Stopped only when every migrated row says so and nothing is left to mark. + const stopped = !unmarked.length && migrated.every((row) => ['stopped', 'done'].includes(migrationOf(row).state || '')); // eslint-disable-next-line no-await-in-loop const pending = await AgentEvent.find({ agentName, instanceId, status: 'pending' }).lean(); - const podIsRunning = (podId: unknown) => todo.some((row) => String(row.podId) === String(podId) && isProvisioned(row)); + const runningPods = new Set(rows.filter(isProvisioned).map((row) => String(row.podId))); const recent = (event: any) => now.getTime() - new Date(event.createdAt).getTime() < STALE_PENDING_MS; - const toSweep = pending.filter((event: any) => recent(event) && podIsRunning(event.podId)); + const toSweep = pending.filter((event: any) => recent(event) && runningPods.has(String(event.podId))); const toClose = pending.filter((event: any) => !toSweep.includes(event)); const entry: IdentityReport = { agentName, instanceId, - installs: todo.length, - provisioned, + installs: rows.length, + from, + reached: 'dry-run', + doProvisioned, pendingToSweep: toSweep.length, pendingClosed: toClose.length, }; report.identities.push(entry); - if (provisioned && !hostedRuntime.isConfigured()) { + if (doProvisioned && !stopped && !hostedRuntime.isConfigured()) { + entry.reached = 'refused'; entry.error = 'its Durable Object is provisioned but the hosted worker is not configured, so it cannot be ' - + 'stopped; refused, nothing changed'; + + 'stopped; refused'; continue; } if (!apply) continue; + // Only rows this migration owns: never a row of the same agent born native. + const ids = [...unmarked, ...migrated].map((row) => row._id); - // 1. Mark every row native first, so minting is refused from here on. - // eslint-disable-next-line no-await-in-loop - const agentUser = await findAgentUser(agentName, instanceId); - const about = aboutFrom(agentUser?.botMetadata?.description); - const previous = new Map(todo.map((row) => [String(row._id), readHostedConfig(row)])); - const restore = async (ids: string[]) => { - for (const id of ids) { - // eslint-disable-next-line no-await-in-loop - await setHosted(id, previous.get(id) || null); - } - }; - const marked: string[] = []; + // 1. Mark. From here the issuer refuses, and takes back an in-flight mint. + entry.reached = 'mark'; try { - for (const row of todo) { + // eslint-disable-next-line no-await-in-loop + const agentUser = unmarked.length ? await findAgentUser(agentName, instanceId) : null; + const about = aboutFrom(agentUser?.botMetadata?.description); + for (const row of unmarked) { // eslint-disable-next-line no-await-in-loop - await setHosted(row._id, { - engine: HOSTED_ENGINE_NATIVE, - provisionedAt: isProvisioned(row) ? now : null, - ...(about ? { about } : {}), - migratedFrom: MIGRATED_FROM, - migratedAt: now, + await AgentInstallation.updateOne({ _id: row._id }, { + $set: { + 'config.hosted': { + engine: HOSTED_ENGINE_NATIVE, + provisionedAt: isProvisioned(row) ? now : null, + ...(about ? { about } : {}), + migratedFrom: MIGRATED_FROM, + migratedAt: now, + migration: { state: 'marked', doProvisioned }, + }, + }, }); - marked.push(String(row._id)); } } catch (error) { - // eslint-disable-next-line no-await-in-loop - await restore(marked).catch(() => undefined); - entry.error = `marking failed (${(error as Error).message}); restored, nothing changed`; + entry.error = `marking stopped part-way (${(error as Error).message}); run again to continue`; continue; } - // 2. Stop the Durable Object; on failure, put every row back. - if (provisioned) { - try { + // 2. Stop the Durable Object, recorded separately from the mark. + entry.reached = 'stop'; + try { + if (doProvisioned && !stopped) { // eslint-disable-next-line no-await-in-loop await hostedRuntime.deprovisionAgent(agentName, instanceId); entry.durableObjectStopped = true; - } catch (error) { - // eslint-disable-next-line no-await-in-loop - await restore(marked).catch(() => undefined); - entry.error = `the Durable Object did not stop (${(error as Error).message}); restored, nothing changed`; - continue; } + // eslint-disable-next-line no-await-in-loop + await setState(ids, 'stopped'); + } catch (error) { + entry.error = `the Durable Object was not stopped (${(error as Error).message}); the rows are native, ` + + 'so no token can be minted; run again to stop it'; + continue; } - // 3. Revoke. The rows are native (no new token can be minted) and the - // Durable Object is stopped, so a failure here is finished by a re-run. + // 3. Revoke. + entry.reached = 'revoke'; try { // eslint-disable-next-line no-await-in-loop - entry.tokensRevoked = await revokeFor(agentName, instanceId, todo[0]); + const agentUser = await findAgentUser(agentName, instanceId); + if (agentUser) { + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { revokeRuntimeTokensForAgent } = require('../routes/registry/tokens'); + // eslint-disable-next-line no-await-in-loop + const revoked = await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation: rows[0] }); + entry.tokensRevoked = revoked.length; + } else { + entry.tokensRevoked = 0; + } } catch (error) { - entry.error = `tokens not revoked (${(error as Error).message}); the rows are native and the Durable ` - + 'Object is stopped; run again to finish'; + entry.error = `tokens not revoked (${(error as Error).message}); run again to finish`; + continue; } - // 4. Pending events: hand recent ones on a running install to the sweep, - // close the rest with a reason. This script never runs a turn itself. - for (const event of toSweep) { - // eslint-disable-next-line no-await-in-loop - await AgentEvent.updateOne( - { _id: event._id, status: 'pending' }, - { $set: { status: 'delivered', deliveredAt: now, deliveryNonce: null }, $inc: { attempts: 1 } }, - ); + // 4. Pending events. Only still-pending events are touched, so a re-run + // picks up whatever a failed run left. + entry.reached = 'events'; + try { + for (const event of toSweep) { + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne( + { _id: event._id, status: 'pending' }, + { $set: { status: 'delivered', deliveredAt: now, deliveryNonce: null }, $inc: { attempts: 1 } }, + ); + } + for (const event of toClose) { + const reason = recent(event) + ? 'hosted engine migrated while this agent was stopped; it was not run' + : 'hosted engine migrated before this event ran (stale)'; + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne({ _id: event._id, status: 'pending' }, { $set: { status: 'failed', error: reason } }); + } + } catch (error) { + entry.error = `pending events not all handled (${(error as Error).message}); run again to finish`; + continue; } - for (const event of toClose) { - const reason = recent(event) - ? 'hosted engine migrated while this agent was stopped; it was not run' - : 'hosted engine migrated before this event ran (stale)'; + + // 5. Done. + entry.reached = 'finish'; + try { // eslint-disable-next-line no-await-in-loop - await AgentEvent.updateOne({ _id: event._id, status: 'pending' }, { $set: { status: 'failed', error: reason } }); + await setState(ids, 'done'); + entry.reached = 'done'; + } catch (error) { + entry.error = `not recorded as done (${(error as Error).message}); run again`; } } return report; From f79d7a6fb09dbf661b7368110310e08c67f27027 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:05:26 -0700 Subject: [PATCH 4/8] test(hosted): resume from a persisted marked/stopped state counts and handles pending events Sprint-review on #2236 (5536956805): an interruption after marking (deploy, pod restart, kill) leaves rows in a persisted state with no failure to inject. The arm builds that state directly, in both 'marked' and 'stopped': the dry run counts the pending event from the same query, and apply resumes, hands the event to the sweep and ends done. Reintroducing the old 'resume skips events' shape fails both arms. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../services/hostedEngineMigration.test.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index 34c749080f..ee5e53d747 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -158,6 +158,35 @@ test('a failed event write is resumed: the next run handles the events and finis expect((await AgentEvent.findById(event._id).lean()).status).toBe('delivered'); }); +// Sprint-review on #2236: an interruption after marking (deploy, pod restart, +// kill) leaves exactly this persisted state, with no failure to inject. The +// dry run must count its events from the same query, and apply must finish it. +test.each([ + ['marked', 1], + ['stopped', 0], +])('an identity persisted in state "%s" with a pending event is counted and resumed to done', async (state, stopCalls) => { + await AgentInstallation.deleteMany({}); + const persisted = (podId) => install('resume-agent', podId, { + engine: 'native', provisionedAt: NOW, migratedFrom: 'durable-object', migratedAt: NOW, + migration: { state, doProvisioned: true }, + }); + const [rowA, rowB] = [await persisted(podA), await persisted(podB)]; + const [event] = await AgentEvent.create([{ + agentName: 'resume-agent', instanceId: 'default', podId: podA, type: 'chat.mention', payload: {}, status: 'pending', + createdAt: new Date(NOW.getTime() - 60 * 1000), + }]); + + const dry = await migrate({ apply: false, now: NOW }); + expect(entryFor(dry, 'resume-agent')).toMatchObject({ from: state, pendingToSweep: 1, pendingClosed: 0 }); + expect((await AgentEvent.findById(event._id).lean()).status).toBe('pending'); + + const applied = await migrate({ apply: true, now: NOW }); + expect(entryFor(applied, 'resume-agent')).toMatchObject({ from: state, reached: 'done', pendingToSweep: 1 }); + expect(mockDeprovision).toHaveBeenCalledTimes(stopCalls); + expect((await AgentEvent.findById(event._id).lean()).status).toBe('delivered'); + expect(await field([rowA._id, rowB._id], (h) => h.migration?.state)).toEqual(['done', 'done']); +}); + test('a provisioned identity is refused when the worker is not configured; a stopped one still moves', async () => { mockConfigured = false; From 35b487c7e85833aa4a658dabd6205a9fcfdc03b9 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:42:36 -0700 Subject: [PATCH 5/8] fix(hosted): an unfinished migration is native for nothing, and a provision in flight cannot undo one Rhea's HOLD on #2236 at a600bf3d (two boundaries): 1. A row the migration marked native counted as native for execution, grant eligibility and joins while its Durable Object could still be running (a failed stop) or its old token still live (a failed revoke). The engine in effect (hostedEngineInEffect) is now native only once the migration records the identity done; hostedRunsNative, runsInNarrowedProcess and every join site use it. The token issuer still refuses on the stored engine, so an unfinished row can neither mint nor count as in-process. A source scan pins which files may read the stored engine. 2. A Durable Object provision in flight across the migration saved the record it read at the start over the migration's, and the next provision minted a fresh token. The DO branch's record write is now conditional on the row still being on the Durable Object; when the migration got there first, the route stops the worker it started, takes back its token and answers 409. Deprovision is fenced the same way, and the native branches write only their own fields, so neither can revert a migration's recorded state. The migration now stops every identity whenever the worker is configured (the worker's stop is idempotent), since a provision can land between the migration's read and its mark. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- backend/__tests__/unit/routes/hosted.test.js | 160 ++++++++++++++++-- .../services/hostedEngineMigration.test.js | 77 ++++++++- .../outsiderGrantAgentJoinPolicy.test.js | 52 ++++++ .../outsiderGrantPodEligibility.test.js | 20 +++ backend/routes/agentsRuntime.ts | 6 +- backend/routes/hosted.ts | 62 +++++-- backend/routes/registry/agent-config.ts | 4 +- backend/routes/registry/provision.ts | 4 +- backend/routes/registry/reprovision.ts | 4 +- backend/services/agentAutoJoinService.ts | 4 +- backend/services/hostedEngine.ts | 36 +++- backend/services/hostedEngineMigration.ts | 32 ++-- 12 files changed, 408 insertions(+), 53 deletions(-) diff --git a/backend/__tests__/unit/routes/hosted.test.js b/backend/__tests__/unit/routes/hosted.test.js index f5cab6bf4d..a09d7ccc57 100644 --- a/backend/__tests__/unit/routes/hosted.test.js +++ b/backend/__tests__/unit/routes/hosted.test.js @@ -29,8 +29,29 @@ jest.mock('../../../middleware/auth', () => (req, _res, next) => { req.user = { id: 'owner-1' }; next(); }); +// The route writes config.hosted with updateOne (fenced on the stored engine +// for the Durable Object branch), so the mock applies it to the in-memory +// installation the test holds, honouring the engine condition. +const mockStore = new Map(); +const mockUpdateOne = jest.fn(async (filter, update) => { + const installation = mockStore.get(String(filter._id)); + if (!installation) return { matchedCount: 0, modifiedCount: 0 }; + const engineCondition = filter['config.hosted.engine']; + const engine = String((installation.config.get('hosted') || {}).engine || ''); + if (engineCondition && engineCondition.$not.test(engine)) return { matchedCount: 0, modifiedCount: 0 }; + Object.entries(update.$set || {}).forEach(([path, value]) => { + const [root, key, field] = path.split('.'); + if (root !== 'config') throw new Error(`unexpected path ${path}`); + installation.config.set(key, field ? { ...(installation.config.get(key) || {}), [field]: value } : value); + }); + return { matchedCount: 1, modifiedCount: 1 }; +}); jest.mock('../../../models/AgentRegistry', () => ({ - AgentInstallation: { findOne: (...args) => mockFindOne(...args), find: (...args) => mockFind(...args) }, + AgentInstallation: { + findOne: (...args) => mockFindOne(...args), + find: (...args) => mockFind(...args), + updateOne: (...args) => mockUpdateOne(...args), + }, })); jest.mock('../../../models/User', () => ({ findOne: (...args) => mockUserFindOne(...args) })); const mockCredentialUpdateMany = jest.fn(); @@ -56,14 +77,27 @@ const app = express(); app.use(express.json()); app.use('/api/hosted', hostedRouter); -const makeInstallation = (overrides = {}) => ({ - agentName: 'scout', - instanceId: 'default', - podId: 'pod-1', - config: new Map([['runtime', { runtimeType: 'hosted' }]]), - save: jest.fn().mockResolvedValue(undefined), - ...overrides, -}); +// The route's limiter (30 a minute) is module-level and shared by this file; +// the migration arms use their own key so they do not spend the others' budget. +// The auth middleware is mocked, so the header is read only by the limiter. +let armSeq = 0; +const asMigrationArm = (req) => { armSeq += 1; return req.set('Authorization', `Bearer migration-arm-${armSeq}`); }; + +let installationSeq = 0; +const makeInstallation = (overrides = {}) => { + installationSeq += 1; + const installation = { + _id: `inst-${installationSeq}`, + agentName: 'scout', + instanceId: 'default', + podId: 'pod-1', + config: new Map([['runtime', { runtimeType: 'hosted' }]]), + save: jest.fn().mockResolvedValue(undefined), + ...overrides, + }; + mockStore.set(installation._id, installation); + return installation; +}; describe('/api/hosted', () => { beforeEach(() => { @@ -251,7 +285,6 @@ describe('/api/hosted', () => { }); expect(mockHosted.provisionAgent).toHaveBeenCalledWith({ agentName: 'scout', instanceId: 'demo', runtimeToken: 'cm_agent_secret' }); expect(installation.config.get('hosted').provisionedAt).toBeInstanceOf(Date); - expect(installation.save).toHaveBeenCalled(); }); it('rotates: revokes the old credential rows, clears the stored hashes, mints fresh', async () => { @@ -285,7 +318,8 @@ describe('/api/hosted', () => { const res = await request(app).post('/api/hosted/provision').send({ agentName: 'scout' }); expect(res.status).toBe(502); expect(res.body.code).toBe('hosted_runtime_unreachable'); - expect(installation.save).not.toHaveBeenCalled(); + expect(mockUpdateOne).not.toHaveBeenCalled(); + expect(installation.config.get('hosted')).toBeUndefined(); }); it('deprovisions the owner\'s agent and keeps the installation', async () => { @@ -296,7 +330,79 @@ describe('/api/hosted', () => { expect(res.status).toBe(200); expect(res.body).toMatchObject({ deprovisioned: true }); expect(installation.config.get('hosted').provisionedAt).toBeNull(); - expect(installation.save).toHaveBeenCalled(); + }); + + // TASK-265, Rhea on #2236: a migration that marks the row native while the + // worker call is in flight must win. Before the fence, the route saved the + // Durable Object record it read at the start over the migration's, and the + // next provision minted a fresh token. + describe('against a migration landing mid-request', () => { + const MIGRATED = { + engine: 'native', + provisionedAt: new Date('2026-10-15T17:00:00Z'), + migratedFrom: 'durable-object', + migration: { state: 'done', doProvisioned: true }, + }; + + it('provision: the migrated record stands, the started worker is stopped, its token is taken back, 409', async () => { + const installation = makeInstallation(); + mockFindOne.mockResolvedValue(installation); + mockNativeAvailable.mockReturnValue(true); + mockHosted.provisionAgent.mockImplementation(async () => { + installation.config.set('hosted', { ...MIGRATED }); + return { provisioned: true }; + }); + mockHosted.deprovisionAgent.mockResolvedValue({ deprovisioned: true }); + + const res = await asMigrationArm(request(app).post('/api/hosted/provision')).send({ agentName: 'scout' }); + + expect(res.status).toBe(409); + expect(res.body.code).toBe('hosted_engine_migrated'); + expect(installation.config.get('hosted')).toEqual(MIGRATED); + expect(mockHosted.deprovisionAgent).toHaveBeenCalledWith('scout', 'default'); + // Once before minting, once to take back the token minted before the mark. + expect(mockRevokeRuntimeTokens).toHaveBeenCalledTimes(2); + expect(JSON.stringify(res.body)).not.toContain('cm_agent_secret'); + + // The next provision finds the native record and mints nothing. + const again = await asMigrationArm(request(app).post('/api/hosted/provision')).send({ agentName: 'scout' }); + expect(again.status).toBe(200); + expect(again.body.engine).toBe('native'); + expect(mockIssueToken).toHaveBeenCalledTimes(1); + expect(mockHosted.provisionAgent).toHaveBeenCalledTimes(1); + expect(installation.config.get('hosted')).toEqual(MIGRATED); + }); + + it('provision: a failed stop of the overtaken worker still takes the token back and reports 409', async () => { + const installation = makeInstallation(); + mockFindOne.mockResolvedValue(installation); + mockHosted.provisionAgent.mockImplementation(async () => { + installation.config.set('hosted', { ...MIGRATED }); + return { provisioned: true }; + }); + mockHosted.deprovisionAgent.mockRejectedValue(new Error('worker unreachable')); + jest.spyOn(console, 'warn').mockImplementation(() => {}); + + const res = await asMigrationArm(request(app).post('/api/hosted/provision')).send({ agentName: 'scout' }); + + expect(res.status).toBe(409); + expect(mockRevokeRuntimeTokens).toHaveBeenCalledTimes(2); + expect(installation.config.get('hosted')).toEqual(MIGRATED); + }); + + it('deprovision: the worker is stopped and the migrated record is kept', async () => { + const installation = makeInstallation(); + mockFindOne.mockResolvedValue(installation); + mockHosted.deprovisionAgent.mockImplementation(async () => { + installation.config.set('hosted', { ...MIGRATED }); + return { deprovisioned: true }; + }); + + const res = await asMigrationArm(request(app).post('/api/hosted/deprovision')).send({ agentName: 'scout' }); + + expect(res.status).toBe(200); + expect(installation.config.get('hosted')).toEqual(MIGRATED); + }); }); it('reports runtime status together with today\'s meter for the owner', async () => { @@ -393,4 +499,34 @@ describe('/api/hosted on the native engine', () => { const status = await request(app).get('/api/hosted/status?agentName=scout'); expect(status.body.runtime).toMatchObject({ engine: 'native', ready: false }); }); + + // Rhea on #2236: the native branches write only their own fields, so a + // migration that advances while the request is in flight is not reverted to + // the record read at the start. + it.each([ + ['provision', { provisionedAt: PROVISIONED }], + ['deprovision', { provisionedAt: null }], + ])('%s on a migrating row never writes back the migration record it read', async (route, expected) => { + const installation = nativeInstallation({ migratedFrom: 'durable-object', migration: { state: 'marked', doProvisioned: true } }); + // The route holds the copy it read; saving it persists that copy's record, + // as a hydrated document would. The migration finishes right after the read. + mockFindOne.mockImplementation(async () => { + const read = { + ...installation, + config: new Map(installation.config), + save: jest.fn(async () => { installation.config.set('hosted', read.config.get('hosted')); }), + }; + installation.config.set('hosted', { + ...installation.config.get('hosted'), migration: { state: 'done', doProvisioned: true }, + }); + return read; + }); + + const res = await asMigrationArm(request(app).post(`/api/hosted/${route}`)).send({ agentName: 'scout' }); + + expect(res.status).toBe(200); + const hosted = installation.config.get('hosted'); + expect(hosted.migration).toEqual({ state: 'done', doProvisioned: true }); + expect(hosted).toMatchObject({ engine: 'native', about: 'Keeps the research log.', ...expected }); + }); }); diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index ee5e53d747..266b54d28a 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -45,8 +45,11 @@ const install = (agentName, podId, hosted) => AgentInstallation.create({ config: { runtime: { runtimeType: 'hosted' }, ...(hosted ? { hosted } : {}) }, }); const hostedOf = async (id) => (await AgentInstallation.findById(id).lean()).config.hosted; +const readStoredEngine = (row) => require('../../../services/hostedEngine').hostedEngineOf(row); const field = async (ids, pick) => Promise.all(ids.map(async (id) => pick((await hostedOf(id)) || {}))); const entryFor = (report, name) => report.identities.find((row) => row.agentName === name); +const stopsFor = (name) => mockDeprovision.mock.calls.filter(([agentName]) => agentName === name).length; +const revokedFor = (name) => mockRevoke.mock.calls.filter(([args]) => args.agentName === name).length; let podA; let podB; let maya; let mayaB; let idle; let born; beforeEach(async () => { @@ -120,25 +123,89 @@ test('marking interrupted part-way: the next run marks the rest and finishes', a }); test('a stop that failed is retried on the next run, not assumed (Rhea: marker is not shutdown)', async () => { - mockDeprovision.mockRejectedValueOnce(new Error('worker down')); + let failed = false; + mockDeprovision.mockImplementation(async (name) => { + if (name === 'maya-agent' && !failed) { failed = true; throw new Error('worker down'); } + return true; + }); const first = await migrate({ apply: true, now: NOW }); expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: 'stop', error: expect.stringMatching(/run again to stop/) }); expect(await field([maya._id, mayaB._id], (h) => h.migration?.state)).toEqual(['marked', 'marked']); - expect(mockRevoke).not.toHaveBeenCalled(); + expect(revokedFor('maya-agent')).toBe(0); const second = await migrate({ apply: true, now: NOW }); expect(entryFor(second, 'maya-agent')).toMatchObject({ from: 'marked', reached: 'done', durableObjectStopped: true }); - expect(mockDeprovision).toHaveBeenCalledTimes(2); + expect(stopsFor('maya-agent')).toBe(2); }); test('a failed revoke resumes at the revoke, without stopping the worker twice', async () => { - mockRevoke.mockRejectedValueOnce(new Error('db down')); + let failed = false; + mockRevoke.mockImplementation(async ({ agentName }) => { + if (agentName === 'maya-agent' && !failed) { failed = true; throw new Error('db down'); } + return ['hash-1']; + }); const first = await migrate({ apply: true, now: NOW }); expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: 'revoke' }); const second = await migrate({ apply: true, now: NOW }); expect(entryFor(second, 'maya-agent')).toMatchObject({ from: 'stopped', reached: 'done', tokensRevoked: 1 }); - expect(mockDeprovision).toHaveBeenCalledTimes(1); + expect(stopsFor('maya-agent')).toBe(1); +}); + +// Rhea on #2236 (a600bf3d): a provision can complete between this run's read +// and its mark, so the stop cannot be decided from what the run read. With the +// worker configured every identity is stopped; the worker's stop is a no-op for +// an agent it is not running. +test('an identity this run saw unprovisioned is still stopped when the worker is configured', async () => { + const report = await migrate({ apply: true, now: NOW }); + expect(entryFor(report, 'idle-agent')).toMatchObject({ doProvisioned: false, reached: 'done', durableObjectStopped: true }); + expect(stopsFor('idle-agent')).toBe(1); + expect(stopsFor('native-agent')).toBe(0); +}); + +test('with no worker configured, an unprovisioned identity migrates without a stop and a provisioned one is refused', async () => { + mockConfigured = false; + const report = await migrate({ apply: true, now: NOW }); + expect(entryFor(report, 'idle-agent')).toMatchObject({ reached: 'done' }); + expect(entryFor(report, 'idle-agent').durableObjectStopped).toBeUndefined(); + expect(entryFor(report, 'maya-agent')).toMatchObject({ reached: 'refused' }); + expect(mockDeprovision).not.toHaveBeenCalled(); +}); + +// Rhea on #2236 (a600bf3d), item 1: until the identity is done, its rows are +// not native for execution, grant eligibility or joins, whichever step failed. +// Checked on the rows exactly as the migration leaves them in the store. +test.each([ + ['a failed stop', 'stop'], + ['a failed revoke', 'revoke'], +])('after %s the rows are native for nothing; once done, for everything', async (_label, failAt) => { + const { hostedRunsNative, runsInNarrowedProcess, hostedEngineInEffect } = require('../../../services/hostedEngine'); + // Fail maya's step once; the other identities run normally. + let failed = false; + const failMayaOnce = (fallback) => async (arg) => { + const name = typeof arg === 'string' ? arg : arg.agentName; + if (name === 'maya-agent' && !failed) { failed = true; throw new Error(`${failAt} failed`); } + return fallback; + }; + if (failAt === 'stop') mockDeprovision.mockImplementation(failMayaOnce(true)); + else mockRevoke.mockImplementation(failMayaOnce(['hash-1'])); + const first = await migrate({ apply: true, now: NOW }); + expect(entryFor(first, 'maya-agent')).toMatchObject({ reached: failAt }); + + const rowsNow = async () => Promise.all([maya._id, mayaB._id].map((id) => AgentInstallation.findById(id))); + const stuck = await rowsNow(); + expect(stuck.map((row) => readStoredEngine(row))).toEqual(['native', 'native']); + expect(stuck.map(hostedEngineInEffect)).toEqual(['durable-object', 'durable-object']); + expect(stuck.map(hostedRunsNative)).toEqual([false, false]); + expect(stuck.map(runsInNarrowedProcess)).toEqual([false, false]); + + await migrate({ apply: true, now: NOW }); + const finished = await rowsNow(); + expect(finished.map(hostedRunsNative)).toEqual([true, true]); + expect(finished.map(runsInNarrowedProcess)).toEqual([true, true]); + // A row born native was never migrated and is native throughout. + const bornRow = await AgentInstallation.findById(born._id); + expect(hostedRunsNative(bornRow)).toBe(true); }); test('a failed event write is resumed: the next run handles the events and finishes (Rhea 4)', async () => { diff --git a/backend/__tests__/unit/services/outsiderGrantAgentJoinPolicy.test.js b/backend/__tests__/unit/services/outsiderGrantAgentJoinPolicy.test.js index 539137f954..2e14a6e312 100644 --- a/backend/__tests__/unit/services/outsiderGrantAgentJoinPolicy.test.js +++ b/backend/__tests__/unit/services/outsiderGrantAgentJoinPolicy.test.js @@ -74,6 +74,31 @@ describe('outsider-writable grant agent join policy', () => { expect(AgentInstallation.findOne.mock.results[0].value.select).toHaveBeenCalledWith('config'); }); + // Rhea on #2236 (a600bf3d): an unfinished TASK-265 migration is not native + // for joins, on either the installed-agent path or the engine string the + // other join sites pass (hostedEngineInEffect). + it('refuses an agent whose migration is unfinished and allows it once done', async () => { + // eslint-disable-next-line global-require + const { hostedEngineInEffect } = require('../../../services/hostedEngine'); + const migrated = (state) => new Map([ + ['runtime', { runtimeType: 'hosted' }], + ['hosted', { engine: 'native', migratedFrom: 'durable-object', migration: { state, doProvisioned: true } }], + ]); + const lean = jest.fn() + .mockResolvedValueOnce({ config: migrated('marked') }) + .mockResolvedValueOnce({ config: migrated('done') }); + AgentInstallation.findOne.mockReturnValue({ select: jest.fn().mockReturnValue({ lean }) }); + await expect(assertInstalledAgentMayJoinPod('pod-1', 'bot', 'default')).rejects.toHaveProperty( + 'code', 'outsider_grant_non_native_agent_join', + ); + await expect(assertInstalledAgentMayJoinPod('pod-1', 'bot', 'default')).resolves.toBeUndefined(); + + await expect(assertAgentRuntimeMayJoinPod('pod-1', 'hosted', hostedEngineInEffect({ config: migrated('stopped') }))) + .rejects.toHaveProperty('code', 'outsider_grant_non_native_agent_join'); + await expect(assertAgentRuntimeMayJoinPod('pod-1', 'hosted', hostedEngineInEffect({ config: migrated('done') }))) + .resolves.toBeUndefined(); + }); + it('never puts a caller-controlled query object in the installation lookup', async () => { AgentInstallation.findOne.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue(null) }), @@ -99,3 +124,30 @@ describe('outsider-writable grant agent join policy', () => { ); }); }); + +// Rhea on #2236 (a600bf3d): every site that decides how an agent runs or what it +// may join must use the engine in effect, which is native only once a +// migration finished. Only these read the STORED engine: the engine module, the +// migration (which writes it), and the hosted route (which picks the provision +// branch; the token issuer refuses on the stored engine by design). +describe('who reads the stored engine', () => { + // eslint-disable-next-line global-require + const fs = require('fs'); + // eslint-disable-next-line global-require + const path = require('path'); + const ROOT = path.join(__dirname, '../../..'); + const ALLOWED = new Set(['services/hostedEngine.ts', 'services/hostedEngineMigration.ts', 'routes/hosted.ts']); + const walk = (dir) => fs.readdirSync(path.join(ROOT, dir), { withFileTypes: true }).flatMap((entry) => { + const rel = path.join(dir, entry.name); + if (entry.isDirectory()) return walk(rel); + return /\.(ts|js)$/.test(entry.name) ? [rel] : []; + }); + + it('is limited to the engine module, the migration and the hosted route', () => { + const readers = ['services', 'routes', 'middleware', 'controllers'] + .filter((dir) => fs.existsSync(path.join(ROOT, dir))) + .flatMap(walk) + .filter((file) => /\bhostedEngineOf\b/.test(fs.readFileSync(path.join(ROOT, file), 'utf8'))); + expect(readers.sort()).toEqual([...ALLOWED].sort()); + }); +}); diff --git a/backend/__tests__/unit/services/outsiderGrantPodEligibility.test.js b/backend/__tests__/unit/services/outsiderGrantPodEligibility.test.js index bdd8d9bc77..a83d1444d1 100644 --- a/backend/__tests__/unit/services/outsiderGrantPodEligibility.test.js +++ b/backend/__tests__/unit/services/outsiderGrantPodEligibility.test.js @@ -108,6 +108,26 @@ test.each([ expect(await outsiderGrantPodRefusal(target)).toBe('non_native_agent'); }); +// Rhea on #2236 (a600bf3d): a row the TASK-265 migration marked native but has +// not finished may still have a running worker or a live token, so it is not +// in-process until the migration records it done. +const MIGRATED = { engine: 'native', provisionedAt: new Date(), migratedFrom: 'durable-object' }; +test.each([ + ['marked', { migration: { state: 'marked', doProvisioned: true } }], + ['stopped', { migration: { state: 'stopped', doProvisioned: true } }], + ['recorded with no state', {}], +])('a hosted agent whose migration is %s is ineligible', async (_label, extra) => { + installations[0].config = { runtime: { runtimeType: 'hosted' }, hosted: { ...MIGRATED, ...extra } }; + expect(await outsiderGrantPodRefusal(target)).toBe('non_native_agent'); +}); + +test('a hosted agent whose migration is done is eligible', async () => { + installations[0].config = { + runtime: { runtimeType: 'hosted' }, hosted: { ...MIGRATED, migration: { state: 'done', doProvisioned: true } }, + }; + expect(await outsiderGrantPodRefusal(target)).toBeNull(); +}); + test('an agent with no matching active installation is ineligible', async () => { installations = []; expect(await outsiderGrantPodRefusal(target)).toBe('non_native_agent'); diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index f1bcf1bf44..6e4d8887e7 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -50,7 +50,7 @@ const { assertAgentRuntimeMayJoinPod, runtimeTypeFromConfig, } = require('../services/outsiderGrantAgentJoinPolicy'); -const { hostedEngineOf } = require('../services/hostedEngine'); +const { hostedEngineInEffect } = require('../services/hostedEngine'); // ADR-003 Phase 4: per-token rate limiter for the cross-agent surface. // Token-global (covers any pod the token is valid for). Complementary to the @@ -3152,7 +3152,7 @@ router.post('/pods', phase4RateLimit, agentRuntimeAuth, async (req: any, res: an ? Object.fromEntries(sourceInstall.config) : sourceInstall?.config || {}; try { await assertAgentRuntimeMayJoinPod( - existingPod._id, runtimeTypeFromConfig(sourceConfig), hostedEngineOf({ config: sourceConfig }), + existingPod._id, runtimeTypeFromConfig(sourceConfig), hostedEngineInEffect({ config: sourceConfig }), ); } catch (joinError: any) { if (joinError?.code === 'outsider_grant_non_native_agent_join') { @@ -3314,7 +3314,7 @@ router.post('/pods/:podId/self-install', phase4RateLimit, agentRuntimeAuth, asyn ? Object.fromEntries(sourceInstall.config) : sourceInstall.config || {}; try { await assertAgentRuntimeMayJoinPod( - podId, runtimeTypeFromConfig(sourceConfig), hostedEngineOf({ config: sourceConfig }), + podId, runtimeTypeFromConfig(sourceConfig), hostedEngineInEffect({ config: sourceConfig }), ); } catch (joinError: any) { if (joinError?.code === 'outsider_grant_non_native_agent_join') { diff --git a/backend/routes/hosted.ts b/backend/routes/hosted.ts index c6c2bc132b..bf3b9b23c8 100644 --- a/backend/routes/hosted.ts +++ b/backend/routes/hosted.ts @@ -59,6 +59,22 @@ const INSTANCE_RE = /^[a-z0-9-]+$/; const getUserId = (req: any) => req.userId || req.user?.id || req.user?._id; +/** + * Write a Durable Object install's `config.hosted` only if the row is still on + * the Durable Object (TASK-265, Rhea on #2236). The migration can mark a row + * native while this request awaits the worker, and a blind save of the record + * read at the start would put it back on the Durable Object, after which the + * next provision mints again. Same spelling of native as the token issuer's + * check. Returns false when the migration got there first. + */ +const writeDurableObjectRecord = async (installation: any, hosted: Record): Promise => { + const result = await AgentInstallation.updateOne( + { _id: installation._id, 'config.hosted.engine': { $not: /^\s*native\s*$/i } }, + { $set: { 'config.hosted': hosted } }, + ); + return Number(result?.matchedCount ?? 0) > 0; +}; + /** * Resolve the identity the caller may act on. Returns the hydrated * installation (the token path saves onto it) or an error tuple. @@ -234,10 +250,14 @@ router.post('/provision', hostedRateLimit, auth, async (req: any, res: any) => { if (engine === HOSTED_ENGINE_NATIVE) { // Nothing to start: turns run in-process. No token is minted, so with // the revoke above nothing outside the backend can act as this agent. - // Idempotent: the install already marked it provisioned. + // Idempotent: the install already marked it provisioned. Only the field + // this route owns is written, so a migration's record (its state, the + // persona) is never replaced by the copy read at the start. const hosted = readHostedConfig(installation); - installation.config.set('hosted', { ...hosted, provisionedAt: hosted.provisionedAt || new Date() }); - await installation.save(); + await AgentInstallation.updateOne( + { _id: installation._id }, + { $set: { 'config.hosted.provisionedAt': hosted.provisionedAt || new Date() } }, + ); return res.json({ provisioned: true, engine, @@ -259,8 +279,23 @@ router.post('/provision', hostedRateLimit, auth, async (req: any, res: any) => { } await hostedRuntime.provisionAgent({ agentName, instanceId, runtimeToken: token.token }); - installation.config.set('hosted', { provisionedAt: new Date() }); - await installation.save(); + if (!(await writeDurableObjectRecord(installation, { provisionedAt: new Date() }))) { + // Migrated to the native engine while the worker call was in flight. The + // migration owns this identity now: stop what this request started and + // take its token back. Both are idempotent with the migration's own stop + // and revoke, and the token was minted before the mark, so the issuer's + // refusal could not have caught it. + try { + await hostedRuntime.deprovisionAgent(agentName, instanceId); + } catch (error: any) { + console.warn('[hosted] stopping a provision overtaken by migration failed:', error?.message || error); + } + await revokeRuntimeTokensForAgent({ agentUser, agentName, instanceId, installation }); + return res.status(409).json({ + code: 'hosted_engine_migrated', + message: 'This agent moved to the native engine while it was starting. Provision it again.', + }); + } return res.json({ provisioned: true, @@ -289,18 +324,19 @@ router.post('/deprovision', hostedRateLimit, auth, async (req: any, res: any) => const { installation, agentName, instanceId } = resolved; if (hostedEngineOf(installation) === HOSTED_ENGINE_NATIVE) { // Stopping is the record alone: an unprovisioned native-engine install - // no longer routes to the runtime (hostedRunsNative). Engine and - // persona stay for the next provision. - installation.config.set('hosted', { - ...readHostedConfig(installation), provisionedAt: null, deprovisionedAt: new Date(), - }); - await installation.save(); + // no longer routes to the runtime (hostedRunsNative). Engine, persona + // and any migration record stay; only these two fields are written. + await AgentInstallation.updateOne( + { _id: installation._id }, + { $set: { 'config.hosted.provisionedAt': null, 'config.hosted.deprovisionedAt': new Date() } }, + ); return res.json({ deprovisioned: true, agentName, instanceId }); } if (!requireConfigured(res, hostedEngineOf(installation))) return undefined; await hostedRuntime.deprovisionAgent(agentName, instanceId); - installation.config.set('hosted', { provisionedAt: null, deprovisionedAt: new Date() }); - await installation.save(); + // A row migrated meanwhile keeps the migration's record; the worker is + // stopped either way, which is what both wanted. + await writeDurableObjectRecord(installation, { provisionedAt: null, deprovisionedAt: new Date() }); return res.json({ deprovisioned: true, agentName, instanceId }); } catch (error) { return relayWorkerError(res, error); diff --git a/backend/routes/registry/agent-config.ts b/backend/routes/registry/agent-config.ts index 7b3bfdbc35..649a7dae09 100644 --- a/backend/routes/registry/agent-config.ts +++ b/backend/routes/registry/agent-config.ts @@ -28,7 +28,7 @@ const { validateEnvironmentMcpEntries } = require('../../utils/environmentSpecVa const { ownsRuntimeIdentity, webhookUrlRefusal } = require('../../services/runtimeIdentityOwnership'); const { configWriteRefusal, runtimeKeepingType } = require('../../services/configWriteGuard'); const { assertAgentRuntimeMayJoinPod } = require('../../services/outsiderGrantAgentJoinPolicy'); -const { hostedEngineOf } = require('../../services/hostedEngine'); +const { hostedEngineInEffect } = require('../../services/hostedEngine'); const agentConfigRouter = express.Router(); @@ -274,7 +274,7 @@ agentConfigRouter.patch('/pods/:podId/agents/:name', auth, async (req: any, res: // eslint-disable-next-line no-await-in-loop await assertAgentRuntimeMayJoinPod( targetInstallation.podId, runtime?.runtimeType, - hostedEngineOf({ config: stored }), + hostedEngineInEffect({ config: stored }), ); } } catch (joinError: any) { diff --git a/backend/routes/registry/provision.ts b/backend/routes/registry/provision.ts index 681caa243c..6fed8f4dba 100644 --- a/backend/routes/registry/provision.ts +++ b/backend/routes/registry/provision.ts @@ -12,7 +12,7 @@ const User = require('../../models/User'); const Integration = require('../../models/Integration'); const AgentIdentityService = require('../../services/agentIdentityService'); const { assertAgentRuntimeMayJoinPod } = require('../../services/outsiderGrantAgentJoinPolicy'); -const { hostedEngineOf } = require('../../services/hostedEngine'); +const { hostedEngineInEffect } = require('../../services/hostedEngine'); const DMService = require('../../services/dmService'); const { provisionAgentRuntime, @@ -156,7 +156,7 @@ provisionRouter.post('/pods/:podId/agents/:name/provision', provisionRateLimit, return res.status(400).json({ error: 'Unknown agent runtime type' }); } try { - await assertAgentRuntimeMayJoinPod(podId, runtimeType, hostedEngineOf(installation)); + await assertAgentRuntimeMayJoinPod(podId, runtimeType, hostedEngineInEffect(installation)); } catch (joinError: any) { if (joinError?.code === 'outsider_grant_non_native_agent_join') { return res.status(403).json({ code: joinError.code, message: joinError.message }); diff --git a/backend/routes/registry/reprovision.ts b/backend/routes/registry/reprovision.ts index 4a913b5a13..6f6d2fb180 100644 --- a/backend/routes/registry/reprovision.ts +++ b/backend/routes/registry/reprovision.ts @@ -5,7 +5,7 @@ const AgentProfile = require('../../models/AgentProfile'); const Integration = require('../../models/Integration'); const AgentIdentityService = require('../../services/agentIdentityService'); const { assertAgentRuntimeMayJoinPod } = require('../../services/outsiderGrantAgentJoinPolicy'); -const { hostedEngineOf } = require('../../services/hostedEngine'); +const { hostedEngineInEffect } = require('../../services/hostedEngine'); const { provisionAgentRuntime, startAgentRuntime, @@ -57,7 +57,7 @@ const reprovisionInstallation = async ({ if (!runtimeType) { throw new Error('Unknown agent runtime type'); } - await assertAgentRuntimeMayJoinPod(podId, runtimeType, hostedEngineOf(installation)); + await assertAgentRuntimeMayJoinPod(podId, runtimeType, hostedEngineInEffect(installation)); const agentUser = await AgentIdentityService.getOrCreateAgentUser(name, { instanceId: normalizedInstanceId, diff --git a/backend/services/agentAutoJoinService.ts b/backend/services/agentAutoJoinService.ts index b7f7b7475d..59bf789642 100644 --- a/backend/services/agentAutoJoinService.ts +++ b/backend/services/agentAutoJoinService.ts @@ -3,7 +3,7 @@ import Activity from '../models/Activity'; import { AgentInstallation } from '../models/AgentRegistry'; import AgentIdentityService from './agentIdentityService'; import { assertAgentRuntimeMayJoinPod, runtimeTypeFromConfig } from './outsiderGrantAgentJoinPolicy'; -import { hostedEngineOf } from './hostedEngine'; +import { hostedEngineInEffect } from './hostedEngine'; const MAX_TOTAL_INSTALLS = Math.max(1, parseInt(process.env.AGENT_AUTO_JOIN_MAX_TOTAL || '200', 10)); const MAX_PER_SOURCE = Math.max(1, parseInt(process.env.AGENT_AUTO_JOIN_MAX_PER_SOURCE || '25', 10)); @@ -80,7 +80,7 @@ class AgentAutoJoinService { await assertAgentRuntimeMayJoinPod( (pod as Record)._id, runtimeTypeFromConfig(sourceInstall.config), - hostedEngineOf(sourceInstall), + hostedEngineInEffect(sourceInstall), ); // eslint-disable-next-line no-await-in-loop await AgentInstallation.install( diff --git a/backend/services/hostedEngine.ts b/backend/services/hostedEngine.ts index 2683d7ee37..d92298073e 100644 --- a/backend/services/hostedEngine.ts +++ b/backend/services/hostedEngine.ts @@ -37,10 +37,42 @@ export const hostedEngineOf = (installation: any): HostedEngine => ( : HOSTED_ENGINE_DURABLE_OBJECT ); +/** Rows the TASK-265 migration moved off the Durable Object carry this in `config.hosted.migratedFrom`. */ +export const MIGRATED_FROM_DURABLE_OBJECT = 'durable-object'; + +/** + * True while the TASK-265 migration has marked a row native but not yet + * recorded it done: its Durable Object may still be running, or its old + * runtime token may still be live (Rhea on #2236). A migrated row with no + * recorded state counts as unfinished, as the migration itself reads it. + */ +export const hostedMigrationUnfinished = (installation: any): boolean => { + const hosted = readHostedConfig(installation); + if (hosted.migratedFrom !== MIGRATED_FROM_DURABLE_OBJECT) return false; + const raw = hosted.migration; + const migration = raw instanceof Map ? Object.fromEntries(raw) : (raw && typeof raw === 'object' ? raw : {}); + return (migration as { state?: unknown }).state !== 'done'; +}; + +/** + * The engine that decides how an install runs and what it may join: native + * only once any migration onto it has finished. Everything that relies on a + * native agent having no channel outside this process (where its turns run, + * grant eligibility, agent joins into granted pods) asks this, never + * `hostedEngineOf`. The token issuer is the deliberate exception: it refuses + * on the stored engine, so an unfinished row can neither mint nor count as + * in-process (routes/registry/tokens.ts). + */ +export const hostedEngineInEffect = (installation: any): HostedEngine => ( + hostedEngineOf(installation) === HOSTED_ENGINE_NATIVE && !hostedMigrationUnfinished(installation) + ? HOSTED_ENGINE_NATIVE + : HOSTED_ENGINE_DURABLE_OBJECT +); + /** The engine boundary shared by grant eligibility and agent-join checks. */ export const runsInNarrowedProcess = (installation: any): boolean => ( String(readRuntimeConfig(installation).runtimeType || '').trim().toLowerCase() === 'native' - || (isHostedInstallation(installation) && hostedEngineOf(installation) === HOSTED_ENGINE_NATIVE) + || (isHostedInstallation(installation) && hostedEngineInEffect(installation) === HOSTED_ENGINE_NATIVE) ); /** @@ -75,7 +107,7 @@ export const nativeHostedConfigForInstall = (about?: string): Record ( isHostedInstallation(installation) - && hostedEngineOf(installation) === HOSTED_ENGINE_NATIVE + && hostedEngineInEffect(installation) === HOSTED_ENGINE_NATIVE && Boolean(readHostedConfig(installation).provisionedAt) ); diff --git a/backend/services/hostedEngineMigration.ts b/backend/services/hostedEngineMigration.ts index a652890201..7de3989fcb 100644 --- a/backend/services/hostedEngineMigration.ts +++ b/backend/services/hostedEngineMigration.ts @@ -16,10 +16,18 @@ * 1. mark: every row native, state 'marked'. From here the token issuer * refuses to mint for the identity, and re-checks after saving, * so a mint already in flight takes its own token back - * (routes/registry/tokens.ts). - * 2. stop: the Durable Object, if one existed; state 'stopped'. Recorded - * apart from the mark, so a run interrupted before the stop - * stops it on the next run instead of assuming it happened. + * (routes/registry/tokens.ts). Until 'done' the rows are NOT + * native for execution, grant eligibility or joins + * (hostedEngineInEffect), so no step's failure leaves an + * identity with an outside channel counted as in-process. + * 2. stop: the Durable Object, whenever the worker is configured, since + * its stop is idempotent and a provision can land after this + * run read the rows; state 'stopped'. Recorded apart from the + * mark, so a run interrupted before the stop stops it on the + * next run instead of assuming it happened. A provision in + * flight across the migration does not overwrite it either: the + * route's record write is fenced on the stored engine + * (routes/hosted.ts). * 3. revoke: the identity's runtime tokens. * 4. events: pending events, which nothing polls once the Durable Object * stops. A recent one on a provisioned install is handed to the @@ -30,7 +38,9 @@ * report says which, and the next run resumes it. Nothing is reported done * that is not. */ -import { HOSTED_ENGINE_NATIVE, hostedEngineOf, readHostedConfig } from './hostedEngine'; +import { + HOSTED_ENGINE_NATIVE, MIGRATED_FROM_DURABLE_OBJECT, hostedEngineOf, readHostedConfig, +} from './hostedEngine'; // eslint-disable-next-line @typescript-eslint/no-require-imports const { AgentInstallation } = require('../models/AgentRegistry'); @@ -45,7 +55,6 @@ const hostedRuntime = require('./hostedRuntimeService'); const STALE_PENDING_MS = 10 * 60 * 1000; const ABOUT_LIMIT = 160; -const MIGRATED_FROM = 'durable-object'; type MigrationState = 'marked' | 'stopped' | 'done'; @@ -106,7 +115,7 @@ export const migrateHostedInstallsToNative = async ( const instanceId = String(rows[0].instanceId || 'default'); const isProvisioned = (row: any) => Boolean(readHostedConfig(row).provisionedAt); const unmarked = rows.filter((row) => hostedEngineOf(row) !== HOSTED_ENGINE_NATIVE); - const migrated = rows.filter((row) => readHostedConfig(row).migratedFrom === MIGRATED_FROM); + const migrated = rows.filter((row) => readHostedConfig(row).migratedFrom === MIGRATED_FROM_DURABLE_OBJECT); const unfinished = migrated.filter((row) => migrationOf(row).state !== 'done'); // Done, or born native: nothing to do. if (!unmarked.length && !unfinished.length) continue; @@ -158,7 +167,7 @@ export const migrateHostedInstallsToNative = async ( engine: HOSTED_ENGINE_NATIVE, provisionedAt: isProvisioned(row) ? now : null, ...(about ? { about } : {}), - migratedFrom: MIGRATED_FROM, + migratedFrom: MIGRATED_FROM_DURABLE_OBJECT, migratedAt: now, migration: { state: 'marked', doProvisioned }, }, @@ -170,10 +179,13 @@ export const migrateHostedInstallsToNative = async ( continue; } - // 2. Stop the Durable Object, recorded separately from the mark. + // 2. Stop the Durable Object, recorded separately from the mark. Whenever + // the worker is configured, not only when this run saw it provisioned: a + // provision can complete between the read above and the mark, and the + // worker's stop is a no-op for an agent it is not running. entry.reached = 'stop'; try { - if (doProvisioned && !stopped) { + if (!stopped && hostedRuntime.isConfigured()) { // eslint-disable-next-line no-await-in-loop await hostedRuntime.deprovisionAgent(agentName, instanceId); entry.durableObjectStopped = true; From 5bb5d230968cbe78b04e4a660e4c7d136899f199 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:44:28 -0700 Subject: [PATCH 6/8] fix(hosted): hand off events that arrive while the migration runs An event enqueued between the events step and 'done' went to the Durable Object path, since the rows are not native until 'done', and a later run skips a done identity, so nothing would ever run or close it. The run now hands off whatever is pending after it records done, the same way as step 4, and reports the count (lateEvents). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../services/hostedEngineMigration.test.js | 22 +++++++ backend/services/hostedEngineMigration.ts | 63 ++++++++++++++----- 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index 266b54d28a..e96d01ef47 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -289,3 +289,25 @@ test('an install born native is never touched or reported', async () => { expect(hosted.migratedFrom).toBeUndefined(); expect(hosted.migration).toBeUndefined(); }); + +// An event that arrives while the run works goes to the Durable Object path, +// since the rows are not native until 'done', and a later run skips a done +// identity. The run hands it off after recording done. +test('an event that arrives during the run is handed off after done, not stranded', async () => { + let late; + mockRevoke.mockImplementation(async ({ agentName }) => { + if (agentName === 'maya-agent') { + [late] = await AgentEvent.create([{ + agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', status: 'pending', + createdAt: new Date(NOW.getTime() - 1000), payload: {}, + }]); + } + return ['hash-1']; + }); + + const report = await migrate({ apply: true, now: NOW }); + + expect(entryFor(report, 'maya-agent')).toMatchObject({ reached: 'done', pendingToSweep: 0, lateEvents: 1 }); + const after = await AgentEvent.findById(late._id).lean(); + expect(after).toMatchObject({ status: 'delivered', deliveryNonce: null }); +}); diff --git a/backend/services/hostedEngineMigration.ts b/backend/services/hostedEngineMigration.ts index 7de3989fcb..b4b058506f 100644 --- a/backend/services/hostedEngineMigration.ts +++ b/backend/services/hostedEngineMigration.ts @@ -34,6 +34,10 @@ * in-process sweep (marked delivered; #2226 re-runs it), so this * script never runs a turn. Others are closed with a reason. * 5. done. + * 6. late events: any that arrived while this run worked went to the + * Durable Object path (the rows were not native until 'done') + * and are handed off the same way; a later run skips a done + * identity, so this is the only pass that sees them. * Every step is idempotent. A step that fails stops that identity there; the * report says which, and the next run resumes it. Nothing is reported done * that is not. @@ -71,6 +75,8 @@ export interface IdentityReport { tokensRevoked?: number; pendingToSweep: number; pendingClosed: number; + /** Events that arrived while this run worked, handed off after 'done'. */ + lateEvents?: number; error?: string; } @@ -129,8 +135,29 @@ export const migrateHostedInstallsToNative = async ( const pending = await AgentEvent.find({ agentName, instanceId, status: 'pending' }).lean(); const runningPods = new Set(rows.filter(isProvisioned).map((row) => String(row.podId))); const recent = (event: any) => now.getTime() - new Date(event.createdAt).getTime() < STALE_PENDING_MS; - const toSweep = pending.filter((event: any) => recent(event) && runningPods.has(String(event.podId))); + const sweepable = (event: any) => recent(event) && runningPods.has(String(event.podId)); + const toSweep = pending.filter(sweepable); const toClose = pending.filter((event: any) => !toSweep.includes(event)); + // A recent event on a running install goes to the in-process sweep (#2226 + // re-runs a delivered event with no nonce); any other is closed with a + // reason. Only still-pending events are touched. + const handOff = async (events: any[]) => { + for (const event of events) { + if (sweepable(event)) { + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne( + { _id: event._id, status: 'pending' }, + { $set: { status: 'delivered', deliveredAt: now, deliveryNonce: null }, $inc: { attempts: 1 } }, + ); + } else { + const reason = recent(event) + ? 'hosted engine migrated while this agent was stopped; it was not run' + : 'hosted engine migrated before this event ran (stale)'; + // eslint-disable-next-line no-await-in-loop + await AgentEvent.updateOne({ _id: event._id, status: 'pending' }, { $set: { status: 'failed', error: reason } }); + } + } + }; const entry: IdentityReport = { agentName, instanceId, @@ -217,24 +244,11 @@ export const migrateHostedInstallsToNative = async ( continue; } - // 4. Pending events. Only still-pending events are touched, so a re-run - // picks up whatever a failed run left. + // 4. Pending events, so a re-run picks up whatever a failed run left. entry.reached = 'events'; try { - for (const event of toSweep) { - // eslint-disable-next-line no-await-in-loop - await AgentEvent.updateOne( - { _id: event._id, status: 'pending' }, - { $set: { status: 'delivered', deliveredAt: now, deliveryNonce: null }, $inc: { attempts: 1 } }, - ); - } - for (const event of toClose) { - const reason = recent(event) - ? 'hosted engine migrated while this agent was stopped; it was not run' - : 'hosted engine migrated before this event ran (stale)'; - // eslint-disable-next-line no-await-in-loop - await AgentEvent.updateOne({ _id: event._id, status: 'pending' }, { $set: { status: 'failed', error: reason } }); - } + // eslint-disable-next-line no-await-in-loop + await handOff(pending); } catch (error) { entry.error = `pending events not all handled (${(error as Error).message}); run again to finish`; continue; @@ -248,6 +262,21 @@ export const migrateHostedInstallsToNative = async ( entry.reached = 'done'; } catch (error) { entry.error = `not recorded as done (${(error as Error).message}); run again`; + continue; + } + + // 6. Events that arrived during this run went to the Durable Object path, + // because the rows were not native until 'done'. Nothing polls that path + // now, and a later run skips a done identity, so hand them off here. + try { + // eslint-disable-next-line no-await-in-loop + const late = await AgentEvent.find({ agentName, instanceId, status: 'pending' }).lean(); + // eslint-disable-next-line no-await-in-loop + await handOff(late); + entry.lateEvents = late.length; + } catch (error) { + entry.error = `done, but events that arrived during the run were not handed off (${(error as Error).message}); ` + + 'a later run skips a done identity, so check its pending events by hand'; } } return report; From 326a9edb49dfa60618c008bcac612c05db5f51f8 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:58:08 -0700 Subject: [PATCH 7/8] fix(events): the recurring sweep runs a pending event stranded by a move in-process Rhea's HOLD on #2236 at b42558aa: a one-shot scan is not a durable handoff. Enqueue decides routing before it inserts, so an event enqueued while the hosted migration runs can persist 'pending' after the agent became in-process, and a rerun skips a done identity. A failed or interrupted late scan stranded events the same way. In-process events are born 'delivered', so a 'pending' event whose agent now resolves in-process is stranded by construction. The recurring in-process sweep (garbageCollect, beside #2226's restart re-run) now runs those: - once they are a minute old (never racing an enqueue), and not past half an hour (left to the retention delete); - candidates limited to identities with an in-process install (the filter the token issuer uses, now shared from hostedEngine), so a backlog of external agents' pending events cannot crowd them out; - each decided by enqueue's own routing with the engine in effect, so an unfinished migration is left alone; - claimed by compare-and-set from 'pending', counted as a delivery, under the attempt cap. The migration's one-shot late scan is removed; the script still runs no turn. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../agentEventService.deliveryNonce.test.js | 111 ++++++++++++++++++ .../services/hostedEngineMigration.test.js | 64 +++++++--- backend/routes/registry/tokens.ts | 13 +- backend/services/agentEventService.ts | 72 +++++++++++- backend/services/hostedEngine.ts | 18 +++ backend/services/hostedEngineMigration.ts | 26 +--- 6 files changed, 253 insertions(+), 51 deletions(-) diff --git a/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js b/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js index 0abc17d2b6..46808a2e36 100644 --- a/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js +++ b/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js @@ -562,6 +562,117 @@ describe('the sweep re-runs a stuck in-process event', () => { }); }); +// TASK-265, Rhea on #2236: enqueue decides routing before the insert, so an +// event enqueued while the hosted migration runs can persist 'pending' after +// the agent became in-process, and nothing polls it. The recurring sweep runs +// it. Real installation rows, so the routing decision is the production one. +describe('the sweep runs a pending event stranded by a move in-process', () => { + const { runAgent } = require('../../../services/nativeRuntimeService'); + const { AgentInstallation } = require('../../../models/AgentRegistry'); + const OWNER = new mongoose.Types.ObjectId(); + const HOSTED_DO = { runtime: { runtimeType: 'hosted' }, hosted: { provisionedAt: new Date() } }; + const MIGRATED = (state) => ({ + runtime: { runtimeType: 'hosted' }, + hosted: { + engine: 'native', provisionedAt: new Date(), migratedFrom: 'durable-object', migration: { state, doProvisioned: true }, + }, + }); + const install = (agentName, config, podId = POD_ID) => AgentInstallation.create({ + agentName, instanceId: INSTANCE, podId, version: '1.0.0', status: 'active', installedBy: OWNER, config, + }); + // createdAt is immutable through Mongoose, so age it on the collection. + const age = (eventId, ms) => AgentEvent.collection.updateOne( + { _id: eventId }, { $set: { createdAt: new Date(Date.now() - ms) } }, + ); + const sweep = () => AgentEventService.garbageCollect({ requeueDeliveredMinutes: 10 }); + + beforeEach(async () => { + await AgentInstallation.deleteMany({}); + runAgent.mockReset(); + runAgent.mockResolvedValue({ status: 'succeeded' }); + }); + afterEach(async () => { + await AgentInstallation.deleteMany({}); + runAgent.mockReset(); + }); + + test('the enqueue race: routed to the Durable Object, persisted after the migration finished, then run once', async () => { + // The turn stays in flight, so the claim the sweep made is what is stored. + runAgent.mockImplementation(() => new Promise(() => {})); + const row = await install(AGENT, HOSTED_DO); + // Routing is decided while the row is still on the Durable Object... + const event = await AgentEventService.enqueue({ + agentName: AGENT, instanceId: INSTANCE, podId: POD_ID, type: 'chat.mention', payload: { text: 'hi' }, + }); + expect((await AgentEvent.findById(event._id).lean()).status).toBe('pending'); + expect(runAgent).not.toHaveBeenCalled(); + // ...and the migration finishes before anything polls it. + await AgentInstallation.updateOne({ _id: row._id }, { $set: { config: MIGRATED('done') } }); + await age(event._id, 2 * 60 * 1000); + + const result = await sweep(); + + expect(result.rescuedPendingInProcess).toBe(1); + const ran = await AgentEvent.findById(event._id).lean(); + expect(ran.status).toBe('delivered'); + expect(ran.deliveryNonce).toEqual(expect.any(String)); + expect(ran.attempts).toBe(1); + expect(runAgent).toHaveBeenCalledTimes(1); + + await sweep(); + expect(runAgent).toHaveBeenCalledTimes(1); + }); + + test.each([ + ['an agent still on the Durable Object', HOSTED_DO, 2 * 60 * 1000, 0], + ['a migration not yet done', MIGRATED('stopped'), 2 * 60 * 1000, 0], + ['an event under a minute old', MIGRATED('done'), 10 * 1000, 0], + ['an event past half an hour', MIGRATED('done'), 31 * 60 * 1000, 0], + ['an event at the attempt cap', MIGRATED('done'), 2 * 60 * 1000, 3], + ])('leaves alone %s', async (_label, config, ageMs, attempts) => { + await install(AGENT, config); + const event = await seedEvent(); + await AgentEvent.updateOne({ _id: event._id }, { $set: { attempts } }); + await age(event._id, ageMs); + + const result = await sweep(); + + expect(result.rescuedPendingInProcess).toBe(0); + expect(runAgent).not.toHaveBeenCalled(); + }); + + test('two sweeps racing (two replicas) run it once', async () => { + await install(AGENT, MIGRATED('done')); + const event = await seedEvent(); + await age(event._id, 2 * 60 * 1000); + + const counts = await Promise.all([ + AgentEventService.rescueStrandedPendingInProcess(new Date(), 3), + AgentEventService.rescueStrandedPendingInProcess(new Date(), 3), + ]); + + expect(counts[0] + counts[1]).toBe(1); + expect(runAgent).toHaveBeenCalledTimes(1); + }); + + test("a backlog of external agents' pending events does not crowd it out", async () => { + await install(AGENT, MIGRATED('done')); + const backlog = Array.from({ length: 60 }, () => ({ + agentName: 'offline-seat', instanceId: INSTANCE, podId: POD_ID, type: 'chat.mention', payload: {}, status: 'pending', + })); + const external = await AgentEvent.create(backlog); + await AgentEvent.collection.updateMany( + { _id: { $in: external.map((row) => row._id) } }, + { $set: { createdAt: new Date(Date.now() - 20 * 60 * 1000) } }, + ); + const event = await seedEvent(); + await age(event._id, 2 * 60 * 1000); + + expect(await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).toBe(1); + expect((await AgentEvent.findById(event._id).lean()).status).not.toBe('pending'); + }); +}); + describe('recordFailure is as terminal as an ack, and gated the same way', () => { test('a stale runner\'s failure does not retire the replacement\'s delivery', async () => { const event = await seedEvent(); diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index e96d01ef47..fb2c132784 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -290,24 +290,54 @@ test('an install born native is never touched or reported', async () => { expect(hosted.migration).toBeUndefined(); }); -// An event that arrives while the run works goes to the Durable Object path, -// since the rows are not native until 'done', and a later run skips a done -// identity. The run hands it off after recording done. -test('an event that arrives during the run is handed off after done, not stranded', async () => { - let late; - mockRevoke.mockImplementation(async ({ agentName }) => { - if (agentName === 'maya-agent') { - [late] = await AgentEvent.create([{ - agentName: 'maya-agent', instanceId: 'default', podId: podA, type: 'chat.mention', status: 'pending', - createdAt: new Date(NOW.getTime() - 1000), payload: {}, - }]); - } - return ['hash-1']; +// Rhea on #2236 (b42558aa): enqueue routes before it inserts, so an event can +// land 'pending' after the run finished, and a rerun skips a done identity. +// The recurring in-process sweep runs it; the script itself never runs a turn. +describe('an event left pending by the move', () => { + let AgentEventService; let dispatch; + beforeEach(() => { + // eslint-disable-next-line global-require + AgentEventService = require('../../../services/agentEventService'); + dispatch = jest.spyOn(AgentEventService, 'dispatchInProcess').mockImplementation(() => {}); }); + const pendingFor = async (podId) => { + const [event] = await AgentEvent.create([{ + agentName: 'maya-agent', instanceId: 'default', podId, type: 'chat.mention', status: 'pending', payload: {}, + }]); + await AgentEvent.collection.updateOne({ _id: event._id }, { $set: { createdAt: new Date(Date.now() - 2 * 60 * 1000) } }); + return event; + }; - const report = await migrate({ apply: true, now: NOW }); + test('after done: a rerun of the script skips it and runs nothing; the sweep runs it once', async () => { + await migrate({ apply: true, now: NOW }); + const late = await pendingFor(podA); + + const rerun = await migrate({ apply: true, now: NOW }); + expect(entryFor(rerun, 'maya-agent')).toBeUndefined(); + expect(dispatch).not.toHaveBeenCalled(); - expect(entryFor(report, 'maya-agent')).toMatchObject({ reached: 'done', pendingToSweep: 0, lateEvents: 1 }); - const after = await AgentEvent.findById(late._id).lean(); - expect(after).toMatchObject({ status: 'delivered', deliveryNonce: null }); + expect(await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).toBe(1); + expect(dispatch).toHaveBeenCalledTimes(1); + expect((await AgentEvent.findById(late._id).lean()).status).toBe('delivered'); + }); + + test('while unfinished the sweep leaves it; once the run finishes, the sweep runs it', async () => { + let failed = false; + mockDeprovision.mockImplementation(async (name) => { + if (name === 'maya-agent' && !failed) { failed = true; throw new Error('worker down'); } + return true; + }); + await migrate({ apply: true, now: NOW }); + const late = await pendingFor(podB); + + expect(await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).toBe(0); + expect(dispatch).not.toHaveBeenCalled(); + + // The resumed run's step 4 sees it pending on a provisioned install and + // hands it to the delivered-path sweep; the script still runs no turn. + await migrate({ apply: true, now: new Date() }); + expect(dispatch).not.toHaveBeenCalled(); + expect(await AgentEvent.findById(late._id).lean()).toMatchObject({ status: 'delivered', deliveryNonce: null }); + }); }); + diff --git a/backend/routes/registry/tokens.ts b/backend/routes/registry/tokens.ts index f0c5f28c02..2030cde60c 100644 --- a/backend/routes/registry/tokens.ts +++ b/backend/routes/registry/tokens.ts @@ -210,16 +210,9 @@ class InProcessAgentTokenRefused extends Error { } // Any spelling: a type saved before the config-write guard (#2229) normalized -// it must not turn this refusal off. -const IN_PROCESS_RUNTIME = { - $or: [ - { 'config.runtime.runtimeType': { $regex: /^\s*native\s*$/i } }, - { - 'config.runtime.runtimeType': { $regex: /^\s*hosted\s*$/i }, - 'config.hosted.engine': { $regex: /^\s*native\s*$/i }, - }, - ], -}; +// it must not turn this refusal off. On the stored config, so an unfinished +// migration is refused too. +const { IN_PROCESS_RUNTIME_FILTER: IN_PROCESS_RUNTIME } = require('../../services/hostedEngine'); const runsInProcess = async (agentUser: any, installation: any): Promise => { const pairs: { agentName: string; instanceId: string }[] = []; diff --git a/backend/services/agentEventService.ts b/backend/services/agentEventService.ts index 7fa0ebcb46..bd04fa0371 100644 --- a/backend/services/agentEventService.ts +++ b/backend/services/agentEventService.ts @@ -2,7 +2,7 @@ import crypto from 'crypto'; import { Types } from 'mongoose'; import Task from '../models/Task'; import { postToPublicAddress } from './outboundUrlGuard'; -import { hostedRunsNative } from './hostedEngine'; +import { hostedRunsNative, IN_PROCESS_RUNTIME_FILTER } from './hostedEngine'; // eslint-disable-next-line global-require const AgentEvent = require('../models/AgentEvent'); @@ -56,6 +56,15 @@ const OBJECT_ID_HEX = /^[a-f\d]{24}$/i; const STUCK_REDISPATCH_BATCH = 50; const ABANDONED_RUN_MINUTES = 30; +// TASK-265 (Rhea on #2236): an in-process event is born 'delivered', so one +// left 'pending' for an agent that now runs in-process was routed before the +// agent moved in-process (the hosted migration: enqueue decides routing before +// the insert) and nothing polls it. The sweep runs those once they are a +// minute old, so it never races an enqueue, and leaves ones older than half +// an hour to the retention delete: a reply that late is no longer an answer. +const STRANDED_PENDING_MIN_AGE_MS = 60 * 1000; +const STRANDED_PENDING_MAX_AGE_MS = 30 * 60 * 1000; + // The migration on acknowledge() runs in two phases, and BOTH are in this // build so the compatibility mode has an end rather than an intention. // @@ -168,6 +177,8 @@ interface GarbageCollectResult { redispatchedInProcess?: number; ackedInProcess?: number; interruptedRuns?: number; + // TASK-265: pending events of agents that now run in-process, run once. + rescuedPendingInProcess?: number; } interface SessionSizeEntry { @@ -753,16 +764,18 @@ class AgentEventService { let redispatchedInProcess = 0; let ackedInProcess = 0; let interruptedRuns = 0; + let rescuedPendingInProcess = 0; try { ({ redispatched: redispatchedInProcess, acked: ackedInProcess } = await this.redispatchStuckInProcess( requeueThreshold, attemptCap, )); + rescuedPendingInProcess = await this.rescueStrandedPendingInProcess(new Date(now), attemptCap); interruptedRuns = await this.interruptAbandonedRuns(new Date(now - ABANDONED_RUN_MINUTES * 60 * 1000)); - if (redispatchedInProcess || ackedInProcess || interruptedRuns) { + if (redispatchedInProcess || ackedInProcess || interruptedRuns || rescuedPendingInProcess) { console.log( `[agent-event] in-process restart sweep: re-ran ${redispatchedInProcess}, acked ${ackedInProcess}, ` - + `marked ${interruptedRuns} abandoned runs interrupted`, + + `ran ${rescuedPendingInProcess} stranded pending, marked ${interruptedRuns} abandoned runs interrupted`, ); } } catch (err: unknown) { @@ -881,6 +894,7 @@ class AgentEventService { redispatchedInProcess, ackedInProcess, interruptedRuns, + rescuedPendingInProcess, }; } @@ -951,6 +965,58 @@ class AgentEventService { return { redispatched, acked }; } + /** + * Run, once, each recent 'pending' event whose agent now resolves + * in-process (see STRANDED_PENDING_*). Candidates are limited to identities + * with an in-process install, so a backlog of external agents' pending + * events cannot crowd them out; each is then decided by the same routing + * enqueue uses, with the engine in effect, so an unfinished migration is + * left alone. The claim is a compare-and-set from 'pending', so two sweeps + * (two replicas) or a late external poll run it at most once, and it counts + * as a delivery, so the attempt cap bounds a poison event. + */ + static async rescueStrandedPendingInProcess(now: Date, attemptCap: number): Promise { + const identities = await AgentInstallation.find({ status: 'active', ...IN_PROCESS_RUNTIME_FILTER }) + .select('agentName instanceId').lean() as { agentName?: string; instanceId?: string }[]; + const pairs = [...new Map(identities.map((row) => { + const pair = { agentName: String(row.agentName || '').toLowerCase(), instanceId: String(row.instanceId || 'default') }; + return [`${pair.agentName}:${pair.instanceId}`, pair] as const; + })).values()]; + if (!pairs.length) return 0; + const stranded = await AgentEvent.find({ + status: 'pending', + createdAt: { + $lt: new Date(now.getTime() - STRANDED_PENDING_MIN_AGE_MS), + $gte: new Date(now.getTime() - STRANDED_PENDING_MAX_AGE_MS), + }, + $and: [ + { $or: pairs }, + { $or: [{ attempts: { $lt: attemptCap } }, { attempts: { $exists: false } }] }, + ], + }).sort({ createdAt: 1 }).limit(STUCK_REDISPATCH_BATCH).lean() as EventDoc[]; + let rescued = 0; + for (const event of stranded) { + const agentName = String(event.agentName || ''); + const instanceId = String(event.instanceId || 'default'); + // eslint-disable-next-line no-await-in-loop -- one lookup per stranded event + const installation = await this.resolveInProcessInstallation(agentName, instanceId, event.podId) + .catch(() => null); + // Not in-process (yet): an external poller's event, or a migration not done. + if (!installation) continue; + const deliveryNonce = mintDeliveryNonce(); + // eslint-disable-next-line no-await-in-loop + const claimed = await AgentEvent.findOneAndUpdate( + { _id: event._id, status: 'pending' }, + { $set: { status: 'delivered', deliveredAt: new Date(), deliveryNonce }, $inc: { attempts: 1 } }, + { new: true }, + ) as EventDoc | null; + if (!claimed) continue; + this.dispatchInProcess(claimed, installation, agentName, instanceId, String(claimed.type || event.type || '')); + rescued += 1; + } + return rescued; + } + /** * AgentRuns still 'queued' or 'running' long after any turn could have * ended (a native turn is bounded at about two minutes) belong to a turn diff --git a/backend/services/hostedEngine.ts b/backend/services/hostedEngine.ts index d92298073e..0492fd1062 100644 --- a/backend/services/hostedEngine.ts +++ b/backend/services/hostedEngine.ts @@ -69,6 +69,24 @@ export const hostedEngineInEffect = (installation: any): HostedEngine => ( : HOSTED_ENGINE_DURABLE_OBJECT ); +/** + * A Mongo filter for installations whose STORED config runs in-process, in + * any spelling (a type saved before the config-write guard, #2229, normalized + * it). Broader than the engine in effect: an unfinished migration matches. + * The token issuer refuses on it by design (routes/registry/tokens.ts), and + * the stranded-event sweep uses it only to pick candidates, deciding each one + * with the in-effect engine (agentEventService). + */ +export const IN_PROCESS_RUNTIME_FILTER = { + $or: [ + { 'config.runtime.runtimeType': { $regex: /^\s*native\s*$/i } }, + { + 'config.runtime.runtimeType': { $regex: /^\s*hosted\s*$/i }, + 'config.hosted.engine': { $regex: /^\s*native\s*$/i }, + }, + ], +}; + /** The engine boundary shared by grant eligibility and agent-join checks. */ export const runsInNarrowedProcess = (installation: any): boolean => ( String(readRuntimeConfig(installation).runtimeType || '').trim().toLowerCase() === 'native' diff --git a/backend/services/hostedEngineMigration.ts b/backend/services/hostedEngineMigration.ts index b4b058506f..170a6fada2 100644 --- a/backend/services/hostedEngineMigration.ts +++ b/backend/services/hostedEngineMigration.ts @@ -34,10 +34,11 @@ * in-process sweep (marked delivered; #2226 re-runs it), so this * script never runs a turn. Others are closed with a reason. * 5. done. - * 6. late events: any that arrived while this run worked went to the - * Durable Object path (the rows were not native until 'done') - * and are handed off the same way; a later run skips a done - * identity, so this is the only pass that sees them. + * An event enqueued while this runs is routed before it is inserted, so it + * can land 'pending' after step 4 (the rows are not native until 'done') and + * a later run skips a done identity. A one-shot scan here cannot be made + * durable (Rhea on #2236); the recurring in-process sweep runs such events + * instead (AgentEventService.rescueStrandedPendingInProcess). * Every step is idempotent. A step that fails stops that identity there; the * report says which, and the next run resumes it. Nothing is reported done * that is not. @@ -75,8 +76,6 @@ export interface IdentityReport { tokensRevoked?: number; pendingToSweep: number; pendingClosed: number; - /** Events that arrived while this run worked, handed off after 'done'. */ - lateEvents?: number; error?: string; } @@ -262,21 +261,6 @@ export const migrateHostedInstallsToNative = async ( entry.reached = 'done'; } catch (error) { entry.error = `not recorded as done (${(error as Error).message}); run again`; - continue; - } - - // 6. Events that arrived during this run went to the Durable Object path, - // because the rows were not native until 'done'. Nothing polls that path - // now, and a later run skips a done identity, so hand them off here. - try { - // eslint-disable-next-line no-await-in-loop - const late = await AgentEvent.find({ agentName, instanceId, status: 'pending' }).lean(); - // eslint-disable-next-line no-await-in-loop - await handOff(late); - entry.lateEvents = late.length; - } catch (error) { - entry.error = `done, but events that arrived during the run were not handed off (${(error as Error).message}); ` - + 'a later run skips a done identity, so check its pending events by hand'; } } return report; From dcb5c5c8a2ad5fc43dab6396723e08ac0667c20d Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:59:35 -0700 Subject: [PATCH 8/8] fix(events): the stranded-event sweep selects only runnable targets and closes events past its ceiling Two HOLDs on #2236 at 2b69bd57: Rhea (starvation): the sweep's candidate query admitted stored-native identities whose migration is unfinished, sorted oldest-first and capped at 50 before deciding whether each event could run. Fifty such events filled every pass, so a completed agent's event aged out unrun. Sprint review (silent ceiling): an in-process agent's pending event older than 30 minutes was neither run nor closed. The question vanished with no record. Fixes: - The routing rule is extracted from resolveInProcessInstallation as inProcessInstallationFrom(row, sibling), one decision for enqueue and the sweep. - inProcessEventTargets: identities with a stored in-process install, then every active row of them (projections included, inheriting from the identity's most recent row that has a runtime), kept only where that rule runs it with the engine in effect. The pending queries are restricted to these (agent, instance, pod) targets, so a non-runnable event is never selected and cannot fill the batch. Each selected event is still re-checked by enqueue's routing before it runs. - Past the ceiling, a target's pending event is closed 'failed' with a reason instead of left pending. An unfinished migration's events are not targets, so the migration's own events step keeps owning them. Arms: - a 50-event unfinished backlog does not starve a completed agent's event; - past the ceiling an event is closed, never run, including at the attempt cap; - an unfinished migration's old event is left alone even when another target exists; - a projection pod inherits a completed migration's runtime. Mutations caught: targets unfiltered, ceiling close removed, ceiling close unscoped, projection inheritance dropped. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG --- .../agentEventService.deliveryNonce.test.js | 75 +++++++++- .../services/hostedEngineMigration.test.js | 5 +- backend/services/agentEventService.ts | 128 ++++++++++++++---- 3 files changed, 173 insertions(+), 35 deletions(-) diff --git a/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js b/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js index 46808a2e36..69ace026ec 100644 --- a/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js +++ b/backend/__tests__/unit/services/agentEventService.deliveryNonce.test.js @@ -627,9 +627,13 @@ describe('the sweep runs a pending event stranded by a move in-process', () => { ['an agent still on the Durable Object', HOSTED_DO, 2 * 60 * 1000, 0], ['a migration not yet done', MIGRATED('stopped'), 2 * 60 * 1000, 0], ['an event under a minute old', MIGRATED('done'), 10 * 1000, 0], - ['an event past half an hour', MIGRATED('done'), 31 * 60 * 1000, 0], ['an event at the attempt cap', MIGRATED('done'), 2 * 60 * 1000, 3], + // Not in-process yet: the migration's own events step owns it, at any age. + ['a migration not yet done, past half an hour', MIGRATED('stopped'), 31 * 60 * 1000, 0], ])('leaves alone %s', async (_label, config, ageMs, attempts) => { + // A runnable agent elsewhere, so the sweep has targets and its run and + // close passes both execute; this event must still be left alone. + await install('runnable-agent', MIGRATED('done'), new mongoose.Types.ObjectId()); await install(AGENT, config); const event = await seedEvent(); await AgentEvent.updateOne({ _id: event._id }, { $set: { attempts } }); @@ -638,7 +642,72 @@ describe('the sweep runs a pending event stranded by a move in-process', () => { const result = await sweep(); expect(result.rescuedPendingInProcess).toBe(0); + expect(result.closedPendingInProcess).toBe(0); expect(runAgent).not.toHaveBeenCalled(); + expect((await AgentEvent.findById(event._id).lean()).status).toBe('pending'); + }); + + // Sprint review on #2236: the 30-minute ceiling was terminal and silent: an + // event past it was neither run nor closed, so the question vanished with + // no record. Past the ceiling it is now closed with a reason, never run. + test.each([ + ['past half an hour', 31 * 60 * 1000, 0], + ['past half an hour and at the attempt cap', 31 * 60 * 1000, 3], + ])('closes an in-process agent\'s event %s, with a reason, and does not run it', async (_label, ageMs, attempts) => { + await install(AGENT, MIGRATED('done')); + const event = await seedEvent(); + await AgentEvent.updateOne({ _id: event._id }, { $set: { attempts } }); + await age(event._id, ageMs); + + const result = await sweep(); + + expect(result).toMatchObject({ rescuedPendingInProcess: 0, closedPendingInProcess: 1 }); + expect(runAgent).not.toHaveBeenCalled(); + const closed = await AgentEvent.findById(event._id).lean(); + expect(closed.status).toBe('failed'); + expect(closed.error).toMatch(/not run within 30 minutes/); + }); + + // Rhea on #2236: the batch cap used to apply before runnability, so 50 + // pending events of an unfinished migration filled every pass and a + // completed agent's event aged out unrun. Only runnable targets are + // selected now, so it runs on the first pass and the backlog is untouched. + test("an unfinished migration's backlog of 50 does not starve a completed agent's event", async () => { + const OTHER_POD = new mongoose.Types.ObjectId(); + await install('stuck-agent', MIGRATED('stopped'), OTHER_POD); + await install(AGENT, MIGRATED('done')); + const backlog = await AgentEvent.create(Array.from({ length: 50 }, () => ({ + agentName: 'stuck-agent', instanceId: INSTANCE, podId: OTHER_POD, type: 'chat.mention', payload: {}, status: 'pending', + }))); + await AgentEvent.collection.updateMany( + { _id: { $in: backlog.map((row) => row._id) } }, + { $set: { createdAt: new Date(Date.now() - 3 * 60 * 1000) } }, + ); + const event = await seedEvent(); + await age(event._id, 2 * 60 * 1000); + + const result = await sweep(); + + expect(result.rescuedPendingInProcess).toBe(1); + expect(runAgent).toHaveBeenCalledTimes(1); + expect((await AgentEvent.findById(event._id).lean()).status).not.toBe('pending'); + expect(await AgentEvent.countDocuments({ agentName: 'stuck-agent', status: 'pending', attempts: 0 })).toBe(50); + }); + + // A projection row (an agent-room install with no runtime of its own) + // routes like its identity's most recent row that has one, in enqueue and in + // the sweep's targets alike. + test("runs a stranded event in a projection pod that inherits a completed migration's runtime", async () => { + const SOURCE_POD = new mongoose.Types.ObjectId(); + await install(AGENT, MIGRATED('done'), SOURCE_POD); + await install(AGENT, { heartbeat: { enabled: false } }); + const event = await seedEvent(); + await age(event._id, 2 * 60 * 1000); + + const result = await sweep(); + + expect(result.rescuedPendingInProcess).toBe(1); + expect(runAgent).toHaveBeenCalledTimes(1); }); test('two sweeps racing (two replicas) run it once', async () => { @@ -651,7 +720,7 @@ describe('the sweep runs a pending event stranded by a move in-process', () => { AgentEventService.rescueStrandedPendingInProcess(new Date(), 3), ]); - expect(counts[0] + counts[1]).toBe(1); + expect(counts[0].rescued + counts[1].rescued).toBe(1); expect(runAgent).toHaveBeenCalledTimes(1); }); @@ -668,7 +737,7 @@ describe('the sweep runs a pending event stranded by a move in-process', () => { const event = await seedEvent(); await age(event._id, 2 * 60 * 1000); - expect(await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).toBe(1); + expect((await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).rescued).toBe(1); expect((await AgentEvent.findById(event._id).lean()).status).not.toBe('pending'); }); }); diff --git a/backend/__tests__/unit/services/hostedEngineMigration.test.js b/backend/__tests__/unit/services/hostedEngineMigration.test.js index fb2c132784..ac9e601981 100644 --- a/backend/__tests__/unit/services/hostedEngineMigration.test.js +++ b/backend/__tests__/unit/services/hostedEngineMigration.test.js @@ -316,7 +316,7 @@ describe('an event left pending by the move', () => { expect(entryFor(rerun, 'maya-agent')).toBeUndefined(); expect(dispatch).not.toHaveBeenCalled(); - expect(await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).toBe(1); + expect((await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).rescued).toBe(1); expect(dispatch).toHaveBeenCalledTimes(1); expect((await AgentEvent.findById(late._id).lean()).status).toBe('delivered'); }); @@ -330,7 +330,7 @@ describe('an event left pending by the move', () => { await migrate({ apply: true, now: NOW }); const late = await pendingFor(podB); - expect(await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).toBe(0); + expect((await AgentEventService.rescueStrandedPendingInProcess(new Date(), 3)).rescued).toBe(0); expect(dispatch).not.toHaveBeenCalled(); // The resumed run's step 4 sees it pending on a provisioned install and @@ -340,4 +340,3 @@ describe('an event left pending by the move', () => { expect(await AgentEvent.findById(late._id).lean()).toMatchObject({ status: 'delivered', deliveryNonce: null }); }); }); - diff --git a/backend/services/agentEventService.ts b/backend/services/agentEventService.ts index bd04fa0371..68c9290ca0 100644 --- a/backend/services/agentEventService.ts +++ b/backend/services/agentEventService.ts @@ -60,8 +60,9 @@ const ABANDONED_RUN_MINUTES = 30; // left 'pending' for an agent that now runs in-process was routed before the // agent moved in-process (the hosted migration: enqueue decides routing before // the insert) and nothing polls it. The sweep runs those once they are a -// minute old, so it never races an enqueue, and leaves ones older than half -// an hour to the retention delete: a reply that late is no longer an answer. +// minute old, so it never races an enqueue, and closes ones older than half +// an hour as 'failed' with a reason: a reply that late is no longer an answer, +// and a closed event says so where a pending one says nothing. const STRANDED_PENDING_MIN_AGE_MS = 60 * 1000; const STRANDED_PENDING_MAX_AGE_MS = 30 * 60 * 1000; @@ -177,8 +178,10 @@ interface GarbageCollectResult { redispatchedInProcess?: number; ackedInProcess?: number; interruptedRuns?: number; - // TASK-265: pending events of agents that now run in-process, run once. + // TASK-265: pending events of agents that now run in-process, run once, or + // closed with a reason once past the ceiling. rescuedPendingInProcess?: number; + closedPendingInProcess?: number; } interface SessionSizeEntry { @@ -765,17 +768,22 @@ class AgentEventService { let ackedInProcess = 0; let interruptedRuns = 0; let rescuedPendingInProcess = 0; + let closedPendingInProcess = 0; try { ({ redispatched: redispatchedInProcess, acked: ackedInProcess } = await this.redispatchStuckInProcess( requeueThreshold, attemptCap, )); - rescuedPendingInProcess = await this.rescueStrandedPendingInProcess(new Date(now), attemptCap); + ({ rescued: rescuedPendingInProcess, closed: closedPendingInProcess } = await this.rescueStrandedPendingInProcess( + new Date(now), + attemptCap, + )); interruptedRuns = await this.interruptAbandonedRuns(new Date(now - ABANDONED_RUN_MINUTES * 60 * 1000)); - if (redispatchedInProcess || ackedInProcess || interruptedRuns || rescuedPendingInProcess) { + if (redispatchedInProcess || ackedInProcess || interruptedRuns || rescuedPendingInProcess || closedPendingInProcess) { console.log( `[agent-event] in-process restart sweep: re-ran ${redispatchedInProcess}, acked ${ackedInProcess}, ` - + `ran ${rescuedPendingInProcess} stranded pending, marked ${interruptedRuns} abandoned runs interrupted`, + + `ran ${rescuedPendingInProcess} stranded pending, closed ${closedPendingInProcess} past the ceiling, ` + + `marked ${interruptedRuns} abandoned runs interrupted`, ); } } catch (err: unknown) { @@ -895,6 +903,7 @@ class AgentEventService { ackedInProcess, interruptedRuns, rescuedPendingInProcess, + closedPendingInProcess, }; } @@ -966,31 +975,77 @@ class AgentEventService { } /** - * Run, once, each recent 'pending' event whose agent now resolves - * in-process (see STRANDED_PENDING_*). Candidates are limited to identities - * with an in-process install, so a backlog of external agents' pending - * events cannot crowd them out; each is then decided by the same routing - * enqueue uses, with the engine in effect, so an unfinished migration is - * left alone. The claim is a compare-and-set from 'pending', so two sweeps - * (two replicas) or a late external poll run it at most once, and it counts - * as a delivery, so the attempt cap bounds a poison event. + * The (agent, instance, pod) targets whose events enqueue would run + * in-process right now. Starts from identities with a stored in-process + * install (the token issuer's filter), loads every active row of those + * identities (projections included), and keeps only the rows the routing + * rule itself runs, with the engine in effect. An unfinished migration's + * rows are therefore not targets, so their events can never fill the + * sweep's batch and starve a runnable agent's event (Rhea on #2236). */ - static async rescueStrandedPendingInProcess(now: Date, attemptCap: number): Promise { + static async inProcessEventTargets(): Promise<{ agentName: string; instanceId: string; podId: unknown }[]> { const identities = await AgentInstallation.find({ status: 'active', ...IN_PROCESS_RUNTIME_FILTER }) .select('agentName instanceId').lean() as { agentName?: string; instanceId?: string }[]; const pairs = [...new Map(identities.map((row) => { const pair = { agentName: String(row.agentName || '').toLowerCase(), instanceId: String(row.instanceId || 'default') }; return [`${pair.agentName}:${pair.instanceId}`, pair] as const; })).values()]; - if (!pairs.length) return 0; + if (!pairs.length) return []; + const rows = await AgentInstallation.find({ status: 'active', $or: pairs }) + .sort({ createdAt: -1 }).lean() as InstallationDoc[]; + const identityKey = (row: InstallationDoc) => `${String(row.agentName || '').toLowerCase()}:${String(row.instanceId || 'default')}`; + // The sibling a projection inherits from: the identity's most recent row + // that has a runtime (rows are newest first), as resolveInProcessInstallation finds it. + const siblings = new Map(); + for (const row of rows) { + const runtime = normalizeConfig(row.config)?.runtime as Record | undefined; + if (runtime?.runtimeType !== undefined && !siblings.has(identityKey(row))) siblings.set(identityKey(row), row); + } + return rows + .filter((row) => this.inProcessInstallationFrom({ ...row }, siblings.get(identityKey(row)) || null)) + .map((row) => ({ + agentName: String(row.agentName || '').toLowerCase(), + instanceId: String(row.instanceId || 'default'), + podId: row.podId, + })); + } + + /** + * Pending events whose agent now runs in-process, which nothing polls (see + * STRANDED_PENDING_*). One past the ceiling is closed 'failed' with a + * reason, so the dropped question is visible rather than parked (sprint + * review on #2236). A recent one is run once: the claim is a + * compare-and-set from 'pending', so two sweeps (two replicas) or a late + * external poll run it at most once, and it counts as a delivery, so the + * attempt cap bounds a poison event. Only targets that would run are + * selected, and each is re-checked by enqueue's own routing before it runs. + */ + static async rescueStrandedPendingInProcess( + now: Date, + attemptCap: number, + ): Promise<{ rescued: number; closed: number }> { + const targets = await this.inProcessEventTargets(); + if (!targets.length) return { rescued: 0, closed: 0 }; + const targetFilter = { $or: targets }; + const ceiling = new Date(now.getTime() - STRANDED_PENDING_MAX_AGE_MS); + const closedResult = await AgentEvent.updateMany( + { status: 'pending', createdAt: { $lt: ceiling }, ...targetFilter }, + { + $set: { + status: 'failed', + error: 'stranded: this agent runs in-process and the event was not run within 30 minutes of being ' + + 'enqueued; closed so it does not sit pending unanswered', + }, + }, + ) as { modifiedCount?: number }; const stranded = await AgentEvent.find({ status: 'pending', createdAt: { $lt: new Date(now.getTime() - STRANDED_PENDING_MIN_AGE_MS), - $gte: new Date(now.getTime() - STRANDED_PENDING_MAX_AGE_MS), + $gte: ceiling, }, $and: [ - { $or: pairs }, + targetFilter, { $or: [{ attempts: { $lt: attemptCap } }, { attempts: { $exists: false } }] }, ], }).sort({ createdAt: 1 }).limit(STUCK_REDISPATCH_BATCH).lean() as EventDoc[]; @@ -1001,7 +1056,7 @@ class AgentEventService { // eslint-disable-next-line no-await-in-loop -- one lookup per stranded event const installation = await this.resolveInProcessInstallation(agentName, instanceId, event.podId) .catch(() => null); - // Not in-process (yet): an external poller's event, or a migration not done. + // Changed since the targets were read (a deprovision, an install removed). if (!installation) continue; const deliveryNonce = mintDeliveryNonce(); // eslint-disable-next-line no-await-in-loop @@ -1014,7 +1069,7 @@ class AgentEventService { this.dispatchInProcess(claimed, installation, agentName, instanceId, String(claimed.type || event.type || '')); rescued += 1; } - return rescued; + return { rescued, closed: closedResult?.modifiedCount || 0 }; } /** @@ -1224,17 +1279,32 @@ class AgentEventService { status: 'active', }).lean() as InstallationDoc | null; if (!installationDoc) return null; + const ownRuntime = (normalizeConfig(installationDoc.config)?.runtime || {}) as Record; + // A projection written without its runtime (agent-room installs before + // 2026-09-03 did this) routes like the agent it projects: the identity's + // most recent row that has one. + const sibling = ownRuntime.runtimeType ? null : await AgentInstallation.findOne({ + agentName: agentName.toLowerCase(), + instanceId, + status: 'active', + 'config.runtime.runtimeType': { $exists: true }, + }).sort({ createdAt: -1 }).lean() as InstallationDoc | null; + return this.inProcessInstallationFrom(installationDoc, sibling); + } + + /** + * The routing rule itself, without the lookups: the install doc the native + * runtime runs for `installationDoc` (merged with `sibling`'s config when the + * row is a projection with no runtime of its own), or null. Shared by + * `resolveInProcessInstallation` and the stranded-event sweep, so the sweep + * can never select an event that enqueue would not run (Rhea on #2236). + */ + static inProcessInstallationFrom( + installationDoc: InstallationDoc, + sibling: InstallationDoc | null, + ): InstallationDoc | null { let installationRuntimeCfg = (normalizeConfig(installationDoc.config)?.runtime || {}) as Record; if (!installationRuntimeCfg.runtimeType) { - // A projection written without its runtime (agent-room installs - // before 2026-09-03 did this) must still route like the agent it - // projects, or the wake parks in the external queue forever. - const sibling = await AgentInstallation.findOne({ - agentName: agentName.toLowerCase(), - instanceId, - status: 'active', - 'config.runtime.runtimeType': { $exists: true }, - }).sort({ createdAt: -1 }).lean() as InstallationDoc | null; const siblingCfg = (sibling && normalizeConfig(sibling.config)) || null; const siblingRuntime = siblingCfg?.runtime as Record | undefined; if (siblingRuntime?.runtimeType) {