Skip to content

Fix as.sh version selection and startup reliability - #3271

Merged
hengyunabc merged 2 commits into
alibaba:masterfrom
hengyunabc:codex/fix-as-launcher-reliability
Sep 28, 2026
Merged

hengyunabc merged 2 commits into
alibaba:masterfrom
hengyunabc:codex/fix-as-launcher-reliability

Conversation

@hengyunabc

Copy link
Copy Markdown
Collaborator

as.sh could select 4.3.9 over 4.3.10, treat incomplete cache directories as installed versions, and reject attach-only or batch sessions when telnet was absent. Version lookups also had no timeout, while archive downloads did not fail on HTTP errors.

  • Compare the major/minor/patch fields numerically and select only installations containing the required nonempty JAR files.
  • Rebuild incomplete installations in a temporary directory on the destination filesystem, clean up failed downloads, and fall back to a complete cached version when automatic updates fail. An explicitly requested version fails clearly instead of continuing with an unusable directory.
  • Bound version lookups to 5 seconds and archive downloads to 120 seconds, with HTTP failure handling for both.
  • Require telnet only for interactive sessions, before attaching.

Added 18 launcher regression tests in the existing boot JUnit module. They exercise the Bash startup flow and real ZIP extraction with isolated curl/Java/process fixtures, without network access or attaching to a real JVM.

Local validation on macOS with Bash 3.2:

mvn -B -ntp -pl boot -am -Dtest=AsScriptTest,BootstrapTest -Dsurefire.failIfNoSpecifiedTests=false test
Tests run: 20, Failures: 0, Errors: 0, Skipped: 0

bash -n bin/as.sh
git diff --check

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Concurrent installation can remove another launcher's completed installation, causing startup failure or fallback.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Improves as.sh version selection, cache validation, download reliability, and telnet handling.

Changes:

  • Adds numeric version selection and validates cached installations.
  • Adds bounded, failure-aware downloads with fallback behavior.
  • Adds Bash launcher regression tests.
File Description
bin/​as.sh Hardens version lookup, installation, and startup behavior.
boot/​src/​test/​java/​com/​taobao/​arthas/​boot/​AsScriptTest.java Adds launcher regression coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


@Before
public void setUp() throws Exception {
Assume.assumeTrue("Bash launcher tests require a Unix shell", File.separatorChar == '/');
Comment on lines +262 to +266
for (String request : requests) {
assertTrue(request, request.contains("--fail"));
assertTrue(request, request.contains("--connect-timeout"));
assertTrue(request, request.contains("--max-time"));
}

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Explicit versions permit path traversal, and concurrent installations can leave duplicated temporary trees.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 2 Low severity

Open (3)

Comment on lines +111 to +116
@Test
public void choosesNewestNumericCachedVersion() throws Exception {
cache("4.3.9");
Path newest = cache("4.3.10");
assertHome(run(0, "--attach-only", "1234"), newest);
}

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Same-version installations remain racy, and Cygwin/MinGW can still attach before rejecting an unusable telnet client.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 2 Low severity

Open (3)

@hengyunabc
hengyunabc merged commit d6e847e into alibaba:master Sep 28, 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.

2 participants