feat(routingerr): auto-recover from npm _npx cache corruption (#1013)

This commit is contained in:
Carlos Florêncio
2026-05-22 16:25:00 +01:00
committed by GitHub
parent 2bcda51968
commit b427e89d05
9 changed files with 464 additions and 6 deletions
@@ -14,6 +14,7 @@ import (
"github.com/kandev/kandev/internal/agent/executor"
"github.com/kandev/kandev/internal/agent/registry"
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/events/bus"
"github.com/kandev/kandev/internal/secrets"
@@ -116,6 +117,11 @@ type Manager struct {
// the agent process starts. Defaults to a no-op deployer; office wires
// its concrete implementation via SetSkillDeployer.
skillDeployer SkillDeployer
// remediateNpxCache is the hook fired when the routing classifier
// returns CodeNpxCacheCorrupted. NewManager wires routingerr.RemediateNpxCache;
// tests override it to avoid touching the real filesystem.
remediateNpxCache func(path string, log *zap.Logger) error
}
// NewManager creates a new lifecycle manager.
@@ -174,6 +180,7 @@ func NewManager(
remoteStatusBySession: make(map[string]*RemoteStatus),
stopCh: stopCh,
skillDeployer: NoopSkillDeployer(),
remediateNpxCache: routingerr.RemediateNpxCache,
}
// Initialize stream manager with callbacks that delegate to manager methods
@@ -970,17 +970,23 @@ func (m *Manager) MarkCompleted(executionID string, exitCode int, errorMessage s
eventType := events.AgentCompleted
if execution.Status == v1.AgentStatusFailed {
eventType = events.AgentFailed
m.logRoutingClassification(execution, exitCode, errorMessage)
m.classifyAndMaybeRemediate(execution, exitCode, errorMessage)
}
m.eventPublisher.PublishAgentEvent(context.Background(), eventType, execution)
return nil
}
// logRoutingClassification runs routingerr.Classify at the failure boundary
// and logs the structured result. Acting on the result is wired by Phase 4
// of office-provider-routing.
func (m *Manager) logRoutingClassification(execution *AgentExecution, exitCode int, errorMessage string) {
// classifyAndMaybeRemediate runs routingerr.Classify at the failure
// boundary, logs the structured result, and - for codes that carry a
// safe remediation (currently CodeNpxCacheCorrupted) - fires a
// best-effort cleanup goroutine so the next launch attempt (Office
// scheduler retry or Kanban "Resume") finds a clean environment.
//
// The remediation hook is injectable via Manager.remediateNpxCache;
// production wiring points it at routingerr.RemediateNpxCache. Tests
// stub it to avoid touching the real filesystem.
func (m *Manager) classifyAndMaybeRemediate(execution *AgentExecution, exitCode int, errorMessage string) {
phase := routingerr.PhaseSessionInit
if execution.sessionInitialized {
phase = routingerr.PhasePromptSend
@@ -1010,7 +1016,27 @@ func (m *Manager) logRoutingClassification(execution *AgentExecution, exitCode i
zap.String("classifier_rule", e.ClassifierRule),
zap.Bool("fallback_allowed", e.FallbackAllowed),
zap.Bool("auto_retryable", e.AutoRetryable),
zap.Bool("user_action", e.UserAction))
zap.Bool("user_action", e.UserAction),
zap.String("remediation_path", e.RemediationPath))
if e.Code != routingerr.CodeNpxCacheCorrupted || e.RemediationPath == "" {
return
}
remediate := m.remediateNpxCache
path := e.RemediationPath
execID := execution.ID
zapLog := m.logger.Zap().With(
zap.String("execution_id", execID),
zap.String("path", path),
)
go func() {
if err := remediate(path, zapLog); err != nil {
m.logger.Warn("npx cache remediation failed",
zap.String("execution_id", execID),
zap.String("path", path),
zap.Error(err))
}
}()
}
// RespondToPermission sends a response to an agent's permission request.
@@ -0,0 +1,80 @@
package lifecycle
import (
"strings"
"sync"
"testing"
"time"
"github.com/kandev/kandev/internal/common/logger"
"go.uber.org/zap"
)
// newRemediateTestManager builds a Manager with just enough wiring for
// classifyAndMaybeRemediate to run: a logger wrapper around a no-op zap
// and nothing else. Other manager subsystems are nil; the method under
// test must not touch them. We avoid the package-wide newTestManager
// helper because it spins up the full lifecycle DI graph, which is more
// surface area than these failure-boundary tests need.
func newRemediateTestManager(t *testing.T) *Manager {
t.Helper()
log, err := logger.NewFromZap(zap.NewNop())
if err != nil {
t.Fatalf("NewFromZap: %v", err)
}
return &Manager{logger: log}
}
func TestClassifyAndMaybeRemediate_TriggersRemediation_OnNpxCacheCorrupted(t *testing.T) {
stderr := strings.Join([]string{
"npm error code ENOTEMPTY",
"npm error path /Users/test/.npm/_npx/d820eb7d96bc2600/node_modules/foo",
"npm error ENOTEMPTY: directory not empty",
}, "\n")
var (
mu sync.Mutex
calledWith string
done = make(chan struct{}, 1)
)
m := newRemediateTestManager(t)
m.remediateNpxCache = func(path string, _ *zap.Logger) error {
mu.Lock()
calledWith = path
mu.Unlock()
done <- struct{}{}
return nil
}
exec := &AgentExecution{ID: "exec-1", AgentID: "claude-acp", TaskID: "t-1", SessionID: "s-1"}
m.classifyAndMaybeRemediate(exec, 190, stderr)
select {
case <-done:
case <-time.After(time.Second):
t.Fatal("remediation hook was not invoked within 1s")
}
mu.Lock()
defer mu.Unlock()
want := "/Users/test/.npm/_npx/d820eb7d96bc2600"
if calledWith != want {
t.Errorf("remediateNpxCache called with %q, want %q", calledWith, want)
}
}
func TestClassifyAndMaybeRemediate_NoCallOnUnrelatedFailure(t *testing.T) {
m := newRemediateTestManager(t)
called := make(chan struct{}, 1)
m.remediateNpxCache = func(string, *zap.Logger) error {
called <- struct{}{}
return nil
}
exec := &AgentExecution{ID: "exec-2", AgentID: "claude-acp"}
m.classifyAndMaybeRemediate(exec, 1, "some unrelated stderr")
if len(called) != 0 {
t.Fatal("remediation hook should not have been called")
}
}
@@ -219,6 +219,13 @@ func TestNewManager(t *testing.T) {
}
}
func TestNewManager_WiresRemediateNpxCacheDefault(t *testing.T) {
m := newTestManager()
if m.remediateNpxCache == nil {
t.Fatal("NewManager must wire default remediateNpxCache")
}
}
func TestManager_GetExecution(t *testing.T) {
mgr := newTestManager()
@@ -0,0 +1,64 @@
package routingerr
import (
"fmt"
"os"
"path/filepath"
"strings"
"go.uber.org/zap"
)
// RemediateNpxCache wipes a single npx package cache directory (the
// `_npx/<hash>` subtree) so the next `npx -y <pkg>` invocation extracts
// cleanly. It is a no-op-safe operation: if the directory is already
// gone, the call succeeds.
//
// Safety: the path MUST resolve (with symlinks evaluated) to a child of
// `$HOME/.npm/_npx/`. Any other path is rejected without modification,
// so a malformed log line can never trick the caller into removing an
// unrelated tree.
func RemediateNpxCache(path string, log *zap.Logger) error {
if path == "" {
return fmt.Errorf("RemediateNpxCache: empty path")
}
if log == nil {
log = zap.NewNop()
}
resolved, err := filepath.Abs(path)
if err != nil {
return fmt.Errorf("RemediateNpxCache: resolve %q: %w", path, err)
}
evaluated, err := filepath.EvalSymlinks(resolved)
if err != nil {
if os.IsNotExist(err) {
log.Info("npx cache already absent", zap.String("path", resolved))
return nil
}
return fmt.Errorf("RemediateNpxCache: eval symlinks %q: %w", resolved, err)
}
home, err := os.UserHomeDir()
if err != nil {
return fmt.Errorf("RemediateNpxCache: user home: %w", err)
}
expectedRoot := filepath.Join(home, ".npm", "_npx")
evaluatedRoot, err := filepath.EvalSymlinks(expectedRoot)
if err != nil {
if !os.IsNotExist(err) {
return fmt.Errorf("RemediateNpxCache: eval symlinks %q: %w", expectedRoot, err)
}
evaluatedRoot = expectedRoot
}
rootWithSep := evaluatedRoot + string(os.PathSeparator)
if !strings.HasPrefix(evaluated+string(os.PathSeparator), rootWithSep) {
return fmt.Errorf("RemediateNpxCache: refusing path %q outside %q", evaluated, rootWithSep)
}
if evaluated == evaluatedRoot {
return fmt.Errorf("RemediateNpxCache: refusing to delete _npx root %q", evaluated)
}
if err := os.RemoveAll(evaluated); err != nil {
return fmt.Errorf("RemediateNpxCache: remove %q: %w", evaluated, err)
}
log.Info("removed corrupted npx cache directory", zap.String("path", evaluated))
return nil
}
@@ -0,0 +1,97 @@
package routingerr
import (
"os"
"path/filepath"
"strings"
"testing"
"go.uber.org/zap"
)
func TestRemediateNpxCache_RemovesHashDirAndPreservesParent(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
cacheRoot := filepath.Join(home, ".npm", "_npx")
hashDir := filepath.Join(cacheRoot, "d820eb7d96bc2600")
if err := os.MkdirAll(filepath.Join(hashDir, "node_modules", "@anthropic-ai", "claude-agent-sdk-darwin-arm64"), 0o755); err != nil {
t.Fatalf("setup: %v", err)
}
if err := RemediateNpxCache(hashDir, zap.NewNop()); err != nil {
t.Fatalf("RemediateNpxCache: %v", err)
}
if _, err := os.Stat(hashDir); !os.IsNotExist(err) {
t.Errorf("hashDir still exists, err = %v", err)
}
// _npx parent should survive (we only wipe a specific hash dir).
if _, err := os.Stat(cacheRoot); err != nil {
t.Errorf("_npx parent should still exist, err = %v", err)
}
}
func TestRemediateNpxCache_IdempotentWhenAbsent(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
// Create the _npx root but not the hash dir.
if err := os.MkdirAll(filepath.Join(home, ".npm", "_npx"), 0o755); err != nil {
t.Fatalf("setup: %v", err)
}
absent := filepath.Join(home, ".npm", "_npx", "deadbeefcafef00d")
if err := RemediateNpxCache(absent, zap.NewNop()); err != nil {
t.Errorf("RemediateNpxCache on absent path = %v, want nil", err)
}
}
func TestRemediateNpxCache_RejectsPrefixOnlySibling(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
// _npxFOO is NOT a child of _npx/ — the trailing-separator guard must reject it.
bad := filepath.Join(home, ".npm", "_npxFOO", "anything")
if err := os.MkdirAll(bad, 0o755); err != nil {
t.Fatalf("setup: %v", err)
}
err := RemediateNpxCache(bad, zap.NewNop())
if err == nil {
t.Fatal("expected guard error, got nil")
}
if !strings.Contains(err.Error(), "outside") {
t.Errorf("error = %v, want one mentioning 'outside'", err)
}
if _, statErr := os.Stat(bad); statErr != nil {
t.Errorf("guard should not delete the path, stat err = %v", statErr)
}
}
func TestRemediateNpxCache_RejectsPathOutsideNpxCache(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
bad := filepath.Join(home, "etc")
if err := os.MkdirAll(bad, 0o755); err != nil {
t.Fatalf("setup: %v", err)
}
err := RemediateNpxCache(bad, zap.NewNop())
if err == nil {
t.Fatal("expected guard error, got nil")
}
if !strings.Contains(err.Error(), "outside") {
t.Errorf("error = %v, want one mentioning 'outside'", err)
}
if _, statErr := os.Stat(bad); statErr != nil {
t.Errorf("guard should not delete the path, stat err = %v", statErr)
}
}
func TestRemediateNpxCache_RejectsEmptyPath(t *testing.T) {
if err := RemediateNpxCache("", zap.NewNop()); err == nil {
t.Fatal("expected error for empty path")
}
}
@@ -29,6 +29,7 @@ const (
CodeTask Code = "task_error"
CodeRepo Code = "repo_error"
CodePermissionDeniedByUser Code = "permission_denied_by_user"
CodeNpxCacheCorrupted Code = "npx_cache_corrupted"
)
// Confidence reflects how strongly the classifier trusts the matched signal.
@@ -66,6 +67,7 @@ type Error struct {
ExitCode *int
ResetHint *time.Time
RawExcerpt string
RemediationPath string // path to clean before retry; only set for codes that have a known remediation
}
func (e *Error) Error() string {
@@ -100,6 +102,12 @@ func Classify(in Input) *Error {
e.RawExcerpt = excerpt
return applyInvariants(e)
}
if e, ok := matchRuntimeEnvironmentRules(in.Stderr + "\n" + in.Stdout); ok {
e.Phase = in.Phase
e.ExitCode = in.ExitCode
e.RawExcerpt = excerpt
return applyInvariants(e)
}
return applyInvariants(classifyByPhase(in, excerpt))
}
@@ -201,6 +209,9 @@ func applyInvariants(e *Error) *Error {
case CodeProviderUnavailable, CodeUnknownProvider:
e.AutoRetryable = true
e.FallbackAllowed = true
case CodeNpxCacheCorrupted:
e.AutoRetryable = true
e.FallbackAllowed = true
case CodePermissionDeniedByUser, CodeTask, CodeRepo, CodeAgentRuntime:
e.FallbackAllowed = false
e.AutoRetryable = false
@@ -0,0 +1,69 @@
package routingerr
import "regexp"
// runtimeEnvironmentRules match failures caused by the local execution
// environment (npm/npx cache state, missing binaries' install steps,
// etc.) rather than by the provider or remote API. They are tried by
// Classify after provider-specific rules and before the phase-based
// fallback, so a specific environment fingerprint wins over the
// low-confidence "phase.prestart.unknown" verdict.
//
// Each entry pairs a regex matcher with a builder that can extract
// signal-specific metadata (e.g. RemediationPath) from the raw text.
var runtimeEnvironmentRules = []runtimeRule{
{
id: "npm.enotempty.npx.v1",
pattern: regexp.MustCompile(`(?s)npm error code ENOTEMPTY.*?_npx/[0-9a-f]+`),
build: func(text string) *Error {
path := extractNpxCachePath(text)
if path == "" {
return nil
}
return &Error{
Code: CodeNpxCacheCorrupted,
Confidence: ConfHigh,
RemediationPath: path,
}
},
},
}
type runtimeRule struct {
id string
pattern *regexp.Regexp
build func(text string) *Error
}
// npxCachePathRe captures the cache root, e.g.
// `/Users/cfl/.npm/_npx/d820eb7d96bc2600` from any npm error line that
// references a file beneath it. The hash segment is hex (npm uses an
// 8-byte hex of the package spec). `[^\n]*?` lets the home prefix
// contain spaces (e.g. `/Users/John Doe/...`); RemediateNpxCache's
// `EvalSymlinks` + `$HOME/.npm/_npx/` prefix guard validates the path
// before any deletion, so we don't need the regex to do that work too.
var npxCachePathRe = regexp.MustCompile(`(/[^\n]*?/\.npm/_npx/[0-9a-f]+)`)
func extractNpxCachePath(text string) string {
m := npxCachePathRe.FindStringSubmatch(text)
if len(m) < 2 {
return ""
}
return m[1]
}
func matchRuntimeEnvironmentRules(text string) (*Error, bool) {
if text == "" {
return nil, false
}
for _, r := range runtimeEnvironmentRules {
if !r.pattern.MatchString(text) {
continue
}
if e := r.build(text); e != nil {
e.ClassifierRule = r.id
return e, true
}
}
return nil, false
}
@@ -0,0 +1,97 @@
package routingerr
import (
"strings"
"testing"
)
func TestMatchRuntimeEnvironmentRules_NpxENOTEMPTY(t *testing.T) {
stderr := strings.Join([]string{
"npm error code ENOTEMPTY",
"npm error syscall rename",
"npm error path /Users/cfl/.npm/_npx/d820eb7d96bc2600/node_modules/@anthropic-ai/claude-agent-sdk-darwin-arm64",
"npm error dest /Users/cfl/.npm/_npx/d820eb7d96bc2600/node_modules/@anthropic-ai/.claude-agent-sdk-darwin-arm64-n6izKK54",
"npm error errno -66",
"npm error ENOTEMPTY: directory not empty",
}, "\n")
got, ok := matchRuntimeEnvironmentRules(stderr)
if !ok {
t.Fatalf("expected match, got none")
}
if got.Code != CodeNpxCacheCorrupted {
t.Fatalf("Code = %q, want %q", got.Code, CodeNpxCacheCorrupted)
}
if got.ClassifierRule != "npm.enotempty.npx.v1" {
t.Fatalf("ClassifierRule = %q, want npm.enotempty.npx.v1", got.ClassifierRule)
}
if got.Confidence != ConfHigh {
t.Fatalf("Confidence = %q, want %q", got.Confidence, ConfHigh)
}
wantPath := "/Users/cfl/.npm/_npx/d820eb7d96bc2600"
if got.RemediationPath != wantPath {
t.Fatalf("RemediationPath = %q, want %q", got.RemediationPath, wantPath)
}
}
func TestMatchRuntimeEnvironmentRules_NoMatch(t *testing.T) {
cases := []string{
"",
"some unrelated error",
"npm error code EACCES",
"npm error code ENOTEMPTY without any _npx path",
}
for _, in := range cases {
if _, ok := matchRuntimeEnvironmentRules(in); ok {
t.Errorf("expected no match for %q", in)
}
}
}
func TestExtractNpxCachePath_AllowsSpacesInHomePath(t *testing.T) {
// macOS user dirs commonly contain spaces; the regex must not stop
// at whitespace or the rule silently degrades on those machines.
text := "npm error path /Users/John Doe/.npm/_npx/abc123def4/node_modules/foo"
got := extractNpxCachePath(text)
want := "/Users/John Doe/.npm/_npx/abc123def4"
if got != want {
t.Errorf("extractNpxCachePath = %q, want %q", got, want)
}
}
func TestClassify_NpxCacheCorrupted_WiredThroughClassify(t *testing.T) {
resetInjection()
exit := 190
in := Input{
Phase: PhaseSessionInit,
ProviderID: "claude-acp",
ExitCode: &exit,
Stderr: strings.Join([]string{
"npm error code ENOTEMPTY",
"npm error path /Users/cfl/.npm/_npx/d820eb7d96bc2600/node_modules/@anthropic-ai/claude-agent-sdk-darwin-arm64",
"npm error ENOTEMPTY: directory not empty",
}, "\n"),
}
e := Classify(in)
if e == nil {
t.Fatal("expected non-nil Error")
}
if e.Code != CodeNpxCacheCorrupted {
t.Fatalf("Code = %q, want %q", e.Code, CodeNpxCacheCorrupted)
}
if !e.AutoRetryable {
t.Errorf("AutoRetryable = false, want true")
}
if !e.FallbackAllowed {
t.Errorf("FallbackAllowed = false, want true")
}
if e.RemediationPath != "/Users/cfl/.npm/_npx/d820eb7d96bc2600" {
t.Errorf("RemediationPath = %q, want /Users/cfl/.npm/_npx/d820eb7d96bc2600", e.RemediationPath)
}
if e.Phase != PhaseSessionInit {
t.Errorf("Phase = %q, want %q", e.Phase, PhaseSessionInit)
}
if e.ExitCode == nil || *e.ExitCode != 190 {
t.Errorf("ExitCode lost, got %v", e.ExitCode)
}
}