feat: 新增 artifacts 功能#1278
Merged
Merged
Conversation
POST /upload 鉴权上传 HTML 到 R2 返回 hash URL，GET /<7d|30d>/<id>.html 由 Worker 代理读取并直出 text/html。R2 lifecycle rule 自动 7/30 天删除。 独立服务，不被主 CLI 引用（类似 packages/remote-control-server/ 定位）。 Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
- CLAUDE.md 加 cloud-artifacts 到 Workspace Packages 表和新增 HTML Artifact Hosting 段落 - docs.json 注册 cloud-artifacts 到运行模式 group - README 加 Quickstart、架构图（含 Deno Deploy 代理层）、Security Considerations、Troubleshooting - 统一出口域名为 https://cloud-artifacts.claude-code-best.win（wrangler.toml PUBLIC_URL、test.sh 默认 WORKER_URL、所有文档示例） - test.sh expect() 加 [via body] fallback：经 Deno Deploy 代理（status 抹平为 200）时按 body 的 error 字段断言 Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
之前指向不存在的 docs/features/cloud-artifacts.md（用户未注册到 docs.json）， 改为指向已存在的 packages/cloud-artifacts/README.md，并补充生产出口域名 与 Deno Deploy status 抹平副作用的说明。 Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
用户已通过 wrangler secret put 把生产 TOKEN 改为 claude-code-best， test.sh 的默认值（之前用旧 token 作 fallback）和注释示例同步更新。 现在直接 bash scripts/test.sh 即可跑通（无需显式传 TOKEN）。 src/index.ts 不依赖具体 token 值（只读 env.TOKEN 做比较）， wrangler.toml 不含 secret，README/.dev.vars.example 用 <your-token> 占位符故无需改。 Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Scans Message[] for artifact tool_use/tool_result pairs, parses URL/id/expires from the upload response string, and returns ArtifactInfo[] newest-first. Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
- Use double assertion (`as unknown as Record<string, unknown>`) at lines 30 and 90 to fix TS2352 per project convention - Tighten URL_REGEX to avoid capturing trailing punctuation (parens, quotes, commas) when URL is embedded in text - Add test case for array-form tool_result content path Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces an end-to-end HTML artifact hosting feature with a Cloudflare Worker backend, a builtin ArtifactTool client, and CLI browsing/teaching infrastructure. Adds ArtifactTool result rendering UI, ExecuteTool render delegation to inner tools, and TypeScript typing helpers to remote-control-server tests. Refactors query-loop message handling for cleaner toolUseResult cleanup. ChangesHTML Artifacts Feature
Tool Infrastructure Updates
Test Typing Helper Updates
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Claude CLI
participant ArtifactTool
participant uploadArtifact as uploadArtifact client
participant Worker as cloud-artifacts Worker
participant R2 as Cloudflare R2
User->>CLI: /use-artifacts (activates skill)
CLI->>CLI: registerUseArtifactsSkill injects USE_ARTIFACTS_PROMPT
User->>CLI: asks Claude to create HTML artifact
CLI->>ArtifactTool: call({ file_path, hash?, ttl? })
ArtifactTool->>ArtifactTool: fs.stat — verify file exists and is regular
ArtifactTool->>ArtifactTool: check size ≤ 10MB
ArtifactTool->>ArtifactTool: fs.readFile UTF-8
ArtifactTool->>uploadArtifact: { html, token, uploadUrl, hash?, ttl? }
uploadArtifact->>Worker: POST /upload (Bearer token, text/html body)
Worker->>R2: put <ttl>d/<id>.html
Worker-->>uploadArtifact: 200 JSON { id, url, expiresAt }
uploadArtifact-->>ArtifactTool: UploadResult
ArtifactTool-->>CLI: tool_result with URL and id
User->>CLI: /artifacts
CLI->>CLI: extractArtifacts(messages) — scan tool_use/tool_result pairs
CLI->>User: ArtifactsMenu (navigate, Enter to open URL, c to copy)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
claude-code-best
left a comment
claude-code-best
left a comment
Owner
Author
There was a problem hiding this comment.
审查总结
实现质量很高，架构分离清晰（Worker 服务 / Tool / Command / Skill 各自独立），错误处理完善，11 个测试全通过。
✅ 做得好的地方
- Deno Deploy 代理的 body-error 处理：client 端和 README 都有清晰说明，正确处理了代理将 HTTP status 抹平为 200 的副作用
- 文件验证在前、HTTP 在后：
ArtifactTool.call()先stat+ 大小检查，避免无效网络请求 - Scanner 的 URL_REGEX：排除了尾随括号/引号，避免从文本中提取 URL 时把标点带进去
- 测试覆盖完整：scanner 覆盖了 string/array 两种 tool_result content 格式、排序方向、错误情况
- Skill prompt 实用的：详细说明了 cadence（任务开始/里程碑/结束）和两步调用流程
⚠️ 关注项（非阻塞）
-
ARTIFACTS_DEFAULT_TOKEN = 'claude-code-best'(config.ts:5)：生产 token 暴露在源码中。README 的 Security Considerations 说「使用 ≥32 字符随机串」，但默认值就是一个短组织名。建议改为空串占位符，强制通过CLAUDE_ARTIFACTS_TOKEN环境变量设置。 -
命名不一致：常量
ARTIFACTS_DEFAULT_TOKEN和ARTIFACTS_DEFAULT_URL用复数ARTIFACTS，但工具名是单数artifact，命令名是复数artifacts。建议统一为单数（同工具名）。 -
Cloudflare Worker MAX_BYTES(wrangler.toml:13)：toml 里定义了MAX_BYTES = "10485760"，但 Worker 代码的 fallback 也是10 * 1024 * 1024。如果 toml 值被误改，fallback 会生效——这是防御性写法，没毛病但对于 10MB 这个硬限制，两边保持一致即可。 -
requiresUserInteraction: true+shouldDefer: true：工具始终走 deferred 路径，需要用户批准。这是合理的安全设计，因为上传会推送内容到公网。Skill prompt 里的两步调用说明也很清晰。
Hono Response.json() returns Promise<unknown> under strict TypeScript, causing 121 TS errors across middleware and routes test files. Co-Authored-By: glm-5-turbo <zai-org@claude-code-best.win>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cloud-artifacts/scripts/test.sh (1)
119-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSuccess-path checks don’t affect pass/fail outcome.
From Line 119 onward, uploads and R2 reads are printed but not asserted into
fail, so the script can exit 0 even when core success flow breaks.Suggested direction
+assert_http_200() { + local label="$1" code="$2" body="$3" + if [[ "$code" == "200" ]]; then + printf "${G}✓ %s -> HTTP 200${D}\n" "$label" + pass=$((pass+1)) + else + printf "${R}✗ %s -> HTTP %s (expected 200)${D}\n" "$label" "$code" + printf " body: %s\n" "$body" + fail=$((fail+1)) + fi +}Use this helper for each success upload and for the R2 verification commands.
🤖 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 `@packages/cloud-artifacts/scripts/test.sh` around lines 119 - 163, The success-path test cases from line 119 onward (the three curl upload operations and two wrangler R2 verification commands) are not validating their results or incrementing the fail counter, which allows the script to exit with success even when these critical operations fail. Identify the validation helper function used in earlier test cases and apply it to check the HTTP response codes from each curl command (in the three upload sections) and the exit codes from each wrangler r2 object get command (for the two R2 verification sections), ensuring that any failures properly increment the fail counter so that the script only exits with status 0 when all success-path checks actually pass.
🧹 Nitpick comments (4)
src/commands/artifacts/__tests__/scanner.test.ts (1)
102-138: ⚡ Quick winAdd a malformed array-chunk regression test for extractor hardening.
Please add a case where
tool_result.contentincludes non-string/non-object entries (e.g.,null,42) to ensure extraction remains non-throwing and skips invalid chunks safely.🤖 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/commands/artifacts/__tests__/scanner.test.ts` around lines 102 - 138, Add a new test case after the existing test for parsing url/id/expires from array-form tool_result content. The new test should verify that extractArtifacts handles malformed array chunks gracefully by including non-string and non-object entries (such as null or numeric values like 42) within the tool_result content array. The test should verify that the extraction function does not throw an error when encountering these invalid entries and either skips them safely or correctly processes only the valid content chunks.src/commands/artifacts/ArtifactsMenu.tsx (1)
38-40: Use async/await pattern instead of.then()for the clipboard flow.Replace the Promise chain with async/await to align with TypeScript coding guidelines. An async IIFE can preserve the original behavior while improving readability:
void (async () => { const raw = await setClipboard(target.url); if (raw) process.stdout.write(raw); })();🤖 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/commands/artifacts/ArtifactsMenu.tsx` around lines 38 - 40, The setClipboard call in ArtifactsMenu.tsx uses a Promise chain with .then() which should be refactored to use async/await for better readability. Replace the current .then() pattern with an async IIFE (Immediately Invoked Function Expression) that wraps the setClipboard call, await the result into a variable, and then perform the same conditional check on raw and process.stdout.write call within the async function body. This maintains the original behavior while using the more modern async/await syntax.Source: Coding guidelines
src/commands/artifacts/scanner.ts (1)
30-35: Replace forced casts with block-level type guards.The parser uses unsafe
ascasts to access properties likeb.id,b.input, andb.is_errordespite having conditional checks onb.typeandb.name. These checks don't narrow theRecord<string, unknown>type, so TypeScript cannot verify the property accesses. Add explicit type guard functions (e.g.,isArtifactToolUseBlock,isToolResultBlock) to properly narrow types and eliminate the casts at lines 30, 34–35, and 90, 93. This aligns with the coding guideline: "Use type guards (type narrowing) with union types instead of forced type casting."🤖 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/commands/artifacts/scanner.ts` around lines 30 - 35, The code uses unsafe type casts with `as` to access properties like `b.id`, `b.input`, and `b.is_error` on a `Record<string, unknown>` type without proper type narrowing. Create explicit type guard functions such as `isArtifactToolUseBlock` and `isToolResultBlock` that return true when the block has the expected type and name properties, then use these guards to properly narrow the types before accessing properties. Replace all the unsafe `as` casts where `b.id`, `b.input`, and `b.is_error` are accessed with calls to these type guard functions to ensure type safety without forcing casts.Source: Coding guidelines
docs/superpowers/plans/2026-06-20-artifacts-feature.md (1)
1280-1294: 💤 Low valueMinor: Repetitive "Press" verb structure in smoke test instructions.
Lines 1287–1289 begin three successive instructions with "Press" (Press
Enter, Pressc, PressEsc). While functionally clear, rewording one or two of these would improve readability per standard technical writing practice.💡 Suggested revision
- [ ] **Step 5: Verify /artifacts list after upload** After the agent uploads, type `/artifacts` again. Confirm you see the artifact row with the URL. Press `Enter` to open in browser (verifies the URL renders). Press `c` to copy to clipboard (verify pbcopy works on macOS). Press `Esc` to exit. + After the agent uploads, type `/artifacts` again. Confirm you see the artifact row with the URL. Press `Enter` to open in browser (verifies the URL renders), `c` to copy to clipboard (verify pbcopy works on macOS), or `Esc` to exit.🤖 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/superpowers/plans/2026-06-20-artifacts-feature.md` around lines 1280 - 1294, The smoke test instructions in Step 5 (Verify /artifacts list after upload) contain three consecutive sentences that all begin with "Press" (Press `Enter`, Press `c`, Press `Esc`), creating a repetitive structure that impacts readability. Reword one or two of these instructions to use alternative action verbs such as "Use", "Hit", or "Select" while keeping the exact same functionality and parenthetical explanations. This will improve the flow and readability of the technical documentation without changing the meaning or clarity of the steps.Source: Linters/SAST tools
🤖 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 `@docs/superpowers/plans/2026-06-20-artifacts-feature.md`:
- Around line 913-998: The code snippet in the plan document for scanner.ts does
not match the actual implementation and contains incorrect property access and
regex patterns. Update the code snippet shown in lines 913 through 998 of the
plan to match the actual scanner.ts implementation: change the URL_REGEX pattern
from /https?:\/\/\S+\.html\b/ to use the more restrictive character class
/https?:\/\/[^\s)"',]+\.html\b/, and update all property access from
message.role to message.type and from message.content to
message.message?.content throughout the extractArtifacts and findToolResult
functions to ensure developers following the plan will see the correct
implementation details.
In `@packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts`:
- Around line 16-18: The file_path field in the ArtifactTool schema is described
as requiring an absolute path but the current Zod validation does not enforce
this constraint, allowing relative paths that would be cwd-dependent. Add
explicit validation to the file_path field's schema definition to ensure only
absolute paths are accepted, using Zod's refine or superRefine method to
validate that the provided path is an absolute path before accepting it.
In `@packages/builtin-tools/src/tools/ArtifactTool/client.ts`:
- Around line 22-29: The fetch call in the upload flow is missing a timeout
mechanism, which can cause the tool to hang indefinitely if the network stalls.
Add a request timeout by creating an AbortController before the fetch call,
setting a timeout that aborts the controller after a reasonable duration (e.g.,
30 seconds), and passing the abort controller's signal in the fetch options
object alongside the method, headers, and body properties. This will ensure the
fetch operation fails gracefully if it exceeds the timeout threshold instead of
blocking indefinitely.
In `@packages/builtin-tools/src/tools/ArtifactTool/config.ts`:
- Around line 5-11: The getArtifactsToken function currently falls back to a
hardcoded ARTIFACTS_DEFAULT_TOKEN value when the CLAUDE_ARTIFACTS_TOKEN
environment variable is not set, which weakens security by using a publicly
known credential. Remove the ARTIFACTS_DEFAULT_TOKEN constant definition and
modify the getArtifactsToken function to throw an error (e.g., with a
descriptive message about the missing required environment variable) instead of
returning a default value, ensuring the credential must be explicitly configured
and the application fails fast if it is not provided.
In `@packages/cloud-artifacts/.gitignore`:
- Around line 4-5: The .gitignore file contains malformed wildcard patterns
using underscores (_) or escaped asterisks (\*) instead of the correct wildcard
character (*). Fix all instances by replacing underscore wildcards with
asterisks: change patterns like _.log to *.log and npm-debug.log_ to
npm-debug.log*. Apply the same fix to all affected lines mentioned (lines 4-5,
13, 18-20, 29, 62, 89, 159) to ensure common artifacts are properly ignored.
In `@packages/cloud-artifacts/README.md`:
- Line 27: The fenced code block at line 27 is missing a language identifier,
which triggers markdownlint rule MD040. Add a language tag immediately after the
opening triple backticks (for example, change ``` to ```text) to satisfy the
linting requirement. Choose an appropriate language identifier based on the
content of the code block, or use 'text' if no specific language applies.
In `@packages/cloud-artifacts/scripts/test.sh`:
- Around line 13-14: The TOKEN variable in the test.sh script has a hardcoded
default value that exposes a usable bearer token in the public repository,
creating a security vulnerability. Remove the default fallback value from the
TOKEN variable assignment so that the token must be explicitly provided via the
environment variable without any hardcoded default. This ensures the token
cannot be accessed from the repository itself and must be supplied at runtime.
- Around line 149-157: Replace the hardcoded bucket name "cloud-artifacts" in
the R2 object get commands with a BUCKET variable reference to match the
setup.sh convention. In the wrangler r2 object get command for the 7d path with
RANDOM_ID and the 30d path with test-artifact-v1, change the hardcoded
"cloud-artifacts/" prefix to use the BUCKET environment variable (typically
referenced as $BUCKET or ${BUCKET}) to support non-default bucket deployments.
In `@packages/cloud-artifacts/src/index.ts`:
- Around line 89-92: The deletion of old artifact keys using TTL_PREFIXES.map
and env.BUCKET.delete occurs before full body validation is complete. Move this
deletion block to after the body validation checks pass (after the
content-length validation and body reading are confirmed successful around lines
97-100). This ensures that if validation fails and returns an error response,
the original artifact data remains intact and is not lost. Keep the deletion as
part of the successful upload flow, only deleting old keys after confirming the
new upload is valid.
- Around line 5-9: Create a new file worker-configuration.d.ts in the
packages/cloud-artifacts directory containing the type definitions for
ExportedHandler from `@cloudflare/workers-types` and the complete Env interface
with all required properties (TOKEN, BUCKET, MAX_BYTES, DEFAULT_TTL_DAYS,
PUBLIC_URL). Then remove the duplicate declare global interface Env block from
src/index.ts that only contains TOKEN, since it will now be defined in the
centralized worker-configuration.d.ts file and tsconfig.json already references
it.
In `@src/commands/artifacts/scanner.ts`:
- Around line 46-51: The map function processing chunks from tool_result.content
does not guard against non-object values like null, numbers, or booleans before
using the 'text' in c operator, which will throw a runtime error. Add a type
guard to check if c is an object and not null before attempting to check for the
'text' property using the 'in' operator. This ensures that only objects are
evaluated for the text property, and all other types (including null) are safely
handled as empty strings.
---
Outside diff comments:
In `@packages/cloud-artifacts/scripts/test.sh`:
- Around line 119-163: The success-path test cases from line 119 onward (the
three curl upload operations and two wrangler R2 verification commands) are not
validating their results or incrementing the fail counter, which allows the
script to exit with success even when these critical operations fail. Identify
the validation helper function used in earlier test cases and apply it to check
the HTTP response codes from each curl command (in the three upload sections)
and the exit codes from each wrangler r2 object get command (for the two R2
verification sections), ensuring that any failures properly increment the fail
counter so that the script only exits with status 0 when all success-path checks
actually pass.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-06-20-artifacts-feature.md`:
- Around line 1280-1294: The smoke test instructions in Step 5 (Verify
/artifacts list after upload) contain three consecutive sentences that all begin
with "Press" (Press `Enter`, Press `c`, Press `Esc`), creating a repetitive
structure that impacts readability. Reword one or two of these instructions to
use alternative action verbs such as "Use", "Hit", or "Select" while keeping the
exact same functionality and parenthetical explanations. This will improve the
flow and readability of the technical documentation without changing the meaning
or clarity of the steps.
In `@src/commands/artifacts/__tests__/scanner.test.ts`:
- Around line 102-138: Add a new test case after the existing test for parsing
url/id/expires from array-form tool_result content. The new test should verify
that extractArtifacts handles malformed array chunks gracefully by including
non-string and non-object entries (such as null or numeric values like 42)
within the tool_result content array. The test should verify that the extraction
function does not throw an error when encountering these invalid entries and
either skips them safely or correctly processes only the valid content chunks.
In `@src/commands/artifacts/ArtifactsMenu.tsx`:
- Around line 38-40: The setClipboard call in ArtifactsMenu.tsx uses a Promise
chain with .then() which should be refactored to use async/await for better
readability. Replace the current .then() pattern with an async IIFE (Immediately
Invoked Function Expression) that wraps the setClipboard call, await the result
into a variable, and then perform the same conditional check on raw and
process.stdout.write call within the async function body. This maintains the
original behavior while using the more modern async/await syntax.
In `@src/commands/artifacts/scanner.ts`:
- Around line 30-35: The code uses unsafe type casts with `as` to access
properties like `b.id`, `b.input`, and `b.is_error` on a `Record<string,
unknown>` type without proper type narrowing. Create explicit type guard
functions such as `isArtifactToolUseBlock` and `isToolResultBlock` that return
true when the block has the expected type and name properties, then use these
guards to properly narrow the types before accessing properties. Replace all the
unsafe `as` casts where `b.id`, `b.input`, and `b.is_error` are accessed with
calls to these type guard functions to ensure type safety without forcing casts.
🪄 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: 52ebc393-aa77-479a-8a26-67f3dd7de836
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
CLAUDE.mddocs/superpowers/plans/2026-06-20-artifacts-feature.mdpackages/builtin-tools/src/index.tspackages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.tspackages/builtin-tools/src/tools/ArtifactTool/__tests__/ArtifactTool.test.tspackages/builtin-tools/src/tools/ArtifactTool/__tests__/client.test.tspackages/builtin-tools/src/tools/ArtifactTool/client.tspackages/builtin-tools/src/tools/ArtifactTool/config.tspackages/builtin-tools/src/tools/ArtifactTool/prompt.tspackages/cloud-artifacts/.dev.vars.examplepackages/cloud-artifacts/.gitignorepackages/cloud-artifacts/README.mdpackages/cloud-artifacts/package.jsonpackages/cloud-artifacts/scripts/setup.shpackages/cloud-artifacts/scripts/test.shpackages/cloud-artifacts/src/index.tspackages/cloud-artifacts/tsconfig.jsonpackages/cloud-artifacts/wrangler.tomlsrc/commands.tssrc/commands/artifacts/ArtifactsMenu.tsxsrc/commands/artifacts/__tests__/scanner.test.tssrc/commands/artifacts/artifacts.tsxsrc/commands/artifacts/index.tssrc/commands/artifacts/scanner.tssrc/skills/bundled/index.tssrc/skills/bundled/useArtifacts.tssrc/tools.ts
Comment on lines
+913
to
+998
| ```typescript | ||
| // src/commands/artifacts/scanner.ts | ||
| import { basename } from 'path' | ||
| import type { Message } from 'src/types/message.js' | ||
|
|
||
| export type ArtifactInfo = { | ||
| toolUseId: string | ||
| filePath: string | ||
| basename: string | ||
| hash?: string | ||
| url?: string | ||
| expiresAt?: string | ||
| rawContent: string | ||
| isError: boolean | ||
| } | ||
|
|
||
| const URL_REGEX = /https?:\/\/\S+\.html\b/ | ||
| const ID_REGEX = /\bid:\s*([A-Za-z0-9_-]+)/ | ||
| const EXPIRES_REGEX = /\bexpires:\s*([0-9T:.Z+-]+)/ | ||
|
|
||
| export function extractArtifacts(messages: Message[]): ArtifactInfo[] { | ||
| const results: ArtifactInfo[] = [] | ||
|
|
||
| for (const message of messages) { | ||
| if (message.role !== 'assistant') continue | ||
| const content = message.content | ||
| if (!Array.isArray(content)) continue | ||
|
|
||
| for (const block of content) { | ||
| if (typeof block !== 'object' || block === null) continue | ||
| if (block.type !== 'tool_use') continue | ||
| if (block.name !== 'artifact') continue | ||
|
|
||
| const toolUseId = block.id | ||
| const input = block.input as { file_path?: string } | undefined | ||
| const filePath = input?.file_path ?? '<unknown>' | ||
|
|
||
| const resultBlock = findToolResult(messages, toolUseId) | ||
| if (!resultBlock) continue | ||
|
|
||
| const rawContent = | ||
| typeof resultBlock.content === 'string' | ||
| ? resultBlock.content | ||
| : Array.isArray(resultBlock.content) | ||
| ? resultBlock.content.map((c) => (typeof c === 'string' ? c : 'text' in c ? c.text : '')).join('') | ||
| : '' | ||
|
|
||
| const isError = resultBlock.is_error === true | ||
| const urlMatch = rawContent.match(URL_REGEX) | ||
| const idMatch = rawContent.match(ID_REGEX) | ||
| const expiresMatch = rawContent.match(EXPIRES_REGEX) | ||
|
|
||
| results.push({ | ||
| toolUseId, | ||
| filePath, | ||
| basename: basename(filePath), | ||
| hash: idMatch?.[1], | ||
| url: urlMatch?.[0], | ||
| expiresAt: expiresMatch?.[1], | ||
| rawContent, | ||
| isError, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // newest first | ||
| return results.reverse() | ||
| } | ||
|
|
||
| function findToolResult( | ||
| messages: Message[], | ||
| toolUseId: string, | ||
| ): { content: unknown; is_error?: boolean } | null { | ||
| for (const message of messages) { | ||
| if (message.role !== 'user') continue | ||
| const content = message.content | ||
| if (!Array.isArray(content)) continue | ||
| for (const block of content) { | ||
| if (typeof block !== 'object' || block === null) continue | ||
| if (block.type !== 'tool_result') continue | ||
| if (block.tool_use_id !== toolUseId) continue | ||
| return { content: block.content, is_error: block.is_error } | ||
| } | ||
| } | ||
| return null | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual Message type shape and URL_REGEX in the implementation
# Check Message type definition
echo "=== Message type shape ==="
rg -A 20 'type Message' src/types/message.ts --type ts
# Check actual scanner.ts implementation
echo -e "\n=== Actual scanner.ts URL_REGEX ==="
rg 'URL_REGEX|ID_REGEX|EXPIRES_REGEX' src/commands/artifacts/scanner.ts --type ts -A 1
# Check actual extractArtifacts implementation
echo -e "\n=== extractArtifacts property access ==="
rg 'message\.(role|type|message|content)' src/commands/artifacts/scanner.ts --type ts -C 2Repository: claude-code-best/claude-code
Length of output: 882
Fix scanner.ts code snippet in the plan—it doesn't match the actual implementation.
The plan shows URL_REGEX = /https?:\/\/\S+\.html\b/ and property access as message.role with message.content, but the actual implementation uses:
- URL_REGEX:
/https?:\/\/[^\s)"',]+\.html\b/(more restrictive character class) - Property access:
message.typeandmessage.message?.content
Update lines 929–998 to match the actual scanner.ts implementation to prevent confusion when developers follow the plan.
🤖 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/superpowers/plans/2026-06-20-artifacts-feature.md` around lines 913 -
998, The code snippet in the plan document for scanner.ts does not match the
actual implementation and contains incorrect property access and regex patterns.
Update the code snippet shown in lines 913 through 998 of the plan to match the
actual scanner.ts implementation: change the URL_REGEX pattern from
/https?:\/\/\S+\.html\b/ to use the more restrictive character class
/https?:\/\/[^\s)"',]+\.html\b/, and update all property access from
message.role to message.type and from message.content to
message.message?.content throughout the extractArtifacts and findToolResult
functions to ensure developers following the plan will see the correct
implementation details.
Comment on lines
+16
to
+18
| file_path: z | ||
| .string() | ||
| .describe('Absolute path to a local HTML file to upload.'), |
Contributor
There was a problem hiding this comment.
Enforce the absolute-path contract for file_path.
file_path is described as absolute, but the current schema accepts relative paths, which makes behavior cwd-dependent. Validate absolute paths explicitly.
Suggested fix
+import { isAbsolute } from 'path'
import { stat, readFile } from 'fs/promises'
import { z } from 'zod/v4'
@@
file_path: z
.string()
+ .refine((p) => isAbsolute(p), 'must be an absolute path')
.describe('Absolute path to a local HTML file to upload.'),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| file_path: z | |
| .string() | |
| .describe('Absolute path to a local HTML file to upload.'), | |
| file_path: z | |
| .string() | |
| .refine((p) => isAbsolute(p), 'must be an absolute path') | |
| .describe('Absolute path to a local HTML file to upload.'), |
🤖 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 `@packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts` around lines
16 - 18, The file_path field in the ArtifactTool schema is described as
requiring an absolute path but the current Zod validation does not enforce this
constraint, allowing relative paths that would be cwd-dependent. Add explicit
validation to the file_path field's schema definition to ensure only absolute
paths are accepted, using Zod's refine or superRefine method to validate that
the provided path is an absolute path before accepting it.
Comment on lines
+22
to
+29
| const response = await fetch(url, { | ||
| method: 'POST', | ||
| headers: { | ||
| Authorization: `Bearer ${params.token}`, | ||
| 'Content-Type': 'text/html', | ||
| }, | ||
| body: params.html, | ||
| }) |
Contributor
There was a problem hiding this comment.
Add a request timeout for fetch in upload flow.
Line 22 performs an unbounded network call; transient stalls can block the tool indefinitely.
Suggested patch
- const response = await fetch(url, {
- method: 'POST',
- headers: {
- Authorization: `Bearer ${params.token}`,
- 'Content-Type': 'text/html',
- },
- body: params.html,
- })
+ const controller = new AbortController()
+ const timeout = setTimeout(() => controller.abort(), 15_000)
+ const response = await fetch(url, {
+ method: 'POST',
+ headers: {
+ Authorization: `Bearer ${params.token}`,
+ 'Content-Type': 'text/html',
+ },
+ body: params.html,
+ signal: controller.signal,
+ }).finally(() => clearTimeout(timeout))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const response = await fetch(url, { | |
| method: 'POST', | |
| headers: { | |
| Authorization: `Bearer ${params.token}`, | |
| 'Content-Type': 'text/html', | |
| }, | |
| body: params.html, | |
| }) | |
| const controller = new AbortController() | |
| const timeout = setTimeout(() => controller.abort(), 15_000) | |
| const response = await fetch(url, { | |
| method: 'POST', | |
| headers: { | |
| Authorization: `Bearer ${params.token}`, | |
| 'Content-Type': 'text/html', | |
| }, | |
| body: params.html, | |
| signal: controller.signal, | |
| }).finally(() => clearTimeout(timeout)) |
🤖 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 `@packages/builtin-tools/src/tools/ArtifactTool/client.ts` around lines 22 -
29, The fetch call in the upload flow is missing a timeout mechanism, which can
cause the tool to hang indefinitely if the network stalls. Add a request timeout
by creating an AbortController before the fetch call, setting a timeout that
aborts the controller after a reasonable duration (e.g., 30 seconds), and
passing the abort controller's signal in the fetch options object alongside the
method, headers, and body properties. This will ensure the fetch operation fails
gracefully if it exceeds the timeout threshold instead of blocking indefinitely.
Comment on lines
+5
to
+11
| export const ARTIFACTS_DEFAULT_TOKEN = 'claude-code-best' | ||
| export const ARTIFACTS_DEFAULT_URL = | ||
| 'https://cloud-artifacts.claude-code-best.win' | ||
|
|
||
| export function getArtifactsToken(): string { | ||
| return process.env.CLAUDE_ARTIFACTS_TOKEN ?? ARTIFACTS_DEFAULT_TOKEN | ||
| } |
Contributor
There was a problem hiding this comment.
Remove the hardcoded default upload token (Line 5) and require explicit configuration.
A static fallback token makes the upload credential effectively public and weakens the Worker’s bearer-auth boundary. Fail fast when CLAUDE_ARTIFACTS_TOKEN is unset instead of silently using a known default.
Suggested fix
-export const ARTIFACTS_DEFAULT_TOKEN = 'claude-code-best'
export const ARTIFACTS_DEFAULT_URL =
'https://cloud-artifacts.claude-code-best.win'
export function getArtifactsToken(): string {
- return process.env.CLAUDE_ARTIFACTS_TOKEN ?? ARTIFACTS_DEFAULT_TOKEN
+ const token = process.env.CLAUDE_ARTIFACTS_TOKEN
+ if (!token) {
+ throw new Error('CLAUDE_ARTIFACTS_TOKEN is required')
+ }
+ return token
}🤖 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 `@packages/builtin-tools/src/tools/ArtifactTool/config.ts` around lines 5 - 11,
The getArtifactsToken function currently falls back to a hardcoded
ARTIFACTS_DEFAULT_TOKEN value when the CLAUDE_ARTIFACTS_TOKEN environment
variable is not set, which weakens security by using a publicly known
credential. Remove the ARTIFACTS_DEFAULT_TOKEN constant definition and modify
the getArtifactsToken function to throw an error (e.g., with a descriptive
message about the missing required environment variable) instead of returning a
default value, ensuring the credential must be explicitly configured and the
application fails fast if it is not provided.
Comment on lines
+4
to
+5
| _.log | ||
| npm-debug.log_ |
Contributor
There was a problem hiding this comment.
Fix malformed wildcard entries in .gitignore.
Several lines use _ or escaped \* where wildcard * is expected, so common artifacts won’t be ignored as intended.
Suggested patch (examples)
-_.log
-npm-debug.log_
+*.log
+npm-debug.log*
@@
-report.[0-9]_.[0-9]_.[0-9]_.[0-9]_.json
+report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
@@
-_.pid
-_.seed
-\*.pid.lock
+*.pid
+*.seed
+*.pid.lock
@@
-\*.lcov
+*.lcov
@@
-\*.tsbuildinfo
+*.tsbuildinfo
@@
-\*.tgz
+*.tgz
@@
-.pnp.\*
+.pnp.*Also applies to: 13-13, 18-20, 29-29, 62-62, 89-89, 159-159
🤖 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 `@packages/cloud-artifacts/.gitignore` around lines 4 - 5, The .gitignore file
contains malformed wildcard patterns using underscores (_) or escaped asterisks
(\*) instead of the correct wildcard character (*). Fix all instances by
replacing underscore wildcards with asterisks: change patterns like _.log to
*.log and npm-debug.log_ to npm-debug.log*. Apply the same fix to all affected
lines mentioned (lines 4-5, 13, 18-20, 29, 62, 89, 159) to ensure common
artifacts are properly ignored.
Comment on lines
+13
to
+14
| WORKER_URL="${WORKER_URL:-https://cloud-artifacts.claude-code-best.win}" | ||
| TOKEN="${TOKEN:-claude-code-best}" |
Contributor
There was a problem hiding this comment.
Remove default bearer token from the script.
Line 14 hardcodes a usable token fallback. In a public repo, this weakens auth guarantees and can enable unauthorized uploads if that value is active anywhere.
Suggested patch
-WORKER_URL="${WORKER_URL:-https://cloud-artifacts.claude-code-best.win}"
-TOKEN="${TOKEN:-claude-code-best}"
+WORKER_URL="${WORKER_URL:-https://cloud-artifacts.claude-code-best.win}"
+: "${TOKEN:?TOKEN environment variable is required}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| WORKER_URL="${WORKER_URL:-https://cloud-artifacts.claude-code-best.win}" | |
| TOKEN="${TOKEN:-claude-code-best}" | |
| WORKER_URL="${WORKER_URL:-https://cloud-artifacts.claude-code-best.win}" | |
| : "${TOKEN:?TOKEN environment variable is required}" |
🤖 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 `@packages/cloud-artifacts/scripts/test.sh` around lines 13 - 14, The TOKEN
variable in the test.sh script has a hardcoded default value that exposes a
usable bearer token in the public repository, creating a security vulnerability.
Remove the default fallback value from the TOKEN variable assignment so that the
token must be explicitly provided via the environment variable without any
hardcoded default. This ensures the token cannot be accessed from the repository
itself and must be supplied at runtime.
Comment on lines
+149
to
+157
| if [[ -n "$RANDOM_ID" ]]; then | ||
| echo "--- 验证随机 ID 文件存在: 7d/$RANDOM_ID.html ---" | ||
| npx wrangler r2 object get "cloud-artifacts/7d/$RANDOM_ID.html" --remote --file "$TMP/got.html" 2>&1 | tail -5 | ||
| echo "下载内容：" ; cat "$TMP/got.html" 2>/dev/null | ||
| fi | ||
|
|
||
| echo "--- 验证覆盖后 test-artifact-v1 是 v2 内容 ---" | ||
| npx wrangler r2 object get "cloud-artifacts/30d/test-artifact-v1.html" --remote --file "$TMP/got2.html" 2>&1 | tail -5 | ||
| echo "下载内容：" ; cat "$TMP/got2.html" 2>/dev/null |
Contributor
There was a problem hiding this comment.
Use configurable bucket name in verification commands.
Line 151 and Line 156 hardcode cloud-artifacts, but packages/cloud-artifacts/scripts/setup.sh supports BUCKET override (Line 4). This breaks tests for non-default deployments.
Suggested patch
+BUCKET="${BUCKET:-cloud-artifacts}"
@@
- npx wrangler r2 object get "cloud-artifacts/7d/$RANDOM_ID.html" --remote --file "$TMP/got.html" 2>&1 | tail -5
+ npx wrangler r2 object get "$BUCKET/7d/$RANDOM_ID.html" --remote --file "$TMP/got.html" 2>&1 | tail -5
@@
-npx wrangler r2 object get "cloud-artifacts/30d/test-artifact-v1.html" --remote --file "$TMP/got2.html" 2>&1 | tail -5
+npx wrangler r2 object get "$BUCKET/30d/test-artifact-v1.html" --remote --file "$TMP/got2.html" 2>&1 | tail -5📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [[ -n "$RANDOM_ID" ]]; then | |
| echo "--- 验证随机 ID 文件存在: 7d/$RANDOM_ID.html ---" | |
| npx wrangler r2 object get "cloud-artifacts/7d/$RANDOM_ID.html" --remote --file "$TMP/got.html" 2>&1 | tail -5 | |
| echo "下载内容：" ; cat "$TMP/got.html" 2>/dev/null | |
| fi | |
| echo "--- 验证覆盖后 test-artifact-v1 是 v2 内容 ---" | |
| npx wrangler r2 object get "cloud-artifacts/30d/test-artifact-v1.html" --remote --file "$TMP/got2.html" 2>&1 | tail -5 | |
| echo "下载内容：" ; cat "$TMP/got2.html" 2>/dev/null | |
| BUCKET="${BUCKET:-cloud-artifacts}" | |
| if [[ -n "$RANDOM_ID" ]]; then | |
| echo "--- 验证随机 ID 文件存在: 7d/$RANDOM_ID.html ---" | |
| npx wrangler r2 object get "$BUCKET/7d/$RANDOM_ID.html" --remote --file "$TMP/got.html" 2>&1 | tail -5 | |
| echo "下载内容：" ; cat "$TMP/got.html" 2>/dev/null | |
| fi | |
| echo "--- 验证覆盖后 test-artifact-v1 是 v2 内容 ---" | |
| npx wrangler r2 object get "$BUCKET/30d/test-artifact-v1.html" --remote --file "$TMP/got2.html" 2>&1 | tail -5 | |
| echo "下载内容：" ; cat "$TMP/got2.html" 2>/dev/null |
🤖 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 `@packages/cloud-artifacts/scripts/test.sh` around lines 149 - 157, Replace the
hardcoded bucket name "cloud-artifacts" in the R2 object get commands with a
BUCKET variable reference to match the setup.sh convention. In the wrangler r2
object get command for the 7d path with RANDOM_ID and the 30d path with
test-artifact-v1, change the hardcoded "cloud-artifacts/" prefix to use the
BUCKET environment variable (typically referenced as $BUCKET or ${BUCKET}) to
support non-default bucket deployments.
Comment on lines
+5
to
+9
| declare global { | ||
| interface Env { | ||
| TOKEN: string | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== worker type files =="
fd -a "worker-configuration.d.ts" packages/cloud-artifacts || true
echo "== index.ts typed symbols =="
rg -n "ExportedHandler|interface Env|\\bBUCKET\\b|\\bMAX_BYTES\\b|\\bDEFAULT_TTL_DAYS\\b|\\bPUBLIC_URL\\b|\\bTOKEN\\b" \
packages/cloud-artifacts/src/index.ts packages/cloud-artifacts/tsconfig.json
echo "== if worker-configuration exists, inspect declared bindings =="
for f in $(fd -a "worker-configuration.d.ts" packages/cloud-artifacts); do
echo "--- $f ---"
rg -n "interface Env|BUCKET|MAX_BYTES|DEFAULT_TTL_DAYS|PUBLIC_URL|ExportedHandler" "$f" || true
doneRepository: claude-code-best/claude-code
Length of output: 1219
🏁 Script executed:
cat -n packages/cloud-artifacts/src/index.ts | head -50Repository: claude-code-best/claude-code
Length of output: 1993
🏁 Script executed:
cat -n packages/cloud-artifacts/tsconfig.jsonRepository: claude-code-best/claude-code
Length of output: 629
🏁 Script executed:
cd packages/cloud-artifacts && cat -n package.json | grep -A 5 -B 5 "tsc\|typecheck"Repository: claude-code-best/claude-code
Length of output: 54
🏁 Script executed:
fd -a "worker-configuration.d.ts" .Repository: claude-code-best/claude-code
Length of output: 54
🏁 Script executed:
rg -n "ExportedHandler" packages/cloud-artifacts/src/index.ts -B 10 -A 2Repository: claude-code-best/claude-code
Length of output: 471
🏁 Script executed:
rg "import.*ExportedHandler" packages/ -t tsRepository: claude-code-best/claude-code
Length of output: 54
🏁 Script executed:
cat -n packages/cloud-artifacts/src/index.ts | tail -80Repository: claude-code-best/claude-code
Length of output: 3073
🏁 Script executed:
rg "interface Env|BUCKET|MAX_BYTES|DEFAULT_TTL_DAYS|PUBLIC_URL" packages/cloud-artifacts/src/index.ts -A 2Repository: claude-code-best/claude-code
Length of output: 891
🏁 Script executed:
cd packages/cloud-artifacts && bunx tsc --noEmit 2>&1 | head -40Repository: claude-code-best/claude-code
Length of output: 115
🏁 Script executed:
find . -name "worker-configuration.d.ts" -o -name "*worker*" -type f | grep -E "\.(d\.)?ts$" | head -20Repository: claude-code-best/claude-code
Length of output: 328
🏁 Script executed:
rg "cloudflare|workers-types|ExportedHandler" packages/ --type ts -lRepository: claude-code-best/claude-code
Length of output: 199
🏁 Script executed:
cat packages/cloud-artifacts/worker-configuration.d.ts 2>/dev/null || echo "NOT FOUND"Repository: claude-code-best/claude-code
Length of output: 82
🏁 Script executed:
ls -la packages/cloud-artifacts/Repository: claude-code-best/claude-code
Length of output: 688
🏁 Script executed:
cd packages/cloud-artifacts && npx tsc --noEmit 2>&1 || trueRepository: claude-code-best/claude-code
Length of output: 1600
🏁 Script executed:
cd packages/cloud-artifacts && cat wrangler.tomlRepository: claude-code-best/claude-code
Length of output: 386
🏁 Script executed:
rg "ExportedHandler|cloudflare" . --type ts -B 2 -A 2 | head -40Repository: claude-code-best/claude-code
Length of output: 2306
Create worker-configuration.d.ts to resolve missing type bindings blocking typecheck.
The build currently has 21 TypeScript errors because tsconfig.json includes worker-configuration.d.ts (line 16), but the file does not exist. This causes:
- TS2304:
ExportedHandlercannot be found (line 30) - TS2339: Properties
BUCKET,MAX_BYTES,DEFAULT_TTL_DAYS,PUBLIC_URLdo not exist onEnv(lines 39, 66, 75, 91, 103, 109) - Missing globals:
Request,Response,URL,Headers
Create packages/cloud-artifacts/worker-configuration.d.ts with:
import type { ExportedHandler } from '`@cloudflare/workers-types`'
declare global {
interface Env {
TOKEN: string
BUCKET: R2Bucket
MAX_BYTES: string
DEFAULT_TTL_DAYS: string
PUBLIC_URL: string
}
}
export type {}
Then remove the duplicate declare global { interface Env { TOKEN } } from src/index.ts (lines 5-9), as it will be redundant.
As per coding guidelines, TypeScript strict mode must pass with zero errors.
🤖 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 `@packages/cloud-artifacts/src/index.ts` around lines 5 - 9, Create a new file
worker-configuration.d.ts in the packages/cloud-artifacts directory containing
the type definitions for ExportedHandler from `@cloudflare/workers-types` and the
complete Env interface with all required properties (TOKEN, BUCKET, MAX_BYTES,
DEFAULT_TTL_DAYS, PUBLIC_URL). Then remove the duplicate declare global
interface Env block from src/index.ts that only contains TOKEN, since it will
now be defined in the centralized worker-configuration.d.ts file and
tsconfig.json already references it.
Sources: Coding guidelines, Pipeline failures
Comment on lines
+89
to
+92
| // 覆盖：先删所有 ttl prefix 下可能的旧 key（R2 delete 不存在的 key 不报错） | ||
| await Promise.all( | ||
| TTL_PREFIXES.map(p => env.BUCKET.delete(`${p}/${id}.html`)), | ||
| ) |
Contributor
There was a problem hiding this comment.
Avoid deleting existing artifact before upload validation completes.
Line 89 deletes old keys before full body validation (Line 97/98). If content-length is absent/misreported and the body is oversized, this path returns 413 after deletion, causing data loss for hash-based updates.
Suggested reorder to make overwrite safe
if (hashParam !== null) {
if (!HASH_PATTERN.test(hashParam)) {
return json({ error: 'invalid_hash' }, 400)
}
id = hashParam
- // 覆盖：先删所有 ttl prefix 下可能的旧 key（R2 delete 不存在的 key 不报错）
- await Promise.all(
- TTL_PREFIXES.map(p => env.BUCKET.delete(`${p}/${id}.html`)),
- )
} else {
id = nanoid(21)
}
const body = await req.arrayBuffer()
if (body.byteLength > maxBytes) {
return json({ error: 'payload_too_large' }, 413)
}
const key = `${ttl}d/${id}.html`
await env.BUCKET.put(key, body, {
httpMetadata: { contentType: HTML_CONTENT_TYPE },
})
+
+ // hash 覆盖：写成功后再删其它前缀，避免校验/写入失败时丢失旧数据
+ if (hashParam !== null) {
+ await Promise.all(
+ TTL_PREFIXES.filter(p => p !== `${ttl}d`).map(p =>
+ env.BUCKET.delete(`${p}/${id}.html`),
+ ),
+ )
+ }Also applies to: 97-100, 102-105
🧰 Tools
🪛 GitHub Actions: CI / 0_ci.txt
[error] 91-91: TypeScript error TS2339: Property 'BUCKET' does not exist on type 'Env'.
🪛 GitHub Actions: CI / ci
[error] 91-91: tsc (TypeScript) TS2339: Property 'BUCKET' does not exist on type 'Env'.
🤖 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 `@packages/cloud-artifacts/src/index.ts` around lines 89 - 92, The deletion of
old artifact keys using TTL_PREFIXES.map and env.BUCKET.delete occurs before
full body validation is complete. Move this deletion block to after the body
validation checks pass (after the content-length validation and body reading are
confirmed successful around lines 97-100). This ensures that if validation fails
and returns an error response, the original artifact data remains intact and is
not lost. Keep the deletion as part of the successful upload flow, only deleting
old keys after confirming the new upload is valid.
Comment on lines
+46
to
+51
| .map(c => | ||
| typeof c === 'string' | ||
| ? c | ||
| : 'text' in c | ||
| ? (c as { text: string }).text | ||
| : '', |
Contributor
There was a problem hiding this comment.
Guard non-object chunks before using 'text' in c.
If tool_result.content includes null, numbers, or booleans, Line 49 throws at runtime and /artifacts can crash. Narrow to object + non-null before checking text.
🐛 Proposed fix
- .map(c =>
- typeof c === 'string'
- ? c
- : 'text' in c
- ? (c as { text: string }).text
- : '',
- )
+ .map(c => {
+ if (typeof c === 'string') return c
+ if (
+ typeof c === 'object' &&
+ c !== null &&
+ 'text' in c &&
+ typeof (c as { text?: unknown }).text === 'string'
+ ) {
+ return (c as { text: string }).text
+ }
+ return ''
+ })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/artifacts/scanner.ts` around lines 46 - 51, The map function
processing chunks from tool_result.content does not guard against non-object
values like null, numbers, or booleans before using the 'text' in c operator,
which will throw a runtime error. Add a type guard to check if c is an object
and not null before attempting to check for the 'text' property using the 'in'
operator. This ensures that only objects are evaluated for the text property,
and all other types (including null) are safely handled as empty strings.
…figuration.d.ts The wrangler-generated worker-configuration.d.ts is gitignored, causing CI to fail with missing ExportedHandler/Env/R2Bucket types. This file provides minimal stubs for all Cloudflare Workers types used by the artifact upload Worker. Co-Authored-By: glm-5-turbo <zai-org@claude-code-best.win>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/remote-control-server/src/__tests__/routes.test.ts (1)
3-7: ⚡ Quick winConsider extracting
resJsonto a shared test utility module.The same helper is defined in both
middleware.test.tsandroutes.test.ts. Extracting it to a shared test utility (e.g.,packages/remote-control-server/src/__tests__/utils.tsorpackages/remote-control-server/src/__tests__/helpers.ts) would follow DRY principles and make future maintenance easier.♻️ Example implementation
Create
packages/remote-control-server/src/__tests__/test-utils.ts:// res.json() returns Promise<unknown> in strict mode; this helper narrows for test assertions export function resJson(res: Response) { return res.json() as Promise<any> }Then import in both test files:
import { resJson } from './test-utils'🤖 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 `@packages/remote-control-server/src/__tests__/routes.test.ts` around lines 3 - 7, The resJson helper function is duplicated in both middleware.test.ts and routes.test.ts. Create a new shared test utility file (e.g., test-utils.ts in the __tests__ directory), move the resJson function there as an exported function, remove the duplicate definitions from both middleware.test.ts and routes.test.ts, and add import statements in both files to import resJson from the new shared utility module.
🤖 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.
Nitpick comments:
In `@packages/remote-control-server/src/__tests__/routes.test.ts`:
- Around line 3-7: The resJson helper function is duplicated in both
middleware.test.ts and routes.test.ts. Create a new shared test utility file
(e.g., test-utils.ts in the __tests__ directory), move the resJson function
there as an exported function, remove the duplicate definitions from both
middleware.test.ts and routes.test.ts, and add import statements in both files
to import resJson from the new shared utility module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39d4804a-80b2-43cb-8653-561d8cce9302
📒 Files selected for processing (2)
packages/remote-control-server/src/__tests__/middleware.test.tspackages/remote-control-server/src/__tests__/routes.test.ts
Previously the per-query cleanup mutated messagesForQuery entries in place via `delete msg.toolUseResult`. Those entries are references shared with mutableMessages (UI state), so the delete stripped the field from the live message object. The next query can start within milliseconds of tool_result creation — before the React UI commit lands — so UserToolSuccessMessage's `!message.toolUseResult` check returned null and tool.renderToolResultMessage was never called, leaving tool-result rows blank. Map to a stripped copy instead so mutableMessages keeps the original for the UI. Downstream API transformations (applyToolResultBudget, snip, microcompact) already build new arrays via .map(), so they compose cleanly with this copy. Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
Deferred tools (shouldDefer: true) are invoked via SearchExtraTools →
ExecuteExtraTool, so their tool_result rows used to render blank —
the UI looked up ExecuteExtraTool, which had no renderToolResultMessage,
and returned null. Add a generic delegation in ExecuteTool that forwards
renderToolResultMessage to the inner tool when it defines one, unwrapping
the { result, tool_name } envelope and the params from the input shape.
All 28 deferred tools can now render their own UI by defining
renderToolResultMessage.
For ArtifactTool specifically, render the uploaded URL as an OSC 8
hyperlink (Link component) in warning color so it's visually prominent,
with the expiry timestamp on a second line and a separate error branch.
Also add `error: z.string().optional()` to outputSchema — zod's default
strip mode was dropping the field, so error states never reached the UI.
Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
The previous stub file (2e29e36) wrapped `interface Env` in `declare global { ... }`, but the file has no top-level import/export so it's a script, not a module. TS2669 forbids `declare global` in scripts, and in .d.ts files that error is silently swallowed — so the Env stubs were never merged into the global scope. Locally typecheck passed only because worker-configuration.d.ts (gitignored) provided Env separately; in CI / fresh clones, `BUCKET`, `MAX_BYTES`, `DEFAULT_TTL_DAYS`, `PUBLIC_URL` were all missing on Env. Drop the wrapper. Top-level `interface Env` in a script .d.ts is already global ambient and merges with worker-configuration.d.ts via interface declaration merging, so both environments typecheck cleanly. Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit