Skip to content

fix(pds-core): scope chooser enrichment to account rows#137

Closed
Kzoeps wants to merge 2 commits into
mainfrom
account-sticky
Closed

fix(pds-core): scope chooser enrichment to account rows#137
Kzoeps wants to merge 2 commits into
mainfrom
account-sticky

Conversation

@Kzoeps
Copy link
Copy Markdown
Contributor

@Kzoeps Kzoeps commented May 1, 2026

Summary

  • Scope OAuth chooser email enrichment to actual account rows so consent copy is not mutated after account selection.
  • Add regression coverage for consent text, chooser rows, and multiple remembered accounts.
image image

Validation

  • pnpm vitest run packages/pds-core/src/tests/chooser-enrichment.test.ts
  • pnpm typecheck could not run locally due missing pnpm TypeScript binary in the environment.

Summary by CodeRabbit

  • Bug Fixes

    • Improved scoping of chooser enrichment to prevent unwanted modifications outside intended account row UI areas.
  • Tests

    • Added comprehensive test coverage validating correct enrichment targeting for chooser account rows while respecting content outside these elements.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 1, 2026

⚠️ No Changeset found

Latest commit: 2acbc43

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

@vercel
Copy link
Copy Markdown

vercel Bot commented May 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
epds-demo Ready Ready Preview, Comment May 2, 2026 11:54am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65ce401a-7ecf-4fd6-9887-a16d65b6a88b

📥 Commits

Reviewing files that changed from the base of the PR and between ffc17bd and 2acbc43.

📒 Files selected for processing (2)
  • packages/pds-core/src/__tests__/chooser-enrichment.test.ts
  • packages/pds-core/src/chooser-enrichment.ts

📝 Walkthrough

Walkthrough

The PR adds a DOM-context guard to the chooser enrichment script, restricting enrichment to elements within chooser rows (elements with role="button" and tabindex="0"). It also introduces comprehensive test infrastructure and test cases to validate this scoping behavior.

Changes

Chooser Enrichment Row Scoping

Layer / File(s) Summary
Core DOM-Context Guard
packages/pds-core/src/chooser-enrichment.ts
The enrichment function now finds the closest ancestor with role="button" and tabindex="0" for each matched element and skips enrichment if no such row is found.
Testing Infrastructure & Setup
packages/pds-core/src/__tests__/chooser-enrichment.test.ts (lines 1–243)
Added Node vm execution context, fake DOM model classes (FakeDocument, FakeElement, FakeTextNode, FakeClassList), and helper function runChooserEnrichmentScript() to execute the generated script in a sandboxed environment with seeded window.__sessions.
Test Validation
packages/pds-core/src/__tests__/chooser-enrichment.test.ts (lines 244–305)
Test suite verifies that enrichment does not apply to consent copy outside chooser-like rows, and correctly enriches one or multiple chooser account rows by asserting injected labels and class placement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • aspiers
  • s-adamantine

🐰 A script that learns to look around,
Within button rows, enrichment is found,
Outside those bounds? A careful skip,
Tests in a sandbox catch each trip! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(pds-core): scope chooser enrichment to account rows' clearly and specifically summarizes the main change: adding DOM-context scoping to the chooser enrichment so it only applies to account rows, not unrelated content like consent copy.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch account-sticky

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 1, 2026

🚅 Deployed to the ePDS-pr-137 environment in ePDS

Service Status Web Updated (UTC)
@certified-app/pds-core ✅ Success (View Logs) Web May 2, 2026 at 11:56 am
@certified-app/demo ✅ Success (View Logs) Web May 1, 2026 at 10:43 am
@certified-app/demo untrusted ✅ Success (View Logs) Web May 1, 2026 at 10:43 am
@certified-app/auth-service ✅ Success (View Logs) Web May 1, 2026 at 10:43 am

@coveralls-official
Copy link
Copy Markdown

coveralls-official Bot commented May 1, 2026

Coverage Report for CI Build 25251298317

Coverage remained the same at 53.156%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 2806
Covered Lines: 1489
Line Coverage: 53.06%
Relevant Branches: 1709
Covered Branches: 911
Branch Coverage: 53.31%
Branches in Coverage %: Yes
Coverage Strength: 5.44 hits per line

💛 - Coveralls

@aspiers
Copy link
Copy Markdown
Contributor

aspiers commented May 1, 2026

@Kzoeps Please can you file a GH issue for whatever this is fixing so I can understand it? Thanks!

@Kzoeps
Copy link
Copy Markdown
Contributor Author

Kzoeps commented May 1, 2026

@Kzoeps Please can you file a GH issue for whatever this is fixing so I can understand it? Thanks!

@aspiers sure. This is not ready yet and shouldve been a draft. Will add you as reviewer soon as its ready and create a gh issue for it as well

@aspiers
Copy link
Copy Markdown
Contributor

aspiers commented May 1, 2026

@Kzoeps I filed #143 to track it based on our discussions.

@Kzoeps Kzoeps force-pushed the account-sticky branch from 6be5efa to 2acbc43 Compare May 2, 2026 11:53
@railway-app railway-app Bot temporarily deployed to ePDS / ePDS-pr-137 May 2, 2026 11:53 Destroyed
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 2, 2026

@Kzoeps Kzoeps marked this pull request as ready for review May 4, 2026 04:17
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@Kzoeps Kzoeps requested review from aspiers and s-adamantine May 4, 2026 04:17
@Kzoeps
Copy link
Copy Markdown
Contributor Author

Kzoeps commented May 5, 2026

closing this and just pointing #148 to main

@Kzoeps Kzoeps closed this May 5, 2026
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