feat(vibe): reconcile mutable session previews in playback
This commit is contained in:
@@ -22,6 +22,7 @@ async function appWithCoordinator(identityResolver: VibeIdentityResolver = () =>
|
||||
appendEvent: vi.fn().mockResolvedValue({ ...response(), event: { id: 'event-1' }, idempotent: false }),
|
||||
end: vi.fn().mockResolvedValue(response()),
|
||||
serveNext: vi.fn().mockResolvedValue(response()),
|
||||
advancePastUnplayable: vi.fn().mockResolvedValue(response()),
|
||||
} as any;
|
||||
const app = Fastify();
|
||||
await app.register(vibeSessionsRoutes, { coordinator, identityResolver });
|
||||
@@ -133,4 +134,31 @@ describe('durable Vibe session routes', () => {
|
||||
expect(coordinator.serveNext).toHaveBeenCalledTimes(1);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('uses the explicit versioned advancement protocol for a served unplayable item', async () => {
|
||||
const { app, coordinator } = await appWithCoordinator();
|
||||
const result = await app.inject({
|
||||
method: 'POST', url: `/v2/vibe/sessions/${SESSION_ID}/next`,
|
||||
payload: {
|
||||
expectedPlanVersion: 2,
|
||||
unplayable: {
|
||||
eventId: '33333333-3333-4333-8333-333333333333',
|
||||
planVersionId: '44444444-4444-4444-8444-444444444444',
|
||||
ordinal: 0,
|
||||
trackId: TRACK_ID,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.statusCode).toBe(200);
|
||||
expect(coordinator.advancePastUnplayable).toHaveBeenCalledWith(USER_ID, SESSION_ID, {
|
||||
expectedPlanVersion: 2,
|
||||
eventId: '33333333-3333-4333-8333-333333333333',
|
||||
planVersionId: '44444444-4444-4444-8444-444444444444',
|
||||
ordinal: 0,
|
||||
trackId: TRACK_ID,
|
||||
});
|
||||
expect(coordinator.serveNext).not.toHaveBeenCalled();
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -156,8 +156,33 @@ export default async function vibeSessionsRoutes(
|
||||
&& (!Number.isInteger(body.expectedPlanVersion) || (body.expectedPlanVersion as number) < 1)) {
|
||||
return reply.code(400).send({ error: 'expectedPlanVersion must be a positive integer' });
|
||||
}
|
||||
const unplayable = body.unplayable;
|
||||
if (unplayable !== undefined && !isObject(unplayable)) {
|
||||
return reply.code(400).send({ error: 'unplayable must be an object' });
|
||||
}
|
||||
if (isObject(unplayable)) {
|
||||
if (body.expectedPlanVersion === undefined) {
|
||||
return reply.code(400).send({ error: 'expectedPlanVersion is required when advancing an unplayable item' });
|
||||
}
|
||||
if (!validUuid(unplayable.eventId)
|
||||
|| !validUuid(unplayable.planVersionId)
|
||||
|| !validUuid(unplayable.trackId)
|
||||
|| !Number.isInteger(unplayable.ordinal)
|
||||
|| (unplayable.ordinal as number) < 0) {
|
||||
return reply.code(400).send({ error: 'unplayable requires UUID eventId, planVersionId, trackId and a non-negative integer ordinal' });
|
||||
}
|
||||
}
|
||||
try {
|
||||
const expectedPlanVersion = body.expectedPlanVersion as number | undefined;
|
||||
if (isObject(unplayable)) {
|
||||
return reply.send(await coordinator.advancePastUnplayable(userId, sessionId, {
|
||||
expectedPlanVersion: expectedPlanVersion as number,
|
||||
eventId: unplayable.eventId as string,
|
||||
planVersionId: unplayable.planVersionId as string,
|
||||
ordinal: unplayable.ordinal as number,
|
||||
trackId: unplayable.trackId as string,
|
||||
}));
|
||||
}
|
||||
return reply.send(expectedPlanVersion === undefined
|
||||
? await coordinator.serveNext(userId, sessionId)
|
||||
: await coordinator.serveNext(userId, sessionId, expectedPlanVersion));
|
||||
|
||||
@@ -361,6 +361,95 @@ describe('DbService v2 methods', () => {
|
||||
expect(clientQuery.mock.calls.map(([sql]) => sql)).not.toContain(expect.stringContaining('SET committed = true'));
|
||||
});
|
||||
|
||||
it('advances a served unplayable item with a separate idempotent event and commits one replacement', async () => {
|
||||
const { service, clientQuery } = makeTransactionalService();
|
||||
const replacement = {
|
||||
plan_version_id: 'plan-1', ordinal: 1, track_id: 'track-2', slot_role: null,
|
||||
candidate_source: 'discovery', score: 0.8, score_breakdown: {}, explanation: [], committed: true,
|
||||
};
|
||||
clientQuery
|
||||
.mockResolvedValueOnce({ rows: [] }) // BEGIN
|
||||
.mockResolvedValueOnce({ rows: [{ status: 'active' }] }) // session lock
|
||||
.mockResolvedValueOnce({ rows: [{ id: 'plan-1', version: 1 }] }) // latest plan
|
||||
.mockResolvedValueOnce({ rows: [] }) // no prior playback_error event
|
||||
.mockResolvedValueOnce({ rows: [{ track_id: 'track-1', ordinal: 0 }] }) // current served cursor
|
||||
.mockResolvedValueOnce({ rows: [{ id: 'error-1' }] }) // playback_error event
|
||||
.mockResolvedValueOnce({ rows: [replacement] }) // commit replacement
|
||||
.mockResolvedValueOnce({ rows: [] }) // replacement track_served event
|
||||
.mockResolvedValueOnce({ rows: [] }) // playback_error result payload
|
||||
.mockResolvedValueOnce({ rows: [] }) // session timestamp
|
||||
.mockResolvedValueOnce({ rows: [] }); // COMMIT
|
||||
|
||||
await expect(service.advancePastUnplayableVibePlanItem('session-1', 'user-1', {
|
||||
expectedPlanVersion: 1,
|
||||
planVersionId: 'plan-1',
|
||||
ordinal: 0,
|
||||
trackId: 'track-1',
|
||||
eventId: 'event-1',
|
||||
})).resolves.toEqual({ item: replacement, stale: false });
|
||||
|
||||
expect(clientQuery.mock.calls[5][0]).toContain("'playback_error'");
|
||||
expect(clientQuery.mock.calls[6][0]).toContain('SET committed = true');
|
||||
expect(clientQuery.mock.calls[7][0]).toContain("'track_served'");
|
||||
expect(clientQuery.mock.calls[8][0]).toContain('UPDATE vibe_events SET payload');
|
||||
});
|
||||
|
||||
it('refuses an old served cursor when events share a timestamp by ordering the immutable plan ordinal', async () => {
|
||||
const { service, clientQuery } = makeTransactionalService();
|
||||
clientQuery
|
||||
.mockResolvedValueOnce({ rows: [] }) // BEGIN
|
||||
.mockResolvedValueOnce({ rows: [{ status: 'active' }] }) // session lock
|
||||
.mockResolvedValueOnce({ rows: [{ id: 'plan-1', version: 1 }] }) // latest plan
|
||||
.mockResolvedValueOnce({ rows: [] }) // no prior playback_error event
|
||||
// The lower-ordinal event can have a lexically greater UUID at the
|
||||
// same occurred_at. The cursor must still be the highest immutable
|
||||
// plan ordinal, never whichever UUID sorts last.
|
||||
.mockResolvedValueOnce({ rows: [{ track_id: 'track-2', ordinal: 1 }] }) // current served cursor
|
||||
.mockResolvedValueOnce({ rows: [] }); // ROLLBACK
|
||||
|
||||
await expect(service.advancePastUnplayableVibePlanItem('session-1', 'user-1', {
|
||||
expectedPlanVersion: 1,
|
||||
planVersionId: 'plan-1',
|
||||
ordinal: 0,
|
||||
trackId: 'track-1',
|
||||
eventId: 'event-1',
|
||||
})).rejects.toThrow('not the current served cursor');
|
||||
|
||||
expect(clientQuery.mock.calls[4][0]).toContain('JOIN vibe_plan_items');
|
||||
expect(clientQuery.mock.calls[4][0]).toContain('ORDER BY i.ordinal DESC');
|
||||
expect(clientQuery.mock.calls[4][0]).not.toContain('id DESC');
|
||||
expect(clientQuery.mock.calls.map(([sql]) => sql)).not.toContain(expect.stringContaining('SET committed = true'));
|
||||
});
|
||||
|
||||
it('retries an unplayable advancement with the same event id without consuming another item', async () => {
|
||||
const { service, clientQuery } = makeTransactionalService();
|
||||
const replacement = {
|
||||
plan_version_id: 'plan-1', ordinal: 1, track_id: 'track-2', slot_role: null,
|
||||
candidate_source: 'discovery', score: 0.8, score_breakdown: {}, explanation: [], committed: true,
|
||||
};
|
||||
clientQuery
|
||||
.mockResolvedValueOnce({ rows: [] }) // BEGIN
|
||||
.mockResolvedValueOnce({ rows: [{ status: 'active' }] }) // session lock
|
||||
.mockResolvedValueOnce({ rows: [{ id: 'plan-1', version: 1 }] }) // latest plan
|
||||
.mockResolvedValueOnce({ rows: [{ type: 'playback_error', payload: {
|
||||
planVersionId: 'plan-1', ordinal: 0, trackId: 'track-1',
|
||||
advancedTo: { planVersionId: 'plan-1', ordinal: 1 },
|
||||
} }] }) // prior explicit advancement
|
||||
.mockResolvedValueOnce({ rows: [replacement] }) // canonical replacement
|
||||
.mockResolvedValueOnce({ rows: [] }); // COMMIT
|
||||
|
||||
await expect(service.advancePastUnplayableVibePlanItem('session-1', 'user-1', {
|
||||
expectedPlanVersion: 1,
|
||||
planVersionId: 'plan-1',
|
||||
ordinal: 0,
|
||||
trackId: 'track-1',
|
||||
eventId: 'event-1',
|
||||
})).resolves.toEqual({ item: replacement, stale: false });
|
||||
|
||||
expect(clientQuery.mock.calls.map(([sql]) => sql)).not.toContain(expect.stringContaining('SET committed = true'));
|
||||
expect(clientQuery.mock.calls).toHaveLength(6);
|
||||
});
|
||||
|
||||
it('reads the latest revision and reconstructs ordered plan items', async () => {
|
||||
const { service, mockQuery } = makeService();
|
||||
mockQuery.mockResolvedValue({ rows: [{
|
||||
|
||||
@@ -2073,6 +2073,153 @@ export class DbService {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Advance an already-served item which the player could not resolve (for
|
||||
* example, its file was hidden after the revision was published). Unlike a
|
||||
* normal version-aware /next retry, this has a distinct client event id and
|
||||
* therefore intentionally moves beyond the item previously served for that
|
||||
* revision. The event records both the rejected item and the replacement so
|
||||
* a lost response can be retried without consuming another plan item.
|
||||
*/
|
||||
async advancePastUnplayableVibePlanItem(
|
||||
sessionId: string,
|
||||
userId: string,
|
||||
input: {
|
||||
expectedPlanVersion: number;
|
||||
planVersionId: string;
|
||||
ordinal: number;
|
||||
trackId: string;
|
||||
eventId: string;
|
||||
},
|
||||
): Promise<{ item: VibePlanItem | null; stale: boolean }> {
|
||||
return this.withTransaction(async (client) => {
|
||||
const session = await client.query(
|
||||
`SELECT status FROM vibe_sessions WHERE id = $1 AND user_id = $2 FOR UPDATE`,
|
||||
[sessionId, userId],
|
||||
);
|
||||
const sessionRow = session.rows[0] as Pick<VibeSession, 'status'> | undefined;
|
||||
if (!sessionRow) throw new Error('Vibe session was not found or is not owned by this user');
|
||||
|
||||
const latest = await client.query(
|
||||
`SELECT id, version FROM vibe_plan_versions WHERE session_id = $1 ORDER BY version DESC LIMIT 1 FOR UPDATE`,
|
||||
[sessionId],
|
||||
);
|
||||
const plan = latest.rows[0] as Pick<VibePlan, 'id' | 'version'> | undefined;
|
||||
if (!plan) return { item: null, stale: false };
|
||||
if (plan.version !== input.expectedPlanVersion || plan.id !== input.planVersionId) {
|
||||
return { item: null, stale: true };
|
||||
}
|
||||
|
||||
// Idempotency is scoped to this explicit advancement operation, rather
|
||||
// than overloading the version-aware /next retry which must keep
|
||||
// returning the originally served item.
|
||||
const prior = await client.query(
|
||||
`SELECT type, payload
|
||||
FROM vibe_events
|
||||
WHERE session_id = $1 AND client_event_id = $2::uuid`,
|
||||
[sessionId, input.eventId],
|
||||
);
|
||||
const priorEvent = prior.rows[0] as Pick<VibeEvent, 'type' | 'payload'> | undefined;
|
||||
if (priorEvent && priorEvent.type !== 'playback_error') {
|
||||
throw new Error('Vibe client event id was already used for a different event');
|
||||
}
|
||||
const priorPayload = priorEvent?.payload as Record<string, unknown> | undefined;
|
||||
if (priorPayload) {
|
||||
if (priorPayload.planVersionId !== input.planVersionId
|
||||
|| priorPayload.ordinal !== input.ordinal
|
||||
|| priorPayload.trackId !== input.trackId) {
|
||||
throw new Error('Vibe playback-error event does not match the served plan item');
|
||||
}
|
||||
const advancedTo = priorPayload.advancedTo as { planVersionId?: unknown; ordinal?: unknown } | null | undefined;
|
||||
if (!advancedTo || typeof advancedTo.planVersionId !== 'string' || !Number.isInteger(advancedTo.ordinal)) {
|
||||
return { item: null, stale: false };
|
||||
}
|
||||
const replacement = await client.query(
|
||||
`SELECT * FROM vibe_plan_items WHERE plan_version_id = $1 AND ordinal = $2`,
|
||||
[advancedTo.planVersionId, advancedTo.ordinal],
|
||||
);
|
||||
return { item: (replacement.rows[0] as VibePlanItem | undefined) ?? null, stale: false };
|
||||
}
|
||||
|
||||
if (sessionRow.status !== 'active') {
|
||||
throw new Error(`Cannot record a new event for ${sessionRow.status} Vibe session`);
|
||||
}
|
||||
|
||||
// A client may advance only the cursor it was just served. Checking for
|
||||
// any historical serve event would let an old version-aware /next
|
||||
// response consume whichever future item happens to be uncommitted.
|
||||
const current = await client.query(
|
||||
`SELECT i.track_id, i.ordinal
|
||||
FROM vibe_events e
|
||||
JOIN vibe_plan_items i
|
||||
ON i.plan_version_id = $3::uuid
|
||||
AND i.ordinal = (e.payload->>'ordinal')::integer
|
||||
AND i.track_id = e.track_id
|
||||
WHERE e.session_id = $1
|
||||
AND e.type = 'track_served'
|
||||
AND e.payload->>'planVersion' = $2::text
|
||||
AND e.payload->>'planVersionId' = $3
|
||||
ORDER BY i.ordinal DESC
|
||||
LIMIT 1`,
|
||||
[sessionId, input.expectedPlanVersion, input.planVersionId],
|
||||
);
|
||||
const currentCursor = current.rows[0] as Pick<VibePlanItem, 'track_id' | 'ordinal'> | undefined;
|
||||
if (currentCursor?.track_id !== input.trackId || currentCursor.ordinal !== input.ordinal) {
|
||||
throw new Error('Vibe plan item is not the current served cursor for this session revision');
|
||||
}
|
||||
|
||||
const payload = {
|
||||
kind: 'unplayable_plan_item',
|
||||
planVersion: input.expectedPlanVersion,
|
||||
planVersionId: input.planVersionId,
|
||||
ordinal: input.ordinal,
|
||||
trackId: input.trackId,
|
||||
};
|
||||
const playbackError = await client.query(
|
||||
`INSERT INTO vibe_events (client_event_id, session_id, user_id, track_id, type, occurred_at, payload)
|
||||
VALUES ($1::uuid, $2, $3, $4::uuid, 'playback_error', NOW(), $5::jsonb)
|
||||
RETURNING id`,
|
||||
[input.eventId, sessionId, userId, input.trackId, JSON.stringify(payload)],
|
||||
);
|
||||
const eventId = playbackError.rows[0]?.id as string | undefined;
|
||||
if (!eventId) throw new Error('Vibe playback-error event could not be recorded');
|
||||
|
||||
const item = await client.query(
|
||||
`WITH next_item AS (
|
||||
SELECT i.plan_version_id, i.ordinal FROM vibe_plan_items i
|
||||
WHERE i.plan_version_id = $1 AND NOT i.committed ORDER BY i.ordinal ASC LIMIT 1 FOR UPDATE
|
||||
)
|
||||
UPDATE vibe_plan_items i SET committed = true
|
||||
FROM next_item n WHERE i.plan_version_id = n.plan_version_id AND i.ordinal = n.ordinal
|
||||
RETURNING i.*`,
|
||||
[plan.id],
|
||||
);
|
||||
const replacement = item.rows[0] as VibePlanItem | undefined;
|
||||
if (replacement) {
|
||||
await client.query(
|
||||
`INSERT INTO vibe_events (session_id, user_id, track_id, type, occurred_at, payload)
|
||||
VALUES ($1, $2, $3, 'track_served', NOW(), $4::jsonb)`,
|
||||
[sessionId, userId, replacement.track_id, JSON.stringify({
|
||||
planVersion: plan.version,
|
||||
planVersionId: replacement.plan_version_id,
|
||||
ordinal: replacement.ordinal,
|
||||
})],
|
||||
);
|
||||
}
|
||||
await client.query(
|
||||
`UPDATE vibe_events SET payload = $2::jsonb WHERE id = $1`,
|
||||
[eventId, JSON.stringify({
|
||||
...payload,
|
||||
advancedTo: replacement
|
||||
? { planVersionId: replacement.plan_version_id, ordinal: replacement.ordinal }
|
||||
: null,
|
||||
})],
|
||||
);
|
||||
await client.query(`UPDATE vibe_sessions SET last_event_at = NOW() WHERE id = $1`, [sessionId]);
|
||||
return { item: replacement ?? null, stale: false };
|
||||
});
|
||||
}
|
||||
|
||||
/** Read a specific plan revision, or the latest revision for a session. */
|
||||
async getVibePlan(sessionId: string, userId: string, version?: number): Promise<VibePlan | null> {
|
||||
const res = await this.pgClient.query(
|
||||
|
||||
@@ -47,6 +47,7 @@ function setup() {
|
||||
endVibeSessionWithEvent: vi.fn().mockResolvedValue({ session: session('ended'), ended: true }),
|
||||
resumeVibeSession: vi.fn().mockResolvedValue({ session: session(), resumed: true }),
|
||||
serveNextVibePlanItem: vi.fn().mockResolvedValue({ item: plan().items[0], stale: false }),
|
||||
advancePastUnplayableVibePlanItem: vi.fn().mockResolvedValue({ item: plan().items[0], stale: false }),
|
||||
persistNextVibePlan: vi.fn().mockResolvedValue({ ...plan(), version: 2, reason: 'feedback:completed' }),
|
||||
getVibePlanForFeedbackEvent: vi.fn().mockResolvedValue(null),
|
||||
} as unknown as DbService;
|
||||
@@ -179,6 +180,25 @@ describe('VibeSessionCoordinator', () => {
|
||||
expect(result).toMatchObject({ planVersion: 1, now: { track_id: TRACK_ID } });
|
||||
});
|
||||
|
||||
it('advances past an unplayable served item using a distinct idempotent event protocol', async () => {
|
||||
const { db, coordinator } = setup();
|
||||
const planVersionId = '44444444-4444-4444-8444-444444444444';
|
||||
const eventId = '55555555-5555-4555-8555-555555555555';
|
||||
|
||||
const result = await coordinator.advancePastUnplayable('user-1', SESSION_ID, {
|
||||
expectedPlanVersion: 1,
|
||||
planVersionId,
|
||||
ordinal: 0,
|
||||
trackId: TRACK_ID,
|
||||
eventId,
|
||||
});
|
||||
|
||||
expect(db.advancePastUnplayableVibePlanItem).toHaveBeenCalledWith(SESSION_ID, 'user-1', {
|
||||
expectedPlanVersion: 1, planVersionId, ordinal: 0, trackId: TRACK_ID, eventId,
|
||||
});
|
||||
expect(result.now).toMatchObject({ track_id: TRACK_ID });
|
||||
});
|
||||
|
||||
it('ends an active session once and preserves its latest persisted plan', async () => {
|
||||
const { db, coordinator } = setup();
|
||||
const response = await coordinator.end('user-1', SESSION_ID);
|
||||
|
||||
@@ -36,6 +36,19 @@ export interface AppendVibeEventInput {
|
||||
payload?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* An explicit advancement protocol for a plan item that was durably served
|
||||
* but cannot be played locally. `eventId` is the idempotency key for this
|
||||
* state transition; it is intentionally separate from a retry of /next.
|
||||
*/
|
||||
export interface AdvanceUnplayableVibeItemInput {
|
||||
expectedPlanVersion: number;
|
||||
planVersionId: string;
|
||||
ordinal: number;
|
||||
trackId: string;
|
||||
eventId: string;
|
||||
}
|
||||
|
||||
export class VibeSessionNotFoundError extends Error {}
|
||||
export class VibeSessionLifecycleError extends Error {}
|
||||
export class VibePlanNotFoundError extends Error {}
|
||||
@@ -142,6 +155,21 @@ export class VibeSessionCoordinator {
|
||||
}
|
||||
}
|
||||
|
||||
async advancePastUnplayable(
|
||||
userId: string,
|
||||
sessionId: string,
|
||||
input: AdvanceUnplayableVibeItemInput,
|
||||
): Promise<VibeSessionResponse> {
|
||||
try {
|
||||
const served = await this.db.advancePastUnplayableVibePlanItem(sessionId, userId, input);
|
||||
const response = await this.getPlan(userId, sessionId);
|
||||
if (served.stale) return response;
|
||||
return { ...response, now: served.item, preview: response.preview };
|
||||
} catch (error) {
|
||||
throw this.mapLifecycleError(error);
|
||||
}
|
||||
}
|
||||
|
||||
async appendEvent(
|
||||
userId: string,
|
||||
sessionId: string,
|
||||
|
||||
Reference in New Issue
Block a user