From e304246ee79f4f72aa6ddbcd416d13d988f7c00b Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Sun, 18 Oct 2026 17:50:14 -0400 Subject: [PATCH 01/12] fix(scheduler): skip identity-pin before(*) hook on CronService (#2552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #2552 global identity-pin `before('*')` hook (srv/server.js) was registered on EVERY ApplicationService, including the internal CronService. On a scheduled tick the hook calls `pinResolvedUser(req.user, cds.tx(req))`, and `cds.tx(req)` dereferences `ctx.context._set` on the cron event's context β€” which is not a real EventContext β€” throwing `ctx.context._set is not a function` on EVERY tick. The throw is caught fail-open (WARN), so jobs still run, but: - it spams the PROD log on every scheduled tick, and - after an instance restart (e.g. a blue-green swap) resets the canary clocks, the noise coincides with the scheduler-stall canary (#2782) firing a false ALERT. The pin is a pure no-op on a cron tick anyway β€” a scheduled tick's user is always system/anonymous. Skip CronService by its fully-qualified name (com.sap.developers.ims.CronService) in the hook-registration loop. Regression first observed on PROD after the 1.40.1 blue-green swap; the hook + the throwing `cds.tx(req)` form both shipped in 1.40.0 (a3412e099 / 60c6a5bda). Test: extend test/hybrid/cron-service-schedule.test.js to assert the identity-pin before(*) handler is NOT registered on CronService. Runs in the post-deploy hybrid CI job (needs a HANA bind profile; not runnable in a fresh worktree without `npm run bind:setup`). Follow-up (separate issue): the same #2552 hook also throws `Cannot set property userId of # which has only a getter` on every authenticated request in PROD β€” a distinct bug in pinResolvedUser's write to the read-only XsuaaToken getter, not addressed here. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- srv/server.js | 7 ++++++ test/hybrid/cron-service-schedule.test.js | 27 +++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/srv/server.js b/srv/server.js index ef9269185..e6a2540a5 100644 --- a/srv/server.js +++ b/srv/server.js @@ -2352,6 +2352,13 @@ cds.on('served', async () => { if (globalThis.__identityPinHookRegistered) return; for (const srv of Object.values(cds.services)) { if (!(srv instanceof cds.ApplicationService)) continue; + // Skip the internal CronService: scheduled ticks are synthetic events with + // no HTTP request, so `cds.tx(req)` dereferences `ctx.context._set` on a + // non-EventContext and throws on EVERY tick (#2552). The pin is a no-op + // there anyway β€” a cron tick's user is always system/anonymous. The throw + // is fail-open (WARN) but spams the log and trips the scheduler-stall + // canary (#2782) after a restart resets the canary clocks. + if (srv.name === 'com.sap.developers.ims.CronService') continue; srv.before('*', async (req) => { // Only authenticated requests carry a resolvable identity; anonymous // and tech-user paths no-op inside pinResolvedUser. diff --git a/test/hybrid/cron-service-schedule.test.js b/test/hybrid/cron-service-schedule.test.js index 680be92f2..59ec60af3 100644 --- a/test/hybrid/cron-service-schedule.test.js +++ b/test/hybrid/cron-service-schedule.test.js @@ -84,4 +84,31 @@ describe('CronService schedule smoke (hybrid)', () => { ); } }); + + // #2552 regression: the global identity-pin before('*') hook (srv/server.js) + // must NOT be registered on CronService. On a scheduled tick the pin calls + // `cds.tx(req)` against a synthetic cron event whose context is not a real + // EventContext, throwing `ctx.context._set is not a function` on EVERY tick + // (fail-open WARN, but it spams the log and trips the #2782 stall canary + // after a restart). The server.js loop skips CronService by name. + it('does not register the identity-pin before(*) hook on CronService', async () => { + const cron = cds.services['com.sap.developers.ims.CronService']; + expect(cron, 'CronService should be served').toBeTruthy(); + + // CAP keeps registered phase handlers on srv.before (array of {on, handler}). + const beforeHandlers = cron.before || cron._handlers?.before || []; + // The identity-pin handler is an async fn whose source mentions + // pinResolvedUser. A '*'/undefined-event before handler whose body + // references the pin would be the regression. + const pinHandlers = (Array.isArray(beforeHandlers) ? beforeHandlers : []) + .filter((h) => { + const src = String(h?.handler ?? h ?? ''); + return src.includes('pinResolvedUser') || src.includes('identity-pin'); + }); + + expect( + pinHandlers.length, + 'identity-pin before(*) must be skipped on CronService (#2552)' + ).toBe(0); + }); }); From 115ff6fb05093e507073dc96c7d081e10e4e09b3 Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Sun, 18 Oct 2026 18:11:52 -0400 Subject: [PATCH 02/12] fix(identity): tolerate getter-only XsuaaToken.userId in pinResolvedUser (#2552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second #2552 regression, surfaced in PROD logs during the 1.40.1 scheduler diagnosis: on EVERY authenticated request, the identity-pin hook threw `Cannot set property userId of # which has only a getter`. Root cause: pinResolvedUser pinned the resolved sapId by assigning `user.authInfo.token.userId = row.sapId`. In PROD the real @sap/xssec XsuaaToken exposes `userId` as a GETTER-ONLY property, so the bare assignment throws. Caught fail-open (WARN), but it fired on every authed request and the pin silently never cached anything on the XSUAA path. Fix: cache the resolved sapId on `user.attr.sapId` (always a plain writable object) as the source of truth, and only mirror onto `authInfo.token.userId` when that slot is actually writable (checked via getOwnPropertyDescriptor, wrapped defensively). The idempotency short-circuit now reads `user.attr.sapId` first, falling back to the token for older pins. The dev/basic-auth path (writable token) still mirrors userId as before. Test: 4 new cases in test/unit/resolve-db-user-resolve.test.js exercising a getter-only token (reproducing the XsuaaToken), asserting no throw, sapId cached on attr, idempotent re-call, and the writable-token mirror path. All 34 tests in the file pass (`vitest run`). Paired with the CronService guard in this PR β€” both are #2552 before('*') identity-pin regressions from 1.40.0. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- packages/core/resolve-db-user.js | 28 ++++++++-- test/unit/resolve-db-user-resolve.test.js | 62 ++++++++++++++++++++++- 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/packages/core/resolve-db-user.js b/packages/core/resolve-db-user.js index 62dff642b..7d0fa3887 100644 --- a/packages/core/resolve-db-user.js +++ b/packages/core/resolve-db-user.js @@ -348,8 +348,12 @@ export async function resolveUser(user, db) { */ export async function pinResolvedUser(user, db) { if (!user || !user.id || user.id === 'anonymous') return null; - // Idempotent: if already pinned this request, don't re-resolve. - if (user.attr?.dbUserId) return { ID: user.attr.dbUserId, sapId: user.authInfo?.token?.userId ?? null }; + // Idempotent: if already pinned this request, don't re-resolve. Read the + // cached sapId from user.attr (always writable) first, falling back to the + // token for older pins. + if (user.attr?.dbUserId) { + return { ID: user.attr.dbUserId, sapId: user.attr.sapId ?? user.authInfo?.token?.userId ?? null }; + } let row; try { row = await resolveUser(user, db); @@ -362,9 +366,23 @@ export async function pinResolvedUser(user, db) { user.attr = user.attr || {}; user.attr.dbUserId = row.ID; if (row.sapId) { - user.authInfo = user.authInfo || {}; - user.authInfo.token = user.authInfo.token || {}; - user.authInfo.token.userId = row.sapId; + // Cache sapId on user.attr (always a plain writable object). + user.attr.sapId = row.sapId; + // Best-effort mirror onto authInfo.token.userId for any consumer that still + // reads it there. In PROD the real @sap/xssec XsuaaToken exposes `userId` + // as a GETTER-ONLY property, so a bare assignment throws + // `Cannot set property userId of # which has only a getter` + // (#2552). Only assign when the slot is actually writable. + try { + user.authInfo = user.authInfo || {}; + user.authInfo.token = user.authInfo.token || {}; + const desc = Object.getOwnPropertyDescriptor(user.authInfo.token, 'userId'); + const writable = !desc || desc.writable || typeof desc.set === 'function'; + if (writable) user.authInfo.token.userId = row.sapId; + } catch { + // Getter-only token (production XSUAA): the attr.sapId cache above is the + // source of truth; swallow and continue. + } } return row; } diff --git a/test/unit/resolve-db-user-resolve.test.js b/test/unit/resolve-db-user-resolve.test.js index 8f15d61d5..c3eac9342 100644 --- a/test/unit/resolve-db-user-resolve.test.js +++ b/test/unit/resolve-db-user-resolve.test.js @@ -62,7 +62,7 @@ function installSelect() { }); } -const { resolveUser, issuerSubjectFromUser, githubIdentityFromUser } = await import('../../packages/core/resolve-db-user.js'); +const { resolveUser, issuerSubjectFromUser, githubIdentityFromUser, pinResolvedUser } = await import('../../packages/core/resolve-db-user.js'); const iasUser = (over = {}) => ({ id: 'thomas.jung@sap.com', @@ -379,3 +379,63 @@ describe('resolveUser β€” GitHub provider-canonical link (Tier 0)', () => { }); }); +// #2552 regression: in PROD the @sap/xssec XsuaaToken exposes `userId` as a +// GETTER-ONLY property. pinResolvedUser must not throw +// `Cannot set property userId of # which has only a getter`; it +// caches the resolved sapId on user.attr (always writable) instead. +describe('pinResolvedUser β€” getter-only token userId (#2552)', () => { + beforeEach(() => { captured = undefined; stub = {}; installSelect(); installInsert(); installUpdate(); }); + afterEach(() => { delete globalThis.SELECT; delete globalThis.INSERT; delete globalThis.UPDATE; }); + + // A user whose authInfo.token.userId is a read-only getter, like the real + // production XsuaaToken. + const getterOnlyTokenUser = () => { + const token = { payload: { + iss: 'https://atxgsg7zi.accounts.ondemand.com', + sub: 'thomas.jung@sap.com', + } }; + Object.defineProperty(token, 'userId', { + get() { return undefined; }, // getter only β€” no setter + enumerable: true, + configurable: false, + }); + return { id: 'thomas.jung@sap.com', attr: {}, authInfo: { token } }; + }; + + it('does not throw when userId is a getter-only property', async () => { + stub.identityRow = { user_ID: 'u-1' }; + stub.usersRows = [{ ID: 'u-1', sapId: 'I809764', email: 'thomas.jung@sap.com' }]; + const user = getterOnlyTokenUser(); + const row = await pinResolvedUser(user); + expect(row?.ID).toBe('u-1'); + expect(row?.sapId).toBe('I809764'); + }); + + it('caches dbUserId and sapId on user.attr for the getter-only token', async () => { + stub.identityRow = { user_ID: 'u-1' }; + stub.usersRows = [{ ID: 'u-1', sapId: 'I809764', email: 'thomas.jung@sap.com' }]; + const user = getterOnlyTokenUser(); + await pinResolvedUser(user); + expect(user.attr.dbUserId).toBe('u-1'); + expect(user.attr.sapId).toBe('I809764'); + }); + + it('idempotent: a second call with a pinned user does not re-resolve or throw', async () => { + const user = { id: 'thomas.jung@sap.com', attr: { dbUserId: 'u-1', sapId: 'I809764' } }; + const row = await pinResolvedUser(user); + expect(row).toEqual({ ID: 'u-1', sapId: 'I809764' }); + }); + + it('still mirrors userId onto a writable token (dev/basic-auth path)', async () => { + stub.identityRow = { user_ID: 'u-2' }; + stub.usersRows = [{ ID: 'u-2', sapId: 'I000001', email: 'dev@example.com' }]; + const user = { + id: 'dev@example.com', attr: {}, + authInfo: { token: { payload: { iss: 'https://atxgsg7zi.accounts.ondemand.com', sub: 'dev@example.com' } } }, + }; + await pinResolvedUser(user); + expect(user.authInfo.token.userId).toBe('I000001'); + expect(user.attr.sapId).toBe('I000001'); + }); +}); + From 62a0c13a4fb7c133c8fce9c56e1cb21f10203064 Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Mon, 19 Oct 2026 07:05:31 -0400 Subject: [PATCH 03/12] fix(#2839): white hero text for themed App Space banners The Joule/Sapphire App Space hero gradients are dark in both light and dark mode, but the base .hero only inherited --sapShell_TextColor, which resolves to a dark value under the Horizon light theme. That left the banner text near-illegible on the purple gradient (/app-space/?theme=joule in light mode, reported by DJ Adams). Pin the themed hero (and its loading skeleton) to white regardless of the active Horizon palette. Verified via vite dev harness: hero text is rgb(255,255,255) with the fix and rgb(34,53,72) without, in light+joule; dark mode unchanged. --- hugo-apps/src/app-space/AppSpace.vue | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/hugo-apps/src/app-space/AppSpace.vue b/hugo-apps/src/app-space/AppSpace.vue index 3e6225b18..63ae089e9 100644 --- a/hugo-apps/src/app-space/AppSpace.vue +++ b/hugo-apps/src/app-space/AppSpace.vue @@ -1325,6 +1325,25 @@ const emptyStateMessage = computed(() => { } } +/* ═══════════════════════════════════════════════════════════════════ + Themed heroes β€” text colour (#2839) + The Joule/Sapphire hero gradients are dark in both light AND dark mode, + but the base .hero only inherits --sapShell_TextColor, which resolves to + a DARK value in the Horizon light theme β†’ near-illegible banner text on + the purple/blue gradient. Pin white here (and on the skeleton) so the + themed hero reads correctly regardless of the active Horizon palette. + ═══════════════════════════════════════════════════════════════════ */ + +.app-space[data-theme="joule"] .hero, +.app-space[data-theme="sapphire"] .hero { + color: #fff; +} + +.app-space[data-theme="joule"] .hero-title-skeleton, +.app-space[data-theme="sapphire"] .hero-title-skeleton { + background: rgba(255, 255, 255, 0.18); +} + /* ═══════════════════════════════════════════════════════════════════ Joule Theme β€” Light Mode ═══════════════════════════════════════════════════════════════════ */ From 19db8d027878b8177fd89ed647e4391ccdd29e24 Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Mon, 19 Oct 2026 08:29:24 -0400 Subject: [PATCH 04/12] fix(options): keep OPTION-block figures nested in the list item (#2820) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit convertOptionBlocks spliced OPTION shortcodes flush-left, which closed the surrounding numbered list. Figures inside an os-panel β€” or trailing an OPTION group in the same substep β€” then rendered as a top-level
sibling of the list, losing their indentation under the substep (the 'centered vs left-aligned image' QA report). PR #2822 only adjusted figure text-align (CSS), which cannot re-nest a DOM sibling, so the figures stayed un-indented. Approach A: indent the shortcode WRAPPER lines to the substep so the block stays inside the
  • (list never closes), while the panel BODY stays at column 0 so os-panel.html's markdownify pass parses code fences/images as markdown (preserves the #2703 behaviour). The old flush-left tail-dedent is removed β€” with the list open, trailing content keeps its substep indent. Verified through real Hugo renders (incl. the live Tutorials-Contribution hana-clients-install source): figures nest as li > ol > figure; #2703 code fences unaffected. New render-level regression test added. --- scripts/__tests__/options-hugo.test.ts | 51 ++--- scripts/parsers/options.ts | 90 +++++---- test/unit/option-figure-list-nesting.test.ts | 191 +++++++++++++++++++ 3 files changed, 276 insertions(+), 56 deletions(-) create mode 100644 test/unit/option-figure-list-nesting.test.ts diff --git a/scripts/__tests__/options-hugo.test.ts b/scripts/__tests__/options-hugo.test.ts index 41c76977e..4ddedd1ea 100644 --- a/scripts/__tests__/options-hugo.test.ts +++ b/scripts/__tests__/options-hugo.test.ts @@ -285,15 +285,17 @@ ML2 }); }) -describe('convertOptionBlocks (hugo) β€” indented OPTION blocks inside list items (#2703)', () => { +describe('convertOptionBlocks (hugo) β€” indented OPTION blocks inside list items (#2703, #2820)', () => { // When an OPTION block lives inside a numbered substep it is authored - // indented 4 spaces. The shortcode is emitted FLUSH-LEFT (dedented): the - // markdown list closes and Goldmark re-opens it with
      after - // the block, preserving substep numbering, while the panel BODY is dedented - // to column 0 so os-panel.html's `{{ .Inner | markdownify }}` parses the - // code fences / images / notes as markdown instead of literal indented code. - - it('emits the shortcode flush-left and dedents the body for an indented OS group', () => { + // indented 4 spaces. Approach A (#2820): the shortcode WRAPPER lines keep that + // indentation so the block stays inside the surrounding
    1. (the list never + // closes β€” this is what keeps figures nested, see + // test/unit/option-figure-list-nesting.test.ts for the render-level proof), + // while the panel BODY is dedented to column 0 so os-panel.html's + // `{{ .Inner | markdownify }}` parses the code fences / images / notes as + // markdown instead of literal indented code. + + it('indents the shortcode wrappers to the substep and dedents the body for an indented OS group', () => { const input = [ '3. Start the installer.', '', @@ -319,12 +321,12 @@ describe('convertOptionBlocks (hugo) β€” indented OPTION blocks inside list item const out = convertOptionBlocks(input, 'hugo'); - // Shortcode wrappers are flush-left (NOT indented) so Goldmark treats the - // block as a sibling of the list; the following `4.` reopens numbering. - expect(out).toContain('\n{{< os-options >}}'); - expect(out).toContain('\n{{< /os-options >}}'); - expect(out).toContain('{{< os-panel os="Windows" >}}'); - expect(out).not.toContain(' {{< os-options >}}'); + // Shortcode wrappers carry the substep's 4-space indent so Goldmark keeps + // the block inside the
    2. (list stays open; #2820). The body stays at + // column 0. + expect(out).toContain('\n {{< os-options >}}'); + expect(out).toContain('\n {{< /os-options >}}'); + expect(out).toContain(' {{< os-panel os="Windows" >}}'); // The code fence inside the panel is dedented to column 0 so markdownify // parses it as a fenced block (not a 4-space indented-code block). @@ -333,8 +335,6 @@ describe('convertOptionBlocks (hugo) β€” indented OPTION blocks inside list item // The following substep "4." survives at column 0. expect(out).toContain('\n4. Next substep.'); - // No residual leading indent left on the first wrapper line. - expect(out).not.toMatch(/\n {4}\{\{< os-options/); }); it('dedents the body of an indented legacy (non-OS) option-tabs group', () => { @@ -359,7 +359,8 @@ describe('convertOptionBlocks (hugo) β€” indented OPTION blocks inside list item const out = convertOptionBlocks(input, 'hugo'); expect(out).toContain('{{% option-tabs tabs="JSON,XML" %}}'); - expect(out).not.toContain(' {{% option-tabs'); + // Approach A (#2820): wrappers carry the substep indent; body at column 0. + expect(out).toContain(' {{% option-tabs tabs="JSON,XML" %}}'); // Body dedented to column 0. expect(out).toContain('\njson body\n'); expect(out).toContain('\n3. Done.'); @@ -421,7 +422,7 @@ describe('convertOptionBlocks (hugo) β€” real QA substep patterns (#2703 follow- expect(out).not.toMatch(/\n {4}```\n/); }); - it('dedents substep content that trails the OPTION group within the same list item', () => { + it('leaves substep content that trails the OPTION group at its substep indent (stays in the list item)', () => { const input = [ '3. Start the installer.', '', @@ -454,11 +455,15 @@ describe('convertOptionBlocks (hugo) β€” real QA substep patterns (#2703 follow- const out = convertOptionBlocks(input, 'hugo'); - // Trailing substep-3 content must be dedented to column 0 so Goldmark keeps - // it as list-item prose (not an indented-code block) and substep 4 reopens. - expect(out).not.toMatch(/\n {4}Set the install directory/); - expect(out).not.toMatch(/\n {4}!\[Client install\]/); - expect(out).not.toMatch(/\n {4}> \[!TIP\]/); + // Approach A (#2820): the shortcode wrappers stay indented, so the list + // never closes. Trailing substep-3 content therefore KEEPS its 4-space + // substep indent and remains valid list-item continuation β€” dedenting it to + // column 0 (the old behaviour) would eject it from the
    3. , which is the + // bug. See test/unit/option-figure-list-nesting.test.ts for the render-level + // proof that the trailing figure nests inside
    4. /
        . + expect(out).toMatch(/\n {4}Set the install directory/); + expect(out).toMatch(/\n {4}!\[Client install\]/); + expect(out).toMatch(/\n {4}> \[!TIP\]/); expect(out).toContain('\n4. After installation'); }); }) diff --git a/scripts/parsers/options.ts b/scripts/parsers/options.ts index 8c45272cd..811d5e99e 100644 --- a/scripts/parsers/options.ts +++ b/scripts/parsers/options.ts @@ -94,6 +94,26 @@ function dedentBlock(text: string): string { return dedented.join('\n'); } +/** Re-indent only the shortcode WRAPPER lines of a built replacement block by + * `indent`, leaving every body line at its current (column-0) position. + * + * Approach A for #2820: the OPTION shortcode must stay inside the surrounding + * numbered
      1. , so its wrapper lines carry the substep's indentation β€” but the + * panel body must stay at column 0 so os-panel.html's `{{ .Inner | markdownify }}` + * parses it as markdown rather than a literal indented-code block (#2703). + * + * A "wrapper line" is a Hugo shortcode delimiter (`{{< … >}}` / `{{% … %}}`) or + * a VitePress `` / `