Code Quality: PR #416 #1373
codeql
on: dynamic
Matrix: analyze
Annotations
1 error and 11 warnings
|
UI-action tool manifests still declare outputSchema version "2" while runtime emits v3-shaped rs/1 captures:
src/mcp/tools/ui-automation/button.ts#L83
`setUiActionStructuredOutput` in `src/mcp/tools/ui-automation/shared/domain-result.ts` always sets `schemaVersion: '2'` on the envelope, and every UI-action manifest (`manifests/tools/tap.yaml`, `swipe.yaml`, `drag.yaml`, `batch.yaml`, `key_press.yaml`, `key_sequence.yaml`, `long_press.yaml`, `touch.yaml`, `type_text.yaml`, `wait_for_ui.yaml`, `snapshot_ui.yaml`, `button.yaml`) declares `outputSchema.version: "2"`. However, the new rs/1 captures returned by `captureRuntimeSnapshotAfterActionSafely` use `{ type: 'runtime-snapshot', protocol: 'rs/1', elements, actions, ... }`, which only the v3 schema accepts. Either bump the runtime envelope and all UI-action manifests to `"3"`, or keep emitting the legacy compact (`rs`/`targets`/`scroll`/`udid`) capture shape on v2.
|
|
type-text success path snapshot contract removed without replacement:
src/snapshot-tests/suites/ui-automation-suite.ts#L261
The `type-text--success` snapshot test and fixture were deleted with no replacement success-path test, so the `type-text` tool's happy path is no longer verified in snapshot tests at all — only error cases remain.
|
|
verbose=true produces full RuntimeSnapshot data under CAPTURE_SCHEMA version '2', violating schema contract:
src/cli/register-tool-commands.ts#L119
`runtimeSnapshot: 'full'` is set for **all** schemas when `verbose=true`, but the schema version is only bumped to `'3'` for `UI_ACTION_RESULT_SCHEMA`. For `CAPTURE_SCHEMA` (`snapshot_ui` and similar), callers running with `--output json --verbose` receive a full `RuntimeSnapshotV1` (structured element objects) while `schemaVersion` still says `'2'`, which specifies the compact pipe-delimited string format — breaking schema-version-based parsing.
|
|
Scroll step incorrectly prioritized over detected completion action (Save/Add):
src/mcp/tools/ui-automation/shared/runtime-next-steps.ts#L616
When `foregroundIncompleteCompletionTapElement` is set (e.g. a "Save" or "Add" button detected alongside "unsaved"/"not added" state signals), `shouldPrioritizeScroll` at line 702 still evaluates to `true` because `COMPLETION_ACTION_TAP_NEXT_STEP_LABELS` entries like "save" are neither content-rich nor low-priority, causing scroll to appear before the critical completion tap in the next-step list. Add `foregroundIncompleteCompletionTapElement === null` as a guard in the `shouldPrioritizeScroll` condition.
|
|
`/statictext|text/` check fires before image/switch/slider/cell/scroll/list/tab checks, misclassifying any element whose roleText contains "text":
src/mcp/tools/ui-automation/shared/runtime-snapshot.ts#L129
Any element whose combined `role + type + subrole + role_description` contains the substring `"text"` is returned as `'text'` before the `image`, `switch`, `slider`, `cell`, `scroll-view`, `list`, or `tab` branches are ever reached; these misclassified elements then only receive `longPress`/`touch` actions (never `tap`) because `isTapRole` excludes `'text'`.
|
|
[3X8-KA3] `/statictext|text/` check fires before image/switch/slider/cell/scroll/list/tab checks, misclassifying any element whose roleText contains "text" (additional location):
src/mcp/tools/ui-automation/shared/runtime-snapshot.ts#L120
Any element whose combined `role + type + subrole + role_description` contains the substring `"text"` is returned as `'text'` before the `image`, `switch`, `slider`, `cell`, `scroll-view`, `list`, or `tab` branches are ever reached; these misclassified elements then only receive `longPress`/`touch` actions (never `tap`) because `isTapRole` excludes `'text'`.
|
|
Suppressed tap/swipe elements silently disappear from both targets and scroll-areas sections:
src/utils/renderers/domain-result-text.ts#L1329
Any element in `suppressedTargetRefs` that also has both `swipeWithin` and `tap`/`typeText` actions will be excluded from the Targets section (correctly, because it's suppressed) but also excluded from the Scroll section, because `isScrollableRuntimeArea` calls `isLikelyRuntimeTarget(element)` with no `suppressedTargetRefs` argument (defaults to an empty set), so the element is classified as a likely target and filtered out of scroll areas — making it invisible in both rendered sections.
|
|
`null` param values silently rendered as literal string `'null'` in CLI output:
src/utils/responses/next-step-formatting.ts#L27
`hasComplexCliParamValue(null)` returns `false` because `value !== null` fails, so `null` param values skip the `--json` path and reach the else branch of the loop, where `formatCliParamValue(null)` returns `shellEscapeArg(JSON.stringify(null))` = `'null'`, producing `--key 'null'` — passing the string literal `"null"` as a CLI flag value instead of omitting the param.
|
|
O(n²) descendant scan in `findActiveForegroundRoot` for each scrollable candidate:
src/mcp/tools/ui-automation/shared/runtime-next-steps.ts#L388
Inside `foregroundScore`, both `findSheetGrabberDescendant` (a linear `find`) and `records.filter(isForegroundCandidateForRoot)` are O(n) operations executed once per unique candidate the first time their score is computed — making the overall function O(n²) over the element list. For complex screens with many AX elements this runs on a performance-critical path (post-action snapshot rendering). Consider pre-grouping records by path prefix or building a parent→children index once before scoring.
|
|
Duplicate O(n²) foreground root detection per UI action result:
src/mcp/tools/ui-automation/shared/runtime-next-steps.ts#L551
Both `getForegroundCompletionSuppressedRuntimeTargetRefs` and `createRuntimeSnapshotNextSteps` independently call `findStoredSnapshotRecords` and `findActiveForegroundRoot` — the latter performs an O(n²) descendant scan across all snapshot records — so every UI action result triggers this expensive pass twice. Consider extracting the shared intermediate state (`recordsByRef`, `foregroundRoot`, filtered elements) into a shared context object or combining the two exported functions.
|
|
[ZVQ-HZR] Duplicate O(n²) foreground root detection per UI action result (additional location):
src/mcp/tools/ui-automation/shared/domain-result.ts#L299
Both `getForegroundCompletionSuppressedRuntimeTargetRefs` and `createRuntimeSnapshotNextSteps` independently call `findStoredSnapshotRecords` and `findActiveForegroundRoot` — the latter performs an O(n²) descendant scan across all snapshot records — so every UI action result triggers this expensive pass twice. Consider extracting the shared intermediate state (`recordsByRef`, `foregroundRoot`, filtered elements) into a shared context object or combining the two exported functions.
|
|
Success path for `type-text` removed with no replacement test:
src/snapshot-tests/suites/ui-automation-suite.ts#L261
The `type-text` success test was replaced entirely with an error-path test, leaving no coverage for the happy path where text is actually typed into an actionable element.
|