Skip to content

Commit 953b7d1

Browse files
dzehnderclaude
andcommitted
fix(llmo): align prompt-suggestion triggers with confirmed createSchedule signature
Match the shared drs-client createSchedule contract (confirmed): - providerIds is an ARRAY (job_config.provider_ids envelope) — pass a one-element array per single-provider pipeline. - cadence enum values use underscores: 'twice_monthly' / 'quarterly' (SCHEDULE_CADENCES); the client derives the cron server-side. - drop orgId/imsOrgId entirely: DRS derives the tenant-isolation key from site_id server-side and rejects any caller-supplied imsOrgId, so it must not be threaded through createSchedule. - add a length-capped description; priority defaults HIGH in the client. Also harden the new Phase 3 before-hook with an explicit timeout for cold esmock module loads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent dd4d9a3 commit 953b7d1

2 files changed

Lines changed: 32 additions & 24 deletions

File tree

src/controllers/llmo/llmo-onboarding.js

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,18 @@ export async function triggerBrandalfOnboardingJob({
176176
}
177177

178178
// Cadence labels accepted by DRS `createSchedule` for the recurring
179-
// "prompt suggestion" pipelines. DRS derives the concrete cron expression
180-
// server-side from the label (frequency:'cron') — we never send raw cron, so a
181-
// misconfigured/leaked caller cannot schedule a fleet-wide Fargate storm.
182-
// 'twice-monthly' → 1st & 15th (honest label; not a true 14-day interval)
179+
// "prompt suggestion" pipelines (the `SCHEDULE_CADENCES` enum in the drs-client).
180+
// DRS derives the concrete cron expression server-side from the label
181+
// (frequency:'cron', with per-site hour jitter for twice_monthly) — we never send
182+
// raw cron, so a misconfigured/leaked caller cannot schedule a fleet-wide Fargate
183+
// storm; an unknown value is rejected server-side.
184+
// 'twice_monthly' → 1st & 15th (honest label; not a true 14-day interval)
183185
// 'quarterly' → 1st of Jan/Apr/Jul/Oct
186+
// Kept as local literals (matching SCHEDULE_CADENCES values) so this module loads
187+
// against the currently-installed client; can be swapped for the imported
188+
// SCHEDULE_CADENCES const once drs-client 1.14.0 is installed.
184189
// See local/drs-prompt-suggestions-schedules-onboarding-plan.md ("Cadence expression").
185-
const DRS_CADENCE_TWICE_MONTHLY = 'twice-monthly';
190+
const DRS_CADENCE_TWICE_MONTHLY = 'twice_monthly';
186191
const DRS_CADENCE_QUARTERLY = 'quarterly';
187192

188193
/**
@@ -196,19 +201,21 @@ const DRS_CADENCE_QUARTERLY = 'quarterly';
196201
* schedule and nothing self-heals, so it propagates to the caller which logs it
197202
* at ERROR. This helper therefore does not swallow the failure itself.
198203
*
204+
* NOTE: `createSchedule` derives the tenant-isolation key from `siteId`
205+
* server-side and REJECTS any caller-supplied imsOrgId, so we deliberately do not
206+
* thread imsOrgId/orgId into it.
207+
*
199208
* @param {object} params
200209
* @param {object} params.drsClient - Configured DRS client.
201210
* @param {string} params.providerId - DRS provider id to schedule.
202211
* @param {string} params.cadence - One of DRS_CADENCE_* labels.
203212
* @param {string} params.siteId - SpaceCat site UUID.
204-
* @param {string} params.orgId - SpaceCat organization UUID.
205-
* @param {string} params.imsOrgId - IMS org id (isolation key; DRS re-derives server-side).
206213
* @param {object} params.log - Logger.
207214
* @param {Function} [params.say] - Optional Slack say callback.
208215
* @returns {Promise<object|null>} The createSchedule result, or null when DRS is not configured.
209216
*/
210217
async function registerPromptSuggestionSchedule({
211-
drsClient, providerId, cadence, siteId, orgId, imsOrgId, log, say = () => {},
218+
drsClient, providerId, cadence, siteId, log, say = () => {},
212219
}) {
213220
if (!drsClient.isConfigured()) {
214221
log.debug(`DRS client not configured, skipping ${providerId} schedule for site ${siteId}`);
@@ -217,13 +224,13 @@ async function registerPromptSuggestionSchedule({
217224

218225
// Default enable_brand_presence off (plan Phase 0.4): these prompt-suggestion
219226
// pipelines must not push unexpected load into the brand-presence pipeline / SNS
220-
// allowlist unless a site is explicitly BP-enabled.
227+
// allowlist unless a site is explicitly BP-enabled. `providerIds` is an array
228+
// (the job_config.provider_ids envelope) even for a single-provider pipeline.
221229
const result = await drsClient.createSchedule({
222230
siteId,
223-
providerId,
231+
providerIds: [providerId],
224232
cadence,
225-
orgId,
226-
imsOrgId,
233+
description: `${providerId} prompt-suggestion schedule (onboarding)`,
227234
enableBrandPresence: false,
228235
triggerImmediately: true,
229236
});
@@ -1550,8 +1557,6 @@ export async function activateBrandAndGeneratePrompts({
15501557
await trigger({
15511558
drsClient: scheduleDrsClient,
15521559
siteId: site.getId(),
1553-
orgId: organization.getId(),
1554-
imsOrgId,
15551560
log,
15561561
say,
15571562
});

test/controllers/llmo/llmo-onboarding.test.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5763,7 +5763,10 @@ describe('LLMO Onboarding Functions', () => {
57635763
let onboardingModule;
57645764
let sandbox;
57655765

5766-
before(async () => {
5766+
before(async function loadOnboardingModule() {
5767+
// Cold esmock load of the onboarding module (which pulls in many deps) can
5768+
// exceed the 2000ms default on a first, un-warmed run.
5769+
this.timeout(30000);
57675770
onboardingModule = await esmock('../../../src/controllers/llmo/llmo-onboarding.js', {});
57685771
});
57695772

@@ -5784,17 +5787,15 @@ describe('LLMO Onboarding Functions', () => {
57845787
const buildParams = (drsClient) => ({
57855788
drsClient,
57865789
siteId: 'site-123',
5787-
orgId: 'org-123',
5788-
imsOrgId: 'ABC123@AdobeOrg',
57895790
log: {
57905791
info: sandbox.stub(), debug: sandbox.stub(), warn: sandbox.stub(), error: sandbox.stub(),
57915792
},
57925793
say: sandbox.stub(),
57935794
});
57945795

57955796
const HELPERS = [
5796-
['triggerSemrushPromptJob', 'prompt_generation_semrush', 'twice-monthly'],
5797-
['triggerCitationAttemptJob', 'prompt_generation_agentic_traffic', 'twice-monthly'],
5797+
['triggerSemrushPromptJob', 'prompt_generation_semrush', 'twice_monthly'],
5798+
['triggerCitationAttemptJob', 'prompt_generation_agentic_traffic', 'twice_monthly'],
57985799
['triggerSyntheticPersonasJob', 'prompt_generation_synthetic_personas', 'quarterly'],
57995800
];
58005801

@@ -5804,15 +5805,17 @@ describe('LLMO Onboarding Functions', () => {
58045805
const result = await onboardingModule[fnName](buildParams(drsClient));
58055806

58065807
expect(drsClient.createSchedule).to.have.been.calledOnce;
5807-
expect(drsClient.createSchedule.firstCall.args[0]).to.deep.include({
5808+
const arg = drsClient.createSchedule.firstCall.args[0];
5809+
expect(arg).to.deep.include({
58085810
siteId: 'site-123',
5809-
orgId: 'org-123',
5810-
imsOrgId: 'ABC123@AdobeOrg',
5811-
providerId,
58125811
cadence,
58135812
enableBrandPresence: false,
58145813
triggerImmediately: true,
58155814
});
5815+
expect(arg.providerIds).to.deep.equal([providerId]);
5816+
// DRS derives the tenant key from siteId and rejects caller imsOrgId — never thread it.
5817+
expect(arg).to.not.have.property('imsOrgId');
5818+
expect(arg).to.not.have.property('orgId');
58165819
expect(result).to.deep.equal({ scheduleId: 'sched-1', alreadyExisted: false });
58175820
});
58185821

@@ -5892,7 +5895,7 @@ describe('LLMO Onboarding Functions', () => {
58925895
const instance = mockDrsClient.createFrom();
58935896
expect(instance.submitJob).to.have.been.calledOnce; // Brandalf fired first
58945897
expect(instance.createSchedule).to.have.been.calledThrice;
5895-
const providerIds = instance.createSchedule.getCalls().map((c) => c.args[0].providerId);
5898+
const providerIds = instance.createSchedule.getCalls().map((c) => c.args[0].providerIds[0]);
58965899
expect(providerIds).to.have.members([
58975900
'prompt_generation_semrush',
58985901
'prompt_generation_agentic_traffic',

0 commit comments

Comments
 (0)