fix: use artifacts route for daemon downloads#434
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbfdda369c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function buildDaemonArtifactUrl(baseUrl: string, artifactId: string): string { | ||
| const normalizedBase = baseUrl.endsWith('/') ? baseUrl : `${baseUrl}/`; | ||
| return new URL(`upload/${encodeURIComponent(artifactId)}`, normalizedBase).toString(); | ||
| return new URL(`artifacts/${encodeURIComponent(artifactId)}`, normalizedBase).toString(); |
There was a problem hiding this comment.
Preserve daemon artifact download route compatibility
This change makes remote artifact downloads call /artifacts/:id, but the built-in daemon HTTP server still only serves artifact downloads on GET /upload/:id (see src/daemon/http-server.ts route checks around lines 409-410). For clients using --daemon-base-url against a direct daemon endpoint (as documented for remote HTTP daemon usage), commands that return artifacts (e.g., screenshots/recordings) will now fail with 404 during materialization.
Useful? React with 👍 / 👎.
dbfdda3 to
288707d
Compare
Summary
Change daemon artifact download URLs from
/upload/:artifactIdto/artifacts/:artifactIdso bridge routing can keep artifact downloads separate from upload endpoints.Updated the daemon HTTP download route and remote artifact download tests to use
/artifacts/:artifactId; no/upload/:artifactIddownload route or test coverage remains. Upload endpoints (POST /upload,/upload/preflight,/upload/finalize) are unchanged.Touched files: 4. Scope stayed within daemon artifact download behavior. Docs/skills were not updated because this is an internal route shape, not a CLI or workflow surface change.
Validation
pnpm formatpnpm check:quickpnpm check:unitrg -n "startsWith\('/upload/'\)|slice\('/upload/'|includes\('/upload/'\)|/agent-device/upload/|callGet\([^\n]*upload|GET.*upload|/upload/\$\{artifactId\}" src test