Skip to content

Fix detector URL scans and advisory handling - #709

Merged
pbakaus merged 14 commits into
mainfrom
codex/fix-625-detector
Sep 2, 2026
Merged

Fix detector URL scans and advisory handling#709
pbakaus merged 14 commits into
mainfrom
codex/fix-625-detector

Conversation

@pbakaus

@pbakaus pbakaus commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • recover joined multi-URL arguments only when every whitespace-delimited token is independently a URL, while preserving local paths with spaces
  • derive advisory behavior from the canonical registry severity across CLI, JSON, browser, and hooks
  • include readable linked stylesheet CSS in URL pattern scans without reporting unused selectors
  • report only the uniquely dominant primary font and document detector output streams and browser limits

Validation

  • bun run build
  • bun test tests/detect-antipatterns.test.js
  • node --test tests/hook.test.mjs
  • node --test tests/detect-antipatterns-fixtures.test.mjs
  • node --test tests/detect-antipatterns-browser.test.mjs (40 passed)
  • node --test tests/detect-cli-stdin-dispatch.test.mjs
  • node --test tests/live-server.test.mjs (101 passed after clearing pre-existing orphaned fixed-port helpers)
  • node --test tests/live-poll-stream.test.mjs (2 passed after clearing the final pre-existing fixed-port helper)

Refs #625

AI assistance disclosure: Implemented and verified with Codex under maintainer direction.


Note

Medium Risk
Large changes to browser URL scanning and CSSOM filtering can shift which findings appear or disappear; advisory/exit-code behavior changes may affect CI gates that assumed former slop rules were blocking failures.

Overview
Improves URL/browser detection and advisory behavior across CLI, JSON, hooks, and the injected browser engine.

Browser URL scans now fold readable linked stylesheets into the HTML pattern corpus, but only for rules that resolve to live DOM hosts. Inactive @media / @supports / pseudo-class rules and unused selectors are dropped; @container rules are verified with a computed-style probe; keyframes are serialized from the Web Animations API when available. Selector attribution uses smarter pseudo-element → host resolution instead of blunt stripping (so inactive :not() rules are not falsely broadened).

Advisory rules use registry severity: 'advisory' (replacing advisory: true). Runtime advisory flags and exit-code partitioning derive from effective severity everywhere, including per-finding promotions in the browser serializer.

CLI: documents stderr vs stdout (--json on stdout, human output via 2>); expandJoinedUrlTargets splits a single argv string of multiple URLs without breaking paths that contain spaces.

overused-font flags only the uniquely most-used family (no more 15% “primary” threshold). enclosingCssSelector ignores {/; inside CSS comments when attributing matches.

Tests and fixtures cover joined URLs, linked-CSS/container/keyframes cases, and advisory JSON/exit behavior.

Reviewed by Cursor Bugbot for commit 04c5f0b. Bugbot is set up for automated code reviews on this repo. Configure here.

Recover joined URL arguments without splitting local paths, derive advisory behavior from registry severity across consumers, inspect readable linked CSS in URL scans, and report only the dominant primary font.

AI assistance disclosure: Implemented and verified with Codex under maintainer direction.
Copilot AI lite review requested due to automatic review settings September 2, 2026 16:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes core detector URL-scan inputs, advisory exit-code semantics, and browser bundle behavior across multiple runtimes, so a final human review is warranted despite strong test coverage.

Pull request overview

This PR fixes several correctness gaps in the detector’s URL-mode and advisory handling, aligning behavior across the CLI, JSON output, browser bundle, and design hook while adding regression coverage for the previously shipped bugs (Issue #625).

Changes:

  • Fixes multi-URL “joined argv token” handling by only splitting targets when every whitespace-delimited part is itself a URL.
  • Makes severity: 'advisory' the canonical source of truth for advisory behavior (exit codes, filtering, hook behavior), while preserving advisory: true as a compatibility/output flag.
  • Expands URL-mode pattern scans to include readable linked stylesheet CSS (same-origin / CORS-readable), while still dropping selector-scoped findings that don’t match the live DOM.
File summaries
File Description
tests/hook.test.mjs Extends hook filtering tests to cover advisory inclusion config and severity: 'advisory' classification.
tests/fixtures/antipatterns/linked-url-patterns.html Adds a browser fixture page to validate linked-stylesheet pattern scanning in URL mode.
tests/fixtures/antipatterns/linked-url-patterns.css Adds linked CSS that should trigger a pattern while ensuring unused selectors aren’t reported in live URL findings.
tests/detect-cli-stdin-dispatch.test.mjs Updates stdin dispatch test to accept advisory-only runs as non-blocking (exit 0) and assert advisory serialization.
tests/detect-antipatterns.test.js Updates CLI contract tests for advisory-only exit behavior and stderr/stdout stream expectations.
tests/detect-antipatterns-browser.test.mjs Adds browser regression tests for joined multi-URL expansion, linked CSS scanning, advisory serialization, and dominant-font reporting.
skill/scripts/hook-lib.mjs Teaches hook advisory detection to honor severity: 'advisory' in addition to legacy flags.
README.md Documents output streams (stderr vs stdout), URL-mode linked CSS behavior, and limits of detector signal.
cli/engine/rules/checks.mjs Adjusts overused-font to report only the uniquely dominant primary font family (no ties / no “secondary primary”).
cli/engine/registry/antipatterns.mjs Moves advisory classification to severity: 'advisory' and derives the advisory id set from severity.
cli/engine/findings.mjs Stamps advisory: true based on the registry-derived advisory id set (severity-driven), while keeping severity serialized.
cli/engine/detect-antipatterns-browser.js Updates the browser bundle to align advisory semantics, dominant-font logic, and linked stylesheet CSS inclusion in URL scans.
cli/engine/cli/main.mjs Adds joined multi-URL expansion logic, aligns advisory partitioning with severity, and documents output streams in --help.
cli/engine/browser/injected/index.mjs Updates the injected browser path to include readable linked stylesheet CSS in the HTML pattern corpus for URL scans.
Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cli/engine/browser/injected/index.mjs
Comment thread cli/engine/browser/injected/index.mjs
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Greptile Summary

The browser detector now aligns linked stylesheet analysis with rendered page behavior, including conditional rules, container queries, selector applicability, and browser-resolved animation keyframes. The CLI also correctly recovers accidentally whitespace-joined URL inputs while preserving legitimate URL and filesystem paths containing spaces.

Confidence Score: 5/5

No blocking failure remains.

Browser and command-line behavior was exercised against real Chromium and the public CLI interface. The checked behaviors correctly filter inactive or unreachable CSS, retain active styles and effective animations, enforce promoted findings, and preserve valid targets containing spaces.

T-Rex T-Rex Logs

What T-Rex did

  • Ran the T-Rex browser repro script from the repository with exit code 0; Chromium evaluated inactive linked media queries and found no repeating-gradient finding.
  • Retained warning-level findings from the hero blinking-cursor fixture during browser detection and CLI checks; the CLI exits with code 2.
  • Ran Chromium against an inactive container query with a linked repeating-gradient declaration; background-image computed as none and zero repeating-stripe findings.
  • Compared the active linked container-query case before and after implementation; Chromium rendered the repeating gradient in both runs and the detector found the expected repeating-gradient finding.
  • Exercised the linked card and related selector tests on live hosts; no gradients rendered and the detector reported no repeating-stripe findings, while the browser detector suite completed with 40 passing tests.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (12): Last reviewed commit: "Fix keyframe easing detection" | Re-trigger Greptile

Comment thread cli/engine/browser/injected/index.mjs
Comment thread cli/engine/browser/injected/index.mjs Outdated
Flatten linked stylesheet grouping rules and collect only selector rules that target the live DOM, preventing unused grouped and selector-less patterns from leaking into URL findings.

AI assistance disclosure: Implemented and verified with Codex under maintainer direction.
AI assistance disclosure: Codex implemented and verified these fixes under maintainer direction.
Comment thread cli/engine/browser/injected/index.mjs
AI assistance disclosure: Codex implemented and verified this fix under maintainer direction.
Comment thread tests/detect-antipatterns-browser.test.mjs
Comment thread cli/engine/browser/injected/index.mjs Outdated
Resolve pseudo-element selectors to live hosts, reject unresolvable linked CSS findings, and make the regression assertions independent. Also ignore comment delimiters when recovering CSS rule selectors.

AI assistance disclosure: This commit was prepared with Codex under maintainer direction.
Comment thread cli/engine/browser/injected/index.mjs
Exclude linked container-query groups when their current applicability cannot be resolved, with a browser regression proving inactive styles do not leak.

AI assistance disclosure: This commit was prepared with Codex under maintainer direction.
@pbakaus

pbakaus commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Addressed the final Greptile container-query finding in eec431e. Linked @container groups are now omitted when their current applicability cannot be resolved, rather than treating them as active. The browser regression uses a matching live element inside an inactive width query, proves its computed background is none, and verifies no pattern finding leaks. Full browser suite: 40/40. AI assistance disclosure: this comment was prepared with Codex under maintainer direction.

Comment thread cli/engine/browser/injected/index.mjs Outdated
Use a temporary custom-property probe so the browser decides whether a nested style rule actually applies in the current container layout.

AI assistance disclosure: Codex helped implement and test this fix under maintainer direction.
@pbakaus

pbakaus commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Addressed the latest Greptile finding in 720222e.

The linked-stylesheet walker no longer drops all @container rules. For each nested style rule with live selector matches, it temporarily injects a unique custom property into the CSSOM rule and reads computed style from the matched host/pseudo-element. That delegates active-vs-inactive container evaluation to the browser, then restores the rule immediately.

Regression coverage now proves both sides in the same fixture: an inactive container rule is excluded and an active container rule is included and detected.

Validation: bun run build:browser; targeted Chromium regression 1/1; full Chromium browser suite 40/40.

AI assistance disclosure: Codex helped implement, test, and summarize this fix under maintainer direction.

Comment thread cli/engine/browser/injected/index.mjs
Comment thread cli/engine/browser/injected/index.mjs Outdated
Keep valid empty pseudo-class matches authoritative and omit selector-less linked at-rules that cannot be tied to rendered nodes.

AI assistance disclosure: Codex helped implement and test this fix under maintainer direction.
@pbakaus

pbakaus commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Validation for ecfcd77:

  • bun run build:browser — pass
  • targeted linked-CSS Chromium regression — 1/1 pass
  • full Chromium detector suite — 40/40 pass

The two negative cases use independent signatures: inactive pseudo-class CSS would emit repeating-stripes-gradient, while an inactive container-scoped keyframes rule would emit gradient-text. Both remain absent while the active container and pseudo-element positive cases still fire.

AI assistance disclosure: Codex helped implement, test, and summarize this fix under maintainer direction.

Comment thread cli/engine/browser/injected/index.mjs Outdated
Comment thread cli/engine/browser/injected/index.mjs Outdated
Preserve quoted attribute values and escaped identifiers while resolving real pseudo-elements to live hosts.

AI assistance disclosure: Codex helped implement and test this fix under maintainer direction.
@pbakaus

pbakaus commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Validation for a5af5b2:

  • bun run build:browser — pass
  • targeted linked-CSS Chromium regression — 1/1 pass
  • full Chromium detector suite — 40/40 pass

New coverage distinguishes selector syntax from literal data: a quoted ::before attribute value is retained when live, the same literal on an unused selector cannot match an empty-value decoy, and an escaped-colon identifier is retained.

AI assistance disclosure: Codex helped implement, test, and summarize this fix under maintainer direction.

AI assistance disclosure: Codex helped implement and verify this fix under maintainer direction.
Comment thread cli/engine/browser/injected/index.mjs Outdated
AI assistance disclosure: Codex helped implement and verify this fix under maintainer direction.
Comment thread cli/engine/browser/injected/index.mjs Outdated
AI assistance disclosure: Codex helped implement and verify this fix under maintainer direction.
Comment thread cli/engine/browser/injected/index.mjs
AI assistance disclosure: Codex helped implement and verify this fix under maintainer direction.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1902ef0. Configure here.

Comment thread cli/engine/browser/injected/index.mjs
Serialize effective per-keyframe easing back into the linked stylesheet corpus so overshoot motion is detected. Add a browser regression with a neutral animation name.\n\nAI assistance disclosure: Codex helped implement and test this fix under maintainer direction.
@pbakaus
pbakaus merged commit fa44839 into main Sep 2, 2026
12 checks passed
@linear-code

linear-code Bot commented Sep 2, 2026

Copy link
Copy Markdown

REN-293

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.

2 participants