Skip to content

Fail fast on deploy integration auth errors#115

Merged
khaliqgant merged 2 commits into
mainfrom
codex/integration-auth-fail-fast
May 13, 2026
Merged

Fail fast on deploy integration auth errors#115
khaliqgant merged 2 commits into
mainfrom
codex/integration-auth-fail-fast

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

Summary

  • Stop deploy integration preflight from prompting OAuth connect after an auth rejection from the cloud integration status endpoint.
  • Improve 401/403 integration request guidance so already-logged-in users are pointed at API URL: https://origin.agentrelay.cloud/cloud
    Auth source: token
    Subject type: cli
    User: Khaliq Gant khaliq@agentrelay.com
    Organization: Khaliq Gant's Workspace
    Workspace: Default
    Scopes: cli:auth
    Token file: /Users/khaliqgant/.agent-relay/cloud-auth.json and active workspace refresh instead of a generic login retry.
  • Add regression coverage proving auth failures do not call the connect flow.

Verification

  • corepack pnpm --filter @agentworkforce/deploy test

Notes

This does not touch cloud production or merge anything. Current main already includes the functional shared-auth reuse from #113; this PR fixes the confusing follow-on behavior that produced repeated connect prompts after an auth failure.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2593695a-f150-478c-8c5e-2aae2986b6ae

📥 Commits

Reviewing files that changed from the base of the PR and between eaccf24 and 91e8af5.

📒 Files selected for processing (2)
  • packages/deploy/src/connect.test.ts
  • packages/deploy/src/connect.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/deploy/src/connect.test.ts
  • packages/deploy/src/connect.ts

📝 Walkthrough

Walkthrough

connectIntegrations now captures isConnected errors, classifies 401/403 as auth failures, logs them, records a failed outcome for the provider, and skips prompting or connect attempts. requestJson throws dedicated 401/403 messages and isIntegrationAuthFailure detects auth-related failures. Tests added to cover this flow.

Changes

Integration Auth Failure Handling

Layer / File(s) Summary
Auth failure detection helper
packages/deploy/src/connect.ts
requestJson now throws dedicated error messages for HTTP 401 and 403 responses. isIntegrationAuthFailure helper classifies auth-related request failures by matching the error message text.
connectIntegrations error capture and fast-fail
packages/deploy/src/connect.ts
connectIntegrations stores isConnected catch messages in statusCheckFailure, logs the failure, detects auth-related failures via isIntegrationAuthFailure, records a failed outcome with the captured message, and skips prompts/connect for that provider.
Test coverage for auth fast-fail
packages/deploy/src/connect.test.ts
Tests import connectIntegrations and add a case that simulates an auth failure during isConnected, asserting io.confirm and integrations.connect are not called, validating the failed outcomes entry, and checking emitted warn/error logs.

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A 401 hops in, bold and curt,
I sniff the token, find it inert.
No prompts, no hops to connect the vine,
I log the fail and mark the sign.
Hoppity, quick — outcomes tidy and fine.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding fail-fast behavior for deploy integration auth errors.
Description check ✅ Passed The description is directly related to the changeset, detailing the auth error handling improvements and test coverage added.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 codex/integration-auth-fail-fast

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@packages/deploy/src/connect.test.ts`:
- Around line 105-143: The test currently only checks that connect() wasn't
called but doesn't explicitly assert that no prompt was shown; after calling
connectIntegrations and alongside the existing assertions, add an explicit
assertion that the buffered IO emitted no confirm prompts (i.e., assert that io
recorded zero confirm calls or that io.messages contains no confirm-type
entries) so the test fails if any confirm prompt is produced; reference the
createBufferedIO() instance named io and the connectIntegrations invocation when
adding this assertion.

In `@packages/deploy/src/connect.ts`:
- Around line 300-307: The 401/403 error messages thrown when checking
res.status omit the cloud session URL; update the two Error messages (the
unauthorized branch that throws when res.status === 401 and the forbidden branch
for res.status === 403) to include the cloud session URL
"https://origin.agentrelay.cloud/cloud" in the remediation text so users can
visit that page to validate/fix cloud session state; locate the throw new
Error(...) calls in connect.ts that reference 'cloud integration request failed:
unauthorized' and 'cloud integration request failed: forbidden' and append or
interpolate the URL into each message.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c0fc34ad-61d8-403d-bb6a-99f56db93008

📥 Commits

Reviewing files that changed from the base of the PR and between b5c604e and eaccf24.

📒 Files selected for processing (2)
  • packages/deploy/src/connect.test.ts
  • packages/deploy/src/connect.ts

Comment thread packages/deploy/src/connect.test.ts
Comment thread packages/deploy/src/connect.ts
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: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@khaliqgant khaliqgant merged commit edac0b5 into main May 13, 2026
2 checks passed
@khaliqgant khaliqgant deleted the codex/integration-auth-fail-fast branch May 13, 2026 16:28
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