feat(deep-research): provider-neutral sandbox + durable artifact runtime - #280
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a provider-neutral sandbox runtime and durable artifact pipeline, threads them through job execution and API surfaces, updates UI and CLI artifact rendering/export paths, and adds OpenShell sandbox setup/configuration plus related documentation and tests. ChangesDeep Research Sandbox and Artifact Flow
Sequence Diagram(s)sequenceDiagram
participant DeepResearcherAgent
participant ArtifactManager
participant SqlArtifactStore
DeepResearcherAgent->>ArtifactManager: final_harvest()
ArtifactManager->>SqlArtifactStore: list(job_id)
ArtifactManager->>SqlArtifactStore: put(artifact, bytes)
ArtifactManager-->>DeepResearcherAgent: rewritten report markdown
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
85808ac to
6862023
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 38
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@configs/openshell/Dockerfile.aiq-demo`:
- Around line 8-17: The Dockerfile uses unpinned package versions for both
system and Python dependencies, causing non-reproducible builds. Pin specific
versions for all packages in the apt-get install command (bash, ca-certificates,
curl, iproute2, iptables, procps) by adding version constraints after each
package name, and pin all pip install packages (numpy, pandas, matplotlib,
pillow, tabulate, requests) using the == syntax with specific versions.
Additionally, replace the `python:3.13-slim` base image tag with a specific
digest hash to ensure consistent base image versions across builds.
In `@docs/source/architecture/agents/sandbox.md`:
- Around line 8-11: Update the sandbox naming documentation to be more
provider-neutral and accurate. Change the statement that "the sandbox name is
the resolved job ID" to instead say that sandbox names are "derived from the
resolved job ID" to reflect that providers can normalize or scope job IDs before
creating sandboxes. Additionally, identify any Modal-specific details about
character or length limits in the section spanning lines 24-31 and move those to
a provider-specific section rather than keeping them in the general
provider-neutral documentation.
In `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 576-581: The cleanup code currently always calls close() on
sandbox_runtime without distinguishing between normal completion and interrupted
jobs (cancellation/timeout). You need to track whether the job was cancelled or
timed out during execution in the runner, expose the terminate() method from
DeepAgentsRuntime, and then modify the cleanup section where
sandbox_runtime.close() is called to check if the job was interrupted and call
terminate() instead of (or before) close() for those cases.
- Around line 564-568: The final_harvest call on sandbox_runtime is executing
after the job status has been set to SUCCESS, allowing clients to see the job as
complete before terminal artifacts are actually persisted. Move the
final_harvest invocation (the asyncio.to_thread(sandbox_runtime.final_harvest)
call) to occur before the job status is updated to SUCCESS at line 518, ensuring
that all terminal artifacts are harvested and flushed before clients receive the
terminal completion signal.
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 1022-1029: The current artifact cleanup logic in the
SqlArtifactStore.cleanup_old_artifacts() method deletes artifacts based on their
creation timestamp, but it should instead delete artifacts associated with jobs
where job_info.is_expired is true. Modify the cleanup method to join the
artifacts table with job_info and delete artifacts only when their corresponding
job has expired, using age-based cleanup only as a fallback for orphaned
artifact records that have no associated job.
- Around line 108-117: The capacity validation checks for owner_count and
global_count are performed separately from the actual job submission in
submit_authorized_job(), creating a race condition where concurrent requests can
all observe available capacity and submit simultaneously, bypassing the
AIQ_MAX_SANDBOXES_* limits. Refactor the code to make the capacity checks and
job submission atomic by either wrapping both the count_active_jobs_for_owner,
count_active_jobs_global checks and the submit_authorized_job call within a
single database transaction, or by acquiring a per-principal advisory lock (for
owner limits) and a global advisory lock (for global limits) before performing
the checks and submission to ensure only one request can verify and claim
capacity at a time.
- Around line 712-715: The Content-Disposition header in the StreamingResponse
is directly interpolating artifact.filename without proper escaping, which
creates a security vulnerability allowing header injection or response splitting
attacks. In the StreamingResponse headers dictionary where Content-Disposition
is set, replace the direct f-string interpolation of artifact.filename with
properly escaped filename handling. Use Starlette's built-in content-disposition
utilities or implement RFC 5987 encoding to safely encode the filename before
inserting it into the header value.
- Around line 689-691: The artifact serialization in the return statement for
the job list endpoint is exposing the storage_uri field through model_dump(),
which can leak sensitive database credentials and internal paths. Create a
public Data Transfer Object (DTO) for artifacts that explicitly excludes the
storage_uri field and includes only safe, public information such as the
artifact identifier and a content endpoint URL. Then use this public DTO instead
of directly calling model_dump() on the artifact objects from SqlArtifactStore.
In `@frontends/aiq_api/tests/test_sandbox_concurrency.py`:
- Around line 46-76: The tests test_rejects_when_owner_over_limit,
test_rejects_when_global_over_limit, test_allows_under_limit, and
test_fails_open_when_counts_unknown are hardcoded to depend on default
environment variable caps (5 for per-principal, 50 for global). To make these
tests deterministic regardless of CI/dev environment settings, patch the actual
limit configuration values (AIQ_MAX_SANDBOXES_PER_PRINCIPAL and
AIQ_MAX_SANDBOXES_GLOBAL) or the functions that retrieve these limits within
each test's context manager, so the tests always use explicit known limits
rather than relying on environment variables.
In `@frontends/ui/src/lib/pdf/ReactPdfDocument.tsx`:
- Around line 230-236: The collectEmbeddableImages function currently accepts
any data: URI without validation, allowing potential bypass of artifact byte
caps and forcing the PDF renderer to handle oversized or invalid data. Add
validation within the condition that checks t.href.startsWith('data:') to verify
both the MIME type and the decoded size of the data URI before pushing the image
token to the found array. Extract and validate the MIME type from the data URI
scheme, then decode the base64 or encoded content and verify its size is within
acceptable limits before adding it to the embeddable images list.
- Around line 308-312: The collectEmbeddableImages function call is passing
item.tokens which recurses into nested list tokens, causing images from nested
bullets to be duplicated under the parent item. Replace the argument to
collectEmbeddableImages from item.tokens to textTokens so that each list level
only collects images from its own text content without recursing into nested
lists.
In `@frontends/ui/src/pages/api/generate-pdf.ts`:
- Around line 45-46: The fetch call in the generate-pdf.ts file is using
artifactContentPath(jobId, id) which returns a frontend proxy path
/api/jobs/async/... that does not exist on the actual backend. Replace the
artifactContentPath() call with the correct backend /v1 artifact endpoint path
so that the fetch request properly reaches the backend API instead of attempting
to access a non-existent proxy path when backend is set to BACKEND_URL.
- Around line 36-43: The PDF generation endpoint has an unbounded fan-out
vulnerability where extractArtifactIds returns user-controlled data and
Promise.all fetches all artifact references concurrently without limits,
potentially causing excessive backend and memory pressure. Add a cap to limit
the maximum number of artifact IDs processed (e.g., slice the ids array to a
reasonable maximum) and replace the Promise.all pattern with sequential fetching
using a loop that awaits each fetch before proceeding to the next one, ensuring
bounded memory and backend request patterns.
- Around line 89-92: The current implementation forwards the entire browser
cookie header to the backend, which exposes unrelated session/frontend cookies
and violates security boundaries. Remove the line that forwards all cookies from
req.headers.cookie and instead conditionally construct authHeaders based on
whether auth is enabled: if auth is disabled, send no auth headers; if auth is
enabled, forward only the Authorization header and extract and forward only the
scoped idToken cookie (not the entire cookie header). This matches the jobs
proxy behavior pattern and ensures auth-sensitive data stays within proper
runtime boundaries.
In `@frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsx`:
- Around line 172-198: The img renderer function is returning a block-level
`<figure>` element, but react-markdown wraps image syntax in a paragraph
renderer, creating invalid HTML markup `<p><figure>…</figure></p>`. Either
replace the `<figure>` element with a phrasing-safe inline wrapper (like a
styled `<span>` or `<div>`), or implement logic in the paragraph renderer to
detect when it contains figure content and switch the wrapper from `<p>` to a
block-level `<div>` instead. Choose whichever approach maintains the visual
design and maintains the caption functionality.
In `@scripts/setup_openshell.sh`:
- Around line 671-672: The pkill command with the -f openshell-gateway pattern
is too broad and can terminate unrelated gateway processes on the host. Instead,
modify the script to save the PID of the openshell-gateway process when it is
started (typically when invoking the process in the background, capture the $!
variable immediately after), store it in a variable or file, and then use that
specific PID with kill command (e.g., kill $SAVED_PID) rather than pattern
matching with pkill. This ensures only the gateway process started by this
script is terminated during cleanup.
- Around line 27-28: The DEFAULT_LANGCHAIN_NVIDIA_INSTALL_SPEC variable uses a
moving branch reference (`@spastoriza/openshell-sandbox`) instead of a pinned
commit SHA, which reduces reproducibility and supply-chain trust. To fix this,
replace the branch reference with a specific commit SHA in the git URL (e.g.,
`@commit_hash_here`) to ensure consistent deployments, or add inline documentation
above the DEFAULT_LANGCHAIN_NVIDIA_INSTALL_SPEC and LANGCHAIN_NVIDIA_REPO
variables clearly stating the temporary nature of this dependency, the expected
timeline for langchain-nvidia-openshell PyPI publication, and instructions for
users to override with a stable commit if needed for reproducible builds.
In `@skills/aiq-research/scripts/aiq.py`:
- Around line 440-450: The current implementation uses only the basename of
artifact filenames when saving artifacts, which causes data loss when multiple
artifacts with identical filenames exist in the same job (e.g., two "chart.png"
files will overwrite each other). Modify the filename construction logic in the
loop that processes artifacts from list_artifacts to uniquify the filename by
incorporating the artifact_id (such as prefixing it or using it as part of the
filename generation). This same issue appears in two locations in the file as
noted in the comment, so ensure both locations are updated consistently to
prevent artifacts from overwriting each other and to maintain correct mappings
in id_to_relpath.
In `@skills/aiq-research/SKILL.md`:
- Around line 152-157: The documentation in Step 4 contains two command examples
using shorthand syntax (artifacts and report commands) that are inconsistent
with the fully explicit python3 $SKILL_DIR/scripts/aiq.py pattern used elsewhere
in the documentation. Replace the shorthand command forms on lines 152 and 156
with their fully explicit equivalents using the python3
$SKILL_DIR/scripts/aiq.py syntax pattern to ensure consistency and prevent
copy/paste failures. Verify that both the artifacts command example and the
report command example follow the same explicit pattern as other command
examples in the document.
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 537-545: The issue is that when artifact_manager.store.list fails
in the exception handler, setting produced_artifacts to an empty list causes the
downstream no-source path to raise EmptySourceRegistryError, converting
transient artifact-store read errors into hard failures. Instead of forcing an
empty list on listing failure, preserve a sentinel value or flag to distinguish
between a transient listing failure and genuinely no artifacts. This allows the
no-source error path logic (around line 585 and similar locations in the 568-589
range) to handle transient failures gracefully without raising errors when the
harvest at line 537 actually succeeded.
In `@src/aiq_agent/agents/deep_researcher/deepagents_runtime.py`:
- Around line 141-154: The initialization logic checks if sandbox is not None
but fails to verify the sandbox.enabled flag, causing disabled sandbox
configurations to still create a provider and set up artifact capture. Add a
check for sandbox.enabled in the conditional at line 141 so that the entire
block creating _sandbox_provider and ArtifactManager only executes when sandbox
is not None AND sandbox.enabled is True, effectively treating disabled sandbox
configurations the same as None.
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py`:
- Around line 15-19: The code at lines 326-329 interpolates the artifact_dir
configuration value directly into a shell command using an f-string without
proper quoting, which creates a security vulnerability where whitespace or
metacharacters in artifact_dir could change the executed command. Fix this by
adding import shlex to the imports section, then locate the find command that
uses artifact_dir (around lines 326-329) and wrap the artifact_dir variable with
shlex.quote() before interpolating it into the shell command f-string to ensure
special characters are properly escaped.
- Around line 380-408: The filename variable is extracted directly from
entry.path without sanitization before being placed in HTTP response headers
like Content-Disposition, which creates a security risk for header injection
attacks via special characters like quotes, backslashes, or CRLF sequences.
Create a new helper function _safe_filename that removes or replaces dangerous
characters (carriage returns, newlines, double quotes, and backslashes) from
filenames and truncates them to 255 characters with a fallback to "artifact" for
empty strings, then apply this sanitization to the filename variable extracted
from entry.path before it is used anywhere in the Artifact object or response
headers.
- Around line 193-203: The artifact lookup in the _replace function creates
ambiguity when multiple artifacts share the same filename across different
directories. Currently, the by_name dictionary collapses all artifacts by
filename (overwriting duplicates), and then the fallback to
PurePosixPath(token).name resolves basename matches without checking uniqueness.
Refactor the lookup logic to first attempt exact path matching using
sandbox_path or relative paths, then only use basename matching as a fallback if
exactly one artifact in the collection has that basename. Track all artifacts
with the same basename during dictionary construction to enable this uniqueness
check, and avoid silently resolving ambiguous references to potentially
incorrect artifact IDs.
- Around line 420-425: The store.put() method in the artifact manager returns an
existing artifact when deduplication occurs, but the current code still
increments quota counters (_total_bytes and _count) and emits artifact events
via _emit_artifact() for these dedup hits. Modify the code to check if the
artifact returned by store.put() is the same as the one being stored by
comparing their IDs. Only increment the quota counters and call _emit_artifact()
when it's a new artifact (not a dedup); when store.put() returns an existing
artifact with a different ID, skip the quota accounting and SSE emission to
prevent duplicate quota consumption and duplicate UI events.
- Around line 86-132: The _sanitize function currently attempts to sanitize SVG
content by removing only <script> tags and on* attributes using _SVG_SCRIPT_RE
and _SVG_ON_ATTR_RE, but this is insufficient as SVG can still carry other
attack vectors like javascript: links, <foreignObject>, external references, and
CSS payloads. Instead of the partial regex-based sanitization approach for mime
== "image/svg+xml", implement a fail-closed strategy by returning None to reject
SVG content entirely until a proper allowlist sanitizer is implemented.
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py`:
- Line 158: The current non-unique index idx_artifacts_job_sha allows concurrent
operations to insert duplicate (job_id, sha256) pairs. Change the index
definition from a regular index to a unique constraint by adding the UNIQUE
keyword to idx_artifacts_job_sha. Then in the put() method, wrap the INSERT
operation with a try-except block to catch IntegrityError exceptions. When an
IntegrityError is caught (indicating another concurrent operation won the race),
return the result of find_by_digest(artifact.job_id, artifact.sha256) to get the
existing row instead of creating a duplicate.
- Line 163: The logger.info call at line 163 logs self.db_url[:50], which can
expose sensitive database credentials or hostnames. Replace the logged URL with
an opaque identifier or generic message that does not reveal the actual database
URL. Apply the same fix to the similar logging statements at lines 174-178.
Additionally, review the storage_uri attribute and any serialization in the
artifact list route to ensure database URLs are not being exposed through the
artifact metadata. Store an opaque database identifier internally instead of the
actual connection URL to prevent credentials and deployment details from leaking
through logs and API responses.
In `@src/aiq_agent/agents/deep_researcher/sandbox/base.py`:
- Around line 123-130: The lock scope is too broad in the execution flow,
preventing the terminate method from interrupting long-running operations. The
_call method (around lines 205-213) holds self._lock for the entire duration of
the remote invocation, which blocks terminate from acquiring the lock and
cancelling in-flight execute operations. Refactor the locking strategy to only
protect critical sections rather than the entire remote invocation, allowing
terminate to preempt execution without waiting for the lock to be released.
Consider releasing the lock before the actual execute call or using a different
synchronization mechanism that supports cancellation of in-flight operations.
- Around line 187-195: The _reset_session() method overwrites self._session
without closing the previous session instance, causing a resource leak on
recoverable retries. Before assigning the new session created by
self._create_session(), store the previous session instance, attempt to close it
with best-effort error handling (to avoid blocking the reset), and then assign
the newly created session.
In `@src/aiq_agent/agents/deep_researcher/sandbox/capabilities.py`:
- Around line 42-49: The supports_artifact_download field in the capabilities
class defaults to True, which violates fail-closed semantics and allows artifact
capture when not explicitly declared by a provider. Change the default value of
supports_artifact_download from True to False so that artifact capture is only
permitted when explicitly enabled. Apply the same fix to any other capability
fields in the range 82-86 that have similar permissive defaults.
In `@src/aiq_agent/agents/deep_researcher/sandbox/config.py`:
- Around line 186-192: The legacy block_network lifting code treats string
boolean values like "false" or "0" as truthy Python strings, causing incorrect
mapping to network.mode: blocked. When extracting legacy_block from
data.pop("block_network"), parse the value to an actual boolean by checking if
it is a string and converting it appropriately (handle cases like "true",
"false", "1", "0", and boolean types). This ensures that the conditional check
on legacy_block correctly evaluates to determine whether network.mode should be
"blocked" or "open".
In `@src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py`:
- Around line 144-146: The environment variable _ADAPTER_FILE_TRANSFER_ENV is
being checked for existence with os.getenv() rather than being explicitly parsed
as a boolean, which means any non-empty string value (including "0" or "false")
will be treated as enabled. In the upload_files method, replace the simple
existence check with explicit boolean parsing that treats only "true", "True",
"1", and similar affirmative values as enabled while treating "false", "0", and
empty/unset values as disabled. Apply this same fix to all locations where
_ADAPTER_FILE_TRANSFER_ENV is checked (both the upload_files method and any
other methods that reference this environment variable).
- Around line 179-183: The current code in the sandbox.exec call result handling
does not honor the documented contract from _DOWNLOAD_CODE where sys.exit(3)
signals a directory. When exit_code is 3 and stderr is empty, the error is
misclassified as permission_denied instead of is_directory. Add a specific check
after the sandbox.exec call to detect when exit_code equals 3 and handle it as a
directory case before calling _classify_fs_error. This should be done by
checking the exit_code value first and only calling _classify_fs_error for other
non-zero exit codes.
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Around line 18-204: The README file contains multiple markdown linting
violations that need to be fixed. Add language specifiers to all fenced code
blocks (add python after the opening fence for Python code blocks like the
MySandboxProvider example, yaml for config examples like the config YAML block
and Config (sandbox block) section, bash for the setup commands, and toml for
the entry-points example). Additionally, ensure there are blank lines before and
after all fenced code blocks and between headings and content - specifically
before the config YAML block, before and after the Module map table, before the
Adding a provider section, before the Out-of-tree providers section, before the
Config (sandbox block) section, and before each operational knobs and testing
sections to comply with MD031 and MD022 linting rules.
In `@src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md`:
- Around line 25-27: The SKILL.md documentation hardcodes `/sandbox` paths for
artifact directories, which breaks the provider-neutral contract when the
orchestrator supplies different runtime-dependent sandbox directories. Replace
all instances of hardcoded `/sandbox` paths (found in the artifact directory
write instructions, manifest write section, and other locations noted in lines
37-43, 66-67, and 96-97) with a reference to a runtime-supplied sandbox
directory variable or placeholder, such as a parameter that gets provided by the
orchestrator, to ensure the skill works correctly regardless of the actual
workdir location.
In `@src/aiq_agent/common/citation_verification.py`:
- Around line 957-960: In the _collapse_md_link function, the current check for
"artifact://" in match.group(0) will incorrectly preserve links if the artifact
protocol appears anywhere in the markdown token, including in the label text.
Instead, check only the href portion of the markdown link (the URL component,
likely match.group(2) or the appropriate group capturing just the link
destination) to determine if it contains "artifact://" before deciding to
preserve the entire match unchanged.
In `@tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py`:
- Around line 207-213: The test_dedup_by_digest method is not properly
validating digest-based deduplication because both calls to store.put use the
same artifact from self._artifact(), which means they have the same artifact_id.
To properly test digest-based deduplication, create a second artifact with a
different artifact_id but with identical binary content (same sha256 digest) as
the first artifact, then insert both artifacts into the store and verify they
get deduplicated based on their digest rather than just their ID.
🪄 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: c5a1b339-af55-4fe0-bb42-e9480f69f5ff
⛔ Files ignored due to path filters (1)
configs/openshell/generated/.gitignoreis excluded by!**/generated/**
📒 Files selected for processing (55)
.gitignoreconfigs/config_openshell.ymlconfigs/openshell/Dockerfile.aiq-democonfigs/openshell/aiq-research-policy.yamldocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/src/aiq_api/jobs/access.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsscripts/README.mdscripts/setup_openshell.shskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pysrc/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/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/common/citation_verification.pytests/aiq_agent/agents/deep_researcher/sandbox/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pytests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
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/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
frontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tstests/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/prompts/researcher.j2frontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/chat/store.tsconfigs/openshell/Dockerfile.aiq-demofrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxtests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pyfrontends/ui/src/shared/components/MarkdownRenderer/index.tssrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/common/citation_verification.pyfrontends/ui/src/features/layout/components/ReportTab.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/config.pyfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/hooks/use-download-pdf.tssrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pyscripts/README.mdsrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pyskills/aiq-research/SKILL.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pyfrontends/ui/src/pages/api/generate-pdf.tsconfigs/openshell/aiq-research-policy.yamlsrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/app/api/jobs/async/[...path]/route.tssrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pydocs/source/architecture/agents/sandbox.mdsrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pytests/aiq_agent/agents/deep_researcher/test_agent.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pyconfigs/config_openshell.ymlsrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdfrontends/aiq_api/src/aiq_api/jobs/access.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pyskills/aiq-research/scripts/aiq.pyfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pyscripts/setup_openshell.shsrc/aiq_agent/agents/deep_researcher/deepagents_runtime.py
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/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/agents/deep_researcher/sandbox/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pytests/aiq_agent/agents/deep_researcher/test_agent.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pyfrontends/aiq_api/src/aiq_api/jobs/access.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pyskills/aiq-research/scripts/aiq.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.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/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.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/openshell/Dockerfile.aiq-democonfigs/openshell/aiq-research-policy.yamlconfigs/config_openshell.yml
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_agent.pyfrontends/aiq_api/tests/test_sandbox_concurrency.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/providers/__init__.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.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/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.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:
src/aiq_agent/agents/deep_researcher/sandbox/config.py
skills/aiq-research/**
⚙️ CodeRabbit configuration file
skills/aiq-research/**: ---
name: aiq-research
description: |
Use when asked to run deep research or AI-Q research through a reachable NVIDIA AI-Q Blueprint backend.
license: Apache-2.0
permissions:
env:
- AIQ_SERVER_URL
network:
- http://localhost:8000
compatibility: |
Designed for Claude Code, OpenCode, Codex, and Agent Skills-compatible tools. Requires Python 3.11+ and network
access to a running local AI-Q Blueprint server athttp://localhost:8000by default. Non-local backends must be
explicitly trusted by the user and granted by the host tool outside this public skill.
metadata:
version: "2.1.0"
author: "NVIDIA AI-Q Blueprint Team aiq-blueprint@nvidia.com"
github-url: "https://github.com/NVIDIA-AI-Blueprints/aiq"
tags:
- nvidia
- aiq
- blueprint
- deep-research
- research-agents
- agent-skills
languages:
- python
- bash
domain: "research-agents"
allowed-tools: Read BashAIQ Research Skill
Purpose
Use this skill to call a locally running NVIDIA AI-Q Blueprint server through the helper script at
scripts/aiq.py.Use this skill for research-shaped requests, including:
- "deep research on ..."
- "AIQ research ..."
- "research ..."
- "use AI-Q to answer ..."
- "ask AI-Q about ..."
Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests. Those
belong toaiq-deploy.Prerequisites
Users need:
- Python 3.11+ available as
python3.- A reachable local or self-hosted AI-Q Blueprint backend.
AIQ_SERVER_URLset when the backend is not running athttp://localhost:8000; non-local values must be trusted by
the user before any query is sent.- A backend configured with authentication disabled for this public helper, or a separate authenticated AI-Q skill for
authenticated environments.- Network access from the local machine to the AI-Q backend URL.
- Credentials configured in the backend environment, not in this skill. Thi...
Files:
skills/aiq-research/SKILL.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/SKILL.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
{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/access.pyfrontends/aiq_api/src/aiq_api/jobs/runner.py
skills/aiq-research/scripts/aiq.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/scripts/aiq.py: Usepython3as the Python interpreter when calling the AI-Q helper script
For asynchronous deep research jobs returning a job_id, poll usingpython3 $SKILL_DIR/scripts/aiq.py research_poll <JOB_ID>and do not force polling when there is no job_id in the response
Files:
skills/aiq-research/scripts/aiq.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Set `AIQ_SERVER_URL` environment variable when the AI-Q backend is not running at the default `http://localhost:8000`
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Run `health` check before sending research requests to verify backend reachability
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: State the exact AI-Q backend URL before sending any user query, and require explicit user confirmation for non-local URLs
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Do not send credentials, cookies, bearer tokens, or secret values through query text to the AI-Q endpoint
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Keep citations and source URLs intact when presenting returned reports from AI-Q research
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Use semantic versioning compatibility rules: Skill version X.Y.Z is compatible with Blueprint version A.B.C only when A == X (major versions must match) and B >= Y (minor version must be equal or greater)
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Store backend credentials in the AI-Q deployment environment, not in the skill or command examples
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T18:09:10.510Z
Learning: Do not retry automatically on failed jobs; stop and show the returned error message
📚 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_sandbox_concurrency.py
🪛 ast-grep (0.44.0)
tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
[info] 41-41: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"version": 1, "artifacts": [{"path": path, "kind": kind, "inline": True}]})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 117-119: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{"version": 1, "artifacts": [{"path": a, "kind": "image"}, {"path": b, "kind": "image"}]}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
skills/aiq-research/scripts/aiq.py
[warning] 235-235: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req, timeout=timeout)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[info] 458-458: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"job_id": job_id, "report": report_path, "artifacts": saved}, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 470-470: use jsonify instead of json.dumps for JSON output
Context: json.dumps(get_report(job_id), indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 481-481: use jsonify instead of json.dumps for JSON output
Context: json.dumps(listing, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 500-500: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"job_id": job_id, "downloaded": saved}, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[warning] 446-446: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(dest, "wb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 456-456: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(report_path, "w", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 497-497: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(dest, "wb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
[warning] 181-187: Do not use text() as it leads to SQL injection
Context: text(
"INSERT INTO artifacts (artifact_id, job_id, kind, mime_type, filename, sandbox_path, "
"storage_uri, sha256, size_bytes, title, caption, inline, workflow, source_tool_call_id, "
"provenance, status, content) VALUES (:artifact_id, :job_id, :kind, :mime_type, :filename, "
":sandbox_path, :storage_uri, :sha256, :size_bytes, :title, :caption, :inline, :workflow, "
":source_tool_call_id, :provenance, :status, :content)"
)
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 216-216: Do not use text() as it leads to SQL injection
Context: text("SELECT content FROM artifacts WHERE job_id = :job_id AND artifact_id = :artifact_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 230-230: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE job_id = :job_id AND artifact_id = :artifact_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 240-240: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE job_id = :job_id AND sha256 = :sha256 LIMIT 1")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 250-250: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE job_id = :job_id ORDER BY created_at")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 259-259: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE job_id = :job_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 270-270: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE created_at < NOW() - :seconds * INTERVAL '1 second'")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 275-275: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE created_at < datetime('now', :interval)")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
🪛 Checkov (3.3.1)
configs/openshell/Dockerfile.aiq-demo
[low] 1-25: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
🪛 Hadolint (2.14.0)
configs/openshell/Dockerfile.aiq-demo
[warning] 8-8: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[warning] 8-8: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>
(DL3013)
🪛 LanguageTool
scripts/README.md
[style] ~184-~184: Consider a different adjective to strengthen your wording.
Context: ...Index | | configs/config_skills.yml | Deep research with DeepAgents skills + Modal...
(DEEP_PROFOUND)
[style] ~185-~185: Consider a different adjective to strengthen your wording.
Context: ...ox | | configs/config_openshell.yml | Deep research with skills + OpenShell sandbo...
(DEEP_PROFOUND)
🪛 markdownlint-cli2 (0.22.1)
src/aiq_agent/agents/deep_researcher/sandbox/README.md
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 77-77: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 87-87: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 91-91: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 122-122: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 140-140: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 148-148: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 165-165: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 169-169: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 177-177: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 180-180: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 204-204: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 OpenGrep (1.23.0)
tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py
[ERROR] 219-219: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 242-242: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 252-252: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 261-261: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 268-268: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py
[ERROR] 328-328: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 Trivy (0.69.3)
configs/openshell/Dockerfile.aiq-demo
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (11)
skills/aiq-research/SKILL.md (1)
152-159: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse fully explicit command forms for copy/paste consistency.
Lines 152–156 use shorthand (
artifacts ...,report ...) that is inconsistent with the fully explicitpython3 $SKILL_DIR/scripts/aiq.py ...pattern used in Steps 1–3. Update to the explicit form to match the rest of the documentation:-The final report may reference generated artifacts (charts, CSVs) as `artifact://<id>` links. To materialize them as local -files, run `artifacts <JOB_ID> --download-dir ./aiq-artifacts`; it downloads each artifact and prints the local path. Do not +The final report may reference generated artifacts (charts, CSVs) as `artifact://<id>` links. To materialize them as local +files, run `python3 $SKILL_DIR/scripts/aiq.py artifacts <JOB_ID> --download-dir ./aiq-artifacts`; it downloads each artifact and prints the local path. Do not expect base64 image data in the report itself. -For a self-contained, shareable report, run `report <JOB_ID> --out-dir ./my-report`. It writes `report.md` plus an +For a self-contained, shareable report, run `python3 $SKILL_DIR/scripts/aiq.py report <JOB_ID> --out-dir ./my-report`. It writes `report.md` plus an `artifacts/` folder and rewrites each `artifact://<id>` link to the matching local file, so the report renders (charts and all) in any markdown viewer without a running backend.🤖 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 `@skills/aiq-research/SKILL.md` around lines 152 - 159, Replace the shorthand command forms in the documentation section with fully explicit forms to match the pattern used in earlier steps. Update the `artifacts <JOB_ID> --download-dir ./aiq-artifacts` command to use the explicit `python3 $SKILL_DIR/scripts/aiq.py` pattern, and similarly update the `report <JOB_ID> --out-dir ./my-report` command to use the same explicit form for consistency throughout the documentation.Source: Path instructions
docs/source/architecture/agents/sandbox.md (2)
24-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove Modal-specific naming constraints to a provider-specific section.
Lines 30–31 impose Modal's 64-character alphanumeric limit as a general requirement, but this is provider-specific. Revise the "Current Behavior" section to be provider-neutral, then add provider-specific guidance separately:
## Current Behavior -- One sandbox name is used per deep research job when sandboxing is enabled; the - name is the resolved job ID, and different jobs produce different names. +- One sandbox name is used per deep research job when sandboxing is enabled; the + name is derived from the resolved job ID via the selected provider's naming rules.Then add a "Provider-Specific Constraints" subsection listing Modal's and OpenShell's exact name constraints.
🤖 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/architecture/agents/sandbox.md` around lines 24 - 31, The Current Behavior section currently includes Modal-specific naming constraints (64 characters, alphanumeric with dash/period/underscore) mixed with general provider-agnostic requirements. Restructure the documentation by removing the Modal-specific details from the general listing in the Current Behavior section, making it provider-neutral instead. Then add a new Provider-Specific Constraints subsection that explicitly lists the exact naming requirements for Modal and OpenShell separately.Source: Path instructions
8-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify that sandbox names are derived from (not identical to) the job ID.
Providers may normalize or scope job IDs before creating sandboxes (e.g., Modal truncates, OpenShell applies naming rules). Soften the language to be provider-neutral:
-Deep research can optionally run DeepAgents `execute` calls in a provider-neutral -sandbox (Modal, OpenShell, or any registered provider). Sandboxes are scoped to a -single async job: the sandbox name is the resolved job ID, so unrelated jobs never -share filesystem state. +Deep research can optionally run DeepAgents `execute` calls in a provider-neutral +sandbox (Modal, OpenShell, or any registered provider). Sandboxes are scoped to a +single async job: the sandbox name is derived from the resolved job ID, so unrelated jobs never +share filesystem state.🤖 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/architecture/agents/sandbox.md` around lines 8 - 11, Update the sentence in the sandbox scoping explanation that currently states "the sandbox name is the resolved job ID" to clarify that sandbox names are derived from (not identical to) the resolved job ID. Acknowledge that different providers may apply their own normalization or transformation rules (such as truncation for Modal or naming conventions for OpenShell) when creating the sandbox name from the job ID. This change should make the documentation provider-neutral and more accurately reflect the actual behavior across different sandbox providers.Source: Path instructions
src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md (1)
25-27: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace hardcoded
/sandboxpaths with runtime-supplied variables.The skill references absolute
/sandboxpaths, but the orchestrator now provides runtime-dependent{{ sandbox_workdir }}and{{ sandbox_artifact_dir }}variables. Change documentation to reference variables supplied by the prompt context:4. **Write to the artifact directory:** save the PNG and its CSV under - `/sandbox/aiq-artifacts/` with descriptive filenames. + the sandbox artifact directory provided by the prompt context + (for example, `<sandbox_artifact_dir>/`) with descriptive filenames.Also update lines 37–42 (script path and artifact paths), line 66 (manifest path), and the example code (lines 74, 96) to reference the placeholder variable instead of
/sandbox.🤖 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/skills/chart-generation/SKILL.md` around lines 25 - 27, Replace all hardcoded `/sandbox` path references in the SKILL.md documentation with runtime-supplied placeholder variables. Specifically, update the artifact directory path references to use `{{ sandbox_artifact_dir }}` instead of `/sandbox/aiq-artifacts/`, the script paths to use `{{ sandbox_workdir }}` instead of `/sandbox`, the manifest path generation to reference the appropriate sandbox variable, and all example code snippets to use these placeholder variables instead of absolute paths. This ensures the documentation reflects the dynamic runtime context provided by the orchestrator.Source: Path instructions
skills/aiq-research/scripts/aiq.py (1)
440-450: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDuplicate filenames cause data loss and broken artifact references.
When multiple artifacts share the same filename (e.g. two
chart.pngfiles), the second overwrites the first in the output directory. Theid_to_relpathmapping becomes incorrect, and report rewrites point to the wrong bytes.Prefix each filename with the artifact ID to ensure uniqueness:
- filename = os.path.basename(artifact.get("filename") or artifact_id) + filename = f"{artifact_id}_{os.path.basename(artifact.get('filename') or artifact_id)}"🤖 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 `@skills/aiq-research/scripts/aiq.py` around lines 440 - 450, The filename assignment in the artifact download loop does not account for multiple artifacts potentially having the same filename, which causes the second artifact to overwrite the first in the artifacts_dir and creates incorrect id_to_relpath mappings. Modify the filename assignment to prefix the filename with the artifact_id (e.g., using a format like "artifact_id_filename") to ensure each artifact gets a unique destination file, preventing overwrites and maintaining correct artifact references in the id_to_relpath mapping.src/aiq_agent/agents/deep_researcher/sandbox/base.py (2)
187-195: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClose the previous session before replacing it in
_reset_session().Line 195 overwrites
self._sessionwithout best-effort closing the old session, which can leak remote resources on recoverable retries.Proposed fix
def _reset_session(self) -> None: """Drop and recreate the session (used only for idempotent recoverable retries).""" + old_session: BaseSandbox | None = None with self._lock: logger.warning( "Sandbox session RESET: provider=%s name=%s (prior in-sandbox files are lost)", self.provider_name, self.sandbox_name, ) - self._session = self._create_session() + old_session = self._session + self._session = self._create_session() + if old_session is not None and hasattr(old_session, "close"): + try: + old_session.close() + except Exception: # noqa: BLE001 + logger.warning("Sandbox %s previous-session cleanup failed", self.sandbox_name, exc_info=True)🤖 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/sandbox/base.py` around lines 187 - 195, In the _reset_session() method, the old session stored in self._session is being replaced without being closed first, which leaks remote resources during recoverable retries. Before the line that calls self._create_session() to overwrite self._session, add code to safely close the previous session using best-effort error handling (catch and suppress any exceptions that occur during the close operation to ensure the new session is always created).
205-213: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftNarrow
_calllock scope to allow timely cancellation/termination.Line 205 keeps the same lock across the full remote call, so
terminate()/close()cannot preempt long-runningexecute()and are blocked until the call returns.Also applies to: 123-130
🤖 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/sandbox/base.py` around lines 205 - 213, The lock scope in the `_call` method is too broad, preventing timely cancellation and termination. The lock is currently held across the entire remote function call to `fn()`, which blocks `terminate()` and `close()` methods from acquiring the lock. Refactor the code to acquire the lock only for critical sections that access shared state: call `self._session_or_create()` inside the lock to obtain the session, then release the lock before executing `fn()` with that session. On recoverable errors, re-acquire the lock only to call `self._reset_session()` and get a new session, then release before retrying the function call. Apply the same lock-narrowing pattern to the similar code block at lines 123-130.src/aiq_agent/agents/deep_researcher/sandbox/config.py (1)
189-191: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winParse legacy
block_networkvalues before mapping network mode.Line 189 currently relies on Python truthiness, so
"false"/"0"are treated as truthy and incorrectly map tonetwork.mode="blocked".Proposed fix
- legacy_block = data.pop("block_network") + legacy_block_raw = data.pop("block_network") + if isinstance(legacy_block_raw, bool): + legacy_block = legacy_block_raw + elif isinstance(legacy_block_raw, str): + normalized = legacy_block_raw.strip().lower() + if normalized in {"1", "true", "yes", "on"}: + legacy_block = True + elif normalized in {"0", "false", "no", "off"}: + legacy_block = False + else: + raise ValueError(f"Invalid block_network value: {legacy_block_raw!r}") + else: + legacy_block = bool(legacy_block_raw) if "network" not in data or data.get("network") is None: data["network"] = {"mode": "blocked" if legacy_block else "open"}🤖 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/sandbox/config.py` around lines 189 - 191, The issue is that the legacy_block variable obtained from data.pop("block_network") is being used directly in a boolean context without proper parsing. String values like "false" or "0" are truthy in Python, so they incorrectly map to network mode "blocked". Parse the legacy_block value into an actual boolean (handling string representations like "false", "0", "true", etc.) before using it in the conditional expression that sets data["network"]["mode"]. This ensures the legacy block_network configuration is correctly interpreted regardless of its string representation.src/aiq_agent/agents/deep_researcher/sandbox/capabilities.py (1)
47-47: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse fail-closed default for artifact download capability.
Line 47 defaults
supports_artifact_downloadtoTrue, soartifact_capture.enabledcan pass capability checks even when a provider forgot to opt in.Proposed fix
- supports_artifact_download: bool = Field(default=True) + supports_artifact_download: bool = Field(default=False)Also applies to: 82-86
🤖 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/sandbox/capabilities.py` at line 47, The supports_artifact_download field defaults to True, which creates a fail-open security posture allowing artifact downloads by default even when providers haven't explicitly opted in. Change the default value of the supports_artifact_download Field from True to False to implement fail-closed security. Additionally, apply the same fix to any other artifact download capability fields mentioned in the "Also applies to" section (around lines 82-86) that may have similar insecure defaults, ensuring all artifact download capabilities default to False unless explicitly enabled by the provider.src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py (2)
144-146: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winParse
AIQ_OPENSHELL_ADAPTER_FILE_TRANSFERas an explicit boolean.Lines 144 and 151 treat any non-empty string as enabled, so values like
"false"or"0"incorrectly switch to adapter transfer mode.Proposed fix
+def _is_truthy_env(name: str) -> bool: + return os.getenv(name, "").strip().lower() in {"1", "true", "yes", "on"} + @@ - if os.getenv(_ADAPTER_FILE_TRANSFER_ENV): + if _is_truthy_env(_ADAPTER_FILE_TRANSFER_ENV): return self._call("upload_files", lambda session: session.upload_files(files), idempotent=True) @@ - if os.getenv(_ADAPTER_FILE_TRANSFER_ENV): + if _is_truthy_env(_ADAPTER_FILE_TRANSFER_ENV): return self._call("download_files", lambda session: session.download_files(paths), idempotent=True)Also applies to: 151-153
🤖 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/sandbox/providers/openshell.py` around lines 144 - 146, The condition checking the _ADAPTER_FILE_TRANSFER_ENV environment variable treats any non-empty string as truthy, which means values like "false" or "0" incorrectly enable adapter transfer mode. Modify the os.getenv() check on lines 144 and 151-153 to explicitly parse the environment variable as a boolean value instead of relying on truthiness. Parse the string value to recognize common boolean representations (such as "true", "1", "yes" as enabled and "false", "0", "no" as disabled) to ensure the feature behaves correctly based on the user's explicit intent.
179-183: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle
exit_code == 3asis_directoryin download classification.Line 180 ignores the
_DOWNLOAD_CODEdirectory signal (sys.exit(3)), which can misreport directory paths aspermission_deniedwhen stderr is empty.Proposed fix
- if getattr(result, "exit_code", 1) != 0: - error = _classify_fs_error(getattr(result, "stderr", "") or "") + exit_code = getattr(result, "exit_code", 1) + if exit_code != 0: + error = "is_directory" if exit_code == 3 else _classify_fs_error(getattr(result, "stderr", "") or "") responses.append(FileDownloadResponse(path=path, content=None, error=error)) continue🤖 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/sandbox/providers/openshell.py` around lines 179 - 183, The current code at line 180 treats all non-zero exit codes uniformly by passing stderr to _classify_fs_error, but it fails to handle the special case where exit_code equals 3, which _DOWNLOAD_CODE uses as a signal indicating the path is a directory. Add a specific check for exit_code == 3 before calling _classify_fs_error; when exit_code is 3, directly create a FileDownloadResponse with is_directory error instead of classifying stderr, which would incorrectly report permission_denied when stderr is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@configs/config_openshell.yml`:
- Line 22: The allow_origin_regex pattern on line 22 is unanchored and uses
unescaped dots, which allows superdomain bypass attacks such as matching
http://localhost.attacker.tld. Fix this by anchoring the regex with ^ at the
start and $ at the end, and escape the literal dots in the URL patterns
(localhost and 127.0.0.1) with backslashes so they match only those exact
strings instead of any character. Apply this same fix consistently across all 5
config files that contain this allow_origin_regex pattern to ensure uniform
security across your configuration.
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 65-70: The _int_env function does not validate that the returned
integer value is actually non-negative as its docstring claims, allowing
negative values that can silently disable sandbox caps. Additionally, the code
at lines 108/116 allows submission when active-job counts are None, which can
invert or disable the cap logic. Add validation in _int_env to ensure the parsed
integer is non-negative and return the default if it is not, and add checks at
the submission logic (lines 108/116) to reject or fail when active-job count
values are None to ensure the sandbox cost cap guard cannot be bypassed through
invalid configuration.
In `@frontends/ui/src/app/api/jobs/async/`[...path]/route.ts:
- Around line 164-183: The new binary artifact content proxy branch in the
isArtifactContent conditional (which handles raw body streaming, no-body 502
error cases, and header passthrough for Content-Type, Content-Disposition, and
Cache-Control) lacks route-level test coverage for this changed user-visible
workflow. Add focused tests to the route test file that cover both the error
path when response.body is missing and the success path when the response body
is streamed through with the correct headers, then run npm lint, type-check, and
build validation to ensure the UI changes pass all checks.
- Around line 175-182: The artifact passthrough section currently hardcodes the
HTTP status to 200 in the NextResponse constructor, which breaks valid upstream
response semantics like partial-content (206) responses. Replace the hardcoded
status: 200 value with the actual upstream response status code by using
response.status instead. This ensures that the proxy preserves the original HTTP
status semantics from the upstream server.
In `@scripts/README.md`:
- Around line 97-102: The pkill -f openshell-gateway command in the cleanup
section is too broad and could accidentally kill unrelated gateway processes.
Replace this line with a more targeted approach that first inspects and
identifies the specific PID of the openshell-gateway process (for example, using
ps or pgrep with more specific matching criteria) and then kills only that
particular process by its PID, rather than using a pattern-based kill that could
match unintended processes.
In `@skills/aiq-research/scripts/aiq.py`:
- Around line 493-501: The artifact downloading loop iterating over
listing.get("artifacts", []) has the same filename collision issue where
duplicate filenames overwrite each other. Apply the same prefix-based
uniquification fix to the filename variable assignment at the line using
os.path.basename(artifact.get("filename") or artifact_id) to ensure each
downloaded artifact has a unique filename, using the artifact_id as a prefix or
similar approach that was already applied elsewhere in the codebase.
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py`:
- Line 416: The ArtifactProvenance initialization is storing empty strings as
placeholder values in the input_file_hashes dictionary for each source file in
entry.source_files, but the field is documented as a mapping of Path to sha256
hash strings. Remove the input_file_hashes parameter from the ArtifactProvenance
constructor call entirely, or replace the empty string placeholders with actual
computed SHA256 hashes of the confined source files. Choose omission if real
hashes cannot be computed at this point in the code flow.
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.py`:
- Around line 60-61: In the except ValidationError block where logger.warning is
called with the manifest validation failure message, remove the exc_info=True
parameter. The full traceback should not be logged because it includes rejected
input field values from manifest fields (title, caption, source_files) that are
user-controlled and could contain sensitive data or accidentally committed
secrets. Since the code gracefully falls back to directory scanning, the
traceback is not necessary for proper error handling.
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py`:
- Around line 264-278: The cleanup_old_artifacts method does not validate that
retention_seconds is positive before constructing the DELETE query. Add a guard
clause at the beginning of the cleanup_old_artifacts method to reject
non-positive retention_seconds values (raise a ValueError or similar exception
if retention_seconds is less than or equal to zero) before proceeding with the
database connection and DELETE statement construction.
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Around line 18-95: Fix markdown linting violations in the README by adding
language specifiers to all fenced code blocks (MD040) and ensuring blank lines
surround code blocks and precede headings (MD031/MD022). Specifically, add
appropriate language identifiers like yaml, python, or diagram to each opening
code fence marker, ensure there is exactly one blank line before each code block
and heading, and ensure there is exactly one blank line after each code block.
Run markdownlint-cli2 to validate all violations are resolved.
In `@src/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.md`:
- Line 30: The SKILL.md documentation contains hardcoded references to the
`/sandbox` directory path on lines 30 and 39, which breaks compatibility with
different runtime environments like Modal that use different sandbox locations.
Replace all occurrences of the hardcoded `/sandbox` path with the
runtime-supplied variable `{{ sandbox_workdir }}` which is provided by the
prompt context. This applies to both instances where `/sandbox` is mentioned in
the context of writing sandbox-local input files and file operations.
---
Duplicate comments:
In `@docs/source/architecture/agents/sandbox.md`:
- Around line 24-31: The Current Behavior section currently includes
Modal-specific naming constraints (64 characters, alphanumeric with
dash/period/underscore) mixed with general provider-agnostic requirements.
Restructure the documentation by removing the Modal-specific details from the
general listing in the Current Behavior section, making it provider-neutral
instead. Then add a new Provider-Specific Constraints subsection that explicitly
lists the exact naming requirements for Modal and OpenShell separately.
- Around line 8-11: Update the sentence in the sandbox scoping explanation that
currently states "the sandbox name is the resolved job ID" to clarify that
sandbox names are derived from (not identical to) the resolved job ID.
Acknowledge that different providers may apply their own normalization or
transformation rules (such as truncation for Modal or naming conventions for
OpenShell) when creating the sandbox name from the job ID. This change should
make the documentation provider-neutral and more accurately reflect the actual
behavior across different sandbox providers.
In `@skills/aiq-research/scripts/aiq.py`:
- Around line 440-450: The filename assignment in the artifact download loop
does not account for multiple artifacts potentially having the same filename,
which causes the second artifact to overwrite the first in the artifacts_dir and
creates incorrect id_to_relpath mappings. Modify the filename assignment to
prefix the filename with the artifact_id (e.g., using a format like
"artifact_id_filename") to ensure each artifact gets a unique destination file,
preventing overwrites and maintaining correct artifact references in the
id_to_relpath mapping.
In `@skills/aiq-research/SKILL.md`:
- Around line 152-159: Replace the shorthand command forms in the documentation
section with fully explicit forms to match the pattern used in earlier steps.
Update the `artifacts <JOB_ID> --download-dir ./aiq-artifacts` command to use
the explicit `python3 $SKILL_DIR/scripts/aiq.py` pattern, and similarly update
the `report <JOB_ID> --out-dir ./my-report` command to use the same explicit
form for consistency throughout the documentation.
In `@src/aiq_agent/agents/deep_researcher/sandbox/base.py`:
- Around line 187-195: In the _reset_session() method, the old session stored in
self._session is being replaced without being closed first, which leaks remote
resources during recoverable retries. Before the line that calls
self._create_session() to overwrite self._session, add code to safely close the
previous session using best-effort error handling (catch and suppress any
exceptions that occur during the close operation to ensure the new session is
always created).
- Around line 205-213: The lock scope in the `_call` method is too broad,
preventing timely cancellation and termination. The lock is currently held
across the entire remote function call to `fn()`, which blocks `terminate()` and
`close()` methods from acquiring the lock. Refactor the code to acquire the lock
only for critical sections that access shared state: call
`self._session_or_create()` inside the lock to obtain the session, then release
the lock before executing `fn()` with that session. On recoverable errors,
re-acquire the lock only to call `self._reset_session()` and get a new session,
then release before retrying the function call. Apply the same lock-narrowing
pattern to the similar code block at lines 123-130.
In `@src/aiq_agent/agents/deep_researcher/sandbox/capabilities.py`:
- Line 47: The supports_artifact_download field defaults to True, which creates
a fail-open security posture allowing artifact downloads by default even when
providers haven't explicitly opted in. Change the default value of the
supports_artifact_download Field from True to False to implement fail-closed
security. Additionally, apply the same fix to any other artifact download
capability fields mentioned in the "Also applies to" section (around lines
82-86) that may have similar insecure defaults, ensuring all artifact download
capabilities default to False unless explicitly enabled by the provider.
In `@src/aiq_agent/agents/deep_researcher/sandbox/config.py`:
- Around line 189-191: The issue is that the legacy_block variable obtained from
data.pop("block_network") is being used directly in a boolean context without
proper parsing. String values like "false" or "0" are truthy in Python, so they
incorrectly map to network mode "blocked". Parse the legacy_block value into an
actual boolean (handling string representations like "false", "0", "true", etc.)
before using it in the conditional expression that sets data["network"]["mode"].
This ensures the legacy block_network configuration is correctly interpreted
regardless of its string representation.
In `@src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py`:
- Around line 144-146: The condition checking the _ADAPTER_FILE_TRANSFER_ENV
environment variable treats any non-empty string as truthy, which means values
like "false" or "0" incorrectly enable adapter transfer mode. Modify the
os.getenv() check on lines 144 and 151-153 to explicitly parse the environment
variable as a boolean value instead of relying on truthiness. Parse the string
value to recognize common boolean representations (such as "true", "1", "yes" as
enabled and "false", "0", "no" as disabled) to ensure the feature behaves
correctly based on the user's explicit intent.
- Around line 179-183: The current code at line 180 treats all non-zero exit
codes uniformly by passing stderr to _classify_fs_error, but it fails to handle
the special case where exit_code equals 3, which _DOWNLOAD_CODE uses as a signal
indicating the path is a directory. Add a specific check for exit_code == 3
before calling _classify_fs_error; when exit_code is 3, directly create a
FileDownloadResponse with is_directory error instead of classifying stderr,
which would incorrectly report permission_denied when stderr is empty.
In `@src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md`:
- Around line 25-27: Replace all hardcoded `/sandbox` path references in the
SKILL.md documentation with runtime-supplied placeholder variables.
Specifically, update the artifact directory path references to use `{{
sandbox_artifact_dir }}` instead of `/sandbox/aiq-artifacts/`, the script paths
to use `{{ sandbox_workdir }}` instead of `/sandbox`, the manifest path
generation to reference the appropriate sandbox variable, and all example code
snippets to use these placeholder variables instead of absolute paths. This
ensures the documentation reflects the dynamic runtime context provided by the
orchestrator.
🪄 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: 7b3cc76d-2ee6-4823-b12e-272b4989de5b
📒 Files selected for processing (55)
.gitignoreconfigs/config_openshell.ymlconfigs/openshell/Dockerfile.aiq-democonfigs/openshell/aiq-research-policy.yamldocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/src/aiq_api/jobs/access.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsscripts/README.mdscripts/setup_openshell.shskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pysrc/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/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/common/citation_verification.pytests/aiq_agent/agents/deep_researcher/sandbox/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pytests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/jobs/test_runner.py
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
skills/aiq-research/scripts/aiq.py (1)
493-518: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMissing artifact_id validation in artifacts command.
Line 511 extracts
artifact_idbut doesn't validate it before passing todownload_artifact()on line 513. Unlike_export_report_bundle()(lines 457–459), there's no earlycontinueif the ID is missing or empty. If the listing contains artifacts without IDs, the download will fail silently or make an invalid request.Add a guard:
artifact_id = artifact.get("artifact_id", "") if not artifact_id: continue🤖 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 `@skills/aiq-research/scripts/aiq.py` around lines 493 - 518, In the _command_artifacts function, add a validation check immediately after extracting the artifact_id variable from the artifact dictionary. If the artifact_id is empty or falsy, use continue to skip that artifact and move to the next iteration of the loop. This prevents passing invalid or missing artifact IDs to the download_artifact function call, which would cause the download operation to fail. Apply the same defensive pattern that is used in the _export_report_bundle function.src/aiq_agent/agents/deep_researcher/sandbox/capabilities.py (1)
27-29: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the docstring with the fail-closed artifact default.
Line 47 now defaults artifact download support to
False, but the docstring still says unknown providers support artifact download.Proposed fix
- Defaults are conservative: an unknown provider is assumed to support nothing - except artifact download, so the fail-closed gate refuses workloads that - require guarantees the provider has not explicitly claimed. + Defaults are conservative: an unknown provider is assumed to support no + security or lifecycle guarantees, so the fail-closed gate refuses workloads + that require guarantees the provider has not explicitly claimed.Also applies to: 47-47
🤖 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/sandbox/capabilities.py` around lines 27 - 29, The docstring in the fail-closed gate section (lines 27-29) states that unknown providers are assumed to support artifact download by default, but this contradicts the actual default behavior set at line 47 which defaults artifact download support to False. Update the docstring to remove the reference to artifact download being an exception and instead reflect that unknown providers are assumed to support nothing by default, aligning with the conservative fail-closed approach and the False default value.frontends/ui/src/pages/api/generate-pdf.ts (1)
54-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the same embeddable MIME allowlist before buffering.
Line 58 accepts any
image/*, butReactPdfDocument.tsxonly collects PNG/JPEG/WebP data URIs. Non-embeddable images like SVG/GIF are fetched, buffered, rewritten, and then silently skipped by the PDF renderer.Suggested MIME normalization
const MAX_PDF_ARTIFACT_REFS = 25 +const PDF_EMBEDDABLE_IMAGE_RE = /^image\/(?:png|jpe?g|webp)(?:\s*;|$)/i ... }) const contentType = resp.headers.get('Content-Type') ?? 'application/octet-stream' console.log(`[PDF] fetch ${id}: status=${resp.status} type=${contentType}`) if (!resp.ok) return // Only raster images are embeddable in the PDF. - if (!contentType.startsWith('image/')) return + if (!PDF_EMBEDDABLE_IMAGE_RE.test(contentType)) return + const mediaType = contentType.split(';', 1)[0].toLowerCase() ... - dataUris.set(id, `data:${contentType};base64,${buffer.toString('base64')}`) + dataUris.set(id, `data:${mediaType};base64,${buffer.toString('base64')}`)🤖 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/pages/api/generate-pdf.ts` around lines 54 - 70, The content type check on line 58 accepts any image MIME type using startsWith('image/'), but ReactPdfDocument.tsx only embeds PNG/JPEG/WebP formats. Replace the generic image type validation with a specific allowlist that only permits the embeddable MIME types (image/png, image/jpeg, image/webp) before performing the expensive buffer operations, so non-embeddable formats like SVG and GIF are rejected early rather than being buffered and later silently skipped by the PDF renderer.frontends/ui/src/app/api/jobs/async/[...path]/route.ts (1)
175-181: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPreserve artifact security headers through the proxy.
The browser receives this proxy response, so dropping upstream
X-Content-Type-Optionsand overridingCache-Controlcan remove backend artifact hardening for same-origin generated files.Suggested header preservation
const passthroughHeaders: Record<string, string> = { 'Content-Type': response.headers.get('Content-Type') ?? 'application/octet-stream', - 'Cache-Control': 'private, max-age=3600', + 'Cache-Control': response.headers.get('Cache-Control') ?? 'private, max-age=3600', + 'X-Content-Type-Options': response.headers.get('X-Content-Type-Options') ?? 'nosniff', }As per path instructions, UI changes must preserve API contract alignment and auth/session handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontends/ui/src/app/api/jobs/async/`[...path]/route.ts around lines 175 - 181, The passthroughHeaders object in the response proxy is dropping critical security headers from the upstream response. Add preservation of the X-Content-Type-Options header by retrieving it from response.headers.get() and including it in passthroughHeaders. Additionally, instead of hardcoding the Cache-Control value, preserve the upstream Cache-Control header from the response if it exists, or only use the default value as a fallback when the upstream header is absent. This ensures that security hardening and caching policies from the backend artifact are maintained through the proxy.Source: Path instructions
♻️ Duplicate comments (1)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py (1)
158-159: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake digest dedup atomic at the DB boundary.
Line 167 does a read-before-write dedup check, but Line 158 is only a non-unique index. Under concurrent writes, two inserts for the same
(job_id, sha256)can both commit, which breaks idempotency guarantees.Suggested fix
@@ - from sqlalchemy import Index + from sqlalchemy import Index + from sqlalchemy import UniqueConstraint @@ Column("created_at", DateTime, server_default=func.now()), Index("idx_artifacts_job_sha", "job_id", "sha256"), + UniqueConstraint("job_id", "sha256", name="uq_artifacts_job_sha"), ) @@ - with self._engine.connect() as conn: - conn.execute( + from sqlalchemy.exc import IntegrityError + + with self._engine.connect() as conn: + try: + conn.execute( text( @@ - ) - conn.commit() + ) + conn.commit() + except IntegrityError: + conn.rollback() + existing = self.find_by_digest(artifact.job_id, artifact.sha256) + if existing is not None: + return existing + raise return storedAlso applies to: 166-210
🤖 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/sandbox/artifacts/store.py` around lines 158 - 159, The non-unique index "idx_artifacts_job_sha" on columns "job_id" and "sha256" allows duplicate entries to be inserted concurrently, which breaks the dedup check performed at line 167. Convert this regular index to a UNIQUE index or constraint to enforce uniqueness at the database boundary level, preventing concurrent writes from both committing duplicate (job_id, sha256) pairs. This will make the dedup operation atomic at the database level rather than relying on application-level read-before-write checks.
🤖 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 728-740: The current filtering of the artifact filename in the
safe_filename variable prevents injection attacks but doesn't handle non-Latin-1
characters (emoji, CJK, etc.), which cause UnicodeEncodeError during HTTP header
encoding. Implement RFC 5987 encoding by updating the safe_filename logic to
percent-encode non-ASCII characters and modify the Content-Disposition header to
use the filename*=UTF-8'' parameter format for filenames containing non-Latin-1
characters, while keeping a fallback filename parameter for older client
compatibility.
In `@frontends/ui/src/lib/pdf/ReactPdfDocument.tsx`:
- Around line 228-236: The DATA_IMAGE_RE regex pattern in the
isEmbeddableDataImage function currently accepts WebP images, but the
`@react-pdf/renderer` library version 4.3.2 only supports PNG and JPEG formats.
Remove the `webp|` alternative from the regex pattern in DATA_IMAGE_RE so that
only PNG and JPEG image data URIs are matched, preventing WebP images from being
embedded and failing silently during PDF rendering.
In `@src/aiq_agent/agents/deep_researcher/deepagents_runtime.py`:
- Around line 185-186: The artifact directory path construction in this method
directly concatenates self.job_id without encoding, which allows path traversal
attacks if job_id contains characters like / or ... To fix this, encode
self.job_id using URL encoding or a path-safe encoding mechanism (such as
urllib.parse.quote) before concatenating it into the f-string that builds the
artifact directory path. This ensures that malicious job_id values cannot escape
the configured artifact base directory and maintains proper job-scoped artifact
isolation.
In `@src/aiq_agent/agents/deep_researcher/sandbox/config.py`:
- Around line 190-194: The current code in the isinstance check for legacy_block
being a string only validates truthy values from the set {"1", "true", "yes",
"on"}, silently defaulting everything else to False. This is a security issue
because typos like "flase" would open network egress unintentionally. Create an
explicit falsy set (such as {"0", "false", "no", "off"}) and modify the
conditional logic to check against both truthy and falsy sets separately. If the
stripped lowercase legacy_block value matches the truthy set, set it to True; if
it matches the falsy set, set it to False; if it matches neither, raise a
ValueError or similar exception to reject the unrecognized value instead of
silently defaulting to False.
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Line 18: The README.md file has unresolved markdown linting violations that
need to be fixed. Run markdownlint-cli2 to identify all violations, then
systematically fix them by: adding language specifiers to all bare code fences
(MD040) such as adding ```diagram, ```text, or other appropriate language
identifiers to line 18 and other code blocks; adding blank lines before opening
fences (MD031) at lines 77, 87, 91, 122, 177, 180, 204; adding blank lines
before headings (MD022) at lines 83, 140, 148, 165, 169; and adding blank lines
after closing fences at lines 79-82, 96-98, 127-129. Ensure all code blocks have
proper language specifiers and all fences and headings have appropriate spacing
according to markdown linting standards.
In `@src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md`:
- Around line 99-101: The example code at lines 99-101 in the chart-generation
SKILL.md file hardcodes the artifact directory path to `/sandbox/aiq-artifacts`,
which contradicts the earlier guidance in lines 25-28 and 39-40 that correctly
instruct users to substitute `sandbox_artifact_dir` from their prompt context
(which varies by environment: `/sandbox` on OpenShell or `/workspace` on Modal).
Update the ARTIFACT_DIR assignment to either use an environment variable with a
fallback like `os.environ.get("SANDBOX_ARTIFACT_DIR", "/sandbox/aiq-artifacts")`
to dynamically resolve the path, or add a clarifying comment above the hardcoded
line explaining that users must substitute their actual sandbox_artifact_dir
value from the prompt context. This ensures the example works correctly across
different execution environments without breaking on Modal.
In `@tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py`:
- Around line 289-293: The test method
`test_append_artifact_index_noop_without_artifacts` is defined twice in the test
file, with the second definition at lines 289-293 shadowing the first definition
at line 284. Remove the duplicate method definition shown in the diff (the one
at lines 289-293) to ensure both test cases are properly collected and executed
by the test runner.
---
Outside diff comments:
In `@frontends/ui/src/app/api/jobs/async/`[...path]/route.ts:
- Around line 175-181: The passthroughHeaders object in the response proxy is
dropping critical security headers from the upstream response. Add preservation
of the X-Content-Type-Options header by retrieving it from
response.headers.get() and including it in passthroughHeaders. Additionally,
instead of hardcoding the Cache-Control value, preserve the upstream
Cache-Control header from the response if it exists, or only use the default
value as a fallback when the upstream header is absent. This ensures that
security hardening and caching policies from the backend artifact are maintained
through the proxy.
In `@frontends/ui/src/pages/api/generate-pdf.ts`:
- Around line 54-70: The content type check on line 58 accepts any image MIME
type using startsWith('image/'), but ReactPdfDocument.tsx only embeds
PNG/JPEG/WebP formats. Replace the generic image type validation with a specific
allowlist that only permits the embeddable MIME types (image/png, image/jpeg,
image/webp) before performing the expensive buffer operations, so non-embeddable
formats like SVG and GIF are rejected early rather than being buffered and later
silently skipped by the PDF renderer.
In `@skills/aiq-research/scripts/aiq.py`:
- Around line 493-518: In the _command_artifacts function, add a validation
check immediately after extracting the artifact_id variable from the artifact
dictionary. If the artifact_id is empty or falsy, use continue to skip that
artifact and move to the next iteration of the loop. This prevents passing
invalid or missing artifact IDs to the download_artifact function call, which
would cause the download operation to fail. Apply the same defensive pattern
that is used in the _export_report_bundle function.
In `@src/aiq_agent/agents/deep_researcher/sandbox/capabilities.py`:
- Around line 27-29: The docstring in the fail-closed gate section (lines 27-29)
states that unknown providers are assumed to support artifact download by
default, but this contradicts the actual default behavior set at line 47 which
defaults artifact download support to False. Update the docstring to remove the
reference to artifact download being an exception and instead reflect that
unknown providers are assumed to support nothing by default, aligning with the
conservative fail-closed approach and the False default value.
---
Duplicate comments:
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py`:
- Around line 158-159: The non-unique index "idx_artifacts_job_sha" on columns
"job_id" and "sha256" allows duplicate entries to be inserted concurrently,
which breaks the dedup check performed at line 167. Convert this regular index
to a UNIQUE index or constraint to enforce uniqueness at the database boundary
level, preventing concurrent writes from both committing duplicate (job_id,
sha256) pairs. This will make the dedup operation atomic at the database level
rather than relying on application-level read-before-write checks.
🪄 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: 8d318705-95fd-48f7-9ad4-0d7acbc8f0f3
📒 Files selected for processing (27)
configs/config_openshell.ymlfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxscripts/README.mdskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/common/citation_verification.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.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/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pyskills/aiq-research/scripts/aiq.pysrc/aiq_agent/common/citation_verification.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/deepagents_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/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.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:
src/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pyscripts/README.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pyfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxskills/aiq-research/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pyconfigs/config_openshell.ymlfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdskills/aiq-research/scripts/aiq.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/common/citation_verification.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.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/__init__.pysrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
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/SKILL.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/SKILL.mdskills/aiq-research/scripts/aiq.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_openshell.yml
{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.pyfrontends/aiq_api/src/aiq_api/routes/jobs.py
skills/aiq-research/**/*.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/**/*.py: Use Python 3.11+ as the minimum version for running the AIQ Blueprint helper script
Set AIQ_SERVER_URL environment variable when the AI-Q backend is not running at the default http://localhost:8000
Preserve and present citations and source URLs intact in returned research reports without truncation
Run health check before sending research requests to verify backend reachability
Do not retry failed deep research jobs automatically; show the error and ask user whether to retry with a narrower query or different approach
Confirm user explicitly trusts non-local AIQ_SERVER_URL endpoints before sending any query
Do not put sensitive or confidential information in user query text when sending to AIQ_SERVER_URL, as remote endpoints may log prompts and responses
Poll asynchronous deep research jobs by extracting the job_id and running research_poll command until completion
Export portable reports with rewritten artifact links to local files using the report command with --out-dir parameter
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**/*.{py,sh,bash}
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
Do not include API keys, bearer tokens, cookies, or basic-auth credentials in AIQ_SERVER_URL environment variable or command examples
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**/*.{py,sh,bash,yml,yaml,json}
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
Store backend credentials in the AI-Q deployment environment, not in the AIQ skill or command examples
Files:
skills/aiq-research/scripts/aiq.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:
src/aiq_agent/agents/deep_researcher/sandbox/config.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T21:30:24.907Z
Learning: Ensure Blueprint major version matches skill major version; Blueprint minor version must be equal or greater than skill minor version for compatibility
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T21:30:24.907Z
Learning: Use the AIQ research skill only for research-shaped requests; do not use it for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T21:30:24.907Z
Learning: Request explicit user approval before using elevated permissions for non-blocking background execution of deep research jobs
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-23T21:30:24.907Z
Learning: Hand off deployment requests to aiq-deploy skill and preserve the original research request for later processing
📚 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_sandbox_concurrency.py
🪛 OpenGrep (1.23.0)
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx
[ERROR] 231-231: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py
[ERROR] 330-330: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (36)
configs/config_openshell.yml (3)
22-22: LGTM!
8-33: Environment parameterization and CORS configuration are secure.The CORS regex is properly anchored with
^and$and uses escaped dots, preventing superdomain bypass. All sensitive endpoints (db_url, front_end) use environment variables with safe defaults.
134-185: Sandbox configuration properly parameterized.OpenShell gateway, policy file, and sandbox name are all configurable via environment variables with sensible defaults. Network policy is blocked by default (fail-closed). Artifact capture extensions are allowlisted. This aligns with the OpenShellSandboxProvider capabilities declared in the codebase.
scripts/README.md (3)
52-105: Setup workflow documentation is clear and safe.The
setup_openshell.shsection clearly describes the one-time setup, provides version and policy examples, and correctly instructs users to usepgrepto inspect the gateway PID before killing (lines 101–103), avoiding broad pattern-based kills. The note that "inference stays host-side" and only generated code runs in the sandbox is important and explicit.
169-169: LGTM!
186-187: Config table entries are well-documented with dependency notes.The new
config_openshell.ymlentry includes a clear dependency note: "(runsetup_openshell.shfirst)". This helps users understand the prerequisite setup step.tests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py (1)
116-116: LGTM!skills/aiq-research/scripts/aiq.py (3)
423-435: Filename collision fix is sound.The
_unique_filename()helper correctly prevents overwriting by checking both theusedset and filesystem, with numeric suffix fallback. This resolves the data-integrity issue from previous review where duplicate basenames would silently overwrite.
438-475: Artifact export bundle collision handling applied correctly.The rewrite pipeline now uses
_unique_filename()to prevent overwrites when multiple artifacts share basenames, and maintainsid_to_relpathfor markdown link resolution. JSON output format is consistent with CLI conventions.
589-589: LGTM!skills/aiq-research/SKILL.md (2)
152-159: LGTM!
209-210: LGTM!src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md (1)
66-87: LGTM!src/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.md (1)
30-30: LGTM!Also applies to: 39-39
src/aiq_agent/agents/deep_researcher/sandbox/README.md (1)
16-208: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.py (1)
61-61: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.py (1)
56-65: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py (2)
18-18: LGTM!Also applies to: 330-330
418-430: LGTM!frontends/aiq_api/src/aiq_api/jobs/runner.py (2)
585-590: Still useterminate()for interrupted sandbox jobs.The terminal cleanup path still calls
close()unconditionally, so the previously flagged cancellation/timeout concern remains unresolved.
282-282: LGTM!Also applies to: 459-475, 515-523
frontends/aiq_api/src/aiq_api/routes/jobs.py (3)
527-532: Sandbox capacity checks are still not atomic with submission.The cap check still runs before
submit_authorized_job(), so concurrent requests can observe capacity and then all submit.
65-78: LGTM!
699-707: LGTM!src/aiq_agent/agents/deep_researcher/deepagents_runtime.py (2)
135-155: LGTM!
202-237: LGTM!src/aiq_agent/common/citation_verification.py (1)
880-880: LGTM!Also applies to: 943-966
frontends/aiq_api/tests/test_sandbox_concurrency.py (1)
9-21: LGTM!Also applies to: 28-47, 50-85
src/aiq_agent/agents/deep_researcher/sandbox/__init__.py (1)
20-21: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/base.py (1)
195-202: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py (1)
45-49: LGTM!Also applies to: 149-158, 185-188
frontends/ui/src/pages/api/generate-pdf.ts (2)
46-75: Duplicate: avoid concurrent buffering of all artifact refs.The cap helps, but
Promise.allcan still fetch and buffer up to 25 artifacts concurrently; the prior bounded/sequential fetch finding remains partly unresolved.
101-109: Duplicate: gate auth forwarding on auth mode.This still forwards
Authorization/idTokeneven when auth is disabled; the prior auth-boundary finding remains applicable.frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsx (1)
7-18: LGTM!Also applies to: 48-48, 183-198, 238-243
frontends/ui/src/app/api/jobs/async/[...path]/route.ts (1)
92-93: LGTM!Also applies to: 102-111
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx (1)
224-227: LGTM!Also applies to: 239-256, 309-339
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md (1)
99-106:⚠️ Potential issue | 🟡 MinorReplace hardcoded
/sandbox/aiq-artifacts/with runtime-supplied variable.The example Python code at line 112 hardcodes
ARTIFACT_DIR = "/sandbox/aiq-artifacts", but the comment above acknowledges two environments (OpenShell and Modal with different paths). Use a runtime variable instead—for example,ARTIFACT_DIR = os.environ.get("ARTIFACT_DIR", "/workspace/aiq-artifacts")or fetch it from the skill runtime context. Environment-specific paths must not be hardcoded in examples per the coding guidelines.🤖 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/skills/chart-generation/SKILL.md` around lines 99 - 106, The example Python code contains a hardcoded ARTIFACT_DIR path that is environment-specific and violates the coding guidelines. Replace the hardcoded assignment `ARTIFACT_DIR = "/sandbox/aiq-artifacts"` with a runtime-supplied variable using `os.environ.get("ARTIFACT_DIR", "/workspace/aiq-artifacts")` to make the example work across different environments (OpenShell and Modal). Alternatively, fetch the ARTIFACT_DIR from the skill runtime context if available.Source: Path instructions
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
577-602: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTerminate interrupted sandboxes before final harvest.
When
interruptedisTrue, Line 579 still runsfinal_harvest()before Line 600 callsterminate(). If a sandboxexecuteis still in flight, that harvest can block on the provider/session instead of preempting the job. Skip harvest on interrupted paths, or terminate first. As per path instructions, job-runner changes are externally visible async job lifecycle contracts.Suggested fix
finally: # Terminal ordering: final artifact harvest -> flush events -> cleanup sandbox. # The harvest emits artifact SSE events, so it must run before the flush. - if sandbox_runtime is not None and hasattr(sandbox_runtime, "final_harvest"): + if interrupted and sandbox_runtime is not None: + terminate = getattr(sandbox_runtime, "terminate", None) + if terminate is not None: + try: + terminate() + except Exception: + logger.warning("Sandbox cleanup failed for job %s", job_id, exc_info=True) + + if not interrupted and sandbox_runtime is not None and hasattr(sandbox_runtime, "final_harvest"): try: await asyncio.to_thread(sandbox_runtime.final_harvest) except Exception: logger.warning("Final artifact harvest failed for job %s", job_id, exc_info=True) @@ - if sandbox_runtime is not None: + if sandbox_runtime is not None and not interrupted: teardown = None - if interrupted: - teardown = getattr(sandbox_runtime, "terminate", None) - if teardown is None: - teardown = getattr(sandbox_runtime, "close", None) + teardown = getattr(sandbox_runtime, "close", 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 `@frontends/aiq_api/src/aiq_api/jobs/runner.py` around lines 577 - 602, The final_harvest() call starting at line 579 executes before the sandbox terminate() call at line 600 when a job is interrupted, which can cause the harvest to block instead of preempting the job. Modify the condition guarding the final_harvest call (the if statement checking `if sandbox_runtime is not None and hasattr(sandbox_runtime, "final_harvest")`) to also check `and not interrupted` so that the harvest is skipped entirely when the job has been interrupted, allowing the terminate() cleanup to take priority.Source: Path instructions
♻️ Duplicate comments (1)
src/aiq_agent/agents/deep_researcher/deepagents_runtime.py (1)
189-190: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse an injective path-safe encoding for
job_id.Line 189 maps distinct IDs like
a/banda?bto the samea_b, so two different jobs can share anartifact_dirin a persistent sandbox. Use reversible URL-safe encoding or append a stable hash instead of character replacement. As per coding guidelines, “apply owner guardrails before loading protected report or artifact context into an agent.”Suggested fix
+from urllib.parse import quote + ... - safe_job = "".join(c if (c.isalnum() or c in "-_") else "_" for c in self.job_id) or "job" + safe_job = quote(self.job_id, safe="") or "job" return f"{base.rstrip('/')}/{safe_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 `@src/aiq_agent/agents/deep_researcher/deepagents_runtime.py` around lines 189 - 190, The current character replacement logic in the path construction within the method containing safe_job variable creation uses simple underscore substitution, which causes collisions where distinct job IDs like "a/b" and "a?b" both map to the same "a_b", allowing different jobs to share the same artifact_dir. Replace the character replacement approach with a reversible encoding method such as URL-safe base64 encoding of the entire job_id, or append a stable hash of the original job_id to ensure distinct job IDs always produce distinct safe paths. This preserves the security requirement that each job has an isolated artifact directory.Source: Coding guidelines
🤖 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 `@src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2`:
- Around line 102-103: In the orchestrator.j2 file, the "Embedding figures and
artifacts" instruction currently states that generated charts MUST be embedded
unconditionally, while the "Visualization is earned" instruction specifies
conditions when charts should NOT be embedded (unsourced data, incomplete
series, or misleading trends). Revise the "Embedding figures and artifacts"
section to make embedding conditional on passing the quality criteria from
"Visualization is earned" — specifically, a chart should only be embedded if its
data is source-anchored, reasonably complete, and does not mislead. This ensures
the quality standards gate embedding rather than allowing the "MUST" statement
to override completeness checks.
In `@src/aiq_agent/agents/deep_researcher/sandbox/config.py`:
- Line 197: The current condition on line 197 includes an empty string in the
set of falsy values that determine network access mode, creating a fail-open
security vulnerability where unset or empty environment variables default to
allowing network access. Remove the empty string from the set in the elif
condition that checks `raw in {"0", "false", "no", "off", ""}` so that only
explicit falsy tokens like "0", "false", "no", and "off" are recognized as
disabling network access, and empty values must be handled explicitly rather
than defaulting to open network mode.
---
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 577-602: The final_harvest() call starting at line 579 executes
before the sandbox terminate() call at line 600 when a job is interrupted, which
can cause the harvest to block instead of preempting the job. Modify the
condition guarding the final_harvest call (the if statement checking `if
sandbox_runtime is not None and hasattr(sandbox_runtime, "final_harvest")`) to
also check `and not interrupted` so that the harvest is skipped entirely when
the job has been interrupted, allowing the terminate() cleanup to take priority.
In `@src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md`:
- Around line 99-106: The example Python code contains a hardcoded ARTIFACT_DIR
path that is environment-specific and violates the coding guidelines. Replace
the hardcoded assignment `ARTIFACT_DIR = "/sandbox/aiq-artifacts"` with a
runtime-supplied variable using `os.environ.get("ARTIFACT_DIR",
"/workspace/aiq-artifacts")` to make the example work across different
environments (OpenShell and Modal). Alternatively, fetch the ARTIFACT_DIR from
the skill runtime context if available.
---
Duplicate comments:
In `@src/aiq_agent/agents/deep_researcher/deepagents_runtime.py`:
- Around line 189-190: The current character replacement logic in the path
construction within the method containing safe_job variable creation uses simple
underscore substitution, which causes collisions where distinct job IDs like
"a/b" and "a?b" both map to the same "a_b", allowing different jobs to share the
same artifact_dir. Replace the character replacement approach with a reversible
encoding method such as URL-safe base64 encoding of the entire job_id, or append
a stable hash of the original job_id to ensure distinct job IDs always produce
distinct safe paths. This preserves the security requirement that each job has
an isolated artifact directory.
🪄 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: bf34125f-fb02-45b6-8600-f8e7b6ac1107
📒 Files selected for processing (14)
frontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxsrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**
⚙️ 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:
src/aiq_agent/agents/deep_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pysrc/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.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/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.py
**/*.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/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/deepagents_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/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.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:
src/aiq_agent/agents/deep_researcher/sandbox/config.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/runner.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Set AIQ_SERVER_URL environment variable when the AI-Q backend is not running at the default http://localhost:8000; non-local values must be trusted by the user before any query is sent
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Before sending any user query to an AI-Q backend, state the exact backend URL that will receive it and confirm it is trusted for sensitive information
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Do not use the aiq-research skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests; use the aiq-deploy skill for those purposes
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Use the aiq-research skill for research-shaped requests including deep research, AIQ research, research queries, or asking AI-Q to answer questions
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Check health of the AI-Q backend with health command before sending research requests
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: When the backend is not reachable and no explicit AIQ_SERVER_URL is set, ask the user whether they have an existing backend URL or want to deploy a local Skill backend
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: If backend returns 401 or 403 authentication errors, stop and explain that this public skill does not manage authentication; ask user to use an authenticated AI-Q skill
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: If health succeeds but /chat or /v1/jobs/async/agents fails, report that the backend is reachable but not compatible and offer to run aiq-deploy validation
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Do not send credentials, cookies, bearer tokens, or secret values through query text to the AI-Q backend; store backend credentials in the AI-Q deployment environment
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Extract job_id from deep_research_running response and poll with research_poll command until job completes
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Use status, report, or research_poll commands to resume polling after interruptions; use status to inspect job status and artifacts, report when job has finished, research_poll to keep waiting
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: When job includes artifact:// links in report, run artifacts command with --download-dir to materialize them as local files instead of expecting base64 image data
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: For self-contained shareable reports, run report command with --out-dir flag to write report.md plus artifacts/ folder with links rewritten to local files
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Keep citations and source URLs intact when presenting returned reports
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: If job status is failed, failure, or cancelled, show the error and ask whether user wants to retry with narrower query or different approach; do not retry automatically
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Verify that Blueprint version is compatible with this skill using semantic versioning rules: major versions must match, minor version must be equal or greater, patch version can be anything
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: If Blueprint version is not compatible with skill version 2.1.0, check for an updated skill matching the Blueprint version or use a compatible Blueprint version; proceed with caution only when user accepts compatibility risk
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Do not put API keys, bearer tokens, cookies, or basic-auth credentials in AIQ_SERVER_URL environment variable
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Treat returned reports as potentially sensitive if the backend uses private data sources
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Do not truncate citations or source URLs from returned reports
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T01:26:13.464Z
Learning: Report failures instead of fabricating research answers when backend returns HTTP 500 or lacks async agents
🪛 LanguageTool
src/aiq_agent/agents/deep_researcher/skills/chart-generation/SKILL.md
[style] ~42-~42: ‘a majority of’ might be wordy. Consider a shorter alternative.
Context: ...harts:** if a series is mostly missing (a majority of periods undisclosed) or mixes metric...
(EN_WORDINESS_PREMIUM_A_MAJORITY_OF)
🪛 OpenGrep (1.23.0)
tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py
[ERROR] 283-283: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 291-291: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 319-319: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (13)
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx (1)
228-230: LGTM!tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py (1)
169-179: LGTM!Also applies to: 207-212
tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py (1)
17-23: LGTM!Also applies to: 153-157, 280-332
src/aiq_agent/agents/deep_researcher/skills/data-table-analysis/SKILL.md (1)
26-55: LGTM!The updated "Data honesty" section (lines 26–40) establishes clear requirements for data integrity and disclosure. Lines 46 and 55 correctly reference
sandbox_workdiras a runtime-supplied variable with provider-specific examples (/sandboxon OpenShell,/workspaceon Modal), maintaining provider-neutral contracts per path instructions.Source: Path instructions
src/aiq_agent/agents/deep_researcher/sandbox/__init__.py (1)
27-27: LGTM!Also applies to: 40-40
src/aiq_agent/agents/deep_researcher/sandbox/base.py (1)
41-42: LGTM!Also applies to: 70-78, 121-165, 209-272
frontends/aiq_api/src/aiq_api/routes/jobs.py (2)
527-532: Existing capacity-guard thread still applies.Line 530 still documents fail-open behavior when active-job counts are unknown; this matches the earlier sandbox cap verification concern, so I’m not re-raising it as a new issue here.
690-707: LGTM!Also applies to: 716-752
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py (1)
125-133: LGTM!src/aiq_agent/agents/deep_researcher/deepagents_runtime.py (1)
125-155: LGTM!Also applies to: 206-225, 237-251
src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2 (1)
10-19: LGTM!src/aiq_agent/agents/deep_researcher/prompts/researcher.j2 (1)
10-10: LGTM!Also applies to: 25-26
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
282-285: LGTM!Also applies to: 462-478, 518-526, 533-535
|
the durable artifact runtime is the right foundation — capturing generated files alongside the execution context. the missing layer for enterprise/regulated deployments: the artifacts need to be tamper-evidently bound to the session that produced them. the receipt layer closes this: when the sandbox produces an artifact, the runtime emits a signed receipt binding the artifact digest to the agent session. for the provider-neutral angle: the receipt format is runtime-agnostic by design — it's just JCS-canonical JSON + Ed25519. a verifier doesn't need to know which sandbox provider generated the artifact, only that the receipt's signature checks against the agent's public key.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py (1)
327-340: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winApply the scan cap before buffering
findoutput.The new cap is enforced only after
backend.execute(...)returns, so a sandbox with many files can still force a hugefindstdout into memory. Limit in the sandbox command as well, then keep the existing parser-side cap as a backstop.Suggested fix
def _scan_dir(self) -> list[ManifestEntry]: + # Bound the sandbox-side output before it is buffered by the backend. + max_scan = max(self.config.max_file_count * 5, 100) try: - response = self.backend.execute(f"find {shlex.quote(self.artifact_dir)} -type f") + response = self.backend.execute( + f"find {shlex.quote(self.artifact_dir)} -type f | head -n {max_scan + 1}" + ) except Exception: # noqa: BLE001 - scan is best-effort logger.warning("Artifact scan failed for job %s", self.job_id, exc_info=True) return [] output = getattr(response, "output", "") or "" entries: list[ManifestEntry] = [] # Bound the scan so a flood of files can't drive one download round-trip each; # generous relative to the count quota, which is the real ceiling on stored files. - max_scan = max(self.config.max_file_count * 5, 100) for line in output.splitlines():🤖 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/sandbox/artifacts/manager.py` around lines 327 - 340, Apply the scan cap before collecting `find` output in `_scan_dir` so a large sandbox cannot dump an unbounded listing into memory. Update the `backend.execute(...)` command built in `manager.py` to limit results in the sandbox itself, and keep the existing `max_scan` check in `_scan_dir` as a parser-side backstop. Use the `_scan_dir` logic and `self.backend.execute` call as the main places to patch.src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py (1)
249-252: 🩺 Stability & Availability | 🟠 MajorClose the OpenShell context if adapter construction fails
src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py:249-252
_create_session()bubbles out ofOpenShellSandbox(...), and the base provider does not clean up failed session creation. Wrap__enter__()and adapter construction together, and call__exit__()on any exception before re-raising.🤖 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/sandbox/providers/openshell.py` around lines 249 - 252, The OpenShell session setup in `_create_session()` leaves the sandbox open if `OpenShellSandbox(...)` fails after `os_sandbox.__enter__()` succeeds. Wrap the `os_sandbox.__enter__()` and `OpenShellSandbox(...)` construction together in a try/except, and on any exception call `os_sandbox.__exit__()` before re-raising so the context is always cleaned up. Use the existing `_create_session`, `os_sandbox`, and `OpenShellSandbox` symbols to update the failure path without changing the successful flow.
🤖 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 `@src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py`:
- Around line 72-76: The file-size guard in the openshell sandbox is only
checking os.path.getsize before open(...).read(), so a file can grow after the
check and still be fully buffered and encoded. Update the logic in the snippet
built by openshell.py to enforce the limit during the actual read by reading at
most limit + 1 bytes from the path and exiting with code 4 if the read exceeds
the allowed size. Keep the existing realpath/dirname validation and size check
structure, but make the final read in the same code path responsible for the
limit enforcement.
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Around line 189-199: The OpenShell adapter install guidance is inconsistent
between the README and the runtime hint in openshell.py, so align them to one
canonical git source or clearly document a priority/fallback order. Update the
install instructions in the sandbox README and the import/setup hint in the
OpenShell provider so both reference the same repository/branch for the argv
file-transfer fix, using the symbols setup_openshell.sh and OpenShell provider
guidance to locate the affected text.
---
Outside diff comments:
In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py`:
- Around line 327-340: Apply the scan cap before collecting `find` output in
`_scan_dir` so a large sandbox cannot dump an unbounded listing into memory.
Update the `backend.execute(...)` command built in `manager.py` to limit results
in the sandbox itself, and keep the existing `max_scan` check in `_scan_dir` as
a parser-side backstop. Use the `_scan_dir` logic and `self.backend.execute`
call as the main places to patch.
In `@src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py`:
- Around line 249-252: The OpenShell session setup in `_create_session()` leaves
the sandbox open if `OpenShellSandbox(...)` fails after `os_sandbox.__enter__()`
succeeds. Wrap the `os_sandbox.__enter__()` and `OpenShellSandbox(...)`
construction together in a try/except, and on any exception call
`os_sandbox.__exit__()` before re-raising so the context is always cleaned up.
Use the existing `_create_session`, `os_sandbox`, and `OpenShellSandbox` symbols
to update the failure path without changing the successful flow.
🪄 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: b3539c2b-02bb-48c8-8d7b-be283224f83a
📒 Files selected for processing (4)
src/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
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/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py
**/*.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/providers/openshell.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.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/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Use this skill only for research-shaped requests that ask for deep research, AI-Q research, research, or asking AI-Q to answer a question.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests; those belong to `aiq-deploy`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Resolve the target backend URL before sending any research request.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Run `health` before sending research requests, and if no backend is reachable, ask for a backend URL or hand off to `aiq-deploy`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Before sending any user query, state the exact AI-Q backend URL that will receive it, and only continue for non-local URLs after explicit user trust confirmation.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Do not send credentials, cookies, bearer tokens, or other secret values through the query text.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: If AI-Q returns `deep_research_running`, extract the `job_id` and poll asynchronously until the final report is available.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Present returned reports with citations and source URLs intact; do not truncate them.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Stop on failed jobs and show the returned error; do not retry automatically.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Use `AIQ_SERVER_URL` when set; otherwise default to the local backend at `http://localhost:8000`.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: Use Python 3.11+ and the `scripts/aiq.py` helper script to talk to the AI-Q backend.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T17:05:05.000Z
Learning: If `health` succeeds but `/chat` or async job routes fail, report that the backend is reachable but incompatible with the public research flow.
🔇 Additional comments (4)
tests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py (1)
104-105: LGTM!Also applies to: 120-130, 133-142, 145-153
src/aiq_agent/agents/deep_researcher/sandbox/README.md (1)
144-150: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py (1)
153-169: LGTM!Also applies to: 171-187, 198-214, 262-273
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py (1)
363-368: LGTM!
|
OpenShell is not truly job-scoped in the provided config. The code normalizes a job-scoped name, but then attaches to the configured providers.openshell.sandbox_name when present, and the example uses one fixed aiq-openshell-demo sandbox with delete_on_exit: false |
|
OpenShell declares network/filesystem/process capabilities, but the provider does not verify that the named sandbox actually has the configured network mode or policy applied. Can we include a fail-closed attestation step before returning the OpenShell backend. |
|
Artifact storage is okay for this PR, but we need a production scaled version. Production design should split metadata in SQL from bytes in object storage. |
Add the image-processing reference skill under the deep researcher's research-sandbox skill set, covering SKILLS-REF-1's third named category (data analysis and chart generation are covered by data-table-analysis and, in PR NVIDIA-AI-Blueprints#280, chart-generation). The skill teaches the agent to inspect and transform images with Python/Pillow in the job-scoped sandbox (metadata, format conversion, resize/thumbnail, crop, rotate, grayscale, basic color/brightness), mirroring the existing research-sandbox skills: process in /workspace, then RETURN text-survivable results (JSON/Markdown metadata, optional small base64 thumbnail) in the researcher's ResearchNotes, which run_research_batch persists to /shared. Per research.py the researcher must not call write_file/edit_file, so the skill uses the return-in-ResearchNotes pattern, not write_file. No durable binary-artifact capture on develop, so outputs are text. Update test_deepagents_runtime.py to include image-processing in the scanner and state-file assertions. Note: built on develop independent of PR NVIDIA-AI-Blueprints#280 (provider-neutral sandbox + artifact runtime). It will need reconciliation when NVIDIA-AI-Blueprints#280 lands — the flat skills layout, the artifact:// output mechanism, and the shared scanner test all change there. Validation: uv run pytest tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py (23 passed); ruff check/format on the changed test pass. Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
…ile (#286) The researcher worker is instructed (research.py format_research_request): "Do not call write_file or edit_file; run_research_batch will persist the returned ResearchNotes under /shared/." But the research skills told the agent to persist artifacts to /shared via write_file -- a contradicted instruction, since persistence is automatic from the returned ResearchNotes. Reword the persistence step in data-table-analysis, lightweight-calculation, and forecast-analysis to: include the result in your returned ResearchNotes (e.g. a ResearchFinding's evidence / narrative_notes); do not call write_file/edit_file. The separate, correct rule that sandbox code uses /workspace and cannot touch /shared is unchanged. (image-processing carries the same fix in PR #285; chart-generation in PR #280 needs it too -- flagged to its author.) Validation: uv run pytest tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py (18 passed); grep confirms write_file now appears only as "do not call write_file". Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
7d23cb0 to
b6b8436
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (2)
src/aiq_agent/agents/deep_researcher/sandbox/README.md (1)
257-259: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the missing blank line after the test fence.
This section still violates the surrounding-fence spacing rule, so markdownlint will keep failing until there is one blank line after the closing fence.
🧰 Suggested fix
```bash pytest tests/aiq_agent/agents/deep_researcher/sandbox/ -q
All provider/artifact tests run without a live Modal/OpenShell gateway (OpenShell
compliance auto-skips when the SDK is absent).</details> <details> <summary>🤖 Prompt for AI Agents</summary>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/sandbox/README.mdaround lines 257 -
259, Add the missing blank line after the closing code fence in the sandbox
README test section so the surrounding-fence spacing rule is satisfied. Update
the markdown around the pytest example to keep the closing fence followed by
exactly one blank line before the explanatory text, matching the existing
spacing pattern used elsewhere in the document.</details> <!-- cr-comment:v1:aad48bf6ac2819ebffdafeef --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>docs/source/architecture/agents/sandbox.md (1)</summary><blockquote> `8-11`: _📐 Maintainability & Code Quality_ | _🟡 Minor_ | _⚡ Quick win_ **Keep the sandbox-name contract provider-neutral.** This still says the sandbox name *is* the resolved job ID, and it hard-codes Modal-specific length/character limits in the general behavior list. Please soften this to “derived from the resolved job ID” and move the Modal-only constraints into the Modal section. As per path instructions, review documentation for accuracy and stale examples. <details> <summary>🛠️ Suggested fix</summary> ```diff - One sandbox name is used per deep research job when sandboxing is enabled; the - name is the resolved job ID, and different jobs produce different names. + One sandbox name is used per deep research job when sandboxing is enabled; the + name is derived from the resolved job ID, and different jobs produce different names. ... - Job IDs must satisfy each provider's object-name rules (Modal: 64 chars or fewer, - alphanumeric plus dash/period/underscore). + Providers may impose object-name rules; document Modal's limits in the Modal-specific section.Also applies to: 24-31
🤖 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/architecture/agents/sandbox.md` around lines 8 - 11, The sandbox naming description in the agent sandbox docs is still too Modal-specific: update the general behavior in the sandbox document and any related `execute`/sandbox sections to say the sandbox name is derived from the resolved job ID, not that it is exactly the job ID. Move the Modal-only name length/character restrictions out of the provider-neutral behavior list and into the Modal-specific section, and keep the provider-neutral wording aligned with `execute` and sandbox lifecycle docs.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 `@configs/openshell/aiq-research-policy.yaml`:
- Around line 24-25: The default OpenShell policy is using
landlock.compatibility as best_effort, which can silently disable filesystem
confinement when Landlock is unavailable. Update the OpenShell policy in
aiq-research-policy and the setup flow in scripts/setup_openshell.sh to use
hard_requirement for Landlock, or otherwise clearly mark this policy as
non-production/local-only. Keep the change anchored around the landlock
configuration so the default behavior preserves confinement.
In `@frontends/aiq_api/src/aiq_api/jobs/callbacks.py`:
- Around line 358-361: The sandbox path check in the callback helper is too
broad because `path.startswith(self.SHARED_FS_PREFIX)` can incorrectly classify
sibling paths like `/shared_data` as shared. Update the path validation in the
`SANDBOX_FILE_TOOLS` check to anchor the shared prefix on a path boundary in the
same `callbacks.py` helper, so only true descendants of the shared filesystem
prefix are excluded while genuine sandbox paths are preserved.
In `@frontends/ui/src/pages/api/generate-pdf.ts`:
- Around line 101-109: The auth forwarding in generate-pdf should be gated by
isAuthRequired() so no identity headers are sent when REQUIRE_AUTH is false.
Update the authHeaders construction in generate-pdf.ts to only include
req.headers.authorization and the idToken cookie when isAuthRequired() returns
true, while keeping the existing forwarding behavior unchanged for authenticated
mode. Use the existing isAuthRequired() check near the auth header assembly to
locate the fix.
In `@skills/aiq-research/scripts/aiq.py`:
- Around line 524-531: The artifact download loop in aiq.py should skip entries
with missing artifact_id just like the bundle export path does. In the
artifact-processing loop that calls download_artifact and _unique_filename, add
an early continue when artifact_id is empty so a bad listing entry does not
trigger a RuntimeError and abort the whole download flow.
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 268-281: Artifact post-processing in deep_researcher.agent should
be made non-fatal so a valid final_message is not lost if artifact handling
fails. Wrap the manager.final_harvest, manager.store.list, and the subsequent
resolve_report_references, ensure_inline_artifacts_embedded, and
append_artifact_index calls in their own try/except inside the artifact_manager
block, or call the safer DeepAgentsRuntime.final_harvest path, so any
artifact-side exception is logged and the original report is still returned.
Keep the existing final_message flow intact and avoid letting produced-related
failures propagate to the outer handler.
In `@src/aiq_agent/agents/deep_researcher/custom_middleware.py`:
- Around line 482-491: The awrap_model_call() hook in custom_middleware still
invokes _persist_plan() synchronously, which can block the event loop during
planner turns. Update awrap_model_call() to offload the existing
_persist_plan(plan) work to a worker thread (while keeping the current exception
handling/logging), and leave the synchronous after_agent() path unchanged.
In `@src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2`:
- Around line 110-111: The chart guidance in orchestrator.j2 is currently gated
on skills_enabled, but it should follow execution availability just like
writer.j2 and chart generation via execute. Update the relevant prompt lines to
use execution_enabled for the figure/embed instructions so the report prompt
only asks for artifact:// chart embeds when the sandbox is available, and keep
the source-anchored completeness check under the same gate.
In `@src/aiq_agent/agents/deep_researcher/sandbox/capabilities.py`:
- Around line 39-41: Update the docstring in capabilities.py so it no longer
says an unknown provider is assumed to support artifact download; the current
fail-closed default in supports_artifact_download is False. Revise the prose
around the capability gate to match the conservative behavior and avoid implying
any default support, using the existing supports_artifact_download logic as the
reference point.
In `@src/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.py`:
- Around line 147-149: The OpenShell sandbox selection is using a fixed
configured name instead of a per-job scoped name, which can cause all jobs to
share the same sandbox. Update the name resolution in the OpenShell provider so
that `_scoped_name(job_id)` is actually used when building the sandbox name, and
adjust the sandbox-name config path to require a job-id-based template or prefix
rather than accepting a static reusable name. Make sure the change is applied
consistently in the OpenShell provider methods that resolve and attach the
sandbox, including the `providers.openshell.sandbox_name` handling and
`_normalize_openshell_name`/`_scoped_name` flow.
- Around line 75-83: The _DOWNLOAD_CODE sandbox check only rejects a symlink at
the final path component, so update the path validation in openshell to reject
symlinks in any part of the artifact path. Use the existing _DOWNLOAD_CODE flow
to validate each parent component before opening the file, and ensure the
resolved path stays within the intended artifact tree rather than following a
symlinked directory outside it.
- Around line 151-160: Fail closed in OpenShell session setup when the attached
sandbox does not match the expected policy. In `_create_session()` and the
`OpenShellSandbox.__enter__()` flow, validate the runtime sandbox metadata
(policy, labels, and network mode) against the `providers.openshell`
configuration instead of only checking `sandbox_name` and logging
`oscfg.policy`. If the pre-created sandbox’s actual settings differ from the
requested `SandboxCapabilities`/policy, raise an error and refuse to continue.
In `@src/aiq_agent/common/citation_verification.py`:
- Line 1039: The markdown artifact reference handling is stripping the leading !
from image syntax because _MD_LINK_RE only matches the link body and the later
replacement returns match.group(0). Update the regex and/or the replacement
logic in citation_verification.py so the code that processes artifact://
markdown preserves the full original token for  while
still handling normal links correctly, using the existing _MD_LINK_RE and the
related render/rewrite logic around the artifact citation handling.
In `@tests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py`:
- Around line 34-38: The test module currently skips the entire file via
importorskip for openshell and langchain_nvidia_openshell, which prevents
fake-backed unit coverage from running in default CI. Replace those skips in
test_openshell_provider.py with stubbed or monkeypatched optional modules so the
existing SandboxConfig and OpenShellSandboxProvider tests still execute even
when the SDKs are absent, preserving the argv/stdin shim coverage.
---
Duplicate comments:
In `@docs/source/architecture/agents/sandbox.md`:
- Around line 8-11: The sandbox naming description in the agent sandbox docs is
still too Modal-specific: update the general behavior in the sandbox document
and any related `execute`/sandbox sections to say the sandbox name is derived
from the resolved job ID, not that it is exactly the job ID. Move the Modal-only
name length/character restrictions out of the provider-neutral behavior list and
into the Modal-specific section, and keep the provider-neutral wording aligned
with `execute` and sandbox lifecycle docs.
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Around line 257-259: Add the missing blank line after the closing code fence
in the sandbox README test section so the surrounding-fence spacing rule is
satisfied. Update the markdown around the pytest example to keep the closing
fence followed by exactly one blank line before the explanatory text, matching
the existing spacing pattern used elsewhere in the document.
🪄 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: 9f4e2029-c2a3-4632-bddb-fb1855a281d1
📒 Files selected for processing (67)
.gitignoreconfigs/config_openshell.ymlconfigs/openshell/Dockerfile.aiq-democonfigs/openshell/aiq-research-policy.yamldocs/source/architecture/agents/sandbox.mdfrontends/aiq_api/src/aiq_api/jobs/access.pyfrontends/aiq_api/src/aiq_api/jobs/callbacks.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pyfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.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/AgentCard.tsxfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/features/layout/components/ToolCallCard.tsxfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/shared/components/MarkdownRenderer/types.tsscripts/README.mdscripts/setup_openshell.shskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pysrc/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/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/prompts/planner.j2src/aiq_agent/agents/deep_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/prompts/writer.j2src/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/skills/research/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/skills/research/data-table-analysis/SKILL.mdsrc/aiq_agent/common/citation_verification.pytests/aiq_agent/agents/deep_researcher/sandbox/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pytests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.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/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.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/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pytests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pyfrontends/aiq_api/src/aiq_api/jobs/access.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_agent.pysrc/aiq_agent/common/citation_verification.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pyfrontends/aiq_api/src/aiq_api/jobs/callbacks.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pyskills/aiq-research/scripts/aiq.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.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/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/__init__.pyfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxconfigs/openshell/aiq-research-policy.yamlfrontends/ui/src/shared/components/MarkdownRenderer/types.tsconfigs/openshell/Dockerfile.aiq-demofrontends/ui/src/features/layout/components/AgentCard.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pyfrontends/ui/src/features/chat/types.tsfrontends/ui/src/hooks/use-download-pdf.tssrc/aiq_agent/agents/deep_researcher/prompts/researcher.j2tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pyfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsscripts/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pyfrontends/ui/src/features/layout/components/ToolCallCard.tsxsrc/aiq_agent/agents/deep_researcher/prompts/writer.j2frontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tssrc/aiq_agent/agents/deep_researcher/prompts/planner.j2frontends/ui/src/features/layout/components/ExportFooter.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pyconfigs/config_openshell.ymlfrontends/ui/src/features/chat/hooks/use-load-job-data.tsdocs/source/architecture/agents/sandbox.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pyfrontends/ui/src/adapters/api/deep-research-client.tstests/aiq_agent/agents/deep_researcher/test_custom_middleware.pyfrontends/aiq_api/src/aiq_api/jobs/access.pyfrontends/ui/src/pages/api/generate-pdf.tssrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/skills/research/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/agent.pyfrontends/ui/src/features/layout/components/ReportTab.tsxsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pyskills/aiq-research/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_agent.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/deep_researcher/skills/research/data-table-analysis/SKILL.mdfrontends/aiq_api/tests/test_sandbox_concurrency.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pyfrontends/aiq_api/src/aiq_api/jobs/callbacks.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pyfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxtests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.pyfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsxfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pyskills/aiq-research/scripts/aiq.pyscripts/setup_openshell.shsrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.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/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.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/providers/__init__.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.pysrc/aiq_agent/agents/deep_researcher/prompts/researcher.j2src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manifest.pysrc/aiq_agent/agents/deep_researcher/prompts/writer.j2src/aiq_agent/agents/deep_researcher/prompts/planner.j2src/aiq_agent/agents/deep_researcher/sandbox/__init__.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2src/aiq_agent/agents/deep_researcher/skills/research/chart-generation/SKILL.mdsrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/modal.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/skills/research/data-table-analysis/SKILL.mdsrc/aiq_agent/agents/deep_researcher/sandbox/capabilities.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/registry.pysrc/aiq_agent/agents/deep_researcher/sandbox/config.pysrc/aiq_agent/agents/deep_researcher/sandbox/providers/openshell.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/layout/components/AgentCard.tsxfrontends/ui/src/features/chat/types.tsfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/layout/components/ToolCallCard.tsxfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/layout/components/AgentCard.tsxfrontends/ui/src/features/chat/types.tsfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/layout/components/ToolCallCard.tsxfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.spec.tsxfrontends/ui/src/shared/components/MarkdownRenderer/types.tsfrontends/ui/src/features/layout/components/AgentCard.tsxfrontends/ui/src/features/chat/types.tsfrontends/ui/src/hooks/use-download-pdf.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/app/api/jobs/async/[...path]/route.tsfrontends/ui/src/features/layout/components/ToolCallCard.tsxfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.spec.tsfrontends/ui/src/features/layout/components/ExportFooter.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/layout/components/ReportTab.tsxfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/artifact-url.tsfrontends/ui/src/lib/pdf/ReactPdfDocument.tsx
{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/openshell/aiq-research-policy.yamlconfigs/openshell/Dockerfile.aiq-democonfigs/config_openshell.yml
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_provider_compliance.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_agent.pyfrontends/aiq_api/tests/test_sandbox_concurrency.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.pytests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.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
{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/access.pyfrontends/aiq_api/src/aiq_api/jobs/callbacks.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/runner.py
skills/aiq-research/**
⚙️ CodeRabbit configuration file
skills/aiq-research/**: ---
name: aiq-research
description: |
Use when asked to run deep research or AI-Q research through a reachable NVIDIA AI-Q Blueprint backend.
license: Apache-2.0
permissions:
env:
- AIQ_SERVER_URL
network:
- http://localhost:8000
compatibility: |
Designed for Claude Code, OpenCode, Codex, and Agent Skills-compatible tools. Requires Python 3.11+ and network
access to a running local AI-Q Blueprint server athttp://localhost:8000by default. Non-local backends must be
explicitly trusted by the user and granted by the host tool outside this public skill.
metadata:
version: "2.1.0"
author: "NVIDIA AI-Q Blueprint Team aiq-blueprint@nvidia.com"
github-url: "https://github.com/NVIDIA-AI-Blueprints/aiq"
tags:
- nvidia
- aiq
- blueprint
- deep-research
- research-agents
- agent-skills
languages:
- python
- bash
domain: "research-agents"
allowed-tools: Read BashAIQ Research Skill
Purpose
Use this skill to call a locally running NVIDIA AI-Q Blueprint server through the helper script at
scripts/aiq.py.Use this skill for research-shaped requests, including:
- "deep research on ..."
- "AIQ research ..."
- "research ..."
- "use AI-Q to answer ..."
- "ask AI-Q about ..."
Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests. Those
belong toaiq-deploy.Prerequisites
Users need:
- Python 3.11+ available as
python3.- A reachable local or self-hosted AI-Q Blueprint backend.
AIQ_SERVER_URLset when the backend is not running athttp://localhost:8000; non-local values must be trusted by
the user before any query is sent.- A backend configured with authentication disabled for this public helper, or a separate authenticated AI-Q skill for
authenticated environments.- Network access from the local machine to the AI-Q backend URL.
- Credentials configured in the backend environment, not in this skill. Thi...
Files:
skills/aiq-research/SKILL.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/SKILL.mdskills/aiq-research/scripts/aiq.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:
src/aiq_agent/agents/deep_researcher/sandbox/config.py
skills/aiq-research/scripts/aiq.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/scripts/aiq.py: Implement the helper script commands for health, chat, agents, submit, research, research_poll, status, state, report, artifacts, stream, and cancel as documented.
Use the configured AIQ_SERVER_URL, defaulting to http://localhost:8000 when unset, and support the documented command-line arguments and output behaviors.
Files:
skills/aiq-research/scripts/aiq.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Use this skill only for research-shaped requests routed through a reachable NVIDIA AI-Q Blueprint backend; do not use it for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Before sending any user query, resolve the backend URL, run a health check, and if no backend is reachable, ask for a backend URL or hand off to aiq-deploy.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Before sending a request, state the exact AI-Q backend URL and only continue for non-local URLs after the user has explicitly confirmed the endpoint is trusted.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Do not send credentials, cookies, bearer tokens, or other secret values in query text.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: If the backend returns 401 or 403, stop and explain that this public skill does not manage authentication; ask the user to use an authenticated AI-Q skill or configure authentication externally.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: If health succeeds but /chat or async research endpoints fail, report that the backend is reachable but incompatible with the public research flow and offer aiq-deploy validation.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: If AI-Q returns a deep_research_running job ID, poll the job asynchronously until completion; do not force polling when no job ID is returned.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: If deep research is running, tell the user it is running in the background and use the runtime's non-blocking or background execution mechanism when available.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: If polling is interrupted, resume using the existing job ID with status, report, or research_poll; jobs continue server-side.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Present returned reports with citations and source URLs intact; do not truncate them.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Stop on failed, failed/failure/cancelled jobs and show the returned error; do not retry automatically.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: For follow-up questions, answer directly from the existing report when possible; otherwise, send a fresh research request that includes the needed prior context.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: For redo requests, treat them as new jobs, restate the target endpoint before sending, and then poll and present the result again.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Treat the skill as compatible only when the Blueprint major version matches and the Blueprint minor version is greater than or equal to the skill's minor version.
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-25T22:47:00.615Z
Learning: Do not store or embed API keys, bearer tokens, cookies, or basic-auth credentials in AIQ_SERVER_URL or in the skill itself; credentials belong in the deployment environment.
📚 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_sandbox_concurrency.py
🪛 ast-grep (0.44.0)
src/aiq_agent/agents/deep_researcher/custom_middleware.py
[info] 475-475: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload, indent=2, ensure_ascii=False)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
[info] 54-54: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"version": 1, "artifacts": [{"path": path, "kind": kind, "inline": True}]})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 148-150: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{"version": 1, "artifacts": [{"path": a, "kind": "image"}, {"path": b, "kind": "image"}]}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
skills/aiq-research/scripts/aiq.py
[warning] 249-249: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req, timeout=timeout)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[warning] 476-476: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(dest, "wb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 486-486: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(report_path, "w", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 528-528: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(dest, "wb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[info] 488-488: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"job_id": job_id, "report": report_path, "artifacts": saved}, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 500-500: use jsonify instead of json.dumps for JSON output
Context: json.dumps(get_report(job_id), indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 511-511: use jsonify instead of json.dumps for JSON output
Context: json.dumps(listing, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 531-531: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"job_id": job_id, "downloaded": saved}, indent=JSON_INDENT_SPACES)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
[warning] 194-200: Do not use text() as it leads to SQL injection
Context: text(
"INSERT INTO artifacts (artifact_id, job_id, kind, mime_type, filename, sandbox_path, "
"storage_uri, sha256, size_bytes, title, caption, inline, workflow, source_tool_call_id, "
"provenance, status, content) VALUES (:artifact_id, :job_id, :kind, :mime_type, :filename, "
":sandbox_path, :storage_uri, :sha256, :size_bytes, :title, :caption, :inline, :workflow, "
":source_tool_call_id, :provenance, :status, :content)"
)
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 229-229: Do not use text() as it leads to SQL injection
Context: text("SELECT content FROM artifacts WHERE job_id = :job_id AND artifact_id = :artifact_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 243-243: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE job_id = :job_id AND artifact_id = :artifact_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 253-253: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE job_id = :job_id AND sha256 = :sha256 LIMIT 1")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 263-263: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE job_id = :job_id ORDER BY created_at")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 272-272: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE job_id = :job_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 287-287: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE created_at < NOW() - :seconds * INTERVAL '1 second'")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
[warning] 292-292: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE created_at < datetime('now', :interval)")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(disable-sqlalchemy-text)
🪛 Checkov (3.3.1)
configs/openshell/Dockerfile.aiq-demo
[low] 1-25: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
🪛 Hadolint (2.14.0)
configs/openshell/Dockerfile.aiq-demo
[warning] 8-8: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
[warning] 8-8: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>
(DL3013)
🪛 LanguageTool
scripts/README.md
[style] ~186-~186: Consider a different adjective to strengthen your wording.
Context: ...Index | | configs/config_skills.yml | Deep research with DeepAgents skills + Modal...
(DEEP_PROFOUND)
[style] ~187-~187: Consider a different adjective to strengthen your wording.
Context: ...ox | | configs/config_openshell.yml | Deep research with skills + OpenShell sandbo...
(DEEP_PROFOUND)
docs/source/architecture/agents/sandbox.md
[grammar] ~55-~55: Ensure spelling is correct
Context: ...tructured lifecycle logging for sandbox create, reuse, failure, and cleanup.
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
src/aiq_agent/agents/deep_researcher/skills/research/chart-generation/SKILL.md
[style] ~42-~42: ‘a majority of’ might be wordy. Consider a shorter alternative.
Context: ...harts:** if a series is mostly missing (a majority of periods undisclosed) or mixes metric...
(EN_WORDINESS_PREMIUM_A_MAJORITY_OF)
🪛 markdownlint-cli2 (0.22.1)
src/aiq_agent/agents/deep_researcher/sandbox/README.md
[warning] 259-259: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 OpenGrep (1.23.0)
tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py
[ERROR] 276-276: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 299-299: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 309-309: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 318-318: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 325-325: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 333-333: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 341-341: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 369-369: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 417-417: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 432-432: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
frontends/ui/src/lib/pdf/ReactPdfDocument.tsx
[ERROR] 233-233: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py
[ERROR] 341-341: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 Trivy (0.69.3)
configs/openshell/Dockerfile.aiq-demo
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
…edback Job-scope the artifact directory (<artifact_dir>/<job_id>) so a persistent or shared sandbox cannot leak one job's files into another job's harvest, store, or content endpoint, and concurrent jobs no longer collide. Address review findings without changing behavior elsewhere: - routes/jobs: stop exposing storage_uri; sanitize Content-Disposition filename and add nosniff + attachment for non-raster artifacts; clamp negative caps. - store: keep db_url out of storage_uri and logs. - manager: shlex-quote the scan path; drop placeholder provenance; skip quota and SSE on dedup hits. - base: close the stale session on reset. - capabilities: default supports_artifact_download to False (fail-closed). - config: robust legacy block_network parsing. - openshell: parse AIQ_OPENSHELL_ADAPTER_FILE_TRANSFER as an explicit boolean; map download exit code 3 to is_directory. - citation_verification: preserve a link only when its destination is artifact://. - UI: PDF fetch timeout + Content-Length pre-check, scoped cookie forwarding, gap/MIME-gated image embedding; proxy preserves upstream status. Make the chart and data-table skills provider-neutral (reference the runtime sandbox dir instead of a hardcoded /workspace), plus README/markdownlint cleanups. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
…, honest visualization Security (fail-closed): - deepagents_runtime: sanitize job_id before using it as the artifact-dir path segment so a crafted id (/, ..) cannot escape the configured artifact base. - config: reject unrecognized legacy block_network strings instead of mapping them to falsy, so a typo cannot silently open egress on a network-blocked sandbox. - manager: reject SVG at harvest (the regex strip cannot fully neutralize it and the content endpoint serves stored bytes), closing the stored-XSS vector fail-closed. Lifecycle (cancellation can preempt a running sandbox): - base: split the single lock into an operation lock (serializes calls/retry, unchanged) and a short state lock guarding the session ref + a terminated flag. close()/terminate() now tear down out-of-band so a cancelled/timed-out job interrupts an in-flight execute instead of waiting for it; a terminated provider refuses further work. Lock order is operation -> state only, so no deadlock. - deepagents_runtime.terminate() + runner: interrupted jobs (cancel/timeout) call terminate(); normal paths still close() gracefully. Visualization honesty (no new loops/cost): chart/data-table skills + orchestrator/ researcher prompts now require source-anchored, sufficiently complete data and suppress a chart (present the gap-marked table instead) when a series is mostly undisclosed or mixes metric definitions. Tests: terminate preempts an in-flight execute and blocks reuse; block_network typo is rejected; SVG is rejected; dedup test now proves digest-keyed (not id-keyed) dedup. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
- routes/jobs: encode the artifact Content-Disposition filename with an ASCII fallback plus RFC 5987 filename*=UTF-8'' so a non-Latin-1 filename (emoji, CJK) no longer raises UnicodeEncodeError when Starlette writes the Latin-1 header. - ReactPdfDocument: drop WebP from the embeddable data-URI pattern; @react-pdf/ renderer supports only PNG/JPEG, so a WebP data URI passed the size guard and then failed silently during PDF rendering. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
…act-runtime # Conflicts: # src/aiq_agent/agents/deep_researcher/custom_middleware.py # src/aiq_agent/agents/deep_researcher/factory.py # tests/aiq_agent/agents/deep_researcher/test_custom_middleware.py Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
… verification The writer subagent sometimes emits "[N] Title" source lines without the verified URL, causing verify_citations to strip every such line as unverifiable and drop all web sources from the report. Recover the dropped target from the writer-facing source list (reference_sources) via an exact-title, unique match against the same captured registry, then rewrite the line to canonical "[N] Title: url" form. Precision is unchanged: titles with no registry match, ambiguous titles, and aggregate labels still strip exactly as before. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Add an optional RUST_LOG build arg (default warn) to the sandbox image and a --sandbox-log-level flag to setup_openshell.sh so operators can surface container-side command execution for debugging without changing the default quiet behavior. Document the opt-in flow in the sandbox README. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
…de cleanup Reviewer feedback and security hardening for the sandbox/artifact runtime, keeping OpenShell scoped as an experimental provider. Cleanup (review): - Relocate sandbox Dockerfile from configs/ to deploy/openshell/ (deploy convention) - Remove never-wired ArtifactHarvestMiddleware and dead execute_end/collect_on path - Remove dead SandboxConfig.artifact_dir (real dir is job-scoped in the provider) - Remove unreachable Modal legacy backend from deepagents_runtime; dispatch only through the provider registry (Modal lives in sandbox/providers/modal.py) Hardening: - Confine OpenShell artifact downloads to the job-scoped artifact_dir, including when the adapter-transfer toggle is enabled (prevents cross-job symlink reads) - Forced termination now exits the OpenShell SDK context exactly once - Mark OpenShell experimental; document shared-sandbox, policy-verification, cancellation, and artifact-lifecycle limitations Adds regression tests for download confinement, toggle-bypass prevention, and termination. Remaining files reformatted to satisfy the Ruff format gate. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/ok to test 5c3243a |
cdgamarose-nv
left a comment
cdgamarose-nv
left a comment
There was a problem hiding this comment.
Minor comment, otherwise looks good!
Great job on this feature!
|
/ok to test 1e21217 |
…to PyPI CodeRabbit fixes: - store.py: artifacts.created_at uses DateTime(timezone=True) so tz-aware timestamps work on PostgreSQL, not just SQLite. - base.py: re-check the terminated flag after a successful sandbox call so a concurrent terminate() that lands during a winning call surfaces cancellation. - MarkdownRenderer.spec: assert the caption renders in a <span> (phrasing-safe), matching the renderer. - generate-pdf.ts: forward Authorization/idToken only when isAuthRequired(), matching the jobs proxy (no identity headers in anonymous mode). - Document the landlock best_effort tradeoff (drops FS confinement on hosts without Landlock; production must use hard_requirement) in the policy files and sandbox README. Reliability: - Add a guidance nudge to the writer/researcher prompts: each execute runs in a fresh shell, so cd does not persist; use absolute paths. Stops the no-op cd-only execute loop that burned tokens. OpenShell adapter to PyPI: - setup_openshell.sh installs langchain-nvidia-openshell==0.1.0 from PyPI instead of the git fork; bump default/min OpenShell SDK to 0.0.72 (adapter's tested floor); update usage text and the provider import hint and docs. CI: - Patch _create_sandbox_backend in test_init_with_custom_settings and test_require_sandbox_collection_with_sandbox_is_allowed so they no longer require the optional OpenShell adapter (default provider) to be installed. Deferred to follow-up (documented experimental): physical per-job OpenShell isolation and attach-time policy verification. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/ok to test 5c69dc2 |
…archJobId Both specs mock '@/features/chat' but had not exposed selectResolvedDeepResearchJobId, which ExportFooter and ReportTab import — vitest requires every used export to be defined on the mock, so all 16 tests errored at render. Add the selector to each mock and assert the resolved job id is forwarded to downloadPdf. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/ok to test 0e010c7 |
5 participants
Overview
This PR introduces a provider-neutral, fail-closed sandbox runtime for executing agent-generated code securely, alongside a durable artifact runtime that harvests generated files (such as charts and CSVs). These captured artifacts are now seamlessly rendered inline across all system surfaces, including the Web UI, PDF exports, Markdown reports, and the CLI.
Proposed Changes
Sandbox Runtime (src/aiq_agent/agents/deep_researcher/sandbox/)
Artifact Manager (sandbox/artifacts/)
Report Post-Processing
Backend Core (src/aiq_agent/agents/deep_researcher/)
Backend API (frontends/aiq_api)
Frontend & UI UI
Skill & CLI Interface
OpenShell Setup
Documentation
Verification & Testing
Bash
git commit -sor an equivalent sign-off.Where should reviewers start?
sandbox/base.py+sandbox/registry.py(the provider contract), thensandbox/artifacts/manager.py(the harvest/validation pipeline) and agent.py::run() (post-processing order), then frontends/ui/.../MarkdownRenderer/artifact-url.ts + the img/urlTransform for the rendering edge.Related Issues
Dependencies / Install note
The OpenShell path depends on the langchain-nvidia-openshell adapter with the argv file-transfer fix (pastorsj/langchain-nvidia#1, which targets Sam's adapter PR langchain-ai/langchain-nvidia#303).
Until Add a customizable frontend #1 is merged into refactor(clarifier): remove plan approval; keep context + output-type clarification #303 (and refactor(clarifier): remove plan approval; keep context + output-type clarification #303 published to PyPI), the adapter must be installed from the fork branch — not PyPI.
Install via the git spec (or LANGCHAIN_NVIDIA_REPO override that scripts/setup_openshell.sh honors):
Without that fix, OpenShell upload_files/download_files fail (the gateway strips OPENSHELL_* env), surfaced as a misleading permission_denied. Modal is unaffected.
Action items to defer in follow up PR
Summary by CodeRabbit