Plan run preview npm stall fix
This commit is contained in:
@@ -0,0 +1,365 @@
|
||||
# Run Preview npm Install Stall Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make run-preview restart/stop terminate all descendant npm processes and reuse one persistent npm download cache across generated frontends and preview sessions.
|
||||
|
||||
**Architecture:** Wrap Windows preview commands in a no-profile PowerShell process that records its PID, then return a `ManagedPreviewProcess` carrying the PID file alongside the delegated Java `Process`. The killer uses `taskkill /T` for managed processes before its existing Java and port-based fallbacks. `FrontProjectRunPreviewServiceImpl` places one npm cache directly under the configured preview workspace root and passes it to both frontends.
|
||||
|
||||
**Tech Stack:** Java 8, Spring Boot 2.5, JUnit 4, Mockito, Windows PowerShell, npm 6+
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- Create `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/ManagedPreviewProcess.java`: delegates `Process` operations and exposes the recorded Windows root PID.
|
||||
- Modify `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessRunner.java`: launch Windows commands through the PID-recording wrapper.
|
||||
- Modify `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessKiller.java`: terminate managed Windows process trees before existing fallbacks.
|
||||
- Create `ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessKillerTest.java`: regression tests for tree termination and fallback behavior.
|
||||
- Modify `ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessRunnerTest.java`: verify managed process PID capture with a real short-lived child process.
|
||||
- Modify `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImpl.java`: configure one persistent shared npm cache.
|
||||
- Modify `ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImplTest.java`: verify both frontends share that cache and prefer cached packages.
|
||||
|
||||
### Task 1: Managed Windows Process Tree
|
||||
|
||||
**Files:**
|
||||
- Create: `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/ManagedPreviewProcess.java`
|
||||
- Modify: `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessRunner.java`
|
||||
- Modify: `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessKiller.java`
|
||||
- Test: `ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessKillerTest.java`
|
||||
- Test: `ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessRunnerTest.java`
|
||||
|
||||
- [ ] **Step 1: Write the failing killer tests**
|
||||
|
||||
Create `RunPreviewProcessKillerTest` with:
|
||||
|
||||
```java
|
||||
@Test
|
||||
public void stopRequestsWholeTreeTerminationForManagedProcess() throws Exception
|
||||
{
|
||||
File pidFile = temporaryFolder.newFile("preview.pid");
|
||||
Files.write(pidFile.toPath(), "4321".getBytes(StandardCharsets.UTF_8));
|
||||
FakeProcess delegate = new FakeProcess();
|
||||
ManagedPreviewProcess process = new ManagedPreviewProcess(delegate, pidFile);
|
||||
RecordingProcessKiller killer = new RecordingProcessKiller();
|
||||
|
||||
killer.stop(process, null);
|
||||
|
||||
assertEquals(Long.valueOf(4321L), killer.killedPid);
|
||||
assertTrue(delegate.destroyed);
|
||||
assertTrue(!pidFile.exists());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void stopFallsBackWhenManagedProcessHasNoPid() throws Exception
|
||||
{
|
||||
File missingPidFile = new File(temporaryFolder.getRoot(), "missing.pid");
|
||||
FakeProcess delegate = new FakeProcess();
|
||||
ManagedPreviewProcess process = new ManagedPreviewProcess(delegate, missingPidFile);
|
||||
RecordingProcessKiller killer = new RecordingProcessKiller();
|
||||
|
||||
killer.stop(process, null);
|
||||
|
||||
assertEquals(null, killer.killedPid);
|
||||
assertTrue(delegate.destroyed);
|
||||
}
|
||||
```
|
||||
|
||||
The recording killer overrides a production seam:
|
||||
|
||||
```java
|
||||
@Override
|
||||
protected boolean killWindowsProcessTree(long pid)
|
||||
{
|
||||
killedPid = Long.valueOf(pid);
|
||||
return true;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the killer tests and verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator -Dtest=RunPreviewProcessKillerTest test
|
||||
```
|
||||
|
||||
Expected: compilation fails because `ManagedPreviewProcess` and `killWindowsProcessTree(long)` do not exist.
|
||||
|
||||
- [ ] **Step 3: Add a failing runner integration test**
|
||||
|
||||
Add a Windows-only test to `RunPreviewProcessRunnerTest`:
|
||||
|
||||
```java
|
||||
@Test
|
||||
public void startCommandRecordsManagedWindowsProcessPid() throws Exception
|
||||
{
|
||||
if (!isWindows())
|
||||
{
|
||||
return;
|
||||
}
|
||||
File projectDirectory = temporaryFolder.newFolder("managed-process");
|
||||
RunPreviewProcessRunner runner = new RunPreviewProcessRunner();
|
||||
|
||||
Process process = runner.startCommandForTesting(
|
||||
"ping 127.0.0.1 -n 3 > nul", projectDirectory, new HashMap<String, String>());
|
||||
|
||||
try
|
||||
{
|
||||
assertTrue(process instanceof ManagedPreviewProcess);
|
||||
assertTrue(((ManagedPreviewProcess) process).waitForRootPid(2000L) > 0L);
|
||||
}
|
||||
finally
|
||||
{
|
||||
process.destroyForcibly();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the runner test and verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator -Dtest=RunPreviewProcessRunnerTest test
|
||||
```
|
||||
|
||||
Expected: compilation fails because `startCommandForTesting` and `ManagedPreviewProcess` do not exist.
|
||||
|
||||
- [ ] **Step 5: Implement `ManagedPreviewProcess`**
|
||||
|
||||
Create a package-private final class extending `Process`. Delegate all standard stream, wait, exit, destroy, `destroyForcibly`, and `isAlive` methods to the original process. Add:
|
||||
|
||||
```java
|
||||
long waitForRootPid(long timeoutMillis)
|
||||
{
|
||||
long deadline = System.currentTimeMillis() + timeoutMillis;
|
||||
do
|
||||
{
|
||||
Long pid = readRootPid();
|
||||
if (pid != null)
|
||||
{
|
||||
return pid.longValue();
|
||||
}
|
||||
sleepQuietly(25L);
|
||||
}
|
||||
while (System.currentTimeMillis() < deadline && delegate.isAlive());
|
||||
return -1L;
|
||||
}
|
||||
|
||||
void deletePidFile()
|
||||
{
|
||||
if (pidFile.isFile())
|
||||
{
|
||||
pidFile.delete();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`readRootPid()` reads UTF-8 text, trims it, accepts only a positive `long`, and returns `null` for a missing or invalid file.
|
||||
|
||||
- [ ] **Step 6: Wrap Windows commands in `RunPreviewProcessRunner`**
|
||||
|
||||
Keep the existing shell path on non-Windows. On Windows:
|
||||
|
||||
```java
|
||||
private Process startWindowsManaged(String shellCommand, File workingDirectory,
|
||||
Map<String, String> environment) throws IOException
|
||||
{
|
||||
File pidFile = new File(workingDirectory, ".easycode-process-" + UUID.randomUUID() + ".pid");
|
||||
ProcessBuilder builder = new ProcessBuilder(
|
||||
windowsPowerShellExecutable(),
|
||||
"-NoProfile",
|
||||
"-NonInteractive",
|
||||
"-ExecutionPolicy",
|
||||
"Bypass",
|
||||
"-Command",
|
||||
WINDOWS_PROCESS_WRAPPER);
|
||||
builder.directory(workingDirectory);
|
||||
builder.redirectErrorStream(true);
|
||||
builder.environment().putAll(environment);
|
||||
builder.environment().put("EASYCODE_PREVIEW_COMMAND", shellCommand);
|
||||
builder.environment().put("EASYCODE_PREVIEW_PID_FILE", pidFile.getAbsolutePath());
|
||||
return new ManagedPreviewProcess(builder.start(), pidFile);
|
||||
}
|
||||
```
|
||||
|
||||
Use this wrapper script:
|
||||
|
||||
```powershell
|
||||
[System.IO.File]::WriteAllText(
|
||||
$env:EASYCODE_PREVIEW_PID_FILE,
|
||||
[string]$PID,
|
||||
[System.Text.Encoding]::UTF8
|
||||
)
|
||||
& $env:ComSpec /d /s /c $env:EASYCODE_PREVIEW_COMMAND
|
||||
exit $LASTEXITCODE
|
||||
```
|
||||
|
||||
Expose only a package-private `startCommandForTesting(...)` that calls the same private `start(...)` path as production.
|
||||
|
||||
- [ ] **Step 7: Terminate the managed process tree**
|
||||
|
||||
At the start of `RunPreviewProcessKiller.stopProcess`:
|
||||
|
||||
```java
|
||||
ManagedPreviewProcess managed = process instanceof ManagedPreviewProcess
|
||||
? (ManagedPreviewProcess) process
|
||||
: null;
|
||||
if (managed != null)
|
||||
{
|
||||
long pid = managed.waitForRootPid(GRACEFUL_WAIT_MILLIS);
|
||||
if (pid > 0L)
|
||||
{
|
||||
killWindowsProcessTree(pid);
|
||||
waitForExit(process, GRACEFUL_WAIT_MILLIS);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Then retain the existing `destroy()` and `destroyForcibly()` fallback. In `finally`, call `managed.deletePidFile()`.
|
||||
|
||||
Implement the overridable OS seam:
|
||||
|
||||
```java
|
||||
protected boolean killWindowsProcessTree(long pid)
|
||||
{
|
||||
return executeAndWait(Arrays.asList(
|
||||
"taskkill", "/PID", String.valueOf(pid), "/T", "/F"));
|
||||
}
|
||||
```
|
||||
|
||||
Change the current quiet command helper to return `true` only when the command exits with status `0`; callers that do not care may ignore the result.
|
||||
|
||||
- [ ] **Step 8: Run focused tests and verify GREEN**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator -Dtest=RunPreviewProcessKillerTest,RunPreviewProcessRunnerTest test
|
||||
```
|
||||
|
||||
Expected: both test classes pass with zero failures.
|
||||
|
||||
- [ ] **Step 9: Commit the process-tree fix**
|
||||
|
||||
```text
|
||||
git add ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/ManagedPreviewProcess.java ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessRunner.java ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessKiller.java ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessKillerTest.java ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessRunnerTest.java
|
||||
git commit -m "Fix run preview process tree cleanup"
|
||||
```
|
||||
|
||||
### Task 2: Persistent Shared npm Cache
|
||||
|
||||
**Files:**
|
||||
- Modify: `ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImpl.java`
|
||||
- Modify: `ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImplTest.java`
|
||||
|
||||
- [ ] **Step 1: Change service assertions to the desired cache behavior**
|
||||
|
||||
In `startCreatesWorkspaceInitializesDatabaseAndStartsProcesses`, assert:
|
||||
|
||||
```java
|
||||
assertEquals("true", frontendEnvCaptor.getValue().get("NPM_CONFIG_PREFER_OFFLINE"));
|
||||
assertEquals("false", frontendEnvCaptor.getValue().get("NPM_CONFIG_OFFLINE"));
|
||||
assertEquals(".npm-cache", new File(frontendCache).getName());
|
||||
assertEquals(new File(status.getWorkspacePath()).getParentFile().getCanonicalFile(),
|
||||
new File(frontendCache).getParentFile().getCanonicalFile());
|
||||
```
|
||||
|
||||
In `startSupportsCombinedDownloadPackageLayout`, replace the separate-cache assertion with:
|
||||
|
||||
```java
|
||||
assertEquals(frontendCache, adminFrontendCache);
|
||||
assertEquals(".npm-cache", new File(frontendCache).getName());
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the service test and verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator -Dtest=FrontProjectRunPreviewServiceImplTest test
|
||||
```
|
||||
|
||||
Expected: failures show separate `frontend` / `admin-frontend` cache paths and `NPM_CONFIG_PREFER_OFFLINE=false`.
|
||||
|
||||
- [ ] **Step 3: Configure one cache under the preview root**
|
||||
|
||||
After creating the workspace, resolve:
|
||||
|
||||
```java
|
||||
File sharedNpmCache = new File(new File(workspaceRoot).getCanonicalFile(), ".npm-cache");
|
||||
```
|
||||
|
||||
Pass `sharedNpmCache` to `putNpmPreviewEnvironment` for both frontend environments.
|
||||
|
||||
Inside `putNpmPreviewEnvironment`, retain online fallback:
|
||||
|
||||
```java
|
||||
environment.put("NPM_CONFIG_OFFLINE", "false");
|
||||
environment.put("npm_config_offline", "false");
|
||||
environment.put("NPM_CONFIG_PREFER_OFFLINE", "true");
|
||||
environment.put("npm_config_prefer_offline", "true");
|
||||
environment.put("NPM_CONFIG_PREFER_ONLINE", "false");
|
||||
environment.put("npm_config_prefer_online", "false");
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the service test and verify GREEN**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator -Dtest=FrontProjectRunPreviewServiceImplTest test
|
||||
```
|
||||
|
||||
Expected: all tests in the class pass.
|
||||
|
||||
- [ ] **Step 5: Commit the shared-cache fix**
|
||||
|
||||
```text
|
||||
git add ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImpl.java ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImplTest.java
|
||||
git commit -m "Reuse npm cache for run previews"
|
||||
```
|
||||
|
||||
### Task 3: Regression Verification
|
||||
|
||||
**Files:**
|
||||
- Verify: all files changed in Tasks 1 and 2
|
||||
|
||||
- [ ] **Step 1: Run all focused run-preview tests**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator -Dtest=RunPreviewProcessRunnerTest,RunPreviewProcessKillerTest,FrontProjectRunPreviewServiceImplTest test
|
||||
```
|
||||
|
||||
Expected: zero failures and zero errors.
|
||||
|
||||
- [ ] **Step 2: Run the generator module test suite**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
mvn -pl ruoyi-generator test
|
||||
```
|
||||
|
||||
Expected: `BUILD SUCCESS`.
|
||||
|
||||
- [ ] **Step 3: Inspect the scoped diff**
|
||||
|
||||
Run:
|
||||
|
||||
```text
|
||||
git diff HEAD~2 -- ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/ManagedPreviewProcess.java ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessRunner.java ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/RunPreviewProcessKiller.java ruoyi-generator/src/main/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImpl.java ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessRunnerTest.java ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/RunPreviewProcessKillerTest.java ruoyi-generator/src/test/java/com/ruoyi/generator/service/front/FrontProjectRunPreviewServiceImplTest.java
|
||||
```
|
||||
|
||||
Expected: only process-tree management, shared-cache configuration, and their tests are present.
|
||||
|
||||
- [ ] **Step 4: Perform a live process-tree smoke test**
|
||||
|
||||
Start a managed Windows command through the runner test helper, verify the PID file appears, invoke `RunPreviewProcessKiller.stop`, and verify the child command is no longer alive. This behavior is covered by the focused integration test and must pass on the current Windows host.
|
||||
|
||||
- [ ] **Step 5: Report the operational cleanup requirement**
|
||||
|
||||
The code fix cannot retroactively attach PID metadata to npm processes started before deployment. Identify any pre-fix preview npm/node processes still running and stop only those process IDs after confirming they belong to directories under `preview-workspaces`.
|
||||
Reference in New Issue
Block a user