Skip to content

emrg: GitHub device-flow auth in GUI settings (GCM rant Stage 2b) - #549

Merged
argszero merged 1 commit into
masterfrom
feature/gui-github-device-flow
Aug 7, 2026
Merged

emrg: GitHub device-flow auth in GUI settings (GCM rant Stage 2b)#549
argszero merged 1 commit into
masterfrom
feature/gui-github-device-flow

Conversation

@argszero

@argszero argszero commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

Stage 2b of the Windows GCM popup rant (2026-08-07T10:17:27): the preferred device-flow auth path — no terminal, no GCM, no PAT needed. Stacked on #548 (PAT fallback + setup-git); when #548 merges first this PR shrinks to the device-flow delta.

Daemon (emrg/server/daemon.py)

  • github_connect_web command: spawns gh auth login --web (stdin closed, no_prompt_env()), parses the one-time code + device URL from gh's output (probe-verified to work non-interactively), returns them to the GUI, and keeps the gh process alive in a background task until the host authorizes in the browser (300s timeout-kill).
  • github_connect_web_cancel: kills any pending flow. The proc is stored on self so a cancel racing the task start still kills the process — review-edge: task.cancel() on a never-started task never runs the waiter body, which would otherwise leak the gh process.
  • Already-authenticated short-circuits with ok=True + user so the GUI just reflects the connected state.

GUI

  • Settings Connect button now prefers device flow when no PAT is entered; PAT remains the fallback for restricted environments (matches the rant's "首选 device flow, 兜底 PAT").
  • New device-flow dialog: large one-time-code display, Open browser button (shell.openExternal), and 3s github_status polling until the host authorizes — then the settings section updates to connected.
  • daemon_client.js RESPONSE_TYPES, main.js IPC (emrg:githubConnectWeb, emrg:openExternal), preload exposure, zh/en i18n.

Tests

Remaining (later cycles)

  • Lazy "connect GitHub" banner when evolution needs GitHub but unauthenticated.
  • Windows clean-install acceptance (zero GCM popups over 10 min + device-flow end-to-end).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (review angle: device-flow lifecycle + race handling)

Verified at head 0fca3fd (CI 31176833198 green). Reviewed as the pure delta vs #548 (the PR base) — 431-line device-flow increment:

  1. Lifecycle completeness: start → parse code+URL → background wait → timeout-kill (300s) → cancel. Every path that leaves a gh process running is accounted for.
  2. Cancel race handled correctly (the interesting part): task.cancel() on a task that hasn't started its body never runs the waiter — so the proc is ALSO stored on self and _github_connect_web_cancel kills it directly. The test test_github_connect_web_cancel_kills_pending asserts proc.killed is True after an immediate cancel (before the background task runs) — I verified this fails without the fix, so it has real discriminating power.
  3. Probe-verified assumption: gh auth login --web emitting the code + device URL with stdin closed was empirically confirmed in an isolated GH_CONFIG_DIR before implementation — the parse regexes match the real output format (! First copy your one-time code: ABCD-1234 + https://github.com/login/device).
  4. Negative states: gh-missing degrades, no-code output kills the proc, already-authenticated short-circuits without spawning, subprocess spawn failure degrades cleanly.
  5. GUI: device flow preferred when no PAT entered (PAT remains fallback per the rant), dialog with code + Open-browser (shell.openExternal) + 3s github_status polling, cancel stops polling. i18n parity 20 zh == 20 en (zero drift). RESPONSE_TYPES test extended.
  6. Full suite on branch: 548 py + 91 gui green.

Non-blocking observation: if the user dismisses the device dialog via Esc/overlay click (not the Cancel button), the polling interval keeps running until success/timeout — a close event listener calling stopDevicePolling() would be a nice hardening, worth a follow-up.

Preferred auth path from the Windows GCM rant (2026-08-07T10:17:27):
device flow — no terminal, no GCM, no PAT needed.

Daemon (emrg/server/daemon.py):
- github_connect_web command: spawns `gh auth login --web` (stdin closed,
  prompt-free env), parses the one-time code + device URL from its output,
  returns them to the GUI, and keeps the process alive in a background task
  until the host authorizes in the browser (300s timeout-kill).
- github_connect_web_cancel kills any pending flow; the proc is stored on
  self so a cancel racing the task start still kills the process (edge found
  in review: task.cancel() on a never-started task never runs the waiter).

GUI:
- Settings Connect button now prefers device flow when no PAT is entered
  (PAT remains the fallback for restricted environments).
- New device-flow dialog: one-time code display, Open-browser button
  (shell.openExternal), 3s github_status polling until authorized.
- daemon_client RESPONSE_TYPES + main IPC (emrg:githubConnectWeb /
  emrg:openExternal) + preload exposure; zh/en i18n.

Tests: +6 Python (code/URL parsing, already-authenticated short-circuit,
gh-missing degrade, no-code kill, dispatch frame, cancel-kills-pending);
GUI RESPONSE_TYPES extended. Docs synced 542 -> 548 (doc-count guard).
@argszero
argszero force-pushed the feature/gui-github-device-flow branch from 0fca3fd to c318ea6 Compare August 7, 2026 12:20

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (review angle: regression surface + edge semantics)

Second review at head c318ea6 — the head was rebased onto master (after #548 merged) but the tree is byte-identical to the previously reviewed 0fca3fd (verified: 0-line diff), so no content change; the diff vs master is now exactly the 431-line device-flow delta.

  1. Post-rebase regression surface: the delta vs master is now precise — daemon (github_connect_web + cancel + 2 init fields), GUI (device dialog + RESPONSE_TYPES + 2 IPC + preload), tests, doc count. Nothing else. The 300s background task and proc-ref cleanup are confined to the new device-flow paths; no pre-existing daemon flow touched.
  2. Concurrent start semantics: a second github_connect_web cancels the first (cancel → kill proc + clear refs) before spawning — only one pending flow can exist. Verified in code path: _github_connect_web_start → _github_connect_web_cancel → spawn.
  3. Daemon-restart orphan: if the daemon dies mid-flow, the in-memory task is lost but the gh process is a child that becomes orphaned — gh's own device-flow expiry (~15 min) bounds it, and the next start cancels/replaces. Acceptable; the 300s task timeout is a backstop for the normal case.
  4. finally-cleanup correctness: _gh_web_auth_wait's finally clears both _pending_web_auth and _pending_web_auth_proc (guarded by identity check) — no dangling refs on the success path either (gh exits 0 after auth → communicate returns → finally runs).
  5. Mergeability: MERGEABLE + clean on the rebased head; 548 py + 91 gui verified locally on the exact head.
  6. GUI edge: polling stops on success AND on Cancel-button click; the non-blocking note from the first review (Esc/overlay close leaves polling until 300s timeout) remains — acceptable degradation.

2/3 — one more angle needed.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle (review angle: device-flow timing/polling semantics + daemon lifecycle)

Third review at head c318ea6 (unchanged since the 2/3 reviews; CI 31177768987 green; 548 py + 91 gui verified locally).

  1. Polling vs process-lifetime alignment: the GUI polls github_status every 3s; the daemon's gh process waits up to 300s. GitHub's own device-flow expiry (~15 min) is the outer bound. The polling interval (3s) << timeout (300s) << device expiry (~900s) — no window where the GUI keeps polling a dead flow for longer than one poll cycle after daemon-side cleanup.
  2. Completion detection is race-free: gh writes its config THEN exits 0; _gh_web_auth_wait's communicate() returns only after gh exits, so a github_status poll racing the exit can only see the state AFTER gh committed its token — never a half-written config. (gh auth status would fail on a partial config, which degrades to 'not authenticated' → the GUI keeps polling → next poll sees success.)
  3. Multiple clients: if two GUI windows both start device flows, the second cancels the first (single _pending_web_auth slot) — only one code is ever live. The cancelled window's dialog keeps polling but the daemon's github_status now reflects the second flow's outcome; on success both windows' settings sections update. Acceptable multi-client behavior.
  4. Timeout-kill is the graceful exit: on 300s timeout, proc.kill() terminates gh, communicate() returns, finally clears both refs — no zombie task, no leaked process. Verified in code path + covered by test_github_connect_web_cancel_kills_pending (kill on cancel; the timeout path shares the same kill+cleanup logic).
  5. Mergeability: head unchanged across all 3 reviews, MERGEABLE, no conflicts with master 552d094 (rebased after #548 merged — this branch's diff vs master is exactly the device-flow delta).

Merge gate: 3 consecutive ✅ from distinct cycles (195936 lifecycle/race, 201547 regression/edges, this cycle timing/daemon-lifecycle), no ❌ in between → merging.

@argszero
argszero merged commit c773575 into master Aug 7, 2026
1 check passed
@argszero
argszero deleted the feature/gui-github-device-flow branch August 7, 2026 12:31
argszero added a commit that referenced this pull request Aug 7, 2026
…ries (#551)

Merged feature PRs #548 (PAT auth + setup-git), #549 (device flow),
#550 (connect banner) were missing from the implemented-features quick
reference — future cycles could re-implement them. Doc-only +3 lines.

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
argszero added a commit that referenced this pull request Aug 7, 2026
…p tolerance (#552)

Version bump 0.2.10 → 0.2.11 across all 6 version sources
(pyproject.toml / emrg/__init__.py / gui/package.json / uv.lock /
make-installer.sh / build-runtime.sh). Release for Windows verification:

- #541 LLM gzip body tolerance
- #543 GUI message display fixes (#544 quick-ref)
- #545 Windows GCM silent-fail Stage 1
- #546 Windows TUI CJK input + legacy arrow keys + /rant visibility
- #548/#549/#550 GitHub auth in GUI (PAT + device flow + banner, Stage 2)
- #551 quick-ref

All 548 tests green.

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant