feat(api): encrypt async job content artifacts#291
Conversation
Encrypt async final report output and selected artifact event content at rest, including static-key and Vault Transit modes, health/readiness checks, report read decryption, and coverage for worker, route, and event-store behavior. 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds async job content encryption for report output and selected artifact event fields. It introduces static-key and Vault Transit modes, wires encryption into job execution, event persistence, and routes, and updates deployment docs and tests. ChangesAsync Job Content Encryption
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant JobsRoute
participant ContentEncryptionManager
participant Worker
participant EventStore
Client->>JobsRoute: POST /v1/jobs/async/submit
JobsRoute->>ContentEncryptionManager: validate readiness + get policy identity
JobsRoute->>Worker: submit_job(..., content_encryption_policy)
Worker->>ContentEncryptionManager: create_job_content_cipher(job_id)
Worker->>EventStore: store encrypted events / output
Client->>JobsRoute: GET /v1/jobs/async/job/{job_id}/report
JobsRoute->>ContentEncryptionManager: read_job_output_async(job_id, stored_output)
ContentEncryptionManager-->>JobsRoute: decrypted report
JobsRoute-->>Client: report JSON
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Document the default-off behavior, exact encrypted field scope, Vault concurrency and retry model, forward-only rollout constraints, stable key identity requirement, and database-writer replay limitation. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Protect shared encryption state under concurrency, offload blocking Vault work from async routes, surface encrypted event failures, and harden Vault readiness and retry behavior. Add regression coverage for concurrency, async responsiveness, readiness, retry classification, explicit API and SSE failures, and default-off configuration. 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 (1)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
458-462: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate the submit route response contract for encryption failures.
The handler now returns
503for encryption unready and500for invalid encryption config, but OpenAPI still documents503only as Dask unavailable and omits500. As per path instructions, “Treat API, auth, and job-runner changes as externally visible contracts.”Proposed fix
responses={ - 503: {"description": "Dask scheduler not available"}, + 500: {"description": "Content encryption configuration is invalid"}, + 503: {"description": "Dask scheduler not available or content encryption is not ready"}, },Also applies to: 477-533
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/aiq_api/src/aiq_api/routes/jobs.py` around lines 458 - 462, The submit route response contract in jobs.py is out of sync with the handler’s encryption error paths: it documents only 503 for Dask availability and does not include the new 500 invalid-encryption-config case. Update the OpenAPI responses for the submit route handler (the route around the submit endpoint, including the shared responses used across the related blocks) so encryption-unready is documented as 503 and invalid encryption config is documented as 500, while keeping the existing 400 and 422 entries intact.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/aiq_api/src/aiq_api/jobs/crypto.py`:
- Around line 25-33: Reject non-finite numeric environment values in the numeric
parsing/validation path used by the crypto job settings, since float("nan") and
float("inf") currently slip through. Update the validator/helper that reads
these environment values so it explicitly requires finite numbers before
accepting TTLs/timeouts, and apply the same check anywhere the shared parsing
logic is reused in the referenced sections. Use the existing env-setting symbols
in this module to locate the validator and keep the rejection behavior
consistent across readiness TTL, cache TTL, and Vault timeout inputs.
- Around line 904-912: In decode_envelope, malformed non-ASCII envelope payloads
are not being mapped to the encrypted-data error because _b64url_decode() can
raise UnicodeEncodeError. Update the exception handling in decode_envelope to
catch UnicodeEncodeError alongside the existing decode/JSON/value errors and
re-raise ContentEncryptionInvalidData, so all malformed aiqenc envelopes are
classified consistently.
- Around line 99-147: ContentEncryptionConfig is exposing secret-bearing fields
in both dataclass repr and the signature tuple. Update the
ContentEncryptionConfig dataclass to mark static_key, vault_role_id, and
vault_secret_id as repr=False, and change the signature property to use a
non-reversible fingerprint or placeholder derived from those values instead of
including the raw credentials directly. Keep the rest of the signature logic in
ContentEncryptionConfig unchanged.
In `@frontends/aiq_api/tests/test_content_encryption_routes.py`:
- Around line 101-102: The submit tests are patching the wrong symbol, so they
never intercept the route’s real submission path. Update the test setup in
test_content_encryption_routes to patch the hook used by register_job_routes,
specifically submit_authorized_job as resolved in aiq_api.routes.jobs, rather
than aiq_api.jobs.submit.submit_agent_job. This will ensure the 503 cases
actually verify that submission was skipped.
In `@tests/aiq_agent/jobs/test_runner.py`:
- Around line 600-671: This test is using a shared SQLite database URL, which
can leak state and cause flakiness. Update the
`test_final_output_encryption_failure_marks_failure_without_plaintext_write`
setup in `test_runner.py` to build the `sqlite:///...` job store URL from a
per-test temporary path (use the test’s `tmp_path` fixture) instead of
`sqlite:///./test.db`, keeping the rest of the `run_agent_job` invocation
unchanged.
---
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 458-462: The submit route response contract in jobs.py is out of
sync with the handler’s encryption error paths: it documents only 503 for Dask
availability and does not include the new 500 invalid-encryption-config case.
Update the OpenAPI responses for the submit route handler (the route around the
submit endpoint, including the shared responses used across the related blocks)
so encryption-unready is documented as 503 and invalid encryption config is
documented as 500, while keeping the existing 400 and 422 entries intact.
🪄 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: b7e57310-0a09-4e09-afe5-7d2b4bc579b2
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
deploy/Dockerfiledocs/source/deployment/content-encryption.mddocs/source/deployment/index.mddocs/source/index.mdfrontends/aiq_api/pyproject.tomlfrontends/aiq_api/src/aiq_api/jobs/crypto.pyfrontends/aiq_api/src/aiq_api/jobs/event_store.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/tests/test_event_store_content_encryption.pyfrontends/aiq_api/tests/test_job_access.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Run Harbor skill eval
⚠️ CI failures not shown inline (2)
GitHub Actions: AIQ CI / 3_Script Validation.txt: fix(api): harden async job content encryption
Conclusion: failure
Current runner version: '2.335.1'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20260611.554
Commit: 5e0782fdc9014723d3be820dd114dd31555c2bd1
Build Date:
Worker ID: {0841f878-952d-47ee-86aa-3d5642c7e518}
Azure Region: westcentralus
##[endgroup]
##[group]Operating System
Ubuntu
24.04.4
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20260622.220.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260622.220/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260622.220
##[endgroup]
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
##[endgroup]
Secret source: Actions
Prepare workflow directory
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v4' (SHA:34e114876b0b11c390a56381ad16ebd13914f8d5)
Download action repository 'actions/setup-python@v5' (SHA:a26af69be951a213d495a4c3e4e4022e16d87065)
Download action repository 'astral-sh/setup-uv@v4' (SHA:38f3f104447c67c051c4a08e39b64a148898af3a)
Complete job name: Script Validation
Node 20 is being deprecated. This workflow is running with Node 24 by default. If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
##[group]Run actions/checkout@v4
with:
repository: NVIDIA-AI-Blueprints/aiq
***REDACTED***
ssh-strict: true
ssh-user: git
persist-credentials: true
clean: true
sparse-checkout-cone-mode: true
fetch-depth: 1
fetch-tags: false
show-progress: true
lfs: false
submodules: false
set-safe-directory: true
##[endgroup]
Syncing repository: NVIDIA-AI-Blueprints/aiq
##[group]Getting Git version info
Working directory is '/home/runner/work/aiq/aiq'
[command]/usr/bin/git version...
GitHub Actions: AIQ CI / Script Validation: fix(api): harden async job content encryption
Conclusion: failure
##[group]Run . .venv/bin/activate
�[36;1m. .venv/bin/activate�[0m
�[36;1mchmod +x ci/scripts/test_scripts.sh�[0m
�[36;1mci/scripts/test_scripts.sh --skip-setup�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.13.14/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
================================================
AI-Q Blueprint - Script Tests
================================================
Repository: /home/runner/work/aiq/aiq
Scripts: /home/runner/work/aiq/aiq/scripts
============================================
Testing Bash Syntax
============================================
�[0;32m✅ PASS�[0m: dev.sh - valid bash syntax
�[0;32m✅ PASS�[0m: setup.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_as_skill.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_cli.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_e2e.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh - valid bash syntax
�[1;33m⏭️ SKIP�[0m: setup.sh - skipped (--skip-setup flag)
============================================
Testing --help Flags
============================================
�[0;32m✅ PASS�[0m: start_cli.sh --help
�[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh --help
============================================
Testing Virtual Environment Checks
============================================
�[0;32m✅ PASS�[0m: start_cli.sh - venv check works
�[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh - venv check works
============================================
Testing Pytest Integration
============================================
tests/aiq_agent/agents/chat_resear...
🧰 Additional context used
📓 Path-based instructions (7)
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/index.mddocs/source/deployment/index.mddocs/source/deployment/content-encryption.md
**
⚙️ 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:
docs/source/index.mdfrontends/aiq_api/pyproject.tomldeploy/Dockerfiledocs/source/deployment/index.mdfrontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_event_store_content_encryption.pydocs/source/deployment/content-encryption.mdfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/event_store.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
{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/index.mddocs/source/deployment/index.mddocs/source/deployment/content-encryption.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
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
frontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_event_store_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/event_store.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_job_access.pytests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_event_store_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.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/submit.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/event_store.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/tests/test_event_store_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.py
🪛 ast-grep (0.44.0)
frontends/aiq_api/tests/test_event_store_content_encryption.py
[info] 114-114: use jsonify instead of json.dumps for JSON output
Context: json.dumps(raw_event)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 140-140: use jsonify instead of json.dumps for JSON output
Context: json.dumps(raw_event)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
frontends/aiq_api/tests/test_content_encryption.py
[info] 280-280: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"report": report})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
frontends/aiq_api/tests/test_content_encryption_routes.py
[info] 321-321: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"report": "secret"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
frontends/aiq_api/src/aiq_api/routes/jobs.py
[info] 1224-1224: use jsonify instead of json.dumps for JSON output
Context: json.dumps({'error': 'Content encryption is unavailable'})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1227-1227: use jsonify instead of json.dumps for JSON output
Context: json.dumps({'error': 'Job event data is invalid'})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
frontends/aiq_api/src/aiq_api/jobs/event_store.py
[info] 405-405: use jsonify instead of json.dumps for JSON output
Context: json.dumps(stored_event)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 465-465: use jsonify instead of json.dumps for JSON output
Context: json.dumps(stored_event)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
frontends/aiq_api/src/aiq_api/jobs/crypto.py
[info] 864-864: use jsonify instead of json.dumps for JSON output
Context: json.dumps(value)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 899-899: use jsonify instead of json.dumps for JSON output
Context: json.dumps(envelope, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 930-930: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1087-1087: use secrets package over random package
Context: random.uniform(0, _VAULT_RETRY_JITTER_SECONDS)
Note: [CWE-330] Use of Insufficiently Random Values.
(avoid-random-python)
🪛 Trivy (0.69.3)
deploy/Dockerfile
[error] 79-82: 'apt-get' missing '--no-install-recommends'
'--no-install-recommends' flag is missed: 'apt-get update && apt-get install -y curl ca-certificates && rm -rf /var/lib/apt/lists/*'
Rule: DS-0029
(IaC/Dockerfile)
🔇 Additional comments (10)
deploy/Dockerfile (1)
82-82: LGTM!frontends/aiq_api/pyproject.toml (1)
50-51: LGTM!docs/source/deployment/content-encryption.md (1)
1-205: LGTM!docs/source/deployment/index.md (1)
34-35: LGTM!docs/source/index.md (1)
100-100: LGTM!frontends/aiq_api/src/aiq_api/jobs/crypto.py (1)
16-98: LGTM!Also applies to: 150-897, 918-981, 1007-1094
frontends/aiq_api/src/aiq_api/jobs/event_store.py (1)
25-32: LGTM!Also applies to: 107-169, 405-406, 461-466, 539-562, 591-593, 619-621, 652-673
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
316-316: LGTM!Also applies to: 328-341, 484-484, 542-560
frontends/aiq_api/src/aiq_api/jobs/submit.py (1)
120-123: LGTM!Also applies to: 142-143, 162-163, 208-210
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
316-327: LGTM!Also applies to: 427-447, 660-675, 699-715, 1161-1199, 1215-1228, 1252-1424, 1454-1575
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
316-327: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove startup encryption validation behind the async-route availability gate.
Line 326 now makes route registration depend on Vault/content-encryption readiness before the
dask_available/job_storecheck runs. That means a transient encryption startup failure can prevent/v1/data_sourcesand/v1/jobs/async/agentsfrom registering at all, even though those endpoints do not consume encrypted job content. Scope this validation to the async-job surface instead of the whole router setup.Suggested change
- await validate_content_encryption_startup_async() - if not get_all_sources(): logger.warning( "No data sources registered. Add a 'data_sources' function with " @@ if not dask_available or not job_store: logger.warning( "Dask not available - async job submission routes require NAT_DASK_SCHEDULER_ADDRESS" " and NAT_JOB_STORE_DB_URL" ) return + + await validate_content_encryption_startup_async()As per path instructions, treat these API/job-runner route changes as externally visible contracts and preserve the async-job lifecycle boundaries.
Also applies to: 373-378
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/aiq_api/src/aiq_api/routes/jobs.py` around lines 316 - 327, Move the validate_content_encryption_startup_async() call out of the top-level jobs router setup so it only runs when wiring the async-job endpoints, not before the dask_available/job_store gate in jobs.py. Keep the startup encryption check scoped to the async job route registration path used by submit_authorized_job and the related async job handlers, so transient Vault/content-encryption issues cannot block unrelated routes like /v1/data_sources from registering.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 458-462: The documented 500 response in the jobs route does not
match the handler’s actual behavior. Update the OpenAPI `responses` entry in the
job submission endpoint so `500` describes only the real failure cases handled
by the route, namely invalid content encryption configuration and
authorization-metadata persistence failures in the relevant job submission flow.
Keep the response text aligned with the logic in the route handler and any
related tests in `test_content_encryption_routes.py`.
---
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 316-327: Move the validate_content_encryption_startup_async() call
out of the top-level jobs router setup so it only runs when wiring the async-job
endpoints, not before the dask_available/job_store gate in jobs.py. Keep the
startup encryption check scoped to the async job route registration path used by
submit_authorized_job and the related async job handlers, so transient
Vault/content-encryption issues cannot block unrelated routes like
/v1/data_sources from registering.
🪄 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: b8a7f5ff-955f-4f17-897c-d11b02f2f755
📒 Files selected for processing (5)
frontends/aiq_api/src/aiq_api/jobs/crypto.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
tests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.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/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.py
🔇 Additional comments (5)
frontends/aiq_api/src/aiq_api/jobs/crypto.py (2)
905-916:decode_envelope()still misses the non-ASCII failure path.Non-ASCII payloads like the new regression case in
frontends/aiq_api/tests/test_content_encryption.pyLines 263-265 still fail in_b64url_decode()before this handler, so they bypass theContentEncryptionInvalidDatamapping unlessUnicodeEncodeErroris caught here too.
30-30: LGTM!Also applies to: 106-143, 991-1017
frontends/aiq_api/tests/test_content_encryption.py (1)
117-227: LGTM!Also applies to: 263-265
frontends/aiq_api/tests/test_content_encryption_routes.py (1)
101-103: LGTM!Also applies to: 247-258
tests/aiq_agent/jobs/test_runner.py (1)
600-678: LGTM!
Redact credential-bearing configuration fields and fingerprint manager signatures without retaining raw secrets. Reject non-finite timing values, clarify async submission response contracts, and isolate the encryption failure database test. Add regression coverage for credential handling, manager recreation, malformed envelopes, route submission mocks, and OpenAPI responses. Signed-off-by: Tanner Leach <tleach@nvidia.com>
tanleach
force-pushed
the
feat/report_encryption
branch
from
cf0b282 to
40e854b
Compare
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)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
326-326: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not let encryption startup validation prevent health and public route registration.
Line 326 can abort registration before
/health,/v1/data_sources, and agent listing are available, so a missing/invalid encryption secret crashes the surface that should report degradation. CatchContentEncryptionConfigError/ContentEncryptionUnavailablehere and keep submit/read paths gated by their existing checks.Proposed fix
- await validate_content_encryption_startup_async() + try: + await validate_content_encryption_startup_async() + except (ContentEncryptionConfigError, ContentEncryptionUnavailable) as exc: + logger.warning( + "Content encryption startup validation failed; routes will report readiness failures: %s", + exc.__class__.__name__, + )As per coding guidelines, “Missing-secret paths must degrade gracefully (stub/skip), not crash or leak.” As per path instructions, treat API route behavior as an externally visible contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/aiq_api/src/aiq_api/routes/jobs.py` at line 326, Move the startup encryption validation in the jobs route setup so it does not block registration of public/health endpoints. In jobs.py around the route registration flow, catch ContentEncryptionConfigError and ContentEncryptionUnavailable around validate_content_encryption_startup_async() and allow /health, /v1/data_sources, and agent listing routes to register even when encryption is misconfigured. Keep the submit/read handlers protected by their existing per-route checks so only encryption-dependent paths are gated.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Line 326: Move the startup encryption validation in the jobs route setup so it
does not block registration of public/health endpoints. In jobs.py around the
route registration flow, catch ContentEncryptionConfigError and
ContentEncryptionUnavailable around validate_content_encryption_startup_async()
and allow /health, /v1/data_sources, and agent listing routes to register even
when encryption is misconfigured. Keep the submit/read handlers protected by
their existing per-route checks so only encryption-dependent paths are gated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 65983828-0ce4-42b7-bbaa-c8886623a9fd
📒 Files selected for processing (5)
frontends/aiq_api/src/aiq_api/jobs/crypto.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Lint and Hooks
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
frontends/aiq_api/tests/test_content_encryption.pytests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_content_encryption.pytests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_content_encryption_routes.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
frontends/aiq_api/tests/test_content_encryption.pytests/aiq_agent/jobs/test_runner.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.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/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/crypto.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_content_encryption.pyfrontends/aiq_api/tests/test_content_encryption_routes.py
🔇 Additional comments (8)
frontends/aiq_api/src/aiq_api/routes/jobs.py (2)
461-461: Previously flagged: narrow the documented500response.Line 461 still says “job submission failed”, but this handler maps
500to invalid encryption config or async-job authorization metadata persistence failure. This duplicates the existing review thread on this line.
316-325: LGTM!Also applies to: 427-447, 478-534, 661-718
frontends/aiq_api/src/aiq_api/jobs/crypto.py (2)
30-30: LGTM!Also applies to: 984-1005
101-148: LGTM!Also applies to: 1008-1019
frontends/aiq_api/tests/test_content_encryption_routes.py (2)
101-103: 🎯 Functional CorrectnessVerify
_build_jobs_apppatches the submit symbol resolved byregister_job_routes.
submitted_jobis patched onaiq_api.jobs.submit.submit_agent_job, but these tests only intercept the route ifregister_job_routesimports that symbol after the patch. If the handler is bound to a module-local alias instead, the submit success/503 cases will not exercise the intended branch. Based on learnings, patch the symbol where the consuming module resolves it.#!/bin/bash set -euo pipefail ast-grep outline frontends/aiq_api/src/aiq_api/routes/jobs.py --view expanded | sed -n '1,220p' rg -n -C3 'submit_authorized_job|submit_agent_job' \ frontends/aiq_api/src/aiq_api/routes/jobs.py \ frontends/aiq_api/src/aiq_api/jobs/submit.pySource: Learnings
261-270: LGTM!frontends/aiq_api/tests/test_content_encryption.py (1)
117-227: LGTM!Also applies to: 263-265
tests/aiq_agent/jobs/test_runner.py (1)
600-600: LGTM!Also applies to: 639-639, 666-666
Document the submit endpoint's actual HTTP 500 failure cases and update the OpenAPI regression assertion to match. 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)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
665-680: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the new state/report encryption error responses.
These handlers now return
503and500, but the decorators still advertise only404, so the OpenAPI contract omits externally visible failure modes.Proposed fix
summary="Get job artifacts", description="Get tool calls, outputs, and sources collected during job execution.", - responses={404: {"description": "Job not found"}}, + responses={ + 404: {"description": "Job not found"}, + 500: {"description": "Job state data is invalid"}, + 503: {"description": "Content encryption is unavailable"}, + }, @@ summary="Get final report", description="Get the final research report from a completed job.", - responses={404: {"description": "Job not found"}}, + responses={ + 404: {"description": "Job not found"}, + 500: {"description": "Final report data is invalid"}, + 503: {"description": "Content encryption is unavailable"}, + },As per path instructions, treat API error responses as externally visible contracts.
Also applies to: 704-718
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/aiq_api/src/aiq_api/routes/jobs.py` around lines 665 - 680, The job artifact handlers in the jobs route now return additional HTTP failures, but the OpenAPI responses still only advertise 404. Update the response documentation on the affected endpoint decorators and any shared response definitions used by the artifact/state reporting paths (including the helper around the job artifact fetch flow) so the contract explicitly includes the new 503 from ContentEncryptionUnavailable and 500 from ContentEncryptionInvalidData, alongside the existing 404.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 665-680: The job artifact handlers in the jobs route now return
additional HTTP failures, but the OpenAPI responses still only advertise 404.
Update the response documentation on the affected endpoint decorators and any
shared response definitions used by the artifact/state reporting paths
(including the helper around the job artifact fetch flow) so the contract
explicitly includes the new 503 from ContentEncryptionUnavailable and 500 from
ContentEncryptionInvalidData, alongside the existing 404.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c2dd4b8d-69b5-4a42-9ee8-084ad5a065be
📒 Files selected for processing (2)
frontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_content_encryption_routes.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Run Harbor skill eval
- GitHub Check: Lint and Hooks
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
frontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/jobs.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_content_encryption_routes.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
frontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/src/aiq_api/routes/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/routes/jobs.py
🧠 Learnings (1)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_content_encryption_routes.py
🔇 Additional comments (2)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
316-327: LGTM!Also applies to: 427-447, 461-466, 482-496, 525-538, 1414-1429
frontends/aiq_api/tests/test_content_encryption_routes.py (1)
142-195: LGTM!Also applies to: 198-244, 269-271, 448-478
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
Two blocking encryption-boundary issues found in the current PR head.
Propagate a non-secret encryption policy identity with async job submissions and fail workers before RUNNING when their local mode or key identity differs. Document the boundary and cover static-key, Vault, missing-policy, and API-to-worker mismatch behavior. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Propagate event persistence failures when content encryption is enabled and retain background batch flush errors for the runner to observe. Keep off-mode event writes best effort and add timer and job-state regression coverage. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Replace NAT's generic health route with AI-Q readiness so runtime dispatch and OpenAPI expose database and encryption state. Preserve the healthy success response and cover assembled worker behavior for unavailable Vault, off mode, and static-key mode. Signed-off-by: Tanner Leach <tleach@nvidia.com>
Resolve async job runner, submission, route, and test conflicts with the latest develop branch. Preserve encryption policy enforcement across the expanded worker arguments, encrypt report interaction metadata, and decrypt parent reports for report follow-up jobs. Signed-off-by: Tanner Leach <tleach@nvidia.com>
2 participants
Overview
Add application-level envelope encryption at rest for async job final reports
and selected artifact event content. The implementation supports disabled,
static-key, and HashiCorp Vault Transit modes; uses per-job AES-256-GCM data
encryption keys; and fails closed when encrypted report or event data cannot be
authenticated or decrypted.
The follow-up hardening in this branch:
concurrent access;
constraints.
Encryption remains disabled by default. This release intentionally does not
include plaintext migration, backfill, application-managed key rotation,
rewrap, or decrypt-on-rollback tooling. Opt-in operators must follow the
documented forward-only rollout constraints and retain the original encryption
configuration while encrypted jobs exist. Database-writer replay, deletion,
and reordering are outside the initial confidentiality-at-rest threat model.
Validation
Pre-commit was run separately over the three changed documentation files and
the eight runtime-hardening files. Ruff, formatting, merge-conflict, large-file,
trailing-whitespace, secret-detection, and applicable link checks passed.
The unchanged
test_periodic_cleanup.pymodule emits a post-run_cleanup_old_events_loopunawaited-coroutine warning while still passing all20 tests. A Docker image build was not run; a local Buildx check produced no
result and was stopped.
git commit -sor an equivalent sign-off.Where should reviewers start?
Start with
frontends/aiq_api/src/aiq_api/jobs/crypto.pyfor the envelope,readiness, Vault retry, and concurrency model. Then review
frontends/aiq_api/src/aiq_api/jobs/event_store.pyandfrontends/aiq_api/src/aiq_api/routes/jobs.pyfor async decryption and explicitfailure propagation. The focused regression coverage is in the three
test_*content_encryption*.pymodules.Related Issues
Summary by CodeRabbit
job.errorvia SSE on decrypt failures.