Skip to content

fix(build): validate platform build options at the CLI/MCP entry - #874

Open
qiuguohua wants to merge 9 commits into
cocos:mainfrom
qiuguohua:fix/build-platform-param-validation
Open

fix(build): validate platform build options at the CLI/MCP entry#874
qiuguohua wants to merge 9 commits into
cocos:mainfrom
qiuguohua:fix/build-platform-param-validation

Conversation

@qiuguohua

Copy link
Copy Markdown
Contributor

What this PR does

The CLI/MCP build entry did not validate platform build options at all: an invalid value either blew up much later inside the native packaging step, or silently produced a wrong package. The editor validates the same options in three places (verifyRuleMap, verificationFunc, getVerifyMap, plus panel-level checks). This PR adds a single validation gate at the CLI/MCP entry and migrates the editor rules that were missing on the CLI side into the platform verifyRuleMaps, which is the only validation entry the CLI has.

1. Entry-layer validation

verifyBuildOptions(platform, options) runs before build() in Launcher.build (CLI) and BuilderApi.build (MCP):

  • Platform defaults are merged into the options first, so a field the caller omits falls back to its platform default and passes.
  • An explicitly passed invalid value is a hard failure: the build never starts and the process exits with PARAM_ERROR (32), with a per-field reason.
  • skipCheck: true bypasses validation entirely.
  • The caller's options object is never mutated (validation runs on a deep clone).
  • renderPipeline is resolved from the engine config. It is a project setting that checkProjectSetting only fills inside the build task, so rules depending on it (apiLevelRenderPipeline) could never fire at the entry. This matches the editor, which reads the project profile directly.

2. Rules migrated from the editor

Platform Fields
ios targetVersion (format, TaskFlow minimum 12.0, general minimum 11.0), orientation, osTarget
windows renderBackEnd (was only checked in the editor UI layer)
ohos packageName, orientation
harmonyos-next packageName (length 7..128 + reserved keyword tokens), appABIs, renderBackEnd, orientation, deviceTypes
android / google-play conditional keystore rules when useDebugKeystore is off, apiLevel chain split into named rules

harmonyos-next renderBackEnd now exposes vulkan / gles3 / gles2 and declares an object-level default (gles3: true), so the value is no longer undefined at build time.

3. Bugs found while migrating

  • huawei-agc could not build at all. It spreads the android config, so it inherited gates that read a hardcoded options.packages.android. Its own options live under options.packages['huawei-agc'], so useDebugKeystore resolved to undefined, the keystore rules fired, and every huawei-agc build failed with PARAM_ERROR. All linked gates (useDebugKeystore, JobSystem, resizeableActivity, androidInstant) now resolve the package by options.platform.
  • apiLevelTbb read options.packages.native.JobSystem, a path that does not exist in the CLI (the CLI spreads the native common options into each platform package), so the rule never fired.
  • The ios version compare padded only the first separator and judged '9.10' >= '11.0'; replaced with a segment-wise compare.
  • validatorManager short-circuited on empty values, which made conditional required rules unreachable. Empty values now also reach rules whose name ends with Required.
  • google-play logs why the API Level dropdown is empty (no SDK path, no platforms directory, or nothing installed at or above the Google Play minimum of 24).

4. Behaviour changes to be aware of

  • An invalid option value now fails the build instead of being silently replaced by the platform default. Previously, when a rule reported a fixedValue, the error was downgraded to a warning and the build continued with the substituted value; the android/google-play hooks also bumped apiLevel silently. Both are gone.
  • Projects with a saved build profile that has useDebugKeystore: false and empty keystore fields are now blocked, even when no arguments are passed. Fix the profile or set useDebugKeystore: true.
  • Exit code for a rejected option set is 32 (PARAM_ERROR), not the generic build failure code.

5. Tests

npx jest src/core/builder/test — 27 suites / 310 tests.

  • platform-verify-rules.spec.ts — per-platform rule unit tests, plus huawei-agc end-to-end through the real validatorManager.
  • verify-build-options.spec.ts — entry-layer unit tests with pluginManager stubbed (default merge, skipCheck, PARAM_ERROR, warnings do not block).
  • verify-build-options-integration.spec.ts — entry-layer integration tests that register the real android/ios configs into the real pluginManager and drive the whole chain, including the renderPipeline sources.
  • validator-manager-shortcircuit.spec.ts — the empty-value / Required suffix convention.

QA test guide

Setup

  1. Any Cocos project (2D or 3D, empty is fine). Below it is referred to as <PROJECT>.
  2. A working Android SDK/NDK if you want the android/google-play builds to actually finish. Validation itself does not need them — a rejected option set fails before any SDK work.
  3. Build options are passed through a JSON config file: cocos-cli build -j <PROJECT> -p <platform> -c <config.json>. Only the keys you write are overridden; every other field falls back to its platform default, which is why some cases below have to switch off a default explicitly.
  4. After every command, check the exit code, that is the main thing this PR changes:
    • PowerShell: $LASTEXITCODE
    • bash: echo $?

Expected exit codes: 0 = success, 32 = option validation rejected the build, other non-zero = build failure.

Part A — invalid options must be rejected with exit code 32

For each case: write the JSON to config.json, run
cocos-cli build -j <PROJECT> -p <platform> -c config.json,
then confirm exit code 32, that the printed reason names the field, and that no package was produced (<PROJECT>/build/<platform> is not created or not updated — check the folder timestamp).

# Platform config.json Expected message contains
A1 android {"packages":{"android":{"apiLevel":10}}} apiLevel, The minimum API Level required is 19.
A2 android {"packages":{"android":{"apiLevel":"abc"}}} API Level must be a number
A3 android {"packages":{"android":{"androidInstant":true,"apiLevel":22}}} the minimum API Level required is 23
A4 android {"packages":{"android":{"packageName":"123abc"}}} Invalid package name specified
A5 android {"packages":{"android":{"useDebugKeystore":false}}} all four of keystorePath / keystorePassword / keystoreAlias / keystoreAliasPassword, each with Cannot be empty when useDebugKeystore is false
A6 android {"packages":{"android":{"renderBackEnd":{"vulkan":false,"gles3":false,"gles2":false}}}} renderBackEnd must have at least one supported backend enabled
A7 android {"packages":{"android":{"orientation":{"portrait":false,"upsideDown":false,"landscapeLeft":false,"landscapeRight":false}}}} orientation must have at least one direction enabled
A8 android {"packages":{"android":{"resizeableActivity":false,"maxAspectRatio":"1.0"}}} maxAspectRatio must be a number, "w:h", or "n.n (w:h)" with value >= 1.33
A9 google-play {"packages":{"google-play":{"apiLevel":21}}} Google Play requires the minimum API Level to be 24.
A10 windows {"packages":{"windows":{"renderBackEnd":{"vulkan":false,"gles3":false,"gles2":false}}}} renderBackEnd must have at least one supported backend enabled
A11 ios {"packages":{"ios":{"packageName":"com.qa.test","targetVersion":"9.10"}}} The minimum target version required is 11.0.
A12 ios {"packages":{"ios":{"packageName":"com.qa.test","targetVersion":"12"}}} targetVersion must look like "12.0" or "12.0.1"
A13 ios {"packages":{"ios":{"packageName":"com.qa.test","osTarget":{"iphoneos":false,"simulator":false}}}} osTarget must have at least one target enabled
A14 ohos {"packages":{"ohos":{"packageName":"123abc"}}} Invalid package name specified
A15 harmonyos-next {"packages":{"harmonyos-next":{"packageName":"com.harmony.game"}}} Invalid package name specified (reserved keyword token)
A16 harmonyos-next {"packages":{"harmonyos-next":{"packageName":"com.qa"}}} Invalid package name specified (shorter than 7 characters)
A17 harmonyos-next {"packages":{"harmonyos-next":{"packageName":"com.qa.testgame","deviceTypes":{"default":false}}}} deviceTypes must have at least one device type enabled

A18 — project-setting driven case, needs a project change: open <PROJECT> in the editor, set the render pipeline to Deferred, then build android with {"packages":{"android":{"apiLevel":20}}}. Expected: exit code 32 and When Deferred Render Pipeline is enabled, the minimum API Level required is 21. Switch the pipeline back to Forward and the same command must succeed. This is the case that only works because renderPipeline is now read from the engine config at the entry.

Part B — valid options must still build

# Platform config.json Expected
B1 android {} build succeeds, exit 0 (all defaults are valid and back-filled)
B2 android {"packages":{"android":{"apiLevel":24}}} build succeeds
B3 android {"packages":{"android":{"useDebugKeystore":false,"keystorePath":"<a real .keystore>","keystorePassword":"...","keystoreAlias":"...","keystoreAliasPassword":"..."}}} build succeeds and signs with that keystore
B4 google-play {} build succeeds
B5 huawei-agc {} build succeeds. This platform was broken before the fix in this PR — it failed immediately on the keystore rules. Please test it explicitly.
B6 harmonyos-next {} build succeeds; check the generated CMakeLists/cMakeConfig contains CC_USE_GLES3=true, plus CC_USE_VULKAN=false and CC_USE_GLES2=false (these two lines are new and expected)
B7 ios {"packages":{"ios":{"packageName":"com.qa.testgame"}}} build succeeds
B8 windows / mac / web-mobile / web-desktop / ohos {} build succeeds — smoke test that no platform regressed

B9 — escape hatch: {"skipCheck":true,"packages":{"android":{"apiLevel":10}}} must not be rejected by validation (exit code is whatever the build itself returns). Use this to confirm there is a way past the gate.

Part C — regression checks on existing projects

  • C1 (important) Take a project whose saved build profile already has useDebugKeystore: false with empty keystore fields, and run cocos-cli build -j <PROJECT> -p android with no config file. Expected: exit code 32 listing the four keystore fields. This is an intentional behaviour change — previously such a project would start building and fail later or produce an unsigned/incorrectly signed package. Filling the fields, or setting useDebugKeystore: true, must make it build again.
  • C2 ios packageName is required and its default is empty: cocos-cli build -j <PROJECT> -p ios with no config must be rejected with a packageName message. Passing a valid packageName must build.
  • C3 Editor parity spot-check: for a handful of the Part A cases, enter the same value in the editor's build panel and confirm the editor reports an equivalent error. The CLI is intentionally stricter in one spot: renderBackEnd only accepts backend keys the platform supports, so {"metal":true} on android is rejected by the CLI while the editor accepts it.

Part D — MCP path

Call the builder-build tool with the same invalid options as A1 (for example via the MCP client: platform android, options {"packages":{"android":{"apiLevel":10}}}). Expected: the call returns a failure with code: 32 and a reason naming apiLevel — the same gate, not an exception or a started build.

Part E — google-play API Level list logging

In the google-play build panel, open the API Level dropdown and watch the console/log:

  • Normal machine: if some installed SDK platforms are below 24, a log line lists which ones were excluded.
  • Machine with no Android SDK configured: Android SDK path not found, cannot list available API Levels.
  • Machine whose $SDK/platforms only has entries below android-24: the dropdown is empty and a warning names the directory, lists what is installed, and says to install API Level 24 or above. (Easy to reproduce by pointing the SDK path at a copy of the directory with the android-24+ folders removed.)

What a failure looks like

If any Part A case builds instead of stopping, or any Part B case is rejected, that is a bug — please attach the config file, the platform, the printed reason, and the exit code.

@qiuguohua qiuguohua changed the title fix(build): validate platform build options at the CLI/MCP entry fix(build): validate platform build options at the CLI/MCP entry. Aug 20, 2026
CLI and MCP paths bypassed the panel's UI validation, so invalid params
(e.g. malformed android packageName) silently reached the build.
Add verifyBuildOptions in core/builder that runs pluginManager.checkBuildOptions
and is invoked from Launcher.build (CLI) and BuilderApi.build (MCP).
Pink stays untouched — it already validates via its UI layer.

Errors with a fixedValue are downgraded to warn to preserve the
existing default-fallback behaviour from createBuildTask.checkOptions.

Tests: 9 cases covering skipCheck, all-valid, warn-only, error aggregation,
error-with-fixedValue downgrade, hard error without fixedValue, missing
message fallback, checker exception, and undefined options.
Editor's getVerifyMap for android checks keystorePath / keystorePassword /
keystoreAlias / keystoreAliasPassword must be non-empty when
useDebugKeystore is false. CLI's android platform config had no verifyRules
for these fields, so releases with a missing custom keystore silently
proceeded until the packaging step failed.

Add a shared 'keystoreRequired' rule to android verifyRuleMap that
short-circuits when useDebugKeystore is true; wire it into the four
keystore fields via verifyRules. Regex packageName rule unchanged.
Mirror of the android change: editor's google-play getVerifyMap requires
keystorePath / keystorePassword / keystoreAlias / keystoreAliasPassword
to be non-empty when useDebugKeystore is false. CLI had no verifyRules
for these fields.

Add 'keystoreRequired' rule reading options.packages['google-play'].useDebugKeystore,
wire it into the four keystore fields via verifyRules.
Editor's harmonyos-next packageName check has four gates: not empty,
regex format, length 7..128, and no HarmonyOS reserved keywords in any
segment (openharmony/harmonyos/harmony/system/ohos/oh). CLI only ran the
regex, so package names like 'com.harmony.app' would pass the CLI and
later fail at the HAP signing / AGC upload step.

Extend the packageName rule func to include the length check and the
token-aware keyword check (dot-segments, word-boundary, case-insensitive).
appABIs rule unchanged.
Entry-layer validation (verifyBuildOptions):
- merge the platform defaults into the options before checking, so a field
  the caller omits falls back to its platform default instead of failing
- an explicitly passed invalid value is now a hard failure (PARAM_ERROR,
  exit code 32). The previous "downgrade to a warning when the rule reports
  a fixedValue" path is removed, so a build can no longer start with a
  silently substituted value. Pass skipCheck to bypass validation entirely
- resolve renderPipeline from the engine config: it is a project setting
  that checkProjectSetting only fills inside the build task, so any rule
  depending on it (apiLevelRenderPipeline) could never fire at the entry.
  This matches the editor, which reads the project profile directly

Rules migrated from the editor verificationFunc / getVerifyMap into
verifyRuleMap, the only validation entry the CLI has:
- ios: targetVersionStyle / targetVersionTaskFlow / targetVersionMin,
  orientation, osTarget
- windows, harmonyos-next: renderBackEnd (at least one supported backend)
- ohos, harmonyos-next: orientation; harmonyos-next: deviceTypes, appABIs
- harmonyos-next renderBackEnd now exposes vulkan / gles3 / gles2 and
  declares an object-level default, so the value is no longer undefined

Fixes found while migrating:
- android / google-play / huawei-agc: the linked gates (useDebugKeystore,
  JobSystem, resizeableActivity, androidInstant) read
  options.packages[options.platform] instead of a hardcoded packages.android.
  huawei-agc inherits the android config, so the hardcoded path made every
  huawei-agc build fail with PARAM_ERROR on the keystore rules
- android / google-play: apiLevelTbb read packages.native.JobSystem, a path
  that does not exist in the CLI, so the rule never fired
- ios: use a segment-wise version compare, the previous helper padded only
  the first separator and judged '9.10' >= '11.0'
- validator-manager: empty values also reach rules whose name ends with
  Required, which conditional required rules need
- google-play: log why the API level list is empty (no SDK path, no
  platforms directory, or nothing installed at or above the Google Play
  minimum API level 24)

Tests: platform rule specs, an entry-layer integration spec that drives the
real pluginManager with the real platform configs, and the validator
empty-value short-circuit convention.
PR cocos#872 added `if (node instanceof Scene)` in scene-process/service/component.ts
after PR cocos#831 landed the test with a `jest.mock('cc', ...)` factory that had
no Scene export. Both merged the same day, so the interaction only surfaced
on main. Add Scene to the factory so the instanceof check resolves.
@qiuguohua
qiuguohua force-pushed the fix/build-platform-param-validation branch from 6d3fa4d to 81d938d Compare August 20, 2026 10:55
@qiuguohua qiuguohua changed the title fix(build): validate platform build options at the CLI/MCP entry. fix(build): validate platform build options at the CLI/MCP entry Aug 21, 2026
verifyBuildOptions was rejecting builds whose common options relied on
values computed from project state at build time:

- taskName defaults to '' with verifyRules ['required']; createBuildTask
  falls back to platform, but that fallback runs inside build() — after
  verifyBuildOptions had already returned PARAM_ERROR.
- startScene / scenes defaults are '' / [] but the real defaults come
  from asset-db via getDefaultStartScene / getDefaultScenes; the old
  checkOptions path recovered via fixedValue, which the new entry gate
  intentionally does not consume.

Fill these three fields on the merged options before running the
schema check, matching what the downstream build path already does.
Wrap the asset-db calls in try/catch so unit / uninitialized-engine
paths keep working.

Add unit coverage for the taskName / startScene / scenes normalization,
including the user-provided values still winning and the asset-db-throws
fallback path.

@tangkaikk tangkaikk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

creator上next只有一个gles3渲染后端。需要核对下,为什么需要其他渲染后端?

@tangkaikk tangkaikk Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

creator上next只有一个gles3渲染后端。需要核对下,为什么需要其他渲染后端?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Harmonyos next一直在发展,vulkan和gles2 也是支持的,之前只是gles2之前有问题,vulkan没有验证过,暂时关闭了。next一直在发展,我感觉cli应该可以让用户配置,pink应该可以关闭配置。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CLI如果可以配置的话,也会导致用户选择后,构建出错。对于用户来说,排查成本可能比较高。不支持或者还未验证的东西,还是不要保留,以免留下用户报错后,不必要的问题排查的工作量。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

如果确实要开放,就给测试,等测试验证没问题后,才放出去

@tangkaikk tangkaikk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

可以放开其他渲染后端,但是需要测试ok后

@star-e
star-e changed the base branch from main to v0.0.1-alpha.39 August 21, 2026 06:54
@star-e
star-e changed the base branch from v0.0.1-alpha.39 to main August 21, 2026 06:54
vulkan / gles2 haven't been validated on HarmonyOS Next yet, so
renderBackEnd's verifyRule intentionally keeps 'supported' at
['gles3']. Leave the full ['vulkan', 'gles3', 'gles2'] list in a
comment as a reminder to open it back up once those two backends
are proven stable.
@qiuguohua
qiuguohua changed the base branch from main to v0.0.1-alpha.39 August 21, 2026 07:06
@qiuguohua
qiuguohua changed the base branch from v0.0.1-alpha.39 to main August 21, 2026 07:07
…ted list

The docs commit narrowed renderBackEnd's `supported` array to
['gles3'] but this spec still expected vulkan / gles2 alone to
pass. Flip those two assertions to false and leave a comment
pointing at where the expectation should be opened back up when
vulkan / gles2 are validated.
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.

4 participants