diff --git a/apps/backend/internal/agent/agents/ACP_BRIDGE_VERSIONS.md b/apps/backend/internal/agent/agents/ACP_BRIDGE_VERSIONS.md index ba54527a7e..dd9b571547 100644 --- a/apps/backend/internal/agent/agents/ACP_BRIDGE_VERSIONS.md +++ b/apps/backend/internal/agent/agents/ACP_BRIDGE_VERSIONS.md @@ -25,6 +25,14 @@ package is pinned, but npm transitive ranges, its cache, and the registry still affect reproducibility. Kandev records the version reported by the ACP initialize response instead of inferring it from source. +If a managed startup reports the strict npm `ETARGET` error for the selected +exact package and version, Kandev makes one recovery attempt. The colocated +agentctl process resolves its own npm cache, removes only that package's +deterministic `_npx` execution tree, and retries the same command with online +metadata preference. This applies to standalone, local Docker, and remote SSH +executors. Sibling trees, the global npm cache, the registry, and the selected +version remain unchanged. + The **Update agent** action in Settings is the explicit freshness boundary for the Kandev host. Its candidate preparation resolves the requested trusted `package@effective-version` with online preference, then launches a fresh ACP diff --git a/apps/backend/internal/agent/managedruntime/cache.go b/apps/backend/internal/agent/managedruntime/cache.go index f61c93de8a..7effb0599d 100644 --- a/apps/backend/internal/agent/managedruntime/cache.go +++ b/apps/backend/internal/agent/managedruntime/cache.go @@ -20,6 +20,48 @@ func NpxExecutionCacheKey(packageSpec string) string { return hex.EncodeToString(digest[:])[:npxExecutionKeyLength] } +// ValidateExactPackageSpec accepts only a package name followed by a stable +// version. Cache repair must not accept a path or an unversioned npm selector. +func ValidateExactPackageSpec(packageSpec string) error { + if strings.TrimSpace(packageSpec) != packageSpec || packageSpec == "" { + return errors.New("managed runtime package spec is not exact") + } + if strings.ContainsAny(packageSpec, "\x00\r\n\\") || strings.Contains(packageSpec, "..") { + return errors.New("managed runtime package spec is invalid") + } + separator := strings.LastIndexByte(packageSpec, '@') + if separator <= 0 || separator == len(packageSpec)-1 { + return errors.New("managed runtime package spec must include a version") + } + if err := validateManagedPackageName(packageSpec[:separator]); err != nil { + return err + } + if _, err := ParseStableVersion(packageSpec[separator+1:]); err != nil { + return fmt.Errorf("managed runtime package spec version: %w", err) + } + return nil +} + +func validateManagedPackageName(packageName string) error { + if filepath.IsAbs(packageName) || strings.Contains(packageName, ":") || strings.HasSuffix(packageName, "/") { + return errors.New("managed runtime package spec is path-like") + } + if !strings.HasPrefix(packageName, "@") { + if strings.ContainsAny(packageName, "@/") { + return errors.New("managed runtime package spec is path-like") + } + return nil + } + if strings.Contains(packageName[1:], "@") { + return errors.New("managed runtime package spec has an invalid scope") + } + parts := strings.Split(packageName, "/") + if len(parts) != 2 || parts[0] == "@" || parts[1] == "" { + return errors.New("managed runtime package spec has an invalid scope") + } + return nil +} + // RemoveNpxExecutionTree removes only the _npx tree derived from packageSpec. // It refuses broad roots and symlinked paths so a cache repair cannot escape // the exact trusted execution key. diff --git a/apps/backend/internal/agent/managedruntime/cache_validation_test.go b/apps/backend/internal/agent/managedruntime/cache_validation_test.go new file mode 100644 index 0000000000..8ff73192e2 --- /dev/null +++ b/apps/backend/internal/agent/managedruntime/cache_validation_test.go @@ -0,0 +1,37 @@ +package managedruntime + +import "testing" + +func TestValidateExactPackageSpecAcceptsStablePackageVersions(t *testing.T) { + for _, packageSpec := range []string{ + "managed-acp@1.2.3", + "@scope/managed-acp@1.2.3+build.7", + } { + t.Run(packageSpec, func(t *testing.T) { + if err := ValidateExactPackageSpec(packageSpec); err != nil { + t.Fatalf("ValidateExactPackageSpec(%q): %v", packageSpec, err) + } + }) + } +} + +func TestValidateExactPackageSpecRejectsUntrustedSelectors(t *testing.T) { + for _, packageSpec := range []string{ + "managed-acp", + "managed-acp@1.2.3-beta.1", + "managed-acp@latest", + "managed-acp@latest@1.2.3", + "../managed-acp@1.2.3", + "/tmp/managed-acp@1.2.3", + "@scope/managed-acp@1.2.3/extra", + "@scope@other/managed-acp@1.2.3", + " managed-acp@1.2.3", + "managed-acp@1.2.3 ", + } { + t.Run(packageSpec, func(t *testing.T) { + if err := ValidateExactPackageSpec(packageSpec); err == nil { + t.Fatalf("ValidateExactPackageSpec(%q) = nil, want rejection", packageSpec) + } + }) + } +} diff --git a/apps/backend/internal/agent/managedruntime/cache_windows.go b/apps/backend/internal/agent/managedruntime/cache_windows.go index 68cffdce80..058cf30cb7 100644 --- a/apps/backend/internal/agent/managedruntime/cache_windows.go +++ b/apps/backend/internal/agent/managedruntime/cache_windows.go @@ -27,6 +27,15 @@ 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) { return nil diff --git a/apps/backend/internal/agent/runtime/agentctl/client_managed_runtime.go b/apps/backend/internal/agent/runtime/agentctl/client_managed_runtime.go new file mode 100644 index 0000000000..a5c5a191c5 --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/client_managed_runtime.go @@ -0,0 +1,76 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/kandev/kandev/internal/agent/managedruntime" + "github.com/kandev/kandev/internal/agentctl/tracing" +) + +// RepairManagedRuntimeCacheRequest is the authenticated agentctl maintenance +// request for one exact managed npm package specification. +type RepairManagedRuntimeCacheRequest struct { + PackageSpec string `json:"package_spec"` +} + +// RepairManagedRuntimeCacheResponse is the typed success response from the +// authenticated agentctl maintenance endpoint. +type RepairManagedRuntimeCacheResponse struct { + Success bool `json:"success"` + Error string `json:"error,omitempty"` +} + +// RepairManagedRuntimeCache asks the colocated agentctl process to resolve its +// npm cache and remove only the exact execution tree for packageSpec. +func (c *Client) RepairManagedRuntimeCache(ctx context.Context, packageSpec string) error { + if err := managedruntime.ValidateExactPackageSpec(packageSpec); err != nil { + return err + } + ctx, span := tracing.TraceHTTPRequest(ctx, http.MethodPost, "/api/v1/agent/managed-runtime/cache-repair", c.executionID) + defer span.End() + + body, err := json.Marshal(RepairManagedRuntimeCacheRequest{PackageSpec: packageSpec}) + if err != nil { + tracing.TraceHTTPResponse(span, 0, err) + return err + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, + c.baseURL+"/api/v1/agent/managed-runtime/cache-repair", bytes.NewReader(body)) + if err != nil { + tracing.TraceHTTPResponse(span, 0, err) + return err + } + req.Header.Set("Content-Type", "application/json") + resp, err := c.httpClient.Do(req) + if err != nil { + tracing.TraceHTTPResponse(span, 0, err) + return err + } + defer func() { _ = resp.Body.Close() }() + responseBody, err := readResponseBody(resp) + if err != nil { + tracing.TraceHTTPResponse(span, resp.StatusCode, err) + return fmt.Errorf("read managed runtime cache repair response: %w", err) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + err := fmt.Errorf("managed runtime cache repair failed with status %d", resp.StatusCode) + tracing.TraceHTTPResponse(span, resp.StatusCode, err) + return err + } + var result RepairManagedRuntimeCacheResponse + if err := json.Unmarshal(responseBody, &result); err != nil { + tracing.TraceHTTPResponse(span, resp.StatusCode, err) + return fmt.Errorf("parse managed runtime cache repair response: %w", err) + } + if !result.Success { + err := fmt.Errorf("managed runtime cache repair was not successful") + tracing.TraceHTTPResponse(span, resp.StatusCode, err) + return err + } + tracing.TraceHTTPResponse(span, resp.StatusCode, nil) + return nil +} diff --git a/apps/backend/internal/agent/runtime/agentctl/client_managed_runtime_test.go b/apps/backend/internal/agent/runtime/agentctl/client_managed_runtime_test.go new file mode 100644 index 0000000000..810f1e697d --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/client_managed_runtime_test.go @@ -0,0 +1,39 @@ +package client + +import ( + "context" + "encoding/json" + "net/http" + "testing" +) + +func TestRepairManagedRuntimeCachePostsExactPackageSpec(t *testing.T) { + srv, got := captureServer(t, jsonResponder(http.StatusOK, `{"success":true}`)) + client := newHTTPOnlyClient(srv.URL) + + if err := client.RepairManagedRuntimeCache(context.Background(), "@scope/managed-acp@1.2.3"); err != nil { + t.Fatalf("RepairManagedRuntimeCache: %v", err) + } + if got.Method != http.MethodPost || got.Path != "/api/v1/agent/managed-runtime/cache-repair" { + t.Fatalf("request = %s %s, want POST /api/v1/agent/managed-runtime/cache-repair", got.Method, got.Path) + } + var request RepairManagedRuntimeCacheRequest + if err := json.Unmarshal(got.Body, &request); err != nil { + t.Fatalf("decode request: %v", err) + } + if request.PackageSpec != "@scope/managed-acp@1.2.3" { + t.Fatalf("package spec = %q, want exact selected spec", request.PackageSpec) + } +} + +func TestRepairManagedRuntimeCacheRejectsUnversionedSpecBeforeHTTP(t *testing.T) { + srv, got := captureServer(t, jsonResponder(http.StatusOK, `{"success":true}`)) + client := newHTTPOnlyClient(srv.URL) + + if err := client.RepairManagedRuntimeCache(context.Background(), "managed-acp"); err == nil { + t.Fatal("RepairManagedRuntimeCache(unversioned) = nil, want rejection") + } + if got.Method != "" { + t.Fatalf("request method = %q, want no HTTP request", got.Method) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_recovery.go b/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_recovery.go index 04e3ebdae3..156f552285 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_recovery.go +++ b/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_recovery.go @@ -26,18 +26,12 @@ func onlineManagedRuntimeArgs(args []string, spec agents.ManagedNPMRuntimeSpec) } packageSpec := args[npxIndex+3] - if packageSpec != packageName { - versionPrefix := packageName + "@" - if !strings.HasPrefix(packageSpec, versionPrefix) { - return nil, "", false - } - version := strings.TrimPrefix(packageSpec, versionPrefix) - if version == "" { - return nil, "", false - } - if _, err := managedruntime.ParseStableVersion(version); err != nil { - return nil, "", false - } + versionPrefix := packageName + "@" + if !strings.HasPrefix(packageSpec, versionPrefix) { + return nil, "", false + } + if err := managedruntime.ValidateExactPackageSpec(packageSpec); err != nil { + return nil, "", false } recovered := append([]string(nil), args...) diff --git a/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup.go b/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup.go index ac4d2cb62d..b12e457b1f 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup.go +++ b/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup.go @@ -7,7 +7,6 @@ import ( "time" "github.com/kandev/kandev/internal/agent/agents" - "github.com/kandev/kandev/internal/agent/managedruntime" "github.com/kandev/kandev/internal/agent/runtime/routingerr" agentctltypes "github.com/kandev/kandev/internal/agentctl/types" "github.com/kandev/kandev/internal/agentruntime" @@ -26,6 +25,7 @@ func (m *Manager) managedRuntimeNpmStartupFailure( ctx context.Context, execution *AgentExecution, initErr error, + packageSpec string, ) *routingerr.Error { if execution == nil || execution.agentctl == nil || initErr == nil { return nil @@ -37,10 +37,14 @@ func (m *Manager) managedRuntimeNpmStartupFailure( return nil } evidence := strings.Join(lines, "\n") + combined := strings.TrimSpace(evidence + "\n" + initErr.Error()) + if !routingerr.ManagedRuntimeNpmResolutionMatchesPackage(combined, packageSpec) { + return nil + } return routingerr.Classify(routingerr.Input{ Phase: routingerr.PhaseSessionInit, ProviderID: execution.AgentID, - Stderr: strings.TrimSpace(evidence + "\n" + initErr.Error()), + Stderr: combined, }) } @@ -54,28 +58,6 @@ func managedRuntimeRecoveryAborted(ctx context.Context, m *Manager) error { return nil } -func invalidateManagedRuntimeExecutionTree( - ctx context.Context, - invalidator ManagedRuntimeCacheInvalidator, - spec agents.ManagedNPMRuntimeSpec, - packageSpec string, -) error { - if invalidator == nil { - return errors.New("managed runtime cache invalidator is unavailable") - } - if packageSpec == spec.Package { - return invalidator.InvalidateExecutionCache(ctx, spec.Package) - } - version := strings.TrimPrefix(packageSpec, spec.Package+"@") - if version == "" || version == packageSpec { - return errors.New("managed runtime package spec is not versioned") - } - if _, err := managedruntime.ParseStableVersion(version); err != nil { - return err - } - return invalidator.InvalidateExecutionCacheVersion(ctx, spec.Package, version) -} - func (m *Manager) updateExecutionFailure(executionID, message, code, details string) { if m.executionStore == nil { return @@ -104,7 +86,6 @@ func (m *Manager) publishManagedRuntimeStartupFailure( } type managedRuntimeStartupRetry struct { - managed agents.ManagedNPMRuntimeAgent preferOnlineArgs []string packageSpec string failureDetails string @@ -136,33 +117,42 @@ func (m *Manager) prepareManagedRuntimeStartupRetry( initErr error, agentConfig agents.Agent, ) (*managedRuntimeStartupRetry, bool) { - if execution == nil || execution.RuntimeName != agentruntime.RuntimeStandalone || execution.agentctl == nil { + if execution == nil || !supportsManagedRuntimeCacheRepair(execution.RuntimeName) || execution.agentctl == nil { return nil, false } managed, ok := agentConfig.(agents.ManagedNPMRuntimeAgent) - if !ok || m.managedRuntimeCacheInvalidator == nil { - return nil, false - } - classification := m.managedRuntimeNpmStartupFailure(ctx, execution, initErr) - if classification == nil || classification.Code != routingerr.CodeManagedRuntimeNpmResolution { + if !ok { return nil, false } preferOnlineArgs, packageSpec, ok := onlineManagedRuntimeArgs(execution.AgentArgs, managed.ManagedNPMRuntime()) if !ok { return nil, false } + classification := m.managedRuntimeNpmStartupFailure(ctx, execution, initErr, packageSpec) + if classification == nil || classification.Code != routingerr.CodeManagedRuntimeNpmResolution { + return nil, false + } return &managedRuntimeStartupRetry{ - managed: managed, preferOnlineArgs: preferOnlineArgs, packageSpec: packageSpec, failureDetails: classification.RawExcerpt, }, true } -// stopAndInvalidateManagedRuntime stops the failed child and removes only its -// trusted npm execution tree. needsFailure distinguishes an ordinary repair -// error from cancellation or shutdown, which must win over recovery. -func (m *Manager) stopAndInvalidateManagedRuntime( +func supportsManagedRuntimeCacheRepair(runtime agentruntime.Runtime) bool { + switch runtime { + case agentruntime.RuntimeStandalone, agentruntime.RuntimeDocker, agentruntime.RuntimeSSH: + return true + default: + return false + } +} + +// stopAndRepairManagedRuntime stops the failed child and asks its colocated +// agentctl process to remove only the trusted npm execution tree. +// needsFailure distinguishes an ordinary repair error from cancellation or +// shutdown, which must win over recovery. +func (m *Manager) stopAndRepairManagedRuntime( ctx context.Context, execution *AgentExecution, retry *managedRuntimeStartupRetry, @@ -179,8 +169,7 @@ func (m *Manager) stopAndInvalidateManagedRuntime( return false, aborted } - spec := retry.managed.ManagedNPMRuntime() - if err := invalidateManagedRuntimeExecutionTree(ctx, m.managedRuntimeCacheInvalidator, spec, retry.packageSpec); err != nil { + if err := execution.agentctl.RepairManagedRuntimeCache(ctx, retry.packageSpec); err != nil { if aborted := managedRuntimeRecoveryAborted(ctx, m); aborted != nil { return false, aborted } @@ -280,7 +269,7 @@ func (m *Manager) retryManagedRuntimeStartup( // Stop only the child process. The agentctl server remains alive so the // same execution can reconnect its streams and retain its identity. - if needsFailure, err := m.stopAndInvalidateManagedRuntime(ctx, execution, retry); err != nil { + if needsFailure, err := m.stopAndRepairManagedRuntime(ctx, execution, retry); err != nil { if needsFailure { failureCode = routingerr.CodeAgentRuntime failureDetails = routingerr.Sanitize(err.Error()) @@ -313,7 +302,7 @@ func (m *Manager) retryManagedRuntimeStartup( failureCause = retryErr var second *routingerr.Error if initializationFailed { - second = m.managedRuntimeNpmStartupFailure(ctx, execution, retryErr) + second = m.managedRuntimeNpmStartupFailure(ctx, execution, retryErr, retry.packageSpec) } failureCode, failureDetails = managedRuntimeRetryFailureClassification( failureCode, failureDetails, retryErr, initializationFailed, second, diff --git a/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup_recovery_test.go b/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup_recovery_test.go index eceb7c8810..be7619535a 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup_recovery_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup_recovery_test.go @@ -6,7 +6,6 @@ import ( "reflect" "slices" "strings" - "sync" "testing" "github.com/kandev/kandev/internal/agent/agents" @@ -72,20 +71,35 @@ func TestOnlineManagedRuntimeArgsRejectsNonManagedCommands(t *testing.T) { } } -func TestOnlineManagedRuntimeArgsAcceptsTrustedUnversionedPackage(t *testing.T) { +func TestOnlineManagedRuntimeArgsRejectsUnversionedPackage(t *testing.T) { spec := agents.ManagedNPMRuntimeSpec{Package: "managed-acp"} args := []string{"npx", "--yes", "--prefer-offline", "managed-acp", "--acp"} - got, packageSpec, ok := onlineManagedRuntimeArgs(args, spec) - if !ok { - t.Fatal("expected the trusted unversioned command to be eligible") + if _, _, ok := onlineManagedRuntimeArgs(args, spec); ok { + t.Fatal("unversioned managed runtime command should not be eligible") } - if packageSpec != "managed-acp" { - t.Fatalf("package spec = %q, want managed-acp", packageSpec) - } - want := []string{"npx", "--yes", "--prefer-online", "managed-acp", "--acp"} - if !reflect.DeepEqual(got, want) { - t.Fatalf("online args = %#v, want %#v", got, want) +} + +func TestRetryManagedRuntimeStartupUsesAgentctlForExecutorLocalRuntimes(t *testing.T) { + initialErr := errors.New("ACP session initialization failed") + for _, runtimeName := range []agentruntime.Runtime{agentruntime.RuntimeDocker, agentruntime.RuntimeSSH} { + t.Run(runtimeName.String(), func(t *testing.T) { + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) + execution.RuntimeName = runtimeName + + attempted, err := mgr.retryManagedRuntimeStartup( + context.Background(), execution, initialErr, agentConfig, "", "", nil, nil, + ) + if err != nil { + t.Fatalf("retryManagedRuntimeStartup: %v", err) + } + if !attempted { + t.Fatal("expected one managed runtime retry") + } + if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "cache-repair", "configure", "start"}) { + t.Fatalf("HTTP actions = %#v", got) + } + }) } } @@ -122,43 +136,7 @@ func TestManagedRuntimeRetryFailureClassificationPreservesSecondResult(t *testin } } -type managedRuntimeRetryInvalidator struct { - mu sync.Mutex - calls []string - err error - cancel context.CancelFunc - canceled bool -} - -func (i *managedRuntimeRetryInvalidator) InvalidateExecutionCache(_ context.Context, packageName string) error { - i.mu.Lock() - defer i.mu.Unlock() - i.calls = append(i.calls, packageName) - if i.cancel != nil && !i.canceled { - i.canceled = true - i.cancel() - } - return i.err -} - -func (i *managedRuntimeRetryInvalidator) InvalidateExecutionCacheVersion(_ context.Context, packageName, version string) error { - i.mu.Lock() - defer i.mu.Unlock() - i.calls = append(i.calls, packageName+"@"+version) - if i.cancel != nil && !i.canceled { - i.canceled = true - i.cancel() - } - return i.err -} - -func (i *managedRuntimeRetryInvalidator) Calls() []string { - i.mu.Lock() - defer i.mu.Unlock() - return append([]string(nil), i.calls...) -} - -func newManagedRuntimeRetryFixture(t *testing.T, failSessionNew bool) (*Manager, *AgentExecution, *restartMockAgentctlServer, agents.Agent, *managedRuntimeRetryInvalidator) { +func newManagedRuntimeRetryFixture(t *testing.T, failSessionNew bool) (*Manager, *AgentExecution, *restartMockAgentctlServer, agents.Agent) { t.Helper() mgr := newTestManager(t) mock := newRestartMockAgentctlServer(t, false, failSessionNew) @@ -186,16 +164,14 @@ func newManagedRuntimeRetryFixture(t *testing.T, failSessionNew bool) (*Manager, t.Fatalf("add execution: %v", err) } execution.beginStartupAttempt() - invalidator := &managedRuntimeRetryInvalidator{} - mgr.SetManagedRuntimeCacheInvalidator(invalidator) - return mgr, execution, mock, agentConfig, invalidator + return mgr, execution, mock, agentConfig } func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { initialErr := errors.New("ACP session initialization failed") t.Run("successful recovery uses one online replacement", func(t *testing.T) { - mgr, execution, mock, agentConfig, invalidator := newManagedRuntimeRetryFixture(t, false) + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) attempted, err := mgr.retryManagedRuntimeStartup( context.Background(), execution, initialErr, agentConfig, "", "", nil, nil, @@ -206,13 +182,13 @@ func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { if !attempted { t.Fatal("expected one managed runtime retry") } - if got := invalidator.Calls(); !slices.Equal(got, []string{"opencode-ai@1.2.3"}) { - t.Fatalf("invalidator calls = %#v", got) + if got := mock.getManagedRuntimeRepairSpecs(); !slices.Equal(got, []string{"opencode-ai@1.2.3"}) { + t.Fatalf("repair package specs = %#v", got) } if got := execution.AgentArgs; !slices.Contains(got, "--prefer-online") || slices.Contains(got, "--prefer-offline") { t.Fatalf("replacement args = %#v, want online preference only", got) } - if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "configure", "start"}) { + if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "cache-repair", "configure", "start"}) { t.Fatalf("HTTP actions = %#v", got) } if execution.FailureCode != "" || execution.Status == v1.AgentStatusFailed { @@ -220,8 +196,26 @@ func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { } }) + t.Run("transitive ETARGET does not trigger top-level recovery", func(t *testing.T) { + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) + mock.stderrLines = []string{ + "npm error code ETARGET", + "npm error notarget No matching version found for transitive-dependency@9.9.9", + } + + attempted, err := mgr.retryManagedRuntimeStartup( + context.Background(), execution, initialErr, agentConfig, "", "", nil, nil, + ) + if attempted || !errors.Is(err, initialErr) { + t.Fatalf("mismatched ETARGET result = (%v, %v), want no retry and original error", attempted, err) + } + if got := mock.getHTTPActions(); len(got) != 0 { + t.Fatalf("mismatched ETARGET HTTP actions = %#v, want none", got) + } + }) + t.Run("repeated initialization failure is terminal after one retry", func(t *testing.T) { - mgr, execution, mock, agentConfig, invalidator := newManagedRuntimeRetryFixture(t, true) + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, true) attempted, err := mgr.retryManagedRuntimeStartup( context.Background(), execution, initialErr, agentConfig, "", "", nil, nil, @@ -239,17 +233,17 @@ func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { if execution.FailureCode != string(routingerr.CodeManagedRuntimeNpmResolution) { t.Fatalf("execution failure code = %q", execution.FailureCode) } - if got := invalidator.Calls(); len(got) != 1 { - t.Fatalf("invalidator calls = %#v, want one call", got) + if got := mock.getManagedRuntimeRepairSpecs(); len(got) != 1 { + t.Fatalf("repair package specs = %#v, want one call", got) } - if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "configure", "start"}) { + if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "cache-repair", "configure", "start"}) { t.Fatalf("HTTP actions = %#v", got) } }) - t.Run("invalidation failure is generic and does not start a replacement", func(t *testing.T) { - mgr, execution, mock, agentConfig, invalidator := newManagedRuntimeRetryFixture(t, false) - invalidator.err = errors.New("permission denied writing /home/alice/.npm/_npx") + t.Run("repair failure is generic and does not start a replacement", func(t *testing.T) { + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) + mock.failCacheRepair = true attempted, err := mgr.retryManagedRuntimeStartup( context.Background(), execution, initialErr, agentConfig, "", "", nil, nil, @@ -267,10 +261,7 @@ func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { if strings.Contains(startupErr.Details, "/home/alice/") { t.Fatalf("repair details exposed a host path: %q", startupErr.Details) } - if !strings.Contains(startupErr.Details, "permission denied") { - t.Fatalf("repair details = %q, want the sanitized repair error", startupErr.Details) - } - if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop"}) { + if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "cache-repair"}) { t.Fatalf("HTTP actions = %#v, want no configure/start", got) } if execution.FailureCode != string(routingerr.CodeAgentRuntime) { @@ -279,15 +270,15 @@ func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { }) t.Run("cancellation wins over recovery", func(t *testing.T) { - mgr, execution, mock, agentConfig, invalidator := newManagedRuntimeRetryFixture(t, false) + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) ctx, cancel := context.WithCancel(context.Background()) - invalidator.cancel = cancel + mock.onCacheRepair = cancel attempted, err := mgr.retryManagedRuntimeStartup(ctx, execution, initialErr, agentConfig, "", "", nil, nil) if !errors.Is(err, context.Canceled) { t.Fatalf("cancellation error = %v", err) } - if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop"}) { + if got := mock.getHTTPActions(); !slices.Equal(got, []string{"stop", "cache-repair"}) { t.Fatalf("HTTP actions = %#v, want no configure/start", got) } if attempted { @@ -296,20 +287,32 @@ func TestRetryManagedRuntimeStartupLifecycle(t *testing.T) { }) t.Run("remote and native launches are excluded", func(t *testing.T) { - t.Run("remote", func(t *testing.T) { - mgr, execution, mock, agentConfig, _ := newManagedRuntimeRetryFixture(t, false) - execution.RuntimeName = agentruntime.RuntimeSSH + t.Run("remote docker", func(t *testing.T) { + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) + execution.RuntimeName = agentruntime.RuntimeRemoteDocker attempted, err := mgr.retryManagedRuntimeStartup(context.Background(), execution, initialErr, agentConfig, "", "", nil, nil) if attempted || !errors.Is(err, initialErr) { - t.Fatalf("remote result = (%v, %v), want no retry and original error", attempted, err) + t.Fatalf("remote docker result = (%v, %v), want no retry and original error", attempted, err) } if got := mock.getHTTPActions(); len(got) != 0 { - t.Fatalf("remote HTTP actions = %#v", got) + t.Fatalf("remote docker HTTP actions = %#v", got) + } + }) + + t.Run("sprites", func(t *testing.T) { + mgr, execution, mock, agentConfig := newManagedRuntimeRetryFixture(t, false) + execution.RuntimeName = agentruntime.RuntimeSprites + attempted, err := mgr.retryManagedRuntimeStartup(context.Background(), execution, initialErr, agentConfig, "", "", nil, nil) + if attempted || !errors.Is(err, initialErr) { + t.Fatalf("sprites result = (%v, %v), want no retry and original error", attempted, err) + } + if got := mock.getHTTPActions(); len(got) != 0 { + t.Fatalf("sprites HTTP actions = %#v", got) } }) t.Run("native command", func(t *testing.T) { - mgr, execution, mock, _, _ := newManagedRuntimeRetryFixture(t, false) + mgr, execution, mock, _ := newManagedRuntimeRetryFixture(t, false) agentConfig := agents.NewCopilotACP() execution.AgentID = agentConfig.ID() execution.AgentCommand = "copilot" diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager.go b/apps/backend/internal/agent/runtime/lifecycle/manager.go index 1dd5187774..2c637e209f 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager.go @@ -172,9 +172,6 @@ type Manager struct { // managedRuntimeSelections supplies exact versions for host-local managed // npm runtimes. Remote/container runtimes intentionally do not consult it. managedRuntimeSelections managedruntime.SelectionReader - // managedRuntimeCacheInvalidator removes one trusted npm execution tree - // during bounded host-local startup recovery. - managedRuntimeCacheInvalidator ManagedRuntimeCacheInvalidator activityCoordinator *activity.Coordinator activityMu sync.Mutex @@ -190,13 +187,6 @@ type ManagedGoCacheEnvironmentProvider interface { ExecutionEnvironment(ctx context.Context) (map[string]string, error) } -// ManagedRuntimeCacheInvalidator owns the host npm cache boundary. Lifecycle -// recovery never discovers or deletes cache paths directly. -type ManagedRuntimeCacheInvalidator interface { - InvalidateExecutionCache(context.Context, string) error - InvalidateExecutionCacheVersion(ctx context.Context, packageName, version string) error -} - // SetManagedGoCacheEnvironmentProvider wires install-wide managed cache settings. func (m *Manager) SetManagedGoCacheEnvironmentProvider(provider ManagedGoCacheEnvironmentProvider) { m.managedGoCache = provider @@ -208,12 +198,6 @@ func (m *Manager) SetManagedRuntimeSelectionStore(store managedruntime.Selection m.managedRuntimeSelections = store } -// SetManagedRuntimeCacheInvalidator wires the settings-owned exact cache -// deletion boundary. It is optional for embedded/test managers. -func (m *Manager) SetManagedRuntimeCacheInvalidator(invalidator ManagedRuntimeCacheInvalidator) { - m.managedRuntimeCacheInvalidator = invalidator -} - // SetActivityCoordinator wires the install-wide host-resource activity gate. // It is optional so embedded and test configurations retain legacy behavior. func (m *Manager) SetActivityCoordinator(coordinator *activity.Coordinator) { diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_events.go b/apps/backend/internal/agent/runtime/lifecycle/manager_events.go index e60cfc441c..f832080041 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_events.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_events.go @@ -103,10 +103,29 @@ func extractErrorMessage(event *agentctl.AgentEvent) string { return "agent error completion" } +func isUninitializedStartupExecution(execution *AgentExecution) bool { + if execution == nil || execution.startupAttemptSnapshot() == 0 || execution.isSessionInitialized() { + return false + } + return execution.Status == v1.AgentStatusStarting || execution.Status == v1.AgentStatusRunning +} + // handleCompleteEventMarkState marks the execution state after a complete event: // failed+removed on error, ready on success. +func isUninitializedStartupFailure(execution *AgentExecution, event *agentctl.AgentEvent) bool { + return event != nil && event.PromptGeneration == 0 && isUninitializedStartupExecution(execution) +} + func (m *Manager) handleCompleteEventMarkState(execution *AgentExecution, event *agentctl.AgentEvent, isError bool) { if isError { + // A process can exit while the startup owner is still waiting for ACP + // initialization. Leave that failure non-terminal so the startup path can + // classify it and perform its bounded managed-runtime recovery, if any. + if isUninitializedStartupFailure(execution, event) { + m.logger.Debug("deferring uninitialized startup failure to startup owner", + zap.String("execution_id", execution.ID)) + return + } errorMsg := extractErrorMessage(event) // A turn aborted by backend graceful shutdown is not an agent failure. // Redirect it to a benign stop so the session stays resumable and the UI @@ -304,6 +323,14 @@ func (m *Manager) handleCompleteEvent(execution *AgentExecution, event *agentctl event.TurnID = execution.promptTurnIDSnapshot() } isError, stopReason := completeEventResult(event) + if isError && isUninitializedStartupFailure(execution, event) { + // The startup owner is waiting for this failed process to classify its + // stderr. Do not release startup activity or signal prompt completion; + // the closed child stream will return the ACP request error to that owner. + m.logger.Debug("deferring uninitialized startup error event to startup owner", + zap.String("execution_id", execution.ID)) + return true + } claim, claimed := m.claimPromptCompletion(execution, event, isError) if !claimed { return false diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_events_startup_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_events_startup_test.go new file mode 100644 index 0000000000..44d215f6e3 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_events_startup_test.go @@ -0,0 +1,81 @@ +package lifecycle + +import ( + "testing" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/agentctl/types/streams" + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/pkg/api/v1" +) + +func TestHandleCompleteEventMarkState_DefersUninitializedStartupFailure(t *testing.T) { + mgr, eventBus := createTestManagerWithTracking() + execution := createTestExecution("exec-1", "task-1", "session-1") + execution.Status = v1.AgentStatusStarting + execution.beginStartupAttempt() + if err := mgr.executionStore.Add(execution); err != nil { + t.Fatalf("add execution: %v", err) + } + + mgr.handleCompleteEventMarkState(execution, &agentctl.AgentEvent{ + Type: streams.EventTypeComplete, + Error: "Agent process exited with code 1", + Data: map[string]any{"is_error": true}, + }, true) + + got, _ := mgr.GetExecution(execution.ID) + if got.Status != v1.AgentStatusStarting { + t.Fatalf("status = %q, want %q while startup owns failure classification", got.Status, v1.AgentStatusStarting) + } + if containsSubject(publishedSubjects(eventBus), events.AgentFailed) { + t.Fatal("startup process failure published agent.failed before lifecycle recovery classified it") + } +} + +func TestMarkCompleted_DefersUninitializedStartupFailure(t *testing.T) { + mgr, eventBus := createTestManagerWithTracking() + execution := createTestExecution("exec-1", "task-1", "session-1") + execution.Status = v1.AgentStatusStarting + execution.beginStartupAttempt() + if err := mgr.executionStore.Add(execution); err != nil { + t.Fatalf("add execution: %v", err) + } + + if err := mgr.MarkCompleted(execution.ID, 1, "Agent process exited with code 1"); err != nil { + t.Fatalf("MarkCompleted: %v", err) + } + + got, _ := mgr.GetExecution(execution.ID) + if got.Status != v1.AgentStatusStarting { + t.Fatalf("status = %q, want %q while startup owns failure classification", got.Status, v1.AgentStatusStarting) + } + if containsSubject(publishedSubjects(eventBus), events.AgentFailed) { + t.Fatal("startup process failure published agent.failed before lifecycle recovery classified it") + } +} + +func TestHandleAgentEvent_DefersUninitializedStartupFailure(t *testing.T) { + mgr, eventBus := createTestManagerWithTracking() + execution := createTestExecution("exec-1", "task-1", "session-1") + execution.Status = v1.AgentStatusStarting + execution.beginStartupAttempt() + if err := mgr.executionStore.Add(execution); err != nil { + t.Fatalf("add execution: %v", err) + } + + mgr.handleAgentEvent(execution, agentctl.AgentEvent{ + Type: "error", + Error: "Agent process exited with code 1", + Data: map[string]any{"is_error": true}, + }) + + select { + case <-execution.promptDoneCh: + t.Fatal("startup process failure signaled prompt completion before startup recovery classified it") + default: + } + if containsSubject(publishedSubjects(eventBus), events.AgentFailed) { + t.Fatal("startup process failure published agent.failed before lifecycle recovery classified it") + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go index 7357447475..6a32c8780f 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go @@ -1721,6 +1721,12 @@ func (m *Manager) markCompletedWithTurnID( if (exitCode != 0 || errorMessage != "") && m.IsShuttingDown() { return m.markStoppedDuringShutdown(execution, exitCode, errorMessage, turnID) } + if (exitCode != 0 || errorMessage != "") && isUninitializedStartupExecution(execution) { + m.logger.Debug("deferring uninitialized startup process exit to startup owner", + zap.String("execution_id", execution.ID), + zap.Int("exit_code", exitCode)) + return nil + } _ = m.executionStore.WithLock(executionID, func(exec *AgentExecution) { now := time.Now() diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go index bc7617f76b..0e42d9ac6a 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go @@ -39,23 +39,27 @@ func (r *restartProfileResolver) ResolveProfile(_ context.Context, _ string) (*A type restartMockAgentctlServer struct { server *httptest.Server - mu sync.Mutex - httpActions []string - wsActions []string - setModelIDs []string - setModeIDs []string - setOptions []restartConfigOption + mu sync.Mutex + httpActions []string + repairPackageSpecs []string + wsActions []string + setModelIDs []string + setModeIDs []string + setOptions []restartConfigOption failStop bool failSessionNew bool failSessionReset bool + failCacheRepair bool failMode bool failModel bool failConfigOptionID string + stderrLines []string modelState *streams.SessionModelState newModelState *streams.SessionModelState onReset func() onSessionNew func() + onCacheRepair func() } type restartConfigOption struct { @@ -120,6 +124,24 @@ func newRestartMockAgentctlServer(t *testing.T, failStop, failSessionNew bool) * } _, _ = w.Write([]byte(`{"success":true}`)) }) + mux.HandleFunc("/api/v1/agent/managed-runtime/cache-repair", func(w http.ResponseWriter, r *http.Request) { + m.recordHTTP("cache-repair") + var request agentctl.RepairManagedRuntimeCacheRequest + if err := json.NewDecoder(r.Body).Decode(&request); err == nil { + m.mu.Lock() + m.repairPackageSpecs = append(m.repairPackageSpecs, request.PackageSpec) + m.mu.Unlock() + } + if m.onCacheRepair != nil { + m.onCacheRepair() + } + if m.failCacheRepair { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"success":false}`)) + return + } + _, _ = w.Write([]byte(`{"success":true}`)) + }) mux.HandleFunc("/api/v1/agent/configure", func(w http.ResponseWriter, _ *http.Request) { m.recordHTTP("configure") _, _ = w.Write([]byte(`{"success":true}`)) @@ -229,11 +251,15 @@ func newRestartMockAgentctlServer(t *testing.T, failStop, failSessionNew bool) * }) } case "agent.stderr": - resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ - "lines": []string{ + stderrLines := m.stderrLines + if len(stderrLines) == 0 { + stderrLines = []string{ "npm error code ETARGET", "npm error notarget No matching version found for opencode-ai@1.2.3", - }, + } + } + resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ + "lines": stderrLines, }) default: resp, _ = ws.NewError(msg.ID, msg.Action, ws.ErrorCodeUnknownAction, "unknown action", nil) @@ -309,6 +335,14 @@ func (m *restartMockAgentctlServer) getHTTPActions() []string { return out } +func (m *restartMockAgentctlServer) getManagedRuntimeRepairSpecs() []string { + m.mu.Lock() + defer m.mu.Unlock() + out := make([]string, len(m.repairPackageSpecs)) + copy(out, m.repairPackageSpecs) + return out +} + func (m *restartMockAgentctlServer) getWSActions() []string { m.mu.Lock() defer m.mu.Unlock() diff --git a/apps/backend/internal/agent/runtime/routingerr/npm_resolution.go b/apps/backend/internal/agent/runtime/routingerr/npm_resolution.go new file mode 100644 index 0000000000..7901236a93 --- /dev/null +++ b/apps/backend/internal/agent/runtime/routingerr/npm_resolution.go @@ -0,0 +1,22 @@ +package routingerr + +import ( + "regexp" + "strings" +) + +// ManagedRuntimeNpmResolutionMatchesPackage reports whether stderr contains a +// bounded npm ETARGET diagnostic for the exact managed package specification. +// Callers must derive the expected specification from the trusted launch +// arguments before using this guard. +func ManagedRuntimeNpmResolutionMatchesPackage(stderr, packageSpec string) bool { + if packageSpec == "" || strings.TrimSpace(packageSpec) != packageSpec { + return false + } + codePattern := regexp.MustCompile(`(?im)^\s*npm\s+(?:ERR!|error)\s+code\s+ETARGET\b`) + notargetPattern := regexp.MustCompile( + `(?im)^\s*npm\s+(?:ERR!|error)\s+notarget\s+No matching version found for\s+` + + regexp.QuoteMeta(packageSpec) + `(?:\.\s*)?$`, + ) + return codePattern.MatchString(stderr) && notargetPattern.MatchString(stderr) +} diff --git a/apps/backend/internal/agent/runtime/routingerr/npm_resolution_test.go b/apps/backend/internal/agent/runtime/routingerr/npm_resolution_test.go index 161eca427b..deda867407 100644 --- a/apps/backend/internal/agent/runtime/routingerr/npm_resolution_test.go +++ b/apps/backend/internal/agent/runtime/routingerr/npm_resolution_test.go @@ -70,3 +70,41 @@ func TestManagedRuntimeNpmResolutionSanitizesDetails(t *testing.T) { t.Fatalf("raw excerpt contains an unsanitized secret: %q", classified.RawExcerpt) } } + +func TestManagedRuntimeNpmResolutionMatchesExactPackage(t *testing.T) { + const packageSpec = "@kandev/agent@1.2.3" + tests := []struct { + name string + input string + match bool + }{ + { + name: "exact selected package", + input: "npm error code ETARGET\n" + + "npm error notarget No matching version found for @kandev/agent@1.2.3.", + match: true, + }, + { + name: "transitive dependency", + input: "npm error code ETARGET\n" + + "npm error notarget No matching version found for dependency@9.9.9.", + }, + { + name: "nearby version is not exact", + input: "npm error code ETARGET\n" + + "npm error notarget No matching version found for @kandev/agent@1.2.30.", + }, + { + name: "diagnostic without ETARGET code", + input: "npm error notarget No matching version found for @kandev/agent@1.2.3.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ManagedRuntimeNpmResolutionMatchesPackage(tt.input, packageSpec); got != tt.match { + t.Fatalf("ManagedRuntimeNpmResolutionMatchesPackage() = %v, want %v", got, tt.match) + } + }) + } +} diff --git a/apps/backend/internal/agentctl/server/api/managed_runtime.go b/apps/backend/internal/agentctl/server/api/managed_runtime.go new file mode 100644 index 0000000000..4e997968ea --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/managed_runtime.go @@ -0,0 +1,48 @@ +package api + +import ( + "net/http" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/agent/managedruntime" +) + +// ManagedRuntimeCacheRepairRequest contains the trusted exact package spec +// used to derive one npm execution tree. +type ManagedRuntimeCacheRepairRequest struct { + PackageSpec string `json:"package_spec"` +} + +// ManagedRuntimeCacheRepairResponse reports the result without exposing the +// executor's npm path or command output. +type ManagedRuntimeCacheRepairResponse struct { + Success bool `json:"success"` + Error string `json:"error,omitempty"` +} + +func (s *Server) handleManagedRuntimeCacheRepair(c *gin.Context) { + var req ManagedRuntimeCacheRepairRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, ManagedRuntimeCacheRepairResponse{ + Error: "invalid managed runtime cache repair request", + }) + return + } + if err := managedruntime.ValidateExactPackageSpec(req.PackageSpec); err != nil { + c.JSON(http.StatusBadRequest, ManagedRuntimeCacheRepairResponse{ + Error: "invalid managed runtime package specification", + }) + return + } + if err := s.procMgr.RepairManagedRuntimeCache(c.Request.Context(), req.PackageSpec); err != nil { + if c.Request.Context().Err() != nil { + return + } + s.logger.Error("managed runtime cache repair failed") + c.JSON(http.StatusInternalServerError, ManagedRuntimeCacheRepairResponse{ + Error: "managed runtime cache repair failed", + }) + return + } + c.JSON(http.StatusOK, ManagedRuntimeCacheRepairResponse{Success: true}) +} diff --git a/apps/backend/internal/agentctl/server/api/managed_runtime_test.go b/apps/backend/internal/agentctl/server/api/managed_runtime_test.go new file mode 100644 index 0000000000..adb86021f9 --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/managed_runtime_test.go @@ -0,0 +1,49 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/kandev/kandev/internal/agent/managedruntime" +) + +func TestManagedRuntimeCacheRepairUsesAgentEnvironmentAndExactTree(t *testing.T) { + server := newTestServer(t) + cacheRoot := t.TempDir() + server.cfg.AgentEnv = []string{"NPM_CONFIG_CACHE=" + cacheRoot} + + packageSpec := "@scope/managed-acp@1.2.3" + npxRoot := filepath.Join(cacheRoot, "_npx") + target := filepath.Join(npxRoot, managedruntime.NpxExecutionCacheKey(packageSpec)) + sibling := filepath.Join(npxRoot, "0123456789abcdef") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(sibling, 0o755); err != nil { + t.Fatal(err) + } + + body, err := json.Marshal(map[string]string{"package_spec": packageSpec}) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/api/v1/agent/managed-runtime/cache-repair", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + resp := httptest.NewRecorder() + server.router.ServeHTTP(resp, req) + + if resp.Code != http.StatusOK { + t.Fatalf("cache repair status = %d, want %d: %s", resp.Code, http.StatusOK, resp.Body.String()) + } + if _, err := os.Stat(target); !os.IsNotExist(err) { + t.Fatalf("target stat error = %v, want not-exist", err) + } + if _, err := os.Stat(sibling); err != nil { + t.Fatalf("unrelated tree was removed: %v", err) + } +} diff --git a/apps/backend/internal/agentctl/server/api/server.go b/apps/backend/internal/agentctl/server/api/server.go index 5e7b73cb3d..19aecb9391 100644 --- a/apps/backend/internal/agentctl/server/api/server.go +++ b/apps/backend/internal/agentctl/server/api/server.go @@ -100,6 +100,7 @@ func (s *Server) setupRoutes() { // Process control api.POST("/agent/configure", s.handleAgentConfigure) + api.POST("/agent/managed-runtime/cache-repair", s.handleManagedRuntimeCacheRepair) api.POST("/start", s.handleStart) api.POST("/stop", s.handleStop) diff --git a/apps/backend/internal/agentctl/server/process/managed_npm_stderr.go b/apps/backend/internal/agentctl/server/process/managed_npm_stderr.go new file mode 100644 index 0000000000..e3f28b1558 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/managed_npm_stderr.go @@ -0,0 +1,32 @@ +package process + +import ( + "strings" + + "github.com/kandev/kandev/internal/agent/managedruntime" +) + +const ( + npmETargetCodeLine = "npm error code ETARGET" + npmNotargetPrefix = "npm error notarget No matching version found for " + npmLegacyNotargetPrefix = "npm ERR! notarget No matching version found for " +) + +func safeManagedNpmStderrLine(raw string) (string, bool) { + line := strings.TrimSpace(raw) + if strings.EqualFold(line, npmETargetCodeLine) || strings.EqualFold(line, "npm ERR! code ETARGET") { + return npmETargetCodeLine, true + } + + for _, prefix := range []string{npmNotargetPrefix, npmLegacyNotargetPrefix} { + if !strings.HasPrefix(line, prefix) { + continue + } + packageSpec := strings.TrimSuffix(strings.TrimSpace(strings.TrimPrefix(line, prefix)), ".") + if managedruntime.ValidateExactPackageSpec(packageSpec) != nil { + return "", false + } + return npmNotargetPrefix + packageSpec, true + } + return "", false +} diff --git a/apps/backend/internal/agentctl/server/process/managed_runtime.go b/apps/backend/internal/agentctl/server/process/managed_runtime.go new file mode 100644 index 0000000000..3668627af0 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/managed_runtime.go @@ -0,0 +1,59 @@ +package process + +import ( + "context" + "errors" + "path/filepath" + "strings" + + "github.com/kandev/kandev/internal/agent/managedruntime" + tools "github.com/kandev/kandev/internal/tools/installer" +) + +// RepairManagedRuntimeCache resolves npm's cache in the instance environment +// and removes only the execution tree for one exact managed package spec. +func (m *Manager) RepairManagedRuntimeCache(ctx context.Context, packageSpec string) error { + if err := managedruntime.ValidateExactPackageSpec(packageSpec); err != nil { + return err + } + env, err := m.CommandEnvironment() + if err != nil { + return errors.New("resolve agent environment for managed runtime repair") + } + output, err := m.Output(ctx, tools.CommandSpec{ + Path: "npm", + Args: []string{"config", "get", "cache"}, + Dir: m.cfg.WorkDir, + Env: env, + }) + if err != nil { + if ctx.Err() != nil { + return ctx.Err() + } + return errors.New("resolve npm cache for managed runtime repair") + } + cacheRoot, err := npmCacheRootFromOutput(output) + if err != nil { + return err + } + if err := managedruntime.RemoveNpxExecutionTree(cacheRoot, packageSpec); err != nil { + if ctx.Err() != nil { + return ctx.Err() + } + return errors.New("remove managed runtime npm execution tree") + } + m.ClearStderrBuffer() + return nil +} + +func npmCacheRootFromOutput(output []byte) (string, error) { + lines := strings.Split(strings.TrimSpace(string(output)), "\n") + if len(lines) != 1 { + return "", errors.New("npm cache path was not returned") + } + candidate := strings.TrimSpace(lines[0]) + if candidate == "" || !filepath.IsAbs(candidate) { + return "", errors.New("npm cache path was not returned") + } + return candidate, nil +} diff --git a/apps/backend/internal/agentctl/server/process/managed_runtime_test.go b/apps/backend/internal/agentctl/server/process/managed_runtime_test.go new file mode 100644 index 0000000000..4e01904b3c --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/managed_runtime_test.go @@ -0,0 +1,67 @@ +package process + +import ( + "context" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agentctl/server/config" +) + +func TestNpmCacheRootFromOutputUsesAbsolutePath(t *testing.T) { + cacheRoot := t.TempDir() + output := []byte(cacheRoot + "\n") + + got, err := npmCacheRootFromOutput(output) + if err != nil { + t.Fatalf("npmCacheRootFromOutput: %v", err) + } + if got != cacheRoot { + t.Fatalf("cache root = %q, want %q", got, cacheRoot) + } +} + +func TestNpmCacheRootFromOutputRejectsMixedStreams(t *testing.T) { + cacheRoot := t.TempDir() + output := []byte("npm warning using configured registry\n" + cacheRoot + "\n") + if _, err := npmCacheRootFromOutput(output); err == nil { + t.Fatal("npmCacheRootFromOutput() = nil error, want mixed-stream output rejected") + } +} + +func TestNpmCacheRootFromOutputRejectsNonPathOutput(t *testing.T) { + if _, err := npmCacheRootFromOutput([]byte("npm cache unavailable\nrelative/cache\n")); err == nil { + t.Fatal("npmCacheRootFromOutput() = nil error, want rejection") + } +} + +func TestRepairManagedRuntimeCacheRejectsUnversionedSpecBeforeCommand(t *testing.T) { + mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir()}, newTestLogger(t)) + t.Cleanup(func() { _ = mgr.StopForTeardown(context.Background()) }) + + err := mgr.RepairManagedRuntimeCache(context.Background(), "managed-acp") + if err == nil { + t.Fatal("RepairManagedRuntimeCache(unversioned) = nil, want rejection") + } + if strings.Contains(err.Error(), "npm") { + t.Fatalf("validation error = %q, should not start npm", err) + } +} + +func TestRepairManagedRuntimeCacheClearsPreviousStderr(t *testing.T) { + cacheRoot := t.TempDir() + packageSpec := "managed-acp@1.2.3" + mgr := NewManager(&config.InstanceConfig{ + WorkDir: t.TempDir(), + AgentEnv: []string{"NPM_CONFIG_CACHE=" + cacheRoot}, + }, newTestLogger(t)) + t.Cleanup(func() { _ = mgr.StopForTeardown(context.Background()) }) + mgr.appendStderr("stale first-attempt error") + + if err := mgr.RepairManagedRuntimeCache(context.Background(), packageSpec); err != nil { + t.Fatalf("RepairManagedRuntimeCache: %v", err) + } + if got := mgr.GetRecentStderr(); len(got) != 0 { + t.Fatalf("stderr after repair = %#v, want empty", got) + } +} diff --git a/apps/backend/internal/agentctl/server/process/manager.go b/apps/backend/internal/agentctl/server/process/manager.go index 5c25686700..acf9bab8d2 100644 --- a/apps/backend/internal/agentctl/server/process/manager.go +++ b/apps/backend/internal/agentctl/server/process/manager.go @@ -94,6 +94,7 @@ const processDefaultExitGrace = 5 * time.Second const processGroupTerminateGrace = 2 * time.Second const processGroupPollInterval = 50 * time.Millisecond +const processStderrDrainTimeout = time.Second // Manager manages the agent subprocess type Manager struct { @@ -1209,10 +1210,12 @@ func (m *Manager) Start(ctx context.Context) error { return errors.Join(fmt.Errorf("failed to connect adapter: %w", err), reapErr) } - // Start stderr reader and exit waiter + // Start stderr reader and exit waiter. Keep the completion channel local to + // this process generation so a delayed reader cannot signal a replacement. + stderrDone := make(chan struct{}) m.wg.Add(2) - go m.readStderr() - go m.waitForExit() + go m.readStderr(stderrDone) + go m.waitForExit(stderrDone) // Forward adapter updates to our channel m.wg.Add(1) @@ -2254,9 +2257,10 @@ func waitForProcessGroupExit(ctx context.Context, pid int) bool { } } -// readStderr reads and logs stderr from the agent -func (m *Manager) readStderr() { +// readStderr reads and logs stderr from the agent. +func (m *Manager) readStderr(stderrDone chan<- struct{}) { defer m.wg.Done() + defer close(stderrDone) scanner := bufio.NewScanner(m.stderr) for scanner.Scan() { @@ -2272,6 +2276,9 @@ func (m *Manager) readStderr() { if m.stderrSanitizer != nil { line, keep = m.stderrSanitizer.SanitizeStderrLine(rawLine) } + if !keep { + line, keep = safeManagedNpmStderrLine(rawLine) + } if !keep || line == "" { continue } @@ -2286,6 +2293,19 @@ func (m *Manager) readStderr() { } } +func (m *Manager) waitForStderrDrain(stderrDone <-chan struct{}) { + if stderrDone == nil { + return + } + timer := time.NewTimer(processStderrDrainTimeout) + defer timer.Stop() + select { + case <-stderrDone: + case <-timer.C: + m.logger.Warn("timed out waiting for agent stderr to drain") + } +} + // ansiEscapeRegex matches ANSI escape sequences var ansiEscapeRegex = regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]`) @@ -2327,12 +2347,17 @@ func (m *Manager) ClearStderrBuffer() { } // waitForExit waits for the process to exit -func (m *Manager) waitForExit() { +func (m *Manager) waitForExit(stderrDone <-chan struct{}) { defer m.wg.Done() defer close(m.doneCh) pid := m.agentPID() + // StderrPipe must be drained before Wait closes its parent-side pipe. The + // second bounded wait covers a reader that was still blocked when the + // process-exit wait began and was released by Wait closing the pipe. + m.waitForStderrDrain(stderrDone) err := m.cmd.Wait() + m.waitForStderrDrain(stderrDone) intentionalStop := m.Status() == StatusStopping switch { diff --git a/apps/backend/internal/agentctl/server/process/manager_command.go b/apps/backend/internal/agentctl/server/process/manager_command.go index 9bfeea78f3..dcc92d3448 100644 --- a/apps/backend/internal/agentctl/server/process/manager_command.go +++ b/apps/backend/internal/agentctl/server/process/manager_command.go @@ -19,12 +19,26 @@ func (m *Manager) CommandEnvironment() (map[string]string, error) { return mergeAgentEnvIntoShellConfigWithError(m.agentEnvSnapshot(), nil) } +// Output runs a direct command as an instance-owned process and returns bounded +// stdout after its complete process tree is reaped. Stderr is drained but is +// not joined to stdout, so callers that parse stdout cannot mistake a warning +// path for command output. +func (m *Manager) Output(parent context.Context, spec tools.CommandSpec) ([]byte, error) { + stdout, _, err := m.runManagedCommand(parent, spec) + return stdout, err +} + // CombinedOutput runs a direct command as an instance-owned process and // returns bounded stdout/stderr after its complete process tree is reaped. func (m *Manager) CombinedOutput(parent context.Context, spec tools.CommandSpec) ([]byte, error) { + stdout, stderr, err := m.runManagedCommand(parent, spec) + return append(stdout, stderr...), err +} + +func (m *Manager) runManagedCommand(parent context.Context, spec tools.CommandSpec) ([]byte, []byte, error) { ctx, release, err := m.BeginOwnedOperation(parent) if err != nil { - return nil, err + return nil, nil, err } defer release() @@ -36,7 +50,7 @@ func (m *Manager) CombinedOutput(parent context.Context, spec tools.CommandSpec) sessionID = "managed-command" } if err := ctx.Err(); err != nil { - return nil, err + return nil, nil, err } proc, err := m.StartPipedProcess(PipedStartRequest{ SessionID: sessionID, @@ -49,7 +63,7 @@ func (m *Manager) CombinedOutput(parent context.Context, spec tools.CommandSpec) PipeStderr: true, }) if err != nil { - return nil, err + return nil, nil, err } _ = proc.Stdin.Close() stdout := collectManagedCommandOutput(proc.Stdout) @@ -73,15 +87,16 @@ func (m *Manager) CombinedOutput(parent context.Context, spec tools.CommandSpec) cancel() } if !finished { - return nil, errors.Join(ctx.Err(), stopErr) + return nil, nil, errors.Join(ctx.Err(), stopErr) } - output := append(<-stdout, (<-stderr)...) + stdoutOutput := <-stdout + stderrOutput := <-stderr waitErr := proc.Wait() if canceled { - return output, errors.Join(ctx.Err(), stopErr, waitErr) + return stdoutOutput, stderrOutput, errors.Join(ctx.Err(), stopErr, waitErr) } - return output, waitErr + return stdoutOutput, stderrOutput, waitErr } func collectManagedCommandOutput(reader io.ReadCloser) <-chan []byte { diff --git a/apps/backend/internal/agentctl/server/process/manager_killgroup_test.go b/apps/backend/internal/agentctl/server/process/manager_killgroup_test.go index eee4a078f3..b7176e63bf 100644 --- a/apps/backend/internal/agentctl/server/process/manager_killgroup_test.go +++ b/apps/backend/internal/agentctl/server/process/manager_killgroup_test.go @@ -194,8 +194,10 @@ func TestWaitForExit_ReapsProcessGroupAfterNaturalLeaderExit(t *testing.T) { }) childPID := waitForChildPID(t, pidFile, 5*time.Second) + stderrDone := make(chan struct{}) + close(stderrDone) m.wg.Add(1) - go m.waitForExit() + go m.waitForExit(stderrDone) select { case <-m.doneCh: @@ -224,8 +226,10 @@ func TestWaitForExit_DoesNotPublishErrorForIntentionalStop(t *testing.T) { _ = killProcessGroup(parentPID) }) + stderrDone := make(chan struct{}) + close(stderrDone) m.wg.Add(1) - go m.waitForExit() + go m.waitForExit(stderrDone) require.NoError(t, killProcessGroup(parentPID)) select { diff --git a/apps/backend/internal/agentctl/server/process/manager_stderr_exit_test.go b/apps/backend/internal/agentctl/server/process/manager_stderr_exit_test.go new file mode 100644 index 0000000000..c12c3172fa --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/manager_stderr_exit_test.go @@ -0,0 +1,220 @@ +package process + +import ( + "io" + "os" + "os/exec" + "strings" + "sync" + "testing" + "time" + + "github.com/kandev/kandev/internal/agentctl/server/adapter" + "github.com/stretchr/testify/require" +) + +type gatedStderrReader struct { + started chan<- struct{} + release <-chan struct{} + data []byte + done bool +} + +func (r *gatedStderrReader) Read(p []byte) (int, error) { + if r.done { + return 0, io.EOF + } + close(r.started) + <-r.release + r.done = true + return copy(p, r.data), nil +} + +func (r *gatedStderrReader) Close() error { return nil } + +func TestWaitForExitWaitsForStderrReaderBeforePublishingError(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + cmd := exec.Command(os.Args[0], "-test.run=TestManagerProcessExitHelper") + cmd.Env = append(os.Environ(), "KANDEV_MANAGER_PROCESS_EXIT_HELPER=1") + cmd.Stderr = io.Discard + + m := &Manager{ + cmd: cmd, + stderr: &gatedStderrReader{started: started, release: release, data: []byte("npm error code ETARGET\n")}, + logger: newTestLogger(t), + doneCh: make(chan struct{}), + updatesCh: make(chan adapter.AgentEvent, 1), + } + m.status.Store(StatusRunning) + require.NoError(t, cmd.Start()) + var releaseOnce sync.Once + releaseReader := func() { + releaseOnce.Do(func() { close(release) }) + } + t.Cleanup(func() { + releaseReader() + if cmd.ProcessState == nil && cmd.Process != nil { + _ = cmd.Process.Kill() + } + m.wg.Wait() + }) + stderrDone := make(chan struct{}) + m.wg.Add(2) + go m.readStderr(stderrDone) + + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("stderr reader did not start") + } + + waitDone := make(chan struct{}) + go func() { + m.waitForExit(stderrDone) + close(waitDone) + }() + select { + case <-waitDone: + t.Fatal("waitForExit published the exit before stderr drained") + case <-time.After(100 * time.Millisecond): + } + + releaseReader() + select { + case <-waitDone: + case <-time.After(time.Second): + t.Fatal("waitForExit did not finish after stderr drained") + } + m.wg.Wait() + + event := <-m.updatesCh + recent, ok := event.Data["recent_stderr"].([]string) + if !ok { + t.Fatalf("recent_stderr = %#v", event.Data["recent_stderr"]) + } + require.Equal(t, []string{"npm error code ETARGET"}, recent) +} + +func TestReadStderrClosesOnlyItsGenerationChannel(t *testing.T) { + oldDone := make(chan struct{}) + replacementDone := make(chan struct{}) + m := &Manager{ + stderr: io.NopCloser(strings.NewReader("old generation\n")), + logger: newTestLogger(t), + } + m.wg.Add(1) + m.readStderr(oldDone) + + select { + case <-oldDone: + default: + t.Fatal("stderr reader did not signal its generation") + } + select { + case <-replacementDone: + t.Fatal("stderr reader signaled a replacement generation") + default: + } +} + +func TestReadStderrDelayedGenerationCannotCloseReplacementChannel(t *testing.T) { + oldStarted := make(chan struct{}) + oldRelease := make(chan struct{}) + replacementStarted := make(chan struct{}) + replacementRelease := make(chan struct{}) + oldDone := make(chan struct{}) + replacementDone := make(chan struct{}) + m := &Manager{ + logger: newTestLogger(t), + } + var oldReleaseOnce sync.Once + var replacementReleaseOnce sync.Once + releaseOld := func() { + oldReleaseOnce.Do(func() { close(oldRelease) }) + } + releaseReplacement := func() { + replacementReleaseOnce.Do(func() { close(replacementRelease) }) + } + t.Cleanup(func() { + releaseOld() + releaseReplacement() + m.wg.Wait() + }) + + m.stderr = &gatedStderrReader{ + started: oldStarted, + release: oldRelease, + data: []byte("old generation\n"), + } + m.wg.Add(1) + go m.readStderr(oldDone) + select { + case <-oldStarted: + case <-time.After(time.Second): + t.Fatal("old stderr reader did not start") + } + + // The old reader outlives the bounded drain. A replacement starts before + // the old reader is released, which is the interleaving that used to close + // the replacement generation's completion channel. + m.waitForStderrDrain(oldDone) + select { + case <-oldDone: + t.Fatal("old stderr reader finished before the replacement started") + default: + } + + m.stderr = &gatedStderrReader{ + started: replacementStarted, + release: replacementRelease, + data: []byte("replacement generation\n"), + } + m.wg.Add(1) + go m.readStderr(replacementDone) + select { + case <-replacementStarted: + case <-time.After(time.Second): + t.Fatal("replacement stderr reader did not start") + } + + releaseOld() + select { + case <-oldDone: + case <-time.After(time.Second): + t.Fatal("old stderr reader did not finish after release") + } + select { + case <-replacementDone: + t.Fatal("old stderr reader closed the replacement channel") + default: + } + + releaseReplacement() + select { + case <-replacementDone: + case <-time.After(time.Second): + t.Fatal("replacement stderr reader did not finish after release") + } +} + +func TestReadStderrPreservesSafeManagedNpmResolutionLines(t *testing.T) { + m := &Manager{ + stderr: io.NopCloser(strings.NewReader(strings.Join([]string{ + "npm error code ETARGET", + "npm error notarget No matching version found for opencode-ai@1.18.18", + "npm error path /tmp/private-path", + }, "\n"))), + stderrSanitizer: stderrLineSanitizerFunc(func(string) (string, bool) { + return "", false + }), + logger: newTestLogger(t), + } + m.wg.Add(1) + m.readStderr(make(chan struct{})) + + require.Equal(t, []string{ + "npm error code ETARGET", + "npm error notarget No matching version found for opencode-ai@1.18.18", + }, m.GetRecentStderr()) +} diff --git a/apps/backend/internal/agentctl/server/process/manager_test.go b/apps/backend/internal/agentctl/server/process/manager_test.go index 7f44277f25..800f844911 100644 --- a/apps/backend/internal/agentctl/server/process/manager_test.go +++ b/apps/backend/internal/agentctl/server/process/manager_test.go @@ -86,7 +86,7 @@ func TestManagerReadStderrDeliversCleanedLinesToOptionalConsumer(t *testing.T) { logger: newTestLogger(t), } m.wg.Add(1) - m.readStderr() + m.readStderr(make(chan struct{})) got := []string{<-consumer.lines, <-consumer.lines} want := []string{"quota", "plain"} @@ -135,9 +135,10 @@ func TestManagerProcessExitUsesSanitizedStderr(t *testing.T) { if err := cmd.Start(); err != nil { t.Fatalf("start process: %v", err) } + stderrDone := make(chan struct{}) m.wg.Add(2) - go m.readStderr() - m.waitForExit() + go m.readStderr(stderrDone) + m.waitForExit(stderrDone) m.wg.Wait() event := <-m.updatesCh diff --git a/apps/backend/internal/agentctl/server/process/runner_combined_test.go b/apps/backend/internal/agentctl/server/process/runner_combined_test.go index 075440a022..780abcf0e2 100644 --- a/apps/backend/internal/agentctl/server/process/runner_combined_test.go +++ b/apps/backend/internal/agentctl/server/process/runner_combined_test.go @@ -60,3 +60,21 @@ func TestManagerCombinedOutputCapturesFastExitStdoutAndStderr(t *testing.T) { t.Fatalf("CombinedOutput() output = %q, want both streams", output) } } + +func TestManagerOutputCapturesOnlyStdout(t *testing.T) { + mgr := NewManager(&config.InstanceConfig{WorkDir: t.TempDir(), SessionID: "session-1"}, newTestLogger(t)) + t.Cleanup(func() { _ = mgr.StopForTeardown(context.Background()) }) + command, env := fixtureExec("write-both stdout-only stderr-only") + + output, err := mgr.Output(context.Background(), tools.CommandSpec{ + Path: command[0], + Args: command[1:], + Env: env, + }) + if err != nil { + t.Fatalf("Output() error = %v", err) + } + if string(output) != "stdout-only" { + t.Fatalf("Output() = %q, want stdout only", output) + } +} diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index a74e75e76e..bd77343b8b 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -867,9 +867,6 @@ func startGatewayAndServe( // Wire the host utility manager into the settings controller so // /api/v1/agent-models/:agentName reads live capability data. agentSettingsController.SetHostUtility(hostUtilityMgr) - // Reuse the settings-owned exact npm cache boundary for managed-runtime - // startup recovery. The lifecycle manager never derives cache paths. - lifecycleMgr.SetManagedRuntimeCacheInvalidator(agentSettingsController) profileReconciler := agentsettingscontroller.NewProfileReconciler(hostUtilityMgr, agentRegistry, repos.AgentSettings, log) go func() { if err := hostUtilityMgr.Start(ctx); err != nil { diff --git a/apps/web/e2e/fixtures/docker-probe.ts b/apps/web/e2e/fixtures/docker-probe.ts index 564f375a35..66af600a50 100644 --- a/apps/web/e2e/fixtures/docker-probe.ts +++ b/apps/web/e2e/fixtures/docker-probe.ts @@ -15,6 +15,7 @@ export const E2E_ALPINE_IMAGE_TAG = "kandev-agent:e2e-alpine"; /** Label value shared by resources created in this Playwright process. */ export const E2E_DOCKER_SCOPE = `e2e-${process.pid}-${randomUUID().slice(0, 8)}`; const FAKE_LSP_SERVER = path.resolve(__dirname, "fake-lsp-server.mjs"); +const MANAGED_RUNTIME_NPX_WRAPPER = path.resolve(__dirname, "managed-runtime-npx.sh"); const SCOPED_CONTAINER_CLEANUP_TIMEOUT_MS = 30_000; const SCOPED_CONTAINER_CLEANUP_POLL_MS = 250; const SCOPED_CONTAINER_CLEANUP_EMPTY_POLLS = 8; @@ -24,12 +25,16 @@ RUN apt-get update \\ && apt-get install -y --no-install-recommends git ca-certificates curl \\ && rm -rf /var/lib/apt/lists/* COPY --chmod=0755 fake-lsp-server.mjs /usr/local/bin/kotlin-lsp +RUN rm -f /usr/local/bin/npx +COPY --chmod=0755 managed-runtime-npx.sh /usr/local/bin/npx WORKDIR /workspace `; const E2E_ALPINE_DOCKERFILE = `FROM alpine:latest RUN apk add --no-cache bash ca-certificates curl git nodejs npm COPY --chmod=0755 fake-lsp-server.mjs /usr/local/bin/kotlin-lsp +RUN rm -f /usr/local/bin/npx +COPY --chmod=0755 managed-runtime-npx.sh /usr/local/bin/npx WORKDIR /workspace `; @@ -64,6 +69,7 @@ function buildImage(tag: string, dockerfile: string): void { try { fs.writeFileSync(path.join(tmpDir, "Dockerfile"), dockerfile); fs.copyFileSync(FAKE_LSP_SERVER, path.join(tmpDir, "fake-lsp-server.mjs")); + fs.copyFileSync(MANAGED_RUNTIME_NPX_WRAPPER, path.join(tmpDir, "managed-runtime-npx.sh")); execFileSync("docker", ["build", "-t", tag, tmpDir], { stdio: process.env.E2E_DEBUG ? "inherit" : "ignore", }); diff --git a/apps/web/e2e/fixtures/managed-runtime-npx.sh b/apps/web/e2e/fixtures/managed-runtime-npx.sh new file mode 100644 index 0000000000..90200abd25 --- /dev/null +++ b/apps/web/e2e/fixtures/managed-runtime-npx.sh @@ -0,0 +1,43 @@ +#!/bin/sh +set -eu + +cache_root=${NPM_CONFIG_CACHE:-${npm_config_cache:-"$HOME/.npm"}} +package_spec=${3:-} +preference=${2:-} +managed_package_spec=opencode-ai@1.18.18 +real_npx=/usr/bin/npx + +# This image replaces npx only to make the selected managed runtime failure +# deterministic. Let every other package or invocation use the image's real +# npm implementation. +if [ "$package_spec" != "$managed_package_spec" ]; then + exec "$real_npx" "$@" +fi + +key=$(printf '%s' "$package_spec" | sha512sum | cut -c1-16) +target_dir="$cache_root/_npx/$key" +sibling_dir="$cache_root/_npx/0123456789abcdef" +online_invocations="$cache_root/online-invocations" + +if [ "$preference" = "--prefer-offline" ]; then + mkdir -p "$target_dir" "$sibling_dir" + printf 'stale\n' > "$target_dir/stale-marker" + printf 'sibling\n' > "$sibling_dir/sibling-marker" + printf 'npm error code ETARGET\n' >&2 + printf 'npm error notarget No matching version found for %s\n' "$package_spec" >&2 + exit 1 +fi + +if [ "$preference" = "--prefer-online" ]; then + if [ -e "$target_dir/stale-marker" ]; then + printf 'stale managed runtime marker was not removed for %s\n' "$package_spec" >&2 + exit 1 + fi + mkdir -p "$target_dir" + printf 'fresh\n' > "$target_dir/fresh-marker" + printf '%s\n' "$package_spec" >> "$online_invocations" + shift 3 + exec /usr/local/bin/mock-agent "$@" +fi + +exec "$real_npx" "$@" diff --git a/apps/web/e2e/fixtures/ssh-image.ts b/apps/web/e2e/fixtures/ssh-image.ts index dd3fff1eac..d060b8acaa 100644 --- a/apps/web/e2e/fixtures/ssh-image.ts +++ b/apps/web/e2e/fixtures/ssh-image.ts @@ -75,6 +75,8 @@ RUN apk add --no-cache \\ curl \\ sudo \\ shadow \\ + nodejs \\ + npm \\ coreutils \\ ca-certificates \\ iproute2 \\ @@ -109,7 +111,10 @@ RUN apk add --no-cache \\ # Go suite already produces). Without it agentctl can't spawn an agent # subprocess; with it, the SSH executor can drive a full task end-to-end. COPY mock-agent-linux-amd64 /usr/local/bin/mock-agent +RUN rm -f /usr/local/bin/npx +COPY managed-runtime-npx.sh /usr/local/bin/npx RUN chmod +x /usr/local/bin/mock-agent +RUN chmod +x /usr/local/bin/npx COPY entrypoint.sh /entrypoint.sh RUN chmod +x /entrypoint.sh @@ -151,6 +156,10 @@ export function buildE2ESSHImage(): void { fs.writeFileSync(path.join(ctxDir, "Dockerfile"), DOCKERFILE); fs.writeFileSync(path.join(ctxDir, "entrypoint.sh"), ENTRYPOINT); fs.copyFileSync(mockAgentPath, path.join(ctxDir, "mock-agent-linux-amd64")); + fs.copyFileSync( + path.resolve(__dirname, "managed-runtime-npx.sh"), + path.join(ctxDir, "managed-runtime-npx.sh"), + ); execFileSync("docker", ["build", "-t", SSH_E2E_IMAGE_TAG, ctxDir], { stdio: process.env.E2E_DEBUG ? "inherit" : "ignore", // 15-minute ceiling — even cold-cache builds (apk add openssh-server diff --git a/apps/web/e2e/helpers/docker.ts b/apps/web/e2e/helpers/docker.ts index aec5e7e427..89a70d53e2 100644 --- a/apps/web/e2e/helpers/docker.ts +++ b/apps/web/e2e/helpers/docker.ts @@ -73,6 +73,14 @@ export function dockerFileContent(containerID: string, filePath: string): string return res.stdout; } +export function dockerPathExists(containerID: string, filePath: string): boolean { + return ( + spawnSync("docker", ["exec", containerID, "test", "-e", filePath], { + stdio: "ignore", + }).status === 0 + ); +} + export async function waitForDockerContainerRemoved( containerID: string, message: string, diff --git a/apps/web/e2e/helpers/managed-runtime-recovery.ts b/apps/web/e2e/helpers/managed-runtime-recovery.ts new file mode 100644 index 0000000000..85a79f57de --- /dev/null +++ b/apps/web/e2e/helpers/managed-runtime-recovery.ts @@ -0,0 +1,63 @@ +import { expect } from "@playwright/test"; +import { createHash } from "node:crypto"; +import type { BackendContext } from "../fixtures/backend"; +import type { ApiClient } from "./api-client"; +import type { AgentProfile } from "../../lib/types/http-agents"; + +export const MANAGED_RUNTIME_PACKAGE_SPEC = "opencode-ai@1.18.18"; +export const MANAGED_RUNTIME_CACHE_ROOT = "/tmp/kandev-managed-npm-cache"; + +export function managedRuntimeExecutionCacheKey( + packageSpec = MANAGED_RUNTIME_PACKAGE_SPEC, +): string { + return createSha512(packageSpec).slice(0, 16); +} + +/** + * The real managed OpenCode agent is enabled only for this container-backed + * test. Its command runs through the image's npx wrapper, while the wrapper + * starts the Linux mock ACP binary on the online retry. + */ +export async function prepareManagedRuntimeProfile( + apiClient: ApiClient, + backend: BackendContext, +): Promise { + await backend.restart({ KANDEV_MOCK_AGENT: "true" }); + + let agentId = ""; + let observedAgents = ""; + try { + await expect + .poll( + async () => { + const { agents } = await apiClient.listAgents(); + observedAgents = agents.map((agent) => `${agent.id}:${agent.name}`).join(", "); + agentId = agents.find((agent) => agent.name === "opencode-acp")?.id ?? ""; + return agentId; + }, + { + timeout: 30_000, + message: "OpenCode managed runtime should be registered for container recovery", + }, + ) + .not.toBe(""); + } catch (error) { + throw new Error( + `${error instanceof Error ? error.message : String(error)}; agents=${observedAgents}`, + ); + } + + return apiClient.createAgentProfile(agentId, "E2E managed npm recovery", { + model: "mock-fast", + env_vars: [{ key: "NPM_CONFIG_CACHE", value: MANAGED_RUNTIME_CACHE_ROOT }], + }); +} + +/** Restore the normal e2e-only mock registry after a managed-runtime test. */ +export async function restoreE2EAgentRegistry(backend: BackendContext): Promise { + await backend.restart(); +} + +function createSha512(value: string): string { + return createHash("sha512").update(value).digest("hex"); +} diff --git a/apps/web/e2e/tests/docker/managed-runtime-npm-recovery.spec.ts b/apps/web/e2e/tests/docker/managed-runtime-npm-recovery.spec.ts new file mode 100644 index 0000000000..e2c7d04854 --- /dev/null +++ b/apps/web/e2e/tests/docker/managed-runtime-npm-recovery.spec.ts @@ -0,0 +1,62 @@ +import { test, expect } from "../../fixtures/docker-test-base"; +import { dockerFileContent, dockerPathExists } from "../../helpers/docker"; +import { waitForLatestSessionDone } from "../../helpers/session"; +import { + MANAGED_RUNTIME_CACHE_ROOT, + MANAGED_RUNTIME_PACKAGE_SPEC, + managedRuntimeExecutionCacheKey, + prepareManagedRuntimeProfile, + restoreE2EAgentRegistry, +} from "../../helpers/managed-runtime-recovery"; +import { SessionPage } from "../../pages/session-page"; + +test.describe("Docker executor - managed npm runtime recovery", () => { + test("repairs the executor cache and completes the original session", async ({ + apiClient, + backend, + seedData, + testPage, + }) => { + test.setTimeout(240_000); + let profileId = ""; + try { + const profile = await prepareManagedRuntimeProfile(apiClient, backend); + profileId = profile.id; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Docker managed npm recovery", + profile.id, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + executor_profile_id: seedData.dockerExecutorProfileId, + }, + ); + + await waitForLatestSessionDone(apiClient, task.id, 1, "Wait for Docker managed recovery"); + const environment = await apiClient.getTaskEnvironment(task.id); + expect(environment?.container_id).toBeTruthy(); + const target = `${MANAGED_RUNTIME_CACHE_ROOT}/_npx/${managedRuntimeExecutionCacheKey()}`; + const sibling = `${MANAGED_RUNTIME_CACHE_ROOT}/_npx/0123456789abcdef`; + const containerID = environment!.container_id!; + expect(dockerPathExists(containerID, `${target}/stale-marker`)).toBe(false); + expect(dockerFileContent(containerID, `${target}/fresh-marker`)).toBe("fresh\n"); + expect(dockerFileContent(containerID, `${sibling}/sibling-marker`)).toBe("sibling\n"); + const onlineInvocations = dockerFileContent( + containerID, + `${MANAGED_RUNTIME_CACHE_ROOT}/online-invocations`, + ); + expect(onlineInvocations.trim().split(/\r?\n/)).toEqual([MANAGED_RUNTIME_PACKAGE_SPEC]); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.activeChat().getByTestId("managed-runtime-npm-recovery")).toHaveCount(0); + } finally { + if (profileId) await apiClient.deleteAgentProfile(profileId, true).catch(() => undefined); + await restoreE2EAgentRegistry(backend); + } + }); +}); diff --git a/apps/web/e2e/tests/ssh/managed-runtime-npm-recovery.spec.ts b/apps/web/e2e/tests/ssh/managed-runtime-npm-recovery.spec.ts new file mode 100644 index 0000000000..c22277e0e0 --- /dev/null +++ b/apps/web/e2e/tests/ssh/managed-runtime-npm-recovery.spec.ts @@ -0,0 +1,64 @@ +import { test, expect } from "../../fixtures/ssh-test-base"; +import { readRemoteFile, remotePathExists } from "../../helpers/ssh"; +import { waitForLatestSessionDone } from "../../helpers/session"; +import { + MANAGED_RUNTIME_CACHE_ROOT, + MANAGED_RUNTIME_PACKAGE_SPEC, + managedRuntimeExecutionCacheKey, + prepareManagedRuntimeProfile, + restoreE2EAgentRegistry, +} from "../../helpers/managed-runtime-recovery"; +import { SessionPage } from "../../pages/session-page"; + +test.describe("SSH executor - managed npm runtime recovery", () => { + test("repairs the remote cache and completes the original session", async ({ + apiClient, + backend, + seedData, + testPage, + }) => { + test.setTimeout(240_000); + let profileId = ""; + try { + const profile = await prepareManagedRuntimeProfile(apiClient, backend); + profileId = profile.id; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "SSH managed npm recovery", + profile.id, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + executor_profile_id: seedData.sshExecutorProfileId, + }, + ); + + await waitForLatestSessionDone(apiClient, task.id, 1, "Wait for SSH managed recovery"); + const session = (await apiClient.listSSHSessions(seedData.sshExecutorId)).find( + (candidate) => candidate.task_id === task.id, + ); + expect(session?.remote_task_dir).toBeTruthy(); + const target = `${MANAGED_RUNTIME_CACHE_ROOT}/_npx/${managedRuntimeExecutionCacheKey()}`; + const sibling = `${MANAGED_RUNTIME_CACHE_ROOT}/_npx/0123456789abcdef`; + expect(remotePathExists(seedData.sshTarget, `${target}/stale-marker`)).toBe(false); + expect(readRemoteFile(seedData.sshTarget, `${target}/fresh-marker`)).toBe("fresh\n"); + expect(readRemoteFile(seedData.sshTarget, `${sibling}/sibling-marker`)).toBe("sibling\n"); + const onlineInvocations = readRemoteFile( + seedData.sshTarget, + `${MANAGED_RUNTIME_CACHE_ROOT}/online-invocations`, + ); + expect(onlineInvocations.trim().split(/\r?\n/)).toEqual([MANAGED_RUNTIME_PACKAGE_SPEC]); + expect(MANAGED_RUNTIME_PACKAGE_SPEC).toBe("opencode-ai@1.18.18"); + + await testPage.goto(`/t/${task.id}`); + const page = new SessionPage(testPage); + await page.waitForLoad(); + await expect(page.activeChat().getByTestId("managed-runtime-npm-recovery")).toHaveCount(0); + } finally { + if (profileId) await apiClient.deleteAgentProfile(profileId, true).catch(() => undefined); + await restoreE2EAgentRegistry(backend); + } + }); +}); diff --git a/docs/decisions/2026-08-12-validated-managed-runtime-version-selection.md b/docs/decisions/2026-08-12-validated-managed-runtime-version-selection.md index 32b492306d..56d6deba75 100644 --- a/docs/decisions/2026-08-12-validated-managed-runtime-version-selection.md +++ b/docs/decisions/2026-08-12-validated-managed-runtime-version-selection.md @@ -1,6 +1,6 @@ # ADR-2026-08-12-validated-managed-runtime-version-selection: Validate and Persist Managed Runtime Version Selection -**Status:** accepted (amended 2026-08-21) +**Status:** accepted (amended 2026-08-21 and 2026-08-24) **Date:** 2026-08-12 **Area:** backend, frontend, protocol, workflow **Supersedes:** @@ -88,6 +88,12 @@ manual updates, and the bounded stale-metadata retry use online-preferred metadata. Kandev accepts that a cold or incomplete npm cache can still place the registry on the launch path. +The stale-metadata retry runs cache repair through the `agentctl` instance that +hosts the npm process. This boundary covers local PC, local Docker, and remote +SSH runtimes. It preserves the configured registry and the exact selected +version. [ADR-2026-08-24](2026-08-24-agentctl-local-managed-runtime-cache-repair.md) +owns the cache-repair placement and security rationale. + A weekly repository workflow compares the shipped defaults with each trusted package's stable npm `latest` tag and opens a reviewable PR for changes. It does not merge automatically. The same centralized version catalogue drives command diff --git a/docs/decisions/2026-08-24-agentctl-local-managed-runtime-cache-repair.md b/docs/decisions/2026-08-24-agentctl-local-managed-runtime-cache-repair.md new file mode 100644 index 0000000000..fa82137f04 --- /dev/null +++ b/docs/decisions/2026-08-24-agentctl-local-managed-runtime-cache-repair.md @@ -0,0 +1,53 @@ +# ADR-2026-08-24-agentctl-local-managed-runtime-cache-repair: Run cache repair where npm runs + +**Status:** accepted +**Date:** 2026-08-24 +**Area:** backend, agentctl, protocol, security + +## Context + +Kandev launches managed npm runtimes on the host, in local Docker, and through +remote SSH. Each location has a separate npm environment and cache. + +The first stale-metadata recovery ran cache repair in the Kandev backend. It +therefore supported only host-local standalone sessions. Docker and SSH +failures reached users as npm `ETARGET` errors. + +Executor-specific shell repair would duplicate cache discovery, authentication, +quoting, and cleanup logic. Automatic rollback would also violate exact +version selection. + +## Decision + +The lifecycle manager owns recovery policy and trusted command reconstruction. +The session-scoped `agentctl` instance owns npm cache discovery and exact cache +repair. + +The Kandev backend sends only the trusted exact package specification through +an authenticated agentctl request. Agentctl resolves the npm cache with the +configured agent environment. It removes only the deterministic `_npx` tree +for that specification. + +The same contract applies to local PC, local Docker, and remote SSH runtimes. +Kandev preserves the configured registry and retries the same package version +once with online-preferred metadata. + +## Consequences + +Cache repair runs in the environment that owns the failed npm process. The +backend does not need Docker or SSH shell implementations for this repair. + +Agentctl gains one authenticated maintenance action. Its process manager must +resolve npm configuration with the agent environment and retain process-tree +cleanup guarantees. + +Future executors can adopt automatic recovery only when they use this +authenticated colocated contract and have executor-specific evidence. + +## Alternatives considered + +- Backend-host repair was rejected because it cannot reach Docker or SSH caches. +- Executor-specific shell commands were rejected because they duplicate security and environment logic. +- Global npm cache cleanup was rejected because it removes unrelated user data. +- Automatic version rollback was rejected because it changes the selected runtime without operator consent. +- Registry replacement was rejected because Kandev must preserve operator network policy. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 880d90d372..af12bbff1d 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -201,4 +201,5 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-22-pr-walkthrough-description-link | [Own a top-level PR walkthrough callout](2026-08-22-pr-walkthrough-description-link.md) | accepted | workflow, infra, security, GitHub | 2026-08-22 | | 2026-08-23-pr-walkthrough-short-urls | [Use 12-character SHA prefixes for PR walkthrough URLs](2026-08-23-pr-walkthrough-short-urls.md) | accepted | workflow, infra | 2026-08-23 | | 2026-08-23-pr-walkthrough-workflow-provenance | [Use the workflow SHA for trusted PR walkthrough inputs](2026-08-23-pr-walkthrough-workflow-provenance.md) | accepted | workflow, infra, security | 2026-08-23 | +| 2026-08-24-agentctl-local-managed-runtime-cache-repair | [Run cache repair where npm runs](2026-08-24-agentctl-local-managed-runtime-cache-repair.md) | accepted | backend, agentctl, protocol, security | 2026-08-24 | | 2026-08-24-unified-fork-approval-label | [Use One Maintainer Approval Label for Contributor PR Automation](2026-08-24-unified-fork-approval-label.md) | accepted | infra, workflow, security | 2026-08-24 | diff --git a/docs/plans/executor-local-managed-runtime-recovery/plan.md b/docs/plans/executor-local-managed-runtime-recovery/plan.md new file mode 100644 index 0000000000..49a34f2027 --- /dev/null +++ b/docs/plans/executor-local-managed-runtime-recovery/plan.md @@ -0,0 +1,141 @@ +--- +created: 2026-08-24 +status: completed +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: + - ../../specs/agents/runtime-updates.md +--- + +# Implementation plan: Executor-local managed runtime recovery + +## Overview + +Move exact npm cache repair from the Kandev host to the colocated `agentctl` +instance. Then enable one transparent retry for local PC, local Docker, and +remote SSH runtimes. + +The API contract comes first because lifecycle recovery depends on it. Runtime +integration follows. Container-backed evidence and public documentation finish +the package. + +## Scope + +### In scope + +- Add an authenticated agentctl action for one exact npm execution tree. +- Resolve the npm cache with the configured agent process environment. +- Route standalone, Docker, and SSH recovery through the same action. +- Preserve the exact package, selected version, registry, executor, and session. +- Add focused integration and container-backed evidence. +- Update public recovery documentation after implementation. + +### Out of scope + +- Automatic rollback or another version selection. +- Registry replacement or global npm cache cleanup. +- Sprites, remote Docker, Kubernetes, native runtimes, and passthrough commands. +- New UI copy or recovery controls. + +## Technical approach + +### Colocated cache repair + +Add an authenticated agentctl endpoint under `/api/v1/agent/`. The request +carries only the trusted exact `package@version` specification. + +The agentctl process manager runs `npm config get cache` through its owned +command runner. This command inherits the agent environment. The endpoint then +calls `managedruntime.RemoveNpxExecutionTree` for the derived execution key. + +Add a typed method to `runtime/agentctl.Client`. Keep paths and raw npm output +inside agentctl. Return bounded errors to the lifecycle manager. + +### Lifecycle recovery + +Replace the backend-host `ManagedRuntimeCacheInvalidator` dependency with the +agentctl client method. Stop the failed child before repair. Keep the agentctl +server and its authenticated control connection alive. + +Allow recovery for `RuntimeStandalone`, `RuntimeDocker`, and `RuntimeSSH`. +Reject all other runtimes. Preserve the startup generation, cancellation gates, +and one-retry limit. + +### Executor evidence + +Add focused lifecycle integration tests with a real agentctl API server. Add +container-backed Playwright tests for Docker and SSH. + +The container tests use a test-only `npx` wrapper in the executor environment. +The wrapper emits the strict ETARGET signature for `--prefer-offline`. It +executes `/usr/local/bin/mock-agent` for the online retry. + +The tests assert that the original session completes. They also assert that +the stale managed-runtime marker is removed, a fresh marker is recreated, +sibling cache state is preserved, and no recovery card appears. + +### Documentation + +Update the recovery section in `docs/public/agents-and-profiles.md`. State the +three supported executor locations and the executor-local cache boundary. + +Update `ACP_BRIDGE_VERSIONS.md` if its host-local recovery text changes. Keep +the no-rollback, no-registry-change, and no-global-cleanup limits visible. + +## Tests + +| Acceptance criterion | Evidence | +| --- | --- | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.1` | Lifecycle integration emits ETARGET and observes one online retry. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.2` | Table tests cover standalone, Docker, and SSH runtime names. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3` | Recovery integration completes the original execution without terminal failure. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.4` | Command identity assertions compare every argument except npm preference. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.5` | Existing repeated-failure and recovery-card tests remain green. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.1` | Agentctl API test uses an environment-specific `NPM_CONFIG_CACHE`. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2` | Cache tests remove only the exact deterministic tree. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.3` | Sibling cache trees and registry environment values remain unchanged. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.4` | Unsafe-root and symbolic-link tests remain green. | +| `AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.5` | Lifecycle cancellation and shutdown tests stop before replacement launch. | + +The first regression test is +`TestRetryManagedRuntimeStartupSupportsAgentctlBackedExecutors`. It fails on +current code because Docker and SSH return before recovery starts. + +## E2E tests + +- `apps/web/e2e/tests/docker/managed-runtime-npm-recovery.spec.ts` covers local Docker recovery. +- `apps/web/e2e/tests/ssh/managed-runtime-npm-recovery.spec.ts` covers remote SSH recovery. +- Existing Kanban, Office, and mobile recovery-card tests cover terminal retry presentation. + +Run the two new specs in the `containers` project. Run existing presentation +specs in their owning desktop, Office, and mobile projects. + +## Work orders + +- [completed] [Task 01: Add executor-local cache repair](task-01-agentctl-cache-repair.md) +- [completed] [Task 02: Enable recovery across executors](task-02-lifecycle-executor-recovery.md) +- [completed] [Task 03: Prove container executor recovery](task-03-container-recovery-evidence.md) +- [completed] [Task 04: Document executor-local recovery](task-04-document-recovery.md) + +## Verification results + +Task 01 passed the focused agentctl, cache, process-manager, and client command +with 1,598 tests across 4 packages. Task 02 passed the lifecycle and +orchestrator command with 4,050 tests across 2 packages. The container-backed +Docker and SSH specs each +passed in the host-backed `containers` Playwright project after rebuilding the +backend, mock agent, and E2E plugin artifacts. The nested `e2e:run` wrapper was +not usable in this environment because its runtime had no Docker daemon. +Public documentation validation passed, including the published-doc tests, +published-doc validator, specification lint, and diff checks. + +## Risks + +- Cache discovery must use the agent environment, not the backend environment. +- Repair must finish before the startup retry closes and replaces the agent stream. +- A delayed first-child event must not fail the replacement process. +- The container E2E fixture must not call the public npm registry. +- The agentctl maintenance endpoint must not accept caller-controlled paths or commands. diff --git a/docs/plans/executor-local-managed-runtime-recovery/task-01-agentctl-cache-repair.md b/docs/plans/executor-local-managed-runtime-recovery/task-01-agentctl-cache-repair.md new file mode 100644 index 0000000000..a5262f1467 --- /dev/null +++ b/docs/plans/executor-local-managed-runtime-recovery/task-01-agentctl-cache-repair.md @@ -0,0 +1,94 @@ +--- +id: "01-agentctl-cache-repair" +title: "Add executor-local cache repair" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002 +acceptance_criteria: + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.1 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.3 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.4 +system_design: + - ../../specs/agents/system-design/managed-npm-runtime-recovery.md +--- + +# Task 01: Add executor-local cache repair + +## Summary + +Add an authenticated agentctl action that removes one exact npm execution tree. +Resolve the npm cache with the configured agent environment. + +## In scope + +- Add the typed agentctl request and response. +- Add the authenticated agentctl API route and handler. +- Resolve npm cache configuration through the process manager. +- Add the runtime agentctl client method. +- Preserve the existing exact-tree and symbolic-link protections. + +## Out of scope + +- Lifecycle retry policy. +- Executor eligibility. +- Version rollback or registry changes. + +## Acceptance + +- The request accepts one exact stable package specification and no path or command. +- Cache discovery uses the agent environment and the local agentctl process. +- The handler removes only the deterministic target tree and preserves siblings. + +## Verification + +```bash +go test ./internal/agent/managedruntime ./internal/agentctl/server/api ./internal/agentctl/server/process ./internal/agent/runtime/agentctl -count=1 +``` + +## Files likely touched + +- `apps/backend/internal/agent/managedruntime/cache.go` +- `apps/backend/internal/agent/managedruntime/cache_validation_test.go` +- `apps/backend/internal/agentctl/server/process/managed_runtime.go` +- `apps/backend/internal/agentctl/server/process/managed_runtime_test.go` +- `apps/backend/internal/agentctl/server/process/manager_command.go` +- `apps/backend/internal/agentctl/server/process/manager_stderr_exit_test.go` +- `apps/backend/internal/agentctl/server/api/server.go` +- `apps/backend/internal/agentctl/server/api/managed_runtime.go` +- `apps/backend/internal/agentctl/server/api/managed_runtime_test.go` +- `apps/backend/internal/agent/runtime/agentctl/client_managed_runtime.go` +- `apps/backend/internal/agent/runtime/agentctl/client_managed_runtime_test.go` + +## Dependencies + +None. + +## Risks + +- The endpoint can become too broad if it accepts a cache path or shell input. +- Cache discovery can target the wrong tree if it drops the agent environment. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002` +- Executor-local cache contract in the system design. +- Existing `hostRuntimeUpdater` and `RemoveNpxExecutionTree` tests. + +## Results + +- Added exact stable `package@version` validation and retained the existing + deterministic `_npx` tree and symlink protections. +- Added the authenticated agentctl cache-repair route and typed runtime client + method. The route resolves npm cache through the configured agent environment. +- Added API, client, managedruntime, and process-manager tests for exact-tree + repair, sibling preservation, unsafe selectors, and bounded cache discovery. +- Verification passed: + `go test ./internal/agent/managedruntime ./internal/agentctl/server/api ./internal/agentctl/server/process ./internal/agent/runtime/agentctl -count=1` diff --git a/docs/plans/executor-local-managed-runtime-recovery/task-02-lifecycle-executor-recovery.md b/docs/plans/executor-local-managed-runtime-recovery/task-02-lifecycle-executor-recovery.md new file mode 100644 index 0000000000..234b123726 --- /dev/null +++ b/docs/plans/executor-local-managed-runtime-recovery/task-02-lifecycle-executor-recovery.md @@ -0,0 +1,96 @@ +--- +id: "02-lifecycle-executor-recovery" +title: "Enable recovery across executors" +status: done +wave: 2 +depends_on: + - "01-agentctl-cache-repair" +plan: "plan.md" +requirements: + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001 + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002 +acceptance_criteria: + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.1 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.2 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.4 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.5 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.5 +system_design: + - ../../specs/agents/system-design/managed-npm-runtime-recovery.md +--- + +# Task 02: Enable recovery across executors + +## Summary + +Route startup repair through the execution-scoped agentctl client. Enable the same +one-retry flow for standalone, Docker, and SSH runtimes. + +## In scope + +- Add the failing executor-parity regression test first. +- Replace the backend-host cache invalidator in startup recovery. +- Allow only standalone, Docker, and SSH managed npm runtimes. +- Preserve cancellation, shutdown, startup generation, and command identity. +- Remove obsolete lifecycle wiring without changing manual Settings updates. + +## Out of scope + +- Settings version preparation and host update jobs. +- Sprites, remote Docker, Kubernetes, native runtimes, and passthrough commands. +- New UI behavior. + +## Acceptance + +- Standalone, Docker, and SSH start one online replacement after strict ETARGET evidence. +- The repair call reaches the same agentctl instance that reported the failed process. +- Unsupported runtimes and repeated failures remain terminal without another retry. + +## Verification + +```bash +go test ./internal/agent/runtime/lifecycle ./internal/orchestrator -count=1 +``` + +## Files likely touched + +- `apps/backend/internal/agent/runtime/lifecycle/manager.go` +- `apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup.go` +- `apps/backend/internal/agent/runtime/lifecycle/managed_runtime_startup_recovery_test.go` +- `apps/backend/internal/backendapp/main.go` +- `apps/backend/internal/orchestrator/event_handlers_test.go` + +## Dependencies + +- Task 01 supplies the executor-local agentctl action. + +## Risks + +- Closing the agent stream before repair can make the request unavailable. +- A stale event from the failed child can overwrite replacement state. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001` +- Recovery flow in the system design. +- Existing managed runtime startup generation tests. + +## Results + +- Added the failing executor-parity regression for Docker and SSH before the + lifecycle implementation. +- Recovery now calls the execution-scoped agentctl repair action for standalone, + Docker, and SSH, while remote Docker and Sprites remain unsupported. +- Removed the backend host cache-invalidator lifecycle dependency without + changing manual Settings cache updates. +- Startup generation guards defer stale process and completion events only for + tracked startup attempts, so a failed first child cannot terminate the + replacement while ordinary untracked lifecycle executions retain their + terminal behavior. +- `go test ./internal/agent/runtime/lifecycle ./internal/orchestrator -count=1` + passed with 4,050 tests across 2 packages. diff --git a/docs/plans/executor-local-managed-runtime-recovery/task-03-container-recovery-evidence.md b/docs/plans/executor-local-managed-runtime-recovery/task-03-container-recovery-evidence.md new file mode 100644 index 0000000000..4db12935aa --- /dev/null +++ b/docs/plans/executor-local-managed-runtime-recovery/task-03-container-recovery-evidence.md @@ -0,0 +1,97 @@ +--- +id: "03-container-recovery-evidence" +title: "Prove container executor recovery" +status: done +wave: 3 +depends_on: + - "02-lifecycle-executor-recovery" +plan: "plan.md" +requirements: + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001 + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002 +acceptance_criteria: + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.1 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.2 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.4 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.1 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2 +system_design: + - ../../specs/agents/system-design/managed-npm-runtime-recovery.md +--- + +# Task 03: Prove container executor recovery + +## Summary + +Add real container-backed evidence for local Docker and remote SSH. Use a +test-only npx wrapper and no public registry access. + +## In scope + +- Add one Docker executor recovery spec. +- Add one SSH executor recovery spec. +- Add the smallest shared fixture helper for the deterministic npx wrapper. +- Assert original-session completion, exact retry arguments, and no failure card. + +## Out of scope + +- New UI controls or copy. +- Live npm registry tests. +- Mobile layout changes. + +## Acceptance + +- The offline attempt emits the strict ETARGET signature inside each target. +- The online attempt starts the mock ACP agent in the same executor and session. +- Each test proves the exact tree is gone and an unrelated tree remains. + +## Verification + +```bash +KANDEV_E2E_CONTAINERS=1 pnpm e2e:run --project containers tests/docker/managed-runtime-npm-recovery.spec.ts +KANDEV_E2E_CONTAINERS=1 pnpm e2e:run --project containers tests/ssh/managed-runtime-npm-recovery.spec.ts +``` + +## Files likely touched + +- `apps/web/e2e/tests/docker/managed-runtime-npm-recovery.spec.ts` +- `apps/web/e2e/tests/ssh/managed-runtime-npm-recovery.spec.ts` +- `apps/web/e2e/fixtures/docker-probe.ts` +- `apps/web/e2e/fixtures/managed-runtime-npx.sh` +- `apps/web/e2e/fixtures/ssh-image.ts` +- `apps/web/e2e/helpers/managed-runtime-recovery.ts` + +## Dependencies + +- Task 02 completes the production recovery flow. + +## Risks + +- The fixture must keep the wrapper inside the target executor. +- The fixture must not enable unrelated real provider probes. + +## Parallelism + +`sequential` + +## Inputs + +- Docker and SSH E2E fixture contracts. +- Existing managed runtime recovery presentation specs. +- `/usr/local/bin/mock-agent` in both E2E target images. + +## Results + +- Docker recovery passed in the host-backed `containers` project with + `KANDEV_E2E_CONTAINERS=1 pnpm exec playwright test --config + e2e/playwright.config.ts --project=containers + tests/docker/managed-runtime-npm-recovery.spec.ts --workers=1`. +- SSH recovery passed with the corresponding `tests/ssh/managed-runtime-npm-recovery.spec.ts` + command. +- Both specs proved that the original session completed, the stale marker was + removed, a fresh marker was recreated, the sibling tree remained, exactly one + online invocation was recorded, and no recovery card was shown. +- The nested `e2e:run` wrapper could not start in this environment because its + runtime had no Docker daemon; the equivalent host-backed containers project + supplied the real Docker and SSH evidence. diff --git a/docs/plans/executor-local-managed-runtime-recovery/task-04-document-recovery.md b/docs/plans/executor-local-managed-runtime-recovery/task-04-document-recovery.md new file mode 100644 index 0000000000..05aa219017 --- /dev/null +++ b/docs/plans/executor-local-managed-runtime-recovery/task-04-document-recovery.md @@ -0,0 +1,84 @@ +--- +id: "04-document-recovery" +title: "Document executor-local recovery" +status: done +wave: 4 +depends_on: + - "03-container-recovery-evidence" +plan: "plan.md" +requirements: + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001 + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002 +acceptance_criteria: + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.2 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.4 + - AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.3 +system_design: + - ../../specs/agents/system-design/managed-npm-runtime-recovery.md +--- + +# Task 04: Document executor-local recovery + +## Summary + +Update public and bridge documentation after executor recovery passes. Explain +the supported locations and the limits that protect user npm configuration. + +## In scope + +- Update the public agent runtime recovery section. +- Update bridge version documentation when its recovery boundary changes. +- Keep recovery limits visible and task-oriented. + +## Out of scope + +- General npm troubleshooting. +- Manual cache cleanup instructions. +- New public pages or navigation changes. + +## Acceptance + +- Public documentation names local PC, local Docker, and remote SSH support. +- Documentation states that Kandev preserves the registry and exact version. +- Documentation states that Kandev never clears the global npm cache. + +## Verification + +```bash +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +git diff --check -- docs/public apps/backend/internal/agent/agents +``` + +## Files likely touched + +- `docs/public/agents-and-profiles.md` +- `apps/backend/internal/agent/agents/ACP_BRIDGE_VERSIONS.md` + +## Dependencies + +- Task 03 proves the documented executor behavior. + +## Risks + +- Documentation can imply automatic rollback if it does not name the same exact version. + +## Parallelism + +`sequential` + +## Inputs + +- Managed npm runtime recovery requirement and system design. +- Public agent and profile documentation. +- Existing bridge version documentation. + +## Results + +- Updated `docs/public/agents-and-profiles.md` with local PC, local Docker, and + remote SSH recovery behavior and the executor-local cache boundary. +- Updated `ACP_BRIDGE_VERSIONS.md` with the same exact-version, one-retry, and + no-global-cleanup limits. +- `node --test scripts/validate-public-docs.test.mjs` passed all 61 tests. +- `node scripts/validate-public-docs.mjs`, `python3 scripts/lint-spec-files.py + --all`, and the requested diff checks passed. diff --git a/docs/public/agents-and-profiles.md b/docs/public/agents-and-profiles.md index af01d644b1..937a51812b 100644 --- a/docs/public/agents-and-profiles.md +++ b/docs/public/agents-and-profiles.md @@ -96,10 +96,17 @@ resolution error, removes only the deterministic `_npx` execution tree for the selected package and version, then retries the same command once with an online-preferred metadata lookup. +The same recovery applies to managed runtime startup on a local PC, in a local +Docker executor, or in a remote SSH executor. Kandev sends the repair request +to the agentctl process that owns the failed execution. That process resolves +npm's cache with the agent environment and removes only the selected execution +tree. It does not repair the Kandev host cache, delete sibling execution trees, +change the registry, or clear the global npm cache. + The retry keeps the selected package, exact version, command prefix, model, permissions, and session identity. It does not change the npm registry or silently select another version. When the retry succeeds, no recovery card is -shown. When it fails again, Kanban and Office show one **Retry runtime** action +shown. When it fails again, Kandev and Office show one **Retry runtime** action with collapsed technical details. Do not use `npm cache clean --force` as the normal recovery step. It removes diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 8ac1799465..186a58553f 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -6,7 +6,7 @@ This catalog is the entry point for the system-oriented specification layout. Ea | System | Index | Migration | Canonical documents | | --- | --- | --- | --- | -| Agents | [README](agents/README.md) | complete | 25 requirements, 15 designs | +| Agents | [README](agents/README.md) | complete | 26 requirements, 16 designs | | Auth | [README](auth/README.md) | complete | 7 requirements, 2 designs | | CLI | [README](cli/README.md) | complete | 2 requirements, 0 designs | | Costs | [README](costs/README.md) | complete | 2 requirements, 0 designs | diff --git a/docs/specs/agents/README.md b/docs/specs/agents/README.md index 306eed5ee9..cf11249274 100644 --- a/docs/specs/agents/README.md +++ b/docs/specs/agents/README.md @@ -32,8 +32,6 @@ surface shared by task and Office consumers. ### Requirements - - - [Agent Resume and Runtime Recovery](requirements/agent-resume-runtime-recovery.md) - [Agent Rich Output](requirements/agent-rich-output.md) - [Agent Stall Recovery](requirements/agent-stall-recovery.md) @@ -54,6 +52,7 @@ surface shared by task and Office consumers. - [Disable an Agent Profile](requirements/profile-disable.md) - [Duplicate an Agent Profile](requirements/profile-duplicate.md) - [Agent Roles — Security, QA, and DevOps](requirements/roles.md) +- [Managed npm runtime recovery](requirements/managed-npm-runtime-recovery.md) - [Managed Agent Runtime Versions and Updates](requirements/runtime-updates.md) - [Simplify the agent settings profile layout](requirements/settings-profile-layout.md) - [Spawn Session Effective Agent Profile](requirements/spawn-session-effective-profile.md) @@ -62,14 +61,13 @@ surface shared by task and Office consumers. ### System design - - - [Dynamic Agent Routing System Design Part 1](system-design/dynamic-agent-routing-01.md) - [Dynamic Agent Routing System Design Part 2](system-design/dynamic-agent-routing-02.md) - [No Silent Model Fallback System Design Part 1](system-design/no-silent-model-fallback-01.md) - [No Silent Model Fallback System Design Part 2](system-design/no-silent-model-fallback-02.md) - [Managed Agent Runtime Versions and Updates System Design Part 1](system-design/runtime-updates-01.md) - [Managed Agent Runtime Versions and Updates System Design Part 2](system-design/runtime-updates-02.md) +- [Managed npm runtime recovery](system-design/managed-npm-runtime-recovery.md) - [Subagent context persistence System Design Part 1](system-design/subagent-context-persistence-01.md) - [Subagent context persistence System Design Part 2](system-design/subagent-context-persistence-02.md) - [Subagent context persistence System Design Part 3](system-design/subagent-context-persistence-03.md) diff --git a/docs/specs/agents/requirements/managed-npm-runtime-recovery.md b/docs/specs/agents/requirements/managed-npm-runtime-recovery.md new file mode 100644 index 0000000000..372a107068 --- /dev/null +++ b/docs/specs/agents/requirements/managed-npm-runtime-recovery.md @@ -0,0 +1,58 @@ +--- +status: draft +system: agents +created: 2026-08-24 +owners: + - kandev +--- + +# Managed npm runtime recovery requirements + +## Overview + +Managed npm runtimes use exact reviewed package versions. Stale npm metadata +can hide a published version and stop the agent before ACP initialization. + +Kandev repairs this error without requiring the end user to operate npm. The +same behavior applies on local PC, local Docker, and remote SSH executors. + +## Terminology + +- **Managed npm runtime:** A built-in ACP runtime that Kandev launches through an exact npm package version. +- **Execution tree:** The deterministic npm `_npx` directory for one exact package specification. +- **Executor-local:** An operation that runs in the environment that hosts the agent process and its npm cache. + +## Requirements + +### REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001: Transparent executor recovery + +**Intent:** Restore a managed runtime after stale npm metadata without changing the selected package, version, executor, or session. + +**User story:** As a Kandev user, I want runtime repair to occur automatically, so that I do not operate npm on an execution host. + +#### Acceptance criteria + +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.1:** When a supported executor reports strict npm `ETARGET` evidence before ACP initialization, Kandev shall retry the same runtime once. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.2:** The supported executors shall be local PC, local Docker, and remote SSH. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.3:** A successful retry shall continue the original session without a failure card or user action. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.4:** The retry shall preserve the trusted package, exact version, registry, command prefix, ACP arguments, model, permissions, executor, and session identity. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-001.5:** When the retry fails, Kandev shall report the npm preparation error and offer one **Retry runtime** action. + +### REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002: Scoped executor-local repair + +**Intent:** Repair only the cache entry that belongs to the failed trusted runtime. + +#### Acceptance criteria + +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.1:** Kandev shall resolve the npm cache with the failed agent process environment on its execution host. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.2:** Kandev shall remove only the deterministic execution tree for the trusted exact package specification. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.3:** Kandev shall preserve the configured npm registry, the global npm cache, and unrelated execution trees. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.4:** Cache repair shall reject broad roots, path-like package values, symbolic links, and paths outside the npm execution cache. +- **AC-AGENTS-MANAGED-RUNTIME-RECOVERY-002.5:** Cancellation and backend shutdown shall stop repair before a replacement process starts. + +## Out of scope + +- Automatic version rollback or selection of another package version. +- Registry replacement, dependency substitution, or global npm cache cleanup. +- Native runtimes, passthrough commands, unrelated npm errors, and a second online retry. +- Sprites, remote Docker, Kubernetes, and future executors without the same authenticated executor-local repair contract. diff --git a/docs/specs/agents/system-design/managed-npm-runtime-recovery.md b/docs/specs/agents/system-design/managed-npm-runtime-recovery.md new file mode 100644 index 0000000000..e93f0ffc1d --- /dev/null +++ b/docs/specs/agents/system-design/managed-npm-runtime-recovery.md @@ -0,0 +1,90 @@ +--- +status: draft +system: agents +requirements: + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001 + - REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002 +--- + +# Managed npm runtime recovery system design + +## Purpose and boundaries + +The lifecycle manager owns recovery policy and trusted command reconstruction. +The colocated `agentctl` instance owns cache discovery and exact cache repair. + +This split applies to `standalone`, `docker`, and `ssh` runtimes. The design +does not add executor-specific shell commands to the Kandev backend. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-001` | [Recovery flow](#recovery-flow) | +| `REQ-AGENTS-MANAGED-RUNTIME-RECOVERY-002` | [Executor-local cache contract](#executor-local-cache-contract) | + +## Components and responsibilities + +- `runtime/lifecycle.Manager` classifies bounded startup evidence and limits recovery to one retry. +- `runtime/agentctl.Client` calls the authenticated cache-repair endpoint on the session-scoped `agentctl` instance. +- `agentctl/server/api.Server` validates the request and coordinates the local repair operation. +- `agentctl/server/process.Manager` runs `npm config get cache` with the configured agent environment. +- `agent/managedruntime` validates the package specification and removes one deterministic `_npx` tree. + +## Executor-local cache contract + +The backend sends the trusted exact package specification to the authenticated +session-scoped `agentctl` API. The request does not contain a cache path, +registry URL, shell command, or package data from stderr. + +The `agentctl` process resolves the cache root with its current agent +environment. This environment includes `NPM_CONFIG_CACHE`, `HOME`, npm +configuration, and profile values that also affect the failed child process. + +The repair operation uses `managedruntime.RemoveNpxExecutionTree`. This helper +derives the `_npx` key from the trusted package specification. Its descriptor +walk rejects symbolic links and path replacement races. + +The endpoint requires the existing agentctl bearer token and instance identity. +It accepts one exact stable package specification. It returns no host path or +raw npm output. + +## Recovery flow + +1. Kandev starts the exact managed runtime with `--prefer-offline`. +2. The process exits before ACP initialization. +3. The lifecycle manager reads bounded sanitized stderr from `agentctl`. +4. Recovery requires npm `ETARGET` and a matching missing `package@version` message. +5. The lifecycle manager stops the failed child process. +6. The same `agentctl` instance resolves its npm cache and removes one execution tree. +7. Kandev changes only `--prefer-offline` to `--prefer-online`. +8. Kandev starts the replacement child and initializes the original ACP session. + +The startup generation rejects delayed events from the first child. The +existing cancellation and shutdown gates remain authoritative during repair. + +## Failure behavior + +If cache discovery or repair fails, Kandev emits `agent_runtime`. If the +second npm attempt fails, Kandev emits `managed_runtime_npm_resolution`. + +Both errors contain bounded sanitized details. The UI keeps the existing +single **Retry runtime** action. Kandev does not change the active version. + +Unsupported runtime types do not call the repair endpoint. Native commands, +passthrough commands, unrelated npm errors, and repeated failures remain on the +normal terminal error path. + +## Observability + +Structured recovery logs include the execution ID, agent ID, runtime type, and +startup generation. They do not include cache paths, registry URLs, or raw +stderr. + +Existing failure metadata stores the stable failure code and sanitized details. +No database migration is necessary. + +## Related decisions + +- [Validate and persist managed runtime version selection](../../../decisions/2026-08-12-validated-managed-runtime-version-selection.md) +- [Run cache repair where npm runs](../../../decisions/2026-08-24-agentctl-local-managed-runtime-cache-repair.md) diff --git a/docs/specs/agents/system-design/runtime-updates-01.md b/docs/specs/agents/system-design/runtime-updates-01.md index 3b0f524c23..29c571cf92 100644 --- a/docs/specs/agents/system-design/runtime-updates-01.md +++ b/docs/specs/agents/system-design/runtime-updates-01.md @@ -4,7 +4,7 @@ system: agents requirements: - REQ-AGENTS-RUNTIME-UPDATES-001 created: 2026-07-26 -updated: 2026-08-22 +updated: 2026-08-24 owners: - Kandev --- @@ -35,11 +35,8 @@ they open its version dialog. Kandev must provide a stable reviewed default for unmodified installations without preventing an operator from selecting a newer validated version between Kandev releases. -Managed runtimes can also fail when npm has stale package metadata. In that -case, npm can report `ETARGET` and `No matching version found` for a dependency -that the configured registry now contains. Kandev currently reports the later -ACP disconnect instead of the npm cause. The user then has no useful recovery -action unless they inspect backend logs. +Managed npm runtime recovery now has an authoritative requirement and design: +[managed npm runtime recovery](requirements/managed-npm-runtime-recovery.md). ## What @@ -77,11 +74,8 @@ action unless they inspect backend logs. recipe. - Jobs for one agent are idempotent while queued or running. Installation and version management for the same agent cannot run concurrently. -- A host-local managed runtime launch detects a strict npm `ETARGET` version - resolution failure before ACP initialization. Kandev refreshes npm metadata - and retries the same trusted package and version once. -- If the retry also fails, Kandev reports an npm runtime preparation error and - offers one runtime retry action. The user does not need to understand ACP. +- Managed npm runtime recovery follows the authoritative + [recovery requirement](requirements/managed-npm-runtime-recovery.md). - When the Agents settings page loads, Kandev checks each available managed package against npm's stable `latest` version through one batch status request. A newer version marks the existing update control with a blue dot @@ -324,29 +318,9 @@ produces a repair job. ## Launch-time stale metadata recovery -- Normal managed runtime commands continue to use `--prefer-offline`. -- Kandev inspects bounded process stderr when ACP initialization ends before a - response. Automatic recovery requires both npm `ETARGET` and a matching - `No matching version found for package@version` message. -- Kandev classifies the npm error from stderr. It does not build a command from - package names, versions, paths, or registry values found in stderr. -- Recovery removes only the deterministic `_npx` execution tree for the - trusted built-in package specification. It never clears npm's full cache. -- Kandev starts the same managed runtime once with `--prefer-online`. The - command keeps the same trusted package, exact effective version, - ACP arguments, configured npm registry, command prefix, permissions, model, - and session identity. -- Recovery is limited to one retry for each launch attempt. A delayed event - from the first child process cannot fail or complete the replacement process. -- User cancellation and backend shutdown stop recovery. Kandev does not retry - a remote executor, a native runtime, a passthrough command, an unrelated npm - error, or a second failed online attempt. -- A successful retry continues the original session without a failure card. - Kandev records structured recovery telemetry without exposing host paths or - raw process logs. -- A failed retry emits a stable npm runtime failure code and bounded sanitized - details. This evidence is stored with the last agent error so the focused UI - survives a page reload. +The [managed npm runtime recovery requirement](requirements/managed-npm-runtime-recovery.md) +and its [system design](system-design/managed-npm-runtime-recovery.md) are +authoritative for launch-time stale metadata recovery. ## Failure and recovery behavior @@ -366,8 +340,9 @@ produces a repair job. - Browser disconnect does not cancel a running job. The jobs endpoint can recover process-local progress while the backend remains running. - Active sessions are never restarted, replaced, or hot-swapped. -- A launch-time stale metadata retry is not a version rollback. It prepares the - same package selection again and does not change the active version. +- Launch-time recovery does not change the active version. The authoritative + [recovery requirement](requirements/managed-npm-runtime-recovery.md) defines + its observable behavior. - Registry failure during the batch update-status check returns `unknown` for only the affected package. It does not disable the update control, show an error badge, or claim that the package is up to date. @@ -397,9 +372,8 @@ produces a repair job. version again; it must not advance to another version. - Dialog selection, output, and result remain page-local after a browser page restart. -- A terminal launch-time npm resolution error stores its stable failure code - and bounded sanitized details in `last_agent_error`. No database migration is - required because the record is JSON metadata with optional fields. +- Terminal launch-time npm resolution metadata follows the authoritative + [recovery design](system-design/managed-npm-runtime-recovery.md). ## Desktop and mobile behavior @@ -436,3 +410,84 @@ produces a repair job. does not present session history loss as a fix for an npm problem. - Phone actions stack when needed, remain at least 44 px high, and do not add a second scroll container. +- Launch-time recovery presentation follows the authoritative + [recovery requirement](requirements/managed-npm-runtime-recovery.md). + +## Scenarios + +- **GIVEN** OpenCode latest is partly published and its ACP probe fails, + **WHEN** an operator selects an older published stable version and approves + **Roll back runtime**, **THEN** Kandev prepares and probes that exact version, + persists it only after success, and restores its model list without restart. +- **GIVEN** a healthy exact active version, **WHEN** Kandev restarts, **THEN** + boot probes and later managed commands use the same exact version. +- **GIVEN** an agent has no operator selection, **WHEN** Kandev builds any of + its managed npm ACP commands, **THEN** the command uses the exact reviewed + Kandev default and never an unversioned package spec. +- **GIVEN** an agent has a validated operator selection, **WHEN** Kandev builds + a local, container, or SSH managed npm ACP command, **THEN** the command uses + that exact selection instead of the Kandev default. +- **GIVEN** an operator selection exists, **WHEN** the operator chooses **Use + Kandev default**, **THEN** Kandev validates the exact default, deletes the + selection only after success, and future commands follow shipped defaults. +- **GIVEN** a candidate fails ACP initialization, **WHEN** the job ends, + **THEN** the previous active version and capabilities remain authoritative. +- **GIVEN** the current version is unknown and the effective version is known, + **WHEN** the operator selects a published target, **THEN** the UI offers + **Repair runtime** and validation establishes the selected exact version. +- **GIVEN** effective, current, and target versions match, **WHEN** the dialog + opens, **THEN** it shows **Up to date** and starts no job. +- **GIVEN** a different target is submitted while a job is active for the same + agent, **WHEN** the backend receives it, **THEN** it returns the existing job + and does not run a second candidate concurrently. +- **GIVEN** an agent whose interactive passthrough CLI is separate from its + managed ACP adapter, **WHEN** the effective ACP version changes, **THEN** + later passthrough sessions still launch the declared interactive CLI and do + not launch the ACP package under a PTY. +- **GIVEN** a phone viewport and a long version catalogue or process log, + **WHEN** the operator selects and activates a version, **THEN** the drawer + remains contained and the primary action remains touch-reachable. +- **GIVEN** npm reports a newer stable version than the effective version, + **WHEN** the Agents settings page loads, **THEN** the existing update control + shows a blue dot and exposes both versions in accessible update information. +- **GIVEN** the update-status lookup fails for one managed package, **WHEN** the + Agents settings page loads, **THEN** that package shows no blue dot, its + update control remains usable, and other package statuses still render. +- **GIVEN** npm returns valid stable package metadata in either its object form + or a supported one-element collection form, **WHEN** the operator opens a + managed runtime update dialog, **THEN** the dialog lists the stable versions, + selects npm's stable latest version, and does not show a resolution error. +- **GIVEN** a managed runtime has a long stable version catalogue, **WHEN** the + operator opens its update dialog, **THEN** the dialog shows the status summary + and quick choices without rendering the full version history. +- **GIVEN** the operator opens the full version browser and enters a version + fragment, **WHEN** matching versions exist, **THEN** only matching stable + versions remain selectable and selecting one previews that exact target. +- **GIVEN** the operator opens the full version browser on a phone, **WHEN** the + operator searches or selects a version, **THEN** the existing update drawer + keeps one contained scroll owner, exposes 44px touch rows, and preserves the + same target selection behavior as desktop. +- **GIVEN** the operator opens a long version catalogue, **WHEN** the operator + opens the version selector, **THEN** the catalogue appears in an anchored + popover without increasing the dialog or drawer height, and selecting a + version closes the popover. +- **GIVEN** a dotted update control on a phone, **WHEN** the operator taps it, + **THEN** the existing update drawer opens and shows a live authoritative + preview without requiring hover. +- Managed npm runtime recovery scenarios follow the authoritative + [recovery requirement](requirements/managed-npm-runtime-recovery.md). + +## Out of scope + +- Automatic runtime installation, automatic operator-selection changes, and + automatic rollback after launch failure. +- Global npm cache cleanup, registry replacement, dependency substitution, or + automatic selection of another package version. +- Prerelease, tag, arbitrary package-spec, registry, or shell-command input. +- Kandev-owned npm artifact retention or a package lockfile. +- Removing npm or network access from the launch path, or locking transitive + dependency ranges inside upstream packages. +- Restarting or hot-swapping active sessions. +- Native-only update channels and separately distributed passthrough or + authentication packages. +- Persisting job output or reopening the dialog after a browser restart.