Skip to content

test: handle CRLF in stale identity guard - #2741

Open
abhinavkr26104 wants to merge 2 commits into
browserbase:mainfrom
abhinavkr26104:test/stale-identity-crlf
Open

test: handle CRLF in stale identity guard#2741
abhinavkr26104 wants to merge 2 commits into
browserbase:mainfrom
abhinavkr26104:test/stale-identity-crlf

Conversation

@abhinavkr26104

@abhinavkr26104 abhinavkr26104 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

why

The stale-identity guard split git grep output only on \n. Windows CRLF output therefore left a trailing \r on every line except the last, causing explicitly allowed legacy cleanup strings to be reported as forbidden identities.

Fixes #2735

what changed

  • Split grep output on both LF and CRLF line endings.
  • Extract the match parser so the real guard and regression fixture exercise the same behavior.
  • Add a synthetic CRLF regression test that also runs on non-Windows CI.

test plan

  • vitest run rules/ast-grep/stale-identity.test.ts --reporter=verbose (2 passed on Windows)
  • Full rules/ast-grep suite (7 files passed, 35 tests passed)
  • Targeted Oxfmt and Oxlint checks
  • git diff --check

Summary by cubic

Parses CRLF line endings in the stale identity guard test to avoid Windows-only false positives. Previously we split git grep output on LF only; now we split on LF or CRLF via a shared parser.

  • Extracts and reuses findUnexpectedMatches to parse matches consistently.
  • Adds a CRLF regression test that runs on all CI platforms and documents the parsing rationale.
  • No behavior change for LF-only environments; reduces false positives on Windows.

Written for commit 36a4c3f. Summary will update on new commits.

Review in cubic

@changeset-bot

changeset-bot Bot commented Aug 15, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 36a4c3f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 1 file

Architecture diagram
sequenceDiagram
    participant Test as Test Suite
    participant Guard as Stale Identity Guard
    participant Parser as Match Parser
    participant Grep as Git Grep
    participant Allowlist as Legacy Allowlist

    Note over Test,Allowlist: CRLF/LF Line Ending Handling in Stale Identity Check
    
    Test->>Guard: Run "does not reintroduce" test
    Guard->>Grep: Execute git grep pattern
    alt Grep exits with code 1 (matches found)
        Grep-->>Guard: stdout with results
        Guard->>Parser: findUnexpectedMatches(stdout)
        Parser->>Parser: Trim output and split on /\r?\n/u
        Parser->>Parser: Filter empty lines
        loop Each match line
            Parser->>Parser: Parse file, line number, contents
            alt Parsed successfully
                Parser->>Parser: Build identity string
                Parser->>Allowlist: Check if identity is allowed
                alt Identity in allowlist
                    Allowlist-->>Parser: Allowed
                    Parser->>Parser: Filter out match
                else Identity not in allowlist
                    Allowlist-->>Parser: Not allowed
                    Parser->>Parser: Keep match
                end
            else Parse failed
                Parser->>Parser: Keep original match
            end
        end
        Parser-->>Guard: Unexpected matches list
        Guard-->>Test: Assert no unexpected matches
    else Grep exits with code 0 (no matches)
        Grep-->>Guard: Empty stdout
        Guard-->>Test: Empty unexpected list
    end

    Note over Test,Allowlist: Regression Test with CRLF Line Endings
    
    Test->>Parser: findUnexpectedMatches with CRLF joined output
    Parser->>Parser: Split on CRLF correctly
    Parser->>Allowlist: Verify exact legacy cleanup matches
    Allowlist-->>Parser: All identities allowed
    Parser-->>Test: Empty unexpected list
    Test->>Test: Assert no unexpected matches
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread rules/ast-grep/stale-identity.test.ts
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.

Stale identity guard rejects allowed matches on Windows CRLF output

1 participant