fix: handle missing Windows npm cache trees (#3092) (#3094)

This commit is contained in:
Carlos Florêncio
2026-08-27 15:50:41 +00:00
committed by GitHub
parent a5a15f12ea
commit 8ff9c3491c
6 changed files with 268 additions and 15 deletions
+3
View File
@@ -571,6 +571,9 @@ jobs:
- name: Test windows-sensitive packages
run: go test -race -v ./internal/agentctl/server/process/... ./internal/agent/runtime/agentctl/launcher/... ./internal/common/ptyexec/... ./internal/backendapp/ownershiplock/...
- name: Test Windows managed runtime cache repair
run: go test -race -v ./internal/agent/managedruntime -run TestRemoveNpxExecutionTreeTreatsMissingWindowsChildAsAbsent
# Keep port-collision coverage targeted: the instance package contains
# broader stdio MCP fixture tests that depend on Unix command behavior.
- name: Test Windows port collision handling
+7 -3
View File
@@ -351,12 +351,16 @@ test:
## Run only the Windows-sensitive packages — mirrors the test-windows job in
## .github/workflows/backend-tests.yml. The broader suite uses Unix-only
## shell commands in test fixtures and would need GOOS guards to be portable;
## this narrow scope is the regression coverage for the conpty/job-object
## bug class. Tests that genuinely cannot run on Windows skip themselves via
## runtime.GOOS guards in source.
## this narrow scope is the regression coverage for Windows-specific behavior.
## Tests that genuinely cannot run on Windows skip themselves via runtime.GOOS
## guards in source.
test-windows:
@echo "Running Windows-sensitive packages..."
$(CGO_LOCALE) $(CGO_PREFIX) $(GO) test -tags fts5 -v ./internal/agentctl/server/process/... ./internal/agent/runtime/agentctl/launcher/... ./internal/backendapp/ownershiplock/...
@echo "Running Windows managed runtime cache tests..."
# Scoped to the Windows-tagged test only: untagged tests in this package use os.Symlink
# which requires Developer Mode on Windows. Add new //go:build windows tests here.
$(CGO_LOCALE) $(CGO_PREFIX) $(GO) test -tags fts5 -v ./internal/agent/managedruntime -run TestRemoveNpxExecutionTreeTreatsMissingWindowsChildAsAbsent
@echo "Running Windows port-collision tests..."
$(CGO_LOCALE) $(CGO_PREFIX) $(GO) test -tags fts5 -v ./internal/common/netutil -run TestIsAddrInUse
$(CGO_LOCALE) $(CGO_PREFIX) $(GO) test -tags fts5 -v ./internal/agentctl/server/instance -run TestAllocatePortAndListenerRetriesAddressInUse
@@ -19,7 +19,7 @@ import (
// is opened; deletion is then requested on the opened handle.
func removeNpxExecutionTree(cacheRoot, key string) error {
rootHandle, err := openManagedRuntimeDirectoryPath(cacheRoot)
if errors.Is(err, windows.ERROR_PATH_NOT_FOUND) || errors.Is(err, windows.ERROR_FILE_NOT_FOUND) {
if isManagedRuntimeNotFound(err) {
return nil
}
if err != nil {
@@ -27,17 +27,8 @@ func removeNpxExecutionTree(cacheRoot, key string) error {
}
defer windows.CloseHandle(rootHandle)
// NtCreateFile can report a missing child with a status that does not
// compare equal to the Win32 not-found errors on hosted Windows runners.
// Check the direct child first so an absent _npx tree remains idempotent.
if _, err := os.Lstat(filepath.Join(cacheRoot, "_npx")); errors.Is(err, os.ErrNotExist) {
return nil
} else if err != nil {
return fmt.Errorf("inspect npm execution cache: %w", err)
}
npxHandle, err := openManagedRuntimeDirectoryRelative(rootHandle, "_npx")
if errors.Is(err, windows.ERROR_PATH_NOT_FOUND) || errors.Is(err, windows.ERROR_FILE_NOT_FOUND) {
if isManagedRuntimeNotFound(err) {
return nil
}
if err != nil {
@@ -49,7 +40,7 @@ func removeNpxExecutionTree(cacheRoot, key string) error {
defer windows.CloseHandle(npxHandle)
targetHandle, err := openManagedRuntimeDirectoryRelative(npxHandle, key)
if errors.Is(err, windows.ERROR_PATH_NOT_FOUND) || errors.Is(err, windows.ERROR_FILE_NOT_FOUND) {
if isManagedRuntimeNotFound(err) {
return nil
}
if err != nil {
@@ -66,6 +57,18 @@ func removeNpxExecutionTree(cacheRoot, key string) error {
return markManagedRuntimeForDelete(targetHandle)
}
func isManagedRuntimeNotFound(err error) bool {
if errors.Is(err, windows.ERROR_PATH_NOT_FOUND) || errors.Is(err, windows.ERROR_FILE_NOT_FOUND) {
return true
}
var status windows.NTStatus
if !errors.As(err, &status) {
return false
}
errno := status.Errno()
return errors.Is(errno, windows.ERROR_PATH_NOT_FOUND) || errors.Is(errno, windows.ERROR_FILE_NOT_FOUND)
}
func openManagedRuntimeDirectoryPath(path string) (windows.Handle, error) {
return openManagedRuntimeHandle(0, managedRuntimeWindowsNTPath(path), windows.FILE_DIRECTORY_FILE)
}
@@ -0,0 +1,21 @@
//go:build windows
package managedruntime
import (
"os"
"path/filepath"
"testing"
)
// @covers AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3
func TestRemoveNpxExecutionTreeTreatsMissingWindowsChildAsAbsent(t *testing.T) {
cacheRoot := t.TempDir()
if err := os.Mkdir(filepath.Join(cacheRoot, "_npx"), 0o755); err != nil {
t.Fatal(err)
}
if err := RemoveNpxExecutionTree(cacheRoot, "managed-acp@1.2.3"); err != nil {
t.Fatalf("absent execution tree should be idempotent: %v", err)
}
}
@@ -0,0 +1,112 @@
---
created: 2026-08-27
status: done
requirements:
- REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001
- REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002
system_design:
- ../../specs/agents/system-design/managed-npm-runtime-recovery.md
legacy_specs: []
---
# Implementation Plan: Windows Managed Runtime Missing Cache Tree
## Overview
Correct Windows cache repair when the npm `_npx` directory exists but the
derived execution-tree directory does not exist. One work order owns the
native-error classification, the Windows regression test, and Windows CI
coverage.
## Confirmed root cause
`openManagedRuntimeHandle` calls `windows.NtCreateFile`. This function returns
a `windows.NTStatus` for a missing child. The three callers compare this value
directly with Win32 `syscall.Errno` values through `errors.Is`.
`windows.NTStatus` does not implement `Is` or `Unwrap`. As a result, the
comparison fails for `STATUS_OBJECT_NAME_NOT_FOUND` and
`STATUS_OBJECT_PATH_NOT_FOUND`. The cache repair returns an error instead of
treating the missing execution tree as an idempotent result.
The existing `os.Lstat` check only covers a missing `_npx` directory. It does
not cover a missing `_npx/<key>` directory. It also separates the path check
from the handle open.
## Scope
### In scope
- Classify Win32 and NT status not-found errors through one Windows helper.
- Use the helper for the cache-root, `_npx`, and execution-tree opens.
- Remove the separate `_npx` `os.Lstat` check.
- Add a native Windows regression for an existing `_npx` directory with an
absent execution-tree key.
- Add the focused regression to the repository Windows test target and CI job.
### Out of scope
- Make all cache-repair errors best-effort in the lifecycle manager.
- Change recovery classification, retry count, or command construction.
- Add new agentctl error details or managed-runtime stderr logs.
- Remove cached npm packument metadata or other global npm cache data.
- Change Unix cache deletion.
## Technical approach
Add a small helper in
`apps/backend/internal/agent/managedruntime/cache_windows.go`. The helper will
keep the existing Win32 error comparisons. It will also extract a
`windows.NTStatus` with `errors.As` and convert it with `NTStatus.Errno()`.
Use this helper after each `NtCreateFile`-based directory open. A missing
cache root, `_npx` directory, or exact execution tree will return success. All
other errors will keep their current wrapped error and fail-closed behavior.
Remove the `_npx` `os.Lstat` check. The handle-relative open and shared error
classifier will provide the same idempotent result without a separate path
check.
Add a Windows-only test file that creates the cache root and `_npx` directory.
The test will leave the derived key absent and call the public removal helper.
Before the correction, the test returns `Object Name not found.`. After the
correction, the call returns nil.
Add the focused test to `test-windows` in `apps/backend/Makefile` and to the
Windows backend workflow. This package has Windows-specific code but is not in
the current native Windows test list.
## Tests
- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3:**
`TestRemoveNpxExecutionTreeTreatsMissingWindowsChildAsAbsent` proves that an
absent execution tree does not block the online retry.
- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2:** The existing exact-key deletion
test continues to prove that repair removes only the derived execution tree.
- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.4:** Existing symlink and unsafe-spec
tests continue to prove fail-closed path handling.
## Work orders
- [done] [Task 01: Correct Windows Missing-Tree Detection](task-01-correct-windows-missing-tree-detection.md)
## Verification results
- RED evidence: issue #3092 includes a native Windows probe that returns
`Object Name not found.` for the absent child. The Linux executor cannot run
the Windows-only regression before the correction.
- `go test ./internal/agent/managedruntime` passed with 36 tests.
- The Windows managed-runtime test binary compiled for `windows/amd64` with
`CGO_ENABLED=0`.
- `make -n test-windows` includes the exact managed-runtime regression command.
- `python3 scripts/lint-spec-files.py --all` passed.
- The native Windows race test remains a CI check because this executor runs
Linux.
## Risks
- The NT status conversion must not classify access, reparse-point, or sharing
errors as absence.
- The regression needs a native Windows runner. A cross-compiled test binary
proves compilation only.
- The Make target and CI workflow must use the same focused Windows test.
@@ -0,0 +1,110 @@
---
id: "01-correct-windows-missing-tree-detection"
title: "Correct Windows missing-tree detection"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001
- REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002
acceptance_criteria:
- AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3
- AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2
- AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.4
system_design:
- ../../specs/agents/system-design/managed-npm-runtime-recovery.md
---
# Task 01: Correct Windows Missing-Tree Detection
## Summary
Treat native Windows not-found status values as an absent managed-runtime
cache tree. Add focused native Windows coverage so the recovery path cannot
return an HTTP 500 for this state.
## In scope
- Add one helper that recognizes Win32 and converted NT status not-found
errors.
- Use the helper for all three directory opens in the Windows cache remover.
- Remove the separate `_npx` path precheck.
- Add the missing-child regression to the managed-runtime package.
- Run the regression in the Make target and Windows CI job.
## Out of scope
- Changes to lifecycle recovery policy, retry construction, error payloads,
logging, Unix behavior, or npm metadata storage.
- Changes that convert unrelated cache-repair errors into successful results.
## Acceptance
- If the cache root and `_npx` directory exist but the derived key does not,
cache repair returns nil on Windows.
- If `NtCreateFile` reports another error, cache repair keeps the current
fail-closed behavior and error context.
- The repository Windows test target and CI job run the focused regression.
## Verification
Run the new regression on Windows before the production change. Make sure that
it fails with `Object Name not found.`. After the correction, run:
```bash
# From apps/backend on Windows:
rtk go test -race -v ./internal/agent/managedruntime -run '^TestRemoveNpxExecutionTreeTreatsMissingWindowsChildAsAbsent$'
rtk make test-windows
# From apps/backend on any supported development host:
rtk go test ./internal/agent/managedruntime
```
## Files likely touched
- `apps/backend/internal/agent/managedruntime/cache_windows.go`
- `apps/backend/internal/agent/managedruntime/cache_windows_test.go`
- `apps/backend/Makefile`
- `.github/workflows/backend-tests.yml`
- `docs/plans/windows-managed-runtime-missing-cache-tree/plan.md`
- `docs/plans/windows-managed-runtime-missing-cache-tree/task-01-correct-windows-missing-tree-detection.md`
## Dependencies
None.
## Risks
- `NTStatus.Errno()` uses the Windows status-to-DOS mapping. The helper must
compare only the two existing not-found Win32 values.
- Linux cannot run the native Windows regression.
## Parallelism
`sequential`
## Inputs
- `REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001` and
`AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3`.
- `REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002`,
`AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2`, and
`AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.4`.
- `docs/specs/agents/system-design/managed-npm-runtime-recovery.md`.
- `docs/decisions/2026-08-24-agentctl-local-managed-runtime-cache-repair.md`.
- Existing Windows handle-open and cache-removal tests.
## Results
- Added `isManagedRuntimeNotFound`. The helper converts `windows.NTStatus`
values through `Errno()` before it compares the Win32 not-found values.
- Used the helper for the cache root, `_npx` directory, and derived execution
tree.
- Removed the separate `_npx` `os.Lstat` check.
- Added the missing-child regression and included it in both native Windows
test entry points.
- The managed-runtime package passed 36 tests on Linux. The Windows test binary
compiled for `windows/amd64`.
- The native Windows race test remains a CI check because this executor runs
Linux.