feat(mcp): add public AI-Q MCP server#319
Conversation
Add an anonymous MCP transport for submitting and polling AI-Q research jobs, backed by PostgreSQL. Include deployment assets, documentation, compatibility tests, release checks, and CI gates. Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
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. |
WalkthroughThis PR adds a standalone MCP server with Postgres-backed job orchestration, checkpoint todo reads, Streamable HTTP runtime wiring, Docker/Compose deployment, release validation scripts, isolated CI, and documentation. It also adds hashed identifier logging and sanitizes selected persisted errors. ChangesAI-Q MCP Server
Workspace, CI, and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Log identifier redaction
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPRuntime
participant JobManager
participant JobStore
participant WorkflowRunner
Client->>MCPRuntime: submit_query(query)
MCPRuntime->>JobManager: submit(query, anonymous)
JobManager->>WorkflowRunner: classify(query)
WorkflowRunner-->>JobManager: depth and intent
JobManager->>JobStore: create queued job
JobManager->>WorkflowRunner: run_query(query, conversation_id)
WorkflowRunner-->>JobManager: result or failure
JobManager->>JobStore: update terminal state
Client->>MCPRuntime: poll_query(job_id)
MCPRuntime->>JobManager: poll(job_id, anonymous)
JobManager->>JobStore: record_poll(job_id, anonymous)
Client->>MCPRuntime: get_final_report(job_id)
MCPRuntime->>JobManager: get_final_report(job_id, anonymous)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Require NLTK 3.10, constrain its new defusedxml dependency to the stable 0.7 release line, and refresh the lockfile. Remove the resolved NLTK vulnerability exceptions from the MCP audit gate and update the release security documentation. Signed-off-by: Tanner Leach <tleach@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
91-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin
uvversion consistently across jobs.The
testjob'sSetup uvstep has noversion:pin, whiletest-scripts(Line 186) pins0.11.26. Different jobs in the same workflow run can silently resolve differentuvversions, undermining the reproducibility the version pin intest-scriptsis meant to guarantee.As per path instructions,
.github/**changes should be reviewed for "reproducible uv/npm setup."♻️ Proposed fix
- name: Setup uv uses: astral-sh/setup-uv@v4 with: enable-cache: true + version: "0.11.26"Also applies to: 182-186
🤖 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 @.github/workflows/ci.yml around lines 91 - 95, The Setup uv steps in the workflow are inconsistent because the test job uses astral-sh/setup-uv@v4 without a version pin while the test-scripts job already pins uv, so align both jobs to the same fixed uv version. Update the Setup uv configuration in the workflow so the test and test-scripts jobs use the same version value, keeping the version reference attached to the astral-sh/setup-uv@v4 step.Source: Path instructions
mcp/tests/conftest.py (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid relying on pytest’s importlib namespace setup. This shim is tied to
--import-mode=importlibpackage resolution, which changed in pytest 8.2; if collection behavior shifts or the mode changes,mcpcan be shadowed again. A less order-dependent import path would make this safer.🤖 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 `@mcp/tests/conftest.py` around lines 1 - 19, The pytest collection shim in conftest.py is too dependent on importlib-mode namespace behavior, so replace the current sys.modules cleanup plus importlib.import_module("mcp.server.fastmcp") approach with a more direct, order-independent way to ensure the installed MCP package is imported. Update the logic around protocol_package and the mcp module handling so it explicitly resolves the dependency package before tests run, avoiding reliance on pytest’s collection-created namespace module. Keep the fix localized to the conftest.py import bootstrap path and preserve the intended fastmcp import without depending on collection order.
🤖 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 @.github/workflows/ci.yml:
- Line 68: The CI workflow uses mutable references for the postgres service
image and the upload-artifact actions, so update the workflow to pin those
dependencies to immutable digests/SHA references. In the workflow definition,
replace the postgres service reference and both uses of actions/upload-artifact
with digest-pinned versions, keeping the existing job behavior unchanged while
making the references reproducible and safer to review.
In `@mcp/src/aiq_mcp/jobs.py`:
- Around line 344-349: The `_heartbeat_job` loop should not die on a transient
`self._store.heartbeat(job_id, self._runner_id)` failure; wrap the heartbeat
call in exception handling similar to `_reconcile_jobs_periodically` so the task
keeps running and logs the error. Add a broad catch inside `_heartbeat_job` with
a retry-after-sleep behavior, and make sure the `JobRunner` cleanup path in
`_run_job` still discards the job from `_FAILURE_HANDLER` even if the heartbeat
task ends unexpectedly.
In `@mcp/src/aiq_mcp/server.py`:
- Around line 331-394: `submit_query` currently accepts unlimited anonymous
requests with an unbounded `query` string, so add abuse controls around this
public entrypoint. Enforce a maximum query length before calling
`_get_jobs().submit`, and add a per-IP or global submission rate limit for the
unauthenticated flow, preferably in the MCP server stack (for example via
Starlette middleware or upstream proxy/ingress). Use `submit_query`,
`_get_jobs`, and the anonymous-principal path as the key locations to update.
In `@mcp/tests/test_client_session_integration.py`:
- Around line 416-434: The skip/warning text in the phase6_postgres_url fixture
is embedding the raw exception string, which can leak DSN or credential-related
details. Update the exception handling around _ensure_database and _reset_schema
to use only the exception type name from asyncpg.PostgresError/OSError instead
of interpolating exc directly, while keeping the existing pytest.skip and
warnings.warn behavior and the same fixture flow.
In `@src/aiq_agent/common/logging_utils.py`:
- Around line 9-12: The log_identifier_ref helper currently uses plain SHA-256,
which is deterministic but easier to brute-force for low-entropy identifiers.
Update log_identifier_ref in logging_utils to use a keyed hash such as HMAC with
a server-side secret while preserving the same stable output contract and
sha256:-prefixed 12-character hex format. Keep the existing test expectations in
mind: deterministic for the same input, does not reveal the original identifier,
and still returns exactly 12 hex characters after the prefix.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 91-95: The Setup uv steps in the workflow are inconsistent because
the test job uses astral-sh/setup-uv@v4 without a version pin while the
test-scripts job already pins uv, so align both jobs to the same fixed uv
version. Update the Setup uv configuration in the workflow so the test and
test-scripts jobs use the same version value, keeping the version reference
attached to the astral-sh/setup-uv@v4 step.
In `@mcp/tests/conftest.py`:
- Around line 1-19: The pytest collection shim in conftest.py is too dependent
on importlib-mode namespace behavior, so replace the current sys.modules cleanup
plus importlib.import_module("mcp.server.fastmcp") approach with a more direct,
order-independent way to ensure the installed MCP package is imported. Update
the logic around protocol_package and the mcp module handling so it explicitly
resolves the dependency package before tests run, avoiding reliance on pytest’s
collection-created namespace module. Keep the fix localized to the conftest.py
import bootstrap path and preserve the intended fastmcp import without depending
on collection order.
🪄 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: cbcc9fa2-3a60-49b5-a035-04b70d24b653
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
.dockerignore.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineLICENSE-THIRD-PARTYREADME.mdconfigs/config_mcp.ymldeploy/compose/docker-compose.mcp.yamldocs/source/index.mddocs/source/integration/index.mddocs/source/integration/mcp-server.mdmcp/Dockerfilemcp/LICENSEmcp/README.mdmcp/REFERENCE_PARITY.mdmcp/SECURITY.mdmcp/deploy/README.mdmcp/deploy/init-mcp-db.sqlmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/scripts/check_runtime_dependencies.pymcp/scripts/protocol_smoke.pymcp/src/aiq_mcp/__init__.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/src/aiq_mcp/db_url.pymcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/workflow_runner.pymcp/tests/conftest.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_db_url.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_imports.pymcp/tests/test_job_store.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_release_checks.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_server_runtime.pymcp/tests/test_workflow_runner.pypyproject.tomlsrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/common/logging_utils.pysrc/aiq_agent/knowledge/summary_store.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/common/test_logging_utils.pytests/knowledge_layer_tests/test_summary_store.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Script Validation
- GitHub Check: Pytest and Coverage
⚠️ CI failures not shown inline (2)
GitHub Actions: Skills Eval / Run Harbor skill eval: fix(mcp): upgrade NLTK security baseline
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
GitHub Actions: Skills Eval / 0_Run Harbor skill eval.txt: fix(mcp): upgrade NLTK security baseline
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
🧰 Additional context used
📓 Path-based instructions (11)
**
⚙️ 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:
mcp/deploy/README.mddocs/source/integration/index.mdmcp/src/aiq_mcp/__init__.pytests/aiq_agent/common/test_logging_utils.pymcp/README.mdtests/aiq_agent/agents/chat_researcher/test_register_helpers.pyLICENSE-THIRD-PARTYmcp/LICENSEmcp/REFERENCE_PARITY.mdmcp/tests/conftest.pydocs/source/index.mdmcp/tests/test_db_url.pymcp/src/aiq_mcp/db_url.pymcp/tests/test_job_store.pyREADME.mdmcp/scripts/check_runtime_dependencies.pymcp/tests/test_runtime_dependencies.pymcp/pyproject.tomlsrc/aiq_agent/common/logging_utils.pymcp/tests/test_imports.pymcp/tests/test_dependency_compatibility.pymcp/SECURITY.mdmcp/Dockerfilemcp/tests/test_workflow_runner.pymcp/scripts/protocol_smoke.pytests/knowledge_layer_tests/test_summary_store.pymcp/tests/test_deployment_assets.pymcp/deploy/init-mcp-db.sqlsrc/aiq_agent/agents/chat_researcher/register.pyconfigs/config_mcp.ymldocs/source/integration/mcp-server.mdmcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_client_session_integration.pypyproject.tomlmcp/src/aiq_mcp/checkpoint_todos.pymcp/scripts/check_license_inventory.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/jobs.pydeploy/compose/docker-compose.mcp.yamlmcp/tests/test_jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/integration/index.mddocs/source/index.mddocs/source/integration/mcp-server.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/integration/index.mddocs/source/index.mdREADME.mddocs/source/integration/mcp-server.md
**/*.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:
mcp/src/aiq_mcp/__init__.pytests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/src/aiq_mcp/db_url.pymcp/tests/test_job_store.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_runtime_dependencies.pysrc/aiq_agent/common/logging_utils.pymcp/tests/test_imports.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_workflow_runner.pymcp/scripts/protocol_smoke.pytests/knowledge_layer_tests/test_summary_store.pymcp/tests/test_deployment_assets.pysrc/aiq_agent/agents/chat_researcher/register.pymcp/src/aiq_mcp/workflow_runner.pysrc/aiq_agent/knowledge/summary_store.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_client_session_integration.pymcp/src/aiq_mcp/checkpoint_todos.pymcp/scripts/check_license_inventory.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.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:
.pre-commit-config.yamlpyproject.toml.github/workflows/ci.yml
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/common/test_logging_utils.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pymcp/tests/conftest.pymcp/tests/test_db_url.pymcp/tests/test_job_store.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_imports.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_workflow_runner.pytests/knowledge_layer_tests/test_summary_store.pymcp/tests/test_deployment_assets.pymcp/tests/test_release_checks.pymcp/tests/test_client_session_integration.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.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/common/logging_utils.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/knowledge/summary_store.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.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_mcp.ymldeploy/compose/docker-compose.mcp.yaml
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
src/aiq_agent/knowledge/summary_store.py
**/*config*.py
📄 CodeRabbit inference engine (AGENTS.md)
Config schemas must inherit from FunctionBaseConfig and YAML _type names must come from the registered config class
Files:
mcp/tests/test_config_and_packaging.py
🪛 ast-grep (0.44.1)
mcp/scripts/check_runtime_dependencies.py
[info] 89-89: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/scripts/protocol_smoke.py
[info] 137-137: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/scripts/check_license_inventory.py
[info] 171-171: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload, sort_keys=True, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 418-418: use jsonify instead of json.dumps for JSON output
Context: json.dumps(inventory, indent=2, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 419-419: use jsonify instead of json.dumps for JSON output
Context: json.dumps(validation, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/src/aiq_mcp/server.py
[warning] 66-66: Do not make http calls without encryption
Context: "http://[::1]:*"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 67-67: Do not make http calls without encryption
Context: "http://0.0.0.0:*"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
mcp/tests/test_server_runtime.py
[warning] 479-479: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 546-546: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 559-559: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 566-566: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 628-628: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 640-640: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 645-645: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 661-661: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 695-695: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 710-710: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 721-721: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://untrusted.example"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 933-933: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 721-721: Do not make http calls without encryption
Context: "http://untrusted.example"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[info] 400-400: use jsonify instead of json.dumps for JSON output
Context: json.dumps(schema_contract, sort_keys=True, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Checkov (3.3.2)
mcp/Dockerfile
[low] 10-10: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
🪛 GitHub Actions: Skills Eval / Run Harbor skill eval
mcp/Dockerfile
[error] 79-79: Docker build failed at step [builder 14/17]: uv sync --frozen --extra s3. Error: "Failed to determine installation plan". Caused by: "Distribution not found at: file:///app/mcp". Exit code: 2.
🪛 Hadolint (2.14.0)
mcp/Dockerfile
[warning] 16-16: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[warning] 65-65: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
🪛 OpenGrep (1.23.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 121-137: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 142-153: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 157-168: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 192-195: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 239-239: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 244-255: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 259-259: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 260-269: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 270-288: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 289-291: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 292-294: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 295-295: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 296-298: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 299-301: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 302-310: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 311-319: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_client_session_integration.py
[ERROR] 491-491: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_checkpoint_todos.py
[ERROR] 426-426: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
mcp/tests/test_postgres_job_store.py
[ERROR] 484-484: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 SQLFluff (4.2.2)
mcp/deploy/init-mcp-db.sql
[error] 43-43: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 44-44: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 45-45: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 46-46: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
🪛 zizmor (1.26.1)
.github/workflows/ci.yml
[error] 68-68: unpinned image references (unpinned-images): container image is not pinned to a SHA256 hash
(unpinned-images)
[error] 263-263: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 292-292: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (59)
src/aiq_agent/agents/chat_researcher/register.py (1)
34-34: LGTM!Also applies to: 61-65, 561-574, 618-633, 645-648
src/aiq_agent/knowledge/summary_store.py (1)
30-31: LGTM!Also applies to: 108-108, 135-135, 153-153, 264-270, 286-290, 309-313, 328-334, 347-353, 365-365
tests/aiq_agent/agents/chat_researcher/test_register_helpers.py (1)
18-19: LGTM!Also applies to: 28-40
tests/aiq_agent/common/test_logging_utils.py (1)
1-20: LGTM!tests/knowledge_layer_tests/test_summary_store.py (1)
25-32: LGTM!Also applies to: 218-232
.secrets.baseline (1)
136-136: LGTM!Also applies to: 354-358
mcp/tests/test_deployment_assets.py (2)
91-98: Hash-based schema assertion is reasonably backed by explicit content checks.The SHA-256 comparison against
_EXPECTED_SQL_HASHcould in isolation be "fixed" by simply recomputing the hash after any change, but lines 95-97 also assert specific structural content (mcp_jobstable, index name, migration version rows), which mitigates the rubber-stamp risk. No action needed.
1-155: LGTM!mcp/Dockerfile (2)
10-96: LGTM!
47-58: 🩺 Stability & AvailabilityWrong Dockerfile: the reported
uv sync --frozen --extra s3failure is fromdeploy/Dockerfile:79, notmcp/Dockerfile;mcp/Dockerfiledoesn’t pass--extra s3, and line 79 there isENV HOME=/home/aiq.> Likely an incorrect or invalid review comment.deploy/compose/docker-compose.mcp.yaml (3)
53-53: 🎯 Functional Correctness | ⚡ Quick winInconsistent shell parameter-expansion operator for
AIQ_MCP_CORS_ORIGINS.Every other overridable variable in this service uses
${VAR:-default}(unset-or-empty falls back to default), but this one uses${VAR-default}(only unset falls back; an explicitly empty value is kept as-is). This is inconsistent with the pattern used forAIQ_MCP_PATH,AIQ_MCP_WORKERS,AIQ_MCP_LOG_LEVEL,AIQ_MCP_ALLOWED_HOSTS, andAIQ_MCP_ALLOWED_ORIGINSon the surrounding lines. If this asymmetry is intentional (e.g., to let users explicitly disable CORS by setting an empty string), it should be commented; otherwise it looks like a typo.🔧 Proposed fix
- AIQ_MCP_CORS_ORIGINS: "${AIQ_MCP_CORS_ORIGINS-http://localhost:6274}" + AIQ_MCP_CORS_ORIGINS: "${AIQ_MCP_CORS_ORIGINS:-http://localhost:6274}"
17-22: 🔒 Security & Privacy | ⚡ Quick winPostgres password/checkpoint DB URL has no override mechanism.
Every other configurable value in the
aiq-mcpservice uses the${VAR:-default}pattern so operators can override without editing the file, butPOSTGRES_PASSWORDand the embedded password inAIQ_CHECKPOINT_DBare hardcoded literals. The comment on lines 18-20 explains why a raw password isn't simply interpolated (percent-encoding of reserved URI characters), which is a fair constraint, but it still means anyone reusing this compose file beyond pure loopback-only local testing must hand-edit the YAML to change the credential rather than setting an env var. Since this is adeploy/**config, consider at least parameterizing the default (e.g.,${AIQ_MCP_POSTGRES_PASSWORD:-local_mcp_password}on both lines) so the override path exists even if percent-encoding correctness is the caller's responsibility.As per path instructions, deployment/config changes should be reviewed for "secret separation, safe defaults, local-vs-production behavior" and should flag "committed credentials."
Also applies to: 44-46
Source: Path instructions
1-16: LGTM!Also applies to: 23-43, 47-91
mcp/pyproject.toml (2)
27-38: Version ranges verified as currently valid.Checked
starlette>=1.0,<2anduvicorn>=0.40,<1against current PyPI releases (Starlette is at 1.3.x, Uvicorn is at 0.50.x as of mid-2026), so both ranges resolve correctly and don't exceed real published versions. The exact pin onmcp==1.27.2is also a safe choice given the upstream SDK's own guidance to add a<2upper bound ahead of the v2.0.0 stable release targeted for 2026-07-28.
1-53: LGTM!mcp/src/aiq_mcp/__init__.py (1)
1-7: LGTM!mcp/tests/test_config_and_packaging.py (1)
1-139: LGTM!mcp/tests/test_imports.py (1)
1-67: LGTM!mcp/deploy/init-mcp-db.sql (1)
18-54: LGTM!mcp/deploy/README.md (1)
1-12: LGTM!mcp/scripts/check_license_inventory.py (1)
297-397: LGTM!mcp/tests/test_release_checks.py (1)
1-204: LGTM!mcp/REFERENCE_PARITY.md (1)
1-85: LGTM!LICENSE-THIRD-PARTY (1)
8-11: LGTM!.dockerignore (1)
64-71: 🚀 Performance & ScalabilityKeep
frontends/benchmarks/in the build context.mcp/Dockerfilecopiesfrontends/benchmarks/freshqa/pyproject.tomlandfrontends/benchmarks/deepsearch_qa/pyproject.toml, so re-ignoring that directory would break the build.> Likely an incorrect or invalid review comment.mcp/scripts/check_runtime_dependencies.py (1)
17-96: LGTM!mcp/tests/test_runtime_dependencies.py (1)
1-76: LGTM!mcp/LICENSE (1)
1-202: LGTM!.github/workflows/ci.yml (2)
101-165: LGTM!Also applies to: 208-216, 236-300
217-234: 🩺 Stability & AvailabilityDrop this comment
uv audit --preview-features audit-command,json-outputand--ignore-until-fixedare supported by the pinneduvversion, and the workflow matches the documented invocation.> Likely an incorrect or invalid review comment..pre-commit-config.yaml (1)
70-75: LGTM!pyproject.toml (1)
51-51: LGTM!Also applies to: 101-101, 158-158, 211-212, 214-218, 227-228, 243-243, 257-257
mcp/tests/test_dependency_compatibility.py (2)
33-90: LGTM!
26-30: 🗄️ Data Integrity & Integration
nvidia-natis pinned at the workspace level
pyproject.tomlalready pinsnvidia-nat[langchain,async_endpoints,phoenix,mcp]==1.8.0, so theversion("nvidia-nat")assertion is consistent with the compatibility baseline.> Likely an incorrect or invalid review comment.mcp/scripts/protocol_smoke.py (2)
70-83: 🩺 Stability & Availability | ⚡ Quick winVerify the
ClientSessioncontract before relying on this smoke test.
read_timeout_secondsis passed atimedelta, and the stateless-session assertion depends onget_session_id()semantics. If the pinned MCP SDK expects a numeric timeout or reports a client-side session ID for streamable HTTP, this will fail before it exercises the server contract. Please confirm against the version shipped in this repo and adjust the timeout type or assertion if needed.
1-69: LGTM!Also applies to: 84-143
mcp/SECURITY.md (1)
1-120: LGTM!README.md (1)
41-41: LGTM!Also applies to: 244-244, 305-319
docs/source/index.md (1)
76-76: LGTM!docs/source/integration/index.md (1)
12-12: LGTM!docs/source/integration/mcp-server.md (1)
1-251: LGTM!mcp/README.md (1)
1-167: LGTM!mcp/src/aiq_mcp/job_store.py (2)
121-319: 🔒 Security & PrivacyStatic-analysis SQL-injection hints are false positives.
All f-string interpolations here are table/schema identifiers produced by
_quote_identifier(regex-validated, Line 352-355) — never raw user input. Actual values (job_id, principal, query, etc.) are always passed as$nbind parameters. No injection risk.Source: Linters/SAST tools
1-355: LGTM! Schema migration/versioning, poll/heartbeat reconciliation, and TTL sweep semantics look correct and are exercised by the Postgres integration tests.mcp/src/aiq_mcp/db_url.py (1)
1-21: LGTM!mcp/src/aiq_mcp/checkpoint_todos.py (1)
1-271: LGTM! Fail-soft error handling, redacted logging of the thread capability id, and the latest-todo CTE query all check out against the accompanying tests.mcp/tests/test_db_url.py (1)
1-29: LGTM!mcp/tests/test_job_store.py (1)
1-21: LGTM!mcp/tests/test_checkpoint_todos.py (1)
1-478: LGTM! Solid coverage of the redaction, fail-soft, and namespace-exclusion contracts thatcheckpoint_todos.pyrelies on.mcp/tests/test_postgres_job_store.py (1)
1-509: LGTM! Thorough integration coverage for reconciliation and multi-instance job handoff semantics.configs/config_mcp.yml (1)
1-109: LGTM! Matches the shipped-config contract test exactly, and secrets are correctly deferred to env vars.mcp/src/aiq_mcp/jobs.py (1)
1-343: LGTM!Also applies to: 350-506
mcp/src/aiq_mcp/workflow_runner.py (2)
1-50: LGTM!Also applies to: 90-142
51-89: 🎯 Functional CorrectnessConfirm
load_workflow()doesn’t already own these cachesmcp/src/aiq_mcp/workflow_runner.py:51-89closes checkpointers and pools afterAsyncExitStack.aclose(). If NAT teardown already disposes the same shared_checkpointers/_postgres_poolsentries, this becomes a double-close on shutdown.mcp/tests/test_jobs.py (1)
1-906: LGTM! Thorough golden-contract and redaction coverage; consider adding a case for a heartbeat-write failure mid-run once the_heartbeat_jobfix (see jobs.py) lands.mcp/tests/test_workflow_runner.py (1)
1-170: LGTM!mcp/src/aiq_mcp/server.py (1)
1-330: LGTM!Also applies to: 396-567
mcp/tests/test_server_runtime.py (1)
1-992: LGTM!mcp/tests/test_client_session_integration.py (1)
1-415: LGTM!Also applies to: 435-516
| def log_identifier_ref(identifier: str) -> str: | ||
| """Return a stable correlation reference that does not reveal ``identifier``.""" | ||
| digest = hashlib.sha256(identifier.encode("utf-8")).hexdigest() | ||
| return f"sha256:{digest[:12]}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
Consider keyed hashing for defense-in-depth.
Plain unsalted SHA-256 is deterministic and reversible via offline dictionary lookup if an identifier has low entropy (not an issue for the current UUID call sites, but a caveat for future callers with predictable inputs). An HMAC with a server-side secret would prevent brute-force correlation while keeping the same stability/testability contract.
Test asserts log_identifier_ref output is deterministic, sha256:-prefixed, exactly 12 hex chars long, and does not include the original identifier.
🤖 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 `@src/aiq_agent/common/logging_utils.py` around lines 9 - 12, The
log_identifier_ref helper currently uses plain SHA-256, which is deterministic
but easier to brute-force for low-entropy identifiers. Update log_identifier_ref
in logging_utils to use a keyed hash such as HMAC with a server-side secret
while preserving the same stable output contract and sha256:-prefixed
12-character hex format. Keep the existing test expectations in mind:
deterministic for the same input, does not reveal the original identifier, and
still returns exactly 12 hex characters after the prefix.
Add report follow-up queries to skills. This PR replaces NVIDIA-AI-Blueprints#312 as the branch needs to be on the main repo, not a fork. This PR also fixes a bug where the agent fails to return a report and adds an optional port parameter to the `start_e2e` script Validated on CLI, Claude, and Codex Tests to be added under a follow-up task - [X] I ran the relevant local checks or explained why they are not applicable. - [ ] I added or updated tests for behavior changes. - [X] I updated documentation for user-facing or contributor-facing changes. - [X] I confirmed this PR does not include secrets, credentials, or internal-only data. - [X] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an `--port` option to the end-to-end startup script, updating the backend URL and readiness checks to use the selected port. * Added a `report_edit` command to submit cosmetic report-edit instructions for an existing AIQ job. * **Bug Fixes** * Improved final markdown extraction to correctly use pre-prepared state files when present. * **Documentation** * Updated the AIQ research skill guide and references to include the new **Edit** option and script details. * Refreshed benchmark metadata and revised the skill card evaluation details. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Peter Chang <pechang@nvidia.com> Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com> Co-authored-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com> Co-authored-by: Ajay Thorve <AjayThorve@users.noreply.github.com> Signed-off-by: Tanner Leach <tleach@nvidia.com>
## Summary - preserve the active `ToolRuntime` callback context for batched researcher invocations - name each nested researcher run `researcher-agent` so existing workflow events and tool `agent_id` attribution work - verify concurrent researcher cards keep tool calls isolated and update the Agents tab copy for Source Router ## Root cause `run_research_batch` replaced the active runtime callback manager with the raw callback list and invoked the researcher runnable without a stable run name. Researcher lifecycle events were therefore absent, and nested search/tool events could not be attributed to a researcher in the UI. ## Impact Each concurrent researcher invocation now emits the existing `workflow.start`/`workflow.end` contract with a distinct agent ID. Its nested tools inherit that ID, allowing the current UI grouping logic to display researcher activity without a new event schema or hierarchy model. <img width="1690" height="930" alt="image" src="https://github.com/user-attachments/assets/b25c7821-3ace-4e7d-aec2-b6e486f72f74" /> ## Validation - `uv run pytest -q tests/aiq_agent/agents/deep_researcher/test_agent.py tests/aiq_agent/agents/deep_researcher/test_factory.py tests/aiq_agent/jobs/test_runner.py` (`153 passed`) - `uv run ruff check src/aiq_agent/agents/deep_researcher/tools/research.py tests/aiq_agent/agents/deep_researcher/test_agent.py` - `uv run ruff format --check src/aiq_agent/agents/deep_researcher/tools/research.py tests/aiq_agent/agents/deep_researcher/test_agent.py` - `npm run test:ci -- src/features/layout/components/AgentsTab.spec.tsx` (`8 passed`) - `npx tsc --noEmit` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit * **New Features** * Updated the Agents tab description to additionally list **“Active source router”** alongside planner, researcher, and writer. * **Bug Fixes** * Fixed concurrent researcher display so each agent card shows only its own tool calls—even when agents share the same visible name. * Improved activity tracking/attribution for nested researcher runs to ensure workflow and tool events are consistently attributed to the correct agent. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ajay Thorve <athorve@nvidia.com> Signed-off-by: Tanner Leach <tleach@nvidia.com>
## Summary - emit named NAT workflow spans for DeepAgents sub-agents so their model and tool activity is grouped correctly - replace NAT's inherited LangChain profiler for the async invocation instead of installing a duplicate profiler callback - preserve the existing cross-process parent bridge so completed deep-research traces remain under the originating workflow ## Root cause AIQ manually attached a second `LangchainProfilerHandler` even though NAT already installs an inheritable handler through its framework wrapper. Both handlers emitted events for the same LangChain run IDs, producing overwritten or orphaned span relationships. DeepAgents chain callbacks also lacked explicit named NAT spans for sub-agents. ## User impact Completed Phoenix traces now show `deep_research_agent -> task: <sub-agent> -> <sub-agent> -> model/tools`, nested beneath the original chat workflow. UI agent events remain unchanged. ## Before / After | Before | After | |---|---| | <img width="544" alt="Before: orphaned sub-agent spans" src="https://github.com/user-attachments/assets/cd920a33-68bf-43a4-9264-3afb09d8fb42" /> | <img width="441" alt="After: nested sub-agent hierarchy" src="https://github.com/user-attachments/assets/a56d8b89-12ec-4e71-84ed-5cbaa6cd6b94" /> | | Sub-agent spans appeared orphaned at the trace root. | Sub-agent activity is nested under `deep_research_agent → task → sub-agent`. | ## Validation - `uv run pytest tests/aiq_agent/jobs/test_telemetry.py tests/aiq_agent/jobs/test_runner.py tests/aiq_agent/agents/deep_researcher/test_agent.py tests/aiq_agent/agents/deep_researcher/test_factory.py -q` (156 passed) - `uv run ruff check frontends/aiq_api/src/aiq_api/jobs/runner.py frontends/aiq_api/src/aiq_api/jobs/telemetry.py tests/aiq_agent/jobs/test_telemetry.py` - `uv run ruff format --check frontends/aiq_api/src/aiq_api/jobs/runner.py frontends/aiq_api/src/aiq_api/jobs/telemetry.py tests/aiq_agent/jobs/test_telemetry.py` - manually validated a completed deep-research run in Phoenix <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved agent job tracing with clearer workflow and task-related span naming. * Added lifecycle telemetry for agent execution, emitting consistent workflow start/end events with agent identity metadata. * **Bug Fixes** * Fixed profiler integration to prevent duplicate callback handling while maintaining existing profiling behavior. * Corrected span nesting/parent relationships for nested and parallel agent runs, and ensured lifecycle telemetry does not capture graph state details. * **Tests** * Added coverage for agent lifecycle telemetry, nested/parallel span parenting, lifecycle identity matching, and profiler context swap/restore behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Ajay Thorve <athorve@nvidia.com> Signed-off-by: Tanner Leach <tleach@nvidia.com>
…A-AI-Blueprints#309) #### Overview The aiq.namespace helper rendered ns-<appname> (e.g. ns-aiq) for every resource, ignoring the install namespace. This broke helm install -n and GitOps operators (ArgoCD, Fleet) that don't pre-create ns-aiq, failing with 'namespaces "ns-aiq" not found'. Default the helper to .Release.Namespace and add an optional namespace.name override to pin a specific namespace. Align namespace.yaml to use the same helper. Bump chart versions (aiq 0.0.4->0.0.5, aiq2-web 2.0.0->2.0.1) and refresh the vendored dependency package. #### Validation - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. - [x] I updated documentation for user-facing or contributor-facing changes. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. #### Where should reviewers start? <!-- Point to the most important file, test, or design decision. --> #### Related Issues Fixes NVIDIA-AI-Blueprints#290 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated Helm chart namespace handling so all namespaced resources consistently use the Helm release namespace. * **Tests** * Added a regression test ensuring all rendered manifests that include `metadata.namespace` match the requested release namespace. * **Documentation** * Clarified in chart values that GitOps operators work with resources using the Helm release namespace. * **Chores** * Bumped Helm chart versions and the related dependency version. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: e-minguez <e.minguez@gmail.com> Co-authored-by: Ajay Thorve <AjayThorve@users.noreply.github.com> Signed-off-by: Tanner Leach <tleach@nvidia.com>
#### Overview Extract the provider-neutral artifact lifecycle and UI delivery work from NVIDIA-AI-Blueprints#305 onto current `develop`, so it can land independently of the OpenShell policy-attestation work in NVIDIA-AI-Blueprints#298. This change: - checkpoints artifact manifests after successful sandbox `execute` calls and performs one idempotent terminal harvest before sandbox teardown; - avoids blocking cancellation on an in-flight provider operation while preserving already checkpointed artifacts; - emits canonical `artifact.update` metadata without embedding artifact bytes or credentials; - delivers live and replayed sandbox artifacts to the Files tab, using the authenticated same-origin artifact endpoint for file access; and - preserves the artifact store selected by current configuration, including the existing SQL and S3-compatible implementations on `develop`. The existing NVIDIA-AI-Blueprints#305 branch is unchanged. This PR contains no OpenShell gateway, policy, attestation, Landlock, or cancellation-API changes. #### Validation ```bash PYTHONPATH=src:frontends/aiq_api/src uv run --no-sync pytest \ tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py \ tests/aiq_agent/agents/deep_researcher/test_custom_middleware.py \ tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py \ tests/aiq_agent/agents/deep_researcher/test_factory.py \ tests/aiq_agent/jobs/test_runner.py -q # 225 passed uv run --no-sync ruff check . # All checks passed uv run --no-sync ruff format --check . # 307 files already formatted cd frontends/ui npm run type-check # passed npm run lint # passed with 0 errors (existing warnings remain) npm run test:ci # 79 files passed; 1,324 tests passed; 1 skipped cd ../../docs make html # succeeded; 3 existing cross-reference/version-switcher warnings ``` - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. - [x] I updated documentation for user-facing or contributor-facing changes. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. #### Where should reviewers start? 1. `src/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.py` for the canonical metadata-only SSE contract. 2. `src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py`, `custom_middleware.py`, and `frontends/aiq_api/src/aiq_api/jobs/runner.py` for checkpoint and terminal-harvest ownership. 3. `frontends/ui/src/adapters/api/deep-research-client.ts` and `frontends/ui/src/features/layout/components/FileCard.tsx` for live/replayed Files-tab delivery. #### Related Issues - Relates to NVIDIA-AI-Blueprints#305 - Independent of NVIDIA-AI-Blueprints#298 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Deep research file artifacts now stream as structured `artifact.update` events, enabling durable file links with richer metadata (MIME, size, hashes, titles/captions, etc.). * File cards can display MIME/size and show an **Open file** button when a durable content URL is available. * **Bug Fixes** * Improved artifact harvesting across success, failure, and cancellation to better preserve completed outputs. * Updated streaming/replay behavior now merges incremental file metadata updates without overwriting earlier content. * **Documentation** * Refreshed architecture and SSE documentation to reflect the updated `artifact.update` payload shape. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> Co-authored-by: Ajay Thorve <AjayThorve@users.noreply.github.com> Signed-off-by: Tanner Leach <tleach@nvidia.com>
Signed-off-by: Tanner Leach <tleach@nvidia.com>
Sanitize captured workflow failures before returning them through public MCP poll/final-report responses, and guard async job state transitions by current state and runner ownership to avoid stale background tasks overwriting terminal rows. Signed-off-by: Tanner Leach <tleach@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
mcp/src/aiq_mcp/jobs.py (1)
312-372: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winException during
mark_runningbypasses the new state-guarded failure update.If
mark_runningitself raises (e.g. a transient asyncpg error) rather than returningFalse, execution falls into the genericexcept Exceptionbranch, which marks the job failed usingfrom_states=("running",)andrunner_id=self._runner_id. But the row is stillqueuedwith norunner_idset, so thisupdate()call never matches and silently returnsFalse— the failure is dropped, and the job sitsqueued(visible to pollers as still running) until the stale-job reconciler eventually times it out, rather than failing immediately.🛡️ Proposed fix
async def _run_job(self, job_id: str, query: str) -> None: job_id_token = _current_job_id.set(job_id) job_ref = log_identifier_ref(job_id) heartbeat_task: asyncio.Task | None = None + claimed = False try: claimed = await self._store.mark_running(job_id, self._runner_id) if not claimed: @@ except Exception as exc: # noqa: BLE001 - we want to catch everything for jobs logger.error("Job %s failed (%s)", job_ref, type(exc).__name__) sanitized = _sanitize_error(exc) await self._store.update( job_id, state="failed", error=sanitized, - from_states=("running",), - runner_id=self._runner_id, + from_states=("running",) if claimed else ("queued", "running"), + runner_id=self._runner_id if claimed else None, )🤖 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 `@mcp/src/aiq_mcp/jobs.py` around lines 312 - 372, The _run_job flow in jobs.py drops failures when _store.mark_running raises before the job row is transitioned to running. Update the exception handling in _run_job so that failures from mark_running are recorded against the queued state instead of always using from_states=("running",) and runner_id=self._runner_id; use the job state/runner context returned by mark_running, or branch the failure update based on whether claiming succeeded. Keep the existing behavior for the later run_query path, but ensure the generic except block can persist a failure even when mark_running never claimed the job.mcp/src/aiq_mcp/server.py (1)
379-454: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winWrap the remaining MCP tool calls before they escape
submit_queryonly sanitizesjobs.submit(...);wait_for_completion,poll_query, andget_final_reportcan still propagate raw internal exceptions through the public MCP error path. Wrap them the same way (or factor a generic helper) so anonymous callers never see backend exception text.🤖 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 `@mcp/src/aiq_mcp/server.py` around lines 379 - 454, The remaining public MCP tool paths in `submit_query`, `poll_query`, and `get_final_report` still allow raw backend exceptions to escape. Update the `MCP` server methods to sanitize errors consistently by wrapping `jobs.wait_for_completion(...)`, `self._get_jobs().poll(...)`, and `self._get_jobs().get_final_report(...)` in the same public error handling used for `jobs.submit(...)`, or factor a shared helper for `_public_submit_error`-style translation. Ensure anonymous callers only receive sanitized failures and never backend exception text.frontends/ui/src/features/chat/store.ts (1)
2911-2922: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve existing file content when merging artifact updates
artifact.updatefile events emitcontent: undefinedfor durable artifacts, so{ ...prev, ...file }clears earlier text when a later metadata-only update arrives for the same filename. Merge only defined fields infrontends/ui/src/features/chat/store.ts,frontends/ui/src/features/chat/hooks/use-deep-research.ts, andfrontends/ui/src/features/chat/hooks/use-load-job-data.ts.🤖 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/features/chat/store.ts` around lines 2911 - 2922, The deep research file merge logic is overwriting existing content with undefined fields from later metadata-only updates. Update the merge behavior in addDeepResearchFile, use-deep-research, and use-load-job-data so only defined values from the incoming file are applied, while preserving existing content and other previously stored fields for the same filename. Use the existing identifiers addDeepResearchFile, useDeepResearch, and useLoadJobData to locate the merge points and make the update logic skip undefined properties instead of spreading them blindly.src/aiq_agent/agents/deep_researcher/agent.py (1)
184-202: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd a regression test for the
state.filesfallback. The current coverage hitsresult["files"]and the end-to-end/shared/output.mdflow, but not the branch whereresulthas no usable"files"key and_extract_final_markdownmust read the markdown from thefilesargument.🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 184 - 202, Add a regression test for the state.files fallback in _extract_final_markdown: current coverage only exercises result["files"] and the end-to-end output path, but not the branch where result has no usable "files" key and the method must use the files argument. Create a focused test around DeepResearcherAgent._extract_final_markdown that passes a result without valid files and a files dict containing /shared/output.md or /output.md, then assert the markdown is returned from that fallback path rather than _salvage_inline_report.Source: Path instructions
mcp/tests/test_jobs.py (1)
67-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
_MemoryJobStoreacross test modules.This class is near-identical to
_MemoryJobStoreinmcp/tests/test_client_session_integration.py. Extracting a shared test double (e.g. into aconftest.pyor a_test_doubles.pyhelper) would avoid the two copies drifting out of sync asJobStore's real contract (guards, TTL, heartbeat semantics) evolves.🤖 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 `@mcp/tests/test_jobs.py` around lines 67 - 173, The `_MemoryJobStore` test double is duplicated across test modules and should be shared to prevent drift as `JobStore` behavior changes. Move the common implementation behind a reusable test helper (for example a shared `_test_doubles.py` module or `conftest.py` fixture) and update the tests to import/use that single `_MemoryJobStore` definition, keeping the methods like `create`, `mark_running`, `heartbeat`, `update`, and `mark_stale_running_failed` in one place.frontends/ui/src/features/layout/components/FileCard.tsx (1)
96-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale
aria-expanded/aria-controlson non-expandable (artifact-only) cards.The header
<Button>still exposesaria-expandedandaria-controls={file-content-${file.id}}even whenfile.contentis absent (now a common case for artifact-only cards with justcontentUrl/mimeType). The click handler is a no-op in that case, but the DOM still advertises an expandable/collapsible control to assistive tech, and the referenced region never renders.As per path instructions, UI changes should be reviewed for "accessible controls."♿ Proposed fix
<Button kind="tertiary" size="small" onClick={() => file.content && setIsExpanded(!isExpanded)} - aria-expanded={isExpanded} - aria-controls={`file-content-${file.id}`} + aria-expanded={file.content ? isExpanded : undefined} + aria-controls={file.content ? `file-content-${file.id}` : undefined} className="w-full justify-start text-left p-0" >🤖 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/features/layout/components/FileCard.tsx` around lines 96 - 155, In FileCard, the header Button still advertises expand/collapse behavior for artifact-only files that have no content, so make the accessibility props conditional. Update the FileCard button/expansion logic so aria-expanded, aria-controls, and the click toggle are only applied when file.content exists, and ensure the expandable region and ChevronDown icon are only rendered for expandable cards. Use the existing FileCard and file.content checks to keep non-expandable cards from exposing stale accessible controls.Source: Path instructions
mcp/tests/test_deployment_assets.py (1)
136-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore key/cert exclusions in the deployment context guardrail
mcp/tests/test_deployment_assets.py:136-145no longer asserts**/*.key,**/*.crt, or**/*.pem, andmcp/.dockerignoreonly excludes**/certs/. That leaves loose private key/cert files unguarded in the Docker build context.🤖 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 `@mcp/tests/test_deployment_assets.py` around lines 136 - 145, The deployment context test is missing coverage for private key and certificate file exclusions, so update test_deployment_context_excludes_env_files_and_includes_package_readmes to also assert the Docker ignore rules for **/*.key, **/*.crt, and **/*.pem. Then adjust mcp/.dockerignore so these file patterns are explicitly excluded, using the existing deployment guardrail entries alongside the README allowlist.scripts/start_e2e.sh (1)
89-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the CLI
--portafter sourcingdeploy/.env.check_envloadsdeploy/.envafter argument parsing, so any localPORTexport will override the flag beforeBACKEND_URL,nat serve --port, and the health check use it. The trackeddeploy/.env.exampleleavesPORTunset, but a user.envcan still set it, so re-applying the CLI value aftersourcekeeps the explicit flag authoritative.🤖 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 `@scripts/start_e2e.sh` around lines 89 - 104, After sourcing deploy/.env in start_e2e.sh, the CLI --port value can be overwritten by a PORT entry from the env file, which then affects BACKEND_URL, NEXT_PUBLIC_BACKEND_URL, nat serve --port, and the health check. Re-apply the parsed port value after the source block in check_env/startup flow so the explicit command-line flag remains authoritative, and ensure the PORT variable used by the existing BACKEND_URL export reflects that restored CLI value.
♻️ Duplicate comments (2)
frontends/ui/src/features/chat/hooks/use-load-job-data.ts (1)
577-582: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSame content-drop defect as
use-deep-research.tsandstore.ts'saddDeepResearchFile.
prev ? { ...prev, ...file } : fileoverwritesprev.contentwithundefinedwhen a laterartifact.updateevent for the same filename lacks inline content, contradicting the comment's own intent.🐛 Proposed fix
onFileUpdate: (file) => { // Merge like the live store: a later metadata-only event must not drop // content from an earlier event for the same filename during replay. const prev = buffer.files.get(file.filename) - buffer.files.set(file.filename, prev ? { ...prev, ...file } : file) + const definedUpdates = Object.fromEntries( + Object.entries(file).filter(([, value]) => value !== undefined) + ) as typeof file + buffer.files.set(file.filename, prev ? { ...prev, ...definedUpdates } : file) },🤖 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/features/chat/hooks/use-load-job-data.ts` around lines 577 - 582, The file replay merge in onFileUpdate is still dropping previously captured content when a later artifact.update arrives without inline content. Update the merge logic in use-load-job-data to preserve prev.content unless the incoming file actually provides content, matching the intended behavior already used in use-deep-research.ts and store.ts’s addDeepResearchFile. Keep the rest of the file metadata merged as before, but avoid overwriting existing content with undefined when only metadata changes.frontends/ui/src/features/chat/hooks/use-deep-research.ts (1)
490-505: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBuffer merge doesn't actually preserve content — same root cause as
store.ts'saddDeepResearchFile.
{ ...prev, ...file }copiesfile.contenteven when it's explicitlyundefined(which every non-inline artifact.update event carries perdeep-research-client.ts), overwritingprev.contentfrom an earlier replayed event. This defeats the comment's own stated goal during replay/reconnect.🐛 Proposed fix — filter undefined keys before merging
onFileUpdate: (file) => { if (buf.active) { // Merge like the live store (addDeepResearchFile): a later metadata-only // event must not drop content from an earlier event for the same filename. const prev = buf.files.get(file.filename) - buf.files.set(file.filename, prev ? { ...prev, ...file } : file) + const definedUpdates = Object.fromEntries( + Object.entries(file).filter(([, value]) => value !== undefined) + ) as typeof file + buf.files.set(file.filename, prev ? { ...prev, ...definedUpdates } : file) return }🤖 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/features/chat/hooks/use-deep-research.ts` around lines 490 - 505, The replay buffer merge in onFileUpdate still drops previously captured content because spreading prev and file copies undefined content from later metadata-only events over earlier data. Update the merge logic used in use-deep-research.ts so undefined fields are filtered out before combining, matching the behavior of addDeepResearchFile in store.ts and preserving existing content during reconnect/replay. Keep the fix localized to the buffered path inside onFileUpdate, ensuring only defined properties from file overwrite prev.
🤖 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/jobs/runner.py`:
- Around line 883-884: The failure path in runner.py is persisting the raw
exception text via job_store.update_status in the job runner, which can leak
secrets or internal hostnames. Update the exception handling around the job
status failure update to store only a sanitized error summary or the exception
class name, following the same pattern used by _teardown_sandbox, and keep the
rest of the context in logs without exposing the raw message.
- Around line 859-866: The cancellation cleanup in runner.py is too narrowly
guarded, so driver-specific or transient job-store errors can still escape and
prevent the job from reaching INTERRUPTED. Update the exception handling around
job_store.get_job and job_store.update_status in the cancellation branch of the
runner logic to catch and swallow/log a broader set of job-store failures, using
the existing job_store and JobStatus.INTERRUPTED flow so cancellation never
leaves the job stranded.
In `@frontends/ui/src/adapters/api/deep-research-client.spec.ts`:
- Around line 5-27: Add a regression test around
createDeepResearchClient/getJobStatus using FakeEventSource that emits an
initial file-artifact event with content and then a later metadata-only
artifact.update for the same filename; assert the stored result keeps the
original content instead of being replaced. This should exercise the merge
behavior that was broken in addDeepResearchFile and the hook buffer paths, so
the test should verify the filename entry retains both metadata and previously
captured content after the second update.
In `@skills/aiq-research/skill-card.md`:
- Around line 33-34: The markdown under the Skill Output section is missing the
required blank line after the heading, causing the MD022 lint failure. Update
the content around the “## Skill Output:” heading in skill-card.md so that there
is an empty line before the “Output Type(s):” line, keeping the existing wording
intact.
---
Outside diff comments:
In `@frontends/ui/src/features/chat/store.ts`:
- Around line 2911-2922: The deep research file merge logic is overwriting
existing content with undefined fields from later metadata-only updates. Update
the merge behavior in addDeepResearchFile, use-deep-research, and
use-load-job-data so only defined values from the incoming file are applied,
while preserving existing content and other previously stored fields for the
same filename. Use the existing identifiers addDeepResearchFile,
useDeepResearch, and useLoadJobData to locate the merge points and make the
update logic skip undefined properties instead of spreading them blindly.
In `@frontends/ui/src/features/layout/components/FileCard.tsx`:
- Around line 96-155: In FileCard, the header Button still advertises
expand/collapse behavior for artifact-only files that have no content, so make
the accessibility props conditional. Update the FileCard button/expansion logic
so aria-expanded, aria-controls, and the click toggle are only applied when
file.content exists, and ensure the expandable region and ChevronDown icon are
only rendered for expandable cards. Use the existing FileCard and file.content
checks to keep non-expandable cards from exposing stale accessible controls.
In `@mcp/src/aiq_mcp/jobs.py`:
- Around line 312-372: The _run_job flow in jobs.py drops failures when
_store.mark_running raises before the job row is transitioned to running. Update
the exception handling in _run_job so that failures from mark_running are
recorded against the queued state instead of always using
from_states=("running",) and runner_id=self._runner_id; use the job state/runner
context returned by mark_running, or branch the failure update based on whether
claiming succeeded. Keep the existing behavior for the later run_query path, but
ensure the generic except block can persist a failure even when mark_running
never claimed the job.
In `@mcp/src/aiq_mcp/server.py`:
- Around line 379-454: The remaining public MCP tool paths in `submit_query`,
`poll_query`, and `get_final_report` still allow raw backend exceptions to
escape. Update the `MCP` server methods to sanitize errors consistently by
wrapping `jobs.wait_for_completion(...)`, `self._get_jobs().poll(...)`, and
`self._get_jobs().get_final_report(...)` in the same public error handling used
for `jobs.submit(...)`, or factor a shared helper for
`_public_submit_error`-style translation. Ensure anonymous callers only receive
sanitized failures and never backend exception text.
In `@mcp/tests/test_deployment_assets.py`:
- Around line 136-145: The deployment context test is missing coverage for
private key and certificate file exclusions, so update
test_deployment_context_excludes_env_files_and_includes_package_readmes to also
assert the Docker ignore rules for **/*.key, **/*.crt, and **/*.pem. Then adjust
mcp/.dockerignore so these file patterns are explicitly excluded, using the
existing deployment guardrail entries alongside the README allowlist.
In `@mcp/tests/test_jobs.py`:
- Around line 67-173: The `_MemoryJobStore` test double is duplicated across
test modules and should be shared to prevent drift as `JobStore` behavior
changes. Move the common implementation behind a reusable test helper (for
example a shared `_test_doubles.py` module or `conftest.py` fixture) and update
the tests to import/use that single `_MemoryJobStore` definition, keeping the
methods like `create`, `mark_running`, `heartbeat`, `update`, and
`mark_stale_running_failed` in one place.
In `@scripts/start_e2e.sh`:
- Around line 89-104: After sourcing deploy/.env in start_e2e.sh, the CLI --port
value can be overwritten by a PORT entry from the env file, which then affects
BACKEND_URL, NEXT_PUBLIC_BACKEND_URL, nat serve --port, and the health check.
Re-apply the parsed port value after the source block in check_env/startup flow
so the explicit command-line flag remains authoritative, and ensure the PORT
variable used by the existing BACKEND_URL export reflects that restored CLI
value.
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 184-202: Add a regression test for the state.files fallback in
_extract_final_markdown: current coverage only exercises result["files"] and the
end-to-end output path, but not the branch where result has no usable "files"
key and the method must use the files argument. Create a focused test around
DeepResearcherAgent._extract_final_markdown that passes a result without valid
files and a files dict containing /shared/output.md or /output.md, then assert
the markdown is returned from that fallback path rather than
_salvage_inline_report.
---
Duplicate comments:
In `@frontends/ui/src/features/chat/hooks/use-deep-research.ts`:
- Around line 490-505: The replay buffer merge in onFileUpdate still drops
previously captured content because spreading prev and file copies undefined
content from later metadata-only events over earlier data. Update the merge
logic used in use-deep-research.ts so undefined fields are filtered out before
combining, matching the behavior of addDeepResearchFile in store.ts and
preserving existing content during reconnect/replay. Keep the fix localized to
the buffered path inside onFileUpdate, ensuring only defined properties from
file overwrite prev.
In `@frontends/ui/src/features/chat/hooks/use-load-job-data.ts`:
- Around line 577-582: The file replay merge in onFileUpdate is still dropping
previously captured content when a later artifact.update arrives without inline
content. Update the merge logic in use-load-job-data to preserve prev.content
unless the incoming file actually provides content, matching the intended
behavior already used in use-deep-research.ts and store.ts’s
addDeepResearchFile. Keep the rest of the file metadata merged as before, but
avoid overwriting existing content with undefined when only metadata changes.
🪄 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: d6f875fe-2cc7-4d87-ac22-6dbd26ae5a84
⛔ Files ignored due to path filters (1)
skills/aiq-research/skill.oms.sigis excluded by!skills/**/skill.oms.sig
📒 Files selected for processing (59)
.dockerignoredeploy/helm/deployment-k8s/Chart.yamldeploy/helm/deployment-k8s/charts/aiq-0.0.4.tgzdeploy/helm/deployment-k8s/charts/aiq-0.0.5.tgzdeploy/helm/helm-charts-k8s/aiq/Chart.yamldeploy/helm/helm-charts-k8s/aiq/templates/_helpers.tpldeploy/helm/helm-charts-k8s/aiq/templates/namespace.yamldeploy/helm/helm-charts-k8s/aiq/values.yamldocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/jobs/telemetry.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/shared/utils/artifact-url.spec.tsfrontends/ui/src/shared/utils/artifact-url.tsmcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/tests/test_client_session_integration.pymcp/tests/test_deployment_assets.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_server_runtime.pyscripts/start_e2e.shskills/aiq-research/BENCHMARK.mdskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pyskills/aiq-research/skill-card.mdsrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/tools/research.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.py
💤 Files with no reviewable changes (1)
- .dockerignore
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: UI Unit Tests
- GitHub Check: Script Validation
- GitHub Check: Pytest and Coverage
⚠️ CI failures not shown inline (3)
GitHub Actions: Skills Eval / Run Harbor skill eval: fix(mcp): harden public job failure handling
Conclusion: failure
##[group]Run set -a
�[36;1mset -a�[0m
�[36;1m# shellcheck disable=SC1090�[0m
�[36;1msource "$RUNNER_ENV_FILE"�[0m
�[36;1mset +a�[0m
�[36;1m# Mirror local recipe: build from source, tag with run id so�[0m
�[36;1m# concurrent runs (if any) don't collide.�[0m
�[36;1mexport BACKEND_IMAGE="aiq-agent:ci-28987905264"�[0m
�[36;1mdocker compose --env-file ../.env -f docker-compose.yaml up -d --build aiq-agent postgres�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
RUNNER_ENV_FILE: /home/ubuntu/aiq-eval/.env
pythonLocation: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
PKG_CONFIG_PATH: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64/lib/pkgconfig
Python_ROOT_DIR: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
Python2_ROOT_DIR: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
Python3_ROOT_DIR: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64
LD_LIBRARY_PATH: /home/ubuntu/actions-runner/_work/_tool/Python/3.13.13/x64/lib
UV_CACHE_DIR: /home/ubuntu/actions-runner/_work/_temp/setup-uv-cache
##[endgroup]
Image aiq-agent:ci-28987905264 Building
`#1` [internal] load local bake definitions
`#1` reading from stdin 583B done
`#1` DONE 0.0s
`#2` [internal] load build definition from Dockerfile
`#2` transferring dockerfile: 5.60kB done
`#2` DONE 0.0s
`#3` [internal] load metadata for nvcr.io/nvidia/base/ubuntu:noble-20260217
`#3` ...
`#4` [internal] load metadata for nvcr.io/nvidia/distroless/python:3.13-v4.0.5
`#4` DONE 0.3s
`#3` [internal] load metadata for nvcr.io/nvidia/base/ubuntu:noble-20260217
`#3` DONE 0.4s
`#5` [internal] load .dockerignore
`#5` transferring context: 1.68kB done
`#5` DONE 0.0s
`#6` [dev 1/4] FROM nvcr.io/nvidia/distroless/python:3.13-v4.0.5@sha256:***REDACTED***
`#6` resolve nvcr.io/nvidia/distroless/python:3.13-v4.0.5@sha256:***REDACTED*** 0.0s done
`#6` DONE 0.0s
`#7` [builder 1/17] FROM nvcr.io/nvidia/base/ubuntu:noble-20260217@sha256:***REDACTED***
`#7` resolve ...
GitHub Actions: Skills Eval / Run Harbor skill eval: fix(mcp): harden public job failure handling
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
GitHub Actions: Skills Eval / 0_Run Harbor skill eval.txt: fix(mcp): harden public job failure handling
Conclusion: failure
##[group]Run if [ ! -r "$RUNNER_ENV_FILE" ]; then
�[36;1mif [ ! -r "$RUNNER_ENV_FILE" ]; then�[0m
�[36;1m echo "::error::Runner credential file missing or unreadable at $RUNNER_ENV_FILE"�[0m
🧰 Additional context used
📓 Path-based instructions (14)
**
⚙️ 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/helm-charts-k8s/aiq/Chart.yamldeploy/helm/helm-charts-k8s/aiq/templates/namespace.yamlfrontends/ui/src/features/layout/components/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsdeploy/helm/helm-charts-k8s/aiq/templates/_helpers.tplskills/aiq-research/BENCHMARK.mddeploy/helm/helm-charts-k8s/aiq/values.yamlfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxdocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/README.mdfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/base.pytests/aiq_agent/agents/deep_researcher/test_factory.pyfrontends/ui/src/shared/components/MarkdownRenderer/index.tsdeploy/helm/deployment-k8s/Chart.yamlfrontends/aiq_api/src/aiq_api/jobs/telemetry.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tstests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pyskills/aiq-research/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pyfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tssrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pyscripts/start_e2e.shfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tstests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pyskills/aiq-research/skill-card.mdsrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pyskills/aiq-research/scripts/aiq.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pyfrontends/ui/src/features/layout/components/FileCard.tsxtests/aiq_agent/agents/deep_researcher/test_agent.pymcp/tests/test_deployment_assets.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.pyfrontends/ui/src/adapters/api/deep-research-client.tstests/aiq_agent/agents/deep_researcher/test_custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdmcp/tests/test_client_session_integration.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_server_runtime.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.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:
deploy/helm/helm-charts-k8s/aiq/Chart.yamldeploy/helm/helm-charts-k8s/aiq/templates/namespace.yamldeploy/helm/helm-charts-k8s/aiq/templates/_helpers.tpldeploy/helm/helm-charts-k8s/aiq/values.yamldeploy/helm/deployment-k8s/Chart.yaml
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/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/adapters/api/deep-research-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/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/adapters/api/deep-research-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/AgentsTab.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/AgentsTab.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/adapters/api/deep-research-client.ts
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/BENCHMARK.mdskills/aiq-research/SKILL.mdskills/aiq-research/skill-card.mdskills/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/BENCHMARK.mdskills/aiq-research/SKILL.mdskills/aiq-research/skill-card.mdskills/aiq-research/scripts/aiq.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/architecture/agents/sandbox.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/architecture/agents/sandbox.md
**/*.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/deep_researcher/sandbox/base.pytests/aiq_agent/agents/deep_researcher/test_factory.pyfrontends/aiq_api/src/aiq_api/jobs/telemetry.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pyskills/aiq-research/scripts/aiq.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_agent.pymcp/tests/test_deployment_assets.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pymcp/tests/test_client_session_integration.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_server_runtime.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.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/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.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/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/tools/research.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.md
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_agent.pymcp/tests/test_deployment_assets.pytests/aiq_agent/jobs/test_telemetry.pytests/deploy/test_helm_deployment_k8s.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pymcp/tests/test_client_session_integration.pymcp/tests/test_server_runtime.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.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/jobs/telemetry.pyfrontends/aiq_api/src/aiq_api/jobs/runner.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Use this skill only for research-shaped requests that need a reachable NVIDIA AI-Q Blueprint backend via `scripts/aiq.py`; do not use it for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests, which belong to `aiq-deploy`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Before sending any user query to AI-Q, resolve the backend URL, run `health`, state the exact endpoint to the user, and only proceed with non-local URLs after explicit trust confirmation in the conversation.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: If no backend is reachable, ask for an AI-Q backend URL or hand off to `aiq-deploy`; if the backend returns `401` or `403`, stop and explain that this public skill does not manage authentication.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Do not send credentials, cookies, bearer tokens, or secret values in AI-Q query text or follow-up query text.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: When AI-Q returns a deep-research job ID, poll asynchronous jobs with `research_poll`, tell the user deep research is running in the background, and do not retry failed jobs automatically.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: If polling is interrupted, resume using `status`, `report`, or `research_poll`; use `status` to inspect job state and artifacts, `report` for finished jobs, and `research_poll` to continue waiting.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Present returned reports with citations and source URLs intact; do not truncate them.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: After presenting a report, answer follow-up questions from the existing report directly when possible; otherwise send a fresh query that carries prior context, and use the same backend/auth/polling flow.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: For a redo, rerun research with a refined query and appropriate agent type, treating it as a new job and restating the target endpoint before sending it.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: Respect the skill’s version compatibility rules: the Blueprint major version must match the skill major version, the Blueprint minor version must be equal or greater, and patch version may vary.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T01:37:03.708Z
Learning: If the backend is reachable but `/chat` or async job routes fail, report that the backend is reachable but not compatible with this public research flow; do not fabricate answers.
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/deep_researcher/test_agent.py
🪛 ast-grep (0.44.1)
tests/deploy/test_helm_deployment_k8s.py
[error] 26-31: Command coming from incoming request
Context: subprocess.run(
["helm", "template", "aiq", str(CHART_PATH), "-n", namespace, *extra_args],
check=True,
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 LanguageTool
skills/aiq-research/skill-card.md
[style] ~12-~12: Consider a different adjective to strengthen your wording.
Context: ... and engineers use this skill to submit deep research queries to a running NVIDIA AI...
(DEEP_PROFOUND)
[style] ~37-~37: Consider a different adjective to strengthen your wording.
Context: ...Output:** [Asynchronous job polling for deep research; artifact download for charts ...
(DEEP_PROFOUND)
🪛 markdownlint-cli2 (0.22.1)
skills/aiq-research/skill-card.md
[warning] 33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 OpenGrep (1.23.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 142-154: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 202-205: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
| if job_store: | ||
| await job_store.update_status(job_id, JobStatus.FAILURE, error=str(e)) | ||
| try: | ||
| job = await job_store.get_job(job_id) | ||
| if job and job.status != JobStatus.INTERRUPTED.value: | ||
| await job_store.update_status(job_id, JobStatus.INTERRUPTED, error="cancelled by user") | ||
| except (ConnectionError, TimeoutError, RuntimeError): | ||
| pass | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Narrow except clause on the cancellation status update may leave jobs stranded.
except (ConnectionError, TimeoutError, RuntimeError): pass won't catch other transient job-store failures (e.g. driver-specific DB exceptions), which would then propagate unhandled out of the CancelledError branch — leaving the job's status un-updated (never reaching INTERRUPTED) despite finally still running teardown. Given the PR's stated goal of avoiding stranded jobs, broaden the guard (or log-and-swallow more defensively) here too.
🛡️ Proposed fix
if job_store:
try:
job = await job_store.get_job(job_id)
if job and job.status != JobStatus.INTERRUPTED.value:
await job_store.update_status(job_id, JobStatus.INTERRUPTED, error="cancelled by user")
- except (ConnectionError, TimeoutError, RuntimeError):
- pass
+ except Exception as exc: # noqa: BLE001 - status update is best-effort on the cancel path
+ logger.warning(
+ "Failed to mark job %s interrupted exception=%s", job_id, exc.__class__.__name__
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if job_store: | |
| await job_store.update_status(job_id, JobStatus.FAILURE, error=str(e)) | |
| try: | |
| job = await job_store.get_job(job_id) | |
| if job and job.status != JobStatus.INTERRUPTED.value: | |
| await job_store.update_status(job_id, JobStatus.INTERRUPTED, error="cancelled by user") | |
| except (ConnectionError, TimeoutError, RuntimeError): | |
| pass | |
| if job_store: | |
| try: | |
| job = await job_store.get_job(job_id) | |
| if job and job.status != JobStatus.INTERRUPTED.value: | |
| await job_store.update_status(job_id, JobStatus.INTERRUPTED, error="cancelled by user") | |
| except Exception as exc: # noqa: BLE001 - status update is best-effort on the cancel path | |
| logger.warning( | |
| "Failed to mark job %s interrupted exception=%s", job_id, exc.__class__.__name__ | |
| ) |
🤖 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/jobs/runner.py` around lines 859 - 866, The
cancellation cleanup in runner.py is too narrowly guarded, so driver-specific or
transient job-store errors can still escape and prevent the job from reaching
INTERRUPTED. Update the exception handling around job_store.get_job and
job_store.update_status in the cancellation branch of the runner logic to catch
and swallow/log a broader set of job-store failures, using the existing
job_store and JobStatus.INTERRUPTED flow so cancellation never leaves the job
stranded.
| import { createDeepResearchClient, getJobStatus } from './deep-research-client' | ||
|
|
||
| class FakeEventSource { | ||
| static latest: FakeEventSource | null = null | ||
| onopen: (() => void) | null = null | ||
| onmessage: ((event: MessageEvent) => void) | null = null | ||
| onerror: (() => void) | null = null | ||
| private listeners = new Map<string, (event: MessageEvent) => void>() | ||
|
|
||
| constructor(_url: string) { | ||
| FakeEventSource.latest = this | ||
| } | ||
|
|
||
| addEventListener(type: string, listener: EventListener): void { | ||
| this.listeners.set(type, listener as (event: MessageEvent) => void) | ||
| } | ||
|
|
||
| close(): void {} | ||
|
|
||
| emit(type: string, data: unknown): void { | ||
| this.listeners.get(type)?.(new MessageEvent(type, { data: JSON.stringify(data) })) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
LGTM! Good coverage of the new structured file-artifact mapping.
One gap worth adding: a test asserting that a later metadata-only artifact.update for the same filename doesn't clobber content captured by an earlier event — this would have caught the merge defect flagged in store.ts's addDeepResearchFile and both hook buffer paths.
Also applies to: 57-134
🤖 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/deep-research-client.spec.ts` around lines 5 -
27, Add a regression test around createDeepResearchClient/getJobStatus using
FakeEventSource that emits an initial file-artifact event with content and then
a later metadata-only artifact.update for the same filename; assert the stored
result keeps the original content instead of being replaced. This should
exercise the merge behavior that was broken in addDeepResearchFile and the hook
buffer paths, so the test should verify the filename entry retains both metadata
and previously captured content after the second update.
Move the MCP server into an opt-in dependency group and select it in CI, setup, pre-commit, and full-suite validation paths. Keep the main AI-Q Docker build independent of absent MCP workspace sources, and add regression coverage and updated usage documentation. Signed-off-by: Tanner Leach <tleach@nvidia.com>
- split MCP from the root uv workspace and add its own lock - keep cryptography 48 limited to the audited MCP release profile - route CI, Docker, SBOM, tests, and docs through the MCP project Signed-off-by: Tanner Leach <tleach@nvidia.com>
Retry heartbeat writes after transient store failures without exposing internal exception details. Ensure heartbeat task failures cannot skip job context and failure-capture cleanup. Signed-off-by: Tanner Leach <tleach@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcp/Dockerfile (1)
53-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the inline version-assertion one-liner into the existing check script.
Line 54 hardcodes
version('mcp') == '1.27.2'andversion('nvidia-nat-core') == '1.8.0'inline in a long, hard-to-diff Python one-liner, duplicating information that already lives inmcp/pyproject.toml/mcp/uv.lock. A dedicatedcheck_runtime_dependencies.pyscript already exists one line above (line 53) for this exact purpose — moving the import/version/entry-point assertions there would keep the release "canary" logic testable and reviewable outside the Dockerfile, instead of as an unwrapped shell string.♻️ Proposed consolidation
-RUN /opt/venv/bin/python mcp/scripts/check_runtime_dependencies.py \ - && /opt/venv/bin/python -c "import importlib; from importlib.metadata import entry_points, version; [importlib.import_module(name) for name in ('aiq_mcp', 'aiq_mcp.server', 'aiq_mcp.jobs', 'aiq_mcp.job_store', 'aiq_mcp.workflow_runner', 'aiq_agent.common', 'tavily_web_search', 'knowledge_layer', 'asyncpg')]; assert version('mcp') == '1.27.2'; assert version('nvidia-nat-core') == '1.8.0'; assert any(ep.name == 'tavily_web_search' for ep in entry_points(group='nat.plugins')); print('AI-Q MCP runtime verified')" +RUN /opt/venv/bin/python mcp/scripts/check_runtime_dependencies.pyMove the imports,
version(...) ==assertions, and entry-point check intocheck_runtime_dependencies.pyso version drift shows up as a clean diff in a reviewable Python file rather than a one-line shell string.🤖 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 `@mcp/Dockerfile` around lines 53 - 54, The Dockerfile’s long Python one-liner duplicates runtime checks that should live in the existing check_runtime_dependencies.py script. Move the import/module validation, version assertions for mcp and nvidia-nat-core, and the nat.plugins entry-point check into check_runtime_dependencies.py, then keep the Dockerfile RUN step limited to invoking that script and the final verification print.
🤖 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.
Outside diff comments:
In `@mcp/Dockerfile`:
- Around line 53-54: The Dockerfile’s long Python one-liner duplicates runtime
checks that should live in the existing check_runtime_dependencies.py script.
Move the import/module validation, version assertions for mcp and
nvidia-nat-core, and the nat.plugins entry-point check into
check_runtime_dependencies.py, then keep the Dockerfile RUN step limited to
invoking that script and the final verification print.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 7ab0f028-d2f3-4e26-b5d2-e4713290cb60
⛔ Files ignored due to path filters (2)
mcp/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.coderabbit.yaml.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineAGENTS.mdCONTRIBUTING.mdLICENSE-THIRD-PARTYREADME.mddeploy/Dockerfiledocs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.mdmcp/Dockerfilemcp/README.mdmcp/SECURITY.mdmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_config_and_packaging.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_jobs.pymcp/tests/test_release_checks.pypyproject.tomlscripts/dev.shscripts/setup.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Script Validation
🧰 Additional context used
📓 Path-based instructions (9)
**
⚙️ 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:
LICENSE-THIRD-PARTYdocs/source/contributing/code-style.mdscripts/dev.shmcp/README.mddeploy/DockerfileCONTRIBUTING.mdscripts/setup.shdocs/source/contributing/pr-workflow.mdmcp/pyproject.tomlREADME.mdmcp/Dockerfiledocs/source/contributing/testing.mdAGENTS.mdmcp/tests/test_dependency_compatibility.pydocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.mdmcp/tests/test_deployment_assets.pymcp/SECURITY.mdmcp/tests/test_config_and_packaging.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_jobs.pymcp/scripts/check_license_inventory.pypyproject.toml
docs/source/contributing/**
⚙️ CodeRabbit configuration file
docs/source/contributing/**:Code Organization
High-level layout (refer to Architecture Overview for component roles):
src/aiq_agent/ ├── agents/ # Chat researcher, shallow/deep research, clarifier │ ├── chat_researcher/ # Orchestrator, orchestration node (intent + meta + depth), nodes │ ├── shallow_researcher/ │ ├── deep_researcher/ # See src/aiq_agent/agents/deep_researcher/README.md │ └── clarifier/ ├── common/ # LLM provider, callbacks, prompt utils, data_sources ├── knowledge/ # Schema, factory, base retriever/ingestor, summary store ├── observability/ # OpenTelemetry header redaction exporter ├── auth/ # Auth utilities └── fastapi_extensions/ # API route extensionsConfigs live in
configs/; refer to the Customization guide for configuration options. Frontends:frontends/cli,frontends/aiq_api,frontends/debug,frontends/ui. Benchmarks:frontends/benchmarks/. Data sources and Knowledge Layer:sources/.
docs/source/contributing/**:Code Style
Development Workflow
Helper script:
./scripts/dev.sh help # List commands ./scripts/dev.sh test # Run tests ./scripts/dev.sh format # Format code ./scripts/dev.sh lint # Lint ./scripts/dev.sh pre-commit # Format + checksOtherwise run
pre-commit run --all-files,pytest, and formatters directly (refer to Code Quality below).Code Quality
- Formatting:
ruff(imports/linting) +yapf(PEP 8 base,column_limit=120). The./scripts/dev.sh formatcommand runs both.- Pre-commit: `pre-commit ru...
Files:
docs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/contributing/code-style.mdCONTRIBUTING.mddocs/source/contributing/pr-workflow.mdREADME.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/integration/mcp-server.md
mcp/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For changes under
mcp/, also runuv sync --project mcp --extra devanduv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/README.mdmcp/pyproject.tomlmcp/Dockerfilemcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/SECURITY.mdmcp/tests/test_config_and_packaging.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_jobs.pymcp/scripts/check_license_inventory.py
deploy/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets or environment-specific hostnames in deployment assets; use
deploy/.envfor secrets.
Files:
deploy/Dockerfile
{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/Dockerfile
{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:
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md
{pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Dependency changes must leave both project locks current: check the root with
uv lock --checkand MCP withuv lock --project mcp --check.
Files:
mcp/pyproject.tomlpyproject.toml
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/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:
mcp/pyproject.toml.pre-commit-config.yaml.github/workflows/ci.ymlpyproject.toml
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: These rules apply to every task in this repository; load the relevant task-specific skill from `.agents/skills/` before starting a workflow it covers.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Stay inside this repository; do not edit adjacent repositories as part of an AI-Q change, and prefer the smallest change scoped to the package you are touching.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Do not commit secrets, tokens, or environment-specific hostnames; use environment variables and `SecretStr`, and resolve API keys at runtime.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Never print or log secret values, including in tool output or error messages.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Missing-secret paths must degrade gracefully (stub or skip) rather than crash or leak.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Respect authenticated data sources: honor `requires_auth`, per-user token pass-through, and backend token validators, and apply owner guardrails before loading protected report or artifact context into an agent.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Do not weaken or bypass `AuthMiddleware`, validators, or auth gating without a prior design discussion.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: For substantial behavior, auth, UI, or architecture changes, open a design discussion before coding rather than landing a large unreviewed change.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Every commit must be signed off with `git commit -s` and include a `Signed-off-by` trailer.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:58:51.275Z
Learning: Keep pull requests scoped: avoid unrelated files, accidental generated artifacts, and secrets, and provide validation evidence for the changes made.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Search existing issues and pull requests before opening new work.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Open an issue or discussion before large design changes, public API changes, deployment changes, or contributor workflow changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts in contributions.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Target the `develop` branch unless a maintainer asks you to use a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Make the smallest coherent change and add or update tests for behavior changes.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Sign off every commit with `git commit -s`; commits without sign-off may be rejected.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Run the relevant local validation before opening the pull request and include the exact output or workflow link in the PR.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Open a pull request into `develop` and fill out the PR template with exact validation evidence.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: Address review feedback until required checks and code-owner review pass.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: For deployment changes, run the relevant Helm or compose validation and describe the environment used.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: AI-Q uses push-triggered GitHub Actions; pull requests are mirrored by copy-pr-bot after `/ok to test`, and maintainers can request bot-driven merge with `/merge` when repository rules are satisfied.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:03.770Z
Learning: All contributors must sign off their commits to certify DCO compliance and acknowledgment that contributions are public and may be redistributed.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:08.070Z
Learning: Use `./scripts/dev.sh help`, `./scripts/dev.sh test`, `./scripts/dev.sh format`, `./scripts/dev.sh lint`, and `./scripts/dev.sh pre-commit` for the documented development workflow; otherwise run `pre-commit run --all-files`, `pytest`, and the relevant formatters directly.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Create a focused branch from `develop`, make changes, and add or update tests before opening a pull request.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Run the narrowest relevant local checks for your change and record the exact commands in the pull request description.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Open pull requests into `develop` unless a maintainer asks you to target a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Address review feedback until required checks, code-owner review, and review-thread resolution pass.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Use the repository's configured merge/bot workflow when available; otherwise follow the normal protected-branch merge flow.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Sign off all commits with a Developer's Certificate of Origin (DCO) using `git commit -s` so each commit includes a `Signed-off-by` line.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: By contributing, certify compliance with the project's Developer's Certificate of Origin 1.1 statement, including that the contribution is properly licensed and that your sign-off may be retained indefinitely.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: For MCP changes, validate the MCP subproject independently with its own dev environment and test command.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: For UI changes, run the UI project's lint, type-check, test, and build commands from `frontends/ui`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-09T18:59:12.430Z
Learning: Repository owners, organization members, and collaborators may request additional validation, including NVSkills validation with `/nvskills-ci`.
🪛 SkillSpector (2.3.7)
.agents/skills/aiq-release-qa/SKILL.md
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🔇 Additional comments (39)
LICENSE-THIRD-PARTY (1)
8-11: LGTM!.secrets.baseline (1)
136-136: LGTM!Also applies to: 354-358
docs/source/contributing/code-style.md (1)
24-25: 📐 Maintainability & Code QualityVerify the pre-commit matrix still matches the hook set.
pre-commit run --all-filesonly covers lock checks if explicit hooks are present; otherwise this doc now overstates the validation contributors get from the command.Source: Path instructions
scripts/dev.sh (1)
8-11: LGTM!Also applies to: 61-61
mcp/README.md (2)
109-115: 📐 Maintainability & Code QualityVerify the standalone wheel build command.
uv build mcp --wheelis a tool-specific invocation; please confirm it is valid for the pinneduvrelease, otherwise this doc will point contributors at a failing command.
3-108: LGTM!Also applies to: 117-154
.coderabbit.yaml (1)
99-103: LGTM!deploy/Dockerfile (1)
81-90: 🩺 Stability & AvailabilityVerify
uv pip installaccepts--no-sourceshere.If the pinned
uvrelease rejects that flag foruv pip install, the builder stage will fail at image build time.Source: Path instructions
docs/source/deployment/docker-build.md (1)
45-47: LGTM!Also applies to: 58-73
.agents/skills/aiq-maintain-ci/SKILL.md (1)
39-52: LGTM!CONTRIBUTING.md (1)
32-42: LGTM!.agents/skills/aiq-release-qa/references/validation-matrix.md (1)
16-19: LGTM!Also applies to: 50-62
scripts/setup.sh (1)
46-55: LGTM!Also applies to: 126-127
docs/source/contributing/pr-workflow.md (1)
30-36: LGTM!mcp/pyproject.toml (1)
55-81: LGTM!Verified the scoped-override table syntax (
{ package = { name, version }, dependencies = [...] }) against uv's documented resolution behavior — this is valid and matches the documented pattern for constraining a specific package's transitive dependency.README.md (1)
305-326: LGTM!The
cryptography<47(root) vscryptography==48.0.1(MCP frozen profile) distinction is consistent with the override comment inmcp/pyproject.toml.mcp/Dockerfile (1)
1-52: LGTM!The multi-stage layout (build wheels into
/opt/venvvia--no-editable --frozen, then copy only the venv/config/license into a minimal non-root release stage) is a sound pattern, and the port/health-check/entrypoint wiring lines up with theaiq-mcpcompose service referencing this Dockerfile.Also applies to: 56-93
.github/workflows/ci.yml (3)
72-72: Unpinnedpostgres:16-alpineservice image andactions/upload-artifact@v4references (new instances of a previously-flagged pattern).This PR adds a third
actions/upload-artifact@v4use (mcp-release-evidence, mcp-compose-logs) and keepspostgres:16-alpineunpinned, consistent with the earlier review comment on this file about pinning images/actions to immutable digests.Also applies to: 154-154, 284-284, 313-313
242-266: 🔒 Security & Privacy | ⚡ Quick winVerify
uv audit/uv exportpreview-feature flag names and the scope of the ignored advisory.Two things worth double-checking here:
- uv audit is a preview feature, and running it as-is will produce an experimental warning. Add --preview-features audit only if you want to suppress the warning. The workflow instead passes
--preview-features audit-command,json-output, which doesn't match the documentedauditfeature name. Unrecognized preview feature names only warn, they don't fail the build, but the intended warning-suppression likely isn't happening.--ignore-until-fixed GHSA-f4j7-r4q5-qw2cpermanently (until a fix appears) exempts this advisory from the production-lock gate; the public advisory record for this ID has no known source code · Dependabot alerts are not supported on this advisory because it does not have a package from a supported ecosystem with an affected and fixed version, so it's worth confirming this ID actually maps to a real MCP dependency vulnerability and not a stale/incorrect reference.
37-53: LGTM! Postgres service wiring, isolated MCP venv provisioning, mandatory 90%-coverage MCP test lane with skipped-test guard, SBOM/license evidence, and Compose smoke test/teardown are otherwise well-structured.Also applies to: 67-165, 184-322
docs/source/contributing/testing.md (1)
10-17: LGTM!.pre-commit-config.yaml (2)
84-89: 📐 Maintainability & Code Quality | ⚡ Quick winVerify
pytest-mcppush-stage hook is runnable locally without CI's dedicated Postgres.This hook runs
pytest mcp/testswith--cov-fail-under=90, but the CItestjob only provisions Postgres via a service container andAIQ_MCP_TEST_DB_URL. Ifmcp/testsrequires a live Postgres and a contributor runspre-commit run --all-files --hook-stage pushlocally without one configured, the hook will fail with no guidance in the docs.
61-66: 🚀 Performance & ScalabilityConfirm
uv-lock-mcpcovers every MCP path dependency. The currentfilesregex only watchessources/knowledge_layerandsources/tavily_web_search; ifmcp/pyproject.tomldepends on any othersources/*package, edits there will not retriggeruv lock --project mcp --check..agents/skills/aiq-release-qa/SKILL.md (1)
38-41: LGTM!Also applies to: 72-87, 104-108
.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md (1)
16-19: LGTM!Also applies to: 40-50
AGENTS.md (2)
43-43: LGTM!Also applies to: 55-61
105-109: 📐 Maintainability & Code QualityAGENTS.md:105-109 — align the
cryptographynote withpyproject.toml
This says the root workspace stays oncryptography>=46.0.6,<47while onlymcp/uses48.0.1, but the dependency change in this PR appears to target the root project. One of these is stale.mcp/src/aiq_mcp/jobs.py (1)
373-404: LGTM! Heartbeat resilience fix looks correct: transient store failures no longer kill the heartbeat loop, and_FAILURE_HANDLER/context cleanup is now guaranteed via the nestedfinallyeven if heartbeat teardown raises. This resolves the previously flagged critical issue.pyproject.toml (2)
100-100: LGTM!Also applies to: 157-157, 184-185, 242-242
211-226: 📐 Maintainability & Code QualityClarify the
cryptographyconstraint andmcp-testsgroup. If the change is meant to affect the rootmcp-testsgroup rather than[project].dependencies, add a short note explaining why this root test-only pin is needed whilemcp/remains excluded from the workspace.mcp/tests/test_dependency_compatibility.py (1)
26-31: LGTM!docs/source/integration/mcp-server.md (1)
32-52: LGTM!Dependency-isolation and container-deployment guidance is internally consistent with
mcp/SECURITY.mdand the packaging contract tests (cryptography floor/override split,mcp/uv.lockusage, loopback-only Compose stack).Also applies to: 221-249
mcp/tests/test_deployment_assets.py (1)
38-50: LGTM!Strict allowlisted
COPYline contract plus the new root-Dockerfile--no-sources --no-deps -e .assertion give solid drift detection for the multi-stage build.Also applies to: 62-96
mcp/SECURITY.md (1)
27-52: LGTM!The
uv export/uv auditpreview-feature flags (sbom-export,audit-command,json-output) match documented uv usage, and the cryptography override rationale is consistent with the packaging tests.Also applies to: 80-101
mcp/tests/test_config_and_packaging.py (1)
129-216: LGTM!Root/MCP lock and source-of-truth split (workspace exclusion, scoped cryptography override, editable path mapping) is thoroughly and correctly asserted.
mcp/tests/test_release_checks.py (1)
176-220: LGTM!mcp/tests/test_jobs.py (1)
676-760: LGTM!Good coverage: transient heartbeat failures retry without leaking the simulated credential or raw job id, and unexpected heartbeat-task crashes still force cleanup with sanitized logs. Directly exercises the no-secret-leakage requirement.
mcp/scripts/check_license_inventory.py (2)
214-236: LGTM!
validate_lock_sources()correctly rejects any non-registry, non-editable source and enforces the exact local-path/registry contract; wiring it beforebuild_inventory()inmain()is the right order.Also applies to: 433-448
189-211: 🗄️ Data Integrity & IntegrationCheck
uv:workspace:pathhandling for local MCP packages.validate_sbomrejects any purl-less component withuv:workspace:path; ifuv export --preview-features sbom-exportstarts tagging the approved path/editable deps frommcp/pyproject.toml, CI will fail them even though they are allowed.
Require uv 0.11.25 or newer for scoped dependency overrides and make setup reject unsupported versions before environment creation. Align the documentation and packaging contracts, and correct the import boundary assertion for the isolated MCP environment. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Sanitize submission, polling, and report infrastructure exceptions before FastMCP serializes them. Preserve the original queued capability when an inline wait fails, add transport-level sentinel coverage, and document the stable failure contract. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Validate queued submit response fields before inline waits so malformed job data returns a public MCP error without exposing internal details. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Pin the MCP PostgreSQL service image and release artifact uploads to immutable references. Avoid exposing raw PostgreSQL connection errors from integration-test fixtures. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Guard failure persistence so queued jobs and running jobs owned by the current runner can transition to failed without overwriting terminal or foreign-owned work. Keep shallow submissions pollable if a local task exits with a nonterminal row, and cover claim and cancellation races in unit and PostgreSQL tests. Signed-off-by: Tanner Leach <tleach@nvidia.com>
_REPO_ROOT was computed as parents[3] of server.py, which is only the repo root in a source checkout. Installed non-editable (wheel or the release image), it resolved to the venv prefix, so a bare aiq-mcp-server run without AIQ_MCP_CONFIG failed with a nonsense site-packages path and the deploy/.env fallback silently never loaded. Detect the checkout by checking for configs/config_mcp.yml at the candidate root. When absent, disable the checkout defaults: require AIQ_MCP_CONFIG with a clear error and skip the default env-file load. Source-checkout and container behavior are unchanged. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Fix the Script Validation CI failure: the Dockerfile import canary loads aiq_mcp.server, which resolves its workflow config at import time, so the builder stage now sets AIQ_MCP_CONFIG for that step. Fold the inline python -c assertions into check_runtime_dependencies.py behind a new --verify-imports flag covering runtime imports, exact mcp/nvidia-nat-core release pins, and the tavily_web_search entry point. Bound anonymous submissions: submit_query now rejects queries longer than AIQ_MCP_MAX_QUERY_CHARS (default 8000) before any job is enqueued, with a static capability-free error. Document the cap and deployment-owned rate limiting in the MCP docs and SECURITY.md. The frozen three-tool contract is unchanged. Sanitize the aiq_api job failure path: persist only the exception class name in the job.error event and stored FAILURE status instead of raw exception text that could embed credentials or internal hostnames. Also add the missing blank line after the Skill Output heading in the aiq-research skill card (MD022). Signed-off-by: Tanner Leach <tleach@nvidia.com>
The job runner now persists only the exception class name instead of raw exception text, so the encrypted event-flush failure test must assert the sanitized error message rather than the injected RuntimeError text. Signed-off-by: Tanner Leach <tleach@nvidia.com>
#### Overview Refresh the AI-Q documentation against the live 2.2 milestone and current `develop` implementation while keeping branch-facing documentation portable across future release cuts. - keeps the root README version-free: “What’s New” highlights current capabilities without embedding release numbers, RC status, or a branch-cut lifecycle - keeps version-specific 2.2 targeting in the existing changelog, which is the detailed unreleased ledger; no separate release-notes document is introduced - makes the roadmap describe implementation in the checked-out branch rather than implying availability in a published release - updates configuration, deployment, quick-start, profiling, and docs-navigation wording to describe current behavior instead of a “2.2 candidate” - documents the newly merged artifact lifecycle (#314), Helm release-namespace behavior (#309), and async trace hierarchy (#321) - keeps the remaining open capabilities explicit: no per-job isolated/attested OpenShell lifecycle (#298) and no standalone public AI-Q MCP server (#319) - resolves Linette's review feedback across link-referral wording, terminology, and documentation clarity - preserves runtime boundaries for advisory routing, focused configuration profiles, MCP reconnect behavior, narrow forward-only encryption, best-effort artifact capture, and best-effort tokenomics phase attribution Milestone audit as of July 10, 2026: 49 items (47 PRs and 2 issues), including 41 PRs merged to `develop`, one PR merged only to `release/2.1`, three open PRs (#298, #319, and this documentation PR), and two closed-unmerged PRs superseded by merged work. AI-Q `v2.1.0` remains the latest stable release. `v2.2.0-rc1` is a prerelease snapshot from `develop`; there is no final `v2.2.0` tag yet, and `release/2.2` has not been cut. These lifecycle details intentionally remain outside the develop-facing README. #### Validation - `make -C docs SPHINXBUILD=../.venv/bin/sphinx-build SPHINXOPTS='-W --keep-going -n' html` - `make -C docs SPHINXBUILD=../.venv/bin/sphinx-build linkcheck` - `pre-commit run --all-files` - `git diff --check origin/develop...HEAD` - independent release-lifecycle, version-free wording, milestone, review-thread, and semantic whole-diff reviews found no remaining Critical or Important issues - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. Not applicable: this PR changes documentation only. - [x] I updated documentation for user-facing or contributor-facing changes. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with `git commit -s` or an equivalent sign-off. #### Where should reviewers start? 1. `README.md` for the version-free “What’s New” highlights and current-branch roadmap semantics. 2. `CHANGELOG.md` for the detailed unreleased 2.2 ledger and release lifecycle. 3. `docs/source/architecture/agents/deep-researcher.md` for the routed planner/researcher/writer contract. 4. `docs/source/architecture/data-flow.md` and `docs/source/integration/rest-api.md` for artifact checkpoint, SSE, replay, and authorization semantics. 5. `docs/source/deployment/kubernetes.md` and `docs/source/deployment/observability.md` for the newly merged namespace and trace-hierarchy behavior. #### Related Issues - Relates to the [AI-Q v2.2 milestone](https://github.com/NVIDIA-AI-Blueprints/aiq/milestone/1). - Documents merged changes from #309, #314, and #321. - Keeps open #298 and #319 explicitly out of the candidate scope. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated “Unreleased” release notes targeting AI-Q v2.2.0, including deep research workflow changes, async job reporting, and sandbox/artifact behavior. * Added/expanded REST API documentation for event-derived job state and durable artifact listing/streaming. * Documented OpenSearch support for knowledge retrieval and multiple paper-search providers, plus refined configuration, guardrails, MCP OAuth behavior, and observability trace hierarchy. * **Chores** * Refreshed secrets baseline metadata timestamps/line references only. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Ajay Thorve <athorve@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mcp/src/aiq_mcp/job_store.py (1)
250-274: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
record_poll's fallback path doesn't re-checkprincipal.When the
UPDATE ... RETURNINGmisses (terminal state or principal mismatch), the fallbackreturn await self.get(job_id)returns the row for any principal, unlikewait_for_completioninjobs.py, which explicitly checksjob.principal != principal. Not exploitable today since every caller shares the constantanonymousprincipal, but it's an inconsistent, latent authorization gap if multi-principal support is ever added.🛡️ Proposed fix
if row is not None: return _row_to_job(row) - return await self.get(job_id) + job = await self.get(job_id) + if job is not None and job.principal != principal: + return None + return job🤖 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 `@mcp/src/aiq_mcp/job_store.py` around lines 250 - 274, Update record_poll so its fallback lookup enforces the supplied principal before returning a job. After get(job_id), return the job only when it is absent or its principal matches principal; otherwise return None, while preserving the existing successful UPDATE path.mcp/tests/test_postgres_job_store.py (1)
640-670: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate Postgres test-infrastructure helpers across two test files.
_ensure_database,_maintenance_url,_quote_database_name, the safe-identifier regex, and thepostgres_urlfixture skip/reset lifecycle are reimplemented nearly identically in both files (this diff even makes the same one-line message tweak in both). A future correctness or security fix to identifier quoting would need to be applied twice.
mcp/tests/test_postgres_job_store.py#L640-L670: extract_ensure_database,_maintenance_url,_quote_database_name, and the identifier regex into a sharedmcp/tests/conftest.py(or a_pg_test_utilsmodule) and import it here.mcp/tests/test_checkpoint_todos.py#L420-L477: replace the duplicated_ensure_database/_maintenance_url/_quote_database_name/regex with the same shared utility.🤖 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 `@mcp/tests/test_postgres_job_store.py` around lines 640 - 670, Extract the shared Postgres test helpers _ensure_database, _maintenance_url, _quote_database_name, and the safe-identifier regex into one utility module, then import and reuse them in mcp/tests/test_postgres_job_store.py lines 640-670 and mcp/tests/test_checkpoint_todos.py lines 420-477. Remove the duplicated definitions while preserving each file’s existing postgres_url fixture skip/reset lifecycle and shared error-message behavior.
🤖 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 @.github/workflows/ci.yml:
- Around line 112-118: Update the “Validate isolated dependency environments”
workflow step to pass --verify-imports when invoking
mcp/scripts/check_runtime_dependencies.py with the production test-venv Python,
ensuring the import canary runs against the --no-dev --no-editable dependency
closure while preserving the existing dependency check.
In `@docs/source/contributing/pr-workflow.md`:
- Around line 30-36: Update the MCP validation section in pr-workflow.md to
include both lock checks: the root-project lock and the MCP project lock,
alongside the existing sync and pytest commands. Preserve the current MCP test
commands and ensure contributors are instructed to verify both lockfiles are
current.
In `@mcp/scripts/check_license_inventory.py`:
- Around line 214-236: Add tests for validate_lock_sources() covering the
approved local-source set and rejection of at least one unapproved registry or
malformed editable source. Use temporary lock files or equivalent fixtures so
the tests exercise parsing and validation through validate_lock_sources(),
including the expected success and ValueError paths.
In `@mcp/src/aiq_mcp/server.py`:
- Line 143: Implement per-caller rate limiting for the unauthenticated research
submission endpoint, covering the request paths around the handlers at lines
183-188, 215, and 413-423, rather than only enforcing max_query_chars. Prefer
the existing deployment’s reverse-proxy/ingress mechanism or add Starlette
middleware, and ensure excess requests are rejected before starting LLM/tool
research jobs.
---
Outside diff comments:
In `@mcp/src/aiq_mcp/job_store.py`:
- Around line 250-274: Update record_poll so its fallback lookup enforces the
supplied principal before returning a job. After get(job_id), return the job
only when it is absent or its principal matches principal; otherwise return
None, while preserving the existing successful UPDATE path.
In `@mcp/tests/test_postgres_job_store.py`:
- Around line 640-670: Extract the shared Postgres test helpers
_ensure_database, _maintenance_url, _quote_database_name, and the
safe-identifier regex into one utility module, then import and reuse them in
mcp/tests/test_postgres_job_store.py lines 640-670 and
mcp/tests/test_checkpoint_todos.py lines 420-477. Remove the duplicated
definitions while preserving each file’s existing postgres_url fixture
skip/reset lifecycle and shared error-message behavior.
🪄 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: 1e4a7271-3b16-4d49-83fb-a9eca46bebc4
⛔ Files ignored due to path filters (2)
mcp/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/SKILL.md.agents/skills/aiq-release-qa/references/validation-matrix.md.coderabbit.yaml.github/workflows/ci.yml.pre-commit-config.yaml.secrets.baselineAGENTS.mdCONTRIBUTING.mdLICENSE-THIRD-PARTYREADME.mddeploy/Dockerfiledocs/source/contributing/code-style.mddocs/source/contributing/pr-workflow.mddocs/source/contributing/testing.mddocs/source/deployment/docker-build.mddocs/source/get-started/installation.mddocs/source/integration/mcp-server.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pymcp/Dockerfilemcp/README.mdmcp/REFERENCE_PARITY.mdmcp/SECURITY.mdmcp/pyproject.tomlmcp/scripts/check_license_inventory.pymcp/scripts/check_runtime_dependencies.pymcp/src/aiq_mcp/job_store.pymcp/src/aiq_mcp/jobs.pymcp/src/aiq_mcp/server.pymcp/tests/test_checkpoint_todos.pymcp/tests/test_client_session_integration.pymcp/tests/test_config_and_packaging.pymcp/tests/test_dependency_compatibility.pymcp/tests/test_deployment_assets.pymcp/tests/test_imports.pymcp/tests/test_jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_release_checks.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_server_runtime.pypyproject.tomlscripts/dev.shscripts/setup.shskills/aiq-research/skill-card.mdtests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{py,ts,tsx,js,jsx,yml,yaml,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and
SecretStr, and resolve API keys at runtime.
Files:
docs/source/contributing/pr-workflow.mddocs/source/get-started/installation.mdCONTRIBUTING.mdmcp/REFERENCE_PARITY.mdmcp/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pydocs/source/contributing/code-style.mdskills/aiq-research/skill-card.mdREADME.mdAGENTS.mdtests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pydocs/source/contributing/testing.mdmcp/SECURITY.mddocs/source/integration/mcp-server.mddocs/source/deployment/docker-build.mdmcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update documentation when behavior, configuration, or workflows change; keep documentation canonical rather than duplicating full documentation pages into skills.
Files:
docs/source/contributing/pr-workflow.mddocs/source/get-started/installation.mddocs/source/contributing/code-style.mddocs/source/contributing/testing.mddocs/source/integration/mcp-server.mddocs/source/deployment/docker-build.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Stay within this repository, do not edit adjacent repositories, and keep changes tosources/*scoped to the smallest relevant independent package.
Run the narrowest relevant validation command first and broaden to the full suite only when a change crosses shared boundaries.
Keep the rootuv.lockandmcp/uv.lockas separate dependency resolutions; scope thecryptography==48.0.1override tomcp/, while the root usescryptography>=46.0.6,<47.
Keep pull requests scoped, avoid unrelated files and generated artifacts, do not include secrets, and provide validation evidence.
Every commit must include DCO sign-off usinggit commit -s, with aSigned-off-bytrailer.
For substantial behavior, authentication, UI, or architecture changes, open a design discussion before coding.
**/*: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts in contributions.
Add or update tests for behavior changes.
Run the narrowest local validation command that covers the change and include exact output or a workflow link in the pull request.
For deployment changes, run the relevant Helm or Compose validation and describe the environment used.
Target thedevelopbranch unless a maintainer explicitly requests a release branch.
Sign off every commit withgit commit -s; commits without sign-off may be rejected.
Open an issue or discussion before large design changes, public APIs, deployment changes, or contributor workflow changes.
Make the smallest coherent change and address review feedback until required checks and code-owner review pass.
Files:
docs/source/contributing/pr-workflow.mdLICENSE-THIRD-PARTYdocs/source/get-started/installation.mdCONTRIBUTING.mdscripts/setup.shmcp/REFERENCE_PARITY.mdmcp/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pydocs/source/contributing/code-style.mdskills/aiq-research/skill-card.mdREADME.mddeploy/Dockerfilemcp/DockerfileAGENTS.mdtests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pyscripts/dev.shmcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pydocs/source/contributing/testing.mdmcp/SECURITY.mddocs/source/integration/mcp-server.mdmcp/pyproject.tomldocs/source/deployment/docker-build.mdmcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pypyproject.tomlmcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
docs/source/contributing/**/*.{py,md,ipynb}
📄 CodeRabbit inference engine (docs/source/contributing/code-style.md)
Run pre-commit checks across all files; these include Ruff checks and formatting, lock-file checks, secret detection, notebook output clearing, and Markdown link validation.
Files:
docs/source/contributing/pr-workflow.mddocs/source/contributing/code-style.mddocs/source/contributing/testing.md
docs/source/contributing/**/*
📄 CodeRabbit inference engine (docs/source/contributing/pr-workflow.md)
For each change, run the narrowest relevant local validation checks and record the exact commands in the pull request description, including repository checks (
uv run ruff check .,uv run ruff format --check ., anduv run pytest), MCP checks when applicable, and UI checks when applicable.
Files:
docs/source/contributing/pr-workflow.mddocs/source/contributing/code-style.mddocs/source/contributing/testing.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/contributing/pr-workflow.mddocs/source/get-started/installation.mdCONTRIBUTING.mddocs/source/contributing/code-style.mdREADME.mddocs/source/contributing/testing.mddocs/source/integration/mcp-server.mddocs/source/deployment/docker-build.md
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Ruff for Python linting and formatting with line length 120, target Python 3.11, rule sets E, F, W, I, PL, and UP; use single-line imports as configured.
Never print or log secret values, including in tool output or error messages.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,pyi}: Runuv run ruff check .anduv run ruff format --check .for Python changes.
Runuv run pytestfor relevant Python changes.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/jobs/test_runner.pymcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.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/jobs/runner.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/skill-card.md.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-release-qa/SKILL.md
{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/Dockerfile
.agents/skills/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use maintainer skills from
.agents/skills/for task-specific repository workflows; do not treat API-consumer skills inskills/as an in-product runtime.
Files:
.agents/skills/aiq-maintain-ci/SKILL.md.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md.agents/skills/aiq-release-qa/references/validation-matrix.md.agents/skills/aiq-release-qa/SKILL.md
mcp/**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For changes under
mcp/, run the MCP project's development dependency sync anduv run --project mcp --extra dev pytest mcp/tests.
Files:
mcp/tests/test_runtime_dependencies.pymcp/tests/test_deployment_assets.pymcp/tests/test_dependency_compatibility.pymcp/scripts/check_runtime_dependencies.pymcp/tests/test_client_session_integration.pymcp/tests/test_release_checks.pymcp/src/aiq_mcp/job_store.pymcp/tests/test_imports.pymcp/tests/test_checkpoint_todos.pymcp/src/aiq_mcp/server.pymcp/scripts/check_license_inventory.pymcp/tests/test_config_and_packaging.pymcp/tests/test_server_runtime.pymcp/src/aiq_mcp/jobs.pymcp/tests/test_postgres_job_store.pymcp/tests/test_jobs.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/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:
.pre-commit-config.yamlmcp/pyproject.toml.github/workflows/ci.ymlpyproject.toml
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: Create a focused branch from `develop` and open pull requests into `develop`, unless a maintainer directs the contributor to target a release branch.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: Address review feedback until required checks, code-owner review, and review-thread resolution pass before merge.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: All contributors must sign off their commits using the Developer's Certificate of Origin, for example with `git commit -s -m "Your message"`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:49.321Z
Learning: Contributions must satisfy the Developer's Certificate of Origin 1.1: the contributor must have the right to submit the work under the applicable open-source license, and acknowledges that contribution and sign-off records are public and retained indefinitely.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:53.553Z
Learning: Use the documented `uv` commands to install dependencies and run the main project or MCP project tests.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:53.553Z
Learning: Refer to each benchmark's README for benchmark-specific instructions and consult the customization guide when adding evaluation harnesses.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-07-14T05:27:53.553Z
Learning: Use verbose CLI logging, Phoenix tracing, and the documented troubleshooting steps when debugging.
🪛 ast-grep (0.44.1)
mcp/scripts/check_runtime_dependencies.py
[info] 134-134: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
mcp/tests/test_server_runtime.py
[warning] 1034-1034: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1083-1083: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1158-1158: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
[warning] 1202-1202: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://localhost"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(llm-client-insecure-http-python)
🪛 OpenGrep (1.25.0)
mcp/src/aiq_mcp/job_store.py
[ERROR] 166-181: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 SkillSpector (2.3.11)
.agents/skills/aiq-release-qa/SKILL.md
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
[error] 112: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🔇 Additional comments (59)
LICENSE-THIRD-PARTY (1)
8-11: LGTM!docs/source/get-started/installation.md (1)
15-15: LGTM!Also applies to: 49-49
CONTRIBUTING.md (1)
32-42: LGTM!scripts/setup.sh (3)
6-38: LGTM!
65-74: LGTM!Also applies to: 89-98
169-170: LGTM!mcp/REFERENCE_PARITY.md (2)
1-75: LGTM!
76-84: 📐 Maintainability & Code Quality
mcp/tests/test_workflow_runner.pyexists, so this reference is valid.> Likely an incorrect or invalid review comment.frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
872-888: LGTM!mcp/src/aiq_mcp/job_store.py (5)
1-106: LGTM!
107-183: LGTM!
184-234: LGTM!
275-358: LGTM!
359-393: LGTM!mcp/README.md (2)
1-179: LGTM! The rest of the documentation (component layout, anonymous capability model, Host/Origin/CORS guidance, dependency isolation, container deployment) is internally consistent and matches the coding guidelines for scopingcryptography==48.0.1tomcp/.
34-36: 🎯 Functional CorrectnessNo issue:
GET /mcpis explicitly rejected The app routesself.settings.pathto a handler that returns405withAllow: POST, so the README matches the implementation.> Likely an incorrect or invalid review comment..coderabbit.yaml (1)
99-99: LGTM! Expanding the automation/packaging path glob to covermcp/pyproject.tomlandmcp/uv.lockmatches the path instructions for reviewing packaging changes.docs/source/contributing/code-style.md (1)
24-25: LGTM!.secrets.baseline (1)
130-138: LGTM!Also applies to: 348-358
mcp/tests/test_dependency_compatibility.py (1)
26-31: LGTM! Thecryptographypin matches the documentedmcp/uv.lockoverride.mcp/tests/test_checkpoint_todos.py (1)
41-51: LGTM! Including onlytype(exc).__name__(not the raw exception message, which could contain the DSN) keeps the skip/warning message free of connection secrets.mcp/tests/test_postgres_job_store.py (2)
57-67: LGTM!
393-468: LGTM! This test precisely covers themark_failed_if_queued_or_ownedownership/state contract (queued, self-owned running, foreign-owned running, complete, already-failed), matching the guarded UPDATE predicate injob_store.py.skills/aiq-research/skill-card.md (1)
33-35: LGTM!README.md (2)
234-244: LGTM!Also applies to: 305-326
312-315: 🎯 Functional CorrectnessThe
aiq-mcp-serverconsole script is declared inmcp/pyproject.toml, so the README command is valid.> Likely an incorrect or invalid review comment.deploy/Dockerfile (1)
81-90: LGTM!--no-sourcescorrectly prevents uv from attempting workspace-member discovery for[tool.uv.sources]entries during the--no-deps -e .install, matching the stated rationale.mcp/Dockerfile (1)
10-57: LGTM! Minimal, well-justified copy set and frozen--no-dev --no-default-groups --no-editablesync for the release closure; the import canary correctly wiresAIQ_MCP_CONFIGto the copied config..agents/skills/aiq-maintain-ci/SKILL.md (1)
39-52: LGTM!mcp/tests/test_release_checks.py (1)
176-238: LGTM! These three tests matchvalidate_sbom's actual branch logic (local-source allowlist check vs. public PyPI purl contract vs. final set-equality check).mcp/scripts/check_license_inventory.py (1)
178-213: LGTM!validate_sbomlogic matches the exact test expectations reviewed inmcp/tests/test_release_checks.py.mcp/tests/test_jobs.py (1)
67-182: LGTM! Solid coverage of claim/cancellation races, heartbeat retry, and log/error redaction — verified against the realJobManager/JobStorecontracts referenced in this review's graph context.Also applies to: 665-1172
AGENTS.md (1)
43-43: LGTM!Also applies to: 55-61
tests/aiq_agent/jobs/test_runner.py (1)
886-894: LGTM!mcp/tests/test_deployment_assets.py (1)
16-16: LGTM!Also applies to: 38-50, 62-88, 91-96
.agents/skills/aiq-maintain-ci/references/workflows-and-hooks.md (1)
13-19: LGTM!Also applies to: 38-51
.agents/skills/aiq-release-qa/references/validation-matrix.md (1)
13-19: LGTM!Also applies to: 50-62
pyproject.toml (2)
184-185: LGTM!
50-50: 🗄️ Data Integrity & Integration | ⚡ Quick winRoot
cryptographyconstraint: verify[project].dependenciesvs.[tool.uv.override-dependencies]and documentation. The change summary sayspyproject.toml's directcryptographydependency moved to>=48.0.1,<49, but the shownoverride-dependenciesstill pins>=46.0.6,<47, andAGENTS.mddocuments the root staying on<47. Since uv'soverride-dependenciesforces the resolved version regardless of the direct dependency declaration, either the bump is a no-op that should be removed/reverted, or the override needs to be updated and the docs are now stale.
pyproject.toml#L50-L50: confirm whether the[project].dependenciescryptography bump to>=48.0.1,<49is intentional; if so, update or remove the conflicting>=46.0.6,<47entry in[tool.uv.override-dependencies]so the direct dependency isn't silently overridden.AGENTS.md#L105-L109: update the documented root cryptography range if the intended behavior changes, or leave as-is oncepyproject.tomlis confirmed to still resolve to<47in practice.Source: Coding guidelines
mcp/tests/test_server_runtime.py (1)
163-249: LGTM!Also applies to: 299-300, 925-943, 977-1222
mcp/src/aiq_mcp/jobs.py (1)
211-232: LGTM! Confirms the previously flagged heartbeat-retry issue is fixed, and the cancellation/failure-persistence paths correctly rely on guarded state-conditioned store writes to avoid clobbering already-terminal jobs.Also applies to: 317-325, 368-424
.agents/skills/aiq-release-qa/SKILL.md (2)
111-112: Static-analysis false positive.The "credential access" flag on the
deploy/.envreference is a false positive — this is a doc instructing contributors how to invokenat evalwithdotenv, not code that reads/logs a secret file.
38-41: LGTM!Also applies to: 72-88, 104-108
mcp/scripts/check_runtime_dependencies.py (2)
134-134: Static-analysis false positive.The "use jsonify instead of json.dumps" hint doesn't apply — this is a standalone CLI script printing to stdout, not a web response body.
108-138: LGTM!mcp/tests/test_runtime_dependencies.py (1)
80-119: LGTM!scripts/dev.sh (1)
6-13: LGTM!Also applies to: 61-61
mcp/tests/test_imports.py (1)
32-41: LGTM!.github/workflows/ci.yml (2)
249-256: 🔒 Security & PrivacyConfirm the ignored advisory is still unresolved and scoped correctly.
GHSA-f4j7-r4q5-qw2cis suppressed via--ignore-until-fixed, which auto-reinstates the gate once a fix ships. Worth a periodic sanity check (or a tracked follow-up) that this advisory still applies to a real dependency inmcp/uv.lockand hasn't been superseded/fixed, since GitHub's advisory record for this ID shows no associated ecosystem package.
37-53: LGTM!Also applies to: 67-85, 99-119, 120-139, 140-165, 187-237, 257-321
mcp/tests/test_client_session_integration.py (2)
443-446: 🔒 Security & PrivacyPreviously flagged raw-exception-in-message issue is fixed here (
type(exc).__name__instead ofexc).
133-140: LGTM!docs/source/contributing/testing.md (1)
10-17: LGTM!.pre-commit-config.yaml (1)
25-31: LGTM!Also applies to: 61-66, 78-89
mcp/SECURITY.md (1)
6-133: LGTM!docs/source/integration/mcp-server.md (1)
18-274: LGTM!docs/source/deployment/docker-build.md (1)
39-73: LGTM!mcp/tests/test_config_and_packaging.py (1)
158-224: LGTM!mcp/pyproject.toml (1)
59-77: 🔒 Security & PrivacyKeep the current prerelease policy.
mcp/SECURITY.mdalready documents prerelease-aware resolution as part of the MCP release profile, and thedefusedxmlcap exists specifically because of that policy.tornado>=6.5.5can stay open if that release line is meant to float.> Likely an incorrect or invalid review comment.
| - name: Validate isolated dependency environments | ||
| run: | | ||
| set -euo pipefail | ||
| . .venv/bin/activate | ||
| python -X faulthandler -m pytest tests/ -v \ | ||
| uv pip check | ||
| "$RUNNER_TEMP/aiq-mcp-test-venv/bin/python" \ | ||
| mcp/scripts/check_runtime_dependencies.py |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- Searching for --verify-imports usage across the repo ---"
rg -n '\-\-verify-imports' --hidden -g '!**/.git/**'
echo "--- Inspecting mcp/Dockerfile for the canary invocation ---"
fd -H Dockerfile mcp | xargs -r cat -nRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ci workflow relevant sections ==\n'
sed -n '100,130p' .github/workflows/ci.yml
printf '\n== workflow production environment section ==\n'
sed -n '220,250p' .github/workflows/ci.yml
printf '\n== runtime dependency script outline ==\n'
ast-grep outline mcp/scripts/check_runtime_dependencies.py --view expanded
printf '\n== search for verify_imports call sites ==\n'
rg -n --hidden --glob '!**/.git/**' --glob '!**/.venv/**' --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/node_modules/**' 'verify_imports|--verify-imports|verify-runtime-imports|verify_runtime_imports'
printf '\n== mcp/Dockerfile (if present) ==\n'
if [ -f mcp/Dockerfile ]; then
cat -n mcp/Dockerfile
else
echo 'mcp/Dockerfile not found'
fiRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 2931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== check_runtime_dependencies.py relevant lines ==\n'
sed -n '1,170p' mcp/scripts/check_runtime_dependencies.py
printf '\n== main() argument parsing only ==\n'
python3 - <<'PY'
from pathlib import Path
text = Path('mcp/scripts/check_runtime_dependencies.py').read_text()
for needle in ['ArgumentParser', '--verify-imports', 'verify_runtime_imports', 'validate_environment', 'main(']:
print(f'\n## {needle}')
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f'{i}: {line}')
PYRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 6043
Run the import canary in the production venv. check_runtime_dependencies.py runs in both environments, but --verify-imports is never passed, so CI can still miss runtime import or entry-point failures in the --no-dev --no-editable closure.
🤖 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 @.github/workflows/ci.yml around lines 112 - 118, Update the “Validate
isolated dependency environments” workflow step to pass --verify-imports when
invoking mcp/scripts/check_runtime_dependencies.py with the production test-venv
Python, ensuring the import canary runs against the --no-dev --no-editable
dependency closure while preserving the existing dependency check.
| For MCP changes, validate its independent project as well: | ||
|
|
||
| ```bash | ||
| uv sync --project mcp --extra dev | ||
| uv run --project mcp --extra dev pytest mcp/tests | ||
| ``` | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Missing lock-check reminder present in CONTRIBUTING.md.
CONTRIBUTING.md's equivalent MCP section also requires uv lock --check and uv lock --project mcp --check to keep both locks current. This section omits that, so a contributor following only this doc could miss a lock-drift failure in CI.
🤖 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 `@docs/source/contributing/pr-workflow.md` around lines 30 - 36, Update the MCP
validation section in pr-workflow.md to include both lock checks: the
root-project lock and the MCP project lock, alongside the existing sync and
pytest commands. Preserve the current MCP test commands and ensure contributors
are instructed to verify both lockfiles are current.
| def validate_lock_sources(lock_path: Path = _MCP_LOCK_PATH) -> None: | ||
| """Preserve path provenance that uv's CycloneDX export does not encode.""" | ||
| lock = tomllib.loads(lock_path.read_text()) | ||
| expected_local = { | ||
| **_LOCAL_SOURCE_COMPONENTS, | ||
| ("aiq-mcp-server", "0.1.0"): ".", | ||
| } | ||
| observed_local: dict[tuple[str, str], str] = {} | ||
| for package in lock.get("package", []): | ||
| name = _canonicalize(str(package.get("name", ""))) | ||
| version = str(package.get("version") or "") | ||
| source = package.get("source", {}) | ||
| if source.get("registry") is not None: | ||
| if source != {"registry": "https://pypi.org/simple"}: | ||
| raise ValueError(f"dependency uses an unapproved registry source: {name}=={version}") | ||
| continue | ||
| editable = source.get("editable") | ||
| if not isinstance(editable, str): | ||
| raise ValueError(f"dependency uses an unapproved lock source: {name}=={version}") | ||
| observed_local[(name, version)] = editable | ||
|
|
||
| if observed_local != expected_local: | ||
| raise ValueError("MCP lock local sources differ from the approved public source contract") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'validate_lock_sources' mcp/testsRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== check_license_inventory.py outline ==\n'
ast-grep outline mcp/scripts/check_license_inventory.py --view expanded || true
printf '\n== test_release_checks.py outline ==\n'
ast-grep outline mcp/tests/test_release_checks.py --view expanded || true
printf '\n== references to validate_lock_sources across mcp ==\n'
rg -n 'validate_lock_sources|lock sources|local source' mcpRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 3402
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== validate_lock_sources implementation ==\n'
sed -n '214,236p' mcp/scripts/check_license_inventory.py
printf '\n== release check tests around SBOM coverage ==\n'
sed -n '176,235p' mcp/tests/test_release_checks.pyRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 3394
Add coverage for validate_lock_sources() This release gate runs from main(), but no test exercises it yet; add an accept case for the approved lock-source set and at least one reject case for an unapproved registry or malformed local source.
🤖 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 `@mcp/scripts/check_license_inventory.py` around lines 214 - 236, Add tests for
validate_lock_sources() covering the approved local-source set and rejection of
at least one unapproved registry or malformed editable source. Use temporary
lock files or equivalent fixtures so the tests exercise parsing and validation
through validate_lock_sources(), including the expected success and ValueError
paths.
| cors_origins: tuple[str, ...] | ||
| allowed_hosts: tuple[str, ...] | ||
| allowed_origins: tuple[str, ...] | ||
| max_query_chars: int = 8000 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial
Query-size bound now enforced; per-caller rate limiting still absent.
max_query_chars closes the size-bound half of the earlier finding. The rate-limiting half ("a per-IP/global submission rate limit... given this endpoint is explicitly designed to be unauthenticated") is still not implemented anywhere in this diff — an anonymous caller can still submit unlimited concurrent research jobs, each triggering LLM+tool workflows.
Consider adding rate limiting at the reverse-proxy/ingress layer or via Starlette middleware, since this is explicitly a public, unauthenticated tool.
Also applies to: 183-188, 215-215, 413-423
🤖 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 `@mcp/src/aiq_mcp/server.py` at line 143, Implement per-caller rate limiting
for the unauthenticated research submission endpoint, covering the request paths
around the handlers at lines 183-188, 215, and 413-423, rather than only
enforcing max_query_chars. Prefer the existing deployment’s
reverse-proxy/ingress mechanism or add Starlette middleware, and ensure excess
requests are rejected before starting LLM/tool research jobs.
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
Review against the current head, including an isolated release-container run with the credentials from deploy/.env.
The unit/protocol suites and image build are green, but the credentialed shallow and deep paths both returned generic error text while the MCP ledger reported state="complete". The inline comments cover that state-contract failure, the standalone shallow runtime failure, retention leakage, duplicate classification, the non-installable wheel, the dropped Compose setting, and the missing report-follow-up surface.
GitHub also currently reports this head as CONFLICTING / DIRTY against develop (.secrets.baseline, README.md, and uv.lock), so the checks need to be rerun after the branch is updated.
| # If a swallowed workflow error was captured during the run, surface | ||
| # it as the job's failure reason instead of the polite fallback text | ||
| # the agent layer returned as ``result``. | ||
| captured = _FAILURE_HANDLER.consume(job_id) |
There was a problem hiding this comment.
[P1] Do not infer workflow success from the absence of one captured log exception.
I ran the release container with the configured NVIDIA/Tavily credentials. A shallow query returned An error occurred while researching your question...; a deep query returned I ran into an error while producing that report...; both jobs were persisted as state="complete". The chat workflow catches unexpected shallow/deep exceptions and returns fallback text, while _WorkflowFailureCapture only recognizes EmptySourceRegistryError (plus one brittle message match). Every other swallowed failure reaches this else and becomes a successful job.
Please make the workflow boundary return a structured terminal outcome (success vs failed, with a stable public error) and persist that directly. Add deterministic coverage where the workflow produces its fallback/error outcome and assert that poll/final-report return failed, not complete.
| "Programming Language :: Python :: 3.13", | ||
| ] | ||
| dependencies = [ | ||
| "aiq-agent>=2.0,<3", |
There was a problem hiding this comment.
[P1] The standalone release image's default shallow path is currently broken.
The image installs aiq-agent but not aiq_api. In shallow_researcher/register.py, PerUserMcpSourceUnavailableError is imported inside the same try whose except references it. When the earlier aiq_api import fails in this standalone environment, exception matching itself raises UnboundLocalError because that local name was never bound. The credentialed release-container query reproduced exactly that failure.
Please separate optional aiq_api integration loading from the exception type used by the handler (or provide a dependency-free fallback), and add a release-image shallow query test so the import canary covers actual workflow execution rather than only module imports.
|
|
||
| async def delete_expired(self) -> int: | ||
| pool = self._require_pool() | ||
| status = await pool.execute(f"DELETE FROM {self._jobs_table} WHERE expires_at < NOW()") |
There was a problem hiding this comment.
[P1] Expiring the MCP ledger row must also remove the LangGraph checkpoint state for the same job/thread ID.
This currently deletes only mcp_jobs. In a live PostgreSQL probe, expiring and deleting one job left 4 checkpoint rows, 6 blob rows, and 14 write rows; a single deep run created 13 checkpoints, 8 blobs, and 27 writes. Those rows can contain the query and intermediate model/tool state, so the advertised 24-hour job TTL neither bounds storage nor actually expires the retained research data.
Please delete the associated checkpoint tables atomically in the correct FK order (or configure an equivalent checkpoint TTL/partitioning policy), and add an integration test that creates real checkpoint rows and proves the full job footprint is removed.
| shallow/deep the return additionally includes `estimated_duration_seconds` and | ||
| `first_poll_after_seconds` (state="queued"). | ||
| """ | ||
| classification = await self._runner.classify(query) |
There was a problem hiding this comment.
[P1] Avoid classifying every research query twice.
submit() classifies here to persist depth and choose inline/queued cadence, but _run_job() then calls WorkflowRunner.run_query(), which starts the full chat graph at its intent_classifier entry point. The configured intent model uses temperature: 0.5, so the second classification can choose a different route. The public job depth, duration, and polling contract can therefore describe different work from what actually executes, while also paying classifier latency/cost twice.
Please carry this classification into the workflow/router (or make one component own classification end to end) and add a test asserting the persisted depth and executed route are the same decision.
| ] | ||
|
|
||
| [tool.uv.sources] | ||
| aiq-agent = { path = "..", editable = true } |
There was a problem hiding this comment.
[P1] The documented standalone wheel is not installable outside this source checkout.
I built aiq_mcp_server-0.1.0-py3-none-any.whl and attempted a clean Python 3.13 install from the public index. Resolution failed because aiq-agent is not available there; tavily-web-search is likewise supplied only by this local source override. [tool.uv.sources] is not wheel metadata, so CI and Docker pass only because they retain the monorepo layout.
Please test the released wheel with sources disabled against the actual publication index and publish its complete dependency closure. If the container/source checkout is the only supported artifact, the README should stop advertising this as a standalone generic wheel.
| AIQ_MCP_PATH: "${AIQ_MCP_PATH:-/mcp}" | ||
| AIQ_MCP_WORKERS: "${AIQ_MCP_WORKERS:-1}" | ||
| AIQ_MCP_LOG_LEVEL: "${AIQ_MCP_LOG_LEVEL:-INFO}" | ||
| AIQ_MCP_SHALLOW_INLINE_WAIT_SECONDS: "${AIQ_MCP_SHALLOW_INLINE_WAIT_SECONDS:-30}" |
There was a problem hiding this comment.
[P2] Pass AIQ_MCP_MAX_QUERY_CHARS through the supported Compose deployment.
The server implements and documents this setting, but the Compose environment omits it. Rendering the stack with AIQ_MCP_MAX_QUERY_CHARS=1 confirmed that the value never reaches the container, so operators cannot tighten the public input bound through the documented deployment path. Please add the interpolation here and cover it in test_deployment_assets.py.
|
|
||
| | Area | Public parity contract | | ||
| |---|---| | ||
| | Tool surface | Exactly `submit_query(query)`, `poll_query(job_id)`, and `get_final_report(job_id)`, in that order. Each argument is one required unconstrained string and each output is a generic object. | |
There was a problem hiding this comment.
[Feature gap] Please include the report follow-up capabilities before freezing the public surface to exactly these three tools.
AI-Q now supports two distinct report-aware operations over durable parent-report context: bounded report_ask Q&A and report_edit, which produces a revised-report child job. This MCP surface can submit and retrieve a report, but it cannot perform either follow-up without forcing the client to start unrelated fresh research.
I recommend first-class tools such as ask_report(job_id, question) and edit_report(job_id, instruction), with the edit returning a child capability compatible with the existing poll/final-report lifecycle. Preserve the current bearer-capability and TTL rules, and update the parity hash/golden tests. If follow-up is deliberately deferred, document that as an explicit product gap and identify the intended follow-up release rather than freezing the omission as parity.
5 participants
Overview
Add a standalone, public AI-Q MCP server under
mcp/without the MaaS/MOS SDK. The server exposes exactly three stateless Streamable HTTP tools—submit_query,poll_query, andget_final_report—and keeps asynchronous research jobs in PostgreSQL while reusing the existing AI-Q/NAT workflow.This change also adds the public NIM + Tavily workflow config, container/Compose assets, user and security documentation, compatibility tests, release smoke tests, dependency/license evidence, and protected CI coverage.
Key scope and security boundaries:
anonymousprincipal, and each random job UUID is a bearer capability.AIQ_MCP_ALLOWED_HOSTS,AIQ_MCP_ALLOWED_ORIGINS, and CORS are DNS-rebinding/browser safeguards; they are not identity or authorization controls.The frozen compatibility contract and intentional public adaptations are documented in
mcp/REFERENCE_PARITY.md. Tool schemas, state transitions, polling cadence, PostgreSQL persistence, background-task ownership, todo normalization, and stable client error shapes remain compatible. The intentional adaptations are FastMCP transport/lifecycle ownership, anonymous capability access, public configuration, capability-safe logging, and deployment-owned TLS.Release-review follow-ups are explicit in
mcp/SECURITY.md:manual_review_requiredfor the releasing organization's legal/NOTICE review;Validation
uv run pre-commit run --all-files— all hooks passed, including Ruff, lock validation, secret detection, YAML checks, and Markdown link checks.PostgreSQL-backed MCP suite — 174 passed, 0 skipped, 96.67% coverage (90% gate).
Complete workspace suite — 1,542 passed, 8 skipped, 0 failed.
Frozen tool-contract extraction — exact ordered three-tool contract SHA-256
81eba67fadd56e64b58a84b700b202841f8636c93c6cbf63752507c8bf5ca96a.actionlint .github/workflows/ci.yml— passed with actionlint v1.7.12.Sphinx documentation build — 62 pages built; only three unrelated pre-existing missing-xref warnings.
Production-only
uv sync --frozen --package aiq-mcp-server --no-dev --no-default-groups --no-editableplusmcp/scripts/check_runtime_dependencies.py— passed.Unfiltered and exception-gated
uv audit, CycloneDX 1.5 SBOM generation, and exact license inventory — passed the documented engineering gates; evidence is archived byScript Validation.MCP wheel build — verified embedded Apache-2.0 license metadata.
Release Compose image boot — exact
/liveand/healthpayloads,GET /mcp405 contract, anonymous initialization, exact tool listing, and unknown-capability response passed throughmcp/scripts/protocol_smoke.py.I ran the relevant local checks or explained why they are not applicable.
I added or updated tests for behavior changes.
I updated documentation for user-facing or contributor-facing changes.
I confirmed this PR does not include secrets, credentials, or internal-only data.
I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with
git commit -sor an equivalent sign-off.Where should reviewers start?
docs/source/integration/mcp-server.mdfor the user-facing protocol, deployment, and security model.mcp/REFERENCE_PARITY.mdfor the compatibility boundary and intentional differences.mcp/src/aiq_mcp/server.pyandmcp/src/aiq_mcp/jobs.pyfor transport/lifecycle and state behavior..github/workflows/ci.ymlandmcp/SECURITY.mdfor mandatory test, SBOM, audit, license, and release-image gates.Related Issues
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests / CI