Skip to content

docs: define OAuth support boundary (#79) - #90

Merged
mohanagy merged 2 commits into
developmentfrom
feat/issue-79-oauth-support-boundary
Jul 16, 2026
Merged

docs: define OAuth support boundary (#79)#90
mohanagy merged 2 commits into
developmentfrom
feat/issue-79-oauth-support-boundary

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • define the supported, delegated, and unsupported OAuth boundaries without claiming universal provider support
  • document future standards-compatible remote HTTP OAuth requirements and profile/identity implications
  • add a deterministic loopback SDK compatibility probe plus documentation contract tests

No production OAuth configuration, token lifecycle, or runtime authentication behavior is introduced by this research issue.

Validation

  • npm test
  • npm run test:core
  • npm run test:coverage
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm run smoke:cli
  • npm run check:pack
  • npm run test:package

Closes #79

Summary by CodeRabbit

  • Documentation

    • Clarified that remote authentication currently supports only explicitly configured static credentials.
    • Documented that OAuth discovery, browser authorization, callbacks, token refresh, and revocation are not currently provided.
    • Added comprehensive OAuth support guidance, compatibility boundaries, security considerations, and future requirements.
    • Cross-linked OAuth guidance throughout configuration, architecture, security, library API, and README documentation.
  • Tests

    • Added coverage validating documented OAuth limitations and supported compatibility flows.
    • Added deterministic checks for authentication discovery, registration, PKCE authorization, token exchange, and authenticated connections.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Documents Miftah’s current OAuth boundary, static remote credential support, future remote OAuth contract, and identity separation. Adds a deterministic loopback compatibility probe and tests for documentation alignment and the remote MCP OAuth flow.

Changes

OAuth support boundary and compatibility probe

Layer / File(s) Summary
Boundary documentation and contract validation
README.md, docs/*.md, tests/oauth-support-docs-contract.test.ts
Documents unsupported OAuth lifecycle ownership, supported credential patterns, strict configuration boundaries, and the absence of OAuth public exports; contract tests verify these statements.
Deterministic OAuth compatibility probe
tests/helpers/fake-remote-upstream.ts
Adds discovery, registration, token exchange, bearer-protected MCP handling, request capture, and cleanup for a loopback test upstream.
Remote OAuth end-to-end validation
tests/remote-oauth-compatibility.test.ts
Exercises unauthorized connection, PKCE authorization, registration, token exchange, authenticated reconnection, tool listing, request counts, and cleanup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Poem

A bunny hops through OAuth’s gate,
“Not brokered yet—but documented straight!”
PKCE blooms in a loopback burrow,
Tokens are tested, neat as a carrot arrow.
Static headers guard the way,
While future flows await their day.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Docstring Coverage ❌ Error Docstring coverage is 22.22% 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 clearly and concisely describes the PR’s main change: defining the OAuth support boundary.
Description check ✅ Passed The description covers the summary and validation, but it omits the required Security impact section from the template.
Linked Issues check ✅ Passed The docs and tests implement the OAuth boundary, capability matrix, compatibility probe, and no-public-surface contract required by #79.
Out of Scope Changes check ✅ Passed The changes stay focused on OAuth boundary docs and contract tests, with no clear unrelated code or behavior changes.
✨ 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 feat/issue-79-oauth-support-boundary

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: 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 `@tests/helpers/fake-remote-upstream.ts`:
- Around line 322-331: Update the fake OAuth upstream’s authorization and token
flows in tests/helpers/fake-remote-upstream.ts (lines 58-65 and 322-331) to
retain the authorization redirect’s code_challenge, derive the S256 challenge
from the token request’s code_verifier, and record or validate that the values
match instead of only checking codeVerifierPresent. Update
tests/remote-oauth-compatibility.test.ts (lines 98-117) to assert the PKCE
verifier/challenge match so mismatched pairs are rejected.

In `@tests/oauth-support-docs-contract.test.ts`:
- Around line 82-92: Replace the exact whitespace-sensitive `upstreamManager`
assertion for `requestInit: { headers }` with a formatting-tolerant check that
still verifies the upstream request uses configured headers and does not
introduce an OAuth provider. Keep the surrounding OAuth documentation contract
assertions unchanged.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: b52ea56e-6e0b-4039-915a-01d1ff0965db

📥 Commits

Reviewing files that changed from the base of the PR and between 0f7e9fe and 81d4c7b.

📒 Files selected for processing (9)
  • README.md
  • docs/architecture.md
  • docs/config.md
  • docs/library-api.md
  • docs/oauth-support.md
  • docs/security.md
  • tests/helpers/fake-remote-upstream.ts
  • tests/oauth-support-docs-contract.test.ts
  • tests/remote-oauth-compatibility.test.ts

Comment thread tests/helpers/fake-remote-upstream.ts
Comment thread tests/oauth-support-docs-contract.test.ts
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.

[Research] Define Miftah's OAuth support boundary and provider capability matrix

1 participant