Merge pull request #102 from rlaope/fix/ux-critical-fixes

rlaope · web-flow · commit 9fd3f4d982ed · 2026-04-08T14:37:02.000+09:00
fix: critical UX fixes for v1.0.0
diff --git a/argus-cli/build.gradle.kts b/argus-cli/build.gradle.kts
@@ -16,6 +16,7 @@ dependencies {
 tasks.jar {
     manifest {
         attributes["Main-Class"] = "io.argus.cli.ArgusCli"
+        attributes["Implementation-Version"] = project.version.toString()
     }
 }
 
diff --git a/argus-cli/src/main/java/io/argus/cli/ArgusCli.java b/argus-cli/src/main/java/io/argus/cli/ArgusCli.java
@@ -67,7 +67,15 @@
  */
 public final class ArgusCli {
 
-    private static final String VERSION = "0.6.0";
+    private static final String VERSION = resolveVersion();
+
+    private static String resolveVersion() {
+        Package pkg = ArgusCli.class.getPackage();
+        if (pkg != null && pkg.getImplementationVersion() != null) {
+            return pkg.getImplementationVersion();
+        }
+        return "dev";
+    }
     private static final String DEFAULT_HOST = "localhost";
     private static final int DEFAULT_PORT = 9202;
 
diff --git a/argus-cli/src/main/java/io/argus/cli/doctor/JvmSnapshotCollector.java b/argus-cli/src/main/java/io/argus/cli/doctor/JvmSnapshotCollector.java
@@ -13,19 +13,24 @@
  */
 public final class JvmSnapshotCollector {
 
+    private static final List<String> warnings = new ArrayList<>();
+
     /**
      * Collect snapshot — routes to local or remote based on PID.
      */
     public static JvmSnapshot collect(long pid) {
+        warnings.clear();
         if (pid <= 0 || pid == ProcessHandle.current().pid()) {
             return collectLocal();
         }
         return collectRemote(pid);
     }
 
-    /**
-     * Collect snapshot for the current JVM (in-process mode).
-     */
+    /** Get warnings from the last collection (e.g., jcmd failures). */
+    public static List<String> lastWarnings() {
+        return List.copyOf(warnings);
+    }
+
     @SuppressWarnings("deprecation")
     public static JvmSnapshot collectLocal() {
         MemoryMXBean mem = ManagementFactory.getMemoryMXBean();
@@ -92,31 +97,51 @@ public static JvmSnapshot collectLocal() {
 
     /**
      * Collect snapshot for a remote JVM by PID using jcmd.
-     * Parses GC.heap_info, Thread.print, VM.version, VM.uptime, VM.flags.
+     * Errors are collected in warnings list instead of silently swallowed.
      */
     public static JvmSnapshot collectRemote(long pid) {
+        int failures = 0;
+
         // Heap via GC.heap_info
         long heapUsed = 0, heapMax = 0, heapCommitted = 0, nonHeapUsed = 0;
         Map<String, JvmSnapshot.PoolInfo> pools = new LinkedHashMap<>();
         try {
             String heapInfo = JcmdExecutor.execute(pid, "GC.heap_info");
             parseHeapInfo(heapInfo, pools);
             for (var p : pools.values()) {
-                if (p.type().equalsIgnoreCase("HEAP") || p.name().toLowerCase().contains("heap")) {
-                    heapUsed += p.used();
-                    if (p.max() > 0) heapMax = Math.max(heapMax, p.max());
-                }
-            }
-            // Fallback: sum all pool used values
-            if (heapUsed == 0) {
-                for (var p : pools.values()) heapUsed += p.used();
+                heapUsed += p.used();
+                if (p.max() > 0) heapMax = Math.max(heapMax, p.max());
             }
-        } catch (RuntimeException ignored) {}
+        } catch (RuntimeException e) {
+            warnings.add("GC.heap_info failed: " + e.getMessage());
+            failures++;
+        }
 
-        // GC via jcmd (parse from VM.flags to detect GC, count from GC.heap_info not available — use defaults)
+        // GC stats via jcmd — parse collector info
         List<JvmSnapshot.GcInfo> gcInfos = new ArrayList<>();
         long totalGcCount = 0, totalGcTime = 0;
         String gcAlgorithm = "unknown";
+        try {
+            // Use VM.info which contains GC collector stats
+            String vmInfo = JcmdExecutor.execute(pid, "VM.info");
+            for (String line : vmInfo.split("\n")) {
+                String t = line.trim();
+                // Parse: "garbage-first heap" or "PS Young Generation" etc.
+                if (t.contains("invocations") && t.contains("ms")) {
+                    // Try to parse GC invocation lines from VM.info
+                    var matcher = Pattern.compile("(\\d+)\\s+invocations.*?(\\d+)\\s*ms").matcher(t);
+                    if (matcher.find()) {
+                        long count = Long.parseLong(matcher.group(1));
+                        long time = Long.parseLong(matcher.group(2));
+                        totalGcCount += count;
+                        totalGcTime += time;
+                    }
+                }
+            }
+        } catch (RuntimeException e) {
+            warnings.add("VM.info failed (GC stats unavailable): " + e.getMessage());
+            failures++;
+        }
 
         // VM version
         String vmName = "", vmVersion = "";
@@ -127,7 +152,10 @@ public static JvmSnapshot collectRemote(long pid) {
                 if (vmName.isEmpty() && !t.isEmpty()) vmName = t;
                 else if (vmVersion.isEmpty() && t.contains("build")) vmVersion = t;
             }
-        } catch (RuntimeException ignored) {}
+        } catch (RuntimeException e) {
+            warnings.add("VM.version failed: " + e.getMessage());
+            failures++;
+        }
 
         // VM uptime
         long uptimeMs = 0;
@@ -141,7 +169,10 @@ public static JvmSnapshot collectRemote(long pid) {
                     break;
                 } catch (NumberFormatException ignored2) {}
             }
-        } catch (RuntimeException ignored) {}
+        } catch (RuntimeException e) {
+            warnings.add("VM.uptime failed: " + e.getMessage());
+            failures++;
+        }
 
         // VM flags — detect GC algorithm
         List<String> vmFlags = new ArrayList<>();
@@ -159,7 +190,10 @@ public static JvmSnapshot collectRemote(long pid) {
                     }
                 }
             }
-        } catch (RuntimeException ignored) {}
+        } catch (RuntimeException e) {
+            warnings.add("VM.flags failed: " + e.getMessage());
+            failures++;
+        }
 
         // Threads via Thread.print
         int threadCount = 0, daemonCount = 0;
@@ -177,45 +211,41 @@ public static JvmSnapshot collectRemote(long pid) {
                     String state = t.split(":\\s*")[1].split("\\s+")[0];
                     threadStates.merge(state, 1, Integer::sum);
                 }
-                if (t.contains("Found one Java-level deadlock") || t.contains("Found") && t.contains("deadlock")) {
+                if (t.contains("Found") && t.contains("deadlock")) {
                     deadlockCount++;
                 }
             }
-        } catch (RuntimeException ignored) {}
+        } catch (RuntimeException e) {
+            warnings.add("Thread.print failed: " + e.getMessage());
+            failures++;
+        }
 
-        // Class loading via jcmd VM.classloader_stats
-        int loadedClasses = 0;
-        long totalLoaded = 0, unloaded = 0;
-        try {
-            String classOut = JcmdExecutor.execute(pid, "VM.classloaders");
-            // Count lines with class counts
-            for (String line : classOut.split("\n")) {
-                if (line.trim().matches(".*\\d+.*classes.*")) loadedClasses++;
+        // Print warnings to stderr
+        if (failures > 0) {
+            System.err.println("[Argus] WARNING: " + failures + " jcmd call(s) failed for PID " + pid + ":");
+            for (String w : warnings) {
+                System.err.println("  → " + w);
+            }
+            if (failures >= 4) {
+                System.err.println("[Argus] Most data unavailable. Are you running as the same user as PID " + pid + "?");
+                System.err.println("[Argus] Try: sudo argus doctor " + pid);
             }
-        } catch (RuntimeException ignored) {}
+        }
 
         return new JvmSnapshot(
                 heapUsed, heapMax, heapCommitted, nonHeapUsed,
                 Map.copyOf(pools),
                 List.copyOf(gcInfos), totalGcCount, totalGcTime, uptimeMs,
                 -1, -1, Runtime.getRuntime().availableProcessors(),
-                threadCount, daemonCount, threadCount, // peak = current for remote
+                threadCount, daemonCount, threadCount,
                 Map.copyOf(threadStates), deadlockCount,
-                List.of(), // buffers not available via jcmd
-                loadedClasses, totalLoaded, unloaded,
-                0, // finalizer not available via jcmd
+                List.of(),
+                0, 0, 0, 0,
                 vmName, vmVersion, gcAlgorithm,
                 List.copyOf(vmFlags)
         );
     }
 
-    /**
-     * Parse GC.heap_info output into pool map.
-     * Format varies by GC but generally:
-     *  garbage-first heap   total 262144K, used 45678K [...]
-     *   region size 1024K, 10 young, 2 survivors
-     *  Metaspace       used 34567K, ...
-     */
     private static void parseHeapInfo(String output, Map<String, JvmSnapshot.PoolInfo> pools) {
         if (output == null) return;
         Pattern sizePattern = Pattern.compile("(\\w[\\w\\s-]*)\\s+(?:total\\s+)?(\\d+)K.*used\\s+(\\d+)K");
diff --git a/argus-cli/src/main/java/io/argus/cli/tui/TuiApp.java b/argus-cli/src/main/java/io/argus/cli/tui/TuiApp.java
@@ -127,29 +127,27 @@ private void handleKey(int key, NonBlockingReader reader) throws Exception {
             return;
         }
 
+        if (key == 27) {
+            // ESC — check for arrow key sequence first
+            int next = reader.read(50);
+            if (next == '[') {
+                int arrow = reader.read(50);
+                if (arrow == 'A') { moveUp(); return; }
+                if (arrow == 'B') { moveDown(); return; }
+            }
+            // Plain ESC — back from output or clear search
+            if (showOutput) showOutput = false;
+            else if (!searchQuery.isEmpty()) { searchQuery = ""; applyFilter(); }
+            return;
+        }
+
         switch (key) {
             case 'q', 'Q' -> running = false;
             case '/' -> { searching = true; searchQuery = ""; }
-            case 27 -> { // Escape — back from output
-                if (showOutput) showOutput = false;
-                else if (!searchQuery.isEmpty()) { searchQuery = ""; applyFilter(); }
-            }
-            case 'j', 66 -> moveDown(); // j or ↓ (66 = arrow down after ESC[)
-            case 'k', 65 -> moveUp();   // k or ↑
+            case 'j' -> moveDown();
+            case 'k' -> moveUp();
             case 10, 13 -> executeSelected(); // Enter
-            case 'p', 'P' -> {} // PID change — future
-            case '?' -> {} // Help — future
-            default -> {
-                // Handle arrow key sequences: ESC [ A/B
-                if (key == 27) {
-                    int next = reader.read(50);
-                    if (next == '[') {
-                        int arrow = reader.read(50);
-                        if (arrow == 'A') moveUp();
-                        else if (arrow == 'B') moveDown();
-                    }
-                }
-            }
+            default -> {}
         }
     }
 
diff --git a/gradle.properties b/gradle.properties
@@ -1,3 +1,3 @@
-argusVersion=0.8.0
+argusVersion=1.0.0
 nettyVersion=4.1.104.Final
 junitVersion=5.10.1

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ dependencies {`
`16`	`16`	`tasks.jar {`
`17`	`17`	`manifest {`
`18`	`18`	`attributes["Main-Class"] = "io.argus.cli.ArgusCli"`
	`19`	`+ attributes["Implementation-Version"] = project.version.toString()`
`19`	`20`	`}`
`20`	`21`	`}`
`21`	`22`