diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index c572a8aa4f..df9e226227 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -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 diff --git a/apps/backend/Makefile b/apps/backend/Makefile index c7e5418bc4..5f505605ea 100644 --- a/apps/backend/Makefile +++ b/apps/backend/Makefile @@ -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 diff --git a/apps/backend/internal/agent/managedruntime/cache_windows.go b/apps/backend/internal/agent/managedruntime/cache_windows.go index 058cf30cb7..a7654aa3ac 100644 --- a/apps/backend/internal/agent/managedruntime/cache_windows.go +++ b/apps/backend/internal/agent/managedruntime/cache_windows.go @@ -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) } diff --git a/apps/backend/internal/agent/managedruntime/cache_windows_test.go b/apps/backend/internal/agent/managedruntime/cache_windows_test.go new file mode 100644 index 0000000000..4604a0309d --- /dev/null +++ b/apps/backend/internal/agent/managedruntime/cache_windows_test.go @@ -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) + } +} diff --git a/docs/plans/windows-managed-runtime-missing-cache-tree/plan.md b/docs/plans/windows-managed-runtime-missing-cache-tree/plan.md new file mode 100644 index 0000000000..32378bae95 --- /dev/null +++ b/docs/plans/windows-managed-runtime-missing-cache-tree/plan.md @@ -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/` 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. diff --git a/docs/plans/windows-managed-runtime-missing-cache-tree/task-01-correct-windows-missing-tree-detection.md b/docs/plans/windows-managed-runtime-missing-cache-tree/task-01-correct-windows-missing-tree-detection.md new file mode 100644 index 0000000000..256a16616b --- /dev/null +++ b/docs/plans/windows-managed-runtime-missing-cache-tree/task-01-correct-windows-missing-tree-detection.md @@ -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.