Artifacts: persistent agent-maintained documents in Monitor - #479
Conversation
…ache detail/history reads via useQuery
|
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 full artifact management feature: DB schema and store helpers, Flask REST blueprint and agent tools, MCP/registry/dispatch wiring, Next.js proxy routes, a TypeScript client service, and a React UI tab with list/create/detail/history views. ChangesArtifact Management System
sequenceDiagram
participant Client
participant NextAPI as Next.js API
participant ArtifactService as Flask artifact_bp
participant Postgres as Postgres DB
Client->>NextAPI: HTTP request (GET/POST/PATCH/DELETE /api/artifacts...)
NextAPI->>ArtifactService: forwardRequest to /api/artifacts[...]
ArtifactService->>Postgres: SELECT / INSERT / UPDATE / create_version / restore
Postgres-->>ArtifactService: rows / new version id
ArtifactService-->>NextAPI: JSON response
NextAPI-->>Client: proxied HTTP response
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@client/src/app/monitor/components/artifacts-tab.tsx`:
- Around line 54-56: The artifacts fetch currently defaults to an empty array
when the query fails which makes the UI show "No artifacts yet." and breaks
overwrite checks; update the useQuery handling around the call to useQuery<{
artifacts: ArtifactSummary[] }> (the destructured data, isLoading, mutate) and
any downstream usage of data.artifacts/existingTitles so that: 1) you detect and
handle query errors (use isError or error from useQuery) and render an explicit
error state instead of treating missing data as an empty list; 2) do not default
artifacts to [] for UI or for create/overwrite guards until the query is
successful (use a guard like if (!data || isError) before deriving
existingTitles); and 3) keep the loading behavior via isLoading; update the
render branches around the artifacts list (and the create flow that references
existingTitles) to disable or show an error when the query failed so overwrite
checks remain correct.
In `@client/src/lib/services/artifacts.ts`:
- Around line 24-31: Remove the userId field from the ArtifactVersion interface
in client/src/lib/services/artifacts.ts (the ArtifactVersion type) and update
all client-side usages that reference ArtifactVersion.userId (including
functions, API response handling, mappers, and tests) to stop expecting or
accessing user identity; adjust any deserialization or type assertions that
assumed a userId and rely on server-side identity resolution instead, and
run/typecheck to fix any compilation errors by removing or replacing references
to userId with server-provided metadata fields if needed.
- Around line 52-60: The current getArtifact function (and the similar function
at lines 96-105) swallows all non-404 errors and returns null/empty data, which
hides real API failures; change the catch blocks to only return null when the
caught error is an ApiError with status 404, and rethrow or propagate other
errors (i.e., throw error) so callers/UI can surface real API errors; locate the
catch in getArtifact (uses apiGet and ApiError) and the analogous catch in the
other artifact/history function and replace the unconditional console.error +
return null with conditional logic that returns null for 404 and throws the
error for any other status.
In `@server/chat/backend/agent/tools/artifact_tool.py`:
- Around line 38-77: list_artifacts currently opens an admin DB connection
before verifying the caller has artifact read access; add an upfront RBAC check
(before calling db_pool.get_admin_connection()) to verify the user has the
artifacts.read permission and return an error JSON if not authorized. Do the
same pattern for read_artifact (check artifacts.read) and write_artifact (check
artifacts.write) entry points—call your existing permission helper (e.g.,
has_permission or check_permission) with user_id and the appropriate permission
string, and bail early if the check fails so no admin DB connection or
set_rls_context is performed.
In `@server/routes/artifact_routes.py`:
- Around line 129-141: The SQL query in cursor.execute is selecting a.content
even though the endpoint calls _serialize_artifact with include_content=False,
causing unnecessary I/O and memory usage; update the SELECT to omit a.content
(or replace it with NULL/'' as a placeholder) so rows do not include full
document content, then ensure _serialize_artifact is still called with
include_content=False (same call site) to keep list responses lightweight.
In `@server/services/artifacts/store.py`:
- Around line 30-37: The INSERT currently uses (SELECT
COALESCE(MAX(version_number), 0) + 1 FROM artifact_versions WHERE artifact_id =
%s) which is race-prone; wrap the allocate-and-insert in a DB transaction and
serialize concurrent allocs for the same artifact by acquiring a lock (e.g. call
SELECT pg_advisory_xact_lock(%s) with artifact_id or SELECT id FROM artifacts
WHERE id=%s FOR UPDATE) before computing MAX(version_number) and performing the
INSERT into artifact_versions, and ensure there is a unique constraint on
(artifact_id, version_number) so any rare race will surface and can be retried;
update the code that runs this SQL (the store logic around
artifact_versions/artifact_id/version_number) to begin a transaction, acquire
the lock, compute MAX+1, insert, and commit.
🪄 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: Pro
Run ID: 3fab6e51-2f9b-4987-b2ba-299a5f4b22b5
📒 Files selected for processing (18)
client/src/app/api/artifacts/[id]/route.tsclient/src/app/api/artifacts/[id]/versions/[versionId]/restore/route.tsclient/src/app/api/artifacts/[id]/versions/[versionId]/route.tsclient/src/app/api/artifacts/[id]/versions/route.tsclient/src/app/api/artifacts/route.tsclient/src/app/monitor/components/artifacts-tab.tsxclient/src/app/monitor/page.tsxclient/src/lib/services/artifacts.tsserver/aurora_mcp/dispatch.pyserver/aurora_mcp/registry.pyserver/chat/backend/agent/tools/artifact_tool.pyserver/chat/backend/agent/tools/cloud_tools.pyserver/main_compute.pyserver/routes/artifact_routes.pyserver/services/artifacts/__init__.pyserver/services/artifacts/store.pyserver/utils/auth/enforcer.pyserver/utils/db/db_utils.py
…nent, hoist duplicate literals to constants, use logging.exception
…ten list query, serialize version allocation
…ke siblings, simplify restoreVersion return type
…essed section and never re-flag listed items
…he user already steers one
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/services/actions/executor.py`:
- Around line 178-191: The current guard only checks for the literal substring
"artifact" before injecting the persistence block, which misses other
user-steering phrases; update the condition around action["instructions"] in
executor.py so it scans a small set of persistence keywords (e.g., "artifact",
"read_artifact", "write_artifact", "living document", "runbook", "persist across
runs") against action["instructions"].lower() and only injects the Maintain a
Living Document block when none of those keywords are present; keep the existing
block contents (using title = action["name"] and the
read_artifact/write_artifact guidance) and ensure the check is applied where the
parts list is appended.
🪄 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: Pro
Run ID: 20ef3383-b71d-4ec0-87b1-5e250840b4c4
📒 Files selected for processing (1)
server/services/actions/executor.py
…roaden user-steering detection beyond the bare artifact substring
… defer to a user-named doc in prose
|



Summary
Adds Artifacts — living markdown documents Aurora maintains across runs (findings lists, cost reports, runbooks), surfaced in a new Monitor → Artifacts tab. Scheduled Actions and chat create/update them through agent tools; users can view, edit, and browse version history in the UI.
Backend
artifacts+artifact_versionstables with per-org RLS policiesartifactsread/write)Frontend
useQuery— re-opening an artifact and re-expanding a version are served from cache instead of refetchingTesting
tscand ESLint clean for the changed frontendSummary by CodeRabbit
New Features
Chores