✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

• [feat] Utilize standardized app_base_url throughout e2e tests #13679 👈 (View in Graphite)
• [feat] Utilize pytest.mark.external_test in 2 tests #13738
• [feat] Add AppTarget class for test abstraction #13737
• [feat] Add external URL runner in e2e playwright #13678
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0000%

• Current PR: 86.8200% (13884 lines, 1829 missed)
• Latest develop: 86.8200% (13884 lines, 1829 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

Summary

This PR refactors the E2E test infrastructure to utilize a standardized app_base_url fixture and build_app_url() helper function throughout the test suite. This enables running E2E tests against externally hosted apps (via --external-app-url CLI option) instead of only local http://localhost:{port} URLs.

Key changes:

• Adds app_base_url fixture (module scope) that returns either http://localhost:{app_port} or an externally provided URL
• Adds build_app_url() helper function for proper URL composition with path, query, and fragment parameters
• Updates 29 test files to replace hardcoded localhost URLs with the new standardized approach
• Updates documentation in .cursor/rules/e2e_playwright.mdc, .github/instructions/e2e_playwright.instructions.md, and e2e_playwright/AGENTS.md

Code Quality

The implementation is well-structured and follows good practices:

Strengths:

1. Well-designed build_app_url function (conftest.py lines 472-522): The function properly handles URL parsing with urllib.parse, preserves base URL query parameters, and correctly composes paths. The logic handles edge cases like:

   • Absolute vs relative paths
   • Base URLs with/without trailing slashes
   • Query parameter merging from both base URL and provided params
   • Fragment handling

2. Helpful utility functions: ensure_nonempty_path() (lines 525-532) and normalize_url_for_compare() (lines 535-541) are useful additions for consistent URL handling.

3. Clear WebSocket URL conversion (web_server_test.py lines 40-46): The app_ws_url() helper properly converts HTTP/HTTPS schemes to WS/WSS.

Minor observations:

1. The app_ws_url() function in web_server_test.py could potentially be moved to conftest.py if other test files need WebSocket URL construction in the future. However, this is not a blocking issue as it's currently only used in that file.

2. The PR consistently uses app_base_url: str parameter type annotation, which is good for type safety.

Test Coverage

This PR is a refactoring effort that updates existing tests rather than adding new functionality to test. The changes are appropriate because:

1. All test files that previously used hardcoded f"http://localhost:{app_port}/..." patterns have been updated to use build_app_url(app_base_url, ...)
2. The changes preserve the original test logic while enabling external app testing
3. Tests that only need browser context (like test_xsrf_cookie_format) appropriately don't require app_base_url

The documentation updates in all three files (.cursor/rules/e2e_playwright.mdc, .github/instructions/e2e_playwright.instructions.md, e2e_playwright/AGENTS.md) provide clear guidance on the new URL handling pattern:

• Never hardcode localhost URLs
• Use app_base_url for navigation
• Use build_app_url() for URL composition

Backwards Compatibility

No breaking changes. This PR only modifies test infrastructure and does not affect the Streamlit library's public API or behavior. All changes are internal to the E2E testing framework.

The app_port fixture is still available for tests that need the raw port number, ensuring backward compatibility with any existing test patterns that require it.

Security & Risk

No security concerns identified.

1. The URL handling properly uses urllib.parse for safe URL manipulation
2. The external app URL is only used when explicitly provided via CLI option or environment variable
3. The changes don't expose any new attack vectors or sensitive information

Low regression risk:

• The refactoring is mechanical (replacing string concatenation with function calls)
• The build_app_url() function has clear, deterministic behavior
• Tests can still run locally with the same behavior as before

Recommendations

The PR is well-implemented and ready for merge. The following are minor suggestions for future consideration (not blocking):

1. Consider adding unit tests for build_app_url(): While the function is exercised through E2E tests, explicit unit tests would document its expected behavior and catch regressions faster.

2. Consider moving app_ws_url() to conftest.py: If other test files need WebSocket URL construction in the future, having this helper in conftest.py would be more consistent.

3. Documentation completeness: The documentation could mention that app_port fixture is still available for cases where only the port number is needed.

Verdict

APPROVED: This PR is a well-executed refactoring that standardizes URL handling across E2E tests, enabling external app testing while maintaining backward compatibility. The code quality is high, documentation is updated, and there are no security concerns or breaking changes.

This is an automated AI review using opus-4.5-thinking. Please verify the feedback and use your judgment.

Summary

This PR standardizes Playwright e2e tests on app_base_url/build_app_url and documents the no-localhost guidance across e2e instructions, improving support for external app runs.

Code Quality

Two remaining localhost assumptions undermine the new URL-handling guidance and will break when running against external app URLs or base paths.

AUTH_SECRETS_TEMPLATE = """
[auth]
redirect_uri = "http://localhost:{app_port}/oauth2callback"
cookie_secret = "your_cookie_secret_here"
expose_tokens = ["id", "access"]

[auth.testprovider]
client_id = "test-client-id"
client_secret = "test-client-secret"
server_metadata_url = "http://localhost:{oidc_server_port}/.well-known/openid-configuration"
"""

@pytest.fixture(scope="module")
def prepare_secrets_file(app_port: int, oidc_server_port: int):
    """Fixture that inject the correct port to auth_secrets.toml file redirect_uri."""
    # Read in the file
    rendered_secrets = AUTH_SECRETS_TEMPLATE.format(
        app_port=app_port, oidc_server_port=oidc_server_port
    )

    # First test without file upload config
    with iframed_app.page.expect_request(lambda request: request.method == "PUT") as r:
        file_chooser.set_files(files=files[0])

    url = r.value.url
    headers = r.value.all_headers()
    response = r.value.response()
    assert response is not None
    assert response.status == 204  # Upload successful
    assert url.startswith("http://localhost")
    assert "_stcore/upload_file" in url
    assert "header1" not in headers

Test Coverage

Changes are confined to e2e tests and guidance; no new unit tests are expected. However, external/embedded test runs will still fail or mis-route in the two locations above until app_base_url is used consistently.

Backwards Compatibility

No runtime behavior changes; these are test-only updates.

Security & Risk

Low risk overall. The main regression risk is e2e runs against externally hosted apps (or base paths) still failing due to remaining hardcoded localhost URLs.

Recommendations

1. Update auth_test.py to build redirect_uri from app_base_url (and thread app_base_url into prepare_secrets_file) instead of http://localhost:{app_port}.
2. Replace the hostframe_app_test.py upload URL assertion with an app_base_url-derived check (e.g., build_app_url(app_base_url, path="/_stcore/upload_file/") prefix) so it works with external/base-path hosts.

Verdict

CHANGES_REQUESTED: A couple of remaining localhost hardcodes conflict with the PR’s new external-compatible URL handling and will break external app runs until updated.

This is an automated AI review using gpt-5.2-codex-high. Please verify the feedback and use your judgment.

I addressed the requested changes by AI in this latest push.

Merge activity

• Feb 12, 8:25 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Feb 12, 9:28 PM UTC: Graphite rebased this pull request as part of a merge.
• Feb 12, 9:48 PM UTC: @sfc-gh-bnisco merged this pull request with Graphite.