feat(web): make artifact file paths clickable in chat messages#1023
feat(web): make artifact file paths clickable in chat messages#1023
Conversation
Artifact paths in AI responses (e.g. `artifacts/runs/{uuid}/report.md`) were
rendered as plain inline code. Now they render as clickable buttons (.md opens
ArtifactViewerModal) or links (other types open in new tab).
Changes:
- Add artifact path detection via regex in MessageBubble code renderer
- Convert MARKDOWN_COMPONENTS to factory function for modal state access
- Add ArtifactViewerModal integration in chat view
- Add 'workflow_artifact' to WORKFLOW_EVENT_TYPES in store.ts
Fixes #1016
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughReplaces module-level markdown components with per-instance factories to detect artifact paths in chat inline code; Changes
Sequence DiagramsequenceDiagram
participant User
participant Msg as MessageBubble/MessageList
participant Modal as ArtifactViewerModal
participant API as /api/artifacts
User->>Msg: View message containing inline code path
Msg->>Msg: extractArtifactInfo(path) -> (runId, filename)?
alt matches artifact path
Msg->>User: render button (if .md) or external link (other)
User->>Msg: click artifact element
alt filename ends with .md
Msg->>Modal: open with runId & filename
Modal->>API: GET /api/artifacts/{runId}/{filename}
API-->>Modal: return artifact content
Modal-->>User: display rendered markdown
else other file types
User->>API: open /api/artifacts/{runId}/{encodedFilename} (new tab)
API-->>User: serve file/download
end
else no artifact match
Msg->>User: render standard inline code
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
🔍 Comprehensive PR ReviewPR: #1023 — feat(web): make artifact file paths clickable in chat messages SummaryThis PR correctly implements clickable artifact file paths in chat messages. The Verdict: ✅
🟡 Medium Issues (Needs Decision)1.
|
| # | Issue | Location | Suggestion |
|---|---|---|---|
| L1 | makeMarkdownComponents return type uses ComponentPropsWithoutRef<never> — opaque widening cast |
MessageBubble.tsx:20 |
Drop explicit return type (let TS infer) or add explanatory comment |
| L2 | ARTIFACT_PATH_RE has no comment explaining capture groups or why no g flag |
MessageBubble.tsx:16 |
Add: // Matches artifact paths (fwd- and back-slash safe); groups: [1] runId, [2] filename |
| L3 | isJsonString catch block catches all exceptions, not just SyntaxError |
MessageBubble.tsx:125-130 |
Pre-existing, intentional probe pattern — optionally narrow to catch (err: unknown) { if (err instanceof SyntaxError) return false; throw err; } |
| L4 | URL encoding logic untested | MessageBubble.tsx:70-79 |
Can be covered as part of extracting extractArtifactInfo (see MEDIUM-3) |
✅ What's Good
- Security-conscious URL construction: Per-segment
encodeURIComponenton bothrunIdand eachfilenamepath segment is exactly right — prevents path traversal in the artifact API route. - Correct modal lifecycle: Conditional mount with
open={true}resets modal fetch state on every open — matchesArtifactSummary.tsxprecedent. - Cross-platform regex: Handles both
\and/separators and normalizes to/before encoding. Correct for Windows artifact paths. workflow_artifactgap fix: Quietly closes a pre-existing type hole inWORKFLOW_EVENT_TYPES— non-breaking, correct, and already covered by existingevent-emitter.test.tsfixtures.// Hoisted to module scope to prevent new references on every render: Textbook "why" comment, placed perfectly.- Documentation:
workflow_artifactwas already documented indocs/adapters/web.mdbefore this PR. No doc changes needed. - ArtifactViewerModal error handling: The modal's
loadArtifactuses try/catch, logs full context, and renders visible error state — solid pre-existing foundation.
📋 Suggested Follow-up Issues
| Issue Title | Priority |
|---|---|
Extract extractArtifactInfo to src/lib/artifact-utils.ts and add unit tests |
P2 |
Fix copyMessage silent clipboard failure in non-HTTPS contexts |
P3 |
Reviewed by Archon comprehensive-pr-review workflow — 5 specialized agents
Full artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/6bd8e7f41496775511f972dfdb17c9df/review/
- Replace opaque `ComponentPropsWithoutRef<never>` return type on `makeMarkdownComponents` with the idiomatic `Components` type from react-markdown, which is the actual contract the prop expects - Add explanatory comment on `ARTIFACT_PATH_RE` describing group semantics and cross-platform path handling intent - Add comment on `useMemo` empty dep array explaining that `setArtifactViewer` is a stable React state setter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix Report: PR #1023 Review FindingsCommit: 2a52c13 Applied 3 fixes from the consolidated review to Fixes AppliedFIX 1: Replace opaque return type on
FIX 2: Add comment on Added: FIX 3: Add comment on Added: Skipped Findings
Validation
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Make ARTIFACT_PATH_RE case-insensitive for hex digits ([a-fA-F0-9-]+) so uppercase UUID characters are matched correctly. Also reject filenames containing '..' path segments to prevent path traversal. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/web/src/components/chat/MessageBubble.tsx (1)
15-25: Add focused coverage for artifact path parsing.This helper is now the gatekeeper for the feature. A small table-driven test set for forward slashes, backslashes, nested filenames, and non-matches would make regressions here cheap to catch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageBubble.tsx` around lines 15 - 25, Add focused, table-driven unit tests for the artifact path parser by exercising ARTIFACT_PATH_RE and extractArtifactInfo with cases covering forward-slash paths, backslash paths, nested filenames (with directories), and non-matching strings; for each case assert that extractArtifactInfo returns the expected { runId, filename } (with backslashes normalized to '/') or null for non-matches. Structure the tests as a data array of inputs and expected outputs, iterate over them in a single test suite targeting the extractArtifactInfo function, and include descriptive test names for each case to make regressions easy to identify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/components/chat/MessageBubble.tsx`:
- Around line 1-3: In MessageBubble's custom ReactMarkdown code renderer (the
`code` component where you inspect `children` to extract artifact paths and make
code clickable), normalize the incoming node text with String(children) before
running your artifact-matching logic so array-like ReactNodes (e.g., ['text'])
are handled; update the code path that currently assumes a plain string
(referencing `children` in the `code` renderer) to use the normalized value for
artifact extraction and subsequent clickability.
---
Nitpick comments:
In `@packages/web/src/components/chat/MessageBubble.tsx`:
- Around line 15-25: Add focused, table-driven unit tests for the artifact path
parser by exercising ARTIFACT_PATH_RE and extractArtifactInfo with cases
covering forward-slash paths, backslash paths, nested filenames (with
directories), and non-matching strings; for each case assert that
extractArtifactInfo returns the expected { runId, filename } (with backslashes
normalized to '/') or null for non-matches. Structure the tests as a data array
of inputs and expected outputs, iterate over them in a single test suite
targeting the extractArtifactInfo function, and include descriptive test names
for each case to make regressions easy to identify.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1368da1c-05c0-4b29-a25f-0bd0b20fab92
📒 Files selected for processing (2)
packages/web/src/components/chat/MessageBubble.tsxpackages/workflows/src/store.ts
| import { memo, useMemo, useState } from 'react'; | ||
| import { Copy, Check, Paperclip } from 'lucide-react'; | ||
| import ReactMarkdown, { type Components } from 'react-markdown'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In react-markdown, can a custom codecomponent receivechildrenas aReactNode[]for inline code, and is flattening/coercingchildren the recommended way to read the code text for custom rendering?
💡 Result:
Yes, react-markdown’s custom code renderer can receive children that at runtime is sometimes an array-like React children value (e.g., ['text']), even though it’s typed as the normal React children prop (i.e., ReactNode, not specifically ReactNode[]). This follows from how react-markdown types components: each tag (including code) gets the corresponding intrinsic element props (ComponentProps<'code'>) plus ExtraProps, which means children is whatever React normally allows for that element, not a dedicated “string” prop. [1]
And yes: coercing/flattening children is the documented pattern when you need the raw code text for custom rendering (especially for fenced code blocks). The official README’s syntax-highlighting example uses:
children={String(children).replace(/\n$/, '')}
to read the code text from children. [1]
Source
react-markdownREADME (components typing + customcodeexample usingString(children)) [1]
Normalize code-node text before artifact matching.
Line 52 assumes children is a plain string, but react-markdown passes custom code renderers a ReactNode that at runtime can be array-like (e.g., ['text']). When children is an array, the artifact path is never extracted and the code remains non-clickable.
Use String(children) to normalize the text before artifact matching, following the documented pattern in react-markdown:
Proposed fix
if (typeof children === 'string') {
- const artifact = extractArtifactInfo(children);
+ const codeText = String(children).replace(/\n$/, '');
+ if (codeText) {
+ const artifact = extractArtifactInfo(codeText);
if (artifact) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/components/chat/MessageBubble.tsx` around lines 1 - 3, In
MessageBubble's custom ReactMarkdown code renderer (the `code` component where
you inspect `children` to extract artifact paths and make code clickable),
normalize the incoming node text with String(children) before running your
artifact-matching logic so array-like ReactNodes (e.g., ['text']) are handled;
update the code path that currently assumes a plain string (referencing
`children` in the `code` renderer) to use the normalized value for artifact
extraction and subsequent clickability.
The artifact-aware code component was only in MessageBubble.tsx but WorkflowResultCard in MessageList.tsx used its own static markdown components without artifact detection. Artifact paths in workflow results were rendering as plain code spans instead of clickable links. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/components/chat/MessageBubble.tsx (1)
155-162:⚠️ Potential issue | 🟡 MinorUnhandled promise rejection on clipboard write failure.
navigator.clipboard.writeTextcan reject (e.g., permissions denied, non-HTTPS context), but only the success case is handled. Users won't know if copy failed.This appears to be pre-existing per PR comments. Consider adding error handling in a follow-up:
Suggested improvement
const copyMessage = (): void => { - void navigator.clipboard.writeText(message.content).then(() => { + navigator.clipboard.writeText(message.content).then(() => { setCopied(true); setTimeout(() => { setCopied(false); }, 1500); - }); + }).catch(() => { + // Optionally show error feedback or log + }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageBubble.tsx` around lines 155 - 162, The copyMessage function currently only handles the success path of navigator.clipboard.writeText and can cause unhandled promise rejections; update copyMessage to handle failures by adding a catch (or use async/await with try/catch) around navigator.clipboard.writeText, log or surface the error (e.g., console.error or trigger an existing toast/error state), and ensure setCopied is only set to true on success and remains false (or set an error state) on failure so the UI reflects copy failure; specifically modify the copyMessage function and its use of navigator.clipboard.writeText and setCopied to include proper error handling.
🧹 Nitpick comments (2)
packages/web/src/components/chat/MessageBubble.tsx (1)
29-121: Consider consolidating markdown component factories.
makeMarkdownComponentshere andmakeResultMarkdownComponentsinMessageList.tsxshare the same artifact-handling logic in thecoderenderer and similararenderer. The main differences are styling and additional elements (pre,table,blockquote).A shared base or extracted artifact-code renderer could reduce duplication, but given the styling differences, this is acceptable as-is if the team prefers co-located component definitions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageBubble.tsx` around lines 29 - 121, The two markdown factories duplicate artifact-handling logic in their code renderers and similar link rendering (see makeMarkdownComponents and makeResultMarkdownComponents, specifically the code renderer and a renderer that use extractArtifactInfo and onArtifactClick); refactor by extracting a shared artifact-aware code renderer and a shared link component (or small helpers that build the artifact anchor/button) and have both factories call those helpers, keeping only the differing wrappers/styles in each factory so styling stays local while eliminating the duplicate artifact parsing and URL construction logic.packages/web/src/components/chat/MessageList.tsx (1)
15-24: Duplicate utility code with MessageBubble.tsx.
ARTIFACT_PATH_REandextractArtifactInfoare identical copies of the same code inMessageBubble.tsx. This violates DRY and creates maintenance burden when changes are needed.Consider extracting to a shared utility, e.g.,
@/lib/artifact-utils.ts:Suggested refactor
// packages/web/src/lib/artifact-utils.ts export const ARTIFACT_PATH_RE = /artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)/; export function extractArtifactInfo(text: string): { runId: string; filename: string } | null { const match = ARTIFACT_PATH_RE.exec(text); if (!match) return null; const filename = match[2].replace(/\\/g, '/'); if (filename.split('/').some(s => s === '..')) return null; return { runId: match[1], filename }; }Then import in both files:
import { extractArtifactInfo } from '@/lib/artifact-utils';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageList.tsx` around lines 15 - 24, The ARTIFACT_PATH_RE constant and extractArtifactInfo function are duplicated in MessageList.tsx (and MessageBubble.tsx); extract them into a single shared module (e.g., create a lib file exporting ARTIFACT_PATH_RE and extractArtifactInfo), remove the local definitions from MessageList.tsx and MessageBubble.tsx, and import the shared exports (e.g., import { ARTIFACT_PATH_RE, extractArtifactInfo } from '@/lib/artifact-utils') so both files use the same implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 53-54: The code in MessageList.tsx treats children as a plain
string before calling extractArtifactInfo but react-markdown can pass children
as array-like ReactNode (e.g., ['text']), causing artifact paths to be missed;
normalize the node by coercing children to a string (use String(children))
before running artifact matching — e.g., inside the branch that currently checks
typeof children === 'string' (or replace that check) create a text variable via
String(children) and call extractArtifactInfo(text) so artifact detection works
for all react-markdown code-node shapes.
---
Outside diff comments:
In `@packages/web/src/components/chat/MessageBubble.tsx`:
- Around line 155-162: The copyMessage function currently only handles the
success path of navigator.clipboard.writeText and can cause unhandled promise
rejections; update copyMessage to handle failures by adding a catch (or use
async/await with try/catch) around navigator.clipboard.writeText, log or surface
the error (e.g., console.error or trigger an existing toast/error state), and
ensure setCopied is only set to true on success and remains false (or set an
error state) on failure so the UI reflects copy failure; specifically modify the
copyMessage function and its use of navigator.clipboard.writeText and setCopied
to include proper error handling.
---
Nitpick comments:
In `@packages/web/src/components/chat/MessageBubble.tsx`:
- Around line 29-121: The two markdown factories duplicate artifact-handling
logic in their code renderers and similar link rendering (see
makeMarkdownComponents and makeResultMarkdownComponents, specifically the code
renderer and a renderer that use extractArtifactInfo and onArtifactClick);
refactor by extracting a shared artifact-aware code renderer and a shared link
component (or small helpers that build the artifact anchor/button) and have both
factories call those helpers, keeping only the differing wrappers/styles in each
factory so styling stays local while eliminating the duplicate artifact parsing
and URL construction logic.
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 15-24: The ARTIFACT_PATH_RE constant and extractArtifactInfo
function are duplicated in MessageList.tsx (and MessageBubble.tsx); extract them
into a single shared module (e.g., create a lib file exporting ARTIFACT_PATH_RE
and extractArtifactInfo), remove the local definitions from MessageList.tsx and
MessageBubble.tsx, and import the shared exports (e.g., import {
ARTIFACT_PATH_RE, extractArtifactInfo } from '@/lib/artifact-utils') so both
files use the same implementation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a25b8914-0f8c-4f39-9610-86b33aebac31
📒 Files selected for processing (2)
packages/web/src/components/chat/MessageBubble.tsxpackages/web/src/components/chat/MessageList.tsx
| if (typeof children === 'string') { | ||
| const artifact = extractArtifactInfo(children); |
There was a problem hiding this comment.
Normalize code-node text before artifact matching.
Line 53 assumes children is a plain string, but react-markdown can pass children as an array-like ReactNode at runtime (e.g., ['text']). When this happens, artifact paths won't be detected.
Use String(children) to normalize, following the documented react-markdown pattern:
Proposed fix
- if (typeof children === 'string') {
- const artifact = extractArtifactInfo(children);
+ const codeText = typeof children === 'string' ? children : String(children).replace(/\n$/, '');
+ if (codeText) {
+ const artifact = extractArtifactInfo(codeText);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/components/chat/MessageList.tsx` around lines 53 - 54, The
code in MessageList.tsx treats children as a plain string before calling
extractArtifactInfo but react-markdown can pass children as array-like ReactNode
(e.g., ['text']), causing artifact paths to be missed; normalize the node by
coercing children to a string (use String(children)) before running artifact
matching — e.g., inside the branch that currently checks typeof children ===
'string' (or replace that check) create a text variable via String(children) and
call extractArtifactInfo(text) so artifact detection works for all
react-markdown code-node shapes.
…right link styling The artifact route derived owner/repo from the working_path, which fails for worktree-based runs (worktrees use local filesystem username, not GitHub username). Now looks up the codebase record by codebase_id to get the correct owner/repo from the codebase name. Also fixes artifact link styling in WorkflowResultCard — uses text-accent-bright instead of text-[inherit] which was inheriting the dim text-text-secondary color from the parent container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bumped artifact link color from accent-bright (0.72) to a brighter oklch(0.78 0.18 250) with font-medium for better readability against the dark text-text-secondary card background. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tailwind classes were being overridden by parent text-text-secondary. Inline styles ensure the bright blue color and pointer cursor always apply regardless of CSS cascade. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web/src/components/chat/MessageList.tsx (1)
53-54:⚠️ Potential issue | 🟡 MinorNormalize code-node text before artifact matching.
Same issue as flagged in
MessageBubble.tsx:react-markdowncan passchildrenas an array-likeReactNode(e.g.,['text']), causing artifact paths to go undetected.Proposed fix
- if (typeof children === 'string') { - const artifact = extractArtifactInfo(children); + const codeText = typeof children === 'string' ? children : String(children).replace(/\n$/, ''); + if (codeText) { + const artifact = extractArtifactInfo(codeText);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageList.tsx` around lines 53 - 54, The artifact extraction misses cases where children is an array-like ReactNode (e.g., ['text']); update the logic in MessageList (around the block using children and extractArtifactInfo) to normalize children to a plain string before matching—e.g., convert React children to an array and join or otherwise coerce to string—then call extractArtifactInfo(normalizedText) so artifact paths inside array-like nodes are detected (mirror the normalization used in MessageBubble.tsx).
🧹 Nitpick comments (1)
packages/web/src/components/chat/MessageList.tsx (1)
15-24: Extract shared artifact path utilities to avoid duplication.
ARTIFACT_PATH_REandextractArtifactInfoare duplicated fromMessageBubble.tsx. This creates maintenance burden—any bug fix or enhancement must be applied in both places.Extract these to a shared utility module (e.g.,
@/lib/artifact-utils.ts):// `@/lib/artifact-utils.ts` export const ARTIFACT_PATH_RE = /artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)/; export function extractArtifactInfo(text: string): { runId: string; filename: string } | null { const match = ARTIFACT_PATH_RE.exec(text); if (!match) return null; const filename = match[2].replace(/\\/g, '/'); if (filename.split('/').some(s => s === '..')) return null; return { runId: match[1], filename }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageList.tsx` around lines 15 - 24, Move the duplicated artifact regex and helper into a shared module and import it where needed: create a new module exporting ARTIFACT_PATH_RE and extractArtifactInfo (matching the provided implementation), then remove the local declarations from MessageList (and ensure MessageBubble uses the shared exports) and update imports to use the new module (e.g., import { ARTIFACT_PATH_RE, extractArtifactInfo } from '@/lib/artifact-utils'). Ensure the exported extractArtifactInfo retains the same runtime behavior (normalizing backslashes, rejecting '..' segments) so existing callers of extractArtifactInfo and any use of ARTIFACT_PATH_RE keep working.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/routes/api.ts`:
- Around line 2455-2466: The handler currently bails with apiError when codebase
lookup fails or codebase.name doesn't contain owner/repo; instead, if
run.working_path exists (i.e., codebase_id is null or codebase deleted)
construct the artifact directory as join(run.working_path, '.archon',
'artifacts', 'runs', runId) and use that as the artifact path rather than
returning 404; otherwise keep the existing getLog().error and return apiError.
Update the logic around codebaseDb.getCodebase / nameParts / [owner, repo] to
branch: prefer owner/repo when available, else fallback to the
run.working_path-based artifact directory (use run, run.working_path, runId,
getLog(), and apiError symbols to locate and modify the code).
---
Duplicate comments:
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 53-54: The artifact extraction misses cases where children is an
array-like ReactNode (e.g., ['text']); update the logic in MessageList (around
the block using children and extractArtifactInfo) to normalize children to a
plain string before matching—e.g., convert React children to an array and join
or otherwise coerce to string—then call extractArtifactInfo(normalizedText) so
artifact paths inside array-like nodes are detected (mirror the normalization
used in MessageBubble.tsx).
---
Nitpick comments:
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 15-24: Move the duplicated artifact regex and helper into a shared
module and import it where needed: create a new module exporting
ARTIFACT_PATH_RE and extractArtifactInfo (matching the provided implementation),
then remove the local declarations from MessageList (and ensure MessageBubble
uses the shared exports) and update imports to use the new module (e.g., import
{ ARTIFACT_PATH_RE, extractArtifactInfo } from '@/lib/artifact-utils'). Ensure
the exported extractArtifactInfo retains the same runtime behavior (normalizing
backslashes, rejecting '..' segments) so existing callers of extractArtifactInfo
and any use of ARTIFACT_PATH_RE keep working.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebacf04e-c03e-48c5-960d-acd21a584d9e
📒 Files selected for processing (2)
packages/server/src/routes/api.tspackages/web/src/components/chat/MessageList.tsx
| // Derive owner/repo from codebase name (format: "owner/repo") | ||
| const codebase = run.codebase_id ? await codebaseDb.getCodebase(run.codebase_id) : null; | ||
| if (!codebase?.name) { | ||
| getLog().error({ runId, codebaseId: run.codebase_id }, 'artifacts.codebase_lookup_failed'); | ||
| return apiError(c, 404, 'Artifact not available: codebase not found'); | ||
| } | ||
| const relative = normalizedWorkingPath.substring(normalizedWorkspacesPath.length + 1); | ||
| const parts = relative.split(sep).filter(p => p.length > 0); | ||
| if (parts.length < 2) { | ||
| getLog().error({ runId, workingPath: run.working_path }, 'artifacts.owner_repo_parse_failed'); | ||
| const nameParts = codebase.name.split('/'); | ||
| if (nameParts.length < 2) { | ||
| getLog().error({ runId, codebaseName: codebase.name }, 'artifacts.owner_repo_parse_failed'); | ||
| return apiError(c, 404, 'Artifact not available: could not determine owner/repo'); | ||
| } | ||
| const [owner, repo] = parts; | ||
| const [owner, repo] = nameParts; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any workflow runs exist with null codebase_id
ast-grep --pattern 'codebase_id UUID REFERENCES'
# Search for any existing fallback logic or migration plans
rg -n 'codebase_id' --type sql -C3Repository: coleam00/Archon
Length of output: 12355
🏁 Script executed:
# Get the artifact endpoint context in api.ts
cd packages/server/src/routes && sed -n '2430,2480p' api.tsRepository: coleam00/Archon
Length of output: 2014
🏁 Script executed:
# Search for any working_path usage and previous artifact retrieval logic
rg -n 'working_path' packages/server/src/routes/api.ts -B2 -A2Repository: coleam00/Archon
Length of output: 824
🏁 Script executed:
# Search for any codebase_id related fallback or migration logic
rg -n 'codebase_id' packages/server/src/routes/api.ts -B2 -A2Repository: coleam00/Archon
Length of output: 462
🏁 Script executed:
# Find getRunArtifactsPath definition
rg -n 'getRunArtifactsPath' packages/server/src -B2 -A5Repository: coleam00/Archon
Length of output: 1254
🏁 Script executed:
# Look for any constants or configuration that shows artifact storage structure
rg -n 'artifacts' packages/server/src/routes/api.ts | head -20Repository: coleam00/Archon
Length of output: 793
🏁 Script executed:
# Check if working_path is even stored in workflow_runs table
rg -n 'working_path' migrations/ -B2 -A2Repository: coleam00/Archon
Length of output: 1672
🏁 Script executed:
# Find the `@archon/paths` module (likely in packages/paths or similar)
find packages -name "paths*" -type dRepository: coleam00/Archon
Length of output: 73
🏁 Script executed:
# Look for getRunArtifactsPath implementation
rg -n 'getRunArtifactsPath' --type ts -B2 -A8Repository: coleam00/Archon
Length of output: 9863
🏁 Script executed:
# Check if there's any fallback logic or migration strategy for null codebase_id
rg -n 'codebase_id.*null\|null.*codebase_id' packages/server/src --type ts -C3Repository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Check git history of the artifact endpoint - see if it's recent
cd packages/server/src/routes && git log --oneline -20 -- api.ts | head -10Repository: coleam00/Archon
Length of output: 112
🏁 Script executed:
# Look for any migration or data population scripts for codebase_id
find . -name "*migrat*" -type f -path "*/scripts/*" -o -name "*backfill*" -type f 2>/dev/null | head -10Repository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Check if there are any TODOs or FIXMEs related to this issue
rg -n 'TODO|FIXME' packages/server/src/routes/api.ts | grep -i 'artifact\|codebase'Repository: coleam00/Archon
Length of output: 41
Add fallback artifact path when codebase_id is unavailable.
The artifact endpoint returns 404 if codebase_id is null or the codebase lookup fails. However, the schema allows codebase_id to be nullable (migration 008: ON DELETE SET NULL), which affects:
- Older workflow runs with
codebase_id = null - Runs whose associated codebase was deleted
Artifacts are otherwise accessible via the worktree path (as seen in packages/workflows/src/executor.ts lines 198 and packages/cli/src/commands/continue.ts lines 220–222, which both provide fallbacks). The endpoint should use a similar fallback: if codebase_id is null and the run has a working_path, construct the artifact directory as join(run.working_path, '.archon', 'artifacts', 'runs', runId) before returning 404.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/api.ts` around lines 2455 - 2466, The handler
currently bails with apiError when codebase lookup fails or codebase.name
doesn't contain owner/repo; instead, if run.working_path exists (i.e.,
codebase_id is null or codebase deleted) construct the artifact directory as
join(run.working_path, '.archon', 'artifacts', 'runs', runId) and use that as
the artifact path rather than returning 404; otherwise keep the existing
getLog().error and return apiError. Update the logic around
codebaseDb.getCodebase / nameParts / [owner, repo] to branch: prefer owner/repo
when available, else fallback to the run.working_path-based artifact directory
(use run, run.working_path, runId, getLog(), and apiError symbols to locate and
modify the code).
Replaced <button> with <a> for .md artifacts (pointer cursor by default), unified styling using !text-accent-bright to override parent text-text-secondary, hover brightens to oklch(0.85). All artifact links now consistently bright and clickable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web/src/components/chat/MessageList.tsx (1)
45-54:⚠️ Potential issue | 🟠 MajorNormalize code text and use a stronger block-code gate before artifact detection.
This branch still has the earlier
children-normalization gap, andclassNamealone is not enough to distinguish inline code from plain triple-backtick blocks. That means some artifact paths will still be missed, while unlabeled fenced blocks can fall into the inline-artifact branch and render as buttons/links.Suggested direction
- const isBlock = className?.startsWith('language-') || className?.startsWith('hljs'); + const codeText = String(children); + const isBlock = + className?.startsWith('language-') || + className?.startsWith('hljs') || + codeText.includes('\n'); if (isBlock) { return ( <code className={className} {...props}> {children} </code> ); } - if (typeof children === 'string') { - const artifact = extractArtifactInfo(children); + const artifact = extractArtifactInfo(codeText.replace(/\n$/, ''));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/chat/MessageList.tsx` around lines 45 - 54, The code currently treats blocks by className only and then calls extractArtifactInfo on children without normalizing children, causing unlabeled fenced blocks to be misdetected; in MessageList.tsx update the gate and normalization: normalize children to a plain string (e.g. convert React children via React.Children.toArray(...) and join or use String(...) so extractArtifactInfo always receives a string), strengthen isBlock (in addition to className checks) to detect multi-line/fenced code by checking for newlines or triple-backtick markers (e.g. childrenString.includes('\n') or /```/), and only run extractArtifactInfo when childrenString is a single-line/inline code candidate; reference the isBlock variable, className, children, and extractArtifactInfo when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 15-19: The regex used by ARTIFACT_PATH_RE allows substring
matches, so extractArtifactInfo() will hit on inline code or surrounding text;
change ARTIFACT_PATH_RE to require a full-string match by adding start (^) and
end ($) anchors (e.g. /^artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)$/) so
extractArtifactInfo only returns when the whole input equals the artifact path;
keep the same capture groups and use ARTIFACT_PATH_RE.exec(text) as before in
extractArtifactInfo.
---
Duplicate comments:
In `@packages/web/src/components/chat/MessageList.tsx`:
- Around line 45-54: The code currently treats blocks by className only and then
calls extractArtifactInfo on children without normalizing children, causing
unlabeled fenced blocks to be misdetected; in MessageList.tsx update the gate
and normalization: normalize children to a plain string (e.g. convert React
children via React.Children.toArray(...) and join or use String(...) so
extractArtifactInfo always receives a string), strengthen isBlock (in addition
to className checks) to detect multi-line/fenced code by checking for newlines
or triple-backtick markers (e.g. childrenString.includes('\n') or /```/), and
only run extractArtifactInfo when childrenString is a single-line/inline code
candidate; reference the isBlock variable, className, children, and
extractArtifactInfo when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cabe93ff-8946-4019-9b75-907b000a137a
📒 Files selected for processing (1)
packages/web/src/components/chat/MessageList.tsx
| // Matches artifact paths (forward- and back-slash safe); groups: [1] runId, [2] filename | ||
| const ARTIFACT_PATH_RE = /artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)/; | ||
|
|
||
| function extractArtifactInfo(text: string): { runId: string; filename: string } | null { | ||
| const match = ARTIFACT_PATH_RE.exec(text); |
There was a problem hiding this comment.
Require a full-string artifact-path match.
extractArtifactInfo() currently accepts substring matches. An inline code span like cat artifacts/runs/<uuid>/plan.md will be collapsed to a plan.md link/button and drop the surrounding command text. Anchor the regex so only bare artifact paths become clickable.
Suggested fix
-const ARTIFACT_PATH_RE = /artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)/;
+const ARTIFACT_PATH_RE = /^artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)$/;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/components/chat/MessageList.tsx` around lines 15 - 19, The
regex used by ARTIFACT_PATH_RE allows substring matches, so
extractArtifactInfo() will hit on inline code or surrounding text; change
ARTIFACT_PATH_RE to require a full-string match by adding start (^) and end ($)
anchors (e.g. /^artifacts[/\\]runs[/\\]([a-fA-F0-9-]+)[/\\](.+)$/) so
extractArtifactInfo only returns when the whole input equals the artifact path;
keep the same capture groups and use ARTIFACT_PATH_RE.exec(text) as before in
extractArtifactInfo.
… cursor - Restore getArchonWorkspacesPath import removed by prior commit (still used in 3 other places in api.ts, breaking the build) - Use text-accent-bright instead of dark text-accent for artifact links in MessageBubble (was nearly invisible on dark background) - Add cursor-pointer to .md artifact buttons in MessageBubble - Replace arbitrary oklch hover value with hover:!text-primary in MessageList artifact links Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve merge conflict in MessageList.tsx by combining: - PR #1025: status/duration/nodes/artifacts enrichment for WorkflowResultCard - PR #1023: ArtifactViewerModal clickable file paths in result card content Both features now work together — the result card shows status-aware headers, node counts, duration, and artifact summaries while also supporting clickable artifact file paths in the markdown content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…m00#1023) * feat: make artifact file paths clickable in chat messages (coleam00#1016) Artifact paths in AI responses (e.g. `artifacts/runs/{uuid}/report.md`) were rendered as plain inline code. Now they render as clickable buttons (.md opens ArtifactViewerModal) or links (other types open in new tab). Changes: - Add artifact path detection via regex in MessageBubble code renderer - Convert MARKDOWN_COMPONENTS to factory function for modal state access - Add ArtifactViewerModal integration in chat view - Add 'workflow_artifact' to WORKFLOW_EVENT_TYPES in store.ts Fixes coleam00#1016 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(web): improve MessageBubble type clarity and code comments - Replace opaque `ComponentPropsWithoutRef<never>` return type on `makeMarkdownComponents` with the idiomatic `Components` type from react-markdown, which is the actual contract the prop expects - Add explanatory comment on `ARTIFACT_PATH_RE` describing group semantics and cross-platform path handling intent - Add comment on `useMemo` empty dep array explaining that `setArtifactViewer` is a stable React state setter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * simplify: merge duplicate lucide-react imports in MessageBubble Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): harden extractArtifactInfo UUID regex and path traversal check Make ARTIFACT_PATH_RE case-insensitive for hex digits ([a-fA-F0-9-]+) so uppercase UUID characters are matched correctly. Also reject filenames containing '..' path segments to prevent path traversal. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): add artifact path detection to WorkflowResultCard markdown The artifact-aware code component was only in MessageBubble.tsx but WorkflowResultCard in MessageList.tsx used its own static markdown components without artifact detection. Artifact paths in workflow results were rendering as plain code spans instead of clickable links. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(server,web): artifact route uses codebase name for owner/repo + bright link styling The artifact route derived owner/repo from the working_path, which fails for worktree-based runs (worktrees use local filesystem username, not GitHub username). Now looks up the codebase record by codebase_id to get the correct owner/repo from the codebase name. Also fixes artifact link styling in WorkflowResultCard — uses text-accent-bright instead of text-[inherit] which was inheriting the dim text-text-secondary color from the parent container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve merge conflict markers in api.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style(web): brighter artifact link color in workflow result cards Bumped artifact link color from accent-bright (0.72) to a brighter oklch(0.78 0.18 250) with font-medium for better readability against the dark text-text-secondary card background. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style(web): use inline styles for artifact links to guarantee visibility Tailwind classes were being overridden by parent text-text-secondary. Inline styles ensure the bright blue color and pointer cursor always apply regardless of CSS cascade. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(web): use <a> tags for all artifact links with !text-accent-bright Replaced <button> with <a> for .md artifacts (pointer cursor by default), unified styling using !text-accent-bright to override parent text-text-secondary, hover brightens to oklch(0.85). All artifact links now consistently bright and clickable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(web,server): restore missing import, fix artifact link colors and cursor - Restore getArchonWorkspacesPath import removed by prior commit (still used in 3 other places in api.ts, breaking the build) - Use text-accent-bright instead of dark text-accent for artifact links in MessageBubble (was nearly invisible on dark background) - Add cursor-pointer to .md artifact buttons in MessageBubble - Replace arbitrary oklch hover value with hover:!text-primary in MessageList artifact links Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve merge conflict in MessageList.tsx by combining: - PR coleam00#1025: status/duration/nodes/artifacts enrichment for WorkflowResultCard - PR coleam00#1023: ArtifactViewerModal clickable file paths in result card content Both features now work together — the result card shows status-aware headers, node counts, duration, and artifact summaries while also supporting clickable artifact file paths in the markdown content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…m00#1023) * feat: make artifact file paths clickable in chat messages (coleam00#1016) Artifact paths in AI responses (e.g. `artifacts/runs/{uuid}/report.md`) were rendered as plain inline code. Now they render as clickable buttons (.md opens ArtifactViewerModal) or links (other types open in new tab). Changes: - Add artifact path detection via regex in MessageBubble code renderer - Convert MARKDOWN_COMPONENTS to factory function for modal state access - Add ArtifactViewerModal integration in chat view - Add 'workflow_artifact' to WORKFLOW_EVENT_TYPES in store.ts Fixes coleam00#1016 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(web): improve MessageBubble type clarity and code comments - Replace opaque `ComponentPropsWithoutRef<never>` return type on `makeMarkdownComponents` with the idiomatic `Components` type from react-markdown, which is the actual contract the prop expects - Add explanatory comment on `ARTIFACT_PATH_RE` describing group semantics and cross-platform path handling intent - Add comment on `useMemo` empty dep array explaining that `setArtifactViewer` is a stable React state setter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * simplify: merge duplicate lucide-react imports in MessageBubble Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): harden extractArtifactInfo UUID regex and path traversal check Make ARTIFACT_PATH_RE case-insensitive for hex digits ([a-fA-F0-9-]+) so uppercase UUID characters are matched correctly. Also reject filenames containing '..' path segments to prevent path traversal. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): add artifact path detection to WorkflowResultCard markdown The artifact-aware code component was only in MessageBubble.tsx but WorkflowResultCard in MessageList.tsx used its own static markdown components without artifact detection. Artifact paths in workflow results were rendering as plain code spans instead of clickable links. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(server,web): artifact route uses codebase name for owner/repo + bright link styling The artifact route derived owner/repo from the working_path, which fails for worktree-based runs (worktrees use local filesystem username, not GitHub username). Now looks up the codebase record by codebase_id to get the correct owner/repo from the codebase name. Also fixes artifact link styling in WorkflowResultCard — uses text-accent-bright instead of text-[inherit] which was inheriting the dim text-text-secondary color from the parent container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve merge conflict markers in api.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style(web): brighter artifact link color in workflow result cards Bumped artifact link color from accent-bright (0.72) to a brighter oklch(0.78 0.18 250) with font-medium for better readability against the dark text-text-secondary card background. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style(web): use inline styles for artifact links to guarantee visibility Tailwind classes were being overridden by parent text-text-secondary. Inline styles ensure the bright blue color and pointer cursor always apply regardless of CSS cascade. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(web): use <a> tags for all artifact links with !text-accent-bright Replaced <button> with <a> for .md artifacts (pointer cursor by default), unified styling using !text-accent-bright to override parent text-text-secondary, hover brightens to oklch(0.85). All artifact links now consistently bright and clickable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(web,server): restore missing import, fix artifact link colors and cursor - Restore getArchonWorkspacesPath import removed by prior commit (still used in 3 other places in api.ts, breaking the build) - Use text-accent-bright instead of dark text-accent for artifact links in MessageBubble (was nearly invisible on dark background) - Add cursor-pointer to .md artifact buttons in MessageBubble - Replace arbitrary oklch hover value with hover:!text-primary in MessageList artifact links Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolve merge conflict in MessageList.tsx by combining: - PR coleam00#1025: status/duration/nodes/artifacts enrichment for WorkflowResultCard - PR coleam00#1023: ArtifactViewerModal clickable file paths in result card content Both features now work together — the result card shows status-aware headers, node counts, duration, and artifact summaries while also supporting clickable artifact file paths in the markdown content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
artifacts/runs/{uuid}/...) are now rendered as clickable buttons (.mdfiles) or links (other types) instead of plain monospace text.mdartifact path opensArtifactViewerModalinline — reusing the modal that already exists in the workflow execution view.mdartifact paths open in a new browser tab via<a>link'workflow_artifact'toWORKFLOW_EVENT_TYPESinstore.ts, unblocking future artifact event persistence at the DB type levelChanges
packages/web/src/components/chat/MessageBubble.tsxMARKDOWN_COMPONENTSconstant to amakeMarkdownComponents(onArtifactClick)factory function, called inside the component withuseMemoto preserve memoizationextractArtifactInfo(text)module-level helper using regex/artifacts[/\]runs[/\]([a-f0-9-]+)[/\](.+)/to detect artifact paths<code>renderer: artifact.mdpaths render as<button>triggeringArtifactViewerModal; non-.mdpaths render as<a target="_blank">; all other inline code unchangeduseStateforartifactViewerstate andArtifactViewerModalbelow theReactMarkdownelementRegExp#exec()per ESLint@typescript-eslint/prefer-regexp-execrulepackages/workflows/src/store.ts'workflow_artifact'toWORKFLOW_EVENT_TYPESarray;WorkflowEventTypeunion derives automaticallyValidation
All checks passed on first run (
bun run validate):Test counts: @archon/core 939, @archon/workflows 637, @archon/adapters 343, @archon/isolation 220, @archon/git 135, @archon/web 110, @archon/server 193, @archon/cli 118, @archon/paths 92.
Test Plan
bun run dev)$ARTIFACTS_DIR(e.g.,archon-gsd)ArtifactViewerModalopens and renders the markdown content`someVariable`) still renders as plain<code>.mdartifact path, confirm it opens in a new tabFixes #1016
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores