Skip to content

Validate enforced sessions and document migration - #7364

Open
masenf wants to merge 4 commits into
codex/session-frontendfrom
codex/session-integration
Open

masenf wants to merge 4 commits into
codex/session-frontendfrom
codex/session-integration

Conversation

@masenf

@masenf masenf commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Exercise session ownership in real servers and browsers, migrate synthetic clients to server-issued credentials, and document deployment and rollout.

The backend-path app runs under enforcement across development/production and zero/one/two-level prefixes. It covers immediate hydration during held cookie exchange, shared/duplicate tabs, uploads, failed exchange recovery with retained or cancelled uploads, idle short-TTL renewal, backend restarts, and cross-site iframe persistence. EmbedPlugin coverage also runs under enforcement. Production restarts preserve their worker and compile-suppression settings.

A real Granian test verifies state events while cookie exchange is held and state continuity on reconnect. It uses rx.State and asserts it leaves no direct BaseState subclass behind for later integration apps. Benchmark reconnects reuse cookies and bound tokens, with issuance outside measured event latency.

The guide covers app-bound credentials, persistent/rotating signing keys, POSIX permissions and Windows ACLs, origin configuration, partitioned cookies, embedding, migration modes, SYSTEM/anonymous authority, idle renewal, retry/cancellation, and readiness during cookie replacement. Existing reverse-proxy/api_transformer facilities handle rate limiting.

Review order: #7361 → #7362 → #7363 → #7364. Rebased on main d02ba5409; #7360 is already merged. Companion: reflex-dev/reflex-enterprise#240.

Validation:

  • Full unit suite: 12,147 passed, 26 skipped; 80.15% coverage (72% required).
  • Final Chromium matrix: 70 passed across dev/prod/backend paths and embedding, including idle renewal, failed exchange recovery, cancellation, and six production/development restart configurations.
  • Isolated real Redis transport/context/upload suite: 390 passed.
  • Real Granian plus database import-order regressions: 15 passed.
  • All 13 frontend runtime/lifecycle wrappers pass, including cookie-replacement readiness and teardown regressions.
  • Ruff check/format and pyright reflex tests pass. Minimum-dependency checks pass for reflex, reflex-base, and reflex-components-core. Stub generation makes no tracked changes.
  • Full local pre-commit has six existing macOS-only Pyright errors in unchanged reflex-bench/reflex-workflow CPU-affinity code. All other hooks pass; CI pre-commit passes on Linux.
  • The advisory playground-0.8.23 job fails on that older release's missing greenlet dependency. Current-head playground and size checks pass. Large CI matrices are still running after the final pushes; cancelled duplicate runs also temporarily report failed gate checks.

All substantive feedback is addressed. Review-thread replies explain the intentional connection-lifetime authority policy and declined hypothetical/style-only requests. Final adversarial review found no additional code blockers.

Related to ENG-12920, ENG-12921, ENG-9088.

@masenf
masenf added this pull request to stack #7365 September 30, 2026 00:41
@masenf masenf changed the title Exercise enforced sessions across servers and browsers and document migration Validate enforced sessions and document migration Sep 30, 2026
@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with one non-blocking correction needed in the guide.

Findings

  1. P2 State update example fails ▶

Summary

Adds session-enforcement coverage in browsers and Granian, updates benchmark clients to use server-issued credentials, and documents migration and deployment.

  • Teams can follow one guide to roll out browser sessions.
  • Enforced sessions now run through real browser and server tests.
  • Benchmark clients now reconnect with server-issued session credentials.
  • The test harness matches cookie hosts and keeps production restart settings.

Reviews (8) · Last reviewed commit: "Document HTTP readiness during cookie re..." · Reviewed by Greptile

Comment thread tests/integration/tests_playwright/test_backend_path.py
Comment thread tests/integration/tests_playwright/test_backend_path.py Outdated
@masenf
masenf force-pushed the codex/session-integration branch from 80666f1 to 62db86e Compare October 5, 2026 21:24
@masenf masenf added the documentation Improvements or additions to documentation label Oct 6, 2026
@linear-code

linear-code Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

ENG-12921

ENG-9088

ENG-12920

@masenf
masenf force-pushed the codex/session-integration branch from 62db86e to c561071 Compare October 8, 2026 17:55
@masenf
masenf force-pushed the codex/session-integration branch from c561071 to db8a6ab Compare October 8, 2026 18:24
@masenf
masenf force-pushed the codex/session-integration branch 3 times, most recently from 5932c7d to 6b07618 Compare October 14, 2026 09:35
@masenf
masenf force-pushed the codex/session-integration branch from 6b07618 to 93bb449 Compare October 14, 2026 09:46
@masenf
masenf requested a review from a team as a code owner October 14, 2026 09:54

@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.

2 issues found across 16 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="news/+session-harness.bugfix.md">

<violation number="1" location="news/+session-harness.bugfix.md:1">
P3: This fragment reads as an instruction to configure a hostname, but AppHarness now sets the backend URL to `localhost` itself. Describe the change instead, e.g. that AppHarness now points the dev frontend at `localhost`, so browser cookies stay same-site in tests.</violation>
</file>

<file name="docs/api-routes/sessions.md">

<violation number="1" location="docs/api-routes/sessions.md:64">
P3: This describes the `__Host-` name as universal, but non-loopback HTTP development uses the bare cookie name because `_cookie_name()` disables the prefix when the cookie is insecure. Qualify this sentence to apply only to secure cookies so deployment and cookie-debugging guidance matches the implementation.</violation>
</file>

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

View guided diff | Turn on auto-fix | Re-trigger cubic

@@ -0,0 +1 @@
Use the same hostname for the development AppHarness frontend and backend so browser cookies remain same-site.

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.

P3: This fragment reads as an instruction to configure a hostname, but AppHarness now sets the backend URL to localhost itself. Describe the change instead, e.g. that AppHarness now points the dev frontend at localhost, so browser cookies stay same-site in tests.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At news/+session-harness.bugfix.md, line 1:

<comment>This fragment reads as an instruction to configure a hostname, but AppHarness now sets the backend URL to `localhost` itself. Describe the change instead, e.g. that AppHarness now points the dev frontend at `localhost`, so browser cookies stay same-site in tests.</comment>

<file context>
@@ -0,0 +1 @@
+Use the same hostname for the development AppHarness frontend and backend so browser cookies remain same-site.
</file context>
Suggested change
Use the same hostname for the development AppHarness frontend and backend so browser cookies remain same-site.
AppHarness now points the development frontend at `localhost` for the backend API, so browser cookies remain same-site during tests.

Comment on lines +64 to +65
Cookies use `Secure; HttpOnly; SameSite=None; Partitioned; Path=/`, no Domain,
and an app-specific `__Host-` name. Set `api_url` to the public backend URL,

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.

P3: This describes the __Host- name as universal, but non-loopback HTTP development uses the bare cookie name because _cookie_name() disables the prefix when the cookie is insecure. Qualify this sentence to apply only to secure cookies so deployment and cookie-debugging guidance matches the implementation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/api-routes/sessions.md, line 64:

<comment>This describes the `__Host-` name as universal, but non-loopback HTTP development uses the bare cookie name because `_cookie_name()` disables the prefix when the cookie is insecure. Qualify this sentence to apply only to secure cookies so deployment and cookie-debugging guidance matches the implementation.</comment>

<file context>
@@ -0,0 +1,140 @@
+
+## Origins, cookies, and embedding
+
+Cookies use `Secure; HttpOnly; SameSite=None; Partitioned; Path=/`, no Domain,
+and an app-specific `__Host-` name. Set `api_url` to the public backend URL,
+including when TLS terminates at a reverse proxy. Cookies are not scoped by
</file context>
Suggested change
Cookies use `Secure; HttpOnly; SameSite=None; Partitioned; Path=/`, no Domain,
and an app-specific `__Host-` name. Set `api_url` to the public backend URL,
Secure cookies use `Secure; HttpOnly; SameSite=None; Partitioned; Path=/`, no Domain,
and an app-specific `__Host-` name; non-loopback HTTP development uses an insecure
cookie without the `__Host-` prefix. Set `api_url` to the public backend URL,

from reflex_base.session import SessionToken

async with app.modify_state(
rx.BaseStateToken(client_token, MyState),

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.

P2 State update example fails

The server-side example passes positional arguments to BaseStateToken, but its inherited constructor requires keyword arguments. Copying it raises TypeError before app.modify_state runs. Use rx.BaseStateToken(ident=client_token, cls=MyState).

Suggested change
rx.BaseStateToken(client_token, MyState),
rx.BaseStateToken(ident=client_token, cls=MyState),

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant