From d32fc69f234fb0808ae7c3d218c854ea34f875a9 Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Tue, 22 Sep 2026 17:17:49 -0400 Subject: [PATCH] feat(semaphore-sync): two-tier tag sync (adopt existing + gated intake) + fix mislog Rework the semaphore-tag-sync applier so a weekly run adopts the existing tag corpus without importing all ~20k SES terms, and picks up genuinely new tags only under an explicit class allowlist. Tier 1 (adopt-only): match each SES term by semaphoreId, then by name. Backfill taxonomy fields (label/name/titlePath/semaphoreId) in place when they differ; NEVER overwrite editorial flags (isActualTag/isInterestItem). Tier 2 (gated intake): INSERT an unmatched term only when its SES class is in semaphore.sync.intakeClasses (case-insensitive, URI-leaf tolerant). New rows land inert (flags forced false), awaiting editor curation. Empty allowlist = intake OFF (adopt-only), the safe default. Dry-run summary now carries a pre-formatted classHistogram string so the real SES class distribution can be read from an in-app dry run and used to choose semaphore.sync.filter / intakeClasses from data. dryRun defaults TRUE. Also fixes a scheduler mislog: a job fn RETURNING { ok:false } (rather than throwing) was recorded as SUCCESS. The chassis now inspects the return value, logs a FAILED PipelineLog row, records JobLastRun.lastErrorAt, and raises an alert. Covered by new admin-job-controls tests. Config keys (all ImsConfig, DB-driven -- no env vars): semaphore.sync.intakeClasses (new), .dryRun, .filter, .model, .lang, .actualTagClasses, .interestItemClasses Refs #2184, #2478 --- srv/jobs/scheduler.js | 21 ++++- srv/jobs/semaphore-tag-sync-job.js | 40 ++++++++- srv/lib/semaphore-sync/applier.js | 98 ++++++++++++++++----- srv/lib/semaphore-sync/mapper.js | 3 + test/unit/semaphore-sync-applier.test.js | 107 +++++++++++++++++++---- test/unit/semaphore-tag-sync-job.test.js | 29 ++++-- test/unit/srv/admin-job-controls.test.js | 51 +++++++++++ 7 files changed, 296 insertions(+), 53 deletions(-) diff --git a/srv/jobs/scheduler.js b/srv/jobs/scheduler.js index 0e223da79..540edd348 100644 --- a/srv/jobs/scheduler.js +++ b/srv/jobs/scheduler.js @@ -170,7 +170,26 @@ async function runWithLock(jobName, durationMs, fn, opts = {}) { try { result = await fn(logId); const summary = formatJobSummary(jobName, result); - await logPipelineEnd(logId, 'SUCCESS', summary); + // A job may fail-shut by RETURNING { ok:false, error } rather than throwing + // (e.g. semaphore-tag-sync on a bad fetch). Treat that as a failed run so the + // PipelineLog STATUS and JobLastRun.lastSuccessAt/lastErrorAt reflect reality + // — otherwise a no-op/errored run is mislogged SUCCESS and looks healthy. + if (result && typeof result === 'object' && result.ok === false) { + outcome = 'error'; + errorMessage = result.error ? String(result.error) : `${jobName} returned ok:false`; + LOG.error(`Job ${jobName} returned failure:`, errorMessage); + await logPipelineEnd(logId, 'FAILED', summary, errorMessage); + void alerting.raise({ + eventType: 'ScheduledJobFailed', + severity: 'ERROR', + category: 'ALERT', + subject: `Scheduled job failed: ${jobName}`, + body: errorMessage, + resource: { resourceName: jobName, resourceType: 'job' } + }); // fail-open, non-blocking + } else { + await logPipelineEnd(logId, 'SUCCESS', summary); + } } catch (err) { outcome = 'error'; errorMessage = err.message ?? String(err); diff --git a/srv/jobs/semaphore-tag-sync-job.js b/srv/jobs/semaphore-tag-sync-job.js index 748747c47..4b18c4375 100644 --- a/srv/jobs/semaphore-tag-sync-job.js +++ b/srv/jobs/semaphore-tag-sync-job.js @@ -41,6 +41,7 @@ const CONFIG_KEYS = [ 'semaphore.sync.filter', 'semaphore.sync.actualTagClasses', 'semaphore.sync.interestItemClasses', + 'semaphore.sync.intakeClasses', 'semaphore.sync.dryRun', ]; @@ -51,6 +52,34 @@ function splitList(v) { .filter(Boolean); } +// Shorten a class URI to its readable leaf ("…schema#Topic" → "Topic") for the +// histogram; leave short names untouched. +function shortClass(c) { + const s = String(c ?? ''); + if (s.includes('#')) return s.slice(s.lastIndexOf('#') + 1); + if (s.includes('/')) return s.slice(s.lastIndexOf('/') + 1); + return s; +} + +// Distinct SES classes across all mapped rows with a term count each, rendered +// as a compact single-line string. formatJobSummary only renders scalar summary +// fields (number/string/boolean) — an object/array would be silently dropped — +// so the histogram MUST be a pre-formatted string to survive into the SUMMARY. +// This is the whole point of the first dry run: it surfaces the real class +// distribution so semaphore.sync.filter / intakeClasses can be chosen from data. +function classHistogram(rows) { + const counts = new Map(); + for (const r of rows) { + for (const c of r.classes ?? []) { + const k = shortClass(c); + counts.set(k, (counts.get(k) ?? 0) + 1); + } + } + const sorted = [...counts.entries()].sort((a, b) => b[1] - a[1]); + if (sorted.length === 0) return '(no classes on any term)'; + return sorted.map(([k, n]) => `${k}=${n}`).join(', '); +} + // Read the semaphore.sync.* string config from ImsConfig in one SELECT. async function readConfig(db) { const { ImsConfig } = cds.entities(NS); @@ -69,6 +98,10 @@ async function readConfig(db) { filter: map.get('semaphore.sync.filter') || undefined, actualTagClasses: splitList(map.get('semaphore.sync.actualTagClasses')), interestItemClasses: splitList(map.get('semaphore.sync.interestItemClasses')), + // Tier-2 intake allowlist: unmatched terms are INSERTed only if their SES + // class is listed here. Empty ⇒ intake OFF (adopt-only) — the safe default + // until the FILTER/classes are chosen from the dry-run histogram. + intakeClasses: splitList(map.get('semaphore.sync.intakeClasses')), // dryRun defaults to TRUE — first live runs report the plan without writing. dryRun: (map.get('semaphore.sync.dryRun') ?? 'true').toLowerCase() !== 'false', }; @@ -130,9 +163,9 @@ export async function runSemaphoreTagSync(_logId, opts = {}) { interestItemClasses: cfg.interestItemClasses, }); - let applied = { inserted: 0, updated: 0, unchanged: 0, total: rows.length }; + let applied = { inserted: 0, updated: 0, unchanged: 0, skippedIntake: 0, total: rows.length }; try { - applied = await applyTerms(rows, { db, dryRun: cfg.dryRun }); + applied = await applyTerms(rows, { db, dryRun: cfg.dryRun, intakeClasses: cfg.intakeClasses }); } catch (e) { LOG.error(`semaphore-sync upsert failed: ${e.message}`); return { ok: false, error: e.message, mapped: rows.length, skipped: skipped.length }; @@ -146,6 +179,9 @@ export async function runSemaphoreTagSync(_logId, opts = {}) { fetched: Array.isArray(data.terms) ? data.terms.length : 0, mapped: rows.length, skipped: skipped.length, + intakeClasses: cfg.intakeClasses.join(', ') || '(none — adopt-only)', + // Pre-formatted string so formatJobSummary renders it into the SUMMARY. + classHistogram: classHistogram(rows), ...applied, }; LOG.info(`semaphore-sync summary: ${JSON.stringify(summary)}`); diff --git a/srv/lib/semaphore-sync/applier.js b/srv/lib/semaphore-sync/applier.js index 83ece2f15..cf32dfdd4 100644 --- a/srv/lib/semaphore-sync/applier.js +++ b/srv/lib/semaphore-sync/applier.js @@ -19,42 +19,72 @@ import cds from '@sap/cds'; -// Fields we consider when deciding whether an existing row needs an UPDATE. -const TRACKED = ['label', 'name', 'titlePath', 'isActualTag', 'isInterestItem', 'semaphoreId']; +// SES SAPCore is the ENTIRE SAP product/topic universe (~21k terms); the Tags +// table is a deliberately curated subset. So the sync is two-tier (#2184): +// +// Tier 1 — ADOPT (existing tags): match by semaphoreId, else by name. Backfill +// ONLY the taxonomy-owned fields (semaphoreId, titlePath, label). The +// isActualTag / isInterestItem flags are EDITORIAL — set per-term by hand, +// not derivable from any SES class — so the sync must never overwrite them +// (verified: 99 tags under the "Software Product" root, only 18/18 flagged). +// +// Tier 2 — INTAKE (new terms): a term with no matching row is INSERTed ONLY +// when it passes the intake allowlist (opts.intakeClasses). New rows land +// INERT (isActualTag=false, isInterestItem=false) awaiting editor curation — +// never active, never flooding the table with all 21k. With no allowlist, +// intake is OFF and unmatched terms are counted `skippedIntake`, not inserted. +// +// Idempotent: a second run with the same payload reports every row `unchanged`. +// dryRun:true computes the same plan without writing. + +// Taxonomy-owned fields the sync may write onto an EXISTING row. The editorial +// flags are deliberately excluded so an adopt/update never clobbers curation. +const ADOPT_FIELDS = ['label', 'name', 'titlePath', 'semaphoreId']; -function differs(existing, row) { - return TRACKED.some((f) => (existing[f] ?? null) !== (row[f] ?? null)); +function adoptDiffers(existing, row) { + return ADOPT_FIELDS.some((f) => (existing[f] ?? null) !== (row[f] ?? null)); +} + +// Case-insensitive membership, tolerant of full class URIs vs short names — +// mirrors mapper.classMatches so intake uses the same class-matching semantics. +function classAllowed(termClasses, allow) { + if (!Array.isArray(allow) || allow.length === 0) return false; + if (!Array.isArray(termClasses) || termClasses.length === 0) return false; + const allowLc = allow.map((w) => String(w).toLowerCase()); + return termClasses.some((c) => { + const cl = String(c ?? '').toLowerCase(); + const short = cl.includes('#') ? cl.slice(cl.lastIndexOf('#') + 1) + : cl.includes('/') ? cl.slice(cl.lastIndexOf('/') + 1) + : cl; + return allowLc.some((w) => w === cl || w === short); + }); } /** - * Upsert mapper rows into Tags. + * Two-tier upsert of mapper rows into Tags. * - * @param {Array} rows Output of mapAllTerms().rows + * @param {Array} rows Output of mapAllTerms().rows (each row also carries + * `classes` for the intake gate) * @param {object} [opts] - * @param {boolean} [opts.dryRun=false] compute the plan without writing - * @param {object} [opts.db] cds db (defaults to cds.db / connect) - * @returns {Promise<{inserted:number, updated:number, unchanged:number, total:number}>} + * @param {boolean} [opts.dryRun=false] compute the plan without writing + * @param {string[]} [opts.intakeClasses=[]] class allowlist for Tier-2 INSERT; + * empty ⇒ intake OFF (adopt-only) + * @param {object} [opts.db] cds db (defaults to cds.db / connect) + * @returns {Promise<{inserted:number, updated:number, unchanged:number, + * skippedIntake:number, total:number}>} */ export async function applyTerms(rows, opts = {}) { - const { dryRun = false } = opts; + const { dryRun = false, intakeClasses = [] } = opts; const db = opts.db ?? cds.db ?? (await cds.connect.to('db')); const { Tags } = cds.entities('com.sap.developers.ims'); let inserted = 0; let updated = 0; let unchanged = 0; + let skippedIntake = 0; const total = Array.isArray(rows) ? rows.length : 0; for (const row of rows ?? []) { - const fields = { - semaphoreId: row.semaphoreId, - label: row.label, - name: row.name, - titlePath: row.titlePath, - isActualTag: !!row.isActualTag, - isInterestItem: !!row.isInterestItem, - }; - // 1. Existing by semaphoreId. let existing = await db.run(SELECT.one.from(Tags).where({ semaphoreId: row.semaphoreId })); // 2. Adopt a legacy/CSV row that matches by name but has no semaphoreId yet. @@ -64,9 +94,16 @@ export async function applyTerms(rows, opts = {}) { ); } + // Tier 1 — ADOPT: update ONLY taxonomy-owned fields; never the editorial flags. if (existing) { - if (differs(existing, fields)) { - if (!dryRun) await db.run(UPDATE(Tags, existing.ID).set(fields)); + const adopt = { + semaphoreId: row.semaphoreId, + label: row.label, + name: row.name, + titlePath: row.titlePath, + }; + if (adoptDiffers(existing, adopt)) { + if (!dryRun) await db.run(UPDATE(Tags, existing.ID).set(adopt)); updated++; } else { unchanged++; @@ -74,12 +111,25 @@ export async function applyTerms(rows, opts = {}) { continue; } - // 3. Insert. Assign the UUID key explicitly for HANA parity. + // Tier 2 — INTAKE: insert a genuinely new term ONLY if class-allowlisted. + // New rows land inert; an editor promotes them by setting the flags later. + if (!classAllowed(row.classes, intakeClasses)) { + skippedIntake++; + continue; + } if (!dryRun) { - await db.run(INSERT.into(Tags).entries({ ID: cds.utils.uuid(), ...fields })); + await db.run(INSERT.into(Tags).entries({ + ID: cds.utils.uuid(), + semaphoreId: row.semaphoreId, + label: row.label, + name: row.name, + titlePath: row.titlePath, + isActualTag: false, + isInterestItem: false, + })); } inserted++; } - return { inserted, updated, unchanged, total }; + return { inserted, updated, unchanged, skippedIntake, total }; } diff --git a/srv/lib/semaphore-sync/mapper.js b/srv/lib/semaphore-sync/mapper.js index 4aa594d11..27c3cd37c 100644 --- a/srv/lib/semaphore-sync/mapper.js +++ b/srv/lib/semaphore-sync/mapper.js @@ -161,6 +161,9 @@ export function mapAllTerms(data, opts = {}) { titlePath: deriveTitlePath(term), isActualTag, isInterestItem, + // Raw SES classes, carried through for the applier's Tier-2 intake gate + // and the dry-run class histogram (job). Not persisted to Tags. + classes: Array.isArray(term.classes) ? term.classes.map(String) : [], }; // De-dupe on semaphoreId (last write wins) — SES should be unique but be safe. diff --git a/test/unit/semaphore-sync-applier.test.js b/test/unit/semaphore-sync-applier.test.js index f6c47b49c..077841e44 100644 --- a/test/unit/semaphore-sync-applier.test.js +++ b/test/unit/semaphore-sync-applier.test.js @@ -5,6 +5,10 @@ import { applyTerms } from '../../srv/lib/semaphore-sync/applier.js'; cds.test('serve', '--project', '.', '--in-memory'); +// A mapper-shaped row. `classes` carries the raw SES classes used by the Tier-2 +// intake gate. `isActualTag`/`isInterestItem` are present on the row but the +// applier deliberately IGNORES them for existing rows (editorial-owned) and +// forces them false on intake inserts. const ROW = (over = {}) => ({ semaphoreId: 's1', label: 'SAP S/4HANA', @@ -12,10 +16,11 @@ const ROW = (over = {}) => ({ titlePath: 'Software Product : SAP S/4HANA', isActualTag: true, isInterestItem: false, + classes: ['Topic'], ...over, }); -describe('semaphore applyTerms', () => { +describe('semaphore applyTerms — two-tier (#2184)', () => { let db; let Tags; @@ -25,51 +30,119 @@ describe('semaphore applyTerms', () => { await DELETE.from(Tags); }); - it('inserts a new term with all Semaphore fields', async () => { + // ── Tier 2: intake ──────────────────────────────────────────────────────── + + it('does NOT insert an unmatched term when intake is off (no allowlist)', async () => { const res = await applyTerms([ROW()], { db }); - expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, total: 1 }); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 0, skippedIntake: 1, total: 1 }); + expect(await SELECT.from(Tags)).toHaveLength(0); + }); + + it('inserts an unmatched term when its class is allowlisted, flags forced inert', async () => { + const res = await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, skippedIntake: 0, total: 1 }); const t = await SELECT.one.from(Tags).where({ semaphoreId: 's1' }); expect(t).toMatchObject({ name: 'sap s 4hana', label: 'SAP S/4HANA', titlePath: 'Software Product : SAP S/4HANA', - isActualTag: true, isInterestItem: false, + // New rows land inert regardless of the row's isActualTag:true. + isActualTag: false, isInterestItem: false, }); expect(t.ID).toBeTruthy(); }); + it('skips an unmatched term whose class is not in the allowlist', async () => { + const res = await applyTerms([ROW({ classes: ['SomethingElse'] })], { db, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 0, skippedIntake: 1, total: 1 }); + expect(await SELECT.from(Tags)).toHaveLength(0); + }); + + it('matches allowlist against a full class URI by its leaf', async () => { + const res = await applyTerms( + [ROW({ classes: ['http://sap.com/schema#Topic'] })], + { db, intakeClasses: ['Topic'] }, + ); + expect(res.inserted).toBe(1); + }); + + // ── Tier 1: adopt/update existing ────────────────────────────────────────── + it('is idempotent: a second run reports everything unchanged', async () => { - await applyTerms([ROW()], { db }); - const res = await applyTerms([ROW()], { db }); - expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 1, total: 1 }); + await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + const res = await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 1, skippedIntake: 0, total: 1 }); expect(await SELECT.from(Tags)).toHaveLength(1); }); - it('updates in place when a synced term is renamed (same semaphoreId)', async () => { - await applyTerms([ROW()], { db }); - const res = await applyTerms([ROW({ label: 'SAP S/4HANA Cloud', titlePath: 'Software Product : SAP S/4HANA Cloud' })], { db }); - expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, total: 1 }); + it('updates taxonomy fields in place when a synced term is renamed', async () => { + await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + const res = await applyTerms( + [ROW({ label: 'SAP S/4HANA Cloud', titlePath: 'Software Product : SAP S/4HANA Cloud' })], + { db }, + ); + expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, skippedIntake: 0, total: 1 }); const rows = await SELECT.from(Tags).where({ semaphoreId: 's1' }); expect(rows).toHaveLength(1); // no duplicate expect(rows[0].label).toBe('SAP S/4HANA Cloud'); }); + it('NEVER overwrites editorial flags on an existing row', async () => { + // Existing curated row: hand-set isActualTag=true, isInterestItem=true. + await INSERT.into(Tags).entries({ + ID: 'curated-1', semaphoreId: 's1', name: 'sap s 4hana', + label: 'SAP S/4HANA', titlePath: 'Software Product : SAP S/4HANA', + isActualTag: true, isInterestItem: true, + }); + // Sync row carries the opposite flags — must be ignored. + const res = await applyTerms( + [ROW({ isActualTag: false, isInterestItem: false })], + { db, intakeClasses: ['Topic'] }, + ); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 1, skippedIntake: 0, total: 1 }); + const t = await SELECT.one.from(Tags).where({ ID: 'curated-1' }); + expect(t.isActualTag).toBe(true); + expect(t.isInterestItem).toBe(true); + }); + + it('updates taxonomy fields yet preserves flags when both change', async () => { + await INSERT.into(Tags).entries({ + ID: 'curated-2', semaphoreId: 's1', name: 'sap s 4hana', + label: 'Old Label', titlePath: 'old', isActualTag: true, isInterestItem: true, + }); + const res = await applyTerms([ROW({ isActualTag: false, isInterestItem: false })], { db }); + expect(res.updated).toBe(1); + const t = await SELECT.one.from(Tags).where({ ID: 'curated-2' }); + expect(t.label).toBe('SAP S/4HANA'); // taxonomy field adopted + expect(t.titlePath).toBe('Software Product : SAP S/4HANA'); + expect(t.isActualTag).toBe(true); // flags untouched + expect(t.isInterestItem).toBe(true); + }); + it('adopts a legacy row matched by name that lacks a semaphoreId', async () => { - await INSERT.into(Tags).entries({ ID: 'legacy-1', name: 'sap s 4hana', titlePath: 'old', legacyId: 42 }); + await INSERT.into(Tags).entries({ + ID: 'legacy-1', name: 'sap s 4hana', titlePath: 'old', legacyId: 42, + isActualTag: true, isInterestItem: false, + }); const res = await applyTerms([ROW()], { db }); - expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, total: 1 }); + expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, skippedIntake: 0, total: 1 }); const t = await SELECT.one.from(Tags).where({ ID: 'legacy-1' }); expect(t.semaphoreId).toBe('s1'); expect(t.titlePath).toBe('Software Product : SAP S/4HANA'); + expect(t.isActualTag).toBe(true); // pre-existing flag preserved through adoption expect(await SELECT.from(Tags)).toHaveLength(1); // adopted, not duplicated }); - it('dryRun computes the plan without writing', async () => { - const res = await applyTerms([ROW()], { db, dryRun: true }); - expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, total: 1 }); + // ── dry run / empty ──────────────────────────────────────────────────────── + + it('dryRun computes the intake plan without writing', async () => { + const res = await applyTerms([ROW()], { db, dryRun: true, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, skippedIntake: 0, total: 1 }); expect(await SELECT.from(Tags)).toHaveLength(0); }); it('handles an empty payload', async () => { - expect(await applyTerms([], { db })).toEqual({ inserted: 0, updated: 0, unchanged: 0, total: 0 }); + expect(await applyTerms([], { db })).toEqual({ + inserted: 0, updated: 0, unchanged: 0, skippedIntake: 0, total: 0, + }); }); }); diff --git a/test/unit/semaphore-tag-sync-job.test.js b/test/unit/semaphore-tag-sync-job.test.js index 7314675f6..56fc535de 100644 --- a/test/unit/semaphore-tag-sync-job.test.js +++ b/test/unit/semaphore-tag-sync-job.test.js @@ -66,28 +66,39 @@ describe('runSemaphoreTagSync', () => { expect(await SELECT.from(Tags)).toHaveLength(0); }); - it('dryRun default: reports the plan without writing', async () => { + it('dryRun default: reports the plan (with class histogram) without writing', async () => { __setFlagForTest('SEMAPHORE_SYNC_ENABLED', true); - await setConfig(db, { 'semaphore.sync.interestItemClasses': 'IndustryCluster' }); const res = await runSemaphoreTagSync(null, { _deps: okDeps() }); expect(res.ok).toBe(true); expect(res.dryRun).toBe(true); - expect(res).toMatchObject({ fetched: 2, mapped: 2, inserted: 2, updated: 0 }); + // Intake is off by default (no intakeClasses) → both unmatched terms are + // counted skippedIntake, nothing inserted. The histogram surfaces the real + // class distribution so the FILTER/intakeClasses can be chosen from data. + expect(res).toMatchObject({ fetched: 2, mapped: 2, inserted: 0, updated: 0, skippedIntake: 2 }); + expect(res.classHistogram).toContain('SoftwareProduct=1'); + expect(res.classHistogram).toContain('IndustryCluster=1'); + expect(res.intakeClasses).toBe('(none — adopt-only)'); expect(await SELECT.from(Tags)).toHaveLength(0); // dry run wrote nothing }); - it('writes tags when dryRun is disabled', async () => { + it('writes only allowlisted new tags (inert) when dryRun is disabled', async () => { __setFlagForTest('SEMAPHORE_SYNC_ENABLED', true); await setConfig(db, { 'semaphore.sync.dryRun': 'false', - 'semaphore.sync.interestItemClasses': 'IndustryCluster', + // Only SoftwareProduct terms are admitted; IndustryCluster (Retail) is skipped. + 'semaphore.sync.intakeClasses': 'SoftwareProduct', }); const res = await runSemaphoreTagSync(null, { _deps: okDeps() }); - expect(res).toMatchObject({ ok: true, dryRun: false, inserted: 2 }); + expect(res).toMatchObject({ ok: true, dryRun: false, inserted: 1, skippedIntake: 1 }); const tags = await SELECT.from(Tags); - expect(tags).toHaveLength(2); - const retail = tags.find((t) => t.semaphoreId === 's2'); - expect(retail.isInterestItem).toBe(true); + expect(tags).toHaveLength(1); + const s4 = tags.find((t) => t.semaphoreId === 's1'); + expect(s4.name).toBe('sap s 4hana'); + // New intake rows always land inert, awaiting editor curation. + expect(s4.isActualTag).toBe(false); + expect(s4.isInterestItem).toBe(false); + // The non-allowlisted term was not written. + expect(tags.find((t) => t.semaphoreId === 's2')).toBeUndefined(); }); it('fails shut on a fetch error — writes nothing', async () => { diff --git a/test/unit/srv/admin-job-controls.test.js b/test/unit/srv/admin-job-controls.test.js index 561e5b173..c57a8e6b5 100644 --- a/test/unit/srv/admin-job-controls.test.js +++ b/test/unit/srv/admin-job-controls.test.js @@ -163,6 +163,57 @@ describe('AdminService.JobControls', () => { expect(row.lastSuccessAt).toBeTruthy(); }); + // ───────────────────────────────────────────────────────────────── + // #2478 — a job that FAIL-SHUTS by RETURNING { ok:false } (rather than + // throwing) must be recorded as a failed run, not mislogged SUCCESS. + // Regression guard for the semaphore-tag-sync "no new tags, but green" + // symptom: the chassis now inspects the runner's return value. + // ───────────────────────────────────────────────────────────────── + it('records a FAILED run when the fn returns { ok:false } (not a throw)', async () => { + const jobName = nextJobName(); + registerOne(jobName, async () => ({ ok: false, error: 'fetch HTTP 500' })); + await callRunJob(jobName); + await new Promise(resolve => setTimeout(resolve, 200)); + + const { JobLastRun, PipelineLog } = cds.entities('com.sap.developers.ims'); + const row = await SELECT.one.from(JobLastRun).where({ jobName }); + expect(row).toBeTruthy(); + // Failure path: lastErrorAt + message set, lastSuccessAt untouched (null). + expect(row.lastErrorAt).toBeTruthy(); + expect(row.lastSuccessAt).toBeFalsy(); + expect(row.lastErrorMessage).toBe('fetch HTTP 500'); + // The PipelineLog end row for this run is FAILED, not SUCCESS. + const plog = await SELECT.from(PipelineLog) + .where({ pipelineType: 'SCHEDULED_JOB' }); + const mine = plog.filter(p => (p.metadata ?? '').includes(jobName)); + expect(mine.length).toBeGreaterThan(0); + expect(mine.every(p => p.status === 'FAILED')).toBe(true); + }); + + it('falls back to a generic error message when ok:false carries no error', async () => { + const jobName = nextJobName(); + registerOne(jobName, async () => ({ ok: false })); + await callRunJob(jobName); + await new Promise(resolve => setTimeout(resolve, 200)); + + const { JobLastRun } = cds.entities('com.sap.developers.ims'); + const row = await SELECT.one.from(JobLastRun).where({ jobName }); + expect(row.lastErrorAt).toBeTruthy(); + expect(row.lastErrorMessage).toContain('returned ok:false'); + }); + + it('still records SUCCESS when the fn returns { ok:true }', async () => { + const jobName = nextJobName(); + registerOne(jobName, async () => ({ ok: true, inserted: 3 })); + await callRunJob(jobName); + await new Promise(resolve => setTimeout(resolve, 200)); + + const { JobLastRun } = cds.entities('com.sap.developers.ims'); + const row = await SELECT.one.from(JobLastRun).where({ jobName }); + expect(row.lastSuccessAt).toBeTruthy(); + expect(row.lastErrorAt).toBeFalsy(); + }); + // ───────────────────────────────────────────────────────────────── // #750: nextRunsIso (forward-visibility window for the Board tile) // ─────────────────────────────────────────────────────────────────