Skip to content

ci: validate dash-app manifests in Android build - #163

Merged
wpmed92 merged 2 commits into
mainfrom
feat/manifest-validation
Jul 29, 2026
Merged

ci: validate dash-app manifests in Android build#163
wpmed92 merged 2 commits into
mainfrom
feat/manifest-validation

Conversation

@dkneeland

@dkneeland dkneeland commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Adds scripts/validate-manifests.js - validates every dash-apps/web-*/manifest.json against docs/dashapp-manifest.schema.json.

Runs as a pre-build step in the Android CI workflow (which fires on every PR), catching what currently goes unnoticed:

  • Unknown keys - a typo like "settngs" silently ignored by Android's ignoreUnknownKeys = true
  • Invalid settings entries - a misspelled setting name passes through
  • id/folder mismatch - logcat warning nobody sees
  • Bare web-* directory with no manifest.json

iOS workflows are intentionally excluded as those checks would be redundant to these.

No npm dependencies. Extracts settings enum and id regex from the schema file at runtime to keep a single source of truth.


Dependencies: none (self-contained)

Comment thread scripts/validate-manifests.js Outdated
if (!ROOT_KEYS.has(key)) errors.push(`${folder}: unknown property "${key}"`);
}
if (!Number.isInteger(m.manifestVersion) || m.manifestVersion !== 1) {
errors.push(`${folder}: manifestVersion must be 1`);

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.

Why does manifest version has to be 1? If we later increase version, do we have to update that in this script too?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — fixed in 5b8f289. Now reads \schema.properties.manifestVersion.const\ from the schema at runtime instead of hardcoding.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I just updated this from a hard-coded value to a constant. Good catch.

for (const key of Object.keys(m.perf)) {
if (!PERF_KEYS.has(key)) errors.push(`${folder}/perf: unknown property "${key}"`);
}
if (m.perf.gate !== undefined && !GATE_VALUES.has(m.perf.gate)) {

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.

What is this perf gate thing?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The perf block validates the optional perf-gate field that ships in #161 (the performance harness). That PR adds perf to the schema and dash-apps/README.md documents the gate values. The validator is forward-compatible - it was written against the contract that #161 establishes, so it works correctly once that PR merges.

@wpmed92
wpmed92 merged commit 921c5f2 into main Jul 29, 2026
2 checks passed
@wpmed92
wpmed92 deleted the feat/manifest-validation branch July 29, 2026 20:11
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.

2 participants