Skip to content

feat(artifacts): prefer server-assembled bundle delivery for downloads#93

Open
riglar wants to merge 3 commits into
devfrom
feat/bundle-download
Open

feat(artifacts): prefer server-assembled bundle delivery for downloads#93
riglar wants to merge 3 commits into
devfrom
feat/bundle-download

Conversation

@riglar

@riglar riglar commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What

Artifact and HTML-report downloads now try a bundle-delivery path before the inline download:

  1. The CLI calls a manifest endpoint (GET /results/{uploadId}/artifacts-bundle?results=… or …/report-bundle).
  2. The API returns a signed manifest plus a URL to a delivery service that streams the ZIP straight from storage.
  3. The CLI relays the signed { manifest, sig } to that URL — no auth header, since the manifest is itself the signed access token — and streams the ZIP to disk.

Large downloads stream directly from storage instead of flowing through the API.

Compatibility

Fully backwards-compatible, no version negotiation:

  • Delivery not offered (older deployment / not configured) → manifest endpoint returns 501 → the CLI falls back to the existing inline download.
  • Anything else about the bundle path doesn't pan out (non-OK manifest, unexpected body, delivery-service error) → same graceful fallback. Definitive errors (e.g. not found) still surface through the inline path.
  • Only artifacts and the HTML report use bundle delivery; junit (a single small file) and allure (separate endpoint) keep their existing paths.

How

  • New shared tryBundleDownload helper in the API gateway; downloadArtifactsZip and the HTML branch of downloadReportGeneric call it first and fall back on false.
  • The inline download code paths are unchanged.

Testing

  • pnpm typecheck + pnpm lint clean.
  • pnpm test171 passing, including new coverage: streams from the bundle URL and skips the inline endpoint, sends no auth header to the (pre-signed) bundle URL, falls back to inline on 501, and uses bundle delivery for the HTML report. Existing inline-download tests unchanged.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

riglar and others added 2 commits July 23, 2026 15:01
Artifact and HTML-report downloads now try a bundle-delivery path first: the
API returns a signed manifest plus a URL to a delivery service that streams
the ZIP straight from storage, so large downloads don't flow through the API.
The client relays the signed { manifest, sig } to that URL (no auth header —
the manifest is the signed token) and streams the result to disk.

Falls back to the existing inline download automatically when bundle delivery
isn't offered (501) or anything about the path doesn't pan out, so behaviour
is unchanged on older deployments. Applies to artifacts and the HTML report;
junit and allure keep using their existing endpoints.

Adds a shared tryBundleDownload helper in the API gateway and unit tests
covering the bundle path, the no-auth relay, the 501 fallback, and the HTML
report.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Comment thread src/gateways/api-gateway.ts Outdated
Comment on lines +248 to +249
await this.streamResponseToFile(zipRes, destinationPath, operation);
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This final streamResponseToFile call is the one failure mode in tryBundleDownload that isn't converted to return false. Every other step (fetch manifest, json(), missing fields, bundle POST, !zipRes.ok) is guarded and falls back to inline. But streamResponseToFile throws on a mid-stream failure — a dropped/truncated connection during the ZIP download, or a null body on an otherwise-OK response (it explicitly throws when res.body === null, and pipeline propagates source-stream errors):

}
await this.streamResponseToFile(zipRes, destinationPath, operation);
return true;
},

Because both call sites invoke this as an if (await this.tryBundleDownload(...)) return; condition that sits before the inline try block, a throw here escapes past the inline /download fallback entirely — contradicting the PR's stated intent ("falls back silently to inline /download on 501 or any bundle-path failure") and leaving a partial file on disk. Wrapping the stream so it returns false restores the fallback (the inline path re-opens the destination with flags: 'w', so the partial file is truncated on retry):

Suggested change
await this.streamResponseToFile(zipRes, destinationPath, operation);
return true;
try {
await this.streamResponseToFile(zipRes, destinationPath, operation);
} catch {
return false;
}
return true;

streamResponseToFile threw past both tryBundleDownload call sites on a
mid-stream failure or null body, escaping the inline fallback and leaving
a partial file. Wrap it to return false like every other failure path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant