Skip to content

Commit 553f05a

Browse files
Iris Alexandrescuclaude
andcommitted
fix(serenity): job-status polling follows the chain to its effective terminal hop (LLMO-7418 external-review Finding 8)
The runner marks the ORIGINAL AsyncJob COMPLETED on any non-throwing handler return, including a self-requeue ({ requeuedJobId }, workspace not yet settled) or a chain hand-off ({ provisioningStatus: 'ready', chainedJobId }, the chained market-create/activate work not yet started). getPromptsJobStatus (the same handler serving the generic /serenity/jobs/:jobId polling alias) just echoed that first hop's status/result verbatim, so a client polling during a chain saw a premature "COMPLETED" the moment the FIRST hop merely handed off, with no market created and no way to tell the difference from a genuinely finished request. The frontend consumers built against this contract this session all resolve on the first COMPLETED with no chain awareness. Fix is entirely backend-scoped, so no frontend change is needed: when the polled job is COMPLETED and its result carries chainedJobId or requeuedJobId, follow that pointer (recursively, capped at 10 hops against a corrupt/cyclic chain) and report the EFFECTIVE terminal hop's status/result/ error instead. jobId/jobType in the response still reflect the ORIGINALLY- requested job, so a caller polling a fixed URL never needs to learn intermediate hop ids. Updated the two OpenAPI descriptions that previously overclaimed "result is the same body a synchronous 2xx would have returned" without qualifying that this only became true once the whole chain, not just the first hop, had run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 59fdcc9 commit 553f05a

3 files changed

Lines changed: 166 additions & 9 deletions

File tree

docs/openapi/serenity-api.yaml

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -597,10 +597,13 @@ v2-serenity-markets:
597597
sub-workspace-mode brand only). The sub-workspace-ready check and the
598598
project create/publish are enqueued to a background job chain instead
599599
of running inline. Poll the returned `jobId` at
600-
`GET .../serenity/jobs/{jobId}` for status + result — on COMPLETED,
601-
`result` is the same body a synchronous `201` would have returned.
602-
Absent/false runs synchronously as today; this is opt-in only and not
603-
yet the default.
600+
`GET .../serenity/jobs/{jobId}` for status + result — the polling
601+
endpoint transparently follows the job chain to its EFFECTIVE
602+
terminal hop, so `status` only reports `COMPLETED` once the actual
603+
market create/publish has run, not merely once the sub-workspace
604+
became ready; on COMPLETED, `result` is the same body a synchronous
605+
`201` would have returned. Absent/false runs synchronously as
606+
today; this is opt-in only and not yet the default.
604607
content:
605608
application/json:
606609
schema: { $ref: './schemas.yaml#/SerenityPromptsJobAccepted' }
@@ -1228,9 +1231,12 @@ v2-serenity-activate:
12281231
batch (sub-workspace ready-check, every market's project create/
12291232
publish, the site link, and the active flip) is enqueued to a
12301233
background job chain instead of running inline. Poll the returned
1231-
`jobId` at `GET .../serenity/jobs/{jobId}` for status + result — on
1232-
COMPLETED, `result` is the same body a synchronous `200`/`207` would
1233-
have returned.
1234+
`jobId` at `GET .../serenity/jobs/{jobId}` for status + result — the
1235+
polling endpoint transparently follows the job chain to its
1236+
EFFECTIVE terminal hop, so `status` only reports `COMPLETED` once
1237+
the actual batch has run, not merely once the sub-workspace became
1238+
ready; on COMPLETED, `result` is the same body a synchronous
1239+
`200`/`207` would have returned.
12341240
content:
12351241
application/json:
12361242
schema: { $ref: './schemas.yaml#/SerenityPromptsJobAccepted' }

src/controllers/serenity.js

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1978,7 +1978,7 @@ function SerenityController(context, log, env) {
19781978
if (!AsyncJob || typeof AsyncJob.findById !== 'function') {
19791979
return internalServerError('AsyncJob data-access not available');
19801980
}
1981-
const job = await AsyncJob.findById(jobId);
1981+
let job = await AsyncJob.findById(jobId);
19821982
// A job that does not exist AND a job that belongs to another brand answer
19831983
// the same 404: whether the id is unknown or simply not yours is not the
19841984
// caller's business (mirrors authorize's brand-not-found contract).
@@ -1987,6 +1987,31 @@ function SerenityController(context, log, env) {
19871987
return notFound(`Job not found: ${jobId}`);
19881988
}
19891989
const metadata = job.getMetadata?.() ?? {};
1990+
// Follow a chain/requeue to its EFFECTIVE terminal hop (LLMO-7418 external-review
1991+
// Finding 8): the runner marks the ORIGINAL job COMPLETED on any non-throwing handler
1992+
// return, including a self-requeue (`{ requeuedJobId }`, workspace not yet settled) or a
1993+
// chain hand-off (`{ provisioningStatus: 'ready', chainedJobId }`, the chained market-
1994+
// create/activate work not yet started) — neither means the real work actually finished.
1995+
// Without this, a caller polling the FIRST hop's id sees a premature COMPLETED the moment
1996+
// the chain merely starts, not when it actually ends. `jobId`/`jobType` below still report
1997+
// the ORIGINALLY-requested job's identity — only status/result/error follow the chain, so a
1998+
// caller polling a fixed URL never needs to learn about intermediate hop ids. Bounded to
1999+
// guard against a corrupt/cyclic chain; a dangling pointer (an id the chain names but that
2000+
// no longer resolves) simply stops following and reports the last hop actually found.
2001+
const MAX_CHAIN_FOLLOW_HOPS = 10;
2002+
for (let hops = 0; job.getStatus() === 'COMPLETED' && hops < MAX_CHAIN_FOLLOW_HOPS; hops += 1) {
2003+
const hopResult = job.getResult?.();
2004+
const nextJobId = hopResult?.chainedJobId || hopResult?.requeuedJobId;
2005+
if (!nextJobId) {
2006+
break;
2007+
}
2008+
// eslint-disable-next-line no-await-in-loop
2009+
const nextJob = await AsyncJob.findById(nextJobId);
2010+
if (!nextJob) {
2011+
break;
2012+
}
2013+
job = nextJob;
2014+
}
19902015
/** @type {'classifyPrompts' | 'bulkTags' | 'tagImpact'} */
19912016
let publicJobType = 'classifyPrompts';
19922017
if (metadata.jobType === BULK_TAGS_JOB_TYPE) {
@@ -2010,7 +2035,9 @@ function SerenityController(context, log, env) {
20102035
const error = status === 'FAILED' ? publicJobError(job.getError?.()) : null;
20112036
return createResponse(
20122037
{
2013-
jobId: job.getId(),
2038+
// The ORIGINALLY-requested id, never a followed hop's — the caller polls a fixed URL
2039+
// and never needs to learn about intermediate chain/requeue job ids.
2040+
jobId,
20142041
jobType: publicJobType,
20152042
status,
20162043
result,

test/controllers/serenity.test.js

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3034,6 +3034,130 @@ describe('SerenityController', () => {
30343034
);
30353035
expect(response.status).to.equal(400);
30363036
});
3037+
3038+
// LLMO-7418 external-review Finding 8: a self-requeue or chained hand-off marks the FIRST
3039+
// hop COMPLETED immediately, even though the real work hasn't run yet. These tests confirm
3040+
// the endpoint follows the chain to its effective terminal status instead.
3041+
describe('chain/requeue following (LLMO-7418 external-review Finding 8)', () => {
3042+
const CHAINED_JOB = 'aaaaaaaa-1111-2222-3333-444444444444';
3043+
const FINAL_JOB = 'bbbbbbbb-1111-2222-3333-444444444444';
3044+
3045+
function ctxWithChain(jobsById, { jobId = JOB } = {}) {
3046+
const ctx = fakeContext({ params: { jobId } });
3047+
ctx.dataAccess.AsyncJob = {
3048+
findById: sinon.stub().callsFake((id) => Promise.resolve(jobsById[id] ?? null)),
3049+
};
3050+
return ctx;
3051+
}
3052+
3053+
it('follows a chainedJobId to the market-create job\'s own terminal status, not the provisioning job\'s premature COMPLETED', async () => {
3054+
const controller = SerenityController({ env: {} }, fakeLog(), {});
3055+
const finalResult = { status: 201, body: { projectId: 'proj-1' } };
3056+
const ctx = ctxWithChain({
3057+
[JOB]: makeAsyncJob({
3058+
id: JOB,
3059+
status: 'COMPLETED',
3060+
result: { provisioningStatus: 'ready', chainedJobId: CHAINED_JOB },
3061+
}),
3062+
[CHAINED_JOB]: makeAsyncJob({
3063+
id: CHAINED_JOB, status: 'COMPLETED', result: finalResult,
3064+
}),
3065+
});
3066+
3067+
const response = await controller.getPromptsJobStatus(ctx);
3068+
const body = await readBody(response);
3069+
3070+
// The ORIGINALLY-requested id is echoed back, not the chained job's.
3071+
expect(body.jobId).to.equal(JOB);
3072+
expect(body.status).to.equal('COMPLETED');
3073+
expect(body.result).to.deep.equal(finalResult);
3074+
});
3075+
3076+
it('follows a requeuedJobId and reports IN_PROGRESS while the chain is still settling', async () => {
3077+
const controller = SerenityController({ env: {} }, fakeLog(), {});
3078+
const ctx = ctxWithChain({
3079+
[JOB]: makeAsyncJob({
3080+
id: JOB, status: 'COMPLETED', result: { requeuedJobId: CHAINED_JOB },
3081+
}),
3082+
[CHAINED_JOB]: makeAsyncJob({ id: CHAINED_JOB, status: 'IN_PROGRESS', result: null }),
3083+
});
3084+
3085+
const response = await controller.getPromptsJobStatus(ctx);
3086+
const body = await readBody(response);
3087+
3088+
expect(body.jobId).to.equal(JOB);
3089+
expect(body.status).to.equal('IN_PROGRESS');
3090+
expect(body.result).to.equal(null);
3091+
});
3092+
3093+
it('follows a multi-hop chain (requeue then chained job) to its final terminal status', async () => {
3094+
const controller = SerenityController({ env: {} }, fakeLog(), {});
3095+
const finalResult = { status: 201, body: {} };
3096+
const ctx = ctxWithChain({
3097+
[JOB]: makeAsyncJob({
3098+
id: JOB, status: 'COMPLETED', result: { requeuedJobId: CHAINED_JOB },
3099+
}),
3100+
[CHAINED_JOB]: makeAsyncJob({
3101+
id: CHAINED_JOB,
3102+
status: 'COMPLETED',
3103+
result: { provisioningStatus: 'ready', chainedJobId: FINAL_JOB },
3104+
}),
3105+
[FINAL_JOB]: makeAsyncJob({ id: FINAL_JOB, status: 'COMPLETED', result: finalResult }),
3106+
});
3107+
3108+
const response = await controller.getPromptsJobStatus(ctx);
3109+
const body = await readBody(response);
3110+
3111+
expect(body.status).to.equal('COMPLETED');
3112+
expect(body.result).to.deep.equal(finalResult);
3113+
});
3114+
3115+
it('reports the last hop actually found when the chain names a dangling job id', async () => {
3116+
const controller = SerenityController({ env: {} }, fakeLog(), {});
3117+
const ctx = ctxWithChain({
3118+
[JOB]: makeAsyncJob({
3119+
id: JOB, status: 'COMPLETED', result: { chainedJobId: 'does-not-exist' },
3120+
}),
3121+
});
3122+
3123+
const response = await controller.getPromptsJobStatus(ctx);
3124+
const body = await readBody(response);
3125+
3126+
expect(response.status).to.equal(200);
3127+
expect(body.jobId).to.equal(JOB);
3128+
expect(body.status).to.equal('COMPLETED');
3129+
expect(body.result).to.deep.equal({ chainedJobId: 'does-not-exist' });
3130+
});
3131+
3132+
it('does not follow a chain when the first hop is not COMPLETED', async () => {
3133+
const controller = SerenityController({ env: {} }, fakeLog(), {});
3134+
const ctx = ctxWithChain({
3135+
[JOB]: makeAsyncJob({ id: JOB, status: 'IN_PROGRESS', result: null }),
3136+
[CHAINED_JOB]: makeAsyncJob({ id: CHAINED_JOB, status: 'COMPLETED', result: {} }),
3137+
});
3138+
3139+
const response = await controller.getPromptsJobStatus(ctx);
3140+
const body = await readBody(response);
3141+
3142+
expect(body.status).to.equal('IN_PROGRESS');
3143+
});
3144+
3145+
it('stops following after the hop cap, never loops forever on a cyclic chain', async () => {
3146+
const controller = SerenityController({ env: {} }, fakeLog(), {});
3147+
const jobA = 'cccccccc-1111-2222-3333-444444444444';
3148+
const jobB = 'dddddddd-1111-2222-3333-444444444444';
3149+
const ctx = ctxWithChain({
3150+
[JOB]: makeAsyncJob({ id: JOB, status: 'COMPLETED', result: { chainedJobId: jobA } }),
3151+
[jobA]: makeAsyncJob({ id: jobA, status: 'COMPLETED', result: { chainedJobId: jobB } }),
3152+
[jobB]: makeAsyncJob({ id: jobB, status: 'COMPLETED', result: { chainedJobId: jobA } }),
3153+
});
3154+
3155+
const response = await controller.getPromptsJobStatus(ctx);
3156+
3157+
// Must resolve (not hang) and still answer 200 with SOME terminal status.
3158+
expect(response.status).to.equal(200);
3159+
});
3160+
});
30373161
});
30383162

30393163
it('builds a working type classifier from the brand name + aliases and passes it to the handler (serenity-docs#31)', async () => {

0 commit comments

Comments
 (0)