fix(agent): add cloud sync sign-in disclosure#976
Conversation
✅ Tests passed — 1223/1227
|
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge; all changes are additive UI disclosure text with no auth logic modifications. The only concern is that CloudSyncDisclosure destructures cloudSyncSignInLinks by position, meaning a future reorder of that array would silently swap which URL is shown for Terms of Service vs Privacy Policy — a legal/compliance risk if unnoticed. The current array order is correct and the tests validate it, so this is a maintainability issue rather than a present defect. packages/browseros-agent/apps/agent/components/auth/CloudSyncDisclosure.tsx — the positional destructuring creates a subtle dependency on array order in productUrls.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant LoginPage / StepTwo
participant CloudSyncDisclosure
participant productUrls.ts
productUrls.ts-->>CloudSyncDisclosure: cloudSyncSignInLinks (ToS, Privacy, Cloud Sync)
User->>LoginPage / StepTwo: Opens sign-in surface
LoginPage / StepTwo->>CloudSyncDisclosure: render()
CloudSyncDisclosure-->>User: "By signing in, you agree to [ToS] and acknowledge [Privacy]. [Learn more about cloud sync]."
User->>LoginPage / StepTwo: Clicks "Continue with Google" or "Send Magic Link"
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/browseros-agent/apps/agent/components/auth/CloudSyncDisclosure.tsx:8-23
Positional destructuring silently mismaps legal links if `cloudSyncSignInLinks` is ever reordered. The prose is hard-coded ("you agree to the Terms of Service… acknowledge the Privacy Policy"), so a reorder in `productUrls.ts` would point "Terms of Service" text at the Privacy Policy URL with no compile-time error. Referencing the individual URL constants directly removes this coupling entirely.
```suggestion
export function CloudSyncDisclosure({ className }: CloudSyncDisclosureProps) {
return (
<p
className={cn(
'text-center text-muted-foreground text-xs leading-relaxed',
className,
)}
>
By signing in, you agree to the{' '}
<DisclosureLink link={cloudSyncSignInLinks[0]} /> and acknowledge the{' '}
<DisclosureLink link={cloudSyncSignInLinks[1]} />.{' '}
<DisclosureLink link={cloudSyncSignInLinks[2]} />.
</p>
)
}
```
Reviews (1): Last reviewed commit: "fix: add cloud sync sign-in disclosure (..." | Re-trigger Greptile |
50bcd37 to
03b61b0
Compare
Summary
Test plan
Fixes #419