Code Quality: PR #416 #1346
cameroncookecodeql
on: dynamic
Matrix: analyze
Annotations
4 warnings
|
`type-text` success snapshot contract deleted without replacement:
src/snapshot-tests/suites/ui-automation-suite.ts#L263
The `type-text--success` snapshot fixture is removed entirely and not replaced — the tool's positive-path output contract is no longer validated end-to-end; per the guardrail, snapshot fixture deletions are contract changes that require review.
|
|
Preserved snapshot shows perpetually-stale switch state, enabling unintended repeated toggling:
src/mcp/tools/ui-automation/batch.ts#L221
When all batch steps are inactive switches, `preserveSnapshot=true` causes the success path to skip the post-action snapshot capture, leaving the cached snapshot showing the switches as OFF even after they have been toggled ON. Every subsequent batch call reads `element.state.selected` from the same stale snapshot, `isSafeSameScreenBatchElement` returns `true` again, and the batch re-executes — toggling each switch back, then forward again on the next call, indefinitely.
|
|
[72M-P45] Preserved snapshot shows perpetually-stale switch state, enabling unintended repeated toggling (additional location):
src/mcp/tools/ui-automation/batch.ts#L129
When all batch steps are inactive switches, `preserveSnapshot=true` causes the success path to skip the post-action snapshot capture, leaving the cached snapshot showing the switches as OFF even after they have been toggled ON. Every subsequent batch call reads `element.state.selected` from the same stale snapshot, `isSafeSameScreenBatchElement` returns `true` again, and the batch re-executes — toggling each switch back, then forward again on the next call, indefinitely.
|
|
Triple-nested Math.max/min calls are overly compact:
src/mcp/tools/ui-automation/shared/runtime-snapshot.ts#L720
The triple-nested `Math.max(1, Math.min(Math.max(...), ...))` expressions are hard to parse at a glance; extracting the inner clamp to an intermediate variable makes the intent clear.
|