Optimize mobile Dataverse metadata execution - #421
Open
Shubham Agarwal (kanu-shubham) wants to merge 22 commits into
Open
Optimize mobile Dataverse metadata execution#421Shubham Agarwal (kanu-shubham) wants to merge 22 commits into
Shubham Agarwal (kanu-shubham) wants to merge 22 commits into
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Add explicit tenant propagation, a sequential long-lived metadata executor, safe reconciliation rules, and planner cost guardrails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e31c561-a943-4464-92f3-04ab071393c0
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 14, 2026 04:44
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the mobile-apps plugin’s Dataverse metadata planning/execution flow by reducing repeated tenant discovery, introducing a strictly sequential metadata executor, and shifting the workflow toward snapshot-based reconciliation to improve performance and idempotency.
Changes:
- Propagates an explicit tenant ID through Dataverse helper scripts to avoid repeated
az/challenge-based tenant discovery. - Adds
BATCH-METADATAtodataverse-request.jsto run ordered metadata operations sequentially in one process, with per-op timing/results. - Updates
/add-dataverseand the data-model architect guidance to use snapshot-driven reconciliation and clearer reuse/extend/create/adapt/defer decision rules; adds targeted tests for auth resolution and metadata batching.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/mobile-apps/skills/add-dataverse/SKILL.md | Updates the skill workflow to reconcile from a batched metadata snapshot, preserve adapt/defer decisions, and prefer sequential execution (including the new batch executor). |
| plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js | Adds tests for BATCH-METADATA ordering/stop-on-failure behavior and for avoiding “429 then collision => success” masking. |
| plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js | Adds regression tests ensuring tenant resolution short-circuits expensive discovery and honors explicit tenant precedence. |
| plugins/mobile-apps/scripts/lib/validation-helpers.js | Refactors getAuthToken to lazily produce tenant candidates and accept an explicit tenant argument. |
| plugins/mobile-apps/scripts/detect-publisher-prefix.js | Adds --tenant-id support so publisher-prefix detection can reuse the resolved tenant. |
| plugins/mobile-apps/scripts/dataverse-request.js | Adds --tenant-id, introduces BATCH-METADATA, and threads tenant through token refresh paths. |
| plugins/mobile-apps/scripts/create-calculated-column.js | Adds --tenant-id support for calculated-column creation so it can reuse resolved tenant context. |
| plugins/mobile-apps/agents/data-model-architect.md | Strengthens “target-grounded” planning guidance and expands decision taxonomy (reuse/extend/create/adapt/defer/unverified/block). |
Suppressed comments (2)
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:103
- The second test also uses a
#!/bin/shfakeaz, which makes the test shell-dependent. Using a Node-based fake keeps the test self-contained and consistent with auth-token-resolution.test.js.
`#!/bin/sh
printf '%s\\n' '{"accessToken":"test-token"}'
`,
{ mode: 0o755 },
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:135
- This PATH update also hard-codes ':' as the separator. Use path.delimiter so the fake
azis reliably found on all OSes.
{ env: { ...process.env, PATH: `${tempDir}:${process.env.PATH}` } },
);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Document the tenant-aware CLI contract, make Azure CLI test shims cross-platform, and add path-filtered mobile-app script CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e31c561-a943-4464-92f3-04ab071393c0
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 14, 2026 05:22
View session
Normalize case-insensitive PATH keys before prepending fake Azure CLI shims so Windows resolves the test executable instead of the ambient CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e31c561-a943-4464-92f3-04ab071393c0
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
plugins/mobile-apps/scripts/dataverse-request.js:34
- The BATCH-METADATA header comment shows
--tenant-id <id>as required, but the implementation accepts a null tenantId and will fall back to env vars / discovery (consistent with the single-request path and SKILL guidance). This doc mismatch can confuse callers into thinking BATCH-METADATA can’t run without an explicit tenant.
// BATCH-METADATA mode (strictly sequential metadata operations in one process):
// node dataverse-request.js <envUrl> BATCH-METADATA <label> --operations '<json>' --tenant-id <id> [--continue-on-error]
//
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 14, 2026 05:26
View session
Intercept Azure CLI child-process calls through a Node preload so the auth and metadata tests run identically without shell or executable-resolution dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e31c561-a943-4464-92f3-04ab071393c0
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:125
- This fake
azemits JSON, but production callsaz account get-access-token --query accessToken -o tsv, which returns a plain token string. Keeping the shim output in the same shape as the real CLI makes the test more faithful and less likely to mask future token-handling regressions.
'BATCH-METADATA',
'collision',
'--operations',
JSON.stringify([{ method: 'POST', apiPath: 'EntityDefinitions', body: { value: 1 } }]),
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:50
- The fake
azused here prints JSON ({"accessToken":...}), but the production code invokesaz account get-access-token --query accessToken -o tsv, which returns the raw token string. Returning JSON here makes the test less representative and could mask future token-parsing issues.
This issue also appears on line 122 of the same file.
res.writeHead(204);
res.end();
};
setTimeout(finish, 25);
});
plugins/mobile-apps/scripts/dataverse-request.js:541
- In BATCH-METADATA retry handling,
Retry-Afteris parsed withparseInt(...)but the result isn’t validated. If the header is malformed (or an HTTP-date),delayMsbecomesNaNandsetTimeout(NaN)effectively retries immediately, which can hammer the service instead of backing off.
if (res.statusCode === 429 && attempt < maxRetries) {
const retryAfter = res.headers?.['retry-after'];
const delayMs = retryAfter ? parseInt(retryAfter, 10) * 1000 : 30000;
rateLimited = true;
await new Promise((resolve) => setTimeout(resolve, delayMs));
plugins/mobile-apps/skills/add-dataverse/SKILL.md:147
- The invocation contract says
<METHOD>is "GET / POST / PATCH / DELETE", butscripts/dataverse-request.jsnow explicitly supports PUT as well. Keeping this list in sync avoids confusion when the workflow (or future steps) needs a PUT metadata call.
- Three positional args, in order:
<envUrl>,<METHOD>(GET / POST / PATCH / DELETE),<apiPath>(everything after/api/data/v9.2/). - Body is a flag, not positional.
--body '<json>'— required for POST/PATCH, never for GET/DELETE. Forgetting--bodyand passing the JSON as a 4th positional arg returns a usage error. --include-headersadds response headers (needed forOData-EntityIdafter a record create).- Output is JSON:
{ "status": <code>, "data": <body> }. Token refresh on 401 and back-off on 429 are automatic — never wrap with manual retry.
</details>
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 14, 2026 05:30
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
plugins/mobile-apps/skills/add-dataverse/SKILL.md:223
- Step 4’s reconciliation matrix introduces an
unverifiedstate that explicitly stops before writes, but the “Decide-before-write barrier” sentence immediately below says every item must resolve only toreuse/extend/create/adapt/defer. This is internally inconsistent and can mislead future edits of the workflow logic.
| Batched query failed (non-2xx) after retry and per-table split | `unverified` | unknown | STOP before writes for the affected reconciliation scope and surface the concrete environment/auth/permission error. |
`replace` is not an automatic state in this workflow. Replacing a table or column requires an explicitly approved migration with dependency analysis and data movement, so a conflict resolves to `adapt` (rename beside it) or `defer` (leave it out) instead — both of which leave existing data untouched.
**Decide-before-write barrier (HARD):** finish reconciliation for every table and column before the first metadata write. Every item must come out of Step 4 as `reuse`, `extend`, `create`, `adapt`, or `defer` — never as an unresolved conflict. Deciding renames up front is what keeps relationships, screens, and sample data pointing at the same names.
plugins/mobile-apps/agents/data-model-architect.md:34
- The hard-rule classification list omits
Block, but the workflow and Step 5 later instruct the agent to produceBlockdecisions. This inconsistency can cause the agent to output a decision the earlier rules say doesn’t exist (or vice versa).
- **No automatic replacement.** This agent classifies schema as `Reuse`, `Extend`, `Create`, `Adapt` (create beside a conflicting object under a new name), `Defer` (leave out of this run), or `Unverified` (target metadata could not be read). Replacing an existing table/column requires a separately approved migration with dependency analysis and data movement; it is outside this workflow. A data-modelling conflict is never a blocker — it is an `Adapt` or a `Defer` with a recorded reason.
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:59
- This test run environment overwrites NODE_OPTIONS rather than appending, which can drop externally supplied Node flags (coverage/instrumentation) and make the tests behave differently depending on how they’re invoked. It’s safer to append the preload require to any existing NODE_OPTIONS.
env: {
...process.env,
NODE_OPTIONS: `--require=${FAKE_AZ_PRELOAD}`,
FAKE_AZ_LOG: logPath,
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:26
- The test helper overwrites NODE_OPTIONS, which can accidentally drop runner/user-provided Node flags (e.g., coverage/instrumentation) when these tests are executed in more constrained environments. Appending the preload to any existing NODE_OPTIONS is safer and still deterministic.
return {
...process.env,
NODE_OPTIONS: `--require=${fakeAzPreload}`,
FAKE_AZ_STATIC_TOKEN: 'test-token',
...overrides,
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3e31c561-a943-4464-92f3-04ab071393c0
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 14, 2026 16:31
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (2)
plugins/mobile-apps/scripts/dataverse-request.js:545
Retry-Afterhandling in the new BATCH-METADATA retry loop can computedelayMsasNaNwhen the header is present but not a simple integer (e.g., an HTTP-date or malformed value).setTimeout(..., NaN)effectively becomes immediate, which can cause tight retry loops during throttling and amplify rate-limiting instead of backing off.
if (res.statusCode === 429 && attempt < maxRetries) {
const retryAfter = res.headers?.['retry-after'];
const delayMs = retryAfter ? parseInt(retryAfter, 10) * 1000 : 30000;
rateLimited = true;
await new Promise((resolve) => setTimeout(resolve, delayMs));
continue;
plugins/mobile-apps/agents/screen-planner.md:399
- The
related_entity_fieldsschema constrainscardinalityto"1:1" | "1:many" | "M:N", but the derivation table uses "direct lookup primary display" as acardinalityvalue. This makes the guidance self-contradictory and could cause downstream agents to emit invalid YAML.
| `archetype_class` | `cardinality` | `recommends` |
|---|---|---|
| `list`, `tab-root`, `dashboard` | direct lookup primary display | `formatted-lookup` |
| `list`, `tab-root`, `dashboard` | any other `1:1` related field | `external-projection-required` |
| `detail` | `1:1` | `chained-fetch` |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78870709-25fe-45d7-8669-dd0ba9470807
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 18, 2026 04:51
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (4)
plugins/mobile-apps/scripts/lib/derived-metadata.js:147
- In the computed-column comparison,
actual.formulaDefinition.trim()/expected.formulaDefinition.trim()can throw if either value is present-but-non-string (malformed normalized metadata). This turns a fail-closed compatibility check into a hard crash instead of returningcompatible: falsewith a reason.
if (!expected.formulaDefinition) {
reasons.push('expected computed metadata must include an exact FormulaDefinition');
} else if (!actual.formulaDefinition) {
reasons.push('live computed metadata did not return FormulaDefinition');
} else if (actual.formulaDefinition.trim() !== expected.formulaDefinition.trim()) {
plugins/mobile-apps/scripts/validate-derived-metadata.js:28
compareDerivedMetadata(expected.columns || expected, actual.columns || actual)will throw (e.g.map is not a function) if either JSON file is an object that lacks acolumnsarray. Since this is a CLI validation barrier, it should fail with a clear error message when the input shape is wrong.
const args = parseArgs(process.argv);
const expected = readJson(args.expected, '--expected');
const actual = readJson(args.actual, '--actual');
const results = compareDerivedMetadata(expected.columns || expected, actual.columns || actual);
const incompatible = results.filter((result) => !result.compatible);
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:26
NODE_OPTIONS:--require=${fakeAzPreload}will break if the repo path contains spaces (NODE_OPTIONS is parsed like a command line). Quote the preload path so tests run reliably on common local paths likeC:\Users<Name>...`.
function fakeAzEnv(overrides = {}) {
return {
...process.env,
NODE_OPTIONS: `--require=${fakeAzPreload}`,
FAKE_AZ_STATIC_TOKEN: 'test-token',
...overrides,
};
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:60
NODE_OPTIONS:--require=${FAKE_AZ_PRELOAD}` can fail when the preload path contains spaces because NODE_OPTIONS is tokenized like a command line. Quoting the path makes the test harness more robust on Windows/macOS local paths.
const result = spawnSync(process.execPath, ['-e', script], {
encoding: 'utf8',
env: {
...process.env,
NODE_OPTIONS: `--require=${FAKE_AZ_PRELOAD}`,
FAKE_AZ_LOG: logPath,
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78870709-25fe-45d7-8669-dd0ba9470807
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 18, 2026 05:09
View session
added 2 commits
August 18, 2026 10:41
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78870709-25fe-45d7-8669-dd0ba9470807
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78870709-25fe-45d7-8669-dd0ba9470807
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (4)
plugins/mobile-apps/skills/add-dataverse/SKILL.md:152
- The script invocation contract lists supported HTTP methods as GET/POST/PATCH/DELETE, but
dataverse-request.jsalso supports PUT (and the skill later uses BATCH-METADATA). This mismatch can cause confusion when following the contract verbatim.
- Three positional args, in order: `<envUrl>`, `<METHOD>` (GET / POST / PATCH / DELETE), `<apiPath>` (everything after `/api/data/v9.2/`).
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:58
- The tests set NODE_OPTIONS to
--require=<path>without quoting the path or preserving any existing NODE_OPTIONS. If the repo path contains spaces (common on Windows/macOS user profiles), Node will parse the value incorrectly and the preload won't load, making the tests flaky on developer machines.
NODE_OPTIONS: `--require=${FAKE_AZ_PRELOAD}`,
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:23
- The test helper overwrites NODE_OPTIONS with an unquoted
--require=<path>. If the path contains spaces, Node won't load the preload. It also clobbers any existing NODE_OPTIONS, which can make local runs behave differently from CI.
NODE_OPTIONS: `--require=${fakeAzPreload}`,
plugins/mobile-apps/shared/samples/src/utils/dataverse.ts:82
- normalizeDataverseGuid() lowercases and strips braces, but it doesn’t trim whitespace. A valid GUID like " {A...} " will currently return undefined, which defeats the function’s purpose for user- or URL-supplied IDs.
if (!value) return undefined;
const normalized = value.replace(/[{}]/g, '').toLowerCase();
return /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/.test(normalized)
? normalized
: undefined;
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 18, 2026 05:14
View session
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78870709-25fe-45d7-8669-dd0ba9470807
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:16
- The header comment still says the tests “place a fake
azexecutable first on PATH”, but the implementation now usesNODE_OPTIONS=--require=<fake-az-preload.js>to interceptexecFileSync('az', ...). This is misleading when debugging failures or porting the approach to new tests.
// These tests exercise the real code path rather than mocking internals: a fake
// `az` executable is placed first on PATH and logs every invocation, so we can
// assert exactly which subcommands ran. The resource URL points at 127.0.0.1:1
// so that if the challenge probe IS reached it fails instantly with
// ECONNREFUSED instead of touching the network.
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 18, 2026 05:20
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (4)
plugins/mobile-apps/scripts/lib/derived-metadata.js:160
- compareDerivedMetadata can throw if actualColumns contains null/non-object entries or rows missing table/logicalName (e.g., malformed derived-metadata-live.json). This should fail closed (mark incompatible) rather than crash the validator.
for (const column of actualColumns || []) {
const key = `${column.table}:${column.logicalName}`;
if (actualByKey.has(key)) duplicateKeys.add(key);
actualByKey.set(key, column);
}
plugins/mobile-apps/scripts/check-routes.js:290
- check-routes can report the same underlying collision twice (e.g., .tsx together with /index.tsx triggers both file-folder-route-collision and duplicate-route), inflating findings and making the output noisy. Consider suppressing duplicate-route when the conflict is an index-vs-file pair already covered by the file/folder collision finding.
if (dests[route]) {
duplicateRouteFindings.push({
route,
file,
otherFile: dests[route].file,
plugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.js:26
- NODE_OPTIONS uses an unquoted --require path. If the repo path contains spaces (common on Windows dev machines), Node can mis-parse NODE_OPTIONS and fail to load the preload, causing these tests to hit the real Azure CLI. Quoting the preload path makes this robust.
return {
...process.env,
NODE_OPTIONS: `--require=${fakeAzPreload}`,
FAKE_AZ_STATIC_TOKEN: 'test-token',
...overrides,
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:60
- NODE_OPTIONS uses an unquoted --require path. If the repo path contains spaces, Node can mis-parse NODE_OPTIONS and fail to load the preload, which would make the test depend on a real az installation/session. Quote the preload path to keep tests hermetic.
env: {
...process.env,
NODE_OPTIONS: `--require=${FAKE_AZ_PRELOAD}`,
FAKE_AZ_LOG: logPath,
// Cleared unless a test opts in — the ambient shell may have them set.
| @@ -0,0 +1,186 @@ | |||
| 'use strict'; | |||
Contributor
There was a problem hiding this comment.
Is this used for metadata comparison? Do we need it, llms are good at text comparisions
Contributor
Author
There was a problem hiding this comment.
Yes. This is the deterministic pre-write metadata comparison used by validate-derived-metadata.js in /add-dataverse Step 4a.
It compares normalized machine-readable semantics—not descriptive text—including exact lookup targets, Choice/Boolean integer mappings and labels, SourceType , SourceTypeMask , column types, duplicate rows, and exact computed FormulaDefinition .
The LLM plans the schema, but this script is a safety barrier before Dataverse mutation. It must be repeatable, testable, and fail closed; an LLM comparison would be nondeterministic and could incorrectly approve an incompatible existing column. The behavior is covered by the derived-metadata regression tests.
I agree with this if you still feel this wrong we can remove.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 18, 2026 07:06
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (1)
plugins/mobile-apps/scripts/dataverse-request.js:545
- In
runOneMetadataOperation, the 429 (throttling) path computesdelayMsfromRetry-Afterbut never actually waits before retrying. This can cause an immediate tight retry loop that worsens throttling and defeats the intended backoff behavior.
if (res.statusCode === 429 && attempt < maxRetries) {
const retryAfter = res.headers?.['retry-after'];
const delaySeconds = retryAfter ? Number.parseInt(String(retryAfter), 10) : NaN;
const delayMs = Number.isFinite(delaySeconds) ? delaySeconds * 1000 : 30000;
rateLimited = true;
continue;
}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 341594d3-c514-4ede-b3d4-4112f67c35c8
Copilot started reviewing on behalf of
Shubham Agarwal (kanu-shubham)
August 18, 2026 07:20
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.
Suppressed comments (2)
plugins/mobile-apps/scripts/dataverse-request.js:545
- In
runOneMetadataOperation, 429 handling computesdelayMsfromRetry-Afterbut immediately retries without waiting. This effectively busy-loops on throttling and can worsen rate limiting in real Dataverse metadata runs (unlike the single-op path above, which sleeps).
if (res.statusCode === 429 && attempt < maxRetries) {
const retryAfter = res.headers?.['retry-after'];
const delaySeconds = retryAfter ? Number.parseInt(String(retryAfter), 10) : NaN;
const delayMs = Number.isFinite(delaySeconds) ? delaySeconds * 1000 : 30000;
rateLimited = true;
continue;
}
plugins/mobile-apps/scripts/check-routes.js:263
- The file/folder collision scan runs for every
*.tsxexcept_layout.tsx, but it doesn’t skip files thatfileToRoute()intentionally maps tonull(e.g.+not-found.tsx). That can produce findings withroute: nulland fail the gate for files that are explicitly out of scope for routing checks.
for (const file of screenFiles) {
const relative = path.relative(appRoot, file).replace(/\\/g, '/');
const base = relative.replace(/\.tsx$/, '');
const children = screenFiles.filter(other => {
const otherRelative = path.relative(appRoot, other).replace(/\\/g, '/');
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR supersedes #313 and contains both the original Dataverse fast-create work and the complete V2 optimization. PR #313 does not need to merge first.
The change makes mobile-app Dataverse schema creation faster, safer, tenant-correct, and efficiently rerunnable while preserving strict sequential metadata writes and equivalent resulting schemas.
Controlled live benchmarks show:
Problems addressed
The previous workflow incurred substantial avoidable overhead around Dataverse's inherently sequential metadata engine:
Dataverse still serializes metadata writes, so this PR does not attempt unsafe parallelism. It removes client-side process, authentication, discovery, and reconciliation overhead around the required sequential requests.
What changed
1. Long-lived sequential metadata executor
plugins/mobile-apps/scripts/dataverse-request.jsAdds
BATCH-METADATA, a long-lived executor that:BATCH-METADATAis not an OData$batch. It sends ordinary Dataverse HTTP requests one at a time from one Node process.2. Explicit tenant propagation
Updated:
scripts/lib/validation-helpers.jsscripts/dataverse-request.jsscripts/detect-publisher-prefix.jsThe target environment's explicit tenant now takes priority during token acquisition and propagates through metadata helpers.
Tenant resolution order is deterministic:
POWER_PLATFORM_TENANT_IDDATAVERSE_TENANT_IDThis avoids repeated
az account showcalls and prevents tokens from being requested against the wrong tenant when Power Apps CLI and Azure CLI identities differ.3. Snapshot-driven metadata reconciliation
The workflow now prefers filtered metadata snapshots over repeated per-component discovery.
The desired schema is compared locally against live metadata before writes begin. The snapshot folds in preflight information for:
This reduces requests and allows zero-write reruns to finish quickly.
4. Safer idempotent reruns
The create flow now distinguishes:
Reruns:
Unreadable metadata is never silently interpreted as “not found.”
5. Collision recovery and reuse behavior
The add-dataverse workflow now prefers:
For example, if a user selects an environment where an appropriate product or warehouse table already exists, the workflow reuses it or adds only missing compatible columns instead of blindly creating a duplicate.
When a rename is required, downstream relationships, screen references, plan artifacts, and run-level aliases must use the resolved logical name.
6. Faster new-table creation
All ordinary planned columns are included in the initial
EntityDefinitionscreate request.Instead of:
V2 uses:
Individual column requests remain only for:
7. Dependency-safe relationships and keys
The workflow keeps metadata writes sequential and tier ordered:
RelationshipDefinitionsActive,Pending, orFailedNo parallel metadata writes were introduced.
8. Solution targeting and mutation hygiene
Metadata mutations consistently target the intended Dataverse solution through
MSCRM.SolutionUniqueName.Project-local scratch files are used for request payloads, and the workflow records server-assigned metadata IDs so future runs can distinguish app-owned components from foreign collisions.
9. Derived metadata and cross-entity-read safety
The workflow now validates the complete semantics of same-named derived columns before reuse:
SourceType0FormulaDefinitionMicrosoft does not expose a supported API contract for authoring formula, calculated, or rollup definitions. The old calculated-column helper therefore fails closed before authentication or mutation. Cross-entity reads use formatted lookup annotations or bounded detail-screen fetches; hot-list fields and unsupported M:N paths remain
external-projection-requiredinstead of generating XAML or N+1 reads.10. Reuse/extend/create cost guardrails
The architect now considers more than name similarity when deciding whether to reuse or extend an existing table.
The decision accounts for:
This allows an app-owned table when extending a merely similar shared table would create excessive coupling and serialized metadata writes.
11. Relationship-count guidance
Relationship creation is the dominant remaining Dataverse cost. The planner now requires every relationship to have a real runtime, integrity, security, offline, or reporting purpose.
Standard Dataverse ownership and audit columns should be reused when they express the requirement. Business-specific relationships remain when they have distinct meaning.
12. Review follow-up and cross-platform CI
Review feedback is addressed in
94e3a20c, with final OS-independent test portability in596602c7:explicitTenantIdparameter ingetAuthTokenJSDoc--tenant-idfor single andBATCH-RECORDScalls--continue-on-errorforBATCH-METADATAFiles changed
plugins/mobile-apps/agents/data-model-architect.mdplugins/mobile-apps/scripts/create-calculated-column.jsplugins/mobile-apps/scripts/lib/derived-metadata.jsplugins/mobile-apps/scripts/validate-derived-metadata.jsplugins/mobile-apps/scripts/tests/derived-metadata.test.jsplugins/mobile-apps/scripts/dataverse-request.jsplugins/mobile-apps/scripts/detect-publisher-prefix.jsplugins/mobile-apps/scripts/lib/validation-helpers.jsplugins/mobile-apps/scripts/tests/auth-token-resolution.test.jsplugins/mobile-apps/scripts/tests/dataverse-request-metadata-batch.test.jsplugins/mobile-apps/skills/add-dataverse/SKILL.md.github/workflows/mobile-apps-script-tests.ymlBenchmark methodology
The benchmark used frozen main and V2 revisions, isolated app shells, unique Dataverse logical names, identical desired schemas, captured HTTP telemetry, zero-write reruns, and normalized schema comparison.
The three workloads included:
Benchmark results
Zero-write reruns
Correctness and reliability
Across all three paired runs:
Process and authentication reduction: 12-table workload
Why the remaining first-run time exists
For the 12-table V2 run, approximately 96% of the first-run duration was spent inside Dataverse metadata requests. Relationship creation alone represented approximately 64% of the full run.
The client-side executor is therefore close to the practical limit for creating the same schema through individual Dataverse metadata operations. Further substantial gains require reducing unnecessary schema operations or changing the metadata transport—not unsafe parallel writes.
Validation
After the conflict-free rebase and review fixes:
Compatibility and safety
BATCH-METADATAis not OData$batchScope boundaries
This PR is intentionally limited to Dataverse metadata execution and data-model planning guidance.
The following findings from the broader Pepsi audit will be handled separately:
Benchmark visuals
6-new-table workload
12-new-table workload
24-new-table workload
Why
fake-az-preload.jsis includedplugins/mobile-apps/scripts/tests/helpers/fake-az-preload.jsis an intentional test-only Azure CLI double. It lets authentication tests simulate tenant discovery, token retrieval, and tenant-specific failures without contacting Azure, depending on a developer’s activeazsession, or risking changes to real environments. It is loaded only by tests and is not used by production skill execution.