Skip to content

Conversation

@elias-ba
Copy link
Contributor

@elias-ba elias-ba commented Oct 24, 2025

Description

This PR fixes AI chat session job scoping to prevent incorrect chat sessions from appearing when switching between jobs in the workflow editor.

Previously, when a user had a chat session open for Job A and switched to Job B, the chat session from Job A would persist or the wrong session would be shown. This PR implements strict job scoping with three layers of protection:

  1. URL parameter management: Clears the j-chat parameter when switching jobs (but preserves it on initial page load)
  2. Session ownership validation: Validates that loaded sessions belong to the currently selected job
  3. Graceful error handling: Falls back to showing the session list if validation fails

Closes #3745

Validation steps

  1. Test URL parameter clearing when switching jobs:

    • Create a workflow with two jobs (Job A and Job B)
    • Open Job A's AI assistant and create a chat session
    • Switch to Job B by clicking it on the canvas
    • Verify that Job B shows its own session list, not Job A's session
  2. Test session list filtering:

    • Create chat sessions for both Job A and Job B
    • Open Job A's AI assistant
    • Verify only Job A's sessions are shown in the list
    • Switch to Job B
    • Verify only Job B's sessions are shown
  3. Test URL tampering protection:

    • Create a chat session for Job A
    • Manually modify the URL to include Job B's ID with Job A's session ID (e.g., ?s=job_b_id&j-chat=job_a_session_id)
    • Verify the page doesn't crash and shows Job B's session list instead
  4. Test persistence on page reload:

    • Open Job A with a chat session
    • Refresh the page
    • Verify the chat session persists correctly
  5. Run the test suite:

    mix test test/lightning_web/live/ai_assistant_live_test.exs
    mix test test/lightning/ai_assistant/ai_assistant_test.exs
    mix test test/lightning_web/ai_assistant/job_code_test.exs

Additional notes for the reviewer

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@github-project-automation github-project-automation bot moved this to New Issues in v2 Oct 24, 2025
@elias-ba elias-ba changed the title Fix AI chat session job scoping fix: scope job chat sessions by step Oct 24, 2025
@elias-ba elias-ba force-pushed the 3745-fix-ai-chat-session-job-switching branch 12 times, most recently from 7cbb484 to 51ea011 Compare October 24, 2025 02:06
@elias-ba elias-ba force-pushed the 3745-fix-ai-chat-session-job-switching branch from 51ea011 to 06b76a2 Compare October 24, 2025 02:13
@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 87.87879% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.85%. Comparing base (94bbbb7) to head (353fa4f).

Files with missing lines Patch % Lines
lib/lightning/ai_assistant/ai_assistant.ex 82.35% 3 Missing ⚠️
lib/lightning_web/live/workflow_live/edit.ex 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3841      +/-   ##
==========================================
+ Coverage   88.84%   88.85%   +0.01%     
==========================================
  Files         422      422              
  Lines       19116    19136      +20     
==========================================
+ Hits        16984    17004      +20     
  Misses       2132     2132              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@midigofrank midigofrank left a comment

Choose a reason for hiding this comment

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

Nicely done

Refactor get_session implementation to use shared query functions and
follow idiomatic Ecto patterns established elsewhere in the codebase:

- Add session_query/2 helper for building queries with optional job scoping
- Add preload_session/1 helper to centralize preloading logic
- Change get_session/1 to return nil instead of {:error, :not_found} tuple
- Add get_session/2 for job-scoped retrieval (returns nil if not found)
- Add get_session!/2 for job-scoped retrieval (raises if not found)
- Refactor get_session!/1 to use shared helpers

This eliminates code duplication, uses single database queries with
efficient preloading, and follows the pattern established by functions
like get_project_user/2 and get_credential_body/2.

Updated callers to handle nil returns instead of error tuples.
Resolved CHANGELOG.md conflict by keeping both fixes:
- AI chat session persistence (#3745)
- Credential preservation during sandbox merge (#3782)
@stuartc
Copy link
Member

stuartc commented Nov 17, 2025

@elias-ba I think I understand the change, I think this is good to merge. While this change didn't jump out as an issue, the logic around switching the job chat and chat in general makes me nervous about what it's going to take to get this into the new collaborative editor.

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

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

Job chat AI assistant: Wrong chat opened when switching jobs

4 participants