Revert "fix(cloud): stop bouncing working users to /cloud/survey mid-session"#12344
Merged
dante01yoon merged 1 commit intoMay 19, 2026
Merged
Conversation
…session …" This reverts commit cf267ac.
dosubot
Bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughThis PR inverts the failure semantics of ChangesSurvey completion failure handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 1626 passed, 0 failed · 1 flaky📊 Browser Reports
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/platform/cloud/onboarding/auth.ts (1)
99-99: ⚡ Quick winRemove newly added inline narrative comments.
These comments restate nearby logic and add maintenance noise; prefer self-describing control flow instead.
As per coding guidelines, "Avoid new usage of code comments; do not add or retain redundant comments".
Also applies to: 112-112, 115-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/cloud/onboarding/auth.ts` at line 99, Remove the redundant inline narrative comments (e.g. "// Not an error case - survey not completed is a valid state" and the similar comments at the other noted locations) so the control flow is self-describing; delete those comment lines around the onboarding/auth logic (the newly added inline comments at the three spots) and ensure no other redundant explanatory comments remain — rely on clear variable/branch naming instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/cloud/onboarding/auth.ts`:
- Around line 98-109: The current survey status check treats any non-OK HTTP
response or exception as "survey incomplete" by returning false; change this to
only return false for explicit API signals of incompletion and return true for
unexpected failures. Specifically, in the block that calls the onboarding
settings endpoint (references: ONBOARDING_SURVEY_KEY, response, and
Sentry.addBreadcrumb) update the non-OK branch to log the event but return true
(not false) unless the response body explicitly indicates incomplete; likewise
adjust the catch/exception branch (the later block around the second Sentry
breadcrumb) to return true on unexpected errors, and only return false when the
API payload explicitly marks the survey as incomplete. Ensure both places
consistently treat transient/network errors as pass-through (true) instead of
failing closed.
---
Nitpick comments:
In `@src/platform/cloud/onboarding/auth.ts`:
- Line 99: Remove the redundant inline narrative comments (e.g. "// Not an error
case - survey not completed is a valid state" and the similar comments at the
other noted locations) so the control flow is self-describing; delete those
comment lines around the onboarding/auth logic (the newly added inline comments
at the three spots) and ensure no other redundant explanatory comments remain —
rely on clear variable/branch naming instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6854f3ac-6987-4f7c-afc2-10348e808729
📒 Files selected for processing (2)
src/platform/cloud/onboarding/auth.test.tssrc/platform/cloud/onboarding/auth.ts
💤 Files with no reviewable changes (1)
- src/platform/cloud/onboarding/auth.test.ts
Comment on lines
98
to
+109
| if (!response.ok) { | ||
| // Ambiguous response (404/5xx/etc). Treat as completed to avoid | ||
| // bouncing working customers to /cloud/survey on transient hiccups. | ||
| // Real "not completed" only comes from a 200 with empty value. | ||
| // Not an error case - survey not completed is a valid state | ||
| Sentry.addBreadcrumb({ | ||
| category: 'auth', | ||
| message: 'Survey status check returned non-ok response', | ||
| level: 'warning', | ||
| level: 'info', | ||
| data: { | ||
| status: response.status, | ||
| endpoint: `/settings/${ONBOARDING_SURVEY_KEY}` | ||
| } | ||
| }) | ||
| return true | ||
| return false |
Contributor
There was a problem hiding this comment.
Fail-closed return values reintroduce false-positive survey gating.
Returning false on every non-OK response and exception treats transient failures as "survey not completed". That can bounce already-onboarded users back into survey flow when the status check is temporarily unavailable. Keep false only for explicit "incomplete" signals from the API; return true for unexpected failures.
Suggested adjustment
if (!response.ok) {
+ if (response.status === 404) {
+ return false
+ }
Sentry.addBreadcrumb({
category: 'auth',
message: 'Survey status check returned non-ok response',
level: 'info',
data: {
status: response.status,
endpoint: `/settings/${ONBOARDING_SURVEY_KEY}`
}
})
- return false
+ return true
}
@@
} catch (error) {
@@
- return false
+ return true
}Also applies to: 114-127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/platform/cloud/onboarding/auth.ts` around lines 98 - 109, The current
survey status check treats any non-OK HTTP response or exception as "survey
incomplete" by returning false; change this to only return false for explicit
API signals of incompletion and return true for unexpected failures.
Specifically, in the block that calls the onboarding settings endpoint
(references: ONBOARDING_SURVEY_KEY, response, and Sentry.addBreadcrumb) update
the non-OK branch to log the event but return true (not false) unless the
response body explicitly indicates incomplete; likewise adjust the
catch/exception branch (the later block around the second Sentry breadcrumb) to
return true on unexpected errors, and only return false when the API payload
explicitly marks the survey as incomplete. Ensure both places consistently treat
transient/network errors as pass-through (true) instead of failing closed.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #12344 +/- ##
===========================================
- Coverage 74.16% 59.79% -14.38%
===========================================
Files 1521 1411 -110
Lines 87335 72130 -15205
Branches 24400 20016 -4384
===========================================
- Hits 64774 43127 -21647
- Misses 21736 28530 +6794
+ Partials 825 473 -352
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1012 files with indirect coverage changes 🚀 New features to boost your workflow:
|
dante01yoon
approved these changes
May 19, 2026
dante01yoon
added
needs-backport
Member
|
@deepme987 Successfully backported to #12346 |
github-actions
Bot
removed
the
needs-backport
dante01yoon
pushed a commit
that referenced
this pull request
May 20, 2026
… to /cloud/survey mid-session" (#12346) Backport of #12344 to `cloud/1.44` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12346-backport-cloud-1-44-Revert-fix-cloud-stop-bouncing-working-users-to-cloud-survey-m-3656d73d365081a2bd91ce5002af5fdc) by [Unito](https://www.unito.io) Co-authored-by: Deep Mehta <42841935+deepme987@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #12301
┆Issue is synchronized with this Notion page by Unito