Skip to content

fix: remove user= from ConnectionDialog snippet, add CLI login hint#883

Merged
PythonFZ merged 6 commits intomainfrom
fix/connection-dialog-snippet
Mar 20, 2026
Merged

fix: remove user= from ConnectionDialog snippet, add CLI login hint#883
PythonFZ merged 6 commits intomainfrom
fix/connection-dialog-snippet

Conversation

@PythonFZ
Copy link
Copy Markdown
Member

@PythonFZ PythonFZ commented Mar 20, 2026

Summary

  • Remove user= parameter from the Python connection snippet in ConnectionDialog — it broke after auth validation started requiring both user + password or neither
  • Add a zndraw-cli auth login command hint shown to all users for persistent authentication
  • Add "(requires authentication)" note to the session code section
  • Fix e2e test helpers: CLI env var passing, --room-id--room flag
  • Fix TypeScript null-guard in cliLoginApprove.tsx

Test plan

  • Playwright test "connection info dialog opens and closes" passes
  • Verifies snippet contains no user= parameter
  • Verifies login hint with zndraw-cli auth login is visible
  • Verifies "requires authentication" text is shown
  • Verifies 3 copy-to-clipboard buttons (snippet, login command, session code)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Added design specification for connection dialog authentication flow.
  • New Features

    • Added authentication guidance with CLI login command in connection dialog.
    • Updated Python code snippet to remove explicit credentials.
  • Bug Fixes

    • Added validation to prevent unintended actions when authentication code is missing.
  • Tests

    • Updated e2e test suite with updated CLI command parameters.
    • Enhanced connection dialog assertions to verify authentication guidance and snippet content.

PythonFZ and others added 2 commits March 20, 2026 21:09
The snippet showed user="xxx@guest.user" without password, which broke
after auth validation started requiring both or neither. Now the snippet
is universal (no user=), and a CLI login command is shown for
authenticating as the current user. Also fixes e2e test helpers
(CLI env var passing, --room flag) and a TS null-guard in cliLoginApprove.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 20, 2026

Warning

Rate limit exceeded

@pre-commit-ci[bot] has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 21 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0b0b12f1-2c10-4b5f-aeb4-d103b977e89a

📥 Commits

Reviewing files that changed from the base of the PR and between d5ed1aa and 2e485d5.

📒 Files selected for processing (2)
  • frontend/e2e/ui-panels-chat.spec.ts
  • frontend/src/components/ConnectionDialog.tsx
📝 Walkthrough

Walkthrough

This change improves credential security in the connection dialog by removing user credentials from Python snippets and adding an authentication workflow. The CLI room creation parameter is standardized across all e2e tests, and a guard clause prevents unauthorized login approvals when code is missing.

Changes

Cohort / File(s) Summary
Credential Security & Component Updates
frontend/src/components/ConnectionDialog.tsx, frontend/src/pages/cliLoginApprove.tsx
Removed user= parameter from Python connection snippet; added login command (zndraw-cli auth login --url) with copy functionality; refactored syntax highlighting to reduce repetition; added guard clause in login approval handler to prevent requests without a code parameter.
Connection Dialog UI Specification
docs/superpowers/specs/2026-03-20-connection-dialog-snippet-design.md
New specification documenting the credential-less Python snippet design, login hint display with email and CLI command, session code authentication label, and Playwright test validation criteria.
E2E Test CLI Parameter Standardization
frontend/e2e/ui-panels-chat.spec.ts, frontend/e2e/camera-session.spec.ts, frontend/e2e/chat-features.spec.ts, frontend/e2e/constraint-visualization.spec.ts, frontend/e2e/editing.spec.ts, frontend/e2e/extensions-analysis.spec.ts, frontend/e2e/frame-invalidation.spec.ts, frontend/e2e/frames-navigation.spec.ts, frontend/e2e/geometry-drawing.spec.ts, frontend/e2e/registration.spec.ts, frontend/e2e/socket-sync.spec.ts
Updated all e2e test room creation commands from rooms create --room-id to rooms create --room; strengthened connection dialog assertions to verify absence of credentials and presence of authentication guidance.
CLI Environment Configuration
frontend/e2e/helpers.ts
Changed CLI invocation to pass ZNDRAW_URL via environment variables (env: { ...process.env, ZNDRAW_URL: BASE_URL }) instead of command-line arguments.

Poem

🐰 A snippet sans secrets, now safe and so bright,
With auth commands dancing beneath golden light,
No credentials exposed on the screen—just the way,
To login with CLI, come what may!
The rooms now have shorter names too, what a delight! ✨

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: removing the user= parameter from the ConnectionDialog snippet and adding a CLI login hint.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/connection-dialog-snippet

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/components/ConnectionDialog.tsx`:
- Around line 78-80: The UI is rendering "null" because userName (from
state.user?.email ?? null) can be null; update ConnectionDialog to either
conditionally render the Typography block only when userName is non-null (e.g.,
guard around the JSX using userName) or provide a clear fallback string (e.g.,
'your email' or 'an account') instead of null; locate the userName usage in
ConnectionDialog and change the render logic around the <Typography> that says
"To authenticate as <strong>{userName}</strong>, run:" accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f0c7e38-0813-460e-bc2a-18e37e475f16

📥 Commits

Reviewing files that changed from the base of the PR and between aa5a587 and f45f70a.

📒 Files selected for processing (6)
  • docs/superpowers/specs/2026-03-20-connection-dialog-snippet-design.md
  • frontend/e2e/helpers.ts
  • frontend/e2e/ui-panels-chat.spec.ts
  • frontend/playwright.config.ts
  • frontend/src/components/ConnectionDialog.tsx
  • frontend/src/pages/cliLoginApprove.tsx

Comment thread frontend/src/components/ConnectionDialog.tsx
PythonFZ and others added 4 commits March 20, 2026 21:21
…2e tests

- Guard against null userName rendering empty <strong> tags
- Replace --room-id with --room in all e2e test files (correct CLI flag)
- Remove unnecessary process.env.BASE_URL fallback

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cede ZnDraw()

- Connection snippet shown first as the default (connects as guest)
- Login command clearly states "before creating the ZnDraw object"
- Session access grouped with login as dependent step

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@PythonFZ PythonFZ merged commit 1eeb600 into main Mar 20, 2026
6 checks passed
@PythonFZ PythonFZ deleted the fix/connection-dialog-snippet branch March 20, 2026 20:40
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@aa5a587). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #883   +/-   ##
=======================================
  Coverage        ?   91.24%           
=======================================
  Files           ?      179           
  Lines           ?    17240           
  Branches        ?        0           
=======================================
  Hits            ?    15731           
  Misses          ?     1509           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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