[Fix-18201][TaskPlugin] Fix RemoteShell task NullPointerException and…

[leocook]

Was this PR generated or assisted by AI?

YES, assisted by Claude Sonnet 4.6 for root cause analysis and code fix.

Purpose of the pull request

close #18201

Brief change log

PR #17800 changed runRemoteAndProcessLines() to use channel.getInvertedOut() instead of channel.setOut(ByteArrayOutputStream), which introduced two regression bugs:

1. NPE: getInvertedOut() is called before channel.open() completes, returns null
2. Pipe closed after 0 cycles: MINA SSHD's ChannelPipedInputStream throws IOException when command produces no output

This PR reverts to the setOut(ByteArrayOutputStream) approach while keeping the line-by-line processing improvement from #17800.

Verify this pull request

This pull request is already covered by existing tests, such as RemoteExecutorTest (12 tests, all passed).

Manually verified the change by testing RemoteShell task against a remote SSH server on local standalone deployment.

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

Verify this pull request

This pull request is already covered by existing tests, such as RemoteExecutorTest (12 tests, all passed).
Manually verified on local standalone deployment with RemoteShell task against a remote SSH server:

Before fix:

After fix:

[Copilot]

Pull request overview

This PR fixes regressions introduced in #17800 for the RemoteShell task by avoiding ChannelExec.getInvertedOut() (which can be null before channel open completes) and by preventing MINA SSHD pipe errors when commands produce no output.

Changes:

• Rework RemoteExecutor.runRemoteAndProcessLines() to use channel.setOut(ByteArrayOutputStream) instead of getInvertedOut().
• Update RemoteExecutorTest mocks to simulate setOut(OutputStream) behavior and cover empty-output/non-zero-exit scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
dolphinscheduler-task-plugin/dolphinscheduler-task-remoteshell/src/main/java/org/apache/dolphinscheduler/plugin/task/remoteshell/RemoteExecutor.java	Switches RemoteShell output capture away from getInvertedOut() to avoid NPE/pipe issues; preserves line-based parsing.
dolphinscheduler-task-plugin/dolphinscheduler-task-remoteshell/src/test/java/org/apache/dolphinscheduler/plugin/task/remoteshell/RemoteExecutorTest.java	Adjusts unit tests to mock setOut() output delivery and updates coverage for empty output / exit status behavior.

Comments suppressed due to low confidence (1)

dolphinscheduler-task-plugin/dolphinscheduler-task-remoteshell/src/test/java/org/apache/dolphinscheduler/plugin/task/remoteshell/RemoteExecutorTest.java:269

• testTrackWithEmptyLogOutput will spend significant time sleeping because RemoteExecutor.track() calls Thread.sleep(TRACK_INTERVAL) when no lines are read. With getTaskPid() mocked to return a non-empty pid and then an empty pid, this test will hit the sleep path multiple times (making the unit test suite slow/flaky). Consider stubbing Thread.sleep (e.g., via Mockito static mocking) or adjusting the test setup so the loop exits without waiting.

    void testTrackWithEmptyLogOutput() throws Exception {
        RemoteExecutor remoteExecutor = spy(new RemoteExecutor(sshConnectionParam));
        String taskId = "1234";
        ChannelExec channel = Mockito.mock(ChannelExec.class, RETURNS_DEEP_STUBS);

        doReturn("9527").doReturn("").when(remoteExecutor).getTaskPid(taskId);
        when(clientSession.auth().verify().isSuccess()).thenReturn(true);
        when(clientSession.createExecChannel(anyString())).thenReturn(channel);
        // no output → readLines=0 → sleep once, then getTaskPid returns "" → exit
        when(channel.getExitStatus()).thenReturn(0);
        when(channel.waitFor(EnumSet.of(ClientChannelEvent.CLOSED), 0))
                .thenReturn(EnumSet.of(ClientChannelEvent.CLOSED));

        Assertions.assertDoesNotThrow(() -> remoteExecutor.track(taskId));

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[SbloodyS]

LGTM

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud