Remove SKIP_MOBILE — always build & gate mobile in CI

[moodyjmz]

Closes #157.

Removes the SKIP_MOBILE escape hatch so CI can no longer skip building and gating mobile — the blind spot that let Euro-Office/DocumentServer#258 ship a broken mobile editor with green CI. Mobile now always builds and verify-deploy.mjs always asserts its artifacts.

Changes (web-apps)

• .github/workflows/e2e.yml — drop SKIP_MOBILE: '1' from the build step. This is the change that matters: it arms the mobile gate Mobile editors: fix not-loading + ES2022 target / i18next crash #156 added.
• build/scripts/build-pipeline.js — mobile install + the 4 editor builds + Phase 2 deploy-mobile are now unconditional.
• build/scripts/verify-deploy.mjs — drop the if (!SKIP_MOBILE) wrapper; mobile artifacts always asserted.
• Readme.md / build/README.md / docs/webpack-migration.md — remove all SKIP_MOBILE documentation.

No deprecation shim: the flag was one day old with only two internal call sites (this workflow + the DocumentServer Makefile), so there's no installed base to warn.

Relationship to #154

#154 Tier 1 (mobile manifest gate) already landed in #156, just gated behind SKIP_MOBILE. This PR makes it unconditional and completes #154's entire web-apps footprint. #154 Tier 2 (real-browser mobile smoke) lives in documentserver/e2e — a separate DocumentServer PR.

Follow-up (not here — DocumentServer repo)

• develop/setup/Makefile still forwards SKIP_MOBILE. It's now a silent no-op (harmless — mobile builds regardless), but dead cruft to remove.
• CI: add mobile-editor validation to build gates and e2e (regression #258) #154 Tier 2 mobile Playwright smoke test.

Verification

• git grep SKIP_MOBILE across web-apps → nothing.
• node --check passes on both edited scripts.
• ⚠️ This is the first time CI will ever build mobile (every prior e2e run had SKIP_MOBILE=1). Watch the build step — if framework7-react has a CI-specific install/build quirk, it surfaces here for the first time.

🤖 Generated with Claude Code

[MonaAghili]

PR #158 Review — Remove SKIP_MOBILE: always build & gate mobile in CI

---

Summary

The mechanical changes are correct: every if (!SKIP_MOBILE) guard is gone, the YAML flag is dropped, docs are cleaned up. The intent is sound. However, this is the first time CI has ever built mobile, and that exposes several gaps — two of them capable of silently hanging the pipeline or producing an incomplete deploy with a green exit code.

Seven findings, ranked by severity.

---

Finding 1 — CONFIRMED / HIGH

WATCH=1 from the parent shell leaks into every mobile webpack build → infinite hang

File: build/scripts/build-pipeline.js:79

CHILD_ENV is built as:

const CHILD_ENV = { ...process.env, BUILD_ROOT, PRODUCT_VERSION, BUILD_NUMBER, THEME, NODE_ENV };

WATCH is never explicitly set to '0' or deleted. vendor/framework7-react/build/webpack.config.js line 56 activates webpack watch mode when process.env.WATCH === '1'. If a developer ran the pipeline from a shell where WATCH=1 was exported (common during a mobile dev session), or if any future CI step exports it, all 4 mobile:* spawns enter watch mode and never exit. phase() uses Promise.all with no timeout — it hangs forever. e2e.yml has no WATCH: '0' defensive override.

Failure scenario: Developer runs WATCH=1 node scripts/build-pipeline.js (or a CI runner inherits it) → all 4 mobile:word/cell/slide/visio processes enter webpack watch mode → phase('Phase 1 — parallel') never resolves → pipeline stalls until GitHub Actions kills the job at 6 hours.

Fix:

const CHILD_ENV = {
    ...process.env,
    BUILD_ROOT, PRODUCT_VERSION, BUILD_NUMBER, THEME, NODE_ENV,
    WATCH: '0',   // never inherit watch mode from the parent shell
};

---

Finding 2 — CONFIRMED / HIGH

No cache: npm in e2e.yml → 1,123 framework7-react packages downloaded cold on every CI run

File: .github/workflows/e2e.yml:54

setup-node@v4 is configured with only node-version: 20 — no cache: key. ~/.npm is never restored between runs. The mobile:install step inside build-pipeline.js runs npm install --include=dev --production=false in vendor/framework7-react/ which has an npm-shrinkwrap.json pinning 1,123 packages (1,067 of them devDependencies, ~980 KB lockfile). Every PR run re-downloads all of them cold from the npm registry. This is also the step that blocks all 11 Phase 1 tasks from starting (see Finding 3).

Failure scenario: npm registry slowness or a transient network hiccup on the ubuntu-latest runner → cold install times out or yields partial node_modules → all 4 mobile webpack builds fail with Cannot find module 'webpack' → Phase 1 aborts → build fails before a single desktop bundle is produced.

Fix:

- uses: actions/setup-node@v4
  with:
    node-version: 20
    cache: npm
    cache-dependency-path: |
      web-apps/build/package-lock.json
      web-apps/vendor/framework7-react/npm-shrinkwrap.json

---

Finding 3 — CONFIRMED / HIGH

mobile:install is awaited before phase('Phase 1 — parallel') — all 11 non-mobile tasks sit idle while npm installs

File: build/scripts/build-pipeline.js:264

phase1Tasks is fully populated (5 deploy scripts + 6 webpack builds) before mobile:install is awaited. Phase 1 cannot start until the await resolves:

// phase1Tasks already has 11 tasks here
const install = await runTask(task('mobile:install', 'npm', ...)).promise;  // line 264 — BLOCKS
// ...4 mobile tasks pushed...
const p1 = await phase('Phase 1 — parallel', phase1Tasks);  // line 278 — starts only after install

Before this PR, SKIP_MOBILE=1 in CI meant Phase 1 started immediately. Now every CI run waits for a full npm install before sprites, deploy-common, or any webpack build can emit a byte. On a cold runner with no cache, this is 60–120 s of serialised dead time on the critical path.

Failure scenario: Cold CI runner + no cache (see Finding 2) → 90 s npm install → 11 Phase 1 tasks that have zero dependency on mobile sit idle for the entire install duration → wall-clock critical path grows by the full install time with no amortisation across parallel tasks.

---

Finding 4 — CONFIRMED / MEDIUM

deploy-mobile.js silently skips killed editors, letting Phases 3-4 run against an incomplete BUILD_ROOT

File: build/scripts/deploy-mobile.js:100

When one of the 4 parallel mobile webpack builds fails, phase() SIGTERMs the other three. The killed builds never flush HtmlWebpackPlugin's index.html. deploy-mobile soft-skips any editor whose source index.html is absent and exits 0 with no error or warning. Phase 3 (deploy-resources, deploy-theme-images) and Phase 4 (inline-svgs) then run against a partially-populated BUILD_ROOT. Only in Phase 5 does verify-deploy fail — but by then two more phases of wasted work have run.

Failure scenario: mobile:slide build fails mid-run → mobile:word/cell/visio killed by SIGTERM before HtmlWebpackPlugin flushes → deploy-mobile skips those 3 editors silently, exits 0 → Phase 3 runs → Phase 4 runs → verify-deploy finally exits 1 naming 3 × 6 missing artifacts. The failure is caught, but later than it should be and with a misleading trail.

---

Finding 5 — CONFIRMED / MEDIUM

mobile:install result is discarded — invisible in the build summary

File: build/scripts/build-pipeline.js:315

const all = [...p0, ...p1, ...pMobile, ...p2, ...p3, ...p4];  // install never spread here

The { label, ms, code } object from runTask(mobile:install) is used only for the exit-code check and then dropped. The final per-task timing table never shows mobile:install. If it runs for 90 s and is the build bottleneck, the summary gives no accounting for that time — the operator sees a wall-clock total inflated by 90 s with no matching task row.

---

Finding 6 — PLAUSIBLE / MEDIUM

framework7.css and framework7-rtl.css are invisible to verify-deploy's CSS check

File: build/scripts/verify-deploy.mjs:120

The mobile CSS verification:

for (const [, ref] of html.matchAll(/href="(css\/[^"]+\.css)"/g)) {
    checkFile(`${base}/${ref}`);
}

framework7-bundle.css and framework7-bundle-rtl.css are copied to css/framework7.css and css/framework7-rtl.css by CopyWebpackPlugin. They are loaded via JavaScript (load_stylesheet('./css/framework7.css')), not via a static <link href> tag. They never appear in index.html as href= attributes. checkDir('css/') passes as long as app.<hash>.css exists.

Failure scenario: A framework7 version bump renames framework7-bundle.css → CopyWebpackPlugin source path breaks → framework7.css absent from deploy → verify-deploy passes (contenthash CSS file still present, regex finds no framework7 href) → mobile editor crashes at runtime for every user loading the stylesheet.

---

Finding 7 — PLAUSIBLE / LOW

Issue #157 required a stray-SKIP_MOBILE warning; none was implemented

File: build/scripts/build-pipeline.js:70 (location of removed constant)

Issue spec item 2 explicitly states: "Log a warning if a stray SKIP_MOBILE env is set-but-ignored." grep SKIP_MOBILE build-pipeline.js returns nothing — process.env.SKIP_MOBILE is never read. The DocumentServer Makefile still forwards SKIP_MOBILE (acknowledged in the PR description as dead cruft). A DevOps engineer passing SKIP_MOBILE=1 to skip mobile in a custom build receives no feedback; mobile runs regardless.

The PR description justifies the omission ("one day old, two internal call sites"), but the requirement is unambiguously unimplemented.

Fix:

if (process.env.SKIP_MOBILE) {
    process.stderr.write('warning: SKIP_MOBILE is set but no longer honoured — mobile always builds\n');
}

---

Findings not confirmed

Candidate	Verdict	Reason
rm('./www/') race between 4 parallel mobile builds	Refuted	vendor/framework7-react/src/static/ does not exist on disk or in git; CopyWebpackPlugin patterns use noErrorOnMissing: true; nothing is written to www/, making the race inert
NODE_ENV=production in CHILD_ENV causes npm to skip devDeps	Refuted for CI	npm 10 (bundled with Node.js 20) honours --production=false over NODE_ENV. Risk exists only on npm ≤6 (local dev on old runtimes)
Math.max(...[]) → RangeError on empty all[]	Refuted (unreachable)	Every phase either exits the process on failure or returns a non-empty array; all can never be empty in the current code paths