Add per-user MCP auth flow - #276
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces per-user OAuth2 authentication for protected MCP data sources (e.g., Google Drive). Adds a new ChangesPer-user MCP OAuth2 authentication
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser/CLI)
participant UI as DataSourcesPanel / aiq.py
participant APIProxy as Next.js API Proxy
participant JobsRoute as FastAPI /v1/jobs
participant AuthRoute as FastAPI /v1/auth/mcp
participant NatProvider as NatMcpAuthProvider
participant OAuthServer as OAuth Authorization Server
participant TokenStore as TokenStorage (Redis/InMemory)
User->>UI: Click Connect on protected source
UI->>APIProxy: POST /api/auth/mcp/{source_id}/connect
APIProxy->>AuthRoute: POST /v1/auth/mcp/{source_id}/connect
AuthRoute->>NatProvider: start_auth(principal, source_id)
NatProvider->>OAuthServer: create_authorization_url(+PKCE+state)
NatProvider-->>AuthRoute: SourceAuthChallenge(auth_url, state)
AuthRoute-->>UI: SourceConnectResponse(auth_url)
UI->>OAuthServer: openAuthPopupAndWait(auth_url)
OAuthServer-->>AuthRoute: GET /v1/auth/mcp/{source_id}/callback?code=...&state=...
AuthRoute->>NatProvider: complete_callback(state, callback_url)
NatProvider->>OAuthServer: fetch_token(authorization_response)
NatProvider->>TokenStore: store AuthResult for user+source
AuthRoute-->>OAuthServer: HTML postMessage page (closes popup)
UI->>UI: popup closed → fetchDataSources()
User->>UI: Submit job with gdrive selected
UI->>APIProxy: POST /api/jobs/async/submit
APIProxy->>JobsRoute: POST /v1/jobs/submit
JobsRoute->>NatProvider: evaluate_mcp_auth(principal, data_sources)
NatProvider->>TokenStore: get_status(user, gdrive)
TokenStore-->>NatProvider: connected
NatProvider-->>JobsRoute: None (no block)
JobsRoute->>JobsRoute: enqueue run_agent_job(owner_user_id)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
477-481:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocument the new 409 auth-required response in the submit endpoint contract.
Line 513 can return HTTP 409, but Lines 477-481 omit 409 from
responses. This leaves OpenAPI clients unaware of a first-class error path.Proposed fix
-from ..mcp_auth.models import PerUserAuthInfo +from ..mcp_auth.models import McpAuthRequiredResponse +from ..mcp_auth.models import PerUserAuthInfo @@ responses={ + 409: { + "description": "One or more selected protected sources require connection before submit", + "model": McpAuthRequiredResponse, + }, },🤖 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 `@frontends/aiq_api/src/aiq_api/routes/jobs.py` around lines 477 - 481, The responses dictionary in the submit endpoint decorator is missing documentation for the 409 status code that the endpoint can return. Add a 409 entry to the responses dictionary (alongside the existing 400, 422, and 503 entries) with a description indicating that this status is returned when authentication is required or authorization fails. This ensures the OpenAPI contract accurately reflects all possible error responses from the endpoint.
🤖 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 `@frontends/aiq_api/src/aiq_api/mcp_auth/preflight.py`:
- Around line 59-64: The preflight check is too broad when data_sources is None.
Currently, the code on line 59 uses get_all_sources() to fetch all protected
sources from the registry, but this should instead be scoped to only the sources
available to the selected agent. Replace get_all_sources() in the if
data_sources is None branch with a call that retrieves only the agent-available
sources, then filter those sources for per_user_auth and required status. This
ensures the preflight validation only checks sources relevant to the agent,
preventing incorrect 409 responses for unrelated disconnected sources.
In `@frontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.py`:
- Around line 110-111: The code accesses the private
`builder._registry.get_tool_wrapper()` API to dynamically wrap MCP functions at
runtime, but this private API usage lacks documentation. Add a clear comment
above the line where `builder._registry.get_tool_wrapper()` is called explaining
why the private API is necessary instead of the public `builder.get_tools()`
method. Explain that the public API requires pre-registered tool names and
returns already-wrapped tools, making it unsuitable for late-binding raw
functions discovered dynamically from an MCP client. This documents the NAT
internal dependency and clarifies the architectural constraint that necessitates
this approach.
In `@frontends/aiq_api/src/aiq_api/routes/auth.py`:
- Around line 42-51: The postMessage call in the _CALLBACK_HTML variable is
using "*" as the targetOrigin parameter, which broadcasts the message to any
listening window. Replace the "*" argument in the postMessage call with a more
restrictive origin such as window.location.origin to ensure the authentication
message is only sent to the intended opener window, improving the security
posture of the callback mechanism.
In `@frontends/ui/src/adapters/api/mcp-auth-client.ts`:
- Around line 131-140: The onMessage callback handler does not validate the
event source before accepting mcp-auth messages, creating a security
vulnerability where any sender can trigger the auth flow. Add validation in the
onMessage function to check that event.source === popup before processing the
data, and ideally also validate that the event comes from a trusted origin. This
ensures only messages from the actual popup window are accepted, preventing
unauthorized auth callback acceptance.
In `@frontends/ui/src/features/layout/components/DataConnectionCard.tsx`:
- Around line 76-84: The handleConnect callback function silently swallows
errors from the onConnect call because the finally block always executes and
resets the connecting state, but no error handling or user feedback is provided.
Modify the function to capture any error thrown by onConnect and either
propagate it (by re-throwing after the finally block executes) or add error
state management to display an error message to the user. Ensure the dependency
array in the useCallback is updated if you add new state variables like an error
state.
In `@frontends/ui/src/features/layout/components/DataSourcesPanel.tsx`:
- Around line 129-142: The handleConnect callback is missing error handling for
the client.connect() call, which can fail with network or HTTP errors, leaving
the UI in an inconsistent state with no error feedback. Wrap the async
operations in a try-catch block to handle potential errors from
client.connect(sourceId) and openAuthPopupAndWait(), and ensure that any errors
are communicated to the user (such as displaying an error message or updating UI
state) so the DataConnectionCard properly reflects the failed state instead of
silently exiting the "Connecting…" state.
In `@pyproject.toml`:
- Around line 34-38: Update all nvidia-nat* package versions (nvidia-nat-core,
nvidia-nat with extras, nvidia-nat-eval, nvidia-nat-profiler, and
nvidia-nat-redis) from 1.8.0rc4 to 1.8.0rc6 in the pyproject.toml dependencies
section, as rc6 is the latest available release candidate on PyPI. Before
applying this change, verify that rc4 → rc6 compatibility is confirmed and there
are no known stability issues with rc6; if rc4 is intentionally pinned for a
specific reason, document that decision. Additionally, before merging this
change to the develop branch, confirm the team's prerelease strategy: whether
the intent is to await the final 1.8.0 stable release or to stabilize on a
specific RC version.
---
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 477-481: The responses dictionary in the submit endpoint decorator
is missing documentation for the 409 status code that the endpoint can return.
Add a 409 entry to the responses dictionary (alongside the existing 400, 422,
and 503 entries) with a description indicating that this status is returned when
authentication is required or authorization fails. This ensures the OpenAPI
contract accurately reflects all possible error responses from the endpoint.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 174a2b71-3197-4d7b-89ea-69bb4f25284a
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
configs/config_web_frag.ymlfrontends/aiq_api/src/aiq_api/auth/middleware.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/active.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_mcp_auth_factory.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/benchmarks/deepsearch_qa/pyproject.tomlfrontends/benchmarks/freshqa/pyproject.tomlfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/features/layout/data-sources.tspyproject.tomlskills/aiq-research/scripts/aiq.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/common/data_source_registry.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
frontends/benchmarks/freshqa/pyproject.tomlfrontends/benchmarks/deepsearch_qa/pyproject.tomlfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/layout/data-sources.tssrc/aiq_agent/agents/chat_researcher/register.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pypyproject.tomlfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/src/aiq_api/mcp_auth/active.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/src/aiq_api/auth/middleware.pyconfigs/config_web_frag.ymlsrc/aiq_agent/common/data_source_registry.pyfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_auth.pyfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyskills/aiq-research/scripts/aiq.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_mcp_auth_factory.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/ui/src/features/layout/components/DataSourcesPanel.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/ui/src/features/layout/components/DataSourcesPanel.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/ui/src/features/layout/components/DataSourcesPanel.tsx
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
src/aiq_agent/agents/chat_researcher/register.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/src/aiq_api/mcp_auth/active.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/src/aiq_api/auth/middleware.pysrc/aiq_agent/common/data_source_registry.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyskills/aiq-research/scripts/aiq.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_mcp_auth_factory.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/data_source_registry.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/src/aiq_api/mcp_auth/active.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/src/aiq_api/auth/middleware.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyfrontends/aiq_api/src/aiq_api/routes/jobs.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
pyproject.toml
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyfrontends/aiq_api/tests/test_mcp_auth_factory.py
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}: Review authentication changes for issuer/audience validation, token parsing, error hygiene, logging safety,
and compatibility with local and deployed modes. Do not accept changes that expose tokens, weaken validation,
or blur trusted server-side identity with client-supplied fields.
Files:
frontends/aiq_api/src/aiq_api/auth/middleware.py
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
configs/config_web_frag.yml
skills/aiq-research/scripts/aiq.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/scripts/aiq.py: Use Python 3.11+ with the helper script atscripts/aiq.pyto call a locally running NVIDIA AI-Q Blueprint server
Resolve the target AI-Q backend URL by checkingAIQ_SERVER_URLenvironment variable first, defaulting tohttp://localhost:8000if not set
Runhealthcommand before sending research requests to verify the backend is reachable
Before sending any user query to a non-local AI-Q backend URL, explicitly confirm in conversation that the URL is trusted
Do not transmit API keys, bearer tokens, cookies, or basic-auth credentials throughAIQ_SERVER_URLor query text; store backend credentials in the AI-Q deployment environment instead
Poll asynchronous deep research jobs usingresearch_poll <JOB_ID>when AI-Q returns a job ID in the response
Present returned research reports with citations and source URLs intact; do not truncate or remove source attribution
Stop on failed jobs and show the returned error; do not retry automatically without user guidance
Verify semantic version compatibility: skill major version must match Blueprint major version; Blueprint minor version must be equal or greater than skill minor version
Files:
skills/aiq-research/scripts/aiq.py
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**
⚙️ CodeRabbit configuration file
skills/aiq-research/**: ---
name: aiq-research
description: |
Use when asked to run deep research or AI-Q research through a reachable NVIDIA AI-Q Blueprint backend.
license: Apache-2.0
permissions:
env:
- AIQ_SERVER_URL
network:
- http://localhost:8000
compatibility: |
Designed for Claude Code, OpenCode, Codex, and Agent Skills-compatible tools. Requires Python 3.11+ and network
access to a running local AI-Q Blueprint server athttp://localhost:8000by default. Non-local backends must be
explicitly trusted by the user and granted by the host tool outside this public skill.
metadata:
version: "2.1.0"
author: "NVIDIA AI-Q Blueprint Team aiq-blueprint@nvidia.com"
github-url: "https://github.com/NVIDIA-AI-Blueprints/aiq"
tags:
- nvidia
- aiq
- blueprint
- deep-research
- research-agents
- agent-skills
languages:
- python
- bash
domain: "research-agents"
allowed-tools: Read BashAIQ Research Skill
Purpose
Use this skill to call a locally running NVIDIA AI-Q Blueprint server through the helper script at
scripts/aiq.py.Use this skill for research-shaped requests, including:
- "deep research on ..."
- "AIQ research ..."
- "research ..."
- "use AI-Q to answer ..."
- "ask AI-Q about ..."
Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests. Those
belong toaiq-deploy.Prerequisites
Users need:
- Python 3.11+ available as
python3.- A reachable local or self-hosted AI-Q Blueprint backend.
AIQ_SERVER_URLset when the backend is not running athttp://localhost:8000; non-local values must be trusted by
the user before any query is sent.- A backend configured with authentication disabled for this public helper, or a separate authenticated AI-Q skill for
authenticated environments.- Network access from the local machine to the AI-Q backend URL.
- Credentials configured in the backend environment, not in this skill. Thi...
Files:
skills/aiq-research/scripts/aiq.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyfrontends/aiq_api/tests/test_mcp_auth_factory.py
🪛 ast-grep (0.43.0)
frontends/aiq_api/src/aiq_api/routes/auth.py
[info] 59-59: use jsonify instead of json.dumps for JSON output
Context: json.dumps(source_id)
Note: Security best practice.
(use-jsonify)
skills/aiq-research/scripts/aiq.py
[info] 517-517: use jsonify instead of json.dumps for JSON output
Context: json.dumps(list_data_sources(), indent=JSON_INDENT_SPACES)
Note: Security best practice.
(use-jsonify)
[info] 522-522: use jsonify instead of json.dumps for JSON output
Context: json.dumps(source_auth_status(source_id), indent=JSON_INDENT_SPACES)
Note: Security best practice.
(use-jsonify)
[info] 534-534: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=JSON_INDENT_SPACES)
Note: Security best practice.
(use-jsonify)
🔇 Additional comments (56)
frontends/aiq_api/tests/test_auth.py (1)
900-913: LGTM!frontends/aiq_api/tests/test_job_submit_data_sources.py (1)
621-640: LGTM!frontends/aiq_api/tests/test_mcp_auth_factory.py (1)
1-114: LGTM!frontends/aiq_api/tests/test_mcp_auth_provider.py (1)
1-156: LGTM!frontends/aiq_api/tests/test_mcp_auth_routes.py (1)
1-178: LGTM!frontends/aiq_api/tests/test_submit_mcp_auth_guard.py (1)
1-157: LGTM!frontends/aiq_api/tests/test_submit_owner_user_id.py (1)
1-80: LGTM!frontends/aiq_api/src/aiq_api/jobs/submit.py (1)
215-254: LGTM!frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
250-251: LGTM!Also applies to: 350-356, 479-516
frontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.py (1)
45-128: LGTM!src/aiq_agent/agents/shallow_researcher/register.py (2)
98-168: LGTM!
132-133: No context leakage risk;user_idisolation is handled by ContextVar semantics.Each Dask job runs in its own async task, and NAT's ContextState vars are ContextVars—automatically isolated per task. The user_id is intentionally set once at job start (runner.py line 356) and must persist for the entire execution because the MCP client reads it during tool resolution and calls (per the comment at lines 130–132). No reset is needed between requests; isolation is guaranteed by the task-local nature of ContextVars.
Note: Unlike
job_auth_token(which stores and resets the token in a finally block),user_idis not reset. This is intentional for per-turn persistence, but consider documenting why the reset pattern differs from auth_token for future maintainers.src/aiq_agent/agents/chat_researcher/register.py (1)
258-294: LGTM!src/aiq_agent/agents/chat_researcher/agent.py (1)
282-292: LGTM!frontends/aiq_api/src/aiq_api/routes/auth.py (5)
54-63: Static analysis hint is a false positive.The
ast-grephint about usingjsonifyinstead ofjson.dumpsis Flask-specific. FastAPI doesn't havejsonify; usingjson.dumpsfor embedding JSON in HTML is correct here.
66-71: LGTM!
77-94: LGTM!
96-125: LGTM!
127-158: LGTM!src/aiq_agent/common/data_source_registry.py (4)
67-100: LGTM!
112-112: LGTM!Also applies to: 137-140
171-180: LGTM!Also applies to: 209-209
298-312: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/provider.py (3)
36-44: LGTM!
47-68: LGTM!
71-94: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.py (6)
65-92: LGTM!Also applies to: 95-136
151-169: LGTM!
171-216: LGTM!
218-238: LGTM!
241-272: LGTM!
275-287: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/factory.py (5)
63-80: LGTM!
83-104: LGTM!
155-177: LGTM!
180-224: LGTM!
132-151: NAT internal API usage already documented and defensively handled.The code explicitly marks this as a version-pinned "NAT seam" (line 134,
# noqa: SLF001) with detailed comments explaining the RFC 9728 requirement: MCP servers advertise auth via the 401 WWW-Authenticate header, not the well-known endpoint, so the discovery probe must come before NAT initialization.All private attribute access uses
getattrwithNonedefaults and is wrapped in a try-except that degrades gracefully, falling back toresource = server_url or None. The pinning ofnvidia-nat==1.8.0rc4across all packages demonstrates awareness of the tight version coupling. If NAT's internal API changes in a later release, this will fail safely with a logged warning rather than silently corrupting behavior.No changes needed; the implementation already mitigates the risk appropriately.
frontends/aiq_api/src/aiq_api/auth/middleware.py (3)
196-196: LGTM!
203-211: LGTM!
403-406: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/models.py (1)
20-89: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/active.py (1)
31-44: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/__init__.py (1)
27-49: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/serialize.py (1)
30-68: LGTM!frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
53-54: LGTM!Also applies to: 236-253, 318-326, 346-360, 390-406, 1219-1241, 1349-1357, 1421-1441, 1478-1480, 1535-1539
frontends/ui/src/adapters/api/data-sources-client.ts (1)
22-45: LGTM!Also applies to: 60-61
frontends/ui/src/adapters/api/index.ts (1)
75-87: LGTM!frontends/ui/src/app/api/jobs/async/[...path]/route.ts (1)
45-64: LGTM!Also applies to: 134-135, 230-231
frontends/benchmarks/deepsearch_qa/pyproject.toml (1)
32-32: LGTM!frontends/benchmarks/freshqa/pyproject.toml (1)
32-32: LGTM!frontends/ui/src/features/layout/data-sources.ts (1)
14-44: LGTM!frontends/ui/src/features/layout/components/DataConnectionCard.tsx (1)
13-16: LGTM!Also applies to: 31-43, 47-68, 86-155, 157-187
frontends/ui/src/features/layout/components/DataSourcesPanel.tsx (1)
13-25: LGTM!Also applies to: 38-68, 76-99, 144-180, 214-291, 327-354
skills/aiq-research/scripts/aiq.py (1)
91-103: LGTM!Also applies to: 154-159, 186-196, 271-284, 286-302, 375-395, 517-536, 557-559, 573-578
configs/config_web_frag.yml (2)
114-156: LGTM!Also applies to: 269-280
259-268: Callback path is correctly configured.The
redirect_uriat/v1/auth/mcp/gdrive/callbackmatches the backend route pattern/v1/auth/mcp/{source_id}/callbackregistered infrontends/aiq_api/src/aiq_api/routes/auth.py:128. No path mismatch.
923e43e to
210ca46
Compare
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
cca7d06 to
288a3ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
frontends/ui/src/adapters/api/mcp-auth-client.ts (1)
131-140: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRe-add strict
postMessagetrust checks for popup callbacks.The callback handler accepts any message with matching payload shape. Validate both
event.source === popupand trustedevent.originbefore acceptingmcp-authdata.Suggested fix
+ const trustedOrigin = (() => { + try { + return new URL(authUrl, window.location.href).origin + } catch { + return window.location.origin + } + })() + const onMessage = (event: MessageEvent) => { + if (event.source !== popup) return + if (event.origin !== trustedOrigin) return const data = event.data if (data && data.type === 'mcp-auth' && data.source_id === sourceId) { try { popup.close() } catch {🤖 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 `@frontends/ui/src/adapters/api/mcp-auth-client.ts` around lines 131 - 140, The onMessage callback handler lacks critical security validation for popup communications. Add two essential checks before processing the mcp-auth data: verify that event.source strictly equals the popup window object and validate that event.origin matches a trusted origin (likely the current window's origin). These checks should be added before the existing data.type and data.source_id validation to ensure messages are only accepted from the expected popup window and not from any arbitrary cross-origin message.
🤖 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 `@configs/config_web_frag.yml`:
- Around line 259-267: The server_url field in the mcp_oauth2_gdrive
authentication block contains an NVIDIA-internal endpoint
(maas.prd.astra.nvidia.com) as a default fallback, which will fail for external
users who don't have access to internal services. Remove the fallback default
from the MCP_GDRIVE_URL variable substitution on line 262, changing it to
require explicit configuration via environment variable without a default value,
or replace it with a non-functional placeholder. This ensures external users
must explicitly provide their own MCP endpoint configuration rather than hitting
an unreachable internal default.
In `@frontends/aiq_api/src/aiq_api/mcp_auth/factory.py`:
- Around line 107-174: The function _resolve_oauth_settings relies on multiple
private NAT attributes (_discover_and_register, _cached_endpoints,
_cached_credentials, _effective_scopes, _discoverer) that are currently only
tested through mocks in unit tests rather than against the actual pinned NAT
version (1.8.0). To reduce upgrade risk when NAT updates, either add an
integration test that exercises the discovery flow against the actual pinned NAT
1.8.0 release to validate these private attributes remain compatible, or create
comprehensive documentation listing each private attribute accessed, its
expected type, and its behavior to serve as a reference during future NAT
version upgrades.
In `@frontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.py`:
- Around line 268-272: The _prune_locked method discards expired flows from
self._pending without properly closing the AsyncOAuth2Client objects they
contain, which leaks httpx connection state. When removing expired flows from
self._pending dictionary, extract the flow object and ensure its
AsyncOAuth2Client is closed by calling aclose() on it before the flow is
discarded. Since _prune_locked is synchronous, either schedule the async cleanup
to run outside the lock context or check if authlib provides a synchronous close
method for AsyncOAuth2Client that can be called within this method.
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 508-514: The submit_job endpoint's OpenAPI schema is missing
documentation for the 409 response that can be returned by the
_preflight_mcp_auth function when unconnected protected sources are detected.
Add a 409 response entry to the responses dict (around lines 475-479) that
documents the MCP-auth-required status code and its purpose, ensuring API
consumers have visibility into this possible response through the generated
OpenAPI schema.
In `@frontends/ui/src/features/layout/components/DataConnectionCard.tsx`:
- Around line 107-115: The accessibility state of the card element in
DataConnectionCard needs to be aligned with the canToggle gating logic.
Currently, tabIndex, onClick, and onKeyDown handlers are gated by canToggle, but
the aria-disabled attribute is missing or not properly reflecting this state.
Add an aria-disabled attribute to the card element that reflects when canToggle
is false, so assistive technology accurately communicates that the card is not
interactive when toggle actions are intentionally disabled.
In `@frontends/ui/src/features/layout/components/DataSourcesPanel.tsx`:
- Around line 189-197: The handleToggleAll function enables all available
sources without filtering for protected sources that lack connections, which
bypasses the per-card connect gate. When turning on all sources in the
updatedIds assignment, filter the availableSources array to exclude protected
sources that are not yet connected before mapping to their IDs. This ensures the
bulk toggle respects the connection requirement for protected sources and
prevents invalid UI states.
- Around line 132-160: Add focused unit tests for the DataSourcesPanel component
that cover the new OAuth connect flows. Create tests for the `handleConnect`
callback function to verify it correctly calls `openAuthPopupAndWait` when auth
is required, test error scenarios that trigger the `connectError` state and
verify the failure banner renders with appropriate error messages, and test the
finally block behavior to ensure `fetchDataSources` is always called to refresh
the data source list regardless of success or failure. These tests should
validate the user-visible paths of the OAuth connect flow, error feedback, and
state refresh behavior.
In `@pyproject.toml`:
- Line 204: The prerelease policy in pyproject.toml is currently set to "allow"
which permits any prerelease versions to be pulled. Since the nvidia-nat
packages are now pinned to stable releases, change the prerelease setting from
"allow" to "if-necessary-or-explicit" to tighten the policy and prevent
accidental prerelease package installations. This change narrows the scope of
prerelease acceptance while maintaining flexibility for any future dependencies
that may require prerelease versions.
In `@src/aiq_agent/common/data_source_registry.py`:
- Around line 298-312: The register_tool_sources function unconditionally
overwrites existing entries in _tool_source_map when merging the new mapping
dict at line 311. This causes silent source ownership reassignments when runtime
MCP tool names collide with already-registered tools. Instead of the simple
dictionary merge {**_tool_source_map, **mapping}, iterate through the mapping
entries and selectively insert only keys that don't already exist, or keys where
the existing and new values are identical (idempotent). For any key that would
cause a conflicting remap (exists in _tool_source_map but maps to a different
source value), log a warning about the collision and skip that entry rather than
silently overwriting it.
---
Duplicate comments:
In `@frontends/ui/src/adapters/api/mcp-auth-client.ts`:
- Around line 131-140: The onMessage callback handler lacks critical security
validation for popup communications. Add two essential checks before processing
the mcp-auth data: verify that event.source strictly equals the popup window
object and validate that event.origin matches a trusted origin (likely the
current window's origin). These checks should be added before the existing
data.type and data.source_id validation to ensure messages are only accepted
from the expected popup window and not from any arbitrary cross-origin 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 3cdcd0a3-42b4-4a6f-958e-f55a88fb4284
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
configs/config_web_frag.ymlfrontends/aiq_api/src/aiq_api/auth/middleware.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/active.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_mcp_auth_factory.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/benchmarks/deepsearch_qa/pyproject.tomlfrontends/benchmarks/freshqa/pyproject.tomlfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/features/layout/data-sources.tspyproject.tomlskills/aiq-research/scripts/aiq.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/common/data_source_registry.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Run Harbor skill eval
🧰 Additional context used
📓 Path-based instructions (17)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
frontends/aiq_api/src/aiq_api/mcp_auth/active.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pyfrontends/aiq_api/src/aiq_api/auth/middleware.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/chat_researcher/agent.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/aiq_api/tests/test_mcp_auth_factory.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyskills/aiq-research/scripts/aiq.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pysrc/aiq_agent/common/data_source_registry.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
frontends/aiq_api/src/aiq_api/mcp_auth/active.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/benchmarks/deepsearch_qa/pyproject.tomlsrc/aiq_agent/agents/chat_researcher/register.pyfrontends/benchmarks/freshqa/pyproject.tomlfrontends/ui/src/adapters/api/index.tssrc/aiq_agent/agents/shallow_researcher/register.pypyproject.tomlfrontends/aiq_api/src/aiq_api/auth/middleware.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/chat_researcher/agent.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyconfigs/config_web_frag.ymlfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/aiq_api/tests/test_mcp_auth_factory.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.pyskills/aiq-research/scripts/aiq.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyfrontends/ui/src/features/layout/components/DataConnectionCard.tsxfrontends/aiq_api/src/aiq_api/routes/jobs.pysrc/aiq_agent/common/data_source_registry.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/mcp_auth/active.pyfrontends/aiq_api/src/aiq_api/mcp_auth/preflight.pyfrontends/aiq_api/src/aiq_api/mcp_auth/__init__.pyfrontends/aiq_api/src/aiq_api/mcp_auth/serialize.pyfrontends/aiq_api/src/aiq_api/auth/middleware.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/mcp_auth/models.pyfrontends/aiq_api/src/aiq_api/mcp_auth/provider.pyfrontends/aiq_api/src/aiq_api/routes/auth.pyfrontends/aiq_api/src/aiq_api/mcp_auth/factory.pyfrontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/features/layout/components/DataConnectionCard.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/features/layout/components/DataConnectionCard.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/features/layout/data-sources.tsfrontends/ui/src/adapters/api/data-sources-client.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/features/layout/components/DataConnectionCard.tsx
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/data_source_registry.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/chat_researcher/agent.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
pyproject.toml
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/auth/**,frontends/aiq_api/src/aiq_api/auth/**}: Review authentication changes for issuer/audience validation, token parsing, error hygiene, logging safety,
and compatibility with local and deployed modes. Do not accept changes that expose tokens, weaken validation,
or blur trusted server-side identity with client-supplied fields.
Files:
frontends/aiq_api/src/aiq_api/auth/middleware.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/tests/test_mcp_auth_factory.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.py
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
configs/config_web_frag.yml
skills/aiq-research/**/*.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
Use Python 3.11+ with the helper script at
scripts/aiq.pyto call a locally running NVIDIA AI-Q Blueprint server
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/scripts/aiq.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/scripts/aiq.py: Before sending any user query, state the exact AI-Q backend URL that will receive it; for non-local URLs, continue only if the user has explicitly confirmed that URL is trusted in the current conversation
Do not send credentials, cookies, bearer tokens, or secret values through query text; user query text is transmitted to the configuredAIQ_SERVER_URL
Keep citations and source URLs intact in returned reports and do not truncate them
Runhealthbefore sending research requests to verify the target backend URL is reachable
Poll asynchronous deep research jobs when AI-Q returns a job ID usingresearch_poll <JOB_ID>and do not retry automatically on failed jobs; show the returned error instead
Do not fabricate a research answer if the backend returns HTTP 500, lacks async agents, or experiences other failures; report the failure instead
For follow-up questions answerable from a report already in hand, answer directly from its content and citations without calling the backend again
For follow-up questions needing new investigation, send a fresh request carrying needed context from the prior question and report into the new query text
Use Python standard-library HTTP modules only; the helper script has no third-party Python package dependencies
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**/{SKILL.md,*.py,setup.py,requirements.txt,Dockerfile}
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
Ensure skill version compatibility with Blueprint or endpoint version: major versions MUST match, minor version of Blueprint must be equal or greater, patch version can be anything
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**
⚙️ CodeRabbit configuration file
skills/aiq-research/**: ---
name: aiq-research
description: |
Use when asked to run deep research or AI-Q research through a reachable NVIDIA AI-Q Blueprint backend.
license: Apache-2.0
permissions:
env:
- AIQ_SERVER_URL
network:
- http://localhost:8000
compatibility: |
Designed for Claude Code, OpenCode, Codex, and Agent Skills-compatible tools. Requires Python 3.11+ and network
access to a running local AI-Q Blueprint server athttp://localhost:8000by default. Non-local backends must be
explicitly trusted by the user and granted by the host tool outside this public skill.
metadata:
version: "2.1.0"
author: "NVIDIA AI-Q Blueprint Team aiq-blueprint@nvidia.com"
github-url: "https://github.com/NVIDIA-AI-Blueprints/aiq"
tags:
- nvidia
- aiq
- blueprint
- deep-research
- research-agents
- agent-skills
languages:
- python
- bash
domain: "research-agents"
allowed-tools: Read BashAIQ Research Skill
Purpose
Use this skill to call a locally running NVIDIA AI-Q Blueprint server through the helper script at
scripts/aiq.py.Use this skill for research-shaped requests, including:
- "deep research on ..."
- "AIQ research ..."
- "research ..."
- "use AI-Q to answer ..."
- "ask AI-Q about ..."
Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests. Those
belong toaiq-deploy.Prerequisites
Users need:
- Python 3.11+ available as
python3.- A reachable local or self-hosted AI-Q Blueprint backend.
AIQ_SERVER_URLset when the backend is not running athttp://localhost:8000; non-local values must be trusted by
the user before any query is sent.- A backend configured with authentication disabled for this public helper, or a separate authenticated AI-Q skill for
authenticated environments.- Network access from the local machine to the AI-Q backend URL.
- Credentials configured in the backend environment, not in this skill. Thi...
Files:
skills/aiq-research/scripts/aiq.py
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/scripts/aiq.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_submit_owner_user_id.pyfrontends/aiq_api/tests/test_mcp_auth_provider.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_mcp_auth_routes.pyfrontends/aiq_api/tests/test_mcp_auth_factory.pyfrontends/aiq_api/tests/test_auth.pyfrontends/aiq_api/tests/test_submit_mcp_auth_guard.py
🪛 ast-grep (0.44.0)
frontends/aiq_api/src/aiq_api/routes/auth.py
[info] 59-59: use jsonify instead of json.dumps for JSON output
Context: json.dumps(source_id)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
skills/aiq-research/scripts/aiq.py
[info] 517-517: use jsonify instead of json.dumps for JSON output
Context: json.dumps(list_data_sources(), indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 522-522: use jsonify instead of json.dumps for JSON output
Context: json.dumps(source_auth_status(source_id), indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 534-534: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 GitHub Actions: AIQ CI / 0_Pytest and Coverage.txt
frontends/aiq_api/src/aiq_api/routes/jobs.py
[warning] 361-361: No data sources registered. Add a 'data_sources' function with _type: data_source_registry to your YAML config to enable data source toggles in the UI.
[warning] 412-412: Dask not available - async job submission routes require NAT_DASK_SCHEDULER_ADDRESS and NAT_JOB_STORE_DB_URL
🪛 GitHub Actions: AIQ CI / Pytest and Coverage
frontends/aiq_api/src/aiq_api/routes/jobs.py
[warning] 361-361: No data sources registered. Add a 'data_sources' function with _type: data_source_registry to your YAML config to enable data source toggles in the UI.
[warning] 412-412: Dask not available - async job submission routes require NAT_DASK_SCHEDULER_ADDRESS and NAT_JOB_STORE_DB_URL
🔇 Additional comments (61)
frontends/aiq_api/src/aiq_api/mcp_auth/nat_provider.py (8)
1-93: LGTM!
95-103: LGTM!
105-148: LGTM!
150-169: LGTM!
171-216: LGTM!
218-238: LGTM!
240-266: LGTM!
275-287: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/factory.py (4)
1-60: LGTM!
63-80: LGTM!
83-104: LGTM!
177-221: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/runtime_tools.py (2)
110-111: Private API access already flagged.This concern was raised in a prior review. The
builder._registry.get_tool_wrapper()access is necessary because the publicbuilder.get_tools()API requires pre-registered tool names and cannot wrap dynamically discovered MCP functions.
1-43: LGTM!Also applies to: 45-109, 112-128
frontends/aiq_api/src/aiq_api/jobs/submit.py (1)
31-31: LGTM!Also applies to: 215-229, 253-253
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
250-250: LGTM!Also applies to: 282-284, 350-357, 479-515
src/aiq_agent/agents/shallow_researcher/register.py (1)
98-141: LGTM!Also applies to: 150-164
src/aiq_agent/agents/chat_researcher/register.py (1)
258-268: LGTM!Also applies to: 291-291
src/aiq_agent/agents/chat_researcher/agent.py (1)
282-291: LGTM!skills/aiq-research/scripts/aiq.py (5)
91-104: LGTM!
154-160: LGTM!
186-194: LGTM!
271-303: LGTM!
391-393: LGTM!Also applies to: 517-537, 557-559, 574-578
configs/config_web_frag.yml (1)
114-155: LGTM!Also applies to: 269-280
pyproject.toml (1)
34-38: LGTM!frontends/benchmarks/deepsearch_qa/pyproject.toml (1)
32-32: LGTM!frontends/benchmarks/freshqa/pyproject.toml (1)
32-32: LGTM!frontends/ui/src/adapters/api/data-sources-client.ts (1)
22-43: LGTM!Also applies to: 58-59
frontends/ui/src/adapters/api/index.ts (1)
75-88: LGTM!frontends/ui/src/adapters/api/mcp-auth-client.ts (1)
53-90: LGTM!frontends/ui/src/app/api/jobs/async/[...path]/route.ts (1)
45-65: LGTM!Also applies to: 134-134, 230-230
frontends/ui/src/features/layout/data-sources.ts (1)
14-27: LGTM!Also applies to: 42-43
frontends/ui/src/features/layout/components/DataConnectionCard.tsx (1)
13-85: LGTM!Also applies to: 145-181
frontends/aiq_api/src/aiq_api/mcp_auth/preflight.py (1)
1-103: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/serialize.py (1)
1-69: LGTM!frontends/aiq_api/src/aiq_api/auth/middleware.py (2)
196-211: LGTM!
403-406: LGTM!frontends/aiq_api/src/aiq_api/routes/auth.py (1)
1-159: LGTM!frontends/aiq_api/src/aiq_api/routes/jobs.py (10)
53-53: LGTM!
236-252: LGTM!
317-325: LGTM!
345-358: LGTM!
388-404: LGTM!
1217-1237: LGTM!
1346-1352: LGTM!
1417-1436: LGTM!
1475-1475: LGTM!
1531-1536: LGTM!frontends/aiq_api/tests/test_auth.py (1)
900-913: LGTM!frontends/aiq_api/tests/test_job_submit_data_sources.py (1)
621-640: LGTM!frontends/aiq_api/tests/test_mcp_auth_factory.py (1)
1-114: LGTM!frontends/aiq_api/tests/test_mcp_auth_provider.py (1)
1-155: LGTM!frontends/aiq_api/tests/test_mcp_auth_routes.py (1)
1-177: LGTM!frontends/aiq_api/tests/test_submit_mcp_auth_guard.py (1)
1-157: LGTM!frontends/aiq_api/tests/test_submit_owner_user_id.py (1)
1-80: LGTM!src/aiq_agent/common/data_source_registry.py (1)
51-209: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/provider.py (1)
1-95: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/models.py (1)
1-89: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/active.py (1)
1-44: LGTM!frontends/aiq_api/src/aiq_api/mcp_auth/__init__.py (1)
1-49: LGTM!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontends/ui/src/adapters/api/mcp-auth-client.ts (1)
157-160: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate
postMessagesender before accepting auth completion.Line 159 processes
mcp-authpayloads without confirming the sender window/origin. That allows spoofed messages to prematurely settle the OAuth flow. Requireevent.source === popupand an origin check before readingevent.data.Suggested fix
+ const trustedOrigin = (() => { + try { + return new URL(authUrl, window.location.href).origin + } catch { + return window.location.origin + } + })() + const onMessage = (event: MessageEvent) => { + if (event.source !== popup) return + if (event.origin !== trustedOrigin) return const data = event.data if (data && data.type === 'mcp-auth' && data.source_id === sourceId) { try { popup.close() } catch {As per coding guidelines, "preserve auth-aware UI states." As per path instructions, "Review UI changes for ... auth/session handling."
🤖 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 `@frontends/ui/src/adapters/api/mcp-auth-client.ts` around lines 157 - 160, The onMessage handler in the mcp-auth-client.ts file validates the message data (type and source_id) but does not verify the sender's identity or origin, which creates a security vulnerability where spoofed messages could complete the OAuth flow. Before accepting the mcp-auth payload, add validation to confirm that event.source equals the popup window reference and that the message origin matches the expected origin. These sender and origin checks should be performed before or alongside the existing data type and source_id validation.Sources: Coding guidelines, Path instructions
🤖 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.
Duplicate comments:
In `@frontends/ui/src/adapters/api/mcp-auth-client.ts`:
- Around line 157-160: The onMessage handler in the mcp-auth-client.ts file
validates the message data (type and source_id) but does not verify the sender's
identity or origin, which creates a security vulnerability where spoofed
messages could complete the OAuth flow. Before accepting the mcp-auth payload,
add validation to confirm that event.source equals the popup window reference
and that the message origin matches the expected origin. These sender and origin
checks should be performed before or alongside the existing data type and
source_id validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 12b67d9a-c509-4c4f-ac5f-5d657a5f8615
📒 Files selected for processing (2)
frontends/ui/src/adapters/api/mcp-auth-client.tsfrontends/ui/src/features/layout/components/DataSourcesPanel.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: UI Unit Tests
- GitHub Check: Run Harbor skill eval
🧰 Additional context used
📓 Path-based instructions (4)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/adapters/api/mcp-auth-client.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/adapters/api/mcp-auth-client.ts
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
frontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/adapters/api/mcp-auth-client.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/features/layout/components/DataSourcesPanel.tsxfrontends/ui/src/adapters/api/mcp-auth-client.ts
🔇 Additional comments (1)
frontends/ui/src/features/layout/components/DataSourcesPanel.tsx (1)
144-149: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@deploy/.env.example`:
- Around line 50-59: The REDIS_PASSWORD environment variable is documented in
the deploy/.env.example file but not actually wired through in either the
docker-compose.yaml or values.yaml deployment configurations. Either complete
the password support by: (1) adding REDIS_PASSWORD environment variable to the
redis service definition in docker-compose.yaml and wiring it through to all
backend services that consume it, (2) updating values.yaml to pass
REDIS_PASSWORD as an environment variable to the backend services and
configuring the Redis Helm chart to require password authentication, OR remove
the REDIS_PASSWORD line from deploy/.env.example and add documentation
clarifying that the current Redis deployment is internal-only and does not
support authentication.
In `@deploy/compose/docker-compose.yaml`:
- Around line 145-161: The redis service in the docker-compose configuration
exposes its port to all network interfaces without authentication, creating a
security vulnerability. In the redis service definition, modify the ports
configuration from mapping to all interfaces (${REDIS_PORT:-6379}:6379) to
restrict binding to the loopback interface only by changing it to
127.0.0.1:${REDIS_PORT:-6379}:6379. Alternatively, if unrestricted access is
intentional, add a clear comment documenting that this configuration should only
be used in isolated development environments and not on shared or remotely
accessible machines.
- Around line 49-51: The backend service environment configuration is missing
the REDIS_PASSWORD variable needed for password-authenticated Redis connections.
Add a new environment variable entry following the same pattern as REDIS_HOST
and REDIS_PORT, setting REDIS_PASSWORD with a default value (or empty string if
no default) using the format REDIS_PASSWORD=${REDIS_PASSWORD:-defaultvalue}.
Additionally, if Redis password authentication will be enabled, update the Redis
service configuration to include the requirepass directive in its settings.
In `@deploy/helm/deployment-k8s/values.yaml`:
- Around line 88-90: The backend environment configuration in the values.yaml
file is missing the REDIS_PASSWORD environment variable needed for authenticated
Redis connections in production. Add REDIS_PASSWORD to the environment section
alongside REDIS_HOST and REDIS_PORT (around the same configuration block), and
define it either in the secretEnv section or configure it to be injected from a
Kubernetes Secret to support secure Redis authentication in production
deployments.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 0130b778-7f12-49a0-a8a2-498515cc7a05
📒 Files selected for processing (4)
.secrets.baselinedeploy/.env.exampledeploy/compose/docker-compose.yamldeploy/helm/deployment-k8s/values.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Run Harbor skill eval
🧰 Additional context used
📓 Path-based instructions (2)
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
deploy/helm/deployment-k8s/values.yamldeploy/compose/docker-compose.yaml
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/helm/deployment-k8s/values.yamldeploy/compose/docker-compose.yaml
🔇 Additional comments (4)
deploy/compose/docker-compose.yaml (2)
81-82: LGTM!
145-169: Redis service configuration is sound for local development.AOF persistence, health checks, volume mount, and restart policy are appropriate for a local token store. The service correctly integrates with the Docker network and provides the durability guarantees needed for OAuth token persistence.
deploy/helm/deployment-k8s/values.yaml (1)
203-264: Redis component structure is appropriate for internal development use.The configuration uses
ALLOW_EMPTY_PASSWORDwith a ClusterIP service (internal-only), which is acceptable for dev/test clusters. Persistence, AOF, and health checks provide the durability needed for OAuth token storage. The inline comments correctly warn maintainers about production password requirements.The
pragma: allowlist secreton line 226 is correct for secret-scanning tools..secrets.baseline (1)
163-163: LGTM!Also applies to: 181-181, 293-293
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/aiq_agent/fastapi_extensions/test_deep_research.py`:
- Around line 207-215: The test assertions in the deep_research test are missing
explicit checks for the MCP auth status, connect, and callback routes that are
stated in the comments to remain registered regardless of Dask availability.
After the existing assertions for post_paths and get_paths (checking for the
async/submit, async/agents, and data_sources routes), add additional assertions
to verify that the MCP auth-related routes (such as status, connect, and
callback endpoints) are present in the appropriate get_paths or post_paths lists
to ensure these control-plane routes are properly registered as documented in
the contract.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: a0254393-817c-41b2-9d3e-d11a0c418dcc
📒 Files selected for processing (2)
tests/aiq_agent/fastapi_extensions/test_deep_research.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Run Harbor skill eval
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/fastapi_extensions/test_deep_research.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/fastapi_extensions/test_deep_research.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/fastapi_extensions/test_deep_research.py
🔇 Additional comments (1)
tests/aiq_agent/jobs/test_runner.py (1)
438-439: LGTM!
5abed7b to
71a2adf
Compare
0513e36 to
dc85626
Compare
45ead14 to
05c94e0
Compare
Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…ect, chat, and compose deploy Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…backend starts Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…e collisions, gate bulk toggle, document 409, and add tests Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…n panel open so the UI no longer shows a dead source as connected Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…defaults Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
- routes/auth.py: HTML-escape the provider-controlled OAuth `error` before rendering the callback page (reflected XSS in the AIQ origin). - mcp_auth/sqlite_object_store.py: create the plaintext token DB (and its WAL/SHM sidecars) with 0600 perms instead of the umask default (0644). - mcp_auth/runtime_tools.py: resolve the tool wrapper via the builder's type-registry chain instead of `builder._registry`, which does not exist on ChildBuilder (server mode) and silently dropped per-user MCP tools. - mcp_auth/factory.py: fail closed when two protected sources share one token-storage object store (NAT keys tokens per user only, so sharing one store lets sources overwrite each other's credentials). - ui: do not auto-select an unconnected protected source on initial fetch, new conversation, or restored conversation. Adds regression tests for each. Partially addresses the "unusable source stays selected" finding (load paths only; mid-session refresh reconciliation and backend fail-explicit still TODO). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…ch path Addresses two P1 review findings on the deep-research path for per-user MCP sources (e.g. Google Drive): #6 — validation ran before per-user MCP resolution. `validate_deep_research_tools` filtered a static, startup-built tool list; a per-user MCP source contributes no tools there, so a connected-GDrive-only deep-research request was rejected as "no tools available" and never submitted. It now treats a selected, configured per-user MCP source as a valid runtime tool candidate. Connectivity stays enforced by the submit preflight (evaluate_mcp_auth -> mcp_auth_required when not connected), and the authoritative tool check happens after the worker resolves tools. (Async path; the synchronous deep path still validates the static list and is a follow-up.) #7 — runtime MCP tools leaked into the orchestrator's callable catalog. Source tools were rendered into the orchestrator prompt's "Available Tools" section even though the orchestrator is bound only to helper tools + run_research_batch, so it called them directly and the runtime rejected them. The orchestrator prompt now advertises only its actually-callable tools, and a dedicated orchestrator middleware restricts the tool-name sanitizer allowlist to those tools. Source access stays delegated through run_research_batch to the researcher. Note: #7 changes the shared orchestrator prompt/middleware — recommend a deepresearch eval run before merge to confirm no quality regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
…job time Completes the "unusable protected source stays selected" finding: - ui: refreshDataSourceStatus now drops a protected source from the effective selection when its refreshed status is no longer 'connected' (e.g. token expired mid-session), instead of leaving it shown in "Selected Data Sources" and submitted while unusable. Other selections (incl. sources absent from the refresh response) are preserved. - backend: open_per_user_mcp_tools now fails closed for an EXPLICITLY selected per-user MCP source it cannot resolve (missing/expired token, unreachable server), raising PerUserMcpSourceUnavailableError instead of silently continuing without it. The "all" case (data_sources is None) stays best-effort. Shallow research surfaces the reconnect message; the async worker fails the job. Adds regression tests for both. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
Two develop changes now interact with the per-user-auth submit path: - The internal-agent gate reads `agent_config.public`; the mcp-auth/owner tests mock `get_agent_config` as a SimpleNamespace, so add `public=True`. - develop's `initial_files`/`output_metadata` worker args now sit between `auth_token` and the appended `owner_user_id`, so fix the trailing-arg index assertion (auth_token is now [-4], owner_user_id remains [-1]). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
The per-user MCP auth wiring (the gdrive protected source, the authentication.mcp_oauth2_gdrive provider, and the object_stores.mcp_token_store) was integrated directly into config_web_frag.yml. Extract it into a dedicated config_web_frag_mcp_auth.yml so the base web+RAG config stays minimal, and the feature has a self-contained, opt-in config. - config_web_frag_mcp_auth.yml: web+RAG + per-user MCP auth, public model defaults and generic placeholders (MCP_GDRIVE_URL=your-mcp-server.example.com). - config_web_frag.yml: restored to a clean web+RAG config (no per-user auth). - Repoint the per-user-auth doc references (deploy/.env.example, compose docker-compose.yaml, helm values.yaml) to config_web_frag_mcp_auth.yml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
Re-applies three of the E2E findings from 66d2fb7 (the deep-research orchestrator source-awareness finding is intentionally left as a separate follow-up): - routes/jobs.py: REST submit no longer 422s a runtime-only MCP source. _get_agent_available_source_ids derived availability from static tools only, so a per-user MCP source (no static tools, e.g. gdrive) was rejected before the auth preflight. Configured protected sources are now runtime candidates; connectivity is enforced by the 409 preflight. - routes/jobs.py: catch McpAuthRequiredError from submit_agent_job in the REST route and return the same 409 mcp_auth_required contract, instead of letting a late auth-state change fall through to a generic 500. - mcp-auth-client.ts: openAuthPopupAndWait requires event.source === popup before accepting an mcp-auth message, so an unrelated page can't spoof an auth-complete (status poll remains the fallback if COOP severs the opener). Added a regression test for the untrusted-source case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ashan Panduwawala <apanduwawala@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
d2e955d to
5a0ebe7
Compare
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
|
/nvskills-ci |
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
|
/ok to test 6f6be71 |
Summary
Adds per-user MCP authentication for protected data sources: a user connects a source (e.g. Google Drive via MCP-as-a-Service) through an OAuth popup, AIQ stores that user's token, and async research jobs later call the source's MCP tools as that user using their stored token. Also gates job submission so a job can't start against a protected source the owner hasn't connected, and binds each job to its owner's identity for token lookup.
Why a shared token store (Redis)
The flow spans two separate processes, so an in-memory cache won't work:
They coordinate through a shared, persistent object store keyed per user.
config_web_frag.ymlwires this as a NATobject_storesentry (mcp_token_store). Redis is the default backend (nvidia-nat-redis), but it's interchangeable withs3/mysql— the only requirement is that both processes can reach the same store by key. Tokens are isolated by key: each is stored underprincipal_user_id = "{principal.type}:{principal.sub}"derived from the verified identity, so a worker only ever retrieves the job owner's token.How the auth flow works
Connect (interactive, API process) — new routes in
routes/auth.py:POST /v1/auth/mcp/{source_id}/connect— starts (or resumes) the OAuth flow via the NATmcp_oauth2provider, returns a provider login URL.GET /v1/auth/mcp/{source_id}/status— current per-user status (connected/not_connected/expired/error).GET /v1/auth/mcp/{source_id}/callback— OAuth redirect target; exchanges the code for a token and persists it to the shared store, then serves a small HTML page thatpostMessages the result to the opener and closes.NatMcpAuthProvider(mcp_auth/nat_provider.py) is the control plane over NAT's public OAuth/token storage;factory.pybuilds it from theauthentication.mcp_oauth2_*config block (deriving redirect_uri, scopes, client_id, and the shared token storage). The connect/callback are keyed by an unguessable OAuthstatebound to(principal, source).Middleware (
auth/middleware.py) — only the…/callbackpath is made auth-exempt (the provider redirects the browser back with no AIQ token; it's secured by the OAuthstate).…/statusand…/connectstill require a verified principal.Job time (headless worker) —
mcp_auth/runtime_tools.py::open_per_user_mcp_toolsbuilds the per-user MCP client in code per job (after settingContext.user_idto the job owner), reads the MCP endpoint from the source'smcp_oauth2provider, connects with the owner's stored token, enumerates tools, wraps them for the agent framework, and maps them back to their data source for citation capture. It is deliberately not declared as aper_user_mcp_clientin config, because NAT's interactive (WebSocket) session builder would fail to build it for a user with no token and break interactive chat.Submission gating + owner binding
mcp_auth/preflight.py::evaluate_mcp_authis the single source of truth for "can this job be enqueued given the selected protected sources?" It's enforced at two points so they can't drift: the REST/v1/jobs/async/submitroute (returns a structured 409mcp_auth_requiredwith connect URLs) andsubmit_agent_jobitself (raisesMcpAuthRequiredError), so programmatic submitters like the chat researcher's deep-research path can't bypass the route check.jobs/submit.pynow resolves and passes the verifiedprincipal, and threadsprincipal_user_id(principal)to the worker so job-time token lookup matches the connect-time key.app/api/jobs/async/[...path]/route.ts) now forwards non-2xx JSON bodies verbatim (instead of wrapping them inBACKEND_ERROR), so the UI can read the 409'ssources/auth_urland prompt the user to connect.Frontend
adapters/api/mcp-auth-client.ts— client for status/connect plusopenAuthPopupAndWait(opens the OAuth popup, resolves onpostMessageor popup close, caller re-checks status).DataConnectionCard.tsx— protected sources show a Connect/Reconnect button and a status line instead of the toggle until connected.DataSourcesPanel.tsx— drives the connect flow, refreshes statuses afterward, and surfaces an error banner on failure.data-sources.ts/data-sources-client.ts— carry the newper_user_authmetadata through to the UI.CLI
skills/aiq-research/scripts/aiq.py— adds status/connect handling so the CLI consumer can connect protected sources and see their state.Other
nvidia-nat*from1.8.0rc4to the1.8.0release acrosspyproject.tomlfiles anduv.lock.Test plan
test_mcp_auth_factory.py,test_mcp_auth_provider.py,test_mcp_auth_routes.py,test_submit_mcp_auth_guard.py,test_submit_owner_user_id.py, plus additions totest_auth.pyandtest_job_submit_data_sources.py.uv lockcompleted successfully after the rebase ontodevelop.Summary by CodeRabbit
Summary