Skip to content

feat: two-shot mp login --start/--finish/--resume for headless environments#158

Draft
jaredmixpanel wants to merge 8 commits into
mainfrom
feat/mp-login-two-shot
Draft

feat: two-shot mp login --start/--finish/--resume for headless environments#158
jaredmixpanel wants to merge 8 commits into
mainfrom
feat/mp-login-two-shot

Conversation

@jaredmixpanel
Copy link
Copy Markdown
Contributor

Summary

  • Two-shot mp login --start / --finish / --resume flow for headless environments (Claude Cowork, CI runners, devcontainers, browserless SSH) where the loopback OAuth callback can't reach the user's host browser. PKCE verifier persists in ~/.mp/oauth/inflight.json (mode 0o600, 10-min TTL) across the boundary; user opens authorize URL on host, copies the redirect URL from the address bar, pastes back.
  • Auto-pick algorithm in _resolve_project for the headless path: region filter → drop demos → drop unintegrated → primary-org-lowest-id, with fallback through unintegrated then demos. Legacy auto_pick=False path preserved unchanged for same-machine mp login.
  • LoginFinishPublishError surfaces the placeholder path on post-exchange publish failures so users (and slash commands) know to mp login --resume <PATH> instead of re-running --finish (which would fail at the IdP — codes are one-time-use).
  • httpx[socks] dep so SOCKS5h handshakes work first call. Default OAuthFlow timeout bumped to (30s read, 10s connect). Paste-reader event-queue race in flow.py fixed via threading.Event (errors propagate in ms instead of waiting 310s for callback timeout).
  • Stderr heartbeat in non-TTY status_spinner so users see activity during slow /me calls (67s+ on busy accounts).
  • Slash command (auth.md) gains a Cowork branch driving the flow via AskUserQuestion. quickstart-claude-cowork.md split into Path A (two-shot inside Cowork) and Path B (bridge from laptop).
  • New ExitCode.NEEDS_SELECTION = 6 for the cross-region case. JSON envelopes on stdout for the new paths so slash commands and scripted wrappers can drive the dance.

Test plan

  • 6418 unit tests pass (52 new across 5 files: test_inflight.py, test_resolve_project_autopick.py, test_login_two_shot.py, test_flow_fixes.py, plus regression patches to existing files)
  • just check passes (lint + format + typecheck + tests + docstring coverage 99%+ + build)
  • Manual smoke against live mixpanel.com/oauth/: --start emits expected JSON, inflight file at mode 0o600, --region eu produces eu.mixpanel.com URL, --finish without --start raises OAUTH_INFLIGHT_MISSING (exit 2), flag mutex rejected at exit 3
  • Codex review run twice — [P1] (file present in patch) resolved by initial commit; [P2] (placeholder discovery on post-exchange failure) addressed by LoginFinishPublishError + 3 new tests

🤖 Generated with Claude Code

jaredmixpanel and others added 2 commits May 12, 2026 16:56
…nments

Adds a non-TTY login path for Claude Cowork, CI, devcontainers, and
browserless SSH where the loopback OAuth callback can't reach the
user's host browser. PKCE verifier persists in ~/.mp/oauth/inflight.json
(mode 0o600, 10-min TTL) across the --start/--finish boundary; the user
opens the authorize URL on their host, copies the redirect URL from the
"site can't be reached" address bar, and pastes it back.

The new CLI paths emit JSON envelopes on stdout so slash commands and
scripted wrappers can drive the flow:

- state: ok on success, with project_pick metadata (method enum,
  primary-org survivor count, funnel counts) so the slash command can
  render the auto-pick result conversationally.
- state: error code: NEEDS_REGION_SWITCH (exit 6, new
  ExitCode.NEEDS_SELECTION) when /me returns zero region-compatible
  projects, with the cross-region project list in details so the
  user can retry with --region eu/in.

Auto-pick in _resolve_project (auto_pick=True only): region filter,
drop demos, drop unintegrated, pick from highest-survivor-count org
with lowest-id tiebreak, with fallbacks through unintegrated then
demos. The legacy auto_pick=False path is preserved unchanged for
same-machine `mp login`.

Other fixes shipped together:

- Bump default OAuthFlow httpx timeout to (30s read, 10s connect) so
  cold SOCKS5h handshakes inside Cowork's ALL_PROXY succeed first call.
- Fix paste-reader event-queue race in flow.py via threading.Event:
  errors propagate in ms instead of waiting 310s for callback timeout.
- Skip paste reader entirely when stdin is non-TTY (Cowork pipes).
- httpx[socks] dep so socksio is pulled in by default.
- Stderr heartbeat in non-TTY status_spinner so users see activity
  during 67s+ /me calls on busy accounts.
- Preserve placeholder dir when tokens.json exists so users can
  `mp login --resume <PATH>` after a publish failure.

Slash command (auth.md) gains a Cowork branch driving the flow via
AskUserQuestion. quickstart-claude-cowork.md split into Path A
(two-shot inside Cowork) and Path B (bridge from laptop).

49 new tests, all 6415 tests pass. Manual smoke against live
mixpanel.com/oauth/ verified.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When `mp login --finish` succeeds at token exchange but fails at
publish (bad --name, project not visible, name collision, /me parse
error, etc.), the OAuth code has been consumed and re-running --finish
won't work — the IdP one-time-uses authorization codes. The user must
`mp login --resume <PATH>` instead, but the path was not surfaced
anywhere structured.

Adds LoginFinishPublishError wrapping the underlying publish failure
with placeholder_dir + original code/message/details.
login_unified_finish and login_unified_resume catch publish failures,
clear the (now-useless) inflight, and re-raise the wrapped exception
when tokens.json still exists. The CLI's --finish/--resume handlers
catch it and emit a state: error code: LOGIN_FINISH_PUBLISH_FAILED
envelope including placeholder_dir, original cause, and a resume_hint
command for the slash command to render.

NeedsRegionSwitchError is special-cased separately: cross-region is
fundamental, not transient. The placeholder is rmtreed on this path
to avoid leaving an orphan .tmp-* dir that can never be resumed (the
user must run `mp login --start --region eu` instead).

3 new tests cover the wrap on invalid --name, the wrap on --project
not visible, and the cross-region cleanup.

Addresses Codex review [P2] on commit 4658d8f.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 13, 2026 00:27
@jaredmixpanel jaredmixpanel added claude-review Trigger Claude Code PR review codex-review Trigger Codex PR Review labels May 13, 2026
Comment thread src/mixpanel_headless/_internal/auth/inflight.py Dismissed
Comment thread src/mixpanel_headless/accounts.py Dismissed
Comment thread src/mixpanel_headless/cli/commands/login.py Fixed
Comment thread tests/unit/cli/test_login_two_shot.py Fixed
Comment thread src/mixpanel_headless/accounts.py Outdated
@github-actions

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a headless-friendly, two-shot OAuth login workflow to the mp CLI and supporting library layers, enabling authentication in environments where the loopback callback can’t reach the user’s browser (e.g., Claude Cowork/CI/devcontainers). This is integrated with a new auto-pick project selection algorithm, structured JSON envelopes for programmatic consumers, and several reliability fixes around OAuth timeouts and non-TTY progress signaling.

Changes:

  • Introduces mp login --start/--finish/--resume with persisted inflight PKCE state and placeholder-based recovery, emitting machine-parseable JSON envelopes.
  • Refactors project resolution to return a structured ProjectPickResult and adds headless auto-pick + cross-region handling via NeedsRegionSwitchError (exit code 6).
  • Improves OAuth robustness (httpx SOCKS support + timeout bump; paste-reader race fix) and CLI UX in non-TTY environments (stderr heartbeat).

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
uv.lock Locks httpx with socks extra and adds socksio dependency metadata.
pyproject.toml Switches dependency to httpx[socks]>=0.27 for SOCKS proxy support.
src/mixpanel_headless/_internal/auth/client_registration.py Adds DOMAIN_FOR_REGION mapping used by headless project region filtering.
src/mixpanel_headless/_internal/auth/flow.py Increases default httpx timeout and fixes paste-reader/callback race using threading.Event.
src/mixpanel_headless/_internal/auth/inflight.py New module for inflight session persistence + placeholder helpers (--start/--finish/--resume).
src/mixpanel_headless/accounts.py Implements two-shot orchestration APIs, placeholder publish tail, structured ProjectPickResult, and headless auto-pick resolution.
src/mixpanel_headless/exceptions.py Adds NeedsRegionSwitchError and LoginFinishPublishError to support structured headless flows.
src/mixpanel_headless/cli/utils.py Adds ExitCode.NEEDS_SELECTION=6, cross-region error mapping, and non-TTY status heartbeat behavior.
src/mixpanel_headless/cli/commands/login.py Adds CLI flags and JSON envelope emission for --start/--finish/--resume, plus argument-mutex validation.
tests/unit/test_inflight.py New unit tests for inflight/placeholder lifecycle and error handling.
tests/unit/test_resolve_project_autopick.py New unit tests covering the auto-pick cascade, fallbacks, and cross-region raise behavior.
tests/unit/test_flow_fixes.py Regression tests for httpx timeout bump and paste-reader race fix.
tests/unit/cli/test_login_two_shot.py New CLI tests validating two-shot flows, envelopes, exit codes, and publish-failure recovery.
tests/unit/cli/test_login_cli.py Updates legacy tests to match the new “preserve placeholder when tokens exist” contract.
tests/unit/test_auth_flow.py Adjusts test stdin fixture semantics (isatty) to align with paste-reader gating.
tests/unit/test_loc_budget.py Updates auth subsystem file/LOC caps for the added two-shot implementation.
mixpanel-plugin/docs/quickstart-claude-cowork.md Splits Cowork setup into two paths (two-shot login vs bridge file), with troubleshooting guidance.
mixpanel-plugin/commands/auth.md Documents same-machine vs headless routing and the JSON envelope-driven two-shot flow.

Comment thread src/mixpanel_headless/accounts.py
Comment thread tests/unit/test_loc_budget.py Outdated
@claude

This comment was marked as resolved.

@austinpray-mixpanel
Copy link
Copy Markdown
Member

You have a whole team reviewing this lolol

Real fixes:
- inflight: read_placeholder_meta raises on corrupt/non-dict (only returns
  None for absent meta) so an EU/IN placeholder with bad meta no longer
  defaults to "us" and gets cleaned up via NeedsRegionSwitchError, which
  destroyed recoverable tokens. load_inflight rejects schema_version
  newer than INFLIGHT_SCHEMA_VERSION with OAUTH_INFLIGHT_SCHEMA_TOO_NEW.
- accounts: --resume now requires the placeholder to live under
  accounts_root() (path-traversal guard). Bad meta region raises loud
  ConfigError instead of falling back to "us". Region default to "us"
  preserved for legacy placeholders without meta.json.
- accounts: rename _PublishResult -> public LoginFinishResult, export
  via __all__, update CLI annotation.
- accounts: PickMethod gains "sole_survivor_filtered". The filtered
  short-circuit in _auto_pick_from_filtered now uses it so the slash
  command can render "the only non-demo, integrated project" rather
  than misleading "your only active project". The line 2684 short-
  circuit (region_n == 1) keeps "sole_survivor".
- accounts: docstrings on _is_demo / _is_integrated.
- flow: OAuthFlow gets http_client / storage public properties +
  __enter__ / __exit__ / close. login_unified_start and
  login_unified_finish now use `with OAuthFlow(...)` so the default
  httpx.Client is closed after DCR. Drops the SLF001 noqa.
- flow: secrets.compare_digest for CSRF state comparison.
- login.py: _emit_json swaps default=str for an explicit encoder that
  accepts only datetime / Path. A future refactor that drops a token,
  SecretStr, or other surprise into the envelope now fails loudly.
- auth.md: render the new sole_survivor_filtered method.
- test_loc_budget: docstring -> repo-relative spec path.

Tests:
- read_placeholder_meta: corrupt + non-dict raise; missing still returns
  None.
- load_inflight: schema_version > current raises OAUTH_INFLIGHT_SCHEMA_TOO_NEW.
- _resolve_project: sole_survivor_filtered when demo gets filtered out.
- --resume: path outside accounts_root() rejected; corrupt meta does not
  delete the placeholder.
- OAuthFlow: http_client / storage properties, context-manager close,
  idempotent close.
- test_login_two_shot: EU URL hostname check via urlparse (CodeQL).

Skipped (with reasoning in the plan file):
- CodeQL clear-text-logging at inflight.py:234 / accounts.py:1913 /
  login.py:301 are false positives — only path + OSError logged, no
  secrets in envelopes. Recommend dismissing in the GitHub Security UI.
- "done_event makes first error win" is a misread; the wait pulls from
  result_q first, so the first SUCCESS wins.
- _parse_pasted_redirect double-`?` is theoretical only; real OAuth
  providers don't emit that shape.
- Splitting accounts.py (~3000 lines) is out of scope.

just check passes (lint + format + mypy --strict + 6428 tests, 91.88%
coverage, build).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jaredmixpanel
Copy link
Copy Markdown
Contributor Author

Thanks for the reviews. Pushed 1b6e6aa addressing 14 of the 17 distinct findings across all bot reviews. Verdicts in my plan file (summarized below); per-thread replies follow.

Real fixes landed in 1b6e6aa

  • --resume path-traversal guard (claude Add copilot-setup-steps.yml for Python environment configuration #4): placeholder_dir must now be under accounts_root(), otherwise ConfigError before any read.
  • --resume region fallback (github-actions blocking, claude Implement Storage Engine (Phase 003): DuckDB-based local storage #9): read_placeholder_meta now distinguishes "absent" from "corrupt". Only absent meta defaults to "us" (legacy placeholders predating this PR); corrupt meta raises OAUTH_PLACEHOLDER_META_CORRUPT before the NeedsRegionSwitchError cleanup path can destroy recoverable EU/IN tokens. New error code, same exception class.
  • OAuthFlow lifecycle + private-surface coupling (claude [WIP] Add .github/copilot-instructions.md for onboarding #2 + Add CI workflow for lint, type check, and test on push/PR to main #5): OAuthFlow now exposes http_client / storage as read-only properties and supports the context-manager protocol. login_unified_start / login_unified_finish use with OAuthFlow(...) so the default httpx.Client is closed after DCR. Drops both # noqa: SLF001 comments.
  • _PublishResultLoginFinishResult (github-actions important, claude Add .github/copilot-instructions.md for agent onboarding #3): renamed, exported via __all__, CLI annotation updated.
  • sole_survivor label (gitar + copilot inline at :2853): added sole_survivor_filtered to PickMethod. The post-filter short-circuit in _auto_pick_from_filtered uses it so the slash command can render "the only non-demo, integrated project" rather than a misleading "your only active project". The region_n == 1 short-circuit at :2684 keeps sole_survivor (semantically accurate — see thread reply).
  • Constant-time CSRF state (claude Rename to mixpanel_data and add README #1): secrets.compare_digest.
  • InflightSession.schema_version validation (claude 001 foundation layer #6): rejects > INFLIGHT_SCHEMA_VERSION with OAUTH_INFLIGHT_SCHEMA_TOO_NEW so a future v2 writer can't silently round-trip through a v1 reader.
  • _emit_json typed encoder (claude Add Claude Code GitHub Workflow #8): replaced default=str with an explicit encoder that accepts only datetime / Path. A future refactor that drops a SecretStr or other non-serializable into an envelope now raises TypeError instead of silently stringifying.
  • _is_demo / _is_integrated docstrings (github-actions nit).
  • test_loc_budget.py docstring (copilot inline): repo-relative path under specs/043-frictionless-auth/.
  • CodeQL eu.mixpanel.com substring (CodeQL inline): test now compares urlparse(...).hostname.

Skipped, with reasoning

  • CodeQL clear-text logging at inflight.py:234, accounts.py:1913, login.py:301 are false positives. The two logger.warning calls log only a path + an OSError (e.g. PermissionError); no token / code / secret is in the log line. _emit_json does not log secrets either — the §9 envelopes contain only public metadata (account name, project id, OAuth authorize_url, etc.). The OAuth code is consumed by flow.exchange_code immediately after parsing the pasted URL and never lands in any envelope or log. Recommend dismissing in the GitHub Security UI as "False positive" rather than littering the code with suppression comments. (The typed encoder change above does make the no-secrets-in-envelopes property explicit at the boundary, which should also satisfy CodeQL on the next scan.)
  • github-actions blocking [WIP] Add .github/copilot-instructions.md for onboarding #2 (done_event regresses paste fallback) is a misread of the new race logic. Both completers put_nowait to either result_q or error_q before setting done_event. The wait pulls result_q.get_nowait() first (line 471), and only consults error_q if no result arrived. So the first success wins, not the first error — if the callback server fails fast, the paste reader can still succeed and its result lands in result_q. The previous version (queue.get(timeout=310.0) on the result queue only) was actually worse: it hung for 5 minutes on a fast paste failure. The existing comments at lines 366-373 / 459-479 already document this; no code change needed.
  • claude Implement DiscoveryService for schema introspection (Phase 004) #10 (_parse_pasted_redirect URL split): line.split("?", 1)[1] extracts everything after the first ?. A URL with two ? keeps the second segment in the query string; parse_qs ignores the malformed part. Real OAuth providers don't emit this shape — theoretical only.
  • claude feat: implement MixpanelAPIClient (Phase 002) #7 (split accounts.py): agreed it's worth doing — _resolve_project + _auto_pick_from_filtered + _pick_from_primary_org belong in _internal/auth/project_picker.py — but that's a separate PR. Tracking as follow-up.

Verification

  • just check passes: ruff lint + format, mypy --strict, 6428 tests pass / 1 skipped, 91.88% coverage, uv build clean.
  • New tests:
    • test_read_placeholder_meta_corrupt_raises + _non_dict_raises
    • test_schema_too_new_raises_inflight_schema_too_new
    • test_sole_survivor_filtered_when_demos_filtered_out
    • test_resume_path_outside_accounts_root_raises
    • test_resume_corrupt_meta_does_not_delete_placeholder
    • TestLifecycle (4 tests for OAuthFlow properties + close + context manager)

Comment thread src/mixpanel_headless/cli/commands/login.py Dismissed
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 13, 2026

Thanks for the clear summary. The approach looks well-considered — sole_survivor_filtered accurately distinguishes the "only non-demo, integrated project" case from the true single-project sole_survivor case, and anchoring the clearer copy in the auth.md render path means users will see the right explanation. Keeping the region_n == 1 path as sole_survivor is correct since no filtering is involved there. The added test also directly validate

jaredmixpanel and others added 4 commits May 13, 2026 12:18
Picks up #156 (dependabot config + uv exclude-newer cooldown) and #157
(PyPI Trusted Publishers + drop devcontainer).

uv.lock conflict: resolved by taking origin/main's resolution and
re-running uv lock against the merged pyproject.toml. Result keeps both
sides' intent — httpx[socks] from this branch (Cowork SOCKS proxy
support) plus the hypothesis[cli]>=6.151.13 bump from main. socksio
v1.0.0 added as the transitive dep behind the [socks] extra.

just check passes: 6428 tests, lint, format, mypy --strict, build.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Patch bump for the two-shot mp login work and review-feedback fixes
landing in PR #158. Library version (`__version__` in
`src/mixpanel_headless/__init__.py`, sourced by `tool.hatch.version`)
and plugin version (`mixpanel-plugin/.claude-plugin/plugin.json`)
both 0.1.0 -> 0.1.1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jaredmixpanel jaredmixpanel marked this pull request as draft May 15, 2026 00:24
uv.lock conflict on `[options].exclude-newer` — took main's newer
2026-05-07 timestamp and re-ran `uv lock`, which re-added socksio v1.0.0
behind httpx[socks] cleanly. No package version drift.

Other files merged automatically: .github/workflows/codex-review.yml,
src/mixpanel_headless/_internal/io_utils.py, tests/unit/test_io_utils.py.

just check passes: lint, format, mypy --strict, 6428 tests, 91.88%
coverage, build (mixpanel_headless-0.1.1.tar.gz + .whl).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 15, 2026

Code Review ✅ Approved 1 resolved / 1 findings

Implements two-shot mp login flow for headless environments using PKCE and inflight state persistence. Addresses the misleading sole_survivor labeling, and no issues remain.

✅ 1 resolved
Edge Case: sole_survivor label misleading when multiple projects filtered to one

📄 src/mixpanel_headless/accounts.py:2853
In _auto_pick_from_filtered, when filtered_n == 1 but the user has multiple region-compatible projects (all others were excluded by the demo/integration filter), the method is set to "sole_survivor". The sole-survivor short-circuit at line 2678 already handles the true single-project case (region_n == 1), so _auto_pick_from_filtered is only reached when region_n > 1. This means the slash command (auth.md) will render "your only active project" when the user actually has multiple projects — they're just demo/unintegrated. A more precise label here would help users understand why their other projects weren't considered.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Labels

claude-review Trigger Claude Code PR review codex-review Trigger Codex PR Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants