Skip to content

Webhook relay multi-user hardening: GitHub App device-flow auth + repo-permission gate#684

Merged
arul28 merged 4 commits into
mainfrom
ade/blocker-and-then-release-6b23f7fe
Jul 2, 2026
Merged

Webhook relay multi-user hardening: GitHub App device-flow auth + repo-permission gate#684
arul28 merged 4 commits into
mainfrom
ade/blocker-and-then-release-6b23f7fe

Conversation

@arul28

@arul28 arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Worker authz: /github/repos/:owner/:repo/{events,status} now require a Bearer GitHub token whose user has push/maintain/admin on the repo (GET /repos permissions; fallback /user + collaborator permission). Read-only/public tokens get 403 with no events/status leakage. Legacy self-hosted /projects/:projectId routes and ade_proj_ tokens unchanged.
  • Client auth pivot: hosted relay reads authenticate with an expiring GitHub App user token from GitHub device flow — never the user's PAT/OAuth/gh token. New shared githubAppUserAuthService factory (desktop + ade-cli) owns the github.appUserToken.v1 store, single-flight refresh with a clear-auth epoch fence, device-session lifecycle, and deduped audit logs.
  • UX: GitHub App panel gains Authorize-ADE device flow (copyable code chip, instruction/link, waiting state, expired-code auto-renew capped at 3, neutral pre-auth status). New typed ade github app-auth login|status|clear for headless/brain setups (CTO-gated; token values never printed).
  • IPC/actions: github.getAppUserAuthStatus/startAppUserDeviceAuth/pollAppUserDeviceAuth/clearAppUserAuth wired through preload + daemon action registry (device-auth trio CTO-only); getAppUserTokenForRelay deliberately not RPC-exposed.

Verification

  • Live E2E against GitHub: device flow enabled on the ADE app; a real app-user token reports the caller's true repo role even with a read-only app (push:true on own repo, push:false on foreign public repo); collaborator-permission fallback works; authorization_pending = HTTP 200; app permissions confirmed all-read.
  • Quality dual-review (correctness/security + maintainability): 10 findings, all fixed (single-flight refresh, auto-renew cap, session pruning, sanitized credential logs, clear-auth epoch fence, dedup of the ~110-line desktop/ade-cli token block into the shared factory).
  • Tests: relay 24, desktop github/ingress 77, ade-cli headless 24 + cli 212, preload 80 — all green under Node 22; desktop + ade-cli + relay typechecks green.

Post-merge note

The Cloudflare Worker must be redeployed (wrangler deploy in apps/webhook-relay) for the new 403 gate to take effect on the hosted relay.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added GitHub App device-flow sign-in for the desktop app and CLI, including login, status, and clear subcommands.
    • Added an “Authorize ADE” device authorization experience in the GitHub panel with polling and verification code copy.
    • Expanded hosted relay integration to use dedicated GitHub App user authorization and token refresh, and exposed new app-user auth controls across the app UI.
  • Bug Fixes

    • Tightened repository and relay permission checks to ensure access is granted only with the required permissions.
    • Prevented hosted relay requests from using the wrong token and improved hosted authorization error messaging.

Greptile Summary

This PR adds GitHub App device-flow authentication to the ADE desktop app and CLI, and tightens the webhook relay worker so that /github/repos/:owner/:repo/{events,status} routes now require a Bearer token with push/maintain/admin access rather than any valid repo-read token. The relay gains a two-tier permission check (permissions hash from GET /repos → collaborator-permission fallback), and the desktop/CLI both adopt a new shared createGitHubAppUserAuthService factory that handles device-flow sessions, single-flight token refresh, and epoch-fenced auth-clear.

  • Relay worker: assertGitHubRepoAuthorized now calls repoPermissionsAllowWebhookRead on the permissions hash, falling back to a three-API-call collaborator-permission check when the hash is absent; read-only tokens receive a 403 at both the events and status endpoints.
  • Auth service: githubAppUserAuthService encapsulates device-flow session state, single-flight refresh with an epoch fence to prevent stale overwrites, and a null-safe refresh-token expiry check (addressed from prior review round).
  • CLI / UX: New ade github app-auth login|status|clear commands and a device-flow panel in the GitHub settings screen complete the headless and interactive authorization surfaces.

Confidence Score: 4/5

Safe to merge for the relay worker and auth service; the preload has a one-line arg-wrapping mistake that breaks device-auth polling when a project runtime is attached to the renderer.

The relay permission gate, auth service, CLI flow, IPC handlers, and UI panel are all correct. One issue in preload.ts passes { args: { sessionId } } instead of { sessionId } to callProjectRuntimeActionOr for pollAppUserDeviceAuth. In runtimes where the runtime path is preferred over IPC, device-flow polling would always return "session not found", silently stalling the authorization UI for any user going through the Settings panel with a runtime attached.

apps/desktop/src/preload/preload.ts — the pollAppUserDeviceAuth args wrapping.

Important Files Changed

Filename Overview
apps/desktop/src/preload/preload.ts Wires four new github IPC endpoints; pollAppUserDeviceAuth passes { args } (nested) instead of args directly to callProjectRuntimeActionOr, breaking the runtime action path for device-flow polling.
apps/webhook-relay/src/relay.ts Adds assertGitHubRepoAuthorized with a permissions-hash check plus a 3-step collaborator-permission fallback; helper functions are cleanly separated and tests cover the new branches.
apps/desktop/src/main/services/github/githubAppUserAuthService.ts New factory that encapsulates device-flow session state, single-flight refresh with epoch fence, and credential persistence; edge cases from the previous round (null refreshTokenExpiresAt, session pruning) are addressed.
apps/desktop/src/main/services/github/githubAppUserAuth.ts Pure HTTP helpers for GitHub device flow and token refresh; correct handling of authorization_pending (HTTP 200 + error field) and the slow-down backoff.
apps/desktop/src/main/services/github/githubRelayConfig.ts Adds deduped audit-log factory (now size-capped at 500) and resolveHostedGitHubRelayAuthToken; fetchGitHubAppInstallationStatus correctly gates on the app-user token for hosted relays.
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx Device-flow UX: auto-renew capped at 3, cleanup timeouts on unmount, copy-feedback timeout cleared on session change; polling loop correctly uses the cancelled flag to guard all post-await state updates.
apps/desktop/src/main/services/automations/automationIngressService.ts Switches from getTokenOrThrow to getAppUserTokenForRelay; hosted-auth resolution and audit logging are correctly gated on the non-legacy route.
apps/ade-cli/src/cli.ts Adds `github app-auth login
apps/desktop/src/main/services/adeActions/registry.ts New device-auth actions added to CTO-only and allowlist; forceRefresh forwarding added to getAppInstallationStatus.
apps/webhook-relay/test/relay.test.ts Solid new test coverage for read-only token rejection, collaborator-permission fallback (both authorized and denied), and admin token happy path.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as GitHubAppInstallPanel
    participant Preload as preload.ts
    participant IPC as Electron IPC
    participant GS as githubService
    participant Auth as githubAppUserAuthService
    participant GH as GitHub API

    UI->>Preload: startAppUserDeviceAuth()
    Preload->>IPC: invoke(githubStartAppUserDeviceAuth)
    IPC->>GS: startAppUserDeviceAuth()
    GS->>Auth: startDeviceAuth()
    Auth->>GH: POST /login/device/code
    GH-->>Auth: "{ device_code, user_code, expires_in }"
    Auth-->>GS: "{ sessionId, userCode, verificationUri }"
    GS-->>UI: DeviceAuthStartResult

    loop Poll until authorized/expired/denied
        UI->>Preload: "pollAppUserDeviceAuth({ sessionId })"
        Preload->>IPC: invoke(githubPollAppUserDeviceAuth, args)
        IPC->>GS: "pollAppUserDeviceAuth({ sessionId })"
        GS->>Auth: "pollDeviceAuth({ sessionId })"
        Auth->>GH: POST /login/oauth/access_token
        GH-->>Auth: "{ access_token } or { error: authorization_pending }"
        Auth-->>UI: "DeviceAuthPollResult { status }"
    end

    UI->>Preload: getAppInstallationStatus()
    Preload->>IPC: invoke(githubGetAppInstallationStatus)
    IPC->>GS: getAppInstallationStatus()
    GS->>Auth: getValidTokenForRelay()
    Note over Auth: Refresh if expiring (single-flight)
    Auth-->>GS: app user token
    GS->>Relay: GET /github/repos/:owner/:repo/status
    Relay->>GH: GET /repos/:owner/:repo
    Note over Relay,GH: Checks permissions.push/maintain/admin or falls back to collaborator API
    GH-->>Relay: repo + permissions
    Relay-->>UI: installation status
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as GitHubAppInstallPanel
    participant Preload as preload.ts
    participant IPC as Electron IPC
    participant GS as githubService
    participant Auth as githubAppUserAuthService
    participant GH as GitHub API

    UI->>Preload: startAppUserDeviceAuth()
    Preload->>IPC: invoke(githubStartAppUserDeviceAuth)
    IPC->>GS: startAppUserDeviceAuth()
    GS->>Auth: startDeviceAuth()
    Auth->>GH: POST /login/device/code
    GH-->>Auth: "{ device_code, user_code, expires_in }"
    Auth-->>GS: "{ sessionId, userCode, verificationUri }"
    GS-->>UI: DeviceAuthStartResult

    loop Poll until authorized/expired/denied
        UI->>Preload: "pollAppUserDeviceAuth({ sessionId })"
        Preload->>IPC: invoke(githubPollAppUserDeviceAuth, args)
        IPC->>GS: "pollAppUserDeviceAuth({ sessionId })"
        GS->>Auth: "pollDeviceAuth({ sessionId })"
        Auth->>GH: POST /login/oauth/access_token
        GH-->>Auth: "{ access_token } or { error: authorization_pending }"
        Auth-->>UI: "DeviceAuthPollResult { status }"
    end

    UI->>Preload: getAppInstallationStatus()
    Preload->>IPC: invoke(githubGetAppInstallationStatus)
    IPC->>GS: getAppInstallationStatus()
    GS->>Auth: getValidTokenForRelay()
    Note over Auth: Refresh if expiring (single-flight)
    Auth-->>GS: app user token
    GS->>Relay: GET /github/repos/:owner/:repo/status
    Relay->>GH: GET /repos/:owner/:repo
    Note over Relay,GH: Checks permissions.push/maintain/admin or falls back to collaborator API
    GH-->>Relay: repo + permissions
    Relay-->>UI: installation status
Loading

Comments Outside Diff (2)

  1. General comment

    P1 CTO app-auth clear hangs instead of clearing authorization promptly

    • Bug
      • At head, timeout 25s npm --prefix apps/ade-cli run dev -- --role cto github app-auth clear --text timed out with exit code 124 and no CLI result. The claimed contract says clear is a typed CTO-gated command that removes stored authorization; a CTO invocation should complete with status metadata rather than hanging at the CLI boundary.
    • Cause
      • The github app-auth clear plan in apps/ade-cli/src/cli.ts routes to the runtime action, but the CTO/headless execution path did not return to the CLI in validation. This appears anchored in the CLI action wiring/connection lifecycle for github.clearAppUserAuth when elevated as CTO.
    • Fix
      • Ensure the github app-auth clear command uses the same bounded, returning runtime path as status and closes the CLI connection after the clearAppUserAuth result is formatted. Add a targeted CLI/integration test that runs --role cto github app-auth clear --text and asserts a zero exit plus no token output.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 Non-CTO app-auth login did not fail fast with the documented CTO gate

    • Bug
      • The contract requires login/start-poll to be CTO-gated. In the after run, non-CTO clear returned an elevated-role error, but github app-auth login --max-wait 1 --text did not return a clear CTO-gate error before the validation process was terminated (exit 143). It should fail fast with the documented requires --role cto usage error rather than hanging/continuing into connection work.
    • Cause
      • apps/ade-cli/src/cli.ts only converts elevated-role errors to a login-specific usage error after creating a connection and attempting the startAppUserDeviceAuth action; the non-CTO path did not reliably return that error at the CLI boundary during validation.
    • Fix
      • Add an explicit role check in buildGithubPlan or at the beginning of runGithubAppLogin for login/start and return the documented usage error before opening runtime/device-flow state. Cover with a CLI test for ade github app-auth login --max-wait 1 --text without --role cto.

    T-Rex Ran code and verified through T-Rex

Reviews (4): Last reviewed commit: "Address review: keep re-authorize CTA vi..." | Re-trigger Greptile

…ssion gate

Worker: /github/repos/:owner/:repo/{events,status} now require a Bearer
GitHub token with push/maintain/admin on the repo (GET /repos permissions,
fallback /user + collaborator permission). Read-only/public tokens get 403
with no event or status leakage. Legacy /projects routes unchanged.

Clients: hosted relay reads authenticate with an expiring GitHub App user
token minted via GitHub device flow (never the user's PAT/gh token). New
shared githubAppUserAuthService factory (desktop + ade-cli) owns the token
store, single-flight refresh with clear-auth epoch fence, device-session
lifecycle, and deduped audit logging. Settings panel gains the Authorize
ADE device-flow UI (copyable code, auto-renew capped at 3); new typed
`ade github app-auth login|status|clear` for headless setups.

Live-verified against GitHub: device flow enabled, read-only app user
token reports true repo role, collaborator fallback works, pending polls
return HTTP 200.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview Jul 2, 2026 7:24am

@mintlify

mintlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
ade-ac1c6011 🟢 Ready View Preview Jul 2, 2026, 6:06 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@arul28 arul28 changed the title Blocker And Then Release Webhook relay multi-user hardening: GitHub App device-flow auth + repo-permission gate Jul 2, 2026
@arul28

arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

@copilot review but do not make fixes

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds GitHub App user device-flow authentication across desktop, CLI, IPC, and renderer surfaces, wires hosted relay status checks to app-user tokens, and rewrites webhook-relay repository authorization to use permission checks with collaborator fallback.

Changes

GitHub App Device Auth Implementation

Layer / File(s) Summary
Device flow OAuth helpers and auth service
apps/desktop/src/main/services/github/githubAppUserAuth.ts, githubAppUserAuthService.ts
Defines device-flow types/helpers and a stateful auth service for sessions, token persistence, refresh, clearing, and audit logging.
Hosted relay auth resolution
apps/desktop/src/main/services/github/githubRelayConfig.ts
Adds hosted auth token resolution, audit-log de-duplication, and hosted-relay status-check token selection.
Desktop GitHub service and automation ingress wiring
apps/desktop/src/main/services/github/githubService.ts, apps/desktop/src/main/services/automations/automationIngressService.ts, .../*.test.ts
Wires app-user auth into desktop GitHub services, updates relay polling to use app-user tokens, and adjusts related tests.
IPC handlers, action registry, and preload types
apps/desktop/src/main/services/ipc/registerIpc.ts, apps/desktop/src/main/services/adeActions/registry.ts, apps/desktop/src/shared/ipc.ts, apps/desktop/src/shared/types/git.ts, apps/desktop/src/preload/global.d.ts, apps/desktop/src/preload/preload.ts
Exposes the new GitHub App auth methods through IPC channels, ADE action permissions, and preload/global type declarations.
Renderer device auth UI and mocks
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx, apps/desktop/src/renderer/browserMock.ts
Adds the GitHub App device authorization UI, polling flow, browser mocks, and updated installation-status mock fields.
ADE CLI app-auth commands and headless wiring
apps/ade-cli/src/cli.ts, apps/ade-cli/src/headlessLinearServices.ts, apps/ade-cli/README.md
Adds github app-auth CLI plans and runtime login flow, updates headless GitHub service auth wiring, and documents the command surface.

Webhook Relay Permission-Based Authorization

Layer / File(s) Summary
Permission-based repo authorization and tests
apps/webhook-relay/src/relay.ts, apps/webhook-relay/test/relay.test.ts, apps/webhook-relay/README.md
Changes repo authorization to inspect permission flags, adds collaborator fallback, and updates documentation and tests for the new behavior.

Estimated code review effort: 4 (Complex) | ~75 minutes

Possibly related PRs

  • arul28/ADE#683: Shares the same GitHub relay and automation polling paths that now consume GitHub App user relay tokens.

Suggested labels: desktop

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly captures the two main changes: GitHub App device-flow auth and repo-permission gating for the webhook relay.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/blocker-and-then-release-6b23f7fe

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx (1)

263-319: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Show the GitHub authorization state before installation states.

Line 313 only returns “Authorize GitHub” when the installation check is already in error, so an unauthenticated user can still see “Configured”, “Installed”, or “Not installed” from earlier branches while the new auth CTA is shown. Move the !appAuthorized branch ahead of the installation-status branches so the header/badge matches the required auth action.

Proposed fix
 function statusView(status: GitHubAppInstallationStatus | null, loading: boolean, appAuthorized: boolean): {
   label: string;
   color: string;
   description: (repoLabel: string | null) => string;
 } {
   if (loading && !status) {
     return {
       label: "Checking",
       color: COLORS.warning,
       description: () => "Checking whether this project already has the ADE GitHub App installed.",
     };
   }
+  if (!appAuthorized) {
+    return {
+      label: "Authorize GitHub",
+      color: COLORS.warning,
+      description: () => "Authorize ADE with GitHub to enable instant PR updates for this repo.",
+    };
+  }
   if (status?.relayConfigured && status.webhookState === "deleted") {
     return {
       label: "Webhook off",
       color: COLORS.warning,
       description: (repoLabel) =>
@@
   }
   if (status?.state === "error") {
-    if (!appAuthorized) {
-      return {
-        label: "Authorize GitHub",
-        color: COLORS.warning,
-        description: () => "Authorize ADE with GitHub to enable instant PR updates for this repo.",
-      };
-    }
     return {
       label: "Check failed",
       color: COLORS.danger,
       description: () => status.error ?? "ADE could not check GitHub App status. Existing GitHub auth remains the fallback.",
🤖 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 `@apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx` around
lines 263 - 319, The statusView() logic in GitHubAppInstallPanel.tsx should
prioritize authorization state before any installation-related labels so
unauthenticated users do not see “Configured,” “Installed,” or other status
badges from earlier branches. Move the !appAuthorized handling ahead of the
status?.relayConfigured / status?.installed checks, and keep the “Authorize
GitHub” CTA as the first applicable return so the displayed header matches the
required auth action.
🤖 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 `@apps/ade-cli/src/cli.ts`:
- Around line 15632-15645: In cli.ts, the polling loop in the device auth flow
currently always sleeps for the full GitHub interval before checking the
deadline, which can push execution past --max-wait. Update the while loop around
runGithubAction("pollAppUserDeviceAuth") to cap the sleep duration by the
remaining time until deadlineMs, and keep the existing timeout branch as the
final guard. Use the existing deadlineMs, intervalSec, and sessionId flow in
this auth polling block so the loop times out exactly when the configured wait
is exceeded.

In `@apps/desktop/src/main/services/github/githubAppUserAuthService.ts`:
- Around line 172-173: The refresh path in githubAppUserAuthService can still
persist an old token after a newer authorization has been stored, because
authEpoch only changes in clearAuth. Update the token replacement flow in
pollDeviceAuth and the refresh handler that calls persistAppUserTokenRecord so
stale in-flight refreshes are rejected, either by bumping authEpoch whenever a
token is replaced or by checking the currently active refresh token before
persisting. Keep the epoch/token guard consistent in the then((refreshed) =>
...) logic and any other persistence paths referenced by the review.

In `@apps/desktop/src/main/services/github/githubService.test.ts`:
- Around line 1578-1612: The in-flight refresh path in githubService should not
yield a usable token after clearAppUserAuth() invalidates the current auth
epoch. Update the token refresh flow in getAppUserTokenForRelay (and any helper
that persists the refreshed token) to detect epoch/status mismatch and reject or
suppress the refreshed response instead of returning "ghu_new_token". Adjust
this test to assert the promise is rejected or resolves without a token, and
keep the credentialStore and getAppUserAuthStatus assertions verifying auth
stays cleared.

In `@apps/desktop/src/main/services/ipc/registerIpc.ts`:
- Around line 7912-7930: The IPC handlers in registerIpc.ts for
githubStartAppUserDeviceAuth and githubPollAppUserDeviceAuth currently forward
requests directly to the GitHub service without any abuse control. Add
per-sender/per-channel throttling around these handlers in ipcMain.handle, and
enforce a small cap on active device auth sessions in the GitHub service so
repeated renderer calls cannot spam GitHub or accumulate pending sessions. Use
the existing getCtx, startAppUserDeviceAuth, and pollAppUserDeviceAuth entry
points to place the guards close to the IPC boundary.

---

Outside diff comments:
In `@apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx`:
- Around line 263-319: The statusView() logic in GitHubAppInstallPanel.tsx
should prioritize authorization state before any installation-related labels so
unauthenticated users do not see “Configured,” “Installed,” or other status
badges from earlier branches. Move the !appAuthorized handling ahead of the
status?.relayConfigured / status?.installed checks, and keep the “Authorize
GitHub” CTA as the first applicable return so the displayed header matches the
required auth action.
🪄 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: 78295a53-d056-408b-a55c-8998eb70f939

📥 Commits

Reviewing files that changed from the base of the PR and between 323406c and 0937d3e.

⛔ Files ignored due to path filters (3)
  • docs/ARCHITECTURE.md is excluded by !docs/**
  • docs/features/automations/README.md is excluded by !docs/**
  • docs/features/onboarding-and-settings/README.md is excluded by !docs/**
📒 Files selected for processing (21)
  • apps/ade-cli/README.md
  • apps/ade-cli/src/cli.ts
  • apps/ade-cli/src/headlessLinearServices.ts
  • apps/desktop/src/main/services/adeActions/registry.ts
  • apps/desktop/src/main/services/automations/automationIngressService.test.ts
  • apps/desktop/src/main/services/automations/automationIngressService.ts
  • apps/desktop/src/main/services/github/githubAppUserAuth.ts
  • apps/desktop/src/main/services/github/githubAppUserAuthService.ts
  • apps/desktop/src/main/services/github/githubRelayConfig.ts
  • apps/desktop/src/main/services/github/githubService.test.ts
  • apps/desktop/src/main/services/github/githubService.ts
  • apps/desktop/src/main/services/ipc/registerIpc.ts
  • apps/desktop/src/preload/global.d.ts
  • apps/desktop/src/preload/preload.ts
  • apps/desktop/src/renderer/browserMock.ts
  • apps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsx
  • apps/desktop/src/shared/ipc.ts
  • apps/desktop/src/shared/types/git.ts
  • apps/webhook-relay/README.md
  • apps/webhook-relay/src/relay.ts
  • apps/webhook-relay/test/relay.test.ts

Comment thread apps/ade-cli/src/cli.ts
Comment on lines +15632 to +15645
while (true) {
if (Number.isFinite(deadlineMs) && Date.now() >= deadlineMs) {
process.stderr.write("GitHub device authorization timed out.\n");
const status = await runGithubAction("getAppUserAuthStatus");
return {
output: formatOutput(
{ ...status, status: "expired", error: "timed_out" },
options,
),
exitCode: 1,
};
}
await sleep(Math.max(1, intervalSec) * 1000);
const poll = await runGithubAction("pollAppUserDeviceAuth", { sessionId });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Bound the sleep to the remaining deadline.

With --max-wait shorter than GitHub’s polling interval, Line 15644 sleeps the full interval and can poll after the configured deadline instead of timing out at N seconds.

🐛 Proposed fix
-      await sleep(Math.max(1, intervalSec) * 1000);
+      const sleepMs = Math.max(1, intervalSec) * 1000;
+      const waitMs = Number.isFinite(deadlineMs)
+        ? Math.min(sleepMs, Math.max(0, deadlineMs - Date.now()))
+        : sleepMs;
+      if (waitMs > 0) await sleep(waitMs);
+      if (Number.isFinite(deadlineMs) && Date.now() >= deadlineMs) {
+        continue;
+      }
       const poll = await runGithubAction("pollAppUserDeviceAuth", { sessionId });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while (true) {
if (Number.isFinite(deadlineMs) && Date.now() >= deadlineMs) {
process.stderr.write("GitHub device authorization timed out.\n");
const status = await runGithubAction("getAppUserAuthStatus");
return {
output: formatOutput(
{ ...status, status: "expired", error: "timed_out" },
options,
),
exitCode: 1,
};
}
await sleep(Math.max(1, intervalSec) * 1000);
const poll = await runGithubAction("pollAppUserDeviceAuth", { sessionId });
while (true) {
if (Number.isFinite(deadlineMs) && Date.now() >= deadlineMs) {
process.stderr.write("GitHub device authorization timed out.\n");
const status = await runGithubAction("getAppUserAuthStatus");
return {
output: formatOutput(
{ ...status, status: "expired", error: "timed_out" },
options,
),
exitCode: 1,
};
}
const sleepMs = Math.max(1, intervalSec) * 1000;
const waitMs = Number.isFinite(deadlineMs)
? Math.min(sleepMs, Math.max(0, deadlineMs - Date.now()))
: sleepMs;
if (waitMs > 0) await sleep(waitMs);
if (Number.isFinite(deadlineMs) && Date.now() >= deadlineMs) {
continue;
}
const poll = await runGithubAction("pollAppUserDeviceAuth", { sessionId });
🤖 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 `@apps/ade-cli/src/cli.ts` around lines 15632 - 15645, In cli.ts, the polling
loop in the device auth flow currently always sleeps for the full GitHub
interval before checking the deadline, which can push execution past --max-wait.
Update the while loop around runGithubAction("pollAppUserDeviceAuth") to cap the
sleep duration by the remaining time until deadlineMs, and keep the existing
timeout branch as the final guard. Use the existing deadlineMs, intervalSec, and
sessionId flow in this auth polling block so the loop times out exactly when the
configured wait is exceeded.

Comment thread apps/desktop/src/main/services/github/githubAppUserAuthService.ts Outdated
Comment thread apps/desktop/src/main/services/github/githubService.test.ts Outdated
Comment on lines +7912 to +7930
ipcMain.handle(IPC.githubStartAppUserDeviceAuth, async (): Promise<GitHubAppDeviceAuthStartResult> => {
const ctx = getCtx();
return await ctx.githubService.startAppUserDeviceAuth();
});

ipcMain.handle(
IPC.githubPollAppUserDeviceAuth,
async (_event, arg: { sessionId?: string }): Promise<GitHubAppDeviceAuthPollResult> => {
const ctx = getCtx();
const sessionId = arg?.sessionId?.trim() ?? "";
if (!sessionId) {
return {
status: "error",
intervalSec: null,
message: "GitHub device authorization session id is required.",
authStatus: ctx.githubService.getAppUserAuthStatus(),
};
}
return await ctx.githubService.pollAppUserDeviceAuth({ sessionId });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Throttle device-flow IPC before it reaches GitHub.

githubStartAppUserDeviceAuth and githubPollAppUserDeviceAuth delegate straight to network-backed device-flow calls, and the service keeps device sessions in memory. Add per-sender/channel throttling and ideally a small active-session cap so a renderer bug cannot spam GitHub or grow pending sessions. As per path instructions, apps/desktop/src/** Electron changes should be checked for IPC security.

🤖 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 `@apps/desktop/src/main/services/ipc/registerIpc.ts` around lines 7912 - 7930,
The IPC handlers in registerIpc.ts for githubStartAppUserDeviceAuth and
githubPollAppUserDeviceAuth currently forward requests directly to the GitHub
service without any abuse control. Add per-sender/per-channel throttling around
these handlers in ipcMain.handle, and enforce a small cap on active device auth
sessions in the GitHub service so repeated renderer calls cannot spam GitHub or
accumulate pending sessions. Use the existing getCtx, startAppUserDeviceAuth,
and pollAppUserDeviceAuth entry points to place the guards close to the IPC
boundary.

Source: Path instructions

Comment thread apps/desktop/src/main/services/github/githubAppUserAuthService.ts Outdated
Comment thread apps/desktop/src/main/services/github/githubRelayConfig.ts
Comment thread apps/desktop/src/main/services/github/githubAppUserAuthService.ts
…on cap

- Bump authEpoch when device auth stores a new token so a stale in-flight
  refresh can never overwrite newer auth state (CodeRabbit)
- On epoch mismatch after refresh, re-resolve against current auth state:
  cleared -> reject, re-authorized -> return the new token (CodeRabbit)
- Treat missing refreshTokenExpiresAt as unknown, not expired (Greptile)
- Cap pending device-auth sessions at 5, evicting oldest (CodeRabbit)
- Drop unreachable post-prune expiry branch in pollDeviceAuth (Greptile)
- Cap the relay audit dedup set at 500 keys (Greptile)
- Bound `ade github app-auth login` poll sleep to the --max-wait deadline
  (CodeRabbit)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arul28

arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f5505b700

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 38 to 40
const authStatus = await window.ade.github.getAppUserAuthStatus?.().catch(() => null);
setAppAuth(authStatus ?? null);
setStatus(await window.ade.github.getAppInstallationStatus({ forceRefresh }));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Refresh auth state after status checks clear it

When a stored App user token is expired or otherwise rejected during getAppInstallationStatus, the main service can clear that credential before returning the auth-required status, but this component keeps the earlier tokenStored: true value read here. In that expired-token scenario the panel shows a check failure while hiding the “Authorize ADE” button until a second refresh/remount, so re-read the auth status after the status call or update it from the failure path.

Useful? React with 👍 / 👎.

The status check can clear an expired stored token; reading auth state
first left the panel showing a stale tokenStored=true with the Authorize
button hidden (Codex P2).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arul28

arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8ce3263be

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


<div style={actionRowStyle}>
{status?.installed ? null : (
{!appAuthorized ? (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep reauthorization available for failing app-user tokens

When the stored app-user token belongs to a different GitHub account, lacks write access to the current repo, or is otherwise stale, the hosted relay status check returns an auth/permission error while getAppUserAuthStatus can still report tokenStored: true; this guard then hides the only Authorize ADE CTA, leaving Settings with no way to switch accounts or re-authorize from the panel. Please keep a reauthorize/clear path available on auth or permission failures, not only when no token is stored.

Useful? React with 👍 / 👎.

A stored token from the wrong account or without repo access produced a
status error with the Authorize button hidden, leaving no recovery path
in Settings (Codex P2). Status errors now surface a Re-authorize button
that restarts the device flow and replaces the stored token.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@arul28

arul28 commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 617fe96d21

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

githubAppInstallationStatusCache.clear();
},
() =>
callProjectRuntimeActionOr("github", "pollAppUserDeviceAuth", { args }, () =>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Pass the session ID at the action argument level

When a project runtime is bound, this call does not reach the IPC fallback; it invokes the GitHub action through run_ade_action, which passes the contents of request.args directly to githubService.pollAppUserDeviceAuth. Wrapping the payload as { args } makes the service receive { args: { sessionId } } instead of { sessionId }, so pollDeviceAuth sees an undefined session id and reports the device session as not found, preventing the desktop Authorize ADE flow from completing for normal open projects.

Useful? React with 👍 / 👎.

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.

Verified against the actual dispatch chain — this is the correct envelope, not a double-wrap. callProjectRuntimeActionOr("github", "pollAppUserDeviceAuth", { args }) produces RemoteRuntimeActionRequest.args = { sessionId }; localRuntimeConnectionPool.callActionForRootUncoalesced then maps it to run_ade_action as arguments: { domain, action, args: request.args } (localRuntimeConnectionPool.ts ~line 995), and the daemon calls service.pollAppUserDeviceAuth(safeObject(toolArgs.args)){ sessionId } (adeRpcServer.ts ~3443). Same pattern as the long-standing listRepoAutolinks/listRepoLabels wiring on this route. Also runtime-verified end-to-end via ade actions run github.pollAppUserDeviceAuth during CLI parity testing.

@arul28 arul28 merged commit 49b5fa5 into main Jul 2, 2026
29 checks passed
@arul28 arul28 deleted the ade/blocker-and-then-release-6b23f7fe branch July 2, 2026 07:53
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