Skip to content

Conversation

@zhravan
Copy link
Collaborator

@zhravan zhravan commented Oct 11, 2025

Issue

Link to related issue(s):


Description

Short summary of what this PR changes or introduces.


Scope of Change

Select all applicable areas impacted by this PR:

  • View (UI/UX)
  • API
  • CLI
  • Infra / Deployment
  • Docs
  • Other (specify): ________

Screenshot / Video / GIF (if applicable)

Attach or embed screenshots, screen recordings, or GIFs demonstrating the feature or fix.


Related PRs (if any)

Link any related or dependent PRs across repos.


Additional Notes for Reviewers (optional)

Anything reviewers should know before testing or merging (e.g., environment variables, setup steps).


Developer Checklist

To be completed by the developer who raised the PR.

  • Add valid/relevant title for the PR
  • Self-review done
  • Manual dev testing done
  • No secrets exposed
  • No merge conflicts
  • Docs added/updated (if applicable)
  • Removed debug prints / secrets / sensitive data
  • Unit / Integration tests passing
  • Follows all standards defined in Nixopus Docs

Reviewer Checklist

To be completed by the reviewer before merge.

  • Peer review done
  • No console.logs / fmt.prints left
  • No secrets exposed
  • If any DB migrations, migration changes are verified
  • Verified release changes are production-ready

Summary by CodeRabbit

  • Bug Fixes

    • Corrected generation of the SuperTokens connection URI to properly handle IP addresses, domain names and ports.
    • Ensures consistent protocol handling to prevent misconfiguration during installation/setup.
    • Fixed computation of the app API domain to preserve the protocol portion when deriving the base URL.
  • Chores

    • Bumped CLI version to 0.1.10.

@zhravan zhravan changed the title Fix port ip vs uri fix: supertokens URI for ip vs domain Oct 11, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 11, 2025

Walkthrough

Adds a private helper to compute SUPERTOKENS_CONNECTION_URI with IP-vs-domain and protocol handling, updates its usage in the install flow, adjusts app domain string handling in front-end config, and bumps the CLI package version from 0.1.9 to 0.1.10.

Changes

Cohort / File(s) Summary
Install command: URI construction
cli/app/commands/install/run.py
Adds _get_supertokens_connection_uri(protocol, api_host, supertokens_api_port) using ipaddress to distinguish IP hosts from domain names and format the URI (preserving http/https); replaces inline SUPERTOKENS_CONNECTION_URI construction in _update_environment_variables with this helper; adds a temporary TODO comment.
Packaging / version bump
cli/pyproject.toml
Bumps package version from 0.1.9 to 0.1.10; no other project config changes.
Front-end app info string handling
view/app/config/appInfo.ts
Changes computation of apiDomain in getAppInfo to temporarily mask :// as TEMP before stripping /api, then restores :// to avoid removing the protocol portion during baseUrl manipulation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant Install
  participant Env as _update_environment_variables
  participant Helper as _get_supertokens_connection_uri
  participant IP as ipaddress

  User->>Install: run install
  Install->>Env: update environment variables
  Env->>Helper: compute SUPERTOKENS_CONNECTION_URI(protocol, api_host, port)
  Helper->>IP: attempt parse api_host as IP
  alt api_host is IP
    Helper-->>Env: return http(s)://<ip>:<port>
  else api_host is domain
    Helper-->>Env: return http(s)://<host>:<port>
  end
  Env-->>Install: env vars updated
  Install-->>User: install complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

nixopus-view

Suggested reviewers

  • raghavyuva

Poem

I twitched my nose at hosts and names,
IPs or domains—different games.
A hop, a skip, the URI’s right,
Protocols kept and ports in sight.
Version bumped—carrots for cheer! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change, namely fixing how the SuperTokens connection URI handles IP addresses versus domain names. It avoids unnecessary detail about secondary tweaks like version bumps or string manipulation in the view config, keeping the focus on the main functional fix. The phrasing is clear and concise, enabling teammates to quickly understand the purpose of the PR.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@zhravan zhravan requested a review from raghavyuva October 11, 2025 10:18
Copy link
Contributor

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 430616c and c2a0d02.

📒 Files selected for processing (2)
  • cli/app/commands/install/run.py (3 hunks)
  • cli/pyproject.toml (1 hunks)
🔇 Additional comments (2)
cli/pyproject.toml (1)

3-3: LGTM! Version bump aligns with the fix.

The version increment from 0.1.9 to 0.1.10 is appropriate for the SuperTokens URI fix in this PR.

cli/app/commands/install/run.py (1)

473-474: Clarify and refine SUPERTOKENS_CONNECTION_URI TODO
Update the TODO to state that this HTTP-only workaround should be removed once TLS is supported for SuperTokens. Verify that _get_supertokens_connection_uri properly distinguishes IP (no port) vs domain (with port) and that all three invocations of _update_environment_variables in run.py apply it consistently.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (1)
view/app/config/appInfo.ts (1)

5-6: Use URL API for robust URL manipulation.
The TEMP-masking can collide if baseUrl contains "TEMP" (e.g. http://temp-server.com/api) and masking :// isn’t needed.
Refactor:

- const apiDomain = baseUrl.replace('://','TEMP').replace('/api','').replace('TEMP','://');
+ const url = new URL(baseUrl);
+ url.pathname = url.pathname.replace(/\/api$/,'');
+ const apiDomain = url.toString().replace(/\/$/,'');

Or, if you only need to drop a trailing /api:

- const apiDomain = baseUrl.replace('://','TEMP').replace('/api','').replace('TEMP','://');
+ const apiDomain = baseUrl.replace(/\/api$/,'');

Please clarify which IP vs domain edge case this change targets.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2a0d02 and 9f59b11.

📒 Files selected for processing (1)
  • view/app/config/appInfo.ts (1 hunks)

@zhravan zhravan merged commit aaddb3c into raghavyuva:master Oct 11, 2025
5 of 6 checks passed
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.

1 participant