Skip to content

PR #416

PR #416 #1385

Triggered via dynamic May 22, 2026 12:29
Status Success
Total duration 1m 26s
Artifacts

codeql

on: dynamic
Matrix: analyze
Fit to window
Zoom out
Zoom in

Annotations

1 error and 15 warnings
Published ui-action-result v2 schema mutated instead of frozen: schemas/structured-output/xcodebuildmcp.output.ui-action-result/2.schema.json#L11
The existing published `2.schema.json` is being modified to add new `$defs` (including `compactRuntimeSnapshot`, `recoverableUiError`) and corresponding `capture`/`uiError` properties to `data`; any consumer that cached the old v2 schema definition under its stable `$id` URL will now reject new payloads that include those fields, since the old schema had `additionalProperties: false` on `data` without those properties. New features should only appear in the newly created v3 schema.
Versioned schema `2.schema.json` mutated with new fields while `schemaVersion` const stays `"2"`: schemas/structured-output/xcodebuildmcp.output.ui-action-result/2.schema.json#L368
New fields `capture` and `uiError`, plus new action types `type-text` and `batch`, were added directly to the already-published v2 schema instead of being exclusive to the new v3 schema; consumers that validated or cached the old v2 definition will encounter unexpected fields without a version signal.
`handler` unsafely cast via `as unknown as` to inject executor in Handler Behavior test: src/mcp/tools/ui-automation/__tests__/batch.test.ts#L12
The `ignores unrelated project session defaults` test uses `handler as unknown as (args, executor) => Promise<void>` to force-inject a `CommandExecutor`; use `batchLogic` with the mock executor directly instead, which is already tested and available in this file.
[L9E-6VL] `handler` unsafely cast via `as unknown as` to inject executor in Handler Behavior test (additional location): src/mcp/tools/ui-automation/__tests__/wait_for_ui.test.ts#L160
The `ignores unrelated project session defaults` test uses `handler as unknown as (args, executor) => Promise<void>` to force-inject a `CommandExecutor`; use `batchLogic` with the mock executor directly instead, which is already tested and available in this file.
[L9E-6VL] `handler` unsafely cast via `as unknown as` to inject executor in Handler Behavior test (additional location): src/mcp/tools/ui-automation/__tests__/batch.test.ts#L383
The `ignores unrelated project session defaults` test uses `handler as unknown as (args, executor) => Promise<void>` to force-inject a `CommandExecutor`; use `batchLogic` with the mock executor directly instead, which is already tested and available in this file.
long-press--success uses hardcoded positional element ref instead of a resolved ref: src/snapshot-tests/suites/ui-automation-suite.ts#L176
The `long-press--success` test uses the same `refreshRuntimeSnapshot()` + hardcoded `'e3'` pattern, adding assumed positional state rather than resolving a verified element ref from the live snapshot.
type-text success snapshot contract silently removed: src/snapshot-tests/suites/ui-automation-suite.ts#L310
The `type-text--success` snapshot fixture is dropped with no replacement success-path test; the positive contract for `type-text` now has zero snapshot coverage.
Active `toStructuredEnvelope` nextSteps contract behaviors lost their only unit tests: src/utils/__tests__/structured-output-envelope.test.ts#L54
The two deleted tests were the sole unit-level coverage for the empty-array omission guard and the error-envelope suppression guard inside `toStructuredEnvelope`; both guards are still active in the implementation and should remain tested.
tapElement ref can appear in both batch step and standalone tap step: src/mcp/tools/ui-automation/shared/runtime-next-steps.ts#L748
When `shouldShowBatch` is true and `tapElement` is a non-content-rich, non-screen-changing, non-low-priority element, the same `elementRef` is emitted twice: once inside the `batch` step params and again as the standalone `tap` step, causing an agent to be given redundant or conflicting instructions to tap the same control twice.
`type_text` success result omits `previousRuntimeSnapshot`, suppressing post-action next-step context: src/types/domain-results.ts#L402
In `createTypeTextExecutor` (src/mcp/tools/ui-automation/type_text.ts), the success path calls `createUiActionSuccessResult` without `previousRuntimeSnapshot`. As a result, `uiActionNextStepContexts` is never populated for type_text successes, so `actionTarget` and `previousScreenHash` are absent from the structured output — defeating the PR's goal of giving agents reusable next-action context after a type-text. All sibling handlers (tap, touch, swipe, drag, long_press, batch) pass `resolution.snapshot.payload` here.
`waitMatch.matches` projected without size limit unlike `uiError.candidates`: src/utils/structured-output-envelope.ts#L5
`uiError.candidates` is capped with `.slice(0, COMPACT_RUNTIME_TARGET_LIMIT)` (64) before serialisation, but `waitMatch.matches` uses an unbounded `.map()`, so a wait predicate matching many elements (e.g. `exists` or `settled` on a large list) can produce arbitrarily large output.
[5FV-TNL] `waitMatch.matches` projected without size limit unlike `uiError.candidates` (additional location): src/utils/structured-output-envelope.ts#L360
`uiError.candidates` is capped with `.slice(0, COMPACT_RUNTIME_TARGET_LIMIT)` (64) before serialisation, but `waitMatch.matches` uses an unbounded `.map()`, so a wait predicate matching many elements (e.g. `exists` or `settled` on a large list) can produce arbitrarily large output.
O(n²) element scan in findActiveForegroundRoot: src/mcp/tools/ui-automation/shared/runtime-next-steps.ts#L388
Each call to `foregroundScore` does a full `records.filter` pass (plus a separate `findSheetGrabberDescendant` scan) over all snapshot elements; with `n` scrollable candidates this is O(n²) total. Have you considered pre-building an ancestor-lookup map keyed by path prefix so each candidate's descendants can be found in O(1) amortised time?
[NTV-D29] O(n²) element scan in findActiveForegroundRoot (additional location): src/mcp/tools/ui-automation/shared/runtime-snapshot.ts#L432
Each call to `foregroundScore` does a full `records.filter` pass (plus a separate `findSheetGrabberDescendant` scan) over all snapshot elements; with `n` scrollable candidates this is O(n²) total. Have you considered pre-building an ancestor-lookup map keyed by path prefix so each candidate's descendants can be found in O(1) amortised time?
Runtime target filtering and sorting logic duplicated across two files: src/utils/renderers/domain-result-text.ts#L1168
The constants `HIDDEN_RUNTIME_TARGET_LABELS` / `LOW_PRIORITY_RUNTIME_TARGET_LABELS` and helper functions (`compactRuntimeSnapshotText`, `normalizedRuntimeSnapshotText`, `isHiddenRuntimeTarget`, `isLowPriorityRuntimeTarget`, `isContentRichTapTarget`, `isAlreadySelectedRuntimeTarget`, `getRuntimeTargetDisplayPriority`, `sortRuntimeTargetsForDisplay`) are copy-pasted verbatim from `src/utils/structured-output-envelope.ts` — future edits to label sets or priority logic in one file won't automatically propagate to the other, causing the text renderer and structured-output envelope to silently diverge.
Duplicated launch-and-snapshot boilerplate in `captureFirstScrollRef` and `captureTapRefByLabel`: src/snapshot-tests/suites/ui-automation-suite.ts#L37
Extract the shared launch-app / wait / snapshot-ui setup into a single helper (e.g. `launchAndCaptureSnapshot(bundleId): Promise<string>`) and call it from both functions to eliminate the duplicated ~7-line block.