fix: fence loopback admission on the claimed config revision

B6. The loopback POST carried only scheduleId/scheduledFor/clientRequestId/
newConversationId, and the admission boundary validated EXISTENCE only (isScheduleLive).
So an owner edit landing between the claim and persistence was not fenced where it
matters: the fire had already been dispatched with the OLD prompt/agent, and the run
would then persist into the edited schedule's history as though it had run under the new
config. Nothing downstream caught it, because by then the schedule legitimately exists.

The fire now carries `scheduleConfigRevision` (the generation it was CLAIMED under) and
isScheduleLive revalidates it BEFORE any message is written, refusing the fire when the
owner has since edited. This composes with the existing claimToken/lease fencing rather
than duplicating it: claimToken rotates on every lease acquisition and so cannot
distinguish an owner edit from a normal re-claim, whereas configRevision moves only on
owner config mutation.

Absent on either side disables the fence, so pre-existing schedules and in-flight fires
from before this change keep working with no migration.

Tests: the revision is present on the POST body; admission admits a matching revision,
REFUSES one an owner edit has moved on, refuses a deleted schedule regardless, and stays
permissive when either side has no revision.
This commit is contained in:
Danny Avila 2026-07-23 09:02:17 -04:00
parent 94d400e3ae
commit 17534d13af
5 changed files with 87 additions and 4 deletions

View file

@ -231,6 +231,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit
responseMessageId: editedResponseMessageId = null,
scheduleId: bodyScheduleId = null,
scheduledFor: bodyScheduledFor = null,
scheduleConfigRevision: bodyScheduleConfigRevision = null,
} = req.body;
// Only honor schedule bookkeeping fields on a server-minted scheduled fire
@ -247,6 +248,9 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit
req._isScheduledFire = isScheduledFire;
const scheduleId = isScheduledFire ? bodyScheduleId : null;
const scheduledFor = isScheduledFire ? bodyScheduledFor : null;
const scheduleConfigRevision = isScheduledFire
? (bodyScheduleConfigRevision ?? undefined)
: undefined;
const userId = req.user.id;
@ -404,7 +408,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit
// affects it.
if (scheduleId) {
await GenerationJobManager.updateMetadata(streamId, { scheduleId, scheduledFor });
if (!(await isScheduleLive(scheduleId))) {
if (!(await isScheduleLive(scheduleId, scheduleConfigRevision))) {
logger.info(
`[AgentController] Scheduled fire aborted before start; schedule ${scheduleId} no longer active`,
);

View file

@ -222,6 +222,22 @@ describe('fireSchedule', () => {
expect([...runs.values()][0].status).toBe('started');
});
it('carries the claimed config revision on the loopback POST', async () => {
const { methods } = makeMethods();
mockFetch(async () => okResponse());
await fireSchedule(
makeDeps(methods),
makeSchedule({ configRevision: 7 } as never),
LIMITS,
dueAt(),
);
const body = JSON.parse((global.fetch as unknown as jest.Mock).mock.calls[0][1].body as string);
// The admission boundary revalidates this before persisting anything, so an owner
// edit landing in the claim -> persistence window is refused rather than written
// into the edited schedule's history.
expect(body.scheduleConfigRevision).toBe(7);
});
it('records a definite HTTP rejection as error', async () => {
const { methods, calls } = makeMethods();
mockFetch(async () => ({ ok: false, status: 500, text: async () => 'boom' }) as Response);

View file

@ -130,6 +130,13 @@ async function postChatMessageInner(
// conversation's job by this id instead of mislabeling the run an orphan.
newConversationId: conversationId,
clientRequestId: buildFireClientRequestId(schedule.id, scheduledFor),
// The owner-config generation this fire was CLAIMED under. The admission
// boundary revalidates it before persisting anything, so an owner edit landing
// in the claim -> persistence window cannot have its old prompt/agent written
// into the edited schedule's history.
...(typeof schedule.configRevision === 'number'
? { scheduleConfigRevision: schedule.configRevision }
: {}),
...(files.length > 0 ? { files } : {}),
}),
});

View file

@ -148,3 +148,39 @@ describe('global kill switch', () => {
expect(await service.engineDeps.isGloballyDisabled()).toBe(false);
});
});
describe('admission revision fence', () => {
const noRuns = () => jest.fn<Promise<ActiveRun[]>, [string]>().mockResolvedValue([]);
function serviceWithSchedule(schedule: { configRevision?: number } | null) {
const service = makeService(noRuns());
(
service.engineDeps.methods as unknown as {
getScheduleById: jest.Mock;
}
).getScheduleById = jest.fn(async () => schedule);
return service;
}
it('admits when the claimed revision still matches', async () => {
const service = serviceWithSchedule({ configRevision: 3 });
expect(await service.isScheduleLive('sched-1', 3)).toBe(true);
});
it('REFUSES when an owner edit moved the revision on after the claim', async () => {
// The fire was claimed under revision 3; the owner edited since (now 4). Persisting
// would write the OLD prompt/agent into the edited schedule's history.
const service = serviceWithSchedule({ configRevision: 4 });
expect(await service.isScheduleLive('sched-1', 3)).toBe(false);
});
it('refuses a schedule that is gone regardless of revision', async () => {
const service = serviceWithSchedule(null);
expect(await service.isScheduleLive('sched-1', 3)).toBe(false);
});
it('stays permissive when either side has no revision (pre-existing rows)', async () => {
expect(await serviceWithSchedule({}).isScheduleLive('sched-1', 3)).toBe(true);
expect(await serviceWithSchedule({ configRevision: 4 }).isScheduleLive('sched-1')).toBe(true);
});
});

View file

@ -121,7 +121,7 @@ export interface SchedulesService {
* the reservation row exists but the job did not yet, so the deletion's abort
* missed it) aborting before any messages are persisted.
*/
isScheduleLive: (scheduleId: string) => Promise<boolean>;
isScheduleLive: (scheduleId: string, expectedConfigRevision?: number) => Promise<boolean>;
/**
* Acquires the durable RESUMING lease for a HITL resume, BEFORE the approval claim
* (so a deferral leaves the approval claimable). Per-schedule overlap is enforced by
@ -532,11 +532,31 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
return false;
}
async function isScheduleLive(scheduleId: string): Promise<boolean> {
async function isScheduleLive(
scheduleId: string,
expectedConfigRevision?: number,
): Promise<boolean> {
if (!scheduleId) {
return false;
}
return (await methods.getScheduleById(scheduleId)) != null;
const schedule = await methods.getScheduleById(scheduleId);
if (schedule == null) {
return false;
}
// REVISION FENCE at the admission boundary. Existence alone is not enough: an owner
// edit landing between the claim and this point means the dispatched prompt/agent
// came from a config the owner has since replaced, and nothing downstream would
// catch it because the run persists under the NEW schedule. Refuse before any
// message is written. Absent on either side disables the fence, so pre-existing
// schedules and older fires keep working.
if (
expectedConfigRevision != null &&
typeof schedule.configRevision === 'number' &&
schedule.configRevision !== expectedConfigRevision
) {
return false;
}
return true;
}
/**