Skip to content

Fix the unstartable production build, clean up what it ships, and close the dependabot round-trip - #26

Merged
oblomov-dev merged 4 commits into
mainfrom
claude/cap2ui5-modernisierung-23otcx
Aug 30, 2026
Merged

Fix the unstartable production build, clean up what it ships, and close the dependabot round-trip#26
oblomov-dev merged 4 commits into
mainfrom
claude/cap2ui5-modernisierung-23otcx

Conversation

@oblomov-dev

Copy link
Copy Markdown
Member

Merge after cap2UI5/builder-abap2UI5-js#26. The second commit mirrors core/ at a commit from that branch; merging this alone fails assemble's drift guard, and merging only that one fails it the other way.

The deployed app could not start

cds build --production stages gen/srv with the app's "abap2UI5": "file:./core" dependency and does not copy core/ there. Nothing failed loudly: npm ci created node_modules/abap2UI5 as a symlink to the missing directory (the lock records it as {"resolved":"core","link":true} and npm does not check link targets), the archive built, cf deploy succeeded — and the instance crash-looped on

Error: Cannot find module 'abap2UI5/engine'   (srv/server.js:3)

deploy-check passed throughout, because it only asserted that gen/srv/srv/server.js and gen/srv/package.json exist. They did.

src/scripts/vendor-core.js is the missing half of the build. npm run build:production chains it after cds build, mta.yaml's before-all calls that, and src/test/production-build.test.js gates it in the suite that gates every publish — it fails 3 of 6 assertions without the step.

What it ships

The same step drops openui5-dist from the staged tree, and the second commit removes it from the app's production dependency set at the source (devDependency here, optional peer of the framework). The deployed server never serves /resources: xs-app.json routes it to the ui5 destination.

Measured on the real build output:

before after
startable no yes
staged module 611 MB 19 MB
npm audit --omit=dev 43 (3 critical, 27 high) 0

Dependency updates, and why they never stuck

@cap-js/hana ^2→^3, @cap-js/cds-types ^0.18→^0.19, @cap-js/cds-test →^1.0.2, lock refreshed (@sap/cds 10.0.6, @sap/cds-dk 10.0.7, jest 30.5.0, express 5.2.1, mbt 1.2.49). The builder's own eslint 9→10 and jest, which clears the brace-expansion and js-yaml advisories; src/scripts is now in the lint scope.

Dependabot had no ecosystem watching the builder's own package.json — which is why it sat a major behind with two high advisories. Added. The app repo's npm ecosystem is retired in the same change set, because a bump merged in a generated repo is reverted by the next publish: @cap-js/cds-test went to ^1.0.2 in cap2UI5/cap2UI5#67 and back to ^1.0.1 in publish commit 22bf052 two days later. App dependencies now move only here, in src/.

Also: actions/checkout 5.1.0→7.0.1, actions/setup-node 5.0.0→7.0.0. This supersedes the six stale dependabot PRs (#14, #15, #18, #20, #24, #25).

One script change

assemble-cap.js no longer treats a zero-entry core lock merge as a defect on its own. That was sound while the core always had a dependency; with none, 0 is the right answer. It now fails only when the core's manifest declares dependencies the merge did not produce — the empty/renamed-lock case it was written for.

AGENTS.md records the coordination the drift guard demands when the core's own dependencies change, since mirror_core does not refresh the frozen "core" lock entry by itself.

Verification

builder 30 tests, app 50 tests (44 + 6 new), lint clean; the staged gen/srv installs with npm ci --omit=dev, loads, and audits clean.


Generated by Claude Code

claude added 2 commits August 30, 2026 09:05
The deployed app could not start. `cds build --production` stages gen/srv
with the app's `"abap2UI5": "file:./core"` dependency and does not copy
core/ there, so the pushed module resolved the framework to a dangling
symlink — npm ci creates it without checking the target — and the instance
died on srv/server.js:3 with `Cannot find module 'abap2UI5/engine'`. Every
step before that was green: cds build exited 0, mbt built the archive, cf
deploy succeeded, and deploy-check passed because it only asserted that
gen/srv/srv/server.js and gen/srv/package.json exist, which they did.

src/scripts/vendor-core.js is the missing half of the build. It copies the
vendored core next to the staged package.json and verifies the subpath
server.js requires resolves through the core's own exports map.
`npm run build:production` chains it after cds build, mta.yaml's before-all
calls that, and src/test/production-build.test.js gates the result in the
suite that gates every publish (it fails on 3 of 6 assertions without the
step).

The same step drops openui5-dist from the staged tree. It is the UI5
runtime cds watch serves at /resources locally; on BTP xs-app.json routes
/resources to the ui5 destination, so the deployed server never serves it —
and it is a deprecated package whose own dependencies are its release
tooling (npm@6, request, jsdom, simple-git). Staged module measured before
and after: 611 MB and 43 advisories (3 critical, 27 high) → 19 MB and none.
The prune bails out if the core ever declares a second dependency, since
the whole core/node_modules/ subtree of the app lock is reachable only from
openui5-dist today.

Dependencies, all green on the suite: @cap-js/hana ^2→^3, @cap-js/cds-types
^0.18→^0.19, @cap-js/cds-test →^1.0.2, and the lock refreshed (@sap/cds
10.0.6, @sap/cds-dk 10.0.7, jest 30.5.0, express 5.2.1, mbt 1.2.49). The
builder's own eslint 9→10 and jest, which clears the brace-expansion and
js-yaml advisories; src/scripts is now in the lint scope.

Dependabot: the builder's own package.json had no ecosystem watching it at
all, which is why it sat a major behind with two high advisories — added.
The app repo's npm ecosystem is retired in the same change set, because a
bump merged in a generated repo is reverted by the next publish (it
happened to @cap-js/cds-test: ^1.0.2 in cap2UI5/cap2UI5#67, back to ^1.0.1
in publish commit 22bf052 two days later). App dependencies now move only
here, in src/.

actions/checkout 5.1.0→7.0.1 and actions/setup-node 5.0.0→7.0.0.

builder 30 tests, app 50 tests (44 + the 6 new), lint clean, and the staged
gen/srv installs with `npm ci --omit=dev`, loads, and audits clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2ofjmLGtR9pKxi3m2KvMq
Follows builder-abap2UI5-js@815593e, which stopped declaring openui5-dist as
a runtime dependency of the framework and made it an optional peer instead.
This mirrors that core and adjusts the app around it, so the two land
together — the mirror alone would fail assemble's drift guard, and the lock
refresh alone would fail it the other way.

- src/package.json takes openui5-dist@1.113.0 as its own devDependency, so
  `cds watch` serves /resources exactly as before (verified:
  engine.ui5_resources_dir() resolves into the app's node_modules and
  sap-ui-core.js is there). It must stay a devDependency — production gets
  UI5 from the `ui5` destination.
- src/package-lock.json refreshed against the mirrored core, which is the
  coordination assemble's drift guard demands and which mirror_core does not
  do by itself. Recorded in AGENTS.md.
- assemble-cap.js no longer treats a zero-entry core lock merge as a defect
  on its own. It was a sound check when the core always had a dependency;
  with none, 0 is the right answer. It now fails only when the core's
  manifest declares dependencies that the merge did not produce — the
  empty/renamed-lock case it was written for. (The redundant second read of
  the core manifest in the drift guard goes with it.)

The effect on the app is the whole point: `npm audit --omit=dev` at the repo
root goes from 43 advisories (3 critical, 27 high) to 0. That was already
true of the staged gen/srv module after scripts/vendor-core.js pruned it;
now it is true at the source, and the prune is a defensive no-op for a
framework version that still declares the dependency.

builder 30 tests, app 50 tests, lint clean; the staged module installs with
`npm ci --omit=dev`, loads, and audits clean at 19 MB.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2ofjmLGtR9pKxi3m2KvMq
Copilot AI lite review requested due to automatic review settings August 30, 2026 10:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

claude added 2 commits August 30, 2026 11:36
builder-abap2UI5-js#26 is merged, so the mirror can point at the real core
rather than the intermediate one this branch was cut against. Picks up the
whole regenerated core — eight days of upstream that the stalled build_core
never published — plus the 997e73d5 wave fixes, notably the seven sample apps
that could not start.

The frozen "core" entry in src/package-lock.json still matches: the new core
declares no dependencies either, so assemble's drift guard is satisfied
without a lock change.

builder 30 tests, app 50 tests, lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2ofjmLGtR9pKxi3m2KvMq
The nightly committed a refreshed mirror to main (7fa4fad, upstream 68729ce)
while this branch was open, which conflicted on run/input/UPSTREAM_COMMIT.
Resolved by re-running mirror_core against the merged
builder-abap2UI5-js@802087b rather than by picking a side: only that token
conflicted, so git had auto-merged the core tree file by file, and a mirror
is only meaningful as one upstream revision taken whole.

802087b is the newer of the two — it is main with #26 merged — so this keeps
the regenerated core and the sample-app fixes.

builder 30 tests, app 50 tests, lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2ofjmLGtR9pKxi3m2KvMq
@oblomov-dev
oblomov-dev merged commit 3b72277 into main Aug 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants