feat: connect PostHog projects through browser OAuth - #116
Conversation
f8d10d5 to
5b1ca20
Compare
There was a problem hiding this comment.
Changes requested: PostHog's per-project flow is blocked for additional projects, and the new tests do not exercise the command path.
Warning
Changes requested · head 5b1ca20 · 2 findings: 1 major, 1 minor
| Severity | Finding | Where |
|---|---|---|
| major | F1 Already-connected shortcut prevents adding another PostHog project | src/commands/integration/connect.ts:1138 |
| minor | F2 Test the actual PostHog command dispatch and browser handoff | test/integration-connect-posthog.test.ts:18 |
Full review
F1 (major, high confidence): PostHog connects one project at a time, but once any PostHog integration exists, integration connect --type posthog returns “already connected” without opening the project-selection flow. A user trying to add a second project must discover and pass --reconnect. Do not apply the singleton shortcut to PostHog; let the browser flow offer another project.
F2 (minor, high confidence): The new URL test calls cliConnectUrl directly, so it passes even if integration connect --type posthog stops dispatching to the browser flow. Add a command-level test that covers the PostHog handoff and the existing-project case.
5b1ca20 to
45ed82a
Compare
There was a problem hiding this comment.
LGTM: PostHog always enters the browser flow, including when another project is already connected, and the command-level test covers both handoffs.
Note
Approved · head 45ed82a · no findings
Full review
No findings at 45ed82a. The command-level test covers browser handoff with zero or one existing PostHog project, and the code no longer skips consent when one is connected. Adding a second project through live consent remains unverified.
What changed
/cli/connect?flow=posthogbrowser flow.Verification
npm run test: 536 passed.npm run typecheck,npm run lint,npm run build, andgit diff --checkpassed.Delivery
This is U5 of the PostHog integration plan. It requires current-head review, a person merge, then a separate CLI package release. Nominal 3.44.0 does not publish the CLI.