Skip to content

Revert "Fix Content-Disposition header missing 'attachment;' prefix (#13093)"#13733

Merged
comfyanonymous merged 1 commit intomasterfrom
glary/revert-pr-13093
May 6, 2026
Merged

Revert "Fix Content-Disposition header missing 'attachment;' prefix (#13093)"#13733
comfyanonymous merged 1 commit intomasterfrom
glary/revert-pr-13093

Conversation

@guill
Copy link
Copy Markdown
Member

@guill guill commented May 6, 2026

PR Created by the Glary-Bot Agent


Reverts #13093 (commit ea6880b04b88629b9dd07774298bdffea6923f9b).

Why

PR #13093 added attachment; to all four Content-Disposition headers in the view_image endpoint of server.py. Post-merge feedback reports this regresses existing UI behavior:

  • PR comment: the Open Image context-menu option (which previously opened the image inline in a new tab) now forces a download, making it functionally identical to the existing Save Image option.
  • This results in two context-menu entries doing the same thing and removes a workflow many users still rely on.
  • Reviewers in the commit thread suggested the original change should live on a Cloud-specific branch rather than core ComfyUI.

Changes

Pure git revert of ea6880b — restores the four Content-Disposition headers in server.py to filename="...". No other files touched.

 server.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Verification

Reviewer notes (from automated review)

A pre-merge code review flagged that the restored Content-Disposition: filename="..." is technically not a fully-formed HTTP value — the spec wants either inline or attachment as the disposition type. That predates #13093 and is intentionally left as-is here to keep this PR a pure revert. If maintainers want a properly-typed header (e.g. inline; filename="..." for preview routes, attachment; filename="..." for download-only routes), that's worth doing as a follow-up alongside /view test coverage.

Requested by user in the #glary Slack channel.

Screenshots

Browser renders /view image inline in a new tab — pre-#13093 behavior restored after revert

@guill guill marked this pull request as ready for review May 6, 2026 17:01
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 642bcd3d-9103-48f3-898e-4f9f27fdf9d3

📥 Commits

Reviewing files that changed from the base of the PR and between 9c34f5f and 60d3785.

📒 Files selected for processing (1)
  • server.py

📝 Walkthrough

Walkthrough

This PR introduces a new /api/jobs endpoint for retrieving and filtering jobs with pagination and sorting capabilities, pulling from running, queued, and history data. The endpoint validates input parameters including status, sort_by, sort_order, limit, and offset, returning structured job data with pagination metadata. Additionally, the PR modifies Content-Disposition headers across multiple image and view delivery endpoints, changing from an attachment directive to filename-only format to alter how browsers present these assets.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the PR's main change: reverting a previous commit that added 'attachment;' to Content-Disposition headers.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the rationale for the revert, the specific changes, and verification steps performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@glary-bot
Copy link
Copy Markdown

glary-bot Bot commented May 6, 2026

Thanks for the review @coderabbitai — noting no actionable comments were generated. The docstring-coverage warning is a pre-existing project-wide threshold and unrelated to this revert (which only flips 4 header lines in server.py), so leaving it alone.

@comfyanonymous comfyanonymous merged commit 6bcd8b9 into master May 6, 2026
16 checks passed
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.

3 participants