Skip to content

fix: preserve sessions across trigger impersonation - #90

Open
z125316840-code wants to merge 1 commit into
frappe:developfrom
z125316840-code:codex/fix-flow-trigger-session
Open

fix: preserve sessions across trigger impersonation#90
z125316840-code wants to merge 1 commit into
frappe:developfrom
z125316840-code:codex/fix-flow-trigger-session

Conversation

@z125316840-code

Copy link
Copy Markdown

What changed

  • replace the trigger's manual frappe.set_user(...) / username-only restoration with a scoped _as_user context manager
  • restore the original session user, SID, session data, request form data, and user-dependent caches after trigger condition evaluation and execution
  • add a regression assertion that a DocType Event trigger preserves the browser SID and request form_dict

Why

DocType Event triggers evaluate their condition synchronously in the request that saves or submits a document. frappe.set_user() resets more than session.user: it also replaces session.sid, session.data, form_dict, and permission/template caches. The previous finally: frappe.set_user(original_user) restored the username but assigned the username as the SID, so the next browser request was treated as an expired session.

Impact

Users who submit a document that dispatches a Flow trigger keep their authenticated browser session. The trigger still runs as its configured run_as user, falling back to the trigger owner, and the original request identity and context are restored even when trigger evaluation or execution raises.

Validation

  • ruff check flow/triggers/triggers.py flow/tests/test_ai_triggers.py
  • ruff format --check flow/triggers/triggers.py flow/tests/test_ai_triggers.py
  • git diff --check
  • regression test added for preserving the original SID and request form data

A full Frappe integration run is left to the repository CI because this checkout does not contain a local Bench test site.

@z125316840-code
z125316840-code marked this pull request as ready for review July 30, 2026 05:53
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

The PR appears safe to merge.

The scoped impersonation restores session identity and request-local state on both normal and exceptional exits, and no concrete changed-code failure was identified.

Reviews (1): Last reviewed commit: "fix: preserve sessions across trigger im..." | Re-trigger Greptile

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