Skip to content

[FE fix] demo workspace banner issues#4475

Merged
bekossy merged 3 commits into
release/v0.100.5from
fe-fix/demo-workspace-banner-issues
May 28, 2026
Merged

[FE fix] demo workspace banner issues#4475
bekossy merged 3 commits into
release/v0.100.5from
fe-fix/demo-workspace-banner-issues

Conversation

@ardaerzin
Copy link
Copy Markdown
Contributor

Summary

...tba

fixes the issue reported here

Testing

QA follow-up

test the branch on staging / testing.

Checklist

  • I have included a video or screen recording for UI changes, or marked Demo as N/A
  • Relevant tests pass locally
  • Relevant linting and formatting pass locally
  • I have signed the CLA, or I will sign it when the bot prompts me

Contributor Resources

ardaerzin added 2 commits May 28, 2026 11:34
Two independent bugs surfaced in #bugs as the "demo workspace banner
issue":

1. URL pollution (Mahmoud's investigation). projectAtom derived
   workspaceId from selectedOrgAtom, which lags /projects/ by one
   /organizations/{id} fetch. During the gap, pickPreferredProject
   ran with workspaceId=null and hit the cross-org
   `projects.find(p => p.is_default_project)` fallback. For users
   with is_demo=true membership, that returned the Demo Workspace's
   default project. WorkspaceRedirect then router.replace'd to
   /p/<demo-project>, pinning the URL and rendering the banner in
   the wrong workspace.

   Fix: projectAtom falls back to selectedOrgIdAtom (URL-derived,
   resolves on first render) so workspaceId is always populated.
   projectMatchesWorkspace already accepts either workspace_id or
   organization_id, so the URL-derived org UUID filters correctly.

2. Banner-missing-on-reload race. Two parallel "session exists"
   atoms existed — @agenta/shared/state's sessionAtom (entity
   packages) and oss/state/session's sessionExistsAtom. The eager
   init in appState/atoms.ts set the shared one; the oss one
   waited for SessionListener's React effect. projectsQueryAtom
   gated on the oss atom AND on profileQueryAtom.data.id, which
   forced /projects/ to wait for /profile/ sequentially. On cold
   reload the demo banner couldn't render until ~2 RTTs +
   1 effect tick later.

   Fix: consolidate to a single source of truth — sessionExistsAtom
   is now a re-export of @agenta/shared/state's sessionAtom. The
   ~14 callers keep their imports unchanged. SessionListener and
   useSession drop their manual dual-write code. projectsQueryAtom
   drops the redundant profile.data.id gate (fetchAllProjects uses
   session cookies, not user.id). Cold-reload banner now appears
   after one network RTT, with the same speedup applying to every
   other query gated on sessionExistsAtom (access, observability,
   selectedOrg, profile).

Includes a vitest regression spec for pickPreferredProject behavior
(workspaceId=null returns cross-org demo; workspaceId scoped to own
org returns own project). web/oss has no vitest runner yet, so
*.test.ts is added to tsconfig exclude — the spec runs once vitest
is wired.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview, Comment May 28, 2026 11:39am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR consolidates session state to use the shared session atom across OSS, updates projects query gating to depend on session existence rather than profile data, fixes a workspaceId null fallback in project selection, and adds Vitest regression tests for pickPreferredProject.

Changes

Session Consolidation and Project Selection

Layer / File(s) Summary
Shared session atom re-export
web/oss/src/state/session/atoms.ts
sessionExistsAtom now directly re-exports the shared sessionAtom from @agenta/shared/state, removing the local wrapper and using a single source of truth.
Session listener and hook sync
web/oss/src/state/session/hooks.ts, web/oss/src/hooks/useSession.ts
SessionListener and useSession now sync only sessionExistsAtom (the shared atom), removing direct writes to the prior shared-session setter; logout and effect dependencies were adjusted accordingly.
Project query gating refactor
web/oss/src/state/project/selectors/project.ts
projectsQueryAtom now gates on sessionExistsAtom (plus workspace/route conditions) instead of profileQueryAtom.data.id, removing the profile-fetch dependency.
Project selection workspace fallback
web/oss/src/state/project/selectors/project.ts
projectAtom's workspaceId derivation now falls back to selectedOrgIdAtom when organization.default_workspace?.id is unavailable; pickPreferredProject is exported for tests.
Regression tests for project selection
web/oss/src/state/project/selectors/projectAtom.race.test.ts
New Vitest tests for pickPreferredProject cover null-workspace legacy fallback, own/demo org selection, and empty-list edge cases.
Documentation and config
web/oss/src/state/appState/atoms.ts, web/oss/tsconfig.json
Expanded comment clarifying eager atom initialization and updated tsconfig exclude to omit test files.

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description references a reported issue and indicates testing has been performed locally, though the summary section is incomplete ('...tba').
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title '[FE fix] demo workspace banner issues' directly summarizes the main changes—fixing demo workspace banner flakiness bugs by refactoring session state management and project selection logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fe-fix/demo-workspace-banner-issues

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.

@ardaerzin ardaerzin marked this pull request as ready for review May 28, 2026 11:19
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working Frontend labels May 28, 2026
Copy link
Copy Markdown

@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 info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 951adb22-12e5-4c22-95c7-2a97a1a7a154

📥 Commits

Reviewing files that changed from the base of the PR and between 0b9012d and 01eb7d3.

📒 Files selected for processing (7)
  • web/oss/src/hooks/useSession.ts
  • web/oss/src/state/appState/atoms.ts
  • web/oss/src/state/project/selectors/project.ts
  • web/oss/src/state/project/selectors/projectAtom.race.test.ts
  • web/oss/src/state/session/atoms.ts
  • web/oss/src/state/session/hooks.ts
  • web/oss/tsconfig.json

Comment thread web/oss/src/state/project/selectors/projectAtom.race.test.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

Railway Preview Environment

Status Destroyed (PR closed)

Updated at 2026-05-28T13:18:02.738Z

The own-org "Default" project had is_default_project: true in the
fixture, which made `projects.find(p => p.is_default_project)` (the
cross-org fallback inside pickPreferredProject when workspaceId is
null) return the OWN project first — not the demo one. That
contradicted the "with workspaceId=null, returns demo project"
assertion and meant the test wasn't locking the bug it claimed to.

Mirror Mahmoud's actual /projects/ response from the investigation:
own-org "Default" is_default_project=false, Demo Workspace "Default"
is_default_project=true. With that ordering, the find lands on the
demo project — exactly the path that pollutes the URL on cold reload.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@junaway junaway changed the title Fe fix/demo workspace banner issues [FE fix] demo workspace banner issues May 28, 2026
@junaway junaway changed the base branch from main to release/v0.100.5 May 28, 2026 12:04
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 28, 2026
@bekossy bekossy merged commit 2957c35 into release/v0.100.5 May 28, 2026
30 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Frontend lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants