Skip to content

feat: skip UI rendering in run mode for performance#156

Open
filiphric wants to merge 1 commit into
mainfrom
feat/disable-ui-run-mode
Open

feat: skip UI rendering in run mode for performance#156
filiphric wants to merge 1 commit into
mainfrom
feat/disable-ui-run-mode

Conversation

@filiphric

Copy link
Copy Markdown
Owner

Closes #135

Problem

The plugin renders a full Vue UI — including Prism syntax-highlighting of the entire request/response body — into the DOM on every cy.api() call. During cypress run / CI this UI can't be interacted with (not even in Test Replay), so it's pure overhead that was crashing the browser renderer on large responses (especially with Test Replay capturing the whole DOM).

Change

Adds a disableUi plugin option that skips the two expensive operations — Prism highlighting (transform()) and mounting + snapshotting the Vue DOM — while still performing the real request and exposing the response (yielded value + command-log console props with cURL).

Behavior:

  • unset (default): render in open mode, skip in run mode (cypress run/CI)
  • true: always skip the UI (also speeds up open mode)
  • false: always render, even in run mode (opt back into full Test Replay capture)

Detection uses Cypress.config('isInteractive').

⚠️ Default behavior change

By default the UI is now skipped in run mode, so Test Replay will no longer capture the rendered response body in CI. Users who want the old behavior can set disableUi: false. Documented in the README.

Files

  • src/utils/shouldRenderUi.ts — new helper deciding render vs skip
  • src/types.tsdisableUi added to PluginEnvOptions
  • src/modules/initialize.ts — skip creating/mounting the Vue app when disabled
  • src/modules/api.ts — skip request-side transformData() when disabled
  • src/modules/handleResponse.ts — gate response-side transform() calls + DOM snapshot/scroll; lightweight log-only path otherwise
  • cypress.config.tsexpose.disableUi: false so the plugin's own UI tests (run via cypress run) keep rendering
  • cypress/e2e/disableUi.cy.ts — new spec covering skip/render/toggle
  • README.md — documents the option

Verification

  • tsc --noEmit clean
  • vite build succeeds
  • Full e2e suite: 94/94 passing (incl. 3 new disableUi tests + retries/snapshotOnly/requestMode which exercise mount/unmount)

Adds a disableUi option that skips the expensive Vue UI mount and Prism
syntax highlighting. Defaults to auto: render in open mode, skip in run
mode (cypress run/CI) where the UI can't be interacted with and was
crashing the browser renderer on large responses.

Closes #135
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 28, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Add disableUi option to skip Vue/Prism UI rendering in cypress run mode
✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Description

• Add disableUi option to skip UI work in cypress run/CI while keeping requests/logging intact.
• Gate Prism syntax-highlighting and Vue mount/snapshots behind shouldRenderUi() for performance.
• Add e2e coverage and README docs for skip/force-render behaviors and defaults.
Diagram

graph TD
A["Test (cy.api)"] --> B["src/modules/api.ts"] --> C["src/utils/shouldRenderUi.ts"] --> D{"UI enabled"} --> E["src/modules/initialize.ts (Vue app)"] --> F["src/modules/handleResponse.ts (snapshots + Prism)"]
D --> G["src/modules/initialize.ts (no app)"] --> H["src/modules/handleResponse.ts (log-only)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Lazy highlighting / render-on-demand UI
  • ➕ Keeps UI available while deferring Prism work until the panel is opened/visible
  • ➕ Reduces worst-case cost in open mode without disabling UI entirely
  • ➖ Higher implementation complexity (UI state, lifecycle hooks, potential flicker)
  • ➖ Still may incur costs in run mode if Test Replay captures post-rendered DOM
2. Size-based guardrails (skip Prism/DOM above N bytes)
  • ➕ Preserves current UI for small/typical responses automatically
  • ➕ Targets the crash scenario directly with minimal behavioral change
  • ➖ Heuristic threshold can surprise users and complicate support
  • ➖ Large responses still require a log-only fallback path similar to this PR
3. Hard-disable UI in run mode (no override)
  • ➕ Simplest mental model and smallest surface area
  • ➕ Max performance in CI by default
  • ➖ Removes opt-in ability for teams relying on Test Replay DOM capture in CI
  • ➖ Less flexible for debugging workflows

Recommendation: The PR’s approach (central shouldRenderUi() with an explicit disableUi tri-state) is the best tradeoff: it fixes CI/run-mode crashes by default, preserves an opt-in path for Test Replay capture, and keeps the request/yielded response semantics unchanged. Consider adding a future size-based guardrail or lazy highlighting if users want UI rendering without full Prism cost, but the current implementation is a pragmatic and low-risk performance win.

Files changed (8) +206 / -57

Enhancement (5) +123 / -57
api.tsSkip request-side Prism transformData when UI is disabled +3/-1

Skip request-side Prism transformData when UI is disabled

• Adds 'shouldRenderUi()' gating so request body/header formatting work is only performed when the UI will render, avoiding unnecessary syntax-highlighting overhead in run mode/CI.

src/modules/api.ts

handleResponse.tsAdd log-only response path and gate Prism/snapshots behind shouldRenderUi +84/-56

Add log-only response path and gate Prism/snapshots behind shouldRenderUi

• Computes a 'render' flag and uses it to (1) avoid Prism formatting of response/request/headers when UI is disabled, (2) skip Cypress snapshots/DOM lookups/scrolling in the disabled case while still logging 'yielded' and generated cURL in consoleProps, and (3) safely unmount the Vue app only when present.

src/modules/handleResponse.ts

initialize.tsShort-circuit Vue app creation when disableUi is active +7/-0

Short-circuit Vue app creation when disableUi is active

• Calls 'shouldRenderUi()' and returns '{ app: null, props }' when rendering is disabled, preventing Vue app creation/mounting while keeping reactive props tracking intact for logging.

src/modules/initialize.ts

types.tsAdd 'disableUi' to PluginEnvOptions with behavior docs +7/-0

Add 'disableUi' to PluginEnvOptions with behavior docs

• Extends the plugin’s public option types with a documented 'disableUi?: boolean' tri-state, describing default auto behavior and explicit override semantics.

src/types.ts

shouldRenderUi.tsIntroduce centralized render/skip decision helper +22/-0

Introduce centralized render/skip decision helper

• Adds 'shouldRenderUi()' which reads 'disableUi' from plugin config and falls back to 'Cypress.config('isInteractive')' to auto-skip UI in run mode while rendering in open mode.

src/utils/shouldRenderUi.ts

Tests (1) +41 / -0
disableUi.cy.tsAdd e2e coverage for disableUi skip/force/toggle behavior +41/-0

Add e2e coverage for disableUi skip/force/toggle behavior

• Introduces tests asserting that 'disableUi: true' skips mounting/rendering while still yielding responses, 'disableUi: false' renders even in run mode, and toggling restores rendering. Includes a helper that uses window config, 'Cypress.expose', or 'Cypress.env' depending on availability.

cypress/e2e/disableUi.cy.ts

Documentation (1) +39 / -0
README.mdDocument 'disableUi' option and new default behavior in run mode +39/-0

Document 'disableUi' option and new default behavior in run mode

• Adds a dedicated section explaining why UI rendering is expensive, the new auto-skip behavior in 'cypress run'/CI, and how to force-enable/disable rendering via 'Cypress.expose', 'Cypress.env', or config.

README.md

Other (1) +3 / -0
cypress.config.tsForce UI rendering in plugin’s own e2e runs +3/-0

Force UI rendering in plugin’s own e2e runs

• Sets 'expose.disableUi: false' so the repository’s UI-centric e2e tests continue to render under 'cypress run', overriding the new default auto-skip behavior.

cypress.config.ts

@cypress

cypress Bot commented Jun 28, 2026

Copy link
Copy Markdown

cypress-plugin-api    Run #211

Run Properties:  status check passed Passed #211  •  git commit 06b35e1dbf: feat: skip UI rendering in run mode for performance
Project cypress-plugin-api
Branch Review feat/disable-ui-run-mode
Run status status check passed Passed #211
Run duration 01m 13s
Commit git commit 06b35e1dbf: feat: skip UI rendering in run mode for performance
Committer Filip Hric
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 94
View all changes introduced in this branch ↗︎

@qodo-for-filiphric

qodo-for-filiphric Bot commented Jun 28, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (0) 📎 Requirement gaps (2) 📜 Skill insights (0)

Context used

Grey Divider


Action required

1. disableUi: false enables run-mode UI 📎 Requirement gap ➹ Performance
Description
shouldRenderUi() allows forcing full UI rendering in Cypress run mode when disableUi is
explicitly set to false, which defeats the run-mode/CI safety requirement and can reintroduce
renderer crashes/perf issues. The repo also configures disableUi: false in cypress.config.ts,
causing UI rendering under cypress run.
Code

src/utils/shouldRenderUi.ts[R17-21]

+  const disableUi = getPluginConfig('disableUi')
+  if (disableUi === true) return false
+  if (disableUi === false) return true
+  // Default: Cypress.config('isInteractive') is true in open mode and false in run mode.
+  return Cypress.config('isInteractive') !== false
Evidence
Rule 6641 requires that in run mode/CI the plugin does not render the full UI. The new helper
explicitly returns true for disableUi === false, enabling UI rendering regardless of run mode,
and the Cypress config sets disableUi: false which will force UI rendering under cypress run.

Do not render cypress-plugin-api UI output in Cypress run mode / CI
src/utils/shouldRenderUi.ts[17-21]
cypress.config.ts[9-11]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires that the plugin must not render the full UI/DOM output in Cypress run mode / CI. The new `disableUi` logic currently allows `disableUi: false` to force rendering even in run mode, and the repo’s `cypress.config.ts` sets `disableUi: false`, which will render UI during `cypress run`.
## Issue Context
The intended performance safeguard for CI/run mode should be unconditional per the compliance rule; users can still use UI in open mode, and can still disable UI in any mode.
## Fix Focus Areas
- src/utils/shouldRenderUi.ts[17-21]
- cypress.config.ts[9-11]
- README.md[80-85]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unsafe duplicate cURL builder 🧑 Team insight ⛨ Security
Description
**Inspired by avrik-berman's coding and review patterns** — handleResponse() reintroduces a local
generateCurl() that doesn’t escape quotes/newlines and bypasses the existing shared helper,
risking malformed commands and credential leakage (especially when hideCredentials is enabled).
This also duplicates logic already centralized in src/utils/generateCurl.ts.
Code

src/modules/handleResponse.ts[R197-212]

+  const generateCurl = () => {
+    let curl = `curl -X ${options.method || 'GET'} "${options.url}"`;
+    if (options.headers) {
+      Object.entries(options.headers).forEach(([key, value]) => {
+        curl += ` -H "${key}: ${value}"`;
+      });
+    }
+    if (options.body) {
+      if (typeof options.body === 'object') {
+        curl += ` -d '${JSON.stringify(options.body)}'`;
+      } else {
+        curl += ` -d '${options.body}'`;
+      }
+    }
+    return curl;
+  };
Evidence
The PR adds a new inline cURL generator in handleResponse() that concatenates headers/body without
escaping and uses request options, while the repo already contains a dedicated generateCurl()
utility that properly escapes values and builds from RequestProps (which are also the ones
anonymized when hideCredentials is enabled). avrik-berman consistently pushes to avoid duplicated
logic and to use existing primitives/helpers to keep behavior consistent and maintainable.

/src/modules/handleResponse.ts[195-212]
/src/utils/generateCurl.ts[3-75]
/src/modules/api.ts[15-23]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/modules/handleResponse.ts` defines a local `generateCurl()` that (a) duplicates existing functionality and (b) builds a cURL string without escaping/quoting safeguards. It also uses `options.*` rather than the (potentially anonymized) `props[index]`, which can leak sensitive values when `hideCredentials` is enabled.
### Issue Context
A shared `generateCurl(item: RequestProps)` already exists and includes escaping for quotes/newlines and composes auth/query/headers/body consistently.
### Fix Focus Areas
- src/modules/handleResponse.ts[195-246]
- src/utils/generateCurl.ts[1-75]
- src/modules/api.ts[15-24]
### Expected change
- Remove the local `generateCurl` closure from `handleResponse()`.
- Import `generateCurl` from `@utils/generateCurl` and use `generateCurl(props[index])` in both render and non-render consoleProps paths.
- Ensure the generated cURL reflects anonymized props when `hideCredentials` is on (already achieved by using `props[index]`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. UI not torn down 🐞 Bug ☼ Reliability ⭐ New
Description
When shouldRenderUi() becomes false after the Vue app was previously mounted, initialize() returns
{app: null} without unmounting/removing the existing app/root, leaving stale UI DOM and retained
mounted state. This undermines disableUi’s semantics when toggled at runtime and can keep
unnecessary UI overhead around.
Code

src/modules/initialize.ts[R78-82]

+  // Skip the expensive Vue app entirely when the UI is disabled (e.g. run mode).
+  // The request still runs and is logged; only the heavy DOM rendering is avoided.
+  if (!shouldRenderUi()) {
+    return { app: null, props }
+  }
Evidence
initialize() now returns early when UI is disabled, but doesn’t unmount/remove any previously
mounted app/root; mountPlugin() shows that the UI mount creates a persistent DOM root, so without
explicit cleanup it will remain.

src/modules/initialize.ts[10-95]
src/modules/mountPlugin.ts[5-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `disableUi` is toggled from enabled-rendering to disabled-rendering, `initialize()` returns early without tearing down any previously mounted Vue app or removing the mount root/styles. This can leave the UI in the DOM even though rendering is now disabled.

## Issue Context
- `mountPlugin()` creates `#api-plugin-root` and injects styles.
- `initialize()` only unmounts/removes the app/root on retries, not when UI is disabled.

## Fix Focus Areas
- src/modules/initialize.ts[64-95]
- src/modules/mountPlugin.ts[5-16]

## Suggested fix
Before returning early on `!shouldRenderUi()`, add cleanup similar to the retry path:
- If `currentApp` exists: `currentApp.unmount(); currentApp = null`
- If `currentMountRoot` exists: `currentMountRoot.remove(); currentMountRoot = null`
- Remove injected styles (call `removeStyles()` or equivalent cleanup)
This ensures `disableUi: true` actually removes the already-mounted UI when toggled at runtime.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Props retained without UI 🐞 Bug ➹ Performance ⭐ New
Description
In the !render path, handleResponse() still assigns window.props[testId] = props even though the Vue
UI is skipped, retaining full request/response bodies for the lifetime of the page. Since
window.props is initialized once and not cleared per test, this can accumulate large responses
across the run and cause memory pressure in the very mode this PR optimizes.
Code

src/modules/handleResponse.ts[R214-230]

+  // No UI mounted: still log the request (with response/cURL in consoleProps), but skip
+  // the DOM lookup, snapshot and scroll that only make sense with the rendered UI.
+  if (!render) {
+    log.set({
+      consoleProps() {
+        return {
+          yielded,
+          cURL: generateCurl()
+        }
+      }
+    })
+
+    window.props[testId] = props
+    log.end()
+
+    return cy.wrap(res, { log: false })
+  }
Evidence
The new no-UI branch still writes props into window.props; support.ts initializes window.props once,
and initialize() only deletes window.props[testId] on retries, so these retained props can
accumulate even when no UI is mounted to read them.

src/modules/handleResponse.ts[214-230]
src/support.ts[7-12]
src/modules/initialize.ts[56-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When UI rendering is disabled (`render === false`), `handleResponse()` still persists the full `props` array (including response bodies) into `window.props[testId]`. With UI disabled, nothing consumes this state, but it remains strongly referenced and can grow across requests/tests.

## Issue Context
- `window.props` is initialized globally in support code and is only deleted on retries.
- The new no-UI path is intended to reduce run-mode overhead; retaining large response bodies in `window.props` works against that goal.

## Fix Focus Areas
- src/modules/handleResponse.ts[214-230]
- src/modules/initialize.ts[56-76]
- src/support.ts[7-12]

## Suggested fix
In the `!render` branch:
- Do not assign `window.props[testId] = props` (or explicitly `delete window.props[testId]` to prevent accumulation).
Optionally, also avoid creating/reactifying a persistent props array in `initialize()` when `!shouldRenderUi()` (build a minimal per-call structure instead), since it won’t be rendered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Prism transform still eager 📎 Requirement gap ➹ Performance
Description
When UI rendering is enabled, the response body Prism transform() runs eagerly during
handleResponse() rather than being deferred until a user explicitly views/expands the heavy
content. This does not meet the lazy/virtual rendering requirement and can still cause DOM-heavy
overhead in open mode.
Code

src/modules/handleResponse.ts[R123-151]

+  // Syntax highlighting (Prism) is only needed to render the UI. Skip it when the UI is
+  // disabled (e.g. run mode) — this is the expensive work that crashes large responses (#135).
+  if (render) {
+    if (contentTypeHeader) {
+      const contentType = contentTypeHeader.split(';')[0]
+      const formats = {
+        'text/xml': 'xml',
+        'application/json': 'json',
+        'text/html': 'html',
+        'text/plain': 'plaintext',
+        'application/octet-stream': 'plaintext',
+      } as const
+      const definedFormat = formats[contentType as keyof typeof formats]
+      const rawLanguage = definedFormat || getFormat(bodyForTransform)
+      // Prism registers XML/HTML under "markup"; use it for highlighting
+      const language = (rawLanguage === 'xml' || rawLanguage === 'html') ? 'markup' : rawLanguage
+      props[index].responseBody.formatted = transform(bodyForTransform, language)
+    } else if (body !== undefined && body !== null && body !== '') {
+      const rawLanguage = getFormat(bodyForTransform)
+      const language = (rawLanguage === 'xml' || rawLanguage === 'html') ? 'markup' : rawLanguage
+      props[index].responseBody.formatted = transform(bodyForTransform, language)
+    }

-  if (!props[index].responseBody.formatted || !props[index].responseBody.formatted.length) {
-    if (bodyRaw && typeof bodyRaw === 'string' && bodyRaw.trim().length > 0) {
-      props[index].responseBody.formatted = transform(bodyRaw, 'plaintext')
-    } else if (bodyRaw && typeof bodyRaw === 'object' && bodyRaw !== null) {
-      props[index].responseBody.formatted = transform(bodyRaw, 'json')
+    if (!props[index].responseBody.formatted || !props[index].responseBody.formatted.length) {
+      if (bodyRaw && typeof bodyRaw === 'string' && bodyRaw.trim().length > 0) {
+        props[index].responseBody.formatted = transform(bodyRaw, 'plaintext')
+      } else if (bodyRaw && typeof bodyRaw === 'object' && bodyRaw !== null) {
+        props[index].responseBody.formatted = transform(bodyRaw, 'json')
+      }
Evidence
Rule 6643 requires deferring DOM-heavy rendering unless explicitly viewed. The changed code still
calls transform(bodyForTransform, language) and related fallback transforms during
handleResponse() whenever render is true, which is eager rendering rather than on-demand.

Implement lazy/virtual UI rendering to avoid DOM-heavy output unless explicitly viewed
src/modules/handleResponse.ts[123-151]
src/modules/api.ts[21-22]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Compliance requires lazy/virtual rendering so DOM-heavy output (e.g., Prism-highlighted full response bodies) is not generated unless explicitly viewed. The current implementation still computes `transform()` for the full body whenever `render` is true.
## Issue Context
The PR added `disableUi` gating, but open mode/default rendering still eagerly generates Prism markup. A compliant approach would store raw response text and only run Prism formatting when the user expands/views the response body (or virtualize long bodies).
## Fix Focus Areas
- src/modules/handleResponse.ts[123-151]
- src/modules/api.ts[21-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
6. Large props retained globally 🐞 Bug ➹ Performance
Description
In the !render path, handleResponse() still stores the full reactive props array (including
responseBody.body with the raw payload) into the global window.props[testId], keeping large
responses strongly referenced even though Prism/Vue rendering is skipped. This undermines the
intended memory/performance benefit of disableUi and can still contribute to large-memory
CI/run-mode executions across many cy.api() calls.
Code

src/modules/handleResponse.ts[R214-230]

+  // No UI mounted: still log the request (with response/cURL in consoleProps), but skip
+  // the DOM lookup, snapshot and scroll that only make sense with the rendered UI.
+  if (!render) {
+    log.set({
+      consoleProps() {
+        return {
+          yielded,
+          cURL: generateCurl()
+        }
+      }
+    })
+
+    window.props[testId] = props
+    log.end()
+
+    return cy.wrap(res, { log: false })
+  }
Evidence
The evidence shows that even when rendering is disabled (render is false / shouldRenderUi() is
false), the code still assigns the raw response body into props[index].responseBody.body and then
persists the entire props object into window.props[testId]. Because window.props is a global
store and initialize() continues to build the reactive props list from window.props, the large
payloads remain referenced for the lifetime of the spec/test rather than being eligible for garbage
collection, negating part of the no-UI mode’s performance/memory goal.

src/modules/handleResponse.ts[121-230]
src/support.ts[7-14]
src/modules/initialize.ts[56-62]
/src/modules/handleResponse.ts[121-122]
/src/modules/handleResponse.ts[214-229]
/src/modules/initialize.ts[51-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `shouldRenderUi()` is false (`!render` path), the implementation correctly skips Prism/Vue/DOM work, but it still writes the full reactive `props` (including potentially huge `responseBody.body`) into `window.props[testId]`. This keeps large payloads alive in a long-lived global store and can accumulate memory across many requests in CI/run mode, undermining the purpose of `disableUi`.
## Issue Context
- `props[index].responseBody.body` is still populated with the raw response body even in no-render mode.
- The entire `props` object is then stored in `window.props[testId]`, and `window.props` is initialized as a global store.
- `initialize()` continues to rebuild the reactive `props` list from `window.props`, reinforcing the long-lived reference chain.
- The feature’s goal is to prevent renderer/UI crashes and reduce memory/perf cost on large responses in `cypress run`/CI; avoiding DOM work helps, but retaining full bodies globally can still cause memory pressure.
## Fix Focus Areas
- src/modules/handleResponse.ts[121-230]
- src/modules/initialize.ts[51-83]
- src/support.ts[7-14]
## Suggested direction
Adjust the `!render` branch to avoid retaining large response bodies in a global store:
- Preferred for performance: don’t persist `window.props[testId]` at all when UI is disabled (and ensure `initialize()` doesn’t depend on it in no-UI mode).
- Or: persist only a minimal/lightweight shape (omit `responseBody.body`, formatted HTML, and other large fields; optionally cap/truncate body to N bytes/chars).
- If retaining history for a later UI re-enable is required, document the tradeoff and enforce a size cap to prevent unbounded growth.
Goal: in no-UI mode, avoid keeping large bodies strongly referenced beyond the command’s lifetime.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Comments in tests 🧑 Team insight ⚙ Maintainability
Description
**Inspired by goldbergyoni's coding and review patterns** — The new disableUi spec adds inline
comments that don’t add much beyond what the test names/assertions already say. This increases test
noise and makes the suite harder to scan/maintain.
Code

cypress/e2e/disableUi.cy.ts[R10-22]

+  afterEach(() => {
+    // Restore the suite default (config forces UI on so other specs render).
+    setPluginConfig('disableUi', false)
+  })
+
+  it('disableUi: true skips rendering the plugin UI but still yields the response', () => {
+    setPluginConfig('disableUi', true)
+    cy.api('/').then((res) => {
+      expect(res.status).to.eq(200)
+    })
+    // No Vue app mounted and no response body rendered into the DOM.
+    cy.get('#api-plugin-root').should('not.exist')
+    cy.get('[data-cy="responseBody"]').should('not.exist')
Evidence
The spec contains inline comments inside the test flow and teardown; per goldbergyoni’s review
conventions, tests should be self-documenting and avoid commentary that doesn’t strengthen the
scenario. This is exactly the kind of “test noise” they tend to push back on for long-term
maintainability.

/cypress/e2e/disableUi.cy.ts[8-23]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Inline comments inside the new e2e spec reduce scan-ability and add noise.
### Issue Context
The assertions/test names already explain intent; comments are redundant.
### Fix Focus Areas
- cypress/e2e/disableUi.cy.ts[10-23]
### Expected change
- Remove the inline comments in `afterEach` and the first test.
- If any nuance is important (e.g., suite default is forced in config), encode it in the `describe`/`it` names instead.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +17 to +21
const disableUi = getPluginConfig('disableUi')
if (disableUi === true) return false
if (disableUi === false) return true
// Default: Cypress.config('isInteractive') is true in open mode and false in run mode.
return Cypress.config('isInteractive') !== false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. disableui: false enables run-mode ui 📎 Requirement gap ➹ Performance

shouldRenderUi() allows forcing full UI rendering in Cypress run mode when disableUi is
explicitly set to false, which defeats the run-mode/CI safety requirement and can reintroduce
renderer crashes/perf issues. The repo also configures disableUi: false in cypress.config.ts,
causing UI rendering under cypress run.
Agent Prompt
## Issue description
Compliance requires that the plugin must not render the full UI/DOM output in Cypress run mode / CI. The new `disableUi` logic currently allows `disableUi: false` to force rendering even in run mode, and the repo’s `cypress.config.ts` sets `disableUi: false`, which will render UI during `cypress run`.

## Issue Context
The intended performance safeguard for CI/run mode should be unconditional per the compliance rule; users can still use UI in open mode, and can still disable UI in any mode.

## Fix Focus Areas
- src/utils/shouldRenderUi.ts[17-21]
- cypress.config.ts[9-11]
- README.md[80-85]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +197 to +212
const generateCurl = () => {
let curl = `curl -X ${options.method || 'GET'} "${options.url}"`;
if (options.headers) {
Object.entries(options.headers).forEach(([key, value]) => {
curl += ` -H "${key}: ${value}"`;
});
}
if (options.body) {
if (typeof options.body === 'object') {
curl += ` -d '${JSON.stringify(options.body)}'`;
} else {
curl += ` -d '${options.body}'`;
}
}
return curl;
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

5. Unsafe duplicate curl builder 🧑 Team insight ⛨ Security

**Inspired by avrik-berman's coding and review patterns** — handleResponse() reintroduces a local
generateCurl() that doesn’t escape quotes/newlines and bypasses the existing shared helper,
risking malformed commands and credential leakage (especially when hideCredentials is enabled).
This also duplicates logic already centralized in src/utils/generateCurl.ts.
Agent Prompt
### Issue description
`src/modules/handleResponse.ts` defines a local `generateCurl()` that (a) duplicates existing functionality and (b) builds a cURL string without escaping/quoting safeguards. It also uses `options.*` rather than the (potentially anonymized) `props[index]`, which can leak sensitive values when `hideCredentials` is enabled.

### Issue Context
A shared `generateCurl(item: RequestProps)` already exists and includes escaping for quotes/newlines and composes auth/query/headers/body consistently.

### Fix Focus Areas
- src/modules/handleResponse.ts[195-246]
- src/utils/generateCurl.ts[1-75]
- src/modules/api.ts[15-24]

### Expected change
- Remove the local `generateCurl` closure from `handleResponse()`.
- Import `generateCurl` from `@utils/generateCurl` and use `generateCurl(props[index])` in both render and non-render consoleProps paths.
- Ensure the generated cURL reflects anonymized props when `hideCredentials` is on (already achieved by using `props[index]`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@qodo-for-filiphric

Copy link
Copy Markdown

Qodo Fixer

🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (2)

Grey Divider

🔗 Fix PR: #157

This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR.

Prompt for coding agent

This is an automated fix prepared on a separate branch (#157). It is NOT applied to this PR.
To use it: review Fix PR #157 (https://github.com/filiphric/cypress-plugin-api/pull/157), evaluate each change critically against your local context, and cherry-pick the changes that are correct into this branch. Do not accept them blindly.
Process — 2 fixed
  • ☑ Fixed: disableUi: false enables run-mode UI
  • ☑ Fixed: Unsafe duplicate cURL builder

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.

Test run crashes and performance issues when plugin API is rendered during 'run mode' in CI

1 participant