Conversation
[JENKINS-59631] Fix styling issue in build history table
* Try to execute flyweight tasks on the master * Fix offline behavior, add tests * Remove obsolete TODOs, also check #isAcceptingTasks
[JENKINS-59793] Avoid hanging jobs with faulty SubTasks
The jenkins-slave.xml file has been copied to the windows-slave-installer-module, but a unused (old) copy is still present in the jenkins core.
…pane of the Plugin Manager (#4298) * refs #JENKINS-59665 Fixed sorting for uninstall column of installed in plugin manager * refs #JENKINS-59665 removed test attribute
* Hide password form fields by default * Trying to bypass enforced autocompletion by just having a test field at first Something like onfocus didn't work, you'd tab through form elements and unless you filled in the user name, changing the form field to password would cause it to autocomplete. It looks like, at least in Mac/Firefox, going from plain text to password in the 'oninput' event handler works. The plain text is revealed neither with typing nor pasting. * Update core/src/main/resources/lib/form/password.jelly Co-Authored-By: daniel-beck <daniel-beck@users.noreply.github.com> * Use previously defined value * Make new password form fields opt-out * Add support for redacting form secrets in new password field * Have a password value pre-set in Jelly * Fix method * Fix test by clicking on the button to change the password * Forgot period separator between class and property name
[JENKINS-59508] Sidebar links show full label on hover
Switch from maven-jenkins-dev-plugin to upstream jetty-maven-plugin
Add StandardOpenOption.CREATE flag to create FileChannelWriter to avoid full fs flush and 5sec log operation on creating empty file with CephFS as a storage
* JENKINS-6722 Show slave error status on Dashboard * Update executors.properties
…unds on the plugin side (#3935) * JENKINS-56553 Make proxy config compatible with JCASC * Use DataBoundSetters as other fields are optional * Fix Jelly for ProxyConfiguration * Use default field name now that field matches * Fix validate proxy * Change field type * After review * Update core/src/main/java/jenkins/model/Jenkins.java Co-Authored-By: timja <t.jacomb@kainos.com> * Update core/src/main/java/jenkins/model/Jenkins.java Co-Authored-By: timja <t.jacomb@kainos.com> * Update core/src/main/java/hudson/ProxyConfiguration.java Co-Authored-By: timja <t.jacomb@kainos.com> * Update core/src/main/java/jenkins/model/Jenkins.java Co-Authored-By: timja <t.jacomb@kainos.com> * Update core/src/main/java/jenkins/model/Jenkins.java Co-Authored-By: timja <t.jacomb@kainos.com> * Update core/src/main/java/hudson/ProxyConfiguration.java Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com> * Tweak javadoc * Align fields * Update core/src/main/java/hudson/ProxyConfiguration.java Co-Authored-By: Adrien Lecharpentier <adrien.lecharpentier@gmail.com>
Document workaround for Stapler data binding bug in repeatableProperty
…ling events (#4345) * Rejuvenate log levels * clean up * Reverting the plugin download Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>
Inlining test-pom into test and removing test-jdk8
…IZABLE, ES_COMPARING_STRINGS_WITH_EQ) (#4379) * fixed Spotbugs issues (SE_COMPARATOR_SHOULD_BE_SERIALIZABLE, ES_COMPARING_STRINGS_WITH_EQ) * replaced wildcard import with specific imports * added missing import * fixed spotbugs issues * removed accidently added import
…stages (#4207) * [JENKINS-59412] Clarify that build history does not include pipeline stages * Clarify that pipelines are merely an example of tasks that may not be listed
Simplify CoreUpdateMonitor/message.properties: changelog.url → jenkins-version.properties
* Add the maintainer checklist to the pull request template * Add checklist items for LTS * Add "Proposed changelog entries" to the pull request template * Pull request template: `Internal: ` prefix is now added automatically by the Core Changelog generator jenkinsci/core-changelog-generator#6 * Apply suggestions from review by @MarkEWaite Co-Authored-By: Mark Waite <mark.earl.waite@gmail.com> Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Upload draft Changelog YAMLs to GitHub Release assets
…or-yaml Revert "Upload draft Changelog YAMLs to GitHub Release assets"
…g YAMLs to Release Drafter
… release metadata
[JENKINS-60740] - Rely on GitHub Actions to generate the release draft YAMLs
Release Drafter Fix: Properly inherit the parent config
[JENKINS-60716] Retain causes of a LogRotator failure for diagnosis
* Spotbugs: Fixed comparing strings with equals instead of == * Spotbugs: Do not return null in @nonnull marked method, instead return empty string. * Added Suppress for Spotbugs * Apply suggestions from code review Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com> * fixed javadoc Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com>
PR Summary by QodoJEP-222: Experimental WebSocket agent transport + Remoting 4.0 + Winstone/Jetty update
AI Description
Diagram
High-Level Assessment
Files changed (30)
|
Code Review by Qodo
1. WebSocket receiver race
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+129
to
+132
| protected void binary(byte[] payload, int offset, int len) { | ||
| LOGGER.finest(() -> "reading block of length " + len + " from " + agent); | ||
| if (offset == 0 && len == payload.length) { | ||
| receiver.handle(payload); |
There was a problem hiding this comment.
1. Websocket receiver race 🐞 Bug ☼ Reliability
WebSocketAgents.Session.binary/closed dereference receiver before Transport.setup assigns it, so early frames or close events can throw NullPointerException and drop inbound agent connections.
Agent Prompt
### Issue description
`WebSocketAgents.Session` uses `receiver.handle(...)` and `receiver.terminate(...)` even though `receiver` is only assigned later in `Transport.setup(...)` and channel setup is submitted asynchronously. This can throw `NullPointerException` and break inbound agent connections.
### Issue Context
`opened()` submits channel setup to `Computer.threadPoolForRemoting`, so WebSocket callbacks (`binary`/`closed`) may run before `Transport.setup` executes.
### Fix Focus Areas
- Ensure `receiver` is non-null before use, e.g.:
- assign a buffering receiver immediately and drain once `setup` runs, OR
- block `binary` handling until `setup` completes, OR
- close the WebSocket with a clear reason if `receiver` is not ready.
- Make `closed(...)` null-safe and ensure state disconnect hooks still fire.
### Fix Focus Areas (code refs)
- core/src/main/java/jenkins/agents/WebSocketAgents.java[117-167]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+91
to
+95
| state.fireAfterProperties(ImmutableMap.of( | ||
| // TODO or just pass all request headers? | ||
| JnlpConnectionState.CLIENT_NAME_KEY, agent, | ||
| JnlpConnectionState.SECRET_KEY, secret | ||
| )); |
There was a problem hiding this comment.
2. Ws agent cookie missing 🐞 Bug ≡ Correctness
WebSocketAgents generates and sends a JnlpConnectionState cookie header but never sets JnlpConnectionState.COOKIE_KEY in the connection state properties, so DefaultJnlpSlaveReceiver cannot store the cookie on the Channel and will reject a reconnect while an old channel is still considered connected.
Agent Prompt
### Issue description
`WebSocketAgents` sends a generated cookie in the HTTP response header, but the `JnlpConnectionState` never receives this cookie as a property. `DefaultJnlpSlaveReceiver` relies on `event.getProperty(JnlpConnectionState.COOKIE_KEY)` to (a) store the cookie on the `Channel` and (b) detect “reconnected from current peer” vs. reject-as-already-connected.
### Issue Context
`state.fireAfterProperties(...)` currently only includes `CLIENT_NAME_KEY` and `SECRET_KEY`, so the cookie is always null to connection listeners.
### Fix Focus Areas
- Generate the cookie once and reuse the same value:
- include `JnlpConnectionState.COOKIE_KEY` in the `fireAfterProperties` property map (or equivalent state property setter), and
- continue to set the same cookie in the response header.
- (Optional but consistent) if reconnect semantics require it, accept an incoming cookie header on subsequent connect attempts and set it into the state properties.
### Fix Focus Areas (code refs)
- core/src/main/java/jenkins/agents/WebSocketAgents.java[87-102]
- core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java[122-160]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+139
to
+143
| try { | ||
| connection = new ServerSideImpl(new OutputImpl(), authentication); | ||
| } catch (IOException x) { | ||
| error(x); | ||
| return; |
There was a problem hiding this comment.
3. Cli ws close npe 🐞 Bug ☼ Reliability
CLIAction.doWs returns from opened() on IOException without closing the WebSocket, leaving connection null; later closed() unconditionally calls connection.handleClose(), which can throw NullPointerException and obscure the original failure.
Agent Prompt
### Issue description
In `CLIAction.doWs`, `opened()` catches an `IOException`, logs via `error(x)`, and returns without initializing `connection` or closing the session. Subsequent `binary(...)`/`closed(...)` callbacks dereference `connection`.
### Issue Context
`connection` is a field inside the anonymous `WebSocketSession` and is only assigned in `opened()`.
### Fix Focus Areas
- If `ServerSideImpl` initialization fails, close the WebSocket session immediately.
- Make `binary(...)` and `closed(...)` null-safe (e.g., check `connection != null` before use).
### Fix Focus Areas (code refs)
- core/src/main/java/hudson/cli/CLIAction.java[117-174]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+236
to
+239
| @DataBoundSetter | ||
| public void setUserName(String userName) { | ||
| this.userName = userName; | ||
| } |
There was a problem hiding this comment.
4. Proxy normalization lost 🐞 Bug ≡ Correctness
ProxyConfiguration moved userName/noProxyHost to @DataBoundSetter but setters no longer apply Util.fixEmptyAndTrim, so blank values remain non-null and can trigger proxy authentication attempts or change no-proxy matching behavior compared to the constructor path.
Agent Prompt
### Issue description
`ProxyConfiguration` constructor normalizes inputs with `Util.fixEmptyAndTrim`, but the new `@DataBoundSetter` methods store raw strings. With databinding (UI/config XML), empty strings are common and will now remain non-null.
### Issue Context
`newAuthenticator()` enables proxy auth whenever `userName != null`, so an empty-string username can now cause proxy auth attempts that previously would not have happened.
### Fix Focus Areas
- Apply `Util.fixEmptyAndTrim` (or equivalent) in setters:
- `setUserName`, `setNoProxyHost`, and likely `setTestUrl`.
- Consider reinitializing `authenticator` when username/password changes if required.
### Fix Focus Areas (code refs)
- core/src/main/java/hudson/ProxyConfiguration.java[137-159]
- core/src/main/java/hudson/ProxyConfiguration.java[226-244]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
20 participants
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.
See JEP-222. Downstream of
jenkinsci/winstone#79+jenkinsci/remoting#357+jenkinsci/jenkins-test-harness#183.Proposed Changelog Entries