Skip to content

fix: suppress browser-wait log in JSON mode; sweep expired OAuth state on insertion#29597

Merged
credence-the-bot[bot] merged 2 commits into
credence-the-bot/oauth-connect-daemon-flowfrom
run-plan/oauth-connect-flow/fix-codex-p1p2
May 5, 2026
Merged

fix: suppress browser-wait log in JSON mode; sweep expired OAuth state on insertion#29597
credence-the-bot[bot] merged 2 commits into
credence-the-bot/oauth-connect-daemon-flowfrom
run-plan/oauth-connect-flow/fix-codex-p1p2

Conversation

@credence-the-bot
Copy link
Copy Markdown
Contributor

@credence-the-bot credence-the-bot Bot commented May 5, 2026

Summary

  • Suppress 'Waiting for authorization in browser...' log.info when --json is set to prevent stdout pollution for machine consumers (connect.ts)
  • Call clearExpiredOAuthConnectStates() at the top of each set* function so flows never polled (--no-browser, abandoned) don't accumulate indefinitely (oauth-connect-state.ts)

Addresses Codex findings P1 and P2 on PR #29596.


Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

…onMode) guard

Configure the logger mock to write to process.stdout in the specific test so
the test would fail if the guard were removed from connect.ts. Also reset
mockLogInfo to no-op in beforeEach for test isolation.
@credence-the-bot
Copy link
Copy Markdown
Contributor Author

@codex review this PR again — the previous issues have been fixed in commit bd99f37

@credence-the-bot
Copy link
Copy Markdown
Contributor Author

@devin review this PR again — the previous issues have been fixed in commit bd99f37

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread assistant/src/cli/commands/oauth/connect.ts
@credence-the-bot credence-the-bot Bot merged commit d76dcfc into credence-the-bot/oauth-connect-daemon-flow May 5, 2026
13 checks passed
@credence-the-bot credence-the-bot Bot deleted the run-plan/oauth-connect-flow/fix-codex-p1p2 branch May 5, 2026 05:16
noanflaherty pushed a commit that referenced this pull request May 5, 2026
…ay` through daemon IPC (#29596)

* fix(oauth): route `assistant oauth connect --callback-transport=gateway` through daemon IPC (#29570)

* fix(oauth): route assistant oauth connect --callback-transport=gateway through daemon IPC

* fix(oauth-connect): pass lint, type-check, and openapi-check

- Sort imports per simple-import-sort/imports rule (autofix)
- Add eslint-disable for the intentionally-let resolvedState binding
- Narrow OAuthConnectResult union before reading .error (split into two if branches)
- Narrow OAuthConnectStatusResponse before reading .error in CLI poll loop
- Regenerate openapi.yaml to include the 2 new internal routes

* fix: use operationId and structured params for cliIpcCall (Devin/Codex P1 review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: thread grantedScopes through IPC path to avoid empty-scopes inconsistency (Devin review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(oauth): align IPC poll timeout with 5-min loopback OAuth window (Codex review)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: credence-the-bot <credence@vellum.ai>
Co-authored-by: credence-the-bot[bot] <183148327+credence-the-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: surface daemon errors instead of silent fallback; add gateway transport test and policy tests (#29590)

Co-authored-by: credence-the-bot <credence@vellum.ai>

* fix: add defensive guards for poll daemon errors and ok:true missing auth_url (#29593)

Co-authored-by: credence-the-bot <credence@vellum.ai>

* test: add missing test coverage for oauth-connect daemon flow (round-3) (#29595)

Co-authored-by: credence-the-bot <credence@vellum.ai>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: suppress browser-wait log in JSON mode; sweep expired OAuth state on insertion (#29597)

* fix: suppress browser-wait log in JSON mode; sweep expired state on insert

* test: make JSON-mode log suppression test exercise the actual if (!jsonMode) guard

Configure the logger mock to write to process.stdout in the specific test so
the test would fail if the guard were removed from connect.ts. Also reset
mockLogInfo to no-op in beforeEach for test isolation.

---------

Co-authored-by: credence-the-bot <credence@vellum.ai>

* fix(oauth): replace 'Daemon' with 'assistant' in user-facing error strings (#29603)

* fix(oauth): replace 'Daemon' with 'assistant' in user-facing error strings

* test: update assertion to match new 'assistant' string from devin nit fix

The previous commit replaced 'Daemon' with 'assistant' in user-facing
error strings (per Devin review), but missed updating the test that
asserts on the old 'Daemon returned unexpected response' wording.

---------

Co-authored-by: credence-the-bot <credence@vellum.ai>
Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>

* fix(security): add IPC policy entries for daemon OAuth connect routes (#29612)

Closes Codex P1 on #29596 by adding IPC policy entries for the new daemon OAuth connect routes (`internal_oauth_connect_start`, `internal_oauth_connect_status`). Mirrors the pattern from #29571 for the MCP OAuth routes. 4-line additive change, 6/6 tests pass. Both Codex and Devin re-reviewed clean.

---------

Co-authored-by: credence-the-bot[bot] <277301654+credence-the-bot[bot]@users.noreply.github.com>
Co-authored-by: credence-the-bot <credence@vellum.ai>
Co-authored-by: credence-the-bot[bot] <183148327+credence-the-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>
Co-authored-by: credence-the-bot[bot] <credence-the-bot@vellum.ai>
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.

0 participants