Fix false 'Build Scheduled' notification when parent multibranch is d…

[LakshyaBagani]

Fixes #16678

Description

When trying to trigger a build of a branch job in a disabled multibranch project, the UI incorrectly showed "Build Scheduled" notification even though the build was not actually scheduled. The queue rejected the build (returned null), but the code sent a 302 redirect which the JavaScript treated as success.

Changes

1. Updated isBuildable() method: Now checks if the parent ParameterizedJob (e.g., multibranch project) is disabled before allowing builds
2. Fixed error handling in doBuild(): Changed from returning 302 redirect to throwing 409 CONFLICT when schedule2() returns null
3. Fixed JavaScript response handling: Changed from checking rsp.ok (which includes 302) to checking rsp.status === 201 specifically

Testing done

Manual testing performed:

1. Created a Multibranch Pipeline project
2. Disabled the multibranch project via configuration page
3. Attempted to trigger a build on a branch job within the disabled multibranch
4. Before fix: UI showed "Build Scheduled" notification (incorrect)
5. After fix: UI shows "Build failed" notification (correct)
6. Verified browser console shows proper error response (409 CONFLICT)
7. Verified that builds work correctly when multibranch is enabled
8. Verified that builds work correctly for regular (non-multibranch) projects

Tested on: Jenkins 2.541-SNAPSHOT

Proposed changelog entries

• Fix false "Build Scheduled" notification when attempting to build branch jobs in disabled multibranch projects

Proposed changelog category

/label bug

Proposed upgrade guidelines

N/A

Submitter checklist

• The issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
  • Manual testing performed as described above. This is a UI/behavior fix that requires testing with actual multibranch projects.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
  • N/A - No new public APIs added
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
  • N/A - No deprecations added
• UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin. In particular, new or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
  • Only changed response status check, no inline JS or eval
• For dependency updates, there are links to external changelogs and, if possible, full differentials.
  • N/A - No dependency updates
• For new APIs and extension points, there is a link to at least one consumer.
  • N/A - No new APIs added

Desired reviewers

@timja @mawinter69 @daniel-beck

[mawinter69]

Things to check:
When you disable a MultiBranchProject, does this also hide the build button from the Build Buttoncolumn like for regular jobs?

Does it also work when you disable an organization folder?

[mawinter69]

A MultiBranchProject is not a ParameterizedJob but a ComputedFolder. So this check will never be true.

[LakshyaBagani]

You’re absolutely right, thanks for pointing that out.

A MultiBranchProject (and organization folders) are ComputedFolder implementations, not ParameterizedJobs, so the parent check I added will never be true for multibranch/organization folders and therefore doesn’t actually affect the build button visibility in those cases.

The real functional change in this PR is:

• Returning 409 CONFLICT from doBuild when schedule2(...).getItem() is null instead of redirecting with 302, and
• Updating the JS to only treat 201 as success.

That fixes the incorrect “Build Scheduled” notification when the build is not actually queued.

To keep this PR focused and correct, I’ll remove the ineffective ParameterizedJob parent check from isBuildable() and leave the PR scoped to the response/notification fix. If we also want to hide the build button for branch jobs when the multibranch (or org folder) is disabled, I agree that needs a different approach that works with ComputedFolder and could be handled in a follow‑up change.

[daniel-beck]

Why did you change both files? Wouldn't one of the changes be enough?

[LakshyaBagani]

@daniel-beck Good question. Either change alone would fix the issue:

• Only Java change: Returning 409 CONFLICT instead of 302 would make rsp.ok false, so the JS would show the error message.
• Only JS change: Checking rsp.status === 201 would ignore 302 redirects, so it wouldn't show success incorrectly.

I included both because:

1. The Java change uses the correct HTTP status code (409 CONFLICT for a rejected build)
2. The JS change is more precise (only treats actual creation as success)

If you prefer, I can remove one of them. Which would you prefer to keep?

[timja]

Both changes are unnecessary. The JS change is less likely to impact other code I'd expect, either change is probably ok though.

[daniel-beck]

Agree with Tim. HTTP response codes for this are a whole thing, and we'd just break someone's API use.

[LakshyaBagani]

Thanks for the feedback. I understand the concern about changing HTTP status codes potentially breaking API clients.

I propose using a custom response header instead, which is already used in Jenkins (e.g., X-Building, X-Jenkins-ValidateButton-Callback).

Proposed approach:

1. Backend: When schedule2() returns null, keep the 302 redirect (for API compatibility) but add a header:

   rsp.setHeader("X-Build-Scheduled", "false");
   rsp.sendRedirect(".");

   When successful, add:

   rsp.setHeader("X-Build-Scheduled", "true");
   rsp.sendRedirect(SC_CREATED, ...);

2. Frontend: Check the header instead of status code:

   const scheduled = rsp.headers.get("X-Build-Scheduled");
   if (scheduled === "true") {
     notificationBar.show(success, notificationBar.SUCCESS);
   } else {
     notificationBar.show(failure, notificationBar.ERROR);
   }

Benefits:

• Maintains API compatibility (keeps 302 redirect)
• Follows existing Jenkins patterns
• Explicit signal for the UI
• No breaking changes

If you prefer, I can keep only the JavaScript change (rsp.status === 201), though it's less precise. What do you think?

[timja]

You already have an explicit signal. 201 created that’s all you need

[LakshyaBagani]

Alternative (more detailed):

Thanks for clarifying. I understand now:

• 201 Created is already the explicit success signal
• The JavaScript change (rsp.status === 201) correctly uses that signal
• The Java change (409 CONFLICT) is unnecessary and could break API clients

I'll revert the Java change back to the original redirect behavior and keep only the JavaScript fix. This addresses the UI notification issue while maintaining API compatibility.

Updating the PR accordingly.

[daniel-beck]

Desired reviewers

… @daniel-beck

Why? I've shown no previous interest in this issue.

[timja]

LGTM, (untested but reviewed code, including in Java not from diff)

[MarkEWaite]

I built a local copy and confirmed that the "Build Now" button on the job page now reports an error instead of reporting that it scheduled the job.

However, when I click the arrow on the folder page that means "Build Now", it still reports that the job was scheduled, even though it was not scheduled.

I created the job as a multibranch Pipeline with the GitHub branch source for https://github.com/jenkinsci/implied-labels-plugin use a fine grained access token with read only access to all my public repositories. I disabled the job by appending the "/disable" to the URL of the folder and submitting the page.

Is it expected that there is a behavioral difference between those two forms of "Build Now"?

Job page

Results from clicking "Build Now"

Folder page

Results from clicking arrow that means "Build Now"

[LakshyaBagani]

Job page: job-subpage.jelly line 63 still references the deleted data-callback="lib_hudson_project_configurable_build_now_callback", which causes the error. Line 68 also has href="${url}/build?delay=0sec" instead of href="#", reintroducing the race condition.

Folder page: BuildButtonColumn/column.jelly line 48 has href="${href}" set directly. If the onclick handler doesn't attach in time, the browser follows the href as GET, which can show "Build scheduled" even when disabled.

Both need the same fix: set href="#" and use a data attribute for the URL. The job page also needs the callback reference removed.

Should I apply these fixes?

[mawinter69]

Ideally on the folder page the button would be removed, when the multibranch project is disabled. But the current APIs don't allow to find this out I think. That is what @LakshyaBagani tried to achieve in the change of the Java file. Might be possible by adding a method isBuildable to the BuildableItem interface with a default returning true. Afaik ComputedFolder is a BuildableItem and has a isBuildable() method.

[mawinter69]

Job page: job-subpage.jelly line 63 still references the deleted data-callback="lib_hudson_project_configurable_build_now_callback", which causes the error. Line 68 also has href="${url}/build?delay=0sec" instead of href="#", reintroducing the race condition.

Folder page: BuildButtonColumn/column.jelly line 48 has href="${href}" set directly. If the onclick handler doesn't attach in time, the browser follows the href as GET, which can show "Build scheduled" even when disabled.

Both need the same fix: set href="#" and use a data attribute for the URL. The job page also needs the callback reference removed.

Should I apply these fixes?

It seems you're mixing things here with the other PR. The problem @MarkEWaite describes has nothing to do with the race condition

[LakshyaBagani]

Sorry for the confusion - I mixed up the two PRs. The folder page issue isn't related to race conditions.

The issue: the folder page uses BuildButtonColumn/icon.js, which still checks rsp.ok, while the job page uses configurable.js that checks rsp.status === 201. That's why the folder page still shows "Build scheduled" incorrectly.

The fix: change icon.js from if (rsp.ok) to if (rsp.status === 201) to match the fix in configurable.js.

I understand hiding the button when the multibranch is disabled would require API changes that are out of scope. This change will at least fix the incorrect notification.

@mawinter69 Am I thinking in the right direction ?

[mawinter69]

I opened #26131 that implements my proposal for the API change.

[MarkEWaite]

I've confirmed that the additional case that I reported is handled by the pull request. Thanks!

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge

[mawinter69]

Do we still need this PR? With #26131 the buttons are no longer there when the parent Multibranch or org folder job is disabled.

[timja]

Do we still need this PR? With #26131 the buttons are no longer there when the parent Multibranch or org folder job is disabled.

I don't think its needed but it may be more correct as fetch will follow redirects and checking for ok isn't enough as that could be a 200 from a redirect.

Dismissing the request for changes from @mawinter69 because he replied with a "thumbs-up" to the answer from @timja

[welcome]

Congratulations on getting your very first Jenkins core pull request merged 🎉🥳

This is a fantastic achievement, and we're thrilled to have you as part of our community! Thank you for your valuable input, and we look forward to seeing more of your contributions in the future!

We would like to invite you to join the community chats and forums to meet other Jenkins contributors 😊
Don't forget to check out the participation page to learn more about how to contribute to Jenkins.