diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d10fc9ae0..05c2b1343 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -74,6 +74,9 @@ jobs: with: go-version-file: go.mod cache: false + - uses: actions/setup-python@v6 + with: + python-version: '3.12' - uses: actions/cache@v6 with: path: | @@ -94,6 +97,9 @@ jobs: run: | sudo apt-get update sudo apt-get install -y build-essential pkg-config libssl-dev pigz + - uses: mlugg/setup-zig@d1434d08867e3ee9daa34448df10607b98908d29 + with: + version: 0.14.0 - name: Cache pinned release downloads uses: actions/cache@v6 with: @@ -123,9 +129,9 @@ jobs: CORE_DISTRIBUTION_OFFLINE: ${{ (github.event_name == 'push' || inputs.offline) && '1' || '0' }} run: | inputs="$HOME/.oac/build/release-inputs/inputs.json" - CODEX_CLI_DIR="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["codex"])' "$inputs")" + CODEX_HARNESS_BUILD_DIR="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["codex"])' "$inputs")" MCODE_HARNESS_BUILD_DIR="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["mcode"])' "$inputs")" - export CODEX_CLI_DIR MCODE_HARNESS_BUILD_DIR + export CODEX_HARNESS_BUILD_DIR MCODE_HARNESS_BUILD_DIR export CORE_DISTRIBUTION_RELEASE_BASE_URL="https://github.com/$RELEASE_REPOSITORY/releases/download/$RELEASE_TAG" bash scripts/build-core-distribution.sh mkdir -p "$HOME/.oac/build/release-upload" diff --git a/apps/daemon/internal/agent/claudesdk/executor.go b/apps/daemon/internal/agent/claudesdk/executor.go index ebbb1f6f2..4821869a8 100644 --- a/apps/daemon/internal/agent/claudesdk/executor.go +++ b/apps/daemon/internal/agent/claudesdk/executor.go @@ -23,11 +23,12 @@ type executor struct { done chan struct{} invalid bool nativeID string + stopMCP func(context.Context, []string) error } // startExecutor checks the installed bridge against probe, starts it through // run and waits until it is ready for Turns. -func startExecutor(ctx context.Context, checked *runtimeCheckCache, probe Config, start startRequest, run func() (*session, error)) (agent.Executor, error) { +func startExecutor(ctx context.Context, checked *runtimeCheckCache, probe Config, start startRequest, stopMCP func(context.Context, []string) error, run func() (*session, error)) (agent.Executor, error) { info, err := checked.check(ctx, probe) if err != nil { return nil, err @@ -39,7 +40,7 @@ func startExecutor(ctx context.Context, checked *runtimeCheckCache, probe Config if err != nil { return nil, err } - e := &executor{base: base, start: start, ready: make(chan error, 1), done: make(chan struct{}), nativeID: start.Resume} + e := &executor{base: base, start: start, ready: make(chan error, 1), done: make(chan struct{}), nativeID: start.Resume, stopMCP: stopMCP} go e.read() if err = e.write(start); err == nil { select { @@ -60,7 +61,7 @@ func startExecutor(ctx context.Context, checked *runtimeCheckCache, probe Config } func validateExecutorFeatures(info RuntimeInfo, start startRequest) error { - if info.Protocol != 3 { + if info.Protocol != 4 { return errors.New("claudesdk: Executor bridge protocol is unavailable") } if start.Workspace != nil && !info.supportsWorkspacePreparation() { @@ -114,7 +115,7 @@ func (e *executor) read() { break } if !ready { - if event.Type != "executor_ready" || event.Protocol != 3 || event.TurnID != "" { + if event.Type != "executor_ready" || event.Protocol != 4 || event.TurnID != "" { if event.Type == "error" { readyFailure = bridgeFailure(event.Code) } diff --git a/apps/daemon/internal/agent/claudesdk/executor_fixture_test.go b/apps/daemon/internal/agent/claudesdk/executor_fixture_test.go index 3ee557c67..516ac9274 100644 --- a/apps/daemon/internal/agent/claudesdk/executor_fixture_test.go +++ b/apps/daemon/internal/agent/claudesdk/executor_fixture_test.go @@ -74,7 +74,7 @@ func startSingleTurn(ctx context.Context, config testBridge, req proto.PromptReq } func helperTurn(scanner *bufio.Scanner) (func(bridgeEvent), func()) { - _ = json.NewEncoder(os.Stdout).Encode(bridgeEvent{Type: "executor_ready", Protocol: 3}) + _ = json.NewEncoder(os.Stdout).Encode(bridgeEvent{Type: "executor_ready", Protocol: 4}) if !scanner.Scan() { os.Exit(4) } diff --git a/apps/daemon/internal/agent/claudesdk/executor_test.go b/apps/daemon/internal/agent/claudesdk/executor_test.go index 56bc2bcea..1341e7fec 100644 --- a/apps/daemon/internal/agent/claudesdk/executor_test.go +++ b/apps/daemon/internal/agent/claudesdk/executor_test.go @@ -38,7 +38,7 @@ func runPersistentExecutorHelper() { _ = file.Close() } printlnReport := json.NewEncoder(os.Stdout) - _ = printlnReport.Encode(RuntimeInfo{Type: "runtime_ready", Protocol: 3, Node: "fixture", SDK: "fixture", MCP: "fixture", Native: "fixture"}) + _ = printlnReport.Encode(RuntimeInfo{Type: "runtime_ready", Protocol: 4, Node: "fixture", SDK: "fixture", MCP: "fixture", Native: "fixture"}) return } scanner := bufio.NewScanner(os.Stdin) @@ -51,7 +51,7 @@ func runPersistentExecutorHelper() { } _ = os.WriteFile(filepath.Join(root, "prepared"), []byte(strconv.Itoa(os.Getpid())), 0600) encode := func(event bridgeEvent) { _ = json.NewEncoder(os.Stdout).Encode(event) } - encode(bridgeEvent{Type: "executor_ready", Protocol: 3}) + encode(bridgeEvent{Type: "executor_ready", Protocol: 4}) if os.Getenv("SDK_EXECUTOR_MODE") == "block" { time.Sleep(time.Hour) return @@ -117,8 +117,27 @@ func runPersistentExecutorHelper() { encode(bridgeEvent{Type: "delta", TurnID: active, ItemID: "message", Delta: "steer-written"}) case "turn_cancel": if active == command.TurnID { + if strings.HasPrefix(os.Getenv("SDK_EXECUTOR_MODE"), "mcp_") { + label := "target" + if os.Getenv("SDK_EXECUTOR_MODE") == "mcp_http" { + label = "http" + } + encode(bridgeEvent{Type: "mcp_stop", TurnID: active, Servers: []string{label}}) + continue + } settle(true) } + case "mcp_stopped": + var response struct { + Confirmed bool `json:"confirmed"` + } + if command.TurnID != active || json.Unmarshal(scanner.Bytes(), &response) != nil { + os.Exit(6) + } + if !response.Confirmed { + _ = os.Setenv("SDK_EXECUTOR_MODE", "unknown_cancel") + } + settle(true) } } } diff --git a/apps/daemon/internal/agent/claudesdk/executor_turn.go b/apps/daemon/internal/agent/claudesdk/executor_turn.go index 2e5e253f6..3ea17a38c 100644 --- a/apps/daemon/internal/agent/claudesdk/executor_turn.go +++ b/apps/daemon/internal/agent/claudesdk/executor_turn.go @@ -60,6 +60,7 @@ func (s *session) runTurn(start startRequest, out chan<- proto.Envelope) { cancelled := false reusable := false reason := "bridge_interrupted" + mcpStopRequested := false for raw := range s.frames { var event bridgeEvent if err := json.Unmarshal(raw, &event); err != nil || event.TurnID != s.runID { @@ -85,6 +86,34 @@ func (s *session) runTurn(start startRequest, out chan<- proto.Envelope) { break } switch event.Type { + case "mcp_stop": + if mcpStopRequested || s.owner.stopMCP == nil || !start.validStdioServers(event.Servers) { + failure = fmt.Errorf("claudesdk: invalid MCP stop request") + s.invalidate() + break + } + select { + case <-s.cancelOutput: + default: + failure = fmt.Errorf("claudesdk: unsolicited MCP stop request") + s.invalidate() + } + if failure != nil { + break + } + mcpStopRequested = true + stopErr := s.owner.stopMCP(s.process.Context(), event.Servers) + if stopErr != nil { + s.settlementErr = fmt.Errorf("claudesdk: MCP process-scope settlement is unconfirmed") + } + if err := s.owner.write(struct { + Type string `json:"type"` + TurnID string `json:"turn_id"` + Confirmed bool `json:"confirmed"` + }{"mcp_stopped", s.runID, stopErr == nil}); err != nil { + failure = fmt.Errorf("claudesdk: MCP stop receipt delivery failed") + s.invalidate() + } case "command_observation": if err := commands.receive(event, start, s.inputSessionID(), emit); err != nil { failure = err diff --git a/apps/daemon/internal/agent/claudesdk/mcp_cancellation_test.go b/apps/daemon/internal/agent/claudesdk/mcp_cancellation_test.go new file mode 100644 index 000000000..712d4b11c --- /dev/null +++ b/apps/daemon/internal/agent/claudesdk/mcp_cancellation_test.go @@ -0,0 +1,109 @@ +//go:build unix + +package claudesdk + +import ( + "context" + "errors" + "slices" + "sync/atomic" + "testing" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" +) + +func TestExecutorMCPStopWaitsForScopeAndRetainsTurnOwnership(t *testing.T) { + config, req := persistentConfig(t, "mcp_cancel") + resource, err := config.factory()(t.Context(), prepared(t, req)) + if err != nil { + t.Fatal(err) + } + defer resource.Close(context.Background()) + owner := resource.(*executor) + owner.start.Workspace = &workspaceProfile{MCP: []environmentMCPServer{ + {mcpHTTPServer: mcpHTTPServer{ServerLabel: "target"}, Command: agent.ViewAlias(0)}, + {mcpHTTPServer: mcpHTTPServer{ServerLabel: "healthy"}, Command: agent.ViewAlias(1)}, + }} + entered, release := make(chan []string, 1), make(chan struct{}) + var calls atomic.Int32 + owner.stopMCP = func(ctx context.Context, labels []string) error { + calls.Add(1) + entered <- slices.Clone(labels) + select { + case <-release: + return nil + case <-ctx.Done(): + return ctx.Err() + } + } + turn, out := consumeExecutorTurn(t, owner, "cancelled", "wait") + <-out + waitCtx, stopWait := context.WithCancel(t.Context()) + first := make(chan error, 1) + go func() { first <- turn.Cancel(waitCtx) }() + if labels := <-entered; !slices.Equal(labels, []string{"target"}) { + t.Fatal(labels) + } + stopWait() + if err := <-first; !errors.Is(err, context.Canceled) { + t.Fatalf("cancel waiter = %v", err) + } + if _, err := turn.AwaitSettlement(waitCtx); !errors.Is(err, context.Canceled) { + t.Fatalf("scope still open: %v", err) + } + joined := make(chan error, 1) + go func() { joined <- turn.Cancel(t.Context()) }() + close(release) + if err := <-joined; err != nil { + t.Fatal(err) + } + awaitExecutorTurn(t, turn, out, true) + next, nextOut := consumeExecutorTurn(t, owner, "successor", "hello") + if err := turn.Cancel(t.Context()); err != nil { + t.Fatal(err) + } + awaitExecutorTurn(t, next, nextOut, true) + if calls.Load() != 1 { + t.Fatalf("scope stops = %d", calls.Load()) + } +} + +func TestExecutorMCPStopFailureAndHTTPRemainUnconfirmed(t *testing.T) { + for _, mode := range []string{"mcp_cancel", "mcp_http"} { + t.Run(mode, func(t *testing.T) { + config, req := persistentConfig(t, mode) + resource, err := config.factory()(t.Context(), prepared(t, req)) + if err != nil { + t.Fatal(err) + } + defer resource.Close(context.Background()) + owner := resource.(*executor) + owner.start.Workspace = &workspaceProfile{MCP: []environmentMCPServer{ + {mcpHTTPServer: mcpHTTPServer{ServerLabel: "target"}, Command: agent.ViewAlias(0)}, + {mcpHTTPServer: mcpHTTPServer{ServerLabel: "http", ServerURL: "http://gateway/mcp/http"}}, + }} + var calls atomic.Int32 + owner.stopMCP = func(context.Context, []string) error { + calls.Add(1) + return errors.New("scope not closed") + } + turn, out := consumeExecutorTurn(t, owner, "cancelled", "wait") + <-out + if err := turn.Cancel(t.Context()); err == nil { + t.Fatal("unconfirmed scope cancellation succeeded") + } + for range out { + } + if settled, err := turn.AwaitSettlement(t.Context()); err == nil || settled.Reusable { + t.Fatalf("settlement = %+v, %v", settled, err) + } + want := int32(1) + if mode == "mcp_http" { + want = 0 + } + if calls.Load() != want { + t.Fatalf("scope stops = %d, want %d", calls.Load(), want) + } + }) + } +} diff --git a/apps/daemon/internal/agent/claudesdk/mcp_environment.go b/apps/daemon/internal/agent/claudesdk/mcp_environment.go index 74b4ec4d8..c4f9f316e 100644 --- a/apps/daemon/internal/agent/claudesdk/mcp_environment.go +++ b/apps/daemon/internal/agent/claudesdk/mcp_environment.go @@ -42,3 +42,25 @@ func (start startRequest) declaredMCP() []mcpHTTPServer { } return servers } + +func (start startRequest) validStdioServers(labels []string) bool { + if start.Workspace == nil || len(labels) == 0 { + return false + } + seen := make(map[string]bool, len(labels)) + for _, label := range labels { + if seen[label] { + return false + } + for _, server := range start.Workspace.MCP { + if server.ServerLabel == label && server.Command != "" { + seen[label] = true + break + } + } + if !seen[label] { + return false + } + } + return true +} diff --git a/apps/daemon/internal/agent/claudesdk/preparation_fixture_test.go b/apps/daemon/internal/agent/claudesdk/preparation_fixture_test.go index f120b9506..3dee43f9e 100644 --- a/apps/daemon/internal/agent/claudesdk/preparation_fixture_test.go +++ b/apps/daemon/internal/agent/claudesdk/preparation_fixture_test.go @@ -72,7 +72,7 @@ func runPreparationHelper() { features = append(features, "workspace_structured_output") } } - _ = json.NewEncoder(os.Stdout).Encode(RuntimeInfo{Type: "runtime_ready", Protocol: 3, Node: "fixture", SDK: "fixture", MCP: "fixture", Native: "fixture", Features: features}) + _ = json.NewEncoder(os.Stdout).Encode(RuntimeInfo{Type: "runtime_ready", Protocol: 4, Node: "fixture", SDK: "fixture", MCP: "fixture", Native: "fixture", Features: features}) return } state := os.Getenv("CLAUDE_CONFIG_DIR") @@ -106,7 +106,7 @@ func runPreparationHelper() { time.Sleep(time.Millisecond) } } - emit(bridgeEvent{Type: "executor_ready", Protocol: 3}) + emit(bridgeEvent{Type: "executor_ready", Protocol: 4}) if !scanner.Scan() { return } diff --git a/apps/daemon/internal/agent/claudesdk/readiness.go b/apps/daemon/internal/agent/claudesdk/readiness.go index 09b2959a3..024d2ce0b 100644 --- a/apps/daemon/internal/agent/claudesdk/readiness.go +++ b/apps/daemon/internal/agent/claudesdk/readiness.go @@ -132,7 +132,7 @@ func CheckRuntime(ctx context.Context, config Config) (RuntimeInfo, error) { return RuntimeInfo{}, fmt.Errorf("claudesdk: runtime check failed") } var info RuntimeInfo - if json.Unmarshal(raw, &info) != nil || info.Type != "runtime_ready" || info.Protocol != 3 || + if json.Unmarshal(raw, &info) != nil || info.Type != "runtime_ready" || info.Protocol != 4 || info.Node == "" || info.SDK == "" || info.MCP == "" || info.Native == "" { return RuntimeInfo{}, fmt.Errorf("claudesdk: invalid runtime readiness report") } diff --git a/apps/daemon/internal/agent/claudesdk/readiness_test.go b/apps/daemon/internal/agent/claudesdk/readiness_test.go index b1ebb0565..3eedb989a 100644 --- a/apps/daemon/internal/agent/claudesdk/readiness_test.go +++ b/apps/daemon/internal/agent/claudesdk/readiness_test.go @@ -15,7 +15,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" ) -const readyReport = `{"type":"runtime_ready","protocol":3,"node":"22.22.2","sdk":"0.3.269","mcp":"1.30.0","native":"2.1.269 (Claude Code)"}` +const readyReport = `{"type":"runtime_ready","protocol":4,"node":"22.22.2","sdk":"0.3.269","mcp":"1.30.0","native":"2.1.269 (Claude Code)"}` func TestRequiredMCPNeedsQualifiedRuntime(t *testing.T) { root := t.TempDir() @@ -107,11 +107,11 @@ func runReadinessHelper() { case "ready": _, _ = fmt.Fprintln(os.Stdout, readyReport) case "ready-http-mcp": - _, _ = fmt.Fprintln(os.Stdout, strings.Replace(readyReport, `"protocol":3`, `"protocol":3,"features":["mcp_http_tools"]`, 1)) + _, _ = fmt.Fprintln(os.Stdout, strings.Replace(readyReport, `"protocol":4`, `"protocol":4,"features":["mcp_http_tools"]`, 1)) case "malformed": _, _ = fmt.Fprintln(os.Stdout, "not-json") case "wrong-protocol": - _, _ = fmt.Fprintln(os.Stdout, strings.Replace(readyReport, `"protocol":3`, `"protocol":1`, 1)) + _, _ = fmt.Fprintln(os.Stdout, strings.Replace(readyReport, `"protocol":4`, `"protocol":3`, 1)) case "missing-version": _, _ = fmt.Fprintln(os.Stdout, strings.Replace(readyReport, `"mcp":"1.30.0"`, `"mcp":""`, 1)) case "multiple": diff --git a/apps/daemon/internal/agent/claudesdk/runtime_cache.go b/apps/daemon/internal/agent/claudesdk/runtime_cache.go index 82b07f95e..e87e9b200 100644 --- a/apps/daemon/internal/agent/claudesdk/runtime_cache.go +++ b/apps/daemon/internal/agent/claudesdk/runtime_cache.go @@ -41,7 +41,7 @@ func (c *runtimeCheckCache) check(ctx context.Context, config Config) (RuntimeIn } c.mu.Lock() defer c.mu.Unlock() - if c.stamp == stamp && c.info.Protocol == 3 { + if c.stamp == stamp && c.info.Protocol == 4 { return c.info, nil } info, err := CheckRuntime(ctx, config) diff --git a/apps/daemon/internal/agent/claudesdk/session.go b/apps/daemon/internal/agent/claudesdk/session.go index 59d02117c..ed4c83395 100644 --- a/apps/daemon/internal/agent/claudesdk/session.go +++ b/apps/daemon/internal/agent/claudesdk/session.go @@ -37,6 +37,7 @@ type bridgeEvent struct { Confirmed *bool `json:"confirmed"` Reason string `json:"reason"` Protocol int `json:"protocol"` + Servers []string `json:"servers"` Fact json.RawMessage `json:"fact"` InputID string `json:"input_id"` diff --git a/apps/daemon/internal/agent/claudesdk/view.go b/apps/daemon/internal/agent/claudesdk/view.go index 2510e61bf..db3d5d1f0 100644 --- a/apps/daemon/internal/agent/claudesdk/view.go +++ b/apps/daemon/internal/agent/claudesdk/view.go @@ -98,7 +98,7 @@ func newViewExecutorFactory(probe Config, layout viewLayout) agent.ViewExecutorF if err != nil { return nil, err } - return startExecutor(ctx, checked, probe, start, func() (*session, error) { + return startExecutor(ctx, checked, probe, start, view.StopMCP, func() (*session, error) { return startSession(view.Launch, clirunner.StartOptions{Parent: ctx, Binary: layout.node, Args: []string{layout.bridge}, Dir: start.Cwd, Env: env, NeedStdin: true}) }) } diff --git a/apps/daemon/internal/agent/claudesdk/view_test.go b/apps/daemon/internal/agent/claudesdk/view_test.go index 49a8b0ff0..50016fc26 100644 --- a/apps/daemon/internal/agent/claudesdk/view_test.go +++ b/apps/daemon/internal/agent/claudesdk/view_test.go @@ -113,6 +113,7 @@ func TestViewExecutorLaunchesAClosedGatewayEnvironment(t *testing.T) { docs := session.MCP session.MCP = []agent.MCPBinding{{ServerLabel: "local", Transport: "stdio", Stdio: &agent.EnvironmentMCP{ Server: agentplugin.MCPServer{Name: "local", Type: "stdio", Command: agent.ViewAlias(0)}}}} + session.StopMCP = func(context.Context, []string) error { return nil } installed := req installed.CapabilityRoot, installed.Skills = agentcapabilities.Directory, []agentcapabilities.InstalledSkill{{InstallationRoot: agentcapabilities.Directory, Metadata: agentskill.Metadata{Type: "inline", Name: "review", Description: "Review."}, RelativeRoot: "skills/review", PackageRoot: "skills/review"}} @@ -210,7 +211,7 @@ func startViewBridge(options clirunner.StartOptions, requests chan<- []byte) (*c go func() { line, _ := bufio.NewReader(stdinReader).ReadBytes('\n') requests <- line - _, _ = fmt.Fprintln(stdoutWriter, `{"type":"executor_ready","protocol":3}`) + _, _ = fmt.Fprintln(stdoutWriter, `{"type":"executor_ready","protocol":4}`) <-bridge.ended _ = stdoutWriter.Close() _ = stdinReader.Close() diff --git a/apps/daemon/internal/agent/claudesdk/workspace_test.go b/apps/daemon/internal/agent/claudesdk/workspace_test.go index c662d44b4..bddf11040 100644 --- a/apps/daemon/internal/agent/claudesdk/workspace_test.go +++ b/apps/daemon/internal/agent/claudesdk/workspace_test.go @@ -45,7 +45,7 @@ func TestPublicMCPUsesWorkspaceProjectionWithoutCredentialCopy(t *testing.T) { if strings.Contains(string(raw)+strings.Join(env, "\n"), token) { t.Fatal("bearer reached the bridge") } - info := RuntimeInfo{Protocol: 3, Features: []string{"workspace_tools", "workspace_prepare", "workspace_command_observations", "local_runtime_v2", "mcp_http_tools", "mcp_http_bearer_auth", "mcp_http_required"}} + info := RuntimeInfo{Protocol: 4, Features: []string{"workspace_tools", "workspace_prepare", "workspace_command_observations", "local_runtime_v2", "mcp_http_tools", "mcp_http_bearer_auth", "mcp_http_required"}} if validateExecutorFeatures(info, start) == nil { t.Fatal("unqualified workspace bridge admitted") } diff --git a/apps/daemon/internal/agent/codex/executor_turn.go b/apps/daemon/internal/agent/codex/executor_turn.go index c3cf658dc..0e35a22aa 100644 --- a/apps/daemon/internal/agent/codex/executor_turn.go +++ b/apps/daemon/internal/agent/codex/executor_turn.go @@ -75,6 +75,15 @@ func (s *Session) settleExecutorTurn(startErr error) { s.settlementErr = errors.Join(s.settlementErr, s.cancelErr, s.rpc.Close()) } } + if s.cancelled.Load() && s.nativeSettled.Load() && s.cancelErr == nil && s.rpc.Alive() { + cleanupCtx, stop := context.WithTimeout(context.Background(), 10*time.Second) + err := s.cleanupCancelledMCP(cleanupCtx) + stop() + if err != nil { + s.settlement = agent.TurnSettlement{Reason: "mcp_cleanup_unconfirmed"} + s.settlementErr = errors.Join(s.settlementErr, err) + } + } if s.cancelled.Load() && s.nativeSettled.Load() && s.cancelErr == nil && s.rpc.Alive() { cleanupCtx, stop := context.WithTimeout(context.Background(), 10*time.Second) err := s.cleanupNativeTerminals(cleanupCtx) @@ -106,6 +115,7 @@ func (s *Session) Cancel(ctx context.Context) error { return err } s.cancelOnce.Do(func() { + s.latchMCPCancellation() s.cancelled.Store(true) s.cancelReady = make(chan struct{}) go func() { diff --git a/apps/daemon/internal/agent/codex/mcp_cancellation.go b/apps/daemon/internal/agent/codex/mcp_cancellation.go new file mode 100644 index 000000000..88b0e1305 --- /dev/null +++ b/apps/daemon/internal/agent/codex/mcp_cancellation.go @@ -0,0 +1,157 @@ +package codex + +import ( + "context" + "encoding/json" + "errors" + "slices" + "sort" + "sync" +) + +type mcpCallIdentity struct{ thread, turn, call string } + +type mcpCancellation struct { + mu sync.Mutex + pending map[mcpCallIdentity]string + latched bool + captured map[string]bool +} + +func (s *Session) stdioMCP(label string) bool { + if s.cfg.view != nil { + for _, binding := range s.cfg.view.MCP { + if binding.ServerLabel == label && binding.Transport == "stdio" && binding.Stdio != nil { + return true + } + } + } + return false +} + +// Native error/end notifications describe the local await, not a remote reply. +// Once cancellation latches, even a later real result cannot release its scope. +func (s *Session) observeMCPCall(thread, turn string, item ThreadItem, started, replied bool) { + if thread == "" || turn == "" || item.ID == "" || item.Type != "mcpToolCall" || !s.stdioMCP(item.Server) { + return + } + m := &s.mcpCancellation + m.mu.Lock() + defer m.mu.Unlock() + key := mcpCallIdentity{thread, turn, item.ID} + if !m.latched && replied { + delete(m.pending, key) + return + } + if started { + if m.pending == nil { + m.pending = make(map[mcpCallIdentity]string) + } + m.pending[key] = item.Server + if m.latched { + m.captured[item.Server] = true + } + } +} + +func (s *Session) observeRootMCP(raw json.RawMessage, started bool) { + var p struct { + ThreadID string `json:"threadId"` + TurnID string `json:"turnId"` + Item struct { + ThreadItem + Result json.RawMessage `json:"result"` + } `json:"item"` + } + if json.Unmarshal(raw, &p) != nil || !s.isRootThread(p.ThreadID) { + return + } + s.steering.mu.Lock() + matches := p.TurnID != "" && p.TurnID == s.steering.id + s.steering.mu.Unlock() + if matches { + s.observeMCPCall(p.ThreadID, p.TurnID, p.Item.ThreadItem, started, mcpReplied(p.Item.Result)) + } +} + +func mcpReplied(result json.RawMessage) bool { + return len(result) > 0 && string(result) != "null" +} + +// Called only for a verified child and a current or interrupted native Turn. +func (s *Session) observeChildMCP(thread, turn string, items []json.RawMessage) { + for _, raw := range items { + var item struct { + ThreadItem + Result json.RawMessage `json:"result"` + } + if json.Unmarshal(raw, &item) == nil { + replied := mcpReplied(item.Result) + s.observeMCPCall(thread, turn, item.ThreadItem, !replied, replied) + } + } +} + +func (s *Session) hasPendingMCP(thread, turn string) bool { + m := &s.mcpCancellation + m.mu.Lock() + defer m.mu.Unlock() + for key := range m.pending { + if key.thread == thread && key.turn == turn { + return true + } + } + return false +} + +func (s *Session) latchMCPCancellation() { + m := &s.mcpCancellation + m.mu.Lock() + defer m.mu.Unlock() + m.latched = true + m.captured = make(map[string]bool) + for _, label := range m.pending { + m.captured[label] = true + } +} + +func (s *Session) cancelledMCPLabels() []string { + m := &s.mcpCancellation + m.mu.Lock() + defer m.mu.Unlock() + labels := make([]string, 0, len(m.captured)) + for label := range m.captured { + labels = append(labels, label) + } + sort.Strings(labels) + return labels +} + +// Native callbacks and verified child observations have drained before this call. +// The host receipt closes all old alias scopes; the native receipt invalidates +// selected clients throughout the owned thread family without eager discovery. +func (s *Session) cleanupCancelledMCP(ctx context.Context) error { + labels := s.cancelledMCPLabels() + if len(labels) == 0 { + return nil + } + if s.cfg.view == nil || s.cfg.view.StopMCP == nil { + return errors.New("codex: stdio MCP cleanup owner unavailable") + } + if err := s.cfg.view.StopMCP(ctx, labels); err != nil { + return err + } + request := func(method string, params any) (json.RawMessage, error) { + return s.rpc.request(ctx, method, params, func(frame any) error { return s.rpc.writeFrameContext(ctx, frame) }) + } + raw, err := request("mcpServer/invalidate", McpServerInvalidateParams{ThreadID: s.currentThreadID(), ServerNames: labels}) + if err != nil { + return err + } + var receipt McpServerInvalidateResponse + if json.Unmarshal(raw, &receipt) != nil || !slices.Equal(receipt.ServerNames, labels) { + return errors.New("codex: native MCP invalidation unconfirmed") + } + _, err = request("config/mcpServer/reload", nil) + return err +} diff --git a/apps/daemon/internal/agent/codex/mcp_cancellation_test.go b/apps/daemon/internal/agent/codex/mcp_cancellation_test.go new file mode 100644 index 000000000..307df25b8 --- /dev/null +++ b/apps/daemon/internal/agent/codex/mcp_cancellation_test.go @@ -0,0 +1,251 @@ +package codex + +import ( + "context" + "encoding/json" + "errors" + "slices" + "testing" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/internal/agentplugin" +) + +func cancellationMCPBindings(labels ...string) []agent.MCPBinding { + var bindings []agent.MCPBinding + for i, label := range labels { + bindings = append(bindings, agent.MCPBinding{ServerLabel: label, ConnectionOrigin: "environment", CredentialAuthority: "none", Transport: "stdio", Stdio: &agent.EnvironmentMCP{Server: agentplugin.MCPServer{Name: label, Type: "stdio", Command: agent.ViewAlias(i)}}}) + } + return bindings +} + +func TestMCPCancellationCapturesUnconfirmedNativeCalls(t *testing.T) { + s := &Session{threadID: "root", cfg: sessionConfig{view: &viewLaunch{ViewSession: agent.ViewSession{MCP: cancellationMCPBindings("active", "failed", "late", "done", "child")}}}} + s.steering.id = "turn" + observe := func(thread, turn, id, label string, started bool, result any) { + t.Helper() + raw, _ := json.Marshal(map[string]any{"threadId": thread, "turnId": turn, "item": map[string]any{"type": "mcpToolCall", "id": id, "server": label, "status": "failed", "result": result, "error": map[string]string{"message": "local failure"}}}) + s.observeRootMCP(raw, started) + } + observe("root", "turn", "1", "active", true, nil) + observe("root", "turn", "2", "failed", true, nil) + observe("root", "turn", "2", "failed", false, nil) + observe("root", "turn", "3", "done", true, nil) + observe("root", "turn", "3", "done", false, map[string]any{"content": []any{}}) + observe("foreign", "turn", "4", "done", true, nil) + observe("root", "old", "4", "done", true, nil) + observe("root", "turn", "4", "http-or-unknown", true, nil) + s.observeChildMCP("child-thread", "child-turn", []json.RawMessage{json.RawMessage(`{"type":"mcpToolCall","id":"1","server":"child","status":"inProgress"}`)}) + s.latchMCPCancellation() + s.terminal.Store(true) + observe("root", "turn", "1", "active", false, map[string]any{"content": []any{}}) + observe("root", "turn", "5", "late", true, nil) + s.observeChildMCP("child-thread", "child-turn", []json.RawMessage{json.RawMessage(`{"type":"mcpToolCall","id":"1","server":"child","result":{"content":[]}}`)}) + if got := s.cancelledMCPLabels(); !slices.Equal(got, []string{"active", "child", "failed", "late"}) { + t.Fatal(got) + } +} + +func TestMCPChildRealReplyBeforeCancellationReleasesOnlyItsCall(t *testing.T) { + s := &Session{cfg: sessionConfig{view: &viewLaunch{ViewSession: agent.ViewSession{MCP: cancellationMCPBindings("child")}}}} + s.observeChildMCP("child", "turn", []json.RawMessage{json.RawMessage(`{"type":"mcpToolCall","id":"call","server":"child"}`)}) + s.observeChildMCP("child", "turn", []json.RawMessage{json.RawMessage(`{"type":"mcpToolCall","id":"call","server":"child","result":{"content":[]}}`)}) + s.latchMCPCancellation() + if s.hasPendingMCP("child", "turn") || len(s.cancelledMCPLabels()) != 0 { + t.Fatal("real child reply was retained as unfinished work") + } +} + +func TestMCPChildFirstObservedTerminalUsesNativeRootAttribution(t *testing.T) { + for _, test := range []struct { + name, status, spawnTurn, rootTurn, result string + captured bool + }{ + {"new-child-local-failure", "failed", "root-turn", "root-turn", `null`, true}, + {"reused-child-send-input-local-failure", "completed", "older-root-turn", "root-turn", `null`, true}, + {"current-child-interrupted", "interrupted", "root-turn", "root-turn", `null`, true}, + {"older-root-terminal", "failed", "older-root-turn", "older-root-turn", `null`, false}, + {"unknown-root-terminal", "failed", "older-root-turn", "", `null`, false}, + {"current-child-remote-reply", "completed", "root-turn", "root-turn", `{"content":[]}`, false}, + } { + t.Run(test.name, func(t *testing.T) { + s, fixture, out := observationSession(t, test.status) + s.cfg.view = &viewLaunch{ViewSession: agent.ViewSession{MCP: cancellationMCPBindings("child")}} + fixture.mu.Lock() + fixture.rootTurnID = test.spawnTurn + fixture.childRootTurnID = test.rootTurn + fixture.rootSendInput = test.spawnTurn != "root-turn" && test.rootTurn == "root-turn" + fixture.childItems = []json.RawMessage{json.RawMessage(`{"type":"mcpToolCall","id":"child-call","server":"child","tool":"wait","arguments":{},"status":"failed","error":{"message":"local wait failed"},"result":` + test.result + `}`)} + fixture.persist(t) + fixture.mu.Unlock() + // No prior history sample has observed the child's running Turn. The + // ordinary terminal drain reads paginated native history and rollout. + s.latchMCPCancellation() + s.onTurnCompleted(rootCompleted) + collectObserved(t, out) + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + if _, err := s.AwaitSettlement(ctx); err != nil { + t.Fatal(err) + } + var want []string + if test.captured { + want = []string{"child"} + } + if got := s.cancelledMCPLabels(); !slices.Equal(got, want) { + t.Fatalf("captured %v, want %v", got, want) + } + fixture.mu.Lock() + interrupts := fixture.interrupted + fixture.mu.Unlock() + if interrupts != 0 { + t.Fatal("terminal child incorrectly interrupted", interrupts) + } + }) + } +} + +func TestExecutorCancelledMCPRequiresScopeCloseThenInvalidationAndReload(t *testing.T) { + for _, mode := range []string{"mcp-confirmed", "mcp-caller-cancelled", "mcp-stop-failed", "mcp-invalidate-unconfirmed", "mcp-reload-failed"} { + t.Run(mode, func(t *testing.T) { + e, root := executorFixture(t, mode) + e.base.cfg.view.MCP = cancellationMCPBindings("active", "late", "healthy") + stopping := make(chan []string, 1) + release := make(chan struct{}) + e.base.cfg.view.StopMCP = func(ctx context.Context, labels []string) error { + stopping <- append([]string(nil), labels...) + select { + case <-release: + case <-ctx.Done(): + return ctx.Err() + } + if mode == "mcp-stop-failed" { + return errors.New("scope closure unconfirmed") + } + return nil + } + out := make(chan proto.Envelope, 30) + turn, err := e.StartTurn(t.Context(), "cancel", proto.TextInput("hold"), out) + if err != nil { + t.Fatal(err) + } + ctx, stop := context.WithTimeout(t.Context(), 5*time.Second) + defer stop() + for { + select { + case frame := <-out: + if frame.Type == proto.TypeDelta { + goto started + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } + started: + cancelled := make(chan error, 1) + callerCtx, cancelCaller := context.WithCancel(ctx) + defer cancelCaller() + go func() { cancelled <- turn.Cancel(callerCtx) }() + select { + case labels := <-stopping: + if !slices.Equal(labels, []string{"active", "late"}) { + t.Fatal(labels) + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + select { + case err := <-cancelled: + t.Fatal("cancel returned before ScopeClosed", err) + default: + } + for _, frame := range preparationFrames(t, root) { + if frame.Method == "mcpServer/invalidate" || frame.Method == "config/mcpServer/reload" { + t.Fatal("native invalidation preceded ScopeClosed") + } + } + callerCancelled := mode == "mcp-caller-cancelled" + if callerCancelled { + cancelCaller() + if err := <-cancelled; !errors.Is(err, context.Canceled) { + t.Fatal("cancel caller could not stop waiting", err) + } + } + close(release) + if callerCancelled { + _, err = turn.AwaitSettlement(ctx) + } else { + err = <-cancelled + } + confirmed := mode == "mcp-confirmed" || callerCancelled + if (err == nil) != confirmed { + t.Fatal(mode, err) + } + settlement, settledErr := turn.AwaitSettlement(ctx) + if (settledErr == nil) != confirmed || settlement.Reusable != confirmed { + t.Fatal(settlement, settledErr) + } + if !e.base.rpc.Alive() { + t.Fatal("cancel released the native owner") + } + done := 0 + for frame := range out { + if frame.Type == proto.TypeDone { + done++ + } + } + if done != 1 { + t.Fatal("terminal count", done) + } + if confirmed { + nextOut := make(chan proto.Envelope, 20) + next, err := e.StartTurn(ctx, "next", proto.TextInput("answer"), nextOut) + if err != nil || !awaitExecutorTurn(t, next, nextOut).Reusable { + t.Fatal(err) + } + } else if _, err := e.StartTurn(ctx, "forbidden", proto.TextInput("answer"), make(chan proto.Envelope, 20)); err == nil { + t.Fatal("unconfirmed MCP cleanup permitted reuse") + } + if err := turn.Cancel(ctx); (err == nil) != confirmed { + t.Fatal("old cancellation changed its result", err) + } + counts := map[string]int{} + pids := map[int]bool{} + for _, frame := range preparationFrames(t, root) { + counts[frame.Method]++ + pids[frame.PID] = true + if frame.Method == "mcpServer/invalidate" { + var p McpServerInvalidateParams + if json.Unmarshal(frame.Params, &p) != nil || p.ThreadID != "fixture-native-thread" || !slices.Equal(p.ServerNames, []string{"active", "late"}) || counts["turn/interrupt"] != 1 { + t.Fatal("wrong native target", string(frame.Params)) + } + } + } + wantInvalidate := 1 + if mode == "mcp-stop-failed" { + wantInvalidate = 0 + } + wantReload := 0 + if confirmed || mode == "mcp-reload-failed" { + wantReload = 1 + } + if counts["mcpServer/invalidate"] != wantInvalidate || counts["config/mcpServer/reload"] != wantReload || counts["turn/interrupt"] != 1 || counts["thread/start"] != 1 || counts["initialize"] != 1 || counts["mcpServerStatus/list"] != 0 || len(pids) != 1 { + t.Fatal(counts, pids) + } + }) + } +} + +func TestPreparationRequiresDeclaredMCPInvalidation(t *testing.T) { + req, cfg, root := preparationFixture(t) + cfg.view.MCP = cancellationMCPBindings("local") + cfg.view.StopMCP = func(context.Context, []string) error { return nil } + t.Setenv("OAC_TEST_MCP_UNSUPPORTED", "1") + _, err := testExecutor(t, "complete", req, cfg) + if !errors.Is(err, agent.ErrUnsupportedOperation) { + t.Fatal(err) + } + assertPreparationOnly(t, root) +} diff --git a/apps/daemon/internal/agent/codex/preparation.go b/apps/daemon/internal/agent/codex/preparation.go index 379be82ee..b36a14154 100644 --- a/apps/daemon/internal/agent/codex/preparation.go +++ b/apps/daemon/internal/agent/codex/preparation.go @@ -86,9 +86,15 @@ func newExecutor(parent context.Context, req agent.PrepareRequest, cfg sessionCo Capabilities: &InitializeCapabilities{ExperimentalAPI: true}, } phase = "initialize" - if _, err := rpc.Start(cancelCtx, initParams); err != nil { + initialized, err := rpc.Start(cancelCtx, initParams) + if err != nil { return e.preparationFailed(fmt.Errorf("codex: rpc start: %w", err)) } + for _, binding := range cfg.view.MCP { + if binding.Transport == "stdio" && !initialized.MCPServerInvalidation { + return e.preparationFailed(fmt.Errorf("%w: codex native runtime lacks MCP client invalidation", agent.ErrUnsupportedOperation)) + } + } if req.ExecutionControls != nil && req.ExecutionControls.DisableProgrammaticToolCalling { phase = "programmatic_tools_configuration" if err := verifyProgrammaticToolsDisabled(cancelCtx, rpc); err != nil { diff --git a/apps/daemon/internal/agent/codex/preparation_helpers_test.go b/apps/daemon/internal/agent/codex/preparation_helpers_test.go index 7aa89e2c5..9af0e33e1 100644 --- a/apps/daemon/internal/agent/codex/preparation_helpers_test.go +++ b/apps/daemon/internal/agent/codex/preparation_helpers_test.go @@ -203,7 +203,21 @@ func TestPreparationFakeCodexProcess(t *testing.T) { var result any = map[string]any{} switch frame.Method { case "initialize": - result = map[string]string{"userAgent": "fixture-codex"} + result = map[string]any{"userAgent": "fixture-codex", "mcpServerInvalidation": os.Getenv("OAC_TEST_MCP_UNSUPPORTED") != "1"} + case "mcpServer/invalidate": + var params McpServerInvalidateParams + if json.Unmarshal(frame.Params, ¶ms) != nil { + os.Exit(8) + } + result = McpServerInvalidateResponse{ServerNames: params.ServerNames} + if executorMode == "mcp-invalidate-unconfirmed" { + result = McpServerInvalidateResponse{} + } + case "config/mcpServer/reload": + if executorMode == "mcp-reload-failed" { + _ = output.Encode(map[string]any{"id": frame.ID, "error": map[string]any{"code": -32603, "message": "reload rejected"}}) + continue + } case "environment/status": if os.Getenv("OAC_TEST_PREPARATION_BLOCK") == "1" { for { @@ -316,10 +330,21 @@ func TestPreparationFakeCodexProcess(t *testing.T) { if frame.Method == "turn/start" { _ = output.Encode(map[string]any{"method": "turn/started", "params": map[string]any{"threadId": "fixture-native-thread", "turn": map[string]string{"id": currentTurn}}}) if held { + if strings.HasPrefix(executorMode, "mcp-") { + _ = output.Encode(map[string]any{"method": "item/started", "params": map[string]any{"threadId": "fixture-native-thread", "turnId": currentTurn, "item": map[string]any{"id": "active", "type": "mcpToolCall", "server": "active", "tool": "wait", "status": "inProgress"}}}) + } _ = output.Encode(map[string]any{"method": "item/agentMessage/delta", "params": map[string]any{"threadId": "fixture-native-thread", "turnId": currentTurn, "itemId": "held-message", "delta": "holding"}}) } } if frame.Method == "turn/interrupt" || !held { + if frame.Method == "turn/interrupt" && strings.HasPrefix(executorMode, "mcp-") { + for _, label := range []string{"active", "late"} { + if label == "late" { + _ = output.Encode(map[string]any{"method": "item/started", "params": map[string]any{"threadId": "fixture-native-thread", "turnId": currentTurn, "item": map[string]any{"id": label, "type": "mcpToolCall", "server": label, "tool": "wait", "status": "inProgress"}}}) + } + _ = output.Encode(map[string]any{"method": "item/completed", "params": map[string]any{"threadId": "fixture-native-thread", "turnId": currentTurn, "item": map[string]any{"id": label, "type": "mcpToolCall", "server": label, "tool": "wait", "status": "failed", "error": map[string]any{"message": "cancelled"}}}}) + } + } status := "completed" if frame.Method == "turn/interrupt" { status = "interrupted" diff --git a/apps/daemon/internal/agent/codex/protocol.go b/apps/daemon/internal/agent/codex/protocol.go index 257ef9124..4bc70e18a 100644 --- a/apps/daemon/internal/agent/codex/protocol.go +++ b/apps/daemon/internal/agent/codex/protocol.go @@ -83,10 +83,22 @@ type InitializeParams struct { } type InitializeResult struct { - UserAgent string `json:"userAgent"` - CodexHome string `json:"codexHome"` - PlatformFamily string `json:"platformFamily,omitempty"` - PlatformOs string `json:"platformOs,omitempty"` + MCPServerInvalidation bool `json:"mcpServerInvalidation"` + UserAgent string `json:"userAgent"` + CodexHome string `json:"codexHome"` + PlatformFamily string `json:"platformFamily,omitempty"` + PlatformOs string `json:"platformOs,omitempty"` +} + +// The selected stdio clients are invalidated in the entire owned thread subtree. +// The response confirms local invalidation, not remote completion or readiness. +type McpServerInvalidateParams struct { + ThreadID string `json:"threadId"` + ServerNames []string `json:"serverNames"` +} + +type McpServerInvalidateResponse struct { + ServerNames []string `json:"serverNames"` } type SkillsExtraRootsSetParams struct { diff --git a/apps/daemon/internal/agent/codex/session.go b/apps/daemon/internal/agent/codex/session.go index d13e62cf8..486f316cf 100644 --- a/apps/daemon/internal/agent/codex/session.go +++ b/apps/daemon/internal/agent/codex/session.go @@ -49,6 +49,7 @@ func defaultSessionConfig() sessionConfig { // 5. turn/completed emits TypeDone and closes out. Cancel interrupts the // native turn; settlement decides whether the Executor stays reusable. type Session struct { + mcpCancellation mcpCancellation retiredTurns map[string]bool outputDone chan struct{} nativeSettled atomic.Bool diff --git a/apps/daemon/internal/agent/codex/session_messages.go b/apps/daemon/internal/agent/codex/session_messages.go index 192267114..6ac0e3620 100644 --- a/apps/daemon/internal/agent/codex/session_messages.go +++ b/apps/daemon/internal/agent/codex/session_messages.go @@ -24,6 +24,7 @@ func (s *Session) onAgentDelta(raw json.RawMessage) { } func (s *Session) onItemStarted(raw json.RawMessage) { + s.observeRootMCP(raw, true) var p ItemStartedNotification if err := json.Unmarshal(raw, &p); err != nil { return @@ -41,6 +42,7 @@ func (s *Session) onItemStarted(raw json.RawMessage) { } func (s *Session) onItemCompleted(raw json.RawMessage) { + s.observeRootMCP(raw, false) s.observeSubagentIdentity(raw) var p ItemCompletedNotification if err := json.Unmarshal(raw, &p); err != nil { diff --git a/apps/daemon/internal/agent/codex/subagent_history.go b/apps/daemon/internal/agent/codex/subagent_history.go index 2d017c951..266091d43 100644 --- a/apps/daemon/internal/agent/codex/subagent_history.go +++ b/apps/daemon/internal/agent/codex/subagent_history.go @@ -15,6 +15,7 @@ type subagentHistory struct { type subagentNativeTurn struct { CompletedItems map[string]bool `json:"-"` + RootTurnID string `json:"-"` ID string `json:"id"` Status string `json:"status"` StartedAt *int64 `json:"startedAt"` diff --git a/apps/daemon/internal/agent/codex/subagent_observations_test.go b/apps/daemon/internal/agent/codex/subagent_observations_test.go index edd3fbc4d..e708c7742 100644 --- a/apps/daemon/internal/agent/codex/subagent_observations_test.go +++ b/apps/daemon/internal/agent/codex/subagent_observations_test.go @@ -14,12 +14,15 @@ import ( ) type subagentFixture struct { - mu sync.Mutex - home string - childStatus string - interrupted int - interruptGate <-chan struct{} - childItems []json.RawMessage + mu sync.Mutex + home string + childStatus string + interrupted int + interruptGate <-chan struct{} + childItems []json.RawMessage + rootTurnID string + childRootTurnID string + rootSendInput bool } func (f *subagentFixture) history(id string) subagentHistory { @@ -27,6 +30,9 @@ func (f *subagentFixture) history(id string) subagentHistory { start, finish := int64(101), int64(103) turn := subagentNativeTurn{ID: id + "-turn", Status: "completed", StartedAt: &start, CompletedAt: &finish, ItemsView: "full"} if id == "root" { + if f.rootTurnID != "" { + turn.ID = f.rootTurnID + } turn.Items = []json.RawMessage{json.RawMessage(`{"type":"collabAgentToolCall","id":"spawn","tool":"spawnAgent","status":"completed","senderThreadId":"root","receiverThreadIds":["child"],"prompt":"real child prompt"}`)} } else { h.Parent = "root" @@ -38,6 +44,10 @@ func (f *subagentFixture) history(id string) subagentHistory { } } h.Turns = []subagentNativeTurn{turn} + if id == "root" && f.rootSendInput { + h.Turns = append(h.Turns, subagentNativeTurn{ID: "root-turn", Status: "completed", StartedAt: &start, CompletedAt: &finish, ItemsView: "full", + Items: []json.RawMessage{json.RawMessage(`{"type":"collabAgentToolCall","id":"send","tool":"sendInput","status":"completed","senderThreadId":"root","receiverThreadIds":["child"],"prompt":"next child input"}`)}}) + } return h } @@ -46,10 +56,15 @@ func (f *subagentFixture) persist(t *testing.T) { for _, id := range []string{"root", "child"} { h := f.history(id) rows := []any{map[string]any{"type": "session_meta", "payload": map[string]any{"id": id, "cwd": h.Cwd}}} - for _, item := range h.Turns[0].Items { - var v map[string]any - _ = json.Unmarshal(item, &v) - rows = append(rows, map[string]any{"type": "event_msg", "payload": map[string]any{"type": "item_completed", "thread_id": id, "turn_id": id + "-turn", "completed_at_ms": 102000, "item": map[string]any{"id": v["id"], "type": "AgentMessage"}}}) + if id == "child" && f.childRootTurnID != "" { + rows = append(rows, map[string]any{"type": "turn_context", "payload": map[string]any{"turn_id": h.Turns[0].ID, "root_turn_id": f.childRootTurnID}}) + } + for _, turn := range h.Turns { + for _, item := range turn.Items { + var v map[string]any + _ = json.Unmarshal(item, &v) + rows = append(rows, map[string]any{"type": "event_msg", "payload": map[string]any{"type": "item_completed", "thread_id": id, "turn_id": turn.ID, "completed_at_ms": 102000, "item": map[string]any{"id": v["id"], "type": "AgentMessage"}}}) + } } var body []byte for _, row := range rows { diff --git a/apps/daemon/internal/agent/codex/subagent_rollout.go b/apps/daemon/internal/agent/codex/subagent_rollout.go index a0fa02882..6c242dd53 100644 --- a/apps/daemon/internal/agent/codex/subagent_rollout.go +++ b/apps/daemon/internal/agent/codex/subagent_rollout.go @@ -71,6 +71,7 @@ func readSubagentEffects(home agent.ViewDir, h *subagentHistory) ([]subagentEffe done := map[string]rolloutCompletion{} verified := false owners := map[string]string{} + rootTurns := map[string]string{} ownedItems := map[string]map[string]bool{} for _, line := range bytes.Split(body, []byte{'\n'}) { if len(line) == 0 { @@ -90,6 +91,20 @@ func readSubagentEffects(home agent.ViewDir, h *subagentHistory) ([]subagentEffe return nil, errors.New("codex: invalid subagent rollout payload") } switch row.Type { + case "turn_context": + var attribution struct { + Turn string `json:"turn_id"` + Root string `json:"root_turn_id"` + } + if json.Unmarshal(row.Payload, &attribution) != nil { + return nil, errors.New("codex: invalid native Turn attribution") + } + if attribution.Turn != "" && attribution.Root != "" { + if previous := rootTurns[attribution.Turn]; previous != "" && previous != attribution.Root { + return nil, errors.New("codex: conflicting native Turn attribution") + } + rootTurns[attribution.Turn] = attribution.Root + } case "session_meta": if verified { continue @@ -173,6 +188,7 @@ func readSubagentEffects(home agent.ViewDir, h *subagentHistory) ([]subagentEffe } if owner == h.ID { turn.CompletedItems = ownedItems[turn.ID] + turn.RootTurnID = rootTurns[turn.ID] owned = append(owned, turn) } } diff --git a/apps/daemon/internal/agent/codex/subagent_snapshots.go b/apps/daemon/internal/agent/codex/subagent_snapshots.go index 9f3eda46c..1db8705f6 100644 --- a/apps/daemon/internal/agent/codex/subagent_snapshots.go +++ b/apps/daemon/internal/agent/codex/subagent_snapshots.go @@ -126,6 +126,9 @@ func (s *Session) snapshotSubagents(ctx context.Context) (bool, error) { func (s *Session) publishSubagentHistory(ctx context.Context, h subagentHistory, known map[string]bool) (bool, error) { busy := false + s.steering.mu.Lock() + rootTurn := s.steering.id + s.steering.mu.Unlock() var nativeStatus struct { Type string `json:"type"` } @@ -135,6 +138,12 @@ func (s *Session) publishSubagentHistory(ctx context.Context, h subagentHistory, } for _, turn := range h.Turns { status := turn.Status + // A child can start and fail locally between history samples. Its durable + // native root attribution admits that Turn without admitting older history. + currentRoot := rootTurn != "" && turn.RootTurnID == rootTurn + if currentRoot || status == "inProgress" || s.subagents.interrupted[h.ID+":"+turn.ID] || s.hasPendingMCP(h.ID, turn.ID) { + s.observeChildMCP(h.ID, turn.ID, turn.Items) + } switch status { case "inProgress": if nativeStatus.Type == "notLoaded" { diff --git a/apps/daemon/internal/agent/codex/view_test.go b/apps/daemon/internal/agent/codex/view_test.go index 23f412adf..cba41546c 100644 --- a/apps/daemon/internal/agent/codex/view_test.go +++ b/apps/daemon/internal/agent/codex/view_test.go @@ -125,6 +125,7 @@ func TestViewExecutorLaunchesInTheSessionView(t *testing.T) { session.MCP = []agent.MCPBinding{{ServerLabel: "local", ConnectionOrigin: "environment", CredentialAuthority: "none", Transport: "stdio", Stdio: &agent.EnvironmentMCP{ Server: agentplugin.MCPServer{Name: "local", Type: "stdio", Command: agent.ViewAlias(0)}}}} + session.StopMCP = func(context.Context, []string) error { return nil } req.LocalEnvironment, req.DisableExecutionEnvironment = nil, true if _, err := view.Executor(t.Context(), req, session); err == nil || len(launched) != 2 { t.Fatalf("environment none: launches %d, err %v", len(launched), err) @@ -149,6 +150,7 @@ func TestViewHandsCodexTheInstalledSkillAndMCP(t *testing.T) { Proxy: "http://127.0.0.1:17100", MCP: []agent.MCPBinding{{ServerLabel: "local", ConnectionOrigin: "environment", CredentialAuthority: "none", Transport: "stdio", Stdio: &agent.EnvironmentMCP{ Server: agentplugin.MCPServer{Name: "local", Type: "stdio", Command: agent.ViewAlias(0)}}}}, + StopMCP: func(context.Context, []string) error { return nil }, // The fake Codex runs on the host, outside the view. Launch: func(opts clirunner.StartOptions) (*clirunner.Process, error) { opts.Binary, opts.Dir, opts.Env = cfg.codexBinary, "", os.Environ() diff --git a/apps/daemon/internal/agent/harness.go b/apps/daemon/internal/agent/harness.go index 130d9d2f1..561c79521 100644 --- a/apps/daemon/internal/agent/harness.go +++ b/apps/daemon/internal/agent/harness.go @@ -300,6 +300,16 @@ type ViewSession struct { // runs a shim's process and with nothing from the Harness's argv, working // directory or environment. A view Executor takes MCP only from here. MCP []MCPBinding + // StopMCP stops the named stdio bindings and all their descendants, including + // earlier background work owned by those same services. The adapter captures + // the affected labels before cancelling native calls and keeps same-Turn + // starts admitted during cancellation. It calls this once after native drain, + // before reconnecting those services or reporting successful settlement. + // Success confirms ScopeClosed for their old sandbox operations; it does not + // confirm native cancellation or reconnect the native clients. Other bindings + // and the workspace remain. Unknown or non-stdio labels are unsupported. + // Failure keeps ownership and the selected aliases closed to new starts. + StopMCP func(context.Context, []string) error // Launch replaces clirunner.Start. Each call builds one view and runs // Binary, which must be a LocalExec path, in it. Dir is a world path, or // the work directory in an empty-root view, and Env is the complete @@ -340,6 +350,9 @@ func checkViewHandoff(req PrepareRequest, session ViewSession) error { return fmt.Errorf("%w: MCP outside ViewSession.MCP", ErrViewHandoff) } for i, binding := range session.MCP { + if binding.Transport == "stdio" && session.StopMCP == nil { + return fmt.Errorf("%w: stdio MCP requires confirmed scope termination", ErrViewHandoff) + } alias := EnvironmentMCP{Server: agentplugin.MCPServer{Name: binding.ServerLabel, Type: "stdio", Command: ViewAlias(i)}} if binding.BearerToken != nil || len(binding.HTTPHeaders) > 0 || (binding.Transport == "http" && !isGatewayURL(binding.ServerURL, true)) || (binding.Transport == "stdio" && (binding.Stdio == nil || !reflect.DeepEqual(*binding.Stdio, alias))) { diff --git a/apps/daemon/internal/agent/mcode/environment_mcp_test.go b/apps/daemon/internal/agent/mcode/environment_mcp_test.go index 045728234..a7b587e89 100644 --- a/apps/daemon/internal/agent/mcode/environment_mcp_test.go +++ b/apps/daemon/internal/agent/mcode/environment_mcp_test.go @@ -124,11 +124,11 @@ func TestEnvironmentMCPCancelSettlesPendingObservationBeforeDone(t *testing.T) { t.Fatal("MCP start was not observed") } if err := session.Cancel(ctx); err != nil { - t.Fatal("stopped nonreusable MCP owner reported cancellation failure", err) + t.Fatal("MCP cancellation failed", err) } settlement, err := session.(*Session).AwaitSettlement(ctx) - if err != nil || settlement.Reusable { - t.Fatal("pending MCP work retained a reusable owner", settlement, err) + if err != nil || !settlement.Reusable { + t.Fatal("settled MCP scopes did not retain the owner", settlement, err) } closed, done := false, false for event := range out { diff --git a/apps/daemon/internal/agent/mcode/executor_test.go b/apps/daemon/internal/agent/mcode/executor_test.go index a6ed1180c..5e43d124d 100644 --- a/apps/daemon/internal/agent/mcode/executor_test.go +++ b/apps/daemon/internal/agent/mcode/executor_test.go @@ -109,14 +109,14 @@ func TestExecutorReusesNativeOwnerWithFreshTurnsAndIdleDrain(t *testing.T) { } } -func TestExecutorCancellationRetiresOwnerAndLateCancelCannotRetarget(t *testing.T) { +func TestExecutorCancellationPreservesOwnerAndLateCancelCannotRetarget(t *testing.T) { for _, disabled := range []bool{true, false} { t.Run(map[bool]string{true: "single-agent", false: "subagents"}[disabled], func(t *testing.T) { e, record := executorFixture(t, "executor-cancel", true) first, _ := runExecutorFixtureTurn(t, e, "first", "one") e.req.DisableSubagents = disabled if !disabled { - // A terminal native history record still cannot prove Bash cleanup. + // This Turn contains no unconfirmed workspace or MCP calls. reader := filepath.Join(t.TempDir(), "reader") body := "#!/bin/sh\nprintf '%s\\n' '{\"version\":1,\"complete\":true,\"rootSessionId\":\"native-1\",\"sessions\":[{\"id\":\"native-1\",\"turns\":[{\"id\":\"root-turn\",\"status\":\"aborted\"}]}]}'\n" if err := os.WriteFile(reader, []byte(body), 0700); err != nil { @@ -150,28 +150,24 @@ func TestExecutorCancellationRetiresOwnerAndLateCancelCannotRetarget(t *testing. ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() if err = second.Cancel(ctx); err != nil { - t.Fatal("stopped nonreusable owner reported cancellation failure", err) + t.Fatal("cancellation failed", err) } settlement, err := second.AwaitSettlement(ctx) - if err != nil || settlement.Reusable { + if err != nil || !settlement.Reusable { t.Fatal(settlement, err) } select { case <-e.connection.exited: + t.Fatal("settled cancellation killed native owner") default: - t.Fatal("nonreusable settlement preceded native process exit") } if got := second.CancellationOutcome(); got.Metadata[proto.DoneMetaAgentSessionID] != "native-1" { t.Fatal("cancel lost settled outcome", got) } for range out { } - thirdOut := make(chan proto.Envelope, 1) - third, err := e.StartTurn(t.Context(), "third", proto.TextInput("three"), thirdOut) - if third != nil || err == nil { - t.Fatal("unproven native owner admitted a successor") - } - close(thirdOut) + e.req.DisableSubagents = true + runExecutorFixtureTurn(t, e, "third", "three") raw, _ = os.ReadFile(record) if strings.Count(string(raw), "session/cancel\n") != 1 || strings.Count(string(raw), "initialize\n") != 1 { t.Fatalf("cancellation replaced owner: %s", raw) diff --git a/apps/daemon/internal/agent/mcode/executor_turn.go b/apps/daemon/internal/agent/mcode/executor_turn.go index 0f6bcb4cc..a9584c5e2 100644 --- a/apps/daemon/internal/agent/mcode/executor_turn.go +++ b/apps/daemon/internal/agent/mcode/executor_turn.go @@ -30,15 +30,19 @@ func (s *Session) runExecutorTurn(prompt string) { err = childErr } } + stopped, stopErr := s.settleCancelledMCP() + if stopErr != nil { + err = stopErr + } reusable := err == nil && s.process.Context().Err() == nil - if len(s.tools) > 0 { - reusable = false + for _, call := range s.tools { + if call.mcp == nil || !stopped[call.mcp.server] { + reusable = false + } } s.finishEnvironmentMCP() s.mu.Lock() - // ACP and native history cancellation do not prove detached tool cleanup. - // Retire the owner and settle its workers before acknowledging cancellation. - if s.cancelled || s.inputUncertain { + if s.inputUncertain { reusable = false } if s.inputUncertain && err == nil { @@ -112,13 +116,14 @@ func (s *Session) Cancel(ctx context.Context) error { } s.mu.Lock() first := !s.cancelled && !s.closing - s.cancelled = true if first { + s.cancelled = true + s.captureMCPCancellation() s.operations.Add(1) } s.mu.Unlock() // Cancellation releases event backpressure but does not cancel native owner - // context. After native settlement, cancellation retires and drains the owner. + // context. Settlement closes only the captured MCP service owners. s.outputCancel() e.mu.Unlock() var err error diff --git a/apps/daemon/internal/agent/mcode/mcp_cancellation_test.go b/apps/daemon/internal/agent/mcode/mcp_cancellation_test.go new file mode 100644 index 000000000..c3879bbf9 --- /dev/null +++ b/apps/daemon/internal/agent/mcode/mcp_cancellation_test.go @@ -0,0 +1,218 @@ +package mcode + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" +) + +func TestEnvironmentMCPCancellationRequiresNativeLifecycle(t *testing.T) { + for _, scenario := range []string{"unpatched-mcp", "old-mcp-lifecycle"} { + _, err := hostExecutor(t, t.Context(), helperInstall(t, scenario, ""), workspaceRequest(t), hostSession(t, stdioBinding())) + if err == nil || !strings.Contains(err.Error(), "native MCP lifecycle is unavailable") { + t.Fatal("stdio MCP accepted a native owner without lifecycle support", scenario, err) + } + } +} + +func TestEnvironmentMCPCancellationRetainsLocalFailureBeforeCancel(t *testing.T) { + for _, outcome := range []string{"local-failure", "server-error"} { + t.Run(outcome, func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + untouched := stdioBinding() + untouched.ServerLabel = "untouched" + host := hostSession(t, stdioBinding(), untouched, httpBinding()) + var stopped []string + stops := 0 + host.StopMCP = func(_ context.Context, names []string) error { stops++; stopped = slices.Clone(names); return nil } + record := filepath.Join(t.TempDir(), "calls") + e, err := hostExecutor(t, ctx, helperInstall(t, "prepared-mcp-prior-"+outcome, record), workspaceRequest(t), host) + if err != nil { + t.Fatal(err) + } + out := make(chan proto.Envelope, 32) + turn, err := e.StartTurn(ctx, "cancelled", proto.TextInput("wait"), out) + if err != nil { + t.Fatal(err) + } + failed := false + for ready := false; !ready; { + select { + case event := <-out: + if event.Type == proto.TypeToolCall { + var call proto.ToolCallPayload + if json.Unmarshal(event.Payload, &call) != nil { + t.Fatal("invalid tool observation") + } + if call.ID == "native-call" && call.Stage == "after" { + failed = call.Observation != nil && call.Observation.Status == "failed" && strings.Contains(string(call.Observation.Output), "native result") + } + } + ready = event.Type == proto.TypeDelta + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } + if !failed { + t.Fatal("failure projection changed") + } + if err := turn.Cancel(ctx); err != nil { + t.Fatal(err) + } + settlement, err := turn.AwaitSettlement(ctx) + if err != nil || !settlement.Reusable { + t.Fatal("cancelled owner was not reusable", settlement, err) + } + want := []string(nil) + if outcome == "local-failure" { + want = []string{"proof.server"} + } + if !slices.Equal(stopped, want) { + t.Fatal("local failure and genuine MCP reply shared a settlement outcome", stopped, want) + } + raw, _ := os.ReadFile(record) + if strings.Contains(string(raw), "oac/session/mcp/disconnect\n") != (len(want) > 0) { + t.Fatalf("native disconnect did not follow the selected scope: %s", raw) + } + runExecutorFixtureTurn(t, e, "next", "next") + nextOut := make(chan proto.Envelope, 8) + next, err := e.StartTurn(ctx, "next-wait", proto.TextInput("next-wait"), nextOut) + if err != nil { + t.Fatal(err) + } + select { + case <-nextOut: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + previousStops := stops + if err := next.Cancel(ctx); err != nil || stops != previousStops { + t.Fatal("old unconfirmed call crossed the Turn boundary", stops, previousStops, err) + } + }) + } +} + +func TestEnvironmentMCPCancellationFencesScopesBeforeNativeDisconnect(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + late, untouched := stdioBinding(), stdioBinding() + late.ServerLabel, untouched.ServerLabel = "late.server", "untouched" + host := hostSession(t, stdioBinding(), late, untouched, httpBinding()) + started, release := make(chan []string, 1), make(chan struct{}) + host.StopMCP = func(ctx context.Context, names []string) error { + started <- slices.Clone(names) + select { + case <-release: + return nil + case <-ctx.Done(): + return ctx.Err() + } + } + record := filepath.Join(t.TempDir(), "calls") + e, err := hostExecutor(t, ctx, helperInstall(t, "prepared-mcp-lifecycle", record), workspaceRequest(t), host) + if err != nil { + t.Fatal(err) + } + out := make(chan proto.Envelope, 32) + turn, err := e.StartTurn(ctx, "cancelled", proto.TextInput("wait"), out) + if err != nil { + t.Fatal(err) + } + select { + case event := <-out: + if event.Type != proto.TypeToolCall { + t.Fatal("native start was not observed", event.Type) + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + done := make(chan error, 1) + go func() { done <- turn.Cancel(ctx) }() + select { + case names := <-started: + if !slices.Equal(names, []string{"late.server", "proof.server"}) { + t.Fatal("late callback, local failure, HTTP or untouched identity was mishandled", names) + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + select { + case err := <-done: + t.Fatal("cancellation completed before ScopeClosed", err) + default: + } + if _, err := os.Stat(record + ".disconnect"); !os.IsNotExist(err) { + t.Fatal("native disconnect preceded ScopeClosed", err) + } + close(release) + if err := <-done; err != nil { + t.Fatal(err) + } + settlement, err := turn.AwaitSettlement(ctx) + if err != nil || !settlement.Reusable { + t.Fatal("settled MCP cancellation discarded the native owner", settlement, err) + } + var request struct { + SessionID string `json:"sessionId"` + Servers []string `json:"servers"` + } + raw, err := os.ReadFile(record + ".disconnect") + if err != nil || json.Unmarshal(raw, &request) != nil || request.SessionID != "native-1" || !slices.Equal(request.Servers, []string{"late.server", "proof.server"}) { + t.Fatal("native disconnect changed the frozen target", string(raw), err) + } + for event := range out { + if event.Type == proto.TypeError { + t.Fatalf("cancellation emitted an error: %s", event.Payload) + } + } + runExecutorFixtureTurn(t, e, "next", "next") + if err := turn.Cancel(ctx); err != nil { + t.Fatal("repeated cancellation lost original settlement", err) + } + raw, _ = os.ReadFile(record) + if strings.Count(string(raw), "session/cancel\n") != 1 || strings.Count(string(raw), "oac/session/mcp/disconnect\n") != 1 || strings.Count(string(raw), "initialize\n") != 1 { + t.Fatalf("old cancellation retargeted a successor or recreated the owner: %s", raw) + } +} + +func TestEnvironmentMCPCancellationRejectsUnconfirmedScope(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + host := hostSession(t, stdioBinding()) + host.StopMCP = func(context.Context, []string) error { return errors.New("scope close not confirmed") } + record := filepath.Join(t.TempDir(), "calls") + e, err := hostExecutor(t, ctx, helperInstall(t, "prepared-mcp-cancel", record), workspaceRequest(t), host) + if err != nil { + t.Fatal(err) + } + out := make(chan proto.Envelope, 16) + turn, err := e.StartTurn(ctx, "cancelled", proto.TextInput("wait"), out) + if err != nil { + t.Fatal(err) + } + select { + case <-out: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + if err := turn.Cancel(ctx); err == nil { + t.Fatal("unconfirmed scope became successful cancellation") + } + settlement, err := turn.AwaitSettlement(ctx) + if err == nil || settlement.Reusable { + t.Fatal("unconfirmed scope admitted a successor", settlement, err) + } + if _, err := os.Stat(record + ".disconnect"); !os.IsNotExist(err) { + t.Fatal("unconfirmed scope reached native disconnect", err) + } +} diff --git a/apps/daemon/internal/agent/mcode/mcp_observations.go b/apps/daemon/internal/agent/mcode/mcp_observations.go index f4c8e01be..6aeb8a671 100644 --- a/apps/daemon/internal/agent/mcode/mcp_observations.go +++ b/apps/daemon/internal/agent/mcode/mcp_observations.go @@ -1,11 +1,14 @@ package mcode import ( + "context" "encoding/json" "fmt" "slices" "strings" + "time" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" ) @@ -58,56 +61,57 @@ func (s *Session) environmentMCPIdentity(name string) (*mcpToolIdentity, error) return found, nil } -func environmentMCPObservation(update toolUpdate, stage string) (*proto.ToolObservation, error) { +func environmentMCPObservation(update toolUpdate, stage string) (*proto.ToolObservation, bool, error) { if update.mcp == nil { - return nil, nil + return nil, false, nil } arguments, err := json.Marshal(update.RawInput) if err != nil { - return nil, fmt.Errorf("mcode: invalid native MCP arguments") + return nil, false, fmt.Errorf("mcode: invalid native MCP arguments") } n := &proto.ToolObservation{Kind: "mcp", Status: "in_progress", Server: update.mcp.server, Name: update.mcp.tool, Arguments: arguments, Output: json.RawMessage("null"), Error: json.RawMessage("null")} if stage == "before" { - return n, nil + return n, false, nil } n.Status = update.Status if update.Status == "incomplete" { - return n, nil + return n, false, nil } raw, err := json.Marshal(update.RawOutput) var output struct { Details *struct { - Server string `json:"server"` - Tool string `json:"tool"` - MCP json.RawMessage `json:"mcp"` - IsError bool `json:"is_error"` + Server string `json:"server"` + Tool string `json:"tool"` + MCP json.RawMessage `json:"mcp"` + IsError bool `json:"is_error"` + ResponseReceived bool `json:"oac_response_received"` } `json:"details"` } if err != nil || json.Unmarshal(raw, &output) != nil { - return nil, fmt.Errorf("mcode: invalid native MCP result") + return nil, false, fmt.Errorf("mcode: invalid native MCP result") } if output.Details == nil && update.Status == "failed" { // Transport failures may have no MCP response. The start registry still // identifies the real call, so retain the native failure without guessing. n.Error = raw - return n, nil + return n, false, nil } if output.Details == nil || output.Details.Server != n.Server || output.Details.Tool != n.Name || len(output.Details.MCP) == 0 || string(output.Details.MCP) == "null" { - return nil, fmt.Errorf("mcode: native MCP result identity does not match its call") + return nil, false, fmt.Errorf("mcode: native MCP result identity does not match its call") } n.Output = output.Details.MCP var result struct { IsError bool `json:"isError"` } if json.Unmarshal(n.Output, &result) != nil { - return nil, fmt.Errorf("mcode: invalid native MCP content") + return nil, false, fmt.Errorf("mcode: invalid native MCP content") } if result.IsError || output.Details.IsError { n.Status = "failed" } - return n, nil + return n, output.Details.ResponseReceived, nil } // The existing Session owner calls this after native settlement. No pending @@ -128,3 +132,76 @@ func (s *Session) finishEnvironmentMCP() { s.completedTools[id] = true } } + +// trackMCPCancellation runs before output delivery, which can block. Cancel +// and late native callbacks therefore agree on the same in-flight identities. +func (s *Session) trackMCPCancellation(call toolUpdate, responseReceived bool) { + if call.mcp == nil { + return + } + stdio := slices.ContainsFunc(s.opts.bindings, func(binding agent.MCPBinding) bool { + return binding.ServerLabel == call.mcp.server && binding.Transport == "stdio" + }) + if !stdio { + return + } + s.mu.Lock() + defer s.mu.Unlock() + if s.mcpCalls == nil { + s.mcpCalls = map[string]string{} + } + if s.cancelled { + if s.cancelledMCP == nil { + s.cancelledMCP = map[string]bool{} + } + s.cancelledMCP[call.mcp.server] = true + } + // A local timeout is also projected as a terminal tool error. Only the + // native wrapper's verified SDK reply releases this Turn's remote owner. + if responseReceived { + delete(s.mcpCalls, call.ID) + } else { + s.mcpCalls[call.ID] = call.mcp.server + } +} + +// captureMCPCancellation is called with mu held before session/cancel is sent. +func (s *Session) captureMCPCancellation() { + if s.cancelledMCP == nil { + s.cancelledMCP = map[string]bool{} + } + for _, server := range s.mcpCalls { + s.cancelledMCP[server] = true + } +} + +func (s *Session) settleCancelledMCP() (map[string]bool, error) { + s.mu.Lock() + servers := make([]string, 0, len(s.cancelledMCP)) + for server := range s.cancelledMCP { + servers = append(servers, server) + } + s.mu.Unlock() + if len(servers) == 0 { + return nil, nil + } + slices.Sort(servers) + if s.opts.stopMCP == nil { + return nil, fmt.Errorf("mcode: MCP scope settlement is unavailable") + } + ctx, cancel := context.WithTimeout(s.process.Context(), 60*time.Second) + defer cancel() + if err := s.opts.stopMCP(ctx, servers); err != nil { + return nil, fmt.Errorf("mcode: MCP scope settlement: %w", err) + } + // The sandbox scope closes first. Native then fences the old transport while + // retaining its configuration, so the next call can reconnect lazily. + if err := s.call("oac/session/mcp/disconnect", map[string]any{"sessionId": s.sessionID, "servers": servers}, nil, false); err != nil { + return nil, err + } + stopped := make(map[string]bool, len(servers)) + for _, server := range servers { + stopped[server] = true + } + return stopped, nil +} diff --git a/apps/daemon/internal/agent/mcode/mcp_observations_test.go b/apps/daemon/internal/agent/mcode/mcp_observations_test.go index 17da013ac..155c3d26c 100644 --- a/apps/daemon/internal/agent/mcode/mcp_observations_test.go +++ b/apps/daemon/internal/agent/mcode/mcp_observations_test.go @@ -43,7 +43,7 @@ func receiveMCPObservation(t *testing.T, out chan proto.Envelope, stage, status } func mcpNativeResult(server, tool string, isError bool) map[string]any { - return map[string]any{"details": map[string]any{"server": server, "tool": tool, "mcp": map[string]any{ + return map[string]any{"details": map[string]any{"server": server, "tool": tool, "oac_response_received": true, "mcp": map[string]any{ "content": []map[string]string{{"type": "text", "text": "native result"}}, "isError": isError, }}} } @@ -152,6 +152,9 @@ func TestEnvironmentMCPResultMustMatchStartAndUnsettledCallsCloseOnce(t *testing if err := s.emitTool(update); err == nil || len(out) != 0 { t.Fatal("inconsistent result was accepted") } + if s.mcpCalls["native-call"] != "proof.server" { + t.Fatal("unverified result cleared remote completion ownership") + } } s.finishEnvironmentMCP() n := receiveMCPObservation(t, out, "after", "incomplete") diff --git a/apps/daemon/internal/agent/mcode/options.go b/apps/daemon/internal/agent/mcode/options.go index 2bec53b28..9fb97bb29 100644 --- a/apps/daemon/internal/agent/mcode/options.go +++ b/apps/daemon/internal/agent/mcode/options.go @@ -1,6 +1,7 @@ package mcode import ( + "context" "encoding/json" "errors" "fmt" @@ -21,6 +22,7 @@ type launchOptions struct { // bindings are the effective MCP bindings rendered into MCP; native MCP // tool calls are observed against them. bindings []agent.MCPBinding + stopMCP func(context.Context, []string) error // start runs node with script, the CLI entry, as the native process. start func(clirunner.StartOptions) (*clirunner.Process, error) script string diff --git a/apps/daemon/internal/agent/mcode/session.go b/apps/daemon/internal/agent/mcode/session.go index 26d5bafb4..1e51d1a8c 100644 --- a/apps/daemon/internal/agent/mcode/session.go +++ b/apps/daemon/internal/agent/mcode/session.go @@ -51,6 +51,10 @@ type Session struct { previousNativeTurns map[string]bool rootCompletedAtMS *int64 subagentHistoryReady bool + // These cancellation identities are protected by mu; tool projection stays + // on the reader. A local failure does not remove a captured remote owner. + mcpCalls map[string]string + cancelledMCP map[string]bool } func launch(ctx context.Context, req proto.PromptRequestPayload, opts launchOptions, binary string) (*Session, error) { @@ -73,7 +77,8 @@ func (s *Session) prepareNative() error { var initialized struct { ProtocolVersion int `json:"protocolVersion"` Meta struct { - Subagents struct { + MCPLifecycle struct{ Version int } `json:"oac/mcp-lifecycle"` + Subagents struct { Version, MaxConcurrent int WorkspaceTools string } `json:"oac/subagents"` @@ -85,6 +90,11 @@ func (s *Session) prepareNative() error { if initialized.ProtocolVersion != 1 { return fmt.Errorf("mcode: unsupported ACP protocol version %d", initialized.ProtocolVersion) } + for _, binding := range s.opts.bindings { + if binding.Transport == "stdio" && initialized.Meta.MCPLifecycle.Version != 2 { + return fmt.Errorf("mcode: native MCP lifecycle is unavailable") + } + } if !s.req.DisableSubagents && (initialized.Meta.Subagents.Version != 1 || initialized.Meta.Subagents.WorkspaceTools != "protected-mcp-v1" || s.req.MaxConcurrentSubagents == nil || initialized.Meta.Subagents.MaxConcurrent != *s.req.MaxConcurrentSubagents) { return fmt.Errorf("mcode: native Subagent admission is unavailable") } diff --git a/apps/daemon/internal/agent/mcode/session_test.go b/apps/daemon/internal/agent/mcode/session_test.go index 37a3c5c21..44a153114 100644 --- a/apps/daemon/internal/agent/mcode/session_test.go +++ b/apps/daemon/internal/agent/mcode/session_test.go @@ -64,7 +64,8 @@ func hostSession(t *testing.T, mcp ...agent.MCPBinding) agent.ViewSession { if err := os.Mkdir(filepath.Join(home, agent.ViewWorkName), 0o700); err != nil { t.Fatal(err) } - return agent.ViewSession{Home: agent.ViewDir{Host: home, View: home}, MCP: mcp, Launch: clirunner.Start, Spawn: clirunner.Start} + return agent.ViewSession{Home: agent.ViewDir{Host: home, View: home}, MCP: mcp, Launch: clirunner.Start, Spawn: clirunner.Start, + StopMCP: func(context.Context, []string) error { return nil }} } // hostExecutor prepares req through install's view Executor in session; @@ -299,10 +300,16 @@ func TestMCodeProcess(t *testing.T) { if scenario == "unattended" && strings.Contains(string(frame.Params), "elicitation") { os.Exit(7) } - result = map[string]int{"protocolVersion": 1} + result = map[string]any{"protocolVersion": 1, "_meta": map[string]any{"oac/mcp-lifecycle": map[string]int{"version": 2}}} + if scenario == "unpatched-mcp" { + result = map[string]any{"protocolVersion": 1} + } + if scenario == "old-mcp-lifecycle" { + result = map[string]any{"protocolVersion": 1, "_meta": map[string]any{"oac/mcp-lifecycle": map[string]int{"version": 1}}} + } case "session/new", "session/load": - if scenario == "prepared-mcp-cancel" { - if writeMCPRegistry(os.Getenv("MINIMAX_DATA_DIR"), mcpRegistryEntry("proof.server", "proof_server", "read.status", "read_status")) != nil { + if strings.HasPrefix(scenario, "prepared-mcp-") { + if writeMCPRegistry(os.Getenv("MINIMAX_DATA_DIR"), mcpRegistryEntry("proof.server", "proof_server", "read.status", "read_status"), mcpRegistryEntry("late.server", "late_server", "read.status", "read_status"), mcpRegistryEntry("remote", "remote", "read.status", "read_status")) != nil { os.Exit(12) } } @@ -362,9 +369,23 @@ func TestMCodeProcess(t *testing.T) { } continue } - if scenario == "prepared-mcp-cancel" { + if strings.HasPrefix(scenario, "prepared-mcp-") && input.Prompt[0]["text"] != "next" { promptID = frame.ID + if input.Prompt[0]["text"] == "next-wait" { + update("agent_message_chunk", map[string]any{"messageId": "next-ready", "content": map[string]string{"type": "text", "text": "next turn ready"}}) + continue + } update("tool_call", map[string]any{"toolCallId": "native-call", "name": "mcp__proof_server__read_status", "status": "in_progress", "rawInput": map[string]any{}}) + if strings.HasPrefix(scenario, "prepared-mcp-prior-") { + output := mcpNativeResult("proof.server", "read.status", true) + output["details"].(map[string]any)["oac_response_received"] = scenario == "prepared-mcp-prior-server-error" + output["details"].(map[string]any)["mcp"].(map[string]any)["_meta"] = map[string]any{"oac_response_received": true} + update("tool_call_update", map[string]any{"toolCallId": "native-call", "status": "completed", "rawOutput": output}) + // Another call's genuine reply must not clear the first call's owner. + update("tool_call", map[string]any{"toolCallId": "other-call", "name": "mcp__proof_server__read_status", "status": "in_progress", "rawInput": map[string]any{}}) + update("tool_call_update", map[string]any{"toolCallId": "other-call", "status": "completed", "rawOutput": mcpNativeResult("proof.server", "read.status", false)}) + update("agent_message_chunk", map[string]any{"messageId": "ready", "content": map[string]string{"type": "text", "text": "failures projected"}}) + } continue } if scenario == "steering" || scenario == "steer-rejected" || scenario == "steer-lost" || scenario == "cancel-wait" { @@ -400,9 +421,21 @@ func TestMCodeProcess(t *testing.T) { case "mcode/session/delegation/stop": result = map[string]any{"receipt": map[string]any{"failedSessionIds": []string{}}} case "session/cancel": + if scenario == "prepared-mcp-lifecycle" { + update("tool_call_update", map[string]any{"toolCallId": "native-call", "status": "failed", "rawOutput": mcpNativeResult("proof.server", "read.status", true)}) + update("tool_call", map[string]any{"toolCallId": "late-call", "name": "mcp__late_server__read_status", "status": "in_progress", "rawInput": map[string]any{}}) + update("tool_call_update", map[string]any{"toolCallId": "late-call", "status": "failed", "rawOutput": mcpNativeResult("late.server", "read.status", true)}) + update("tool_call", map[string]any{"toolCallId": "http-call", "name": "mcp__remote__read_status", "status": "failed", "rawInput": map[string]any{}, "rawOutput": mcpNativeResult("remote", "read.status", true)}) + } raw, _ := json.Marshal(map[string]string{"stopReason": "cancelled"}) send(rpcFrame{JSONRPC: "2.0", ID: promptID, Result: raw}) continue + case "oac/session/mcp/disconnect": + if record := os.Getenv("OAC_TEST_MCODE_RECORD"); record != "" { + if os.WriteFile(record+".disconnect", frame.Params, 0600) != nil { + os.Exit(13) + } + } case "mcode/session/steer": if scenario == "executor-steer-unknown" { send(rpcFrame{JSONRPC: "2.0", ID: frame.ID, Error: &rpcError{Code: -32000, Message: "Unknown input outcome"}}) diff --git a/apps/daemon/internal/agent/mcode/steering_test.go b/apps/daemon/internal/agent/mcode/steering_test.go index 5ece6d280..1bcf80dd5 100644 --- a/apps/daemon/internal/agent/mcode/steering_test.go +++ b/apps/daemon/internal/agent/mcode/steering_test.go @@ -88,7 +88,7 @@ func TestTerminalFollowsAllNativeFrames(t *testing.T) { } } -func TestExecutionCancellationWaitsForOutputAndProcess(t *testing.T) { +func TestExecutionCancellationWaitsForOutputAndKeepsProcess(t *testing.T) { s, out := helperSession(t, "cancel-wait", false) ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() @@ -108,8 +108,8 @@ func TestExecutionCancellationWaitsForOutputAndProcess(t *testing.T) { } select { case <-s.exited: + t.Fatal("settled cancellation killed native owner") default: - t.Fatal("cancel returned before process exit") } select { case <-s.finished: diff --git a/apps/daemon/internal/agent/mcode/subagent_messages.go b/apps/daemon/internal/agent/mcode/subagent_messages.go index 00f20d105..01ea62690 100644 --- a/apps/daemon/internal/agent/mcode/subagent_messages.go +++ b/apps/daemon/internal/agent/mcode/subagent_messages.go @@ -79,7 +79,7 @@ func (s *Session) projectSubagentMessages(session nativeSubagentSession, turn na if err != nil { return err } - observation, err = environmentMCPObservation(update, "after") + observation, _, err = environmentMCPObservation(update, "after") if err != nil { return err } diff --git a/apps/daemon/internal/agent/mcode/tool_observations.go b/apps/daemon/internal/agent/mcode/tool_observations.go index 1dbd6d643..1c0c1ff81 100644 --- a/apps/daemon/internal/agent/mcode/tool_observations.go +++ b/apps/daemon/internal/agent/mcode/tool_observations.go @@ -14,10 +14,12 @@ func (s *Session) emitToolStage(update toolUpdate, stage string) error { payload.Observation = workspaceToolObservation(update, stage) if payload.Observation == nil { var err error - payload.Observation, err = environmentMCPObservation(update, stage) + var responseReceived bool + payload.Observation, responseReceived, err = environmentMCPObservation(update, stage) if err != nil { return err } + s.trackMCPCancellation(update, responseReceived) } // Native task/skill bookkeeping has no qualified public item mapping. if payload.Observation == nil { diff --git a/apps/daemon/internal/agent/mcode/view.go b/apps/daemon/internal/agent/mcode/view.go index 858bd9acb..364ce9f74 100644 --- a/apps/daemon/internal/agent/mcode/view.go +++ b/apps/daemon/internal/agent/mcode/view.go @@ -213,7 +213,7 @@ func (i viewInstall) prepare(req agent.PrepareRequest, session agent.ViewSession } defer data.Close() opts := launchOptions{Dir: dir, DataDir: filepath.Join(session.Home.Host, viewDataName), bindings: session.MCP, - start: session.Launch, script: i.cli, home: session.Home.Host} + start: session.Launch, script: i.cli, home: session.Home.Host, stopMCP: session.StopMCP} dataDir, tempDir := path.Join(session.Home.View, viewDataName), path.Join(session.Home.View, viewTempName) var tools *workspaceTools opts.MCP = []map[string]any{} diff --git a/apps/daemon/internal/agent/view_test.go b/apps/daemon/internal/agent/view_test.go index 692328cae..c31d8f08e 100644 --- a/apps/daemon/internal/agent/view_test.go +++ b/apps/daemon/internal/agent/view_test.go @@ -102,6 +102,7 @@ func TestViewExecutorReceivesOnlyGatewayConnections(t *testing.T) { mcp []agent.MCPBinding }{ "gateway": {req: gatewayRequest, mcp: []agent.MCPBinding{gateway, alias}}, + "missing scope stop": {req: gatewayRequest, mcp: []agent.MCPBinding{gateway, alias}}, "stdio command": {req: gatewayRequest, mcp: []agent.MCPBinding{gateway, command}}, "another alias": {req: gatewayRequest, mcp: []agent.MCPBinding{gateway, misplaced}}, "model key": {req: request("http://127.0.0.1:4101", token)}, @@ -116,7 +117,11 @@ func TestViewExecutorReceivesOnlyGatewayConnections(t *testing.T) { if name == "gateway" { want = errReached } - if _, err := view.Executor(context.Background(), c.req, agent.ViewSession{MCP: c.mcp}); !errors.Is(err, want) { + session := agent.ViewSession{MCP: c.mcp, StopMCP: func(context.Context, []string) error { return nil }} + if name == "missing scope stop" { + session.StopMCP = nil + } + if _, err := view.Executor(context.Background(), c.req, session); !errors.Is(err, want) { t.Errorf("%s: Executor = %v, want %v", name, err, want) } } diff --git a/apps/daemon/internal/agenthost/executor_linux.go b/apps/daemon/internal/agenthost/executor_linux.go index f5acae3d4..f8d18522e 100644 --- a/apps/daemon/internal/agenthost/executor_linux.go +++ b/apps/daemon/internal/agenthost/executor_linux.go @@ -136,11 +136,12 @@ func open(ctx context.Context, cfg Config, req agent.PrepareRequest, bind func(a s.ctx, s.cancel = context.WithCancel(ctx) context.AfterFunc(s.ctx, s.closeLive) s.exec, err = s.plan.view.Executor(s.ctx, s.plan.request, agent.ViewSession{ - Home: agent.ViewDir{Host: s.dir.entry(homeEntry), View: agent.ViewPrivateRoot + "/" + agent.ViewHomeName}, - Proxy: s.plan.proxy, - MCP: s.plan.mcp, - Launch: s.launch, - Spawn: s.spawn, + Home: agent.ViewDir{Host: s.dir.entry(homeEntry), View: agent.ViewPrivateRoot + "/" + agent.ViewHomeName}, + Proxy: s.plan.proxy, + MCP: s.plan.mcp, + StopMCP: s.stopMCP, + Launch: s.launch, + Spawn: s.spawn, }) if err != nil { if errors.Is(err, agent.ErrUnsupportedOperation) || errors.Is(err, agent.ErrViewHandoff) { diff --git a/apps/daemon/internal/agenthost/launch_linux.go b/apps/daemon/internal/agenthost/launch_linux.go index 8ca063197..1f1ebe7b3 100644 --- a/apps/daemon/internal/agenthost/launch_linux.go +++ b/apps/daemon/internal/agenthost/launch_linux.go @@ -9,6 +9,7 @@ import ( "io" "maps" "os" + "path" "slices" "sync" "syscall" @@ -25,7 +26,8 @@ import ( // liveView is the Session's one live view slot. type liveView struct { view runningView // nil while the view is being built - closed bool // the Session ended while the view was being built + broker *processbroker.Broker + closed bool // the Session ended while the view was being built } // runningView is the part of *sessionview.View the Session owns. @@ -157,6 +159,9 @@ func (s *session) start(lv *liveView, opts clirunner.StartOptions) (*clirunner.P var err error if scope, err = s.processScope(startCtx); err != nil { s.release(lv) + if errors.Is(err, agent.ErrUnsupportedOperation) { + return nil, err + } if startCtx.Err() != nil { return nil, fmt.Errorf("%w: describe: %w", ErrLaunch, err) } @@ -227,7 +232,43 @@ func (s *session) processScope(ctx context.Context) (sandboxprocess.Scope, error if err != nil { return 0, err } - return strongestScope(d.Capabilities) + scope, err := strongestScope(d.Capabilities) + if err == nil && len(s.plan.executables.Aliases) > 0 && scope != sandboxprocess.ScopeCgroupV2 { + return 0, unsupported("stdio MCP requires delegated cgroup v2 process scopes") + } + return scope, err +} + +// stopMCP joins the exact services selected by a cancelled Turn. Native client +// recovery belongs to the adapter and happens only after this returns. +func (s *session) stopMCP(ctx context.Context, labels []string) error { + aliases := make([]string, 0, len(labels)) + for _, label := range labels { + index := slices.IndexFunc(s.plan.mcp, func(binding agent.MCPBinding) bool { return binding.ServerLabel == label }) + if index < 0 || s.plan.mcp[index].Transport != "stdio" { + return unsupported("MCP scope termination requires a declared stdio binding") + } + alias := path.Base(agent.ViewAlias(index)) + if !slices.Contains(aliases, alias) { + aliases = append(aliases, alias) + } + } + if len(aliases) == 0 { + return nil + } + s.mu.Lock() + var broker *processbroker.Broker + if s.live != nil && !s.live.closed { + broker = s.live.broker + } + s.mu.Unlock() + if broker == nil { + return fmt.Errorf("%w: stop MCP: %w", ErrProcessBroker, agent.ErrNoLiveView) + } + if err := broker.StopAliases(ctx, aliases); err != nil { + return fmt.Errorf("%w: stop MCP: %w", ErrProcessBroker, err) + } + return nil } // brokerFailed fails the Session with a process broker failure. @@ -257,6 +298,7 @@ func (s *session) own(lv *liveView, v runningView, world viewWorld, stopGateway go h.watch() s.mu.Lock() lv.view = v + lv.broker = h.broker closed := lv.closed s.mu.Unlock() var err error diff --git a/apps/daemon/internal/agenthost/mcp_stop_linux_test.go b/apps/daemon/internal/agenthost/mcp_stop_linux_test.go new file mode 100644 index 000000000..28f601ce3 --- /dev/null +++ b/apps/daemon/internal/agenthost/mcp_stop_linux_test.go @@ -0,0 +1,84 @@ +//go:build linux + +package agenthost + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent" + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/processbroker" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxlink/sandboxlinktest" + sp "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxprocess" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +func TestStdioMCPRequiresDeclaredStrongScope(t *testing.T) { + for _, strong := range []bool{false, true} { + t.Run(map[bool]string{false: "no delegation", true: "delegated"}[strong], func(t *testing.T) { + auth := sandboxlinktest.NewAuthority() + relay := sandboxlinktest.StartRelay(t, auth) + binding, instance := newBinding(newResource()), sandboxwire.NewID() + cfg := Config{RelayURL: relay.URL, TLS: relay.TLS, RuntimeID: sandboxwire.NewID(), Credential: []byte("runtime")} + auth.AddRuntime(cfg.Credential, cfg.RuntimeID) + auth.AddServe([]byte("sandbox"), sandboxlink.ServePeer{PeerID: sandboxwire.NewID(), Resource: binding.Resource}) + auth.AddGrant(binding.AttachGrant, sandboxlinktest.Grant{RuntimeID: cfg.RuntimeID, Resource: binding.Resource, SessionID: binding.SessionID, + AssignmentID: binding.AssignmentID, AssignmentEpoch: binding.AssignmentEpoch, Services: []sandboxlink.Service{sandboxlink.ServiceProcess}, Lease: time.Minute}) + ready, ended := make(chan struct{}), make(chan error, 1) + ctx, cancel := context.WithCancel(t.Context()) + go func() { + ended <- sandboxlink.Serve(ctx, sandboxlink.ServeConfig{URL: relay.URL, TLS: relay.TLS, Credential: []byte("sandbox"), Resource: binding.Resource, + ServerInstanceID: instance, OnConnected: func() { close(ready) }, Services: []sandboxlink.ServiceHandler{{Service: sandboxlink.ServiceProcess, Version: sp.Version, + Serve: func(_ context.Context, _ sandboxlink.Bind, _ uint64, stream sandboxlink.Stream) { + defer stream.Close() + frame, err := sandboxwire.ReadFrame(stream, sandboxwire.MaxPayload) + if err != nil || frame.Type != sp.OpDescribe { + return + } + scopes := []sp.Scope{sp.ScopePOSIXSession} + if strong { + scopes = append(scopes, sp.ScopeCgroupV2) + } + response := sp.DescribeResponse{ServerInstanceID: instance, Capabilities: sp.Capabilities{Platform: sp.PlatformLinux, Scopes: scopes, IOModes: []sp.IOMode{sp.IOPipes}, + Signals: []sp.Signal{15}, SignalTargets: []sp.SignalTarget{sp.TargetInitialProcessGroup}, MaxStartBytes: sandboxwire.MaxPayload, + MaxDataBytes: sandboxwire.MaxChunk, MaxActiveOperations: 8, MaxOperationRecords: 8, MaxReplayBytesPerOperation: 1 << 20, + OwnerLossGraceMillis: 60000, CancelGraceLimitMillis: 60000}} + _ = sandboxwire.WriteFrame(stream, sandboxwire.Frame{Type: response.MessageType(), RequestID: frame.RequestID, Payload: sp.Encode(response)}) + }}}}) + }() + t.Cleanup(func() { cancel(); <-ended }) + select { + case <-ready: + case <-time.After(5 * time.Second): + t.Fatal("serve peer did not connect") + } + s, _ := newOwnerSession(t) + s.plan = &plan{executables: processbroker.Executables{Aliases: map[string]processbroker.Command{"a": {Executable: "/server", Dir: "/"}}}} + s.link = newLinkOwner(relayDial(cfg), binding, sandboxwire.NewID(), s.fail) + t.Cleanup(func() { _ = s.link.close() }) + scope, err := s.processScope(t.Context()) + if strong { + if err != nil || scope != sp.ScopeCgroupV2 { + t.Fatalf("scope %v: %v", scope, err) + } + } else if !errors.Is(err, agent.ErrUnsupportedOperation) || !errors.Is(err, ErrUnsupported) { + t.Fatalf("stdio admitted without strong containment: scope %v, %v", scope, err) + } + }) + } +} + +func TestStopMCPRejectsUndeclaredAndHTTPBindings(t *testing.T) { + s := &session{plan: &plan{mcp: []agent.MCPBinding{{ServerLabel: "stdio", Transport: "stdio"}, {ServerLabel: "http", Transport: "http"}}}} + for _, label := range []string{"unknown", "http"} { + if err := s.stopMCP(t.Context(), []string{label}); !errors.Is(err, agent.ErrUnsupportedOperation) { + t.Fatalf("%s: %v", label, err) + } + } + if err := s.stopMCP(t.Context(), []string{"stdio"}); !errors.Is(err, agent.ErrNoLiveView) { + t.Fatalf("missing live broker: %v", err) + } +} diff --git a/apps/daemon/internal/processbroker/alias_stop_linux_test.go b/apps/daemon/internal/processbroker/alias_stop_linux_test.go new file mode 100644 index 000000000..66e0246ee --- /dev/null +++ b/apps/daemon/internal/processbroker/alias_stop_linux_test.go @@ -0,0 +1,302 @@ +//go:build linux + +package processbroker + +import ( + "context" + "errors" + "net" + "os" + "sync" + "testing" + "time" + + "golang.org/x/sys/unix" + + "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/processshim" + sp "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxprocess" + "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" +) + +func TestStopAliasesRequiresScopeClosed(t *testing.T) { + for _, leaderExited := range []bool{false, true} { + t.Run(map[bool]string{false: "leader running", true: "leader exited"}[leaderExited], func(t *testing.T) { + p := newPeer() + p.holdScope = true + b, relay := newAliasBroker(t, p, 0) + relay.send(aliasOpen(1, "a")) + relay.await(t, relay.started, 1) + if leaderExited { + p.mu.Lock() + status := sp.ExitStatus{Kind: sp.ExitCode} + p.st.State, p.st.Exit, p.background = sp.StateExited, &status, true + p.emit(func(h sp.EventHeader) sp.Event { return sp.ExitedEvent{EventHeader: h, Status: status} }) + p.mu.Unlock() + relay.await(t, relay.exited, 1) + } + done := make(chan error, 1) + go func() { done <- b.StopAliases(t.Context(), []string{"a"}) }() + select { + case <-p.cancelled: + case <-time.After(5 * time.Second): + t.Fatal("target scope received no Cancel") + } + select { + case err := <-done: + t.Fatalf("Cancel acceptance/leader exit settled a live scope: %v", err) + default: + } + p.mu.Lock() + p.closeScope() + p.mu.Unlock() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("ScopeClosed did not settle target") + } + }) + } +} + +func TestStopAliasesFencesUnpreparedOpenAndPreservesOthers(t *testing.T) { + p := newPeer() + b, relay := newAliasBroker(t, p, 0) + // An Open is admitted, but its goroutine has not reached prepare or Start. + b.mu.Lock() + old := b.newInvocation(aliasOpen(1, "a")) + old.alias = "a" + b.invs[1], b.lastID = old, 1 + b.wg.Add(1) + b.mu.Unlock() + done := make(chan error, 1) + go func() { done <- b.StopAliases(t.Context(), []string{"a"}) }() + select { + case <-old.gone.Done(): + case <-time.After(5 * time.Second): + t.Fatal("admitted Open was not selected") + } + relay.send(aliasOpen(2, "a")) + relay.await(t, relay.ended, 2) + relay.send(aliasOpen(3, "b")) + relay.await(t, relay.started, 3) + go old.serve() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("unprepared target did not settle") + } + b.mu.Lock() + blocked, other := b.blocked["a"], b.invs[3] + b.mu.Unlock() + if blocked || other == nil || other.lost() || p.counts().cancels != 0 { + t.Fatal("target admission stayed closed or the unrelated service was cancelled") + } + if !old.lost() { + t.Fatal("reopening the alias released the selected old invocation") + } +} + +func TestStopAliasesRetainsUnconfirmedOperation(t *testing.T) { + b, relay := newAliasBroker(t, newPeer(), 0) + inv := b.newInvocation(aliasOpen(1, "a")) + inv.alias, inv.possibleEffect, inv.inst = "a", true, sandboxwire.NewID() + b.mu.Lock() + b.invs[1], b.lastID = inv, 1 + b.mu.Unlock() + inv.teardown() + if err := b.StopAliases(t.Context(), []string{"a"}); !errors.Is(err, ErrUnsettled) { + t.Fatalf("unconfirmed Start was accepted as no effects: %v", err) + } + b.mu.Lock() + retained := b.invs[1] == inv && inv.unsettled && b.blocked["a"] + b.mu.Unlock() + if !retained || inv.id.IsZero() || inv.inst.IsZero() { + t.Fatal("unconfirmed operation identity was discarded") + } + relay.send(aliasOpen(2, "a")) + relay.await(t, relay.ended, 1) + relay.await(t, relay.ended, 2) +} + +func TestStopAliasesInterruptedWaitKeepsOwnership(t *testing.T) { + p := newPeer() + p.holdScope = true + b, relay := newAliasBroker(t, p, 0) + relay.send(aliasOpen(1, "a")) + relay.await(t, relay.started, 1) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + done := make(chan error, 1) + go func() { done <- b.StopAliases(ctx, []string{"a"}) }() + select { + case <-p.cancelled: + case <-time.After(5 * time.Second): + t.Fatal("missing Cancel") + } + cancel() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) || !errors.Is(err, ErrUnsettled) { + t.Fatalf("wait outcome: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("cancelled wait did not return") + } + b.mu.Lock() + retained := b.blocked["a"] && b.invs[1] != nil + b.mu.Unlock() + if !retained { + t.Fatal("cancelled wait released ownership") + } + relay.send(aliasOpen(2, "a")) + relay.await(t, relay.ended, 2) + p.mu.Lock() + p.closeScope() + p.mu.Unlock() + relay.await(t, relay.ended, 1) +} + +func TestStopAliasesRejectsUnsupportedScopeAndUnknownTarget(t *testing.T) { + b := &Broker{cfg: Config{Scope: sp.ScopePOSIXSession}} + var failure *sp.Failure + if err := b.StopAliases(t.Context(), []string{"a"}); !errors.As(err, &failure) || failure.Code != sp.CodeUnsupported { + t.Fatalf("weak scope: %v", err) + } + b.cfg.Scope = sp.ScopeCgroupV2 + if err := b.StopAliases(t.Context(), []string{"unknown"}); !errors.As(err, &failure) || failure.Code != sp.CodeInvalidArgument { + t.Fatalf("unknown target: %v", err) + } +} + +func TestStopAliasesJoinsRetriedStartingOperation(t *testing.T) { + p := newPeer() + p.startLate, p.holdScope = true, true + b, relay := newAliasBroker(t, p, sp.OpStart) + relay.send(aliasOpen(1, "a")) + select { + case <-p.attachedStarting: + case <-time.After(5 * time.Second): + t.Fatal("uncertain Start was not reattached") + } + done := make(chan error, 1) + go func() { done <- b.StopAliases(t.Context(), []string{"a"}) }() + select { + case <-p.cancelled: + case <-time.After(5 * time.Second): + t.Fatal("Starting operation was not cancelled") + } + b.mu.Lock() + inv := b.invs[1] + b.mu.Unlock() + inv.shimGone() + if p.counts().cancels != 1 { + t.Fatal("shim loss resent the targeted cancellation") + } + select { + case err := <-done: + t.Fatalf("Starting operation settled: %v", err) + default: + } + p.runStarting() + relay.await(t, relay.started, 1) + select { + case err := <-done: + t.Fatalf("late Started/Exited settled without ScopeClosed: %v", err) + default: + } + p.mu.Lock() + p.closeScope() + p.mu.Unlock() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("late scope did not settle") + } +} + +type aliasRelay struct { + conn net.Conn + mu sync.Mutex + started, exited, ended chan uint64 +} + +func newAliasBroker(t *testing.T, p *peer, lose uint16) (*Broker, *aliasRelay) { + t.Helper() + p.strongScope = true + sv, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + t.Fatal(err) + } + left, right := os.NewFile(uintptr(sv[0]), "broker"), os.NewFile(uintptr(sv[1]), "relay") + defer left.Close() + defer right.Close() + c, err := net.FileConn(right) + if err != nil { + t.Fatal(err) + } + r := &aliasRelay{conn: c, started: make(chan uint64, 8), exited: make(chan uint64, 8), ended: make(chan uint64, 8)} + b, err := Start(Config{Relay: left, Scope: sp.ScopeCgroupV2, Dial: p.dial(lose), Executables: Executables{Aliases: map[string]Command{ + "a": {Executable: "/bin/server", Dir: "/"}, "b": {Executable: "/bin/other", Dir: "/"}, + }}}) + if err != nil { + c.Close() + t.Fatal(err) + } + go func() { + for { + f, err := sandboxwire.ReadFrame(c, processshim.MaxFrameBytes) + if err != nil { + return + } + m, err := processshim.DecodeBroker(f) + if err != nil { + return + } + switch m := m.(type) { + case processshim.Started: + r.started <- m.ID + case processshim.Exit: + r.exited <- m.ID + case processshim.End: + r.ended <- m.ID + case processshim.Output: + r.send(processshim.Written{ID: m.ID, FD: m.FD, Seq: m.Seq}) + case processshim.Close: + r.send(processshim.Written{ID: m.ID, FD: m.FD, Seq: m.Seq}) + } + } + }() + t.Cleanup(func() { b.Close(); c.Close() }) + return b, r +} + +func aliasOpen(id uint64, alias string) processshim.Open { + return processshim.Open{ID: id, Request: processshim.Request{Version: processshim.Version, ExecPath: []byte("/.oac/bin/" + alias), Argv: [][]byte{[]byte(alias)}, Cwd: []byte("/")}} +} + +func (r *aliasRelay) send(m processshim.RelayMessage) { + r.mu.Lock() + defer r.mu.Unlock() + _ = sandboxwire.WriteFrame(r.conn, processshim.Frame(m)) +} + +func (r *aliasRelay) await(t *testing.T, events <-chan uint64, want uint64) { + t.Helper() + select { + case id := <-events: + if id != want { + t.Fatalf("invocation %d, want %d", id, want) + } + case <-time.After(5 * time.Second): + t.Fatal("missing relay event") + } +} diff --git a/apps/daemon/internal/processbroker/broker_linux.go b/apps/daemon/internal/processbroker/broker_linux.go index 58810905c..8841f8d30 100644 --- a/apps/daemon/internal/processbroker/broker_linux.go +++ b/apps/daemon/internal/processbroker/broker_linux.go @@ -9,9 +9,12 @@ import ( "fmt" "log/slog" "net" + "path" + "slices" "sync" "github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/processshim" + sp "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxprocess" "github.com/MiniMax-AI/OpenAgentCore/internal/sandboxwire" ) @@ -33,6 +36,7 @@ type Broker struct { lastID uint64 closing bool err error + blocked map[string]bool closeOnce sync.Once } @@ -59,6 +63,7 @@ func Start(cfg Config) (*Broker, error) { b := &Broker{ cfg: cfg, log: log, conn: uc, link: newLink(cfg.Dial, log), ctx: ctx, cancel: cancel, done: make(chan struct{}), invs: map[uint64]*invocation{}, + blocked: map[string]bool{}, } b.wg.Add(1) go b.read() @@ -94,6 +99,77 @@ func (b *Broker) Err() error { return b.err } +// StopAliases stops only the selected frozen commands and waits for their +// sandbox scopes. The caller has drained native call admission and must not +// reconnect until success. Failed settlement leaves their admission closed. +func (b *Broker) StopAliases(ctx context.Context, aliases []string) error { + if len(aliases) == 0 { + return nil + } + if b.cfg.Scope != sp.ScopeCgroupV2 { + return sp.Fail(sp.CodeUnsupported, sandboxwire.EffectNone, "stdio MCP requires cgroup v2 scopes") + } + b.mu.Lock() + if b.closing || b.err != nil { + b.mu.Unlock() + return fmt.Errorf("%w: broker is ending", ErrUnsettled) + } + for _, alias := range aliases { + if _, ok := b.cfg.Executables.Aliases[alias]; !ok { + b.mu.Unlock() + return sp.Fail(sp.CodeInvalidArgument, sandboxwire.EffectNone, "unknown process alias") + } + if b.blocked[alias] { + b.mu.Unlock() + return ErrUnsettled + } + } + for _, alias := range aliases { + b.blocked[alias] = true + } + var selected []*invocation + for _, inv := range b.invs { + if slices.Contains(aliases, inv.alias) { + // Open already carries its complete Request. Mark it while the + // same lock excludes new admissions, even if prepare has not run. + inv.mu.Lock() + inv.stopping = true + inv.mu.Unlock() + inv.loseShim() + inv.helpers.Add(1) // unregister precedes teardown's Wait + selected = append(selected, inv) + } + } + b.mu.Unlock() + for _, inv := range selected { + go func() { + defer inv.helpers.Done() + inv.stopInput() + inv.cancelRemote() + }() + } + for _, inv := range selected { + select { + case <-inv.scopeDone: + case <-inv.finished: + if !inv.scopeSettled() { + return ErrUnsettled + } + case <-ctx.Done(): + return fmt.Errorf("%w: %w", ErrUnsettled, ctx.Err()) + } + } + b.mu.Lock() + defer b.mu.Unlock() + if b.closing || b.err != nil { + return ErrUnsettled + } + for _, alias := range aliases { + delete(b.blocked, alias) + } + return nil +} + // read dispatches the relay's messages until the connection ends or breaks // the IPC. It reads without a control buffer, so the kernel closes any // descriptor the relay attaches, and it never blocks on an invocation. @@ -135,6 +211,10 @@ func (b *Broker) dispatch(m processshim.RelayMessage) error { } b.lastID = id inv := b.newInvocation(open) + if _, command, ok := b.cfg.Executables.resolve(string(open.Request.ExecPath), string(open.Request.Cwd)); ok && command != nil { + inv.alias = path.Base(resolvePath(string(open.Request.ExecPath), string(open.Request.Cwd))) + inv.stopping = b.blocked[inv.alias] + } b.invs[id] = inv b.wg.Add(1) go inv.serve() @@ -142,6 +222,8 @@ func (b *Broker) dispatch(m processshim.RelayMessage) error { } inv := b.invs[id] switch { + case inv != nil && inv.unsettled: + return nil case inv != nil: return inv.receive(m) case id > b.lastID: @@ -154,6 +236,13 @@ func (b *Broker) dispatch(m processshim.RelayMessage) error { func (b *Broker) unregister(id uint64) { b.mu.Lock() defer b.mu.Unlock() + if inv := b.invs[id]; inv != nil && inv.alias != "" && !inv.scopeSettled() { + // Keep the exact operation identity after observation ended. Neither + // an empty map nor a new incarnation can repair an unknown effect. + inv.unsettled = true + b.blocked[inv.alias] = true + return + } delete(b.invs, id) } diff --git a/apps/daemon/internal/processbroker/config.go b/apps/daemon/internal/processbroker/config.go index 97596e84d..a0cf183e1 100644 --- a/apps/daemon/internal/processbroker/config.go +++ b/apps/daemon/internal/processbroker/config.go @@ -83,6 +83,8 @@ var ( // relay message that breaks the IPC. The broker then serves nothing // more, and the Session fails. ErrRelayLost = errors.New("processbroker: process relay lost") + // ErrUnsettled means a selected alias may still own sandbox effects. + ErrUnsettled = errors.New("processbroker: process scope settlement is unconfirmed") ) func (c *Config) validate() error { @@ -168,11 +170,7 @@ func underPrivate(p string) bool { // cwd; resolution is lexical, because the view's symlinks are not the // broker's to follow. func (x Executables) resolve(execPath, cwd string) (string, *Command, bool) { - p := execPath - if !path.IsAbs(p) { - p = path.Join(cwd, p) - } - p = path.Clean(p) + p := resolvePath(execPath, cwd) if name, ok := strings.CutPrefix(p, agent.ViewPrivateRoot+"/"+agent.ViewShimName+"/"); ok { if cmd, ok := x.Aliases[name]; ok { return cmd.Executable, &cmd, true @@ -184,6 +182,13 @@ func (x Executables) resolve(execPath, cwd string) (string, *Command, bool) { return remote, nil, ok } +func resolvePath(execPath, cwd string) string { + if !path.IsAbs(execPath) { + execPath = path.Join(cwd, execPath) + } + return path.Clean(execPath) +} + // privateMarker is the view prefix no value may carry into the sandbox. var privateMarker = []byte(agent.ViewPrivateRoot) diff --git a/apps/daemon/internal/processbroker/invocation_linux.go b/apps/daemon/internal/processbroker/invocation_linux.go index a967e84e8..1f542f55a 100644 --- a/apps/daemon/internal/processbroker/invocation_linux.go +++ b/apps/daemon/internal/processbroker/invocation_linux.go @@ -22,23 +22,28 @@ import ( // invocation is one shim invocation: the operation it started and the // streams the broker forwards for it through the relay. type invocation struct { - b *Broker - rid uint64 // the relay's invocation ID - open processshim.Open - log *slog.Logger - spec sp.ProcessSpec - id sandboxwire.ID - term *processshim.Terminal // nil for pipes + b *Broker + rid uint64 // the relay's invocation ID + open processshim.Open + log *slog.Logger + spec sp.ProcessSpec + id sandboxwire.ID + term *processshim.Terminal // nil for pipes + alias string // frozen alias, assigned before admission + unsettled bool // broker.mu: observation ended without scope proof // halt ends every wait of the invocation; stopIn ends stdin forwarding. - halt chan struct{} - haltOnce sync.Once - stopIn chan struct{} - stopOnce sync.Once - - writing sync.WaitGroup // the output writers - helpers sync.WaitGroup // everything else but the stdin pump - started chan struct{} // closed once the operation exists or never will + halt chan struct{} + haltOnce sync.Once + stopIn chan struct{} + stopOnce sync.Once + cancelOnce sync.Once // shim loss and targeted MCP stop share one Cancel + + writing sync.WaitGroup // the output writers + helpers sync.WaitGroup // everything else but the stdin pump + started chan struct{} // closed once the operation exists or never will + finished chan struct{} // closed after teardown + scopeDone chan struct{} // closed only by StartFailed or ScopeClosed // gone ends when the shim is lost. gone context.Context loseShim context.CancelFunc @@ -56,13 +61,15 @@ type invocation struct { sendMu sync.Mutex ended bool - mu sync.Mutex - inst sandboxwire.ID - cur handle - exited bool // the exit is decided: Exited, StartFailed or exit lost - shimLost bool - replied bool - credit uint32 // the outstanding Read's Max; 0 for none + mu sync.Mutex + inst sandboxwire.ID + cur handle + exited bool // the exit is decided: Exited, StartFailed or exit lost + shimLost bool + stopping bool // selected before the alias admission fence can reopen + possibleEffect bool // a Start may have created an operation + replied bool + credit uint32 // the outstanding Read's Max; 0 for none // settlement, from delivered events startFailed, outputClosed, scopeClosed bool } @@ -86,6 +93,7 @@ func (b *Broker) newInvocation(open processshim.Open) *invocation { b: b, rid: open.ID, open: open, log: b.log.With("invocation", open.ID), id: sandboxwire.NewID(), term: open.Terminal, halt: make(chan struct{}), stopIn: make(chan struct{}), started: make(chan struct{}), + finished: make(chan struct{}), scopeDone: make(chan struct{}), sigs: make(chan processshim.Signaled, 64), input: make(chan processshim.RelayMessage, 1), writers: map[sp.Stream]*writer{}, } @@ -109,6 +117,10 @@ func (b *Broker) newInvocation(open processshim.Open) *invocation { func (inv *invocation) serve() { defer inv.b.wg.Done() defer inv.teardown() + if inv.lost() { + inv.reply(*refuse(processshim.ExitCannotRun, "the MCP service is stopping"), nil) + return + } if refusal := inv.prepare(); refusal != nil { inv.reply(*refusal, nil) return @@ -317,6 +329,9 @@ func (inv *invocation) startOnce(deadline *time.Time, exists *bool) (handle, sta ctx, cancelStart = context.WithDeadline(inv.b.ctx, began.Add(resolveWindow)) defer cancelStart() } + inv.mu.Lock() + inv.possibleEffect = true + inv.mu.Unlock() op, disp, err := s.client.Start(ctx, inv.inst, inv.id, inv.spec) if err == nil { h := handle{op: op, s: s, relinked: make(chan struct{})} @@ -337,6 +352,9 @@ func (inv *invocation) startOnce(deadline *time.Time, exists *bool) (handle, sta *deadline = began.Add(resolveWindow) } if deadline.IsZero() { // nothing has started + inv.mu.Lock() + inv.possibleEffect = false + inv.mu.Unlock() switch { case s.ended(): return handle{}, startNow @@ -354,6 +372,9 @@ func (inv *invocation) startOnce(deadline *time.Time, exists *bool) (handle, sta inv.fail(fmt.Sprintf("the program's start could not be resolved: %s", f.Message)) return handle{}, startEnded case !*exists && f.Effect == sandboxwire.EffectNone && provesAbsence(f.Code): + inv.mu.Lock() + inv.possibleEffect = false + inv.mu.Unlock() inv.reply(*refuse(processshim.ExitCannotRun, "%s: %s", inv.spec.Executable, f.Message), nil) return handle{}, startEnded } @@ -553,10 +574,20 @@ func exitResult(s sp.ExitStatus) processshim.Result { func (inv *invocation) setSettlement(set func()) { inv.mu.Lock() + closed := inv.startFailed || inv.scopeClosed set() + if !closed && (inv.startFailed || inv.scopeClosed) { + close(inv.scopeDone) + } inv.mu.Unlock() } +func (inv *invocation) scopeSettled() bool { + inv.mu.Lock() + defer inv.mu.Unlock() + return !inv.possibleEffect || inv.startFailed || inv.scopeClosed +} + // isSettled mirrors the service's settlement: Release succeeds once it holds. func (inv *invocation) isSettled() bool { inv.mu.Lock() @@ -609,7 +640,7 @@ func (inv *invocation) decideExit() { func (inv *invocation) lost() bool { inv.mu.Lock() defer inv.mu.Unlock() - return inv.shimLost + return inv.shimLost || inv.stopping } func (inv *invocation) current() handle { @@ -689,6 +720,10 @@ func (inv *invocation) shimGone() { } func (inv *invocation) cancelRemote() { + inv.cancelOnce.Do(inv.sendCancel) +} + +func (inv *invocation) sendCancel() { select { case <-inv.started: case <-inv.halt: @@ -865,6 +900,7 @@ func (inv *invocation) stopInput() { // pump is not waited for, so teardown never waits for a stdin request still // on the stream; it sends nothing after End. func (inv *invocation) teardown() { + defer close(inv.finished) inv.halted() inv.b.unregister(inv.rid) inv.writing.Wait() diff --git a/apps/daemon/internal/processbroker/peer_linux_test.go b/apps/daemon/internal/processbroker/peer_linux_test.go index dd710444e..d214404b8 100644 --- a/apps/daemon/internal/processbroker/peer_linux_test.go +++ b/apps/daemon/internal/processbroker/peer_linux_test.go @@ -30,6 +30,9 @@ type peer struct { // write. A background process it leaves copies the rest of stdin to // stdout until end of file. leaderExits bool + strongScope bool + holdScope bool + cancelled chan struct{} instance sandboxwire.ID @@ -49,7 +52,7 @@ type seen struct { } func newPeer() *peer { - return &peer{instance: sandboxwire.NewID(), attachedStarting: make(chan struct{}), + return &peer{instance: sandboxwire.NewID(), attachedStarting: make(chan struct{}), cancelled: make(chan struct{}), seen: seen{st: sp.OperationStatus{State: sp.StateStarting, Scope: sp.ScopeStateActive, FirstRetained: 1}}} } @@ -122,9 +125,13 @@ func send(c net.Conn, requestID uint64, m sp.Message) { func (p *peer) handle(m sp.Message) (sp.Message, *sp.Failure) { switch m := m.(type) { case sp.DescribeRequest: + scopes := []sp.Scope{sp.ScopePOSIXSession} + if p.strongScope { + scopes = append(scopes, sp.ScopeCgroupV2) + } return sp.DescribeResponse{ServerInstanceID: p.instance, Capabilities: sp.Capabilities{ Platform: sp.PlatformLinux, - Scopes: []sp.Scope{sp.ScopePOSIXSession}, + Scopes: scopes, IOModes: []sp.IOMode{sp.IOPipes}, Signals: []sp.Signal{1, 2, 15}, SignalTargets: []sp.SignalTarget{sp.TargetInitialProcessGroup}, @@ -191,6 +198,11 @@ func (p *peer) handle(m sp.Message) (sp.Message, *sp.Failure) { case sp.CancelRequest: p.cancels++ p.exit(sp.ExitStatus{Kind: sp.ExitSignal, Signal: 15}) + select { + case <-p.cancelled: + default: + close(p.cancelled) + } return sp.CancelResponse{}, nil case sp.AckEventsRequest: return sp.AckEventsResponse{}, nil @@ -241,6 +253,9 @@ func (p *peer) run() { default: p.st.State = sp.StateRunning p.emit(func(h sp.EventHeader) sp.Event { return sp.StartedEvent{EventHeader: h} }) + if p.cancels > 0 { + p.exit(sp.ExitStatus{Kind: sp.ExitSignal, Signal: 15}) + } } } @@ -265,8 +280,15 @@ func (p *peer) exit(status sp.ExitStatus) { p.emit(func(h sp.EventHeader) sp.Event { return sp.ExitedEvent{EventHeader: h, Status: status} }) } p.emit(func(h sp.EventHeader) sp.Event { return sp.OutputClosedEvent{EventHeader: h, Disposition: d} }) + p.st.Output = &d + if !p.holdScope { + p.closeScope() + } +} + +func (p *peer) closeScope() { p.emit(func(h sp.EventHeader) sp.Event { return sp.ScopeClosedEvent{EventHeader: h} }) - p.st.Output, p.st.Scope = &d, sp.ScopeStateClosed + p.st.Scope = sp.ScopeStateClosed } // emit records an event and sends it to the subscribed stream; p.mu is diff --git a/contracts/agents-api/harness-onboarding.md b/contracts/agents-api/harness-onboarding.md index db8863d81..16a2a50c7 100644 --- a/contracts/agents-api/harness-onboarding.md +++ b/contracts/agents-api/harness-onboarding.md @@ -115,6 +115,8 @@ A Session owns one reusable Executor in its connected Runtime; a Turn owns one i - Include owned background work in settlement and keep the exact native cleanup target after a failure. Native termination belongs to the adapter; a bulk cleanup acknowledgement alone does not establish quiescence. - Every Turn implements `CancellationOutcome`. The snapshot keeps observed native identity and Usage and remains readable after cancellation. Missing evidence stays unset; an empty `DonePayload` means nothing has been observed, not that cancellation succeeded or is unsupported. Reading the snapshot does not wait for settlement. - `Turn.Cancel` requests cancellation; output closure signals teardown. Turn settlement still requires `AwaitSettlement` and any required `Executor.Close`; neither a successful cancellation request nor its snapshot replaces those waits. +- For a cancelled stdio MCP call, capture its declared service before native abort can finish the local request, retain starts admitted during that Turn's cancellation, and drain the native Turn before calling `ViewSession.StopMCP` with those service labels. The host stops the selected services and all their descendants, including their earlier background work, and confirms each old Process scope with `ScopeClosed`. It fences new alias starts during this operation; an unconfirmed stop retains ownership and blocks new starts for those aliases. Other MCP services and the workspace remain. HTTP MCP is outside this operation. Stdio MCP admission requires the Process service's delegated cgroup v2 scope; a node or container without that capability returns typed unsupported. +- After confirmed scope termination, confirm that the selected old native MCP clients cannot be reused and that subsequent calls create fresh connections through the maintained native owner before declaring the Executor reusable. A local MCP cancellation error, notification, process leader exit or attachment-close acknowledgment does not prove remote scope termination. Repeated or late cancellation joins the original Turn's recorded targets and result; it never selects a successor's services. **What the Runtime does around a Turn.** One output consumer starts before native Start, drains the bounded 64-frame channel and keeps the terminal observation until Start publication, Turn settlement and admitted operation receipts finish. Natural completion never calls Cancel. Input and function admission close before settlement; operations already admitted hold their barrier through native receipts and outbound acknowledgement. The Runtime sends cancellation to the Turn before waiting on that barrier, because a written input may need a native interrupt to produce its receipt. It joins native settlement, any required confirmed Executor close, output drain and all admitted operations before an applied acknowledgement or reuse, and only then forwards Done or an applied cancellation receipt. A failed Close can report failure while keeping the same Run and outstanding operations for retry; a closed caller wait cannot manufacture an applied input receipt. The Runtime commits native continuity and releases the old Run's admission before publishing Done, since the receiver may start another Turn at once; a late terminal-send failure belongs to the old Run and cannot invalidate a successor that already owns the Executor. Connection shutdown owns transport-loss cleanup. The settlement wait is ten seconds and the receipt send budget five seconds; a timeout is not proof of quiescence. diff --git a/contracts/agents-api/index.md b/contracts/agents-api/index.md index f551e352c..cbaf3d513 100644 --- a/contracts/agents-api/index.md +++ b/contracts/agents-api/index.md @@ -121,7 +121,9 @@ Each item is Core's deliberate or native behavior where the official service beh - The stream does not emit reasoning-summary events, Environment `pending` or `ready` events, or every pinned interim tool-output variant. - Native Item variants beyond those listed under [Turns and Items](./sessions-events.md#turns-and-items) are not projected, and Items cannot be modified. - A function result that cancellation prevents from being applied never appears as an Item. -- Under the current [POSIX process scope](../../docs/process-protocol.md#scope-and-signals), an Environment stdio MCP descendant that calls `setsid` can continue producing side effects after the Turn is cancelled and the Session is idle. Cancellation of these detached descendants is not qualified. +- Claude Code's native MCP error frames do not distinguish a local timeout or transport failure from a server's `isError` reply. Failed stdio calls remain eligible for cancellation within their current Turn, including calls for which the server may already have returned an error; successful native results remove their calls from that set. Cancelling such a service also ends its earlier background work under the [cancellation and settlement contract](./harness-onboarding.md#executor-and-turn-lifetimes). The next Turn does not inherit this set. +- MiniMax Code makes declared MCP available to its root ACP Session. Its native task children receive only `oac_workspace` from the generated shared MCP configuration; they do not inherit the root Session's declared MCP bindings. +- Codex cancellation is unqualified for MCP calls in an already active child Turn that a different root Turn steers, when the observer first sees that child Turn after it has settled. Native attribution remains with the original root Turn, and the steering receipt does not identify the child Turn; its locally failed calls cannot be reliably assigned to the cancelling root. - Pinned Codex can lose command output emitted before its stream subscription. - Behind the [credential gateway](./model-execution.md#credential-gateway), pinned Codex compacts history locally and never calls `/responses/compact`. - Claude Code and MiniMax Code report no public usage. diff --git a/contracts/agents-api/zh/harness-onboarding.md b/contracts/agents-api/zh/harness-onboarding.md index efe992c38..d1ef2a330 100644 --- a/contracts/agents-api/zh/harness-onboarding.md +++ b/contracts/agents-api/zh/harness-onboarding.md @@ -1,7 +1,7 @@ --- title: "添加 Harness" source: contracts/agents-api/harness-onboarding.md -source_hash: e69944fa75e85c1154c8ef5921195a84a065c95f2d2d99c91fcf6024651c3634 +source_hash: 699a60a843ec11e87474212b661dd03231d660d7b3bca0b39d34b979be93db49 --- **Harness** 是一种运行模型和工具循环的原生代理引擎（Codex、Claude Code、MiniMax Code）。**Harness 适配器**将 Runtime 的 Executor 和 Turn 契约转换到该引擎的 SDK 或协议。本文档定义 Runtime–Harness 协议：适配器接口及其生命周期义务、注册、支持声明和验收。 @@ -117,6 +117,8 @@ Session 在其已连接的 Runtime 中拥有一个可复用的 Executor；Turn - 结算必须包含所属的后台工作，并在失败后保留精确的原生清理目标。原生终止由适配器负责；仅有批量清理确认并不能证明已达到静默状态。 - 每个 Turn 都实现 `CancellationOutcome`。快照保留已观察到的原生身份和 Usage，并在取消后仍可读取。缺失的证据保持未设置；空的 `DonePayload` 表示未观察到任何内容，而不是表示取消成功或不受支持。读取快照不会等待结算。 - `Turn.Cancel` 请求取消；输出关闭表示拆卸开始。Turn 结算仍需要 `AwaitSettlement` 和所需的任何 `Executor.Close`；取消请求成功或其快照都不能替代这些等待。 +- 取消 stdio MCP 调用时，应在原生 abort 结束本地请求之前记录其已声明服务，保留该 Turn 取消期间准入的调用，在原生 Turn 排空后以这些服务标签调用 `ViewSession.StopMCP`。Host 终止选中的服务及其全部后代，包括这些服务先前产生的后台工作，并通过 `ScopeClosed` 确认各个旧 Process 作用域已结束。该操作期间阻止对应别名的新启动；未确认的终止保留所有权，并继续阻止这些别名的新启动。其他 MCP 服务与 workspace 保留。HTTP MCP 不属于此操作。Stdio MCP 准入要求 Process 服务具备已委派的 cgroup v2 作用域；缺少该能力的节点或容器返回 typed unsupported。 +- 作用域终止得到确认后，应确认选中的旧 MCP client 已无法复用，且后续调用会通过维护中的原生 owner 创建新连接，之后才能声明 Executor 可复用。本地 MCP 取消错误、通知、进程 leader 退出或 attachment 关闭确认都不能证明远端作用域已终止。重复或迟到的取消只等待原 Turn 已记录的目标和结果，不能重新选择后继 Turn 的服务。 **Runtime 在 Turn 前后执行的工作。** 一个输出消费者会在原生 Start 之前启动，耗尽有界的 64 帧通道，并将终态观察保留到 Start 发布、Turn 结算和已准入操作回执完成为止。正常完成绝不调用 Cancel。输入和函数准入会在结算前关闭；已准入的操作会持有其屏障，直至原生回执和出站确认完成。Runtime 会在等待该屏障之前向 Turn 发送取消，因为已写入的输入可能需要原生中断才能生成回执。Runtime 会汇合原生结算、所需的已确认 Executor 关闭、输出耗尽和所有已准入操作，然后应用确认或执行复用，之后才会转发 Done 或已应用的取消回执。Close 失败可以报告失败，同时保留同一 Run 和未完成操作以供重试；已关闭的调用方等待无法凭空生成已应用输入回执。Runtime 会在发布 Done 前提交原生连续性状态并释放旧 Run 的准入，因为接收方可能立即启动另一个 Turn；迟到的终态发送失败属于旧 Run，不能使已拥有 Executor 的后继对象失效。连接关闭负责传输丢失清理。结算等待时间为十秒，回执发送预算为五秒；超时不能证明已达到静默状态。 diff --git a/contracts/agents-api/zh/index.md b/contracts/agents-api/zh/index.md index 75f3236b8..abfb5727b 100644 --- a/contracts/agents-api/zh/index.md +++ b/contracts/agents-api/zh/index.md @@ -1,7 +1,7 @@ --- title: "Agents API 覆盖台账" source: contracts/agents-api/index.md -source_hash: 543ea294db498840ad973baee0255cf993060dcb191457edc85dddc6dfdf7fe6 +source_hash: a99cbf206e87abf84c43ed803ac1a4b054dc26f4ba09bc9ed849af91af066a6f --- Core 旨在以下方固定版本为准支持完整的 OpenAI Agents API（[public API rule](https://github.com/MiniMax-AI/OpenAgentCore/blob/main/AGENTS.md#public-api)）。本台账记录 Core 对各项资源实现了哪些内容、哪些契约保存其详细信息，并列出相对于 OpenAI 服务的所有已知差异和所有未解决缺口。[API namespaces and credentials](../../../docs/zh/api/index.md) 说明谁调用哪些 API；[Agents API guide](../../../docs/zh/api/public-agent-api.md) 介绍使用方法。 @@ -123,7 +123,9 @@ Core 自身字段位于 `x_agents_core` 中（[Core extensions](../../../docs/zh - 流不会发出 reasoning-summary 事件、Environment 的 `pending` 或 `ready` 事件，也不会覆盖固定版本中的所有临时 tool-output 变体。 - 除 [Turns and Items](sessions-events.md#turns-and-items) 中列出的变体外，其他原生 Item 变体不会被投影，而且 Items 无法修改。 - 如果取消导致函数结果无法应用，该结果将永远不会作为 Item 出现。 -- 在当前的 [POSIX 进程作用域](../../../docs/zh/process-protocol.md#scope-and-signals)下，调用 `setsid` 的 Environment stdio MCP 后代进程可能在 Turn 已取消且 Session 已空闲后继续产生副作用。这类脱离作用域的后代进程的取消尚未通过资格验证。 +- Claude Code 的原生 MCP 错误帧无法区分本地超时或传输失败与服务端的 `isError` 回复。失败的 stdio 调用会在其当前 Turn 内保留为取消目标，包括服务端可能已经返回错误的调用；成功的原生结果会将其调用从该集合移除。按照[取消与结算契约](./harness-onboarding.md#executor-and-turn-lifetimes)，取消此类服务也会终止该服务先前的后台工作。后继 Turn 不继承这个集合。 +- MiniMax Code 仅向 root ACP Session 提供已声明的 MCP。其原生 task 子代理只从生成的共享 MCP 配置中获得 `oac_workspace`，不继承 root Session 已声明的 MCP 绑定。 +- 当另一个 root Turn 向已活跃的子 Turn 追加引导，而观察器首次看到该子 Turn 时它已经结算，Codex 对其中 MCP 调用的取消尚未通过资格验证。原生归属仍指向最初的 root Turn，且引导回执不标识子 Turn；因此无法可靠地将其中本地失败的调用归属于正在取消的 root Turn。 - 固定版本的 Codex 可能会丢失在订阅其流之前发出的命令输出。 - 在[凭据网关](./model-execution.md#credential-gateway)之后，固定版本的 Codex 在本地压缩历史，从不调用 `/responses/compact`。 - Claude Code 和 MiniMax Code 都不报告公共用量。 diff --git a/deploy/distribution/AgentHost.Dockerfile b/deploy/distribution/AgentHost.Dockerfile index c6a89664e..668c04a9c 100644 --- a/deploy/distribution/AgentHost.Dockerfile +++ b/deploy/distribution/AgentHost.Dockerfile @@ -25,13 +25,13 @@ RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates && rm -rf /var/lib/apt/lists/* COPY --chmod=0555 oac-daemon oac-process-shim /opt/oac/bin/ COPY --chmod=0555 codex/codex codex/codex-code-mode-host /opt/oac/harnesses/codex/bin/ -COPY codex/package.json /opt/oac/harnesses/codex/package.json +COPY codex/provenance.json /opt/oac/harnesses/codex/provenance.json COPY claude/claude-sdk /opt/oac/harnesses/claude_sdk COPY mcode/mcode-harness /opt/oac/harnesses/mcode COPY < `mcp__functions__${tool.name}`); const allowed = [...names, ...(request.tool_search ? ["ToolSearch"] : [])]; const declarations = request.workspace?.mcp ?? request.mcp_http_servers; - const profile = declarations === undefined ? undefined : new MCPProfile(declarations, names); + let mcp: MCPObserver | undefined; + const profile = declarations === undefined ? undefined : new MCPProfile(declarations, names, (id, name) => mcp!.admit(id, name)); + const stdio = new Set(declarations?.filter(server => "command" in server).map(server => server.server_label)); const subagents = request.subagents ? new Subagents(request.cwd, request.subagents.max_concurrent, request.resume) : undefined; const workspace = request.workspace === undefined ? undefined : new WorkspaceProfile(request.cwd, request.workspace, names, profile, subagents, !!request.output_format, !!request.tool_search); let commands = workspace ? new CommandObserver() : undefined; @@ -63,7 +65,7 @@ export async function execute(request: Start | Prepare | ExecutorPrepare, emit: subagents?.expectSession(request.resume); const mcpServers: Record = Object.create(null); if (definitions.length) mcpServers.functions = createFunctionServer(definitions, (call,signal) => { const target=functions; return turns ? turns.track(()=>target.invoke(call,signal)) : target.invoke(call,signal); }); - let mcp = profile ? new MCPObserver(profile.identities) : undefined; + mcp = profile ? new MCPObserver(profile.identities, stdio) : undefined; if (profile) Object.assign(mcpServers, profile.servers); const children: Promise[] = []; let result: Extract | undefined; @@ -142,15 +144,15 @@ export async function execute(request: Start | Prepare | ExecutorPrepare, emit: messages=new MessageObserver(); structured=request.output_format ? new StructuredOutput() : undefined; commands=workspace ? new CommandObserver() : undefined; - mcp=profile ? new MCPObserver(profile.identities) : undefined; + mcp=profile ? new MCPObserver(profile.identities, stdio) : undefined; subagents?.beginTurn(); return inputs; },async value=>{ if(value.type==="steer") for(const event of inputs.submit(value)) await emit(event); else functions.submit(JSON.stringify(value)); - }); + }, () => mcp?.cancel()); if (request.type !== "executor_prepare" || Date.now() >= request.preparation_deadline) throw new Error("preparation expired"); - await ownerEmit({type:"executor_ready",protocol:3}); + await ownerEmit({type:"executor_ready",protocol:4}); } else await emit({ type: "prepared" }); } else if (profile) { if (inputs.hasInput) throw new Error("MCP input released before initialization"); @@ -204,6 +206,12 @@ export async function execute(request: Start | Prepare | ExecutorPrepare, emit: if (turns.cancelled) functions.cancelUnanswered(); if(!await turns.quiescent() || abort.signal.aborted) throw new Error("unconfirmed interrupt"); functions.assertComplete(); mcp?.assertComplete(); commands?.assertComplete(); + const stopped = mcp?.cancelledServers ?? []; + if (stopped.length) { + await turns.stopMCP(stopped); + for (const server of stopped) await stream.reconnectMcpServer(server); + profile!.verifyReconnected(await stream.mcpServerStatus()); + } if(subagents) for(const event of await subagents.facts()) await emit(event); await emit(turns.cancelled ? {type:"error",code:"cancelled"} : result); functions.close(); diff --git a/packages/claude-sdk-adapter/src/executor_protocol.ts b/packages/claude-sdk-adapter/src/executor_protocol.ts index b8e715adf..24b4cc6af 100644 --- a/packages/claude-sdk-adapter/src/executor_protocol.ts +++ b/packages/claude-sdk-adapter/src/executor_protocol.ts @@ -3,11 +3,12 @@ import { Inputs } from "./inputs.js"; import { parseMessageInput, type MessageInput } from "./message_input.js"; export type ExecutorEvent = - | { type: "executor_ready"; protocol: 3 } + | { type: "executor_ready"; protocol: 4 } | { type: "turn_started"; turn_id: string } + | { type: "mcp_stop"; turn_id: string; servers: string[] } | { type: "turn_settled"; turn_id: string; confirmed: boolean; reusable: boolean; reason: string }; type WireEvent = { type: string; [key: string]: unknown }; -type Turn = { id: string; inputs: Inputs; cancelled: boolean; interrupt?: Promise; callbacks: Set> }; +type Turn = { id: string; inputs: Inputs; cancelled: boolean; interrupt?: Promise; callbacks: Set>; stoppedMCP?: (confirmed: boolean) => void }; // The SDK sees one iterator for the Executor lifetime. Each yielded batch belongs // to a fresh receipt ledger; finishing a Turn never closes this outer iterator. @@ -20,10 +21,11 @@ export class ExecutorTurns implements AsyncIterable { private stream?: Query; private begin?: (input: MessageInput, emit: (event: WireEvent) => Promise) => Inputs; private submitInput?: (value: Record) => Promise; + private cancelCalls?: () => void; private nativeID = ""; constructor(private readonly output: (event: WireEvent) => Promise, private readonly abort: AbortController) {} - configure(stream: Query, begin: NonNullable, submit: (value: Record)=>Promise): void { this.stream = stream; this.begin = begin; this.submitInput=submit; } + configure(stream: Query, begin: NonNullable, submit: (value: Record)=>Promise, cancelCalls: () => void = () => {}): void { this.stream = stream; this.begin = begin; this.submitInput=submit; this.cancelCalls=cancelCalls; } async submit(value: Record): Promise { const payload=this.assertCurrent(value); if(!this.submitInput || (payload.type!=="steer" && payload.type!=="function_result")) throw new Error("invalid turn input"); @@ -65,6 +67,7 @@ export class ExecutorTurns implements AsyncIterable { } if (turn.interrupt) return; turn.cancelled = true; + this.cancelCalls?.(); const discarded=turn.inputs.cancelQueued(); turn.interrupt = this.stream!.interrupt().then(receipt => { const confirmed = !discarded && receipt !== undefined && Array.isArray(receipt.still_queued) && receipt.still_queued.length === 0; @@ -74,6 +77,29 @@ export class ExecutorTurns implements AsyncIterable { await turn.interrupt; } + async stopMCP(servers: string[]): Promise { + const turn = this.active; + if (!turn?.cancelled || turn.stoppedMCP || this.abort.signal.aborted) throw new Error("invalid MCP stop"); + let confirm!: (confirmed: boolean) => void; + const receipt = new Promise(resolve => { confirm = resolve; }); + turn.stoppedMCP = confirm; + const abort = () => confirm(false); + this.abort.signal.addEventListener("abort", abort, { once: true }); + try { + await this.emit({ type: "mcp_stop", servers }); + if (!await receipt) throw new Error("MCP stop unconfirmed"); + } finally { this.abort.signal.removeEventListener("abort", abort); } + } + + mcpStopped(value: Record): void { + const turn = this.active; + if (!turn?.cancelled || turn.id !== value.turn_id || !turn.stoppedMCP || typeof value.confirmed !== "boolean" || + Object.keys(value).some(key => !["type", "turn_id", "confirmed"].includes(key))) throw new Error("invalid MCP stop receipt"); + const complete = turn.stoppedMCP; + turn.stoppedMCP = undefined; + complete(value.confirmed); + } + assertCurrent(value: Record): Record { if (!this.active || this.active.cancelled || value.turn_id !== this.active.id) throw new Error("inactive turn input"); const { turn_id: _id, ...payload } = value; diff --git a/packages/claude-sdk-adapter/src/main.ts b/packages/claude-sdk-adapter/src/main.ts index 891e77080..1e473907d 100644 --- a/packages/claude-sdk-adapter/src/main.ts +++ b/packages/claude-sdk-adapter/src/main.ts @@ -38,6 +38,7 @@ try { const control=value as Record; if(control.type==="turn_start") await turns.start(control); else if(control.type==="turn_cancel") await turns.cancel(control); + else if(control.type==="mcp_stopped") turns.mcpStopped(control); else await turns.submit(control); } else if (request.type === "prepare" && phase !== "running") { if (phase !== "prepared" || abort.signal.aborted) throw new Error("invalid_request"); diff --git a/packages/claude-sdk-adapter/src/mcp.ts b/packages/claude-sdk-adapter/src/mcp.ts index 7d924b8a7..738669c5c 100644 --- a/packages/claude-sdk-adapter/src/mcp.ts +++ b/packages/claude-sdk-adapter/src/mcp.ts @@ -58,7 +58,10 @@ export class MCPProfile { if (!signal.aborted && await Promise.race([this.ready, interrupted]) && !signal.aborted && this.admitted && input.hook_event_name === "PreToolUse" && input.agent_id === undefined && input.session_id === this.sessionID && (id === undefined || id === input.tool_use_id) && - (this.identities.has(input.tool_name) || this.functions.includes(input.tool_name) || this.localTools.has(input.tool_name))) return {}; + (this.identities.has(input.tool_name) || this.functions.includes(input.tool_name) || this.localTools.has(input.tool_name))) { + if (this.identities.has(input.tool_name)) this.admitCall(input.tool_use_id, input.tool_name); + return {}; + } return { hookSpecificOutput: { hookEventName: "PreToolUse", permissionDecision: "deny", permissionDecisionReason: "Tool is outside the verified execution profile." } }; } finally { @@ -66,7 +69,7 @@ export class MCPProfile { } }; - constructor(private readonly declarations: (HTTPServer | StdioServer)[], private readonly functions: string[]) { + constructor(private readonly declarations: (HTTPServer | StdioServer)[], private readonly functions: string[], private readonly admitCall: (id: string, name: string) => void = () => {}) { this.allowed = [...functions]; for (const server of declarations) { const prefix = `mcp__${server.server_label}__`; @@ -132,6 +135,16 @@ export class MCPProfile { } } + verifyReconnected(statuses: McpServerStatus[]): void { + const identities = new Map(this.identities); + const inventory = [...this.functions, ...this.localTools, ...this.identities.keys()]; + this.verify(inventory, statuses, this.sessionID, [...this.localTools]); + for (const [name, identity] of identities) { + const current = this.identities.get(name); + if (current?.server !== identity.server || current.name !== identity.name) throw new Error("changed native MCP identity"); + } + } + permits(name: string): boolean { return this.admitted && this.identities.has(name); } close(): void { this.admitted = false; this.release(false); } diff --git a/packages/claude-sdk-adapter/src/mcp_observer.ts b/packages/claude-sdk-adapter/src/mcp_observer.ts index 6aaccaefb..e394aea26 100644 --- a/packages/claude-sdk-adapter/src/mcp_observer.ts +++ b/packages/claude-sdk-adapter/src/mcp_observer.ts @@ -13,7 +13,24 @@ export type MCPEvent = { type: "mcp_observation"; id: string; stage: "before" | export class MCPObserver { private readonly calls = new Map(); - constructor(private readonly tools: Map) {} + private readonly admitted = new Map(); + private readonly interrupted = new Set(); + private cancelling = false; + constructor(private readonly tools: Map, private readonly stdio = new Set()) {} + + admit(id: string, name: string): void { + const identity = this.tools.get(name); + if (!id || !identity || this.admitted.has(id)) throw new Error("invalid MCP admission identity"); + this.admitted.set(id, { ...identity, pending: true }); + if (this.cancelling && this.stdio.has(identity.server)) this.interrupted.add(identity.server); + } + + cancel(): void { + this.cancelling = true; + for (const { server } of this.admitted.values()) if (this.stdio.has(server)) this.interrupted.add(server); + } + + get cancelledServers(): string[] { return [...this.interrupted].sort(); } consume(message: SDKMessage, sessionID: string): MCPEvent[] { if ((message.type !== "assistant" && message.type !== "user") || message.parent_tool_use_id !== null || @@ -42,6 +59,13 @@ export class MCPObserver { } else { const results = content.filter(block => block.type === "tool_result"); for (const block of results) { + const admission = this.admitted.get(block.tool_use_id); + if (admission) { + admission.pending = false; + // Native failures include local timeouts and indistinguishable server + // errors. Keep their remote ownership until this Turn ends. + if (!block.is_error || !this.stdio.has(admission.server)) this.admitted.delete(block.tool_use_id); + } const observation = this.calls.get(block.tool_use_id); if (!observation) continue; if (observation.status !== "in_progress") throw new Error("repeated MCP result"); @@ -58,7 +82,7 @@ export class MCPObserver { } assertComplete(): void { - if ([...this.calls.values()].some(call => call.status === "in_progress")) throw new Error("unconfirmed MCP result"); + if ([...this.admitted.values()].some(call => call.pending) || [...this.calls.values()].some(call => call.status === "in_progress")) throw new Error("unconfirmed MCP result"); } close(): MCPEvent[] { diff --git a/packages/claude-sdk-adapter/src/runtime_check.ts b/packages/claude-sdk-adapter/src/runtime_check.ts index fe093b1fd..2450424ed 100644 --- a/packages/claude-sdk-adapter/src/runtime_check.ts +++ b/packages/claude-sdk-adapter/src/runtime_check.ts @@ -45,7 +45,7 @@ try { assert.equal(smoke.error, undefined, "bridge_unavailable"); assert.equal(smoke.status, 0, "bridge_unavailable"); assert.deepEqual(JSON.parse(smoke.stdout), { type: "error", code: "invalid_request" }); - process.stdout.write(JSON.stringify({ type: "runtime_ready", protocol: 3, features: ["executor_reuse", ...(["linux", "darwin", "win32"].includes(process.platform) ? ["local_runtime_v2", "workspace_functions", "workspace_structured_output", "workspace_tool_search", "workspace_mcp_http"] : []), "message_images", "function_result_images", "tool_search", "structured_output", "subagent_resources", "mcp_http_tools", "mcp_http_bearer_auth", "mcp_http_required", "workspace_tools", "workspace_prepare", "workspace_command_observations"], node: process.versions.node, sdk: sdk.version, mcp: mcp.version, native: nativeVersion, native_path: relative(root, binary) }) + "\n"); + process.stdout.write(JSON.stringify({ type: "runtime_ready", protocol: 4, features: ["executor_reuse", ...(["linux", "darwin", "win32"].includes(process.platform) ? ["local_runtime_v2", "workspace_functions", "workspace_structured_output", "workspace_tool_search", "workspace_mcp_http"] : []), "message_images", "function_result_images", "tool_search", "structured_output", "subagent_resources", "mcp_http_tools", "mcp_http_bearer_auth", "mcp_http_required", "workspace_tools", "workspace_prepare", "workspace_command_observations"], node: process.versions.node, sdk: sdk.version, mcp: mcp.version, native: nativeVersion, native_path: relative(root, binary) }) + "\n"); } catch { // Native diagnostics can include operator environment; never forward them. process.stdout.write(JSON.stringify({ type: "runtime_unavailable" }) + "\n"); diff --git a/packages/claude-sdk-adapter/tests/executor.test.mjs b/packages/claude-sdk-adapter/tests/executor.test.mjs index 97778816f..3c33e1b90 100644 --- a/packages/claude-sdk-adapter/tests/executor.test.mjs +++ b/packages/claude-sdk-adapter/tests/executor.test.mjs @@ -1,5 +1,8 @@ import assert from "node:assert/strict"; import { spawn } from "node:child_process"; +import { mkdtempSync, mkdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import test from "node:test"; const fixture = ` @@ -20,17 +23,42 @@ globalThis.startupFixture=async({options})=>{ await Promise.all([client.connect(a),options.mcpServers.functions.instance.connect(b)]); } let queried=false,interrupt,number=0; + const mcp=process.argv[1].startsWith('mcp-'); + const statuses=['target','healthy'].map(name=>({name,status:'connected',tools:[{name:'hold'}]})); const close=()=>{child.stdin.end();interrupt?.()}; options.abortController.signal.addEventListener('abort',close); return {close,query(prompt){assert.equal(queried,false);queried=true;process.send({type:'query'});return { close,async initializationResult(){if(process.argv[1]==="late-ready"){const expired=Date.now()+120000;Date.now=()=>expired;}return process.argv[1]==="no-hook-report" ? {} : process.argv[1]==="false-hook-report" ? {hooks_applied:false} : {hooks_applied:true}}, async interrupt(){process.send({type:'interrupt'});interrupt?.();if(process.argv[1]==='rejected')throw new Error('interrupt failed');return ['unknown','pending-function-unknown'].includes(process.argv[1]) ? undefined : {still_queued:process.argv[1]==='queued' ? ['not-consumed'] : []}}, + async mcpServerStatus(){return statuses}, + async reconnectMcpServer(name){ + process.send({type:'reconnect',name}); + await new Promise(resolve=>process.once('message',resolve)); + if(process.argv[1]==='mcp-reconnect-failed')throw new Error('reconnect failed'); + }, async *[Symbol.asyncIterator](){ for await(const user of prompt){ number++; const text=user.message.content[0].text; process.send({type:'input',text}); - yield {type:'system',subtype:'init',session_id:'native',tools:client ? ['mcp__functions__lookup'] : options.outputFormat ? ['StructuredOutput'] : [],mcp_servers:client ? [{name:'functions',status:'connected'}] : []}; + yield {type:'system',subtype:'init',session_id:'native',tools:mcp ? ['Bash','Read','Edit','mcp__target__hold','mcp__healthy__hold'] : client ? ['mcp__functions__lookup'] : options.outputFormat ? ['StructuredOutput'] : [],mcp_servers:client ? [{name:'functions',status:'connected'}] : []}; + if(mcp && text==='hold'){ + assert.deepEqual(await options.hooks.PreToolUse[0].hooks[0]({hook_event_name:'PreToolUse',session_id:'native',tool_use_id:'held',tool_name:'mcp__target__hold',tool_input:{}},'held',{signal:new AbortController().signal}),{}); + const stopped=new Promise(resolve=>{interrupt=resolve}); + process.send({type:'call_admitted'}); + if(process.argv[1]!=='mcp-failed-before-cancel')await Promise.race([stopped,exited]); + yield {type:'assistant',session_id:'native',parent_tool_use_id:null,message:{content:[{type:'tool_use',id:'held',name:'mcp__target__hold',input:{}}]}}; + yield {type:'user',session_id:'native',parent_tool_use_id:null,message:{content:[{type:'tool_result',tool_use_id:'held',content:'Interrupted',is_error:true}]}}; + if(process.argv[1]==='mcp-failed-before-cancel'){ + process.send({type:'local_call_failed'}); + await Promise.race([stopped,exited]); + } + } + if(mcp && text==='wait'){ + const stopped=new Promise(resolve=>{interrupt=resolve}); + process.send({type:'empty_wait'}); + await Promise.race([stopped,exited]); + } if(client){ if(text==='pending-function'){ void client.callTool({name:'lookup',arguments:{text},_meta:{'claudecode/toolUseId':'pending-call'}}).catch(()=>{}); @@ -70,7 +98,7 @@ globalThis.startupFixture=async({options})=>{ yield {type:'result',uuid:'error-result',session_id:'native',user_message_uuids:[user.uuid],subtype:'error_during_execution',is_error:true,usage:{input_tokens:1,output_tokens:1},modelUsage:{},total_cost_usd:0}; continue; } - if(text==='hold' && !options.outputFormat) await Promise.race([new Promise(resolve=>{interrupt=resolve}),exited]); + if(text==='hold' && !options.outputFormat && !mcp) await Promise.race([new Promise(resolve=>{interrupt=resolve}),exited]); if(options.abortController.signal.aborted)return; yield {type:'result',uuid:'result-'+number,session_id:'native',user_message_uuids:[user.uuid],subtype:'success',is_error:false,result:'answer-'+number,...(options.outputFormat ? {structured_output:{number:9007199254740992}} : {}),usage:{input_tokens:1,output_tokens:1},modelUsage:{fixture:{inputTokens:number,outputTokens:number,costUSD:number/100}},total_cost_usd:number/100}; interrupt=undefined; @@ -85,7 +113,14 @@ process.disconnect(); `; async function launch(t,mode="normal") { - const child=spawn(process.execPath,["--input-type=module","-e",fixture,mode],{stdio:["pipe","pipe","pipe","ipc"]}); + let workspace; + if(mode.startsWith("mcp-")) { + const root=mkdtempSync(join(tmpdir(),"claude-mcp-cancel-")); + t.after(()=>rmSync(root,{recursive:true,force:true})); + for(const dir of ["home","state","scratch"])mkdirSync(join(root,dir)); + workspace={home:join(root,"home"),state:join(root,"state"),scratch:join(root,"scratch"),env_names:[],network_access:"enabled",mcp:["target","healthy"].map(server_label=>({server_label,command:"/.oac/bin/"+server_label,allowed_tools:null}))}; + } + const child=spawn(process.execPath,["--input-type=module","-e",fixture,mode],{stdio:["pipe","pipe","pipe","ipc"],env:{...process.env,...(workspace?{HOME:workspace.home,CLAUDE_CONFIG_DIR:workspace.state}:{})}}); const events=[],observations=[];let buffer="",stderr=""; child.stdout.setEncoding("utf8");child.stderr.setEncoding("utf8"); child.stdout.on("data",data=>{buffer+=data;while(buffer.includes("\n")){const end=buffer.indexOf("\n");events.push(JSON.parse(buffer.slice(0,end)));buffer=buffer.slice(end+1)}}); @@ -96,6 +131,7 @@ async function launch(t,mode="normal") { const send=value=>child.stdin.write(JSON.stringify(value)+"\n"); const start=(id,text)=>send({type:"turn_start",turn_id:id,input:[{content:[{type:"input_text",text}]}]}); send({type:"executor_prepare",preparation_deadline:Date.now()+60000,cwd:"/tmp",model:"fixture",system_prompt:"", + ...(workspace?{workspace}:{}), ...(mode==="structured" ? {output_format:{type:"json_schema",schema:{type:"object",properties:{number:{type:"integer"}}}}} : {}), ...(mode==="features" || mode.startsWith("pending-function") ? {functions:[{name:"lookup",description:"lookup",parameters:{type:"object",properties:{text:{type:"string"}}}}]} : {})}); await wait(()=>events.some(event=>event.type===(mode==="late-ready"?"error":"executor_ready"))); @@ -119,6 +155,50 @@ test("executor retains one native process and one Query over two settled Turns", assert.equal(observations.filter(event=>event.type==="native_closed").length,1); }); +for (const mode of ["mcp-stop", "mcp-failed-before-cancel", "mcp-reconnect-failed", "mcp-stop-unconfirmed", "mcp-wrong-receipt"]) test(`stdio cancellation ${mode} awaits remote scope and named reconnect before settlement`, {timeout:10000}, async t => { + const {child,events,observations,closed,wait,send,start}=await launch(t,mode); + start("first","hold"); + await wait(()=>observations.some(event=>event.type===(mode==="mcp-failed-before-cancel"?"local_call_failed":"call_admitted"))); + send({type:"turn_cancel",turn_id:"first"}); + send({type:"turn_cancel",turn_id:"first"}); + await wait(()=>events.some(event=>event.type==="mcp_stop")); + assert.deepEqual(events.find(event=>event.type==="mcp_stop"),{type:"mcp_stop",servers:["target"],turn_id:"first"}); + assert.equal(events.some(event=>event.type==="turn_settled"),false); + assert.equal(observations.some(event=>event.type==="reconnect"),false); + send({type:"mcp_stopped",turn_id:mode==="mcp-wrong-receipt"?"other":"first",confirmed:mode!=="mcp-stop-unconfirmed"}); + if(mode==="mcp-stop-unconfirmed" || mode==="mcp-wrong-receipt") { + await wait(()=>events.some(event=>event.type==="turn_settled")); + assert.equal(events.find(event=>event.type==="turn_settled").confirmed,false); + assert.equal(events.find(event=>event.type==="turn_settled").reusable,false); + assert.equal(observations.some(event=>event.type==="reconnect"),false); + assert.deepEqual(await closed,{code:0,signal:null}); + return; + } + await wait(()=>observations.some(event=>event.type==="reconnect")); + assert.equal(events.some(event=>event.type==="turn_settled"),false); + assert.deepEqual(observations.filter(event=>event.type==="reconnect"),[{type:"reconnect",name:"target"}]); + child.send("release-reconnect"); + await wait(()=>events.some(event=>event.type==="turn_settled")); + const confirmed=mode==="mcp-stop" || mode==="mcp-failed-before-cancel"; + assert.equal(events.find(event=>event.type==="turn_settled").confirmed,confirmed); + assert.equal(events.find(event=>event.type==="turn_settled").reusable,confirmed); + assert.equal(observations.filter(event=>event.type==="interrupt").length,1); + if(confirmed){ + start("second",mode==="mcp-failed-before-cancel"?"wait":"answer"); + send({type:"turn_cancel",turn_id:"first"}); + if(mode==="mcp-failed-before-cancel"){ + await wait(()=>observations.some(event=>event.type==="empty_wait")); + send({type:"turn_cancel",turn_id:"second"}); + } + await wait(()=>events.some(event=>event.type==="turn_settled"&&event.turn_id==="second")); + assert.equal(events.find(event=>event.type==="turn_settled"&&event.turn_id==="second").confirmed,true); + assert.equal(events.filter(event=>event.type==="mcp_stop").length,1); + assert.equal(observations.filter(event=>event.type==="native").length,1); + child.stdin.end(); + } + assert.deepEqual(await closed,{code:0,signal:null}); +}); + test("public interrupt settles cancellation and stale cancellation cannot stop the next Turn",{timeout:10000},async t=>{ const {child,events,observations,closed,wait,send,start}=await launch(t); start("first","hold");await wait(()=>events.some(event=>event.type==="input_ready")); diff --git a/packages/claude-sdk-adapter/tests/mcp.test.mjs b/packages/claude-sdk-adapter/tests/mcp.test.mjs index e48b6c940..6b913364e 100644 --- a/packages/claude-sdk-adapter/tests/mcp.test.mjs +++ b/packages/claude-sdk-adapter/tests/mcp.test.mjs @@ -122,3 +122,56 @@ test("batched results use per-call content, never duplicate a whole-message nati const events = o.consume(first, "session"); assert.deepEqual(events.map(e => [e.id, e.observation.output, e.observation.error]), [["a", [], null], ["b", null, "failed"]]); }); + +test("cancellation retains admitted stdio calls across delayed observations and local abort results", async () => { + let observer; + const labels = ["target", "late", "healthy", "http"]; + const declarations = labels.map(server_label => server_label === "http" ? { ...declaration(null), server_label } : {server_label, command:"/.oac/bin/"+server_label, allowed_tools:null}); + const profile = new MCPProfile(declarations, [], (id, name) => observer.admit(id, name)); + const statuses = labels.map(name => ({name, status:"connected", tools:[{name:"hold"}]})); + profile.verify(labels.map(label => `mcp__${label}__hold`), statuses, "session"); + observer = new MCPObserver(profile.identities, new Set(["target", "late", "healthy"])); + const admit = async label => { + const input = {hook_event_name:"PreToolUse", session_id:"session", tool_use_id:label, tool_name:`mcp__${label}__hold`, tool_input:{}}; + assert.deepEqual(await profile.beforeTool(input, label, {signal:new AbortController().signal}), {}); + }; + await admit("healthy"); + observer.consume(assistant("healthy", "mcp__healthy__hold"), "session"); + observer.consume(user("healthy", "finished"), "session"); + await admit("target"); await admit("http"); + observer.cancel(); + await admit("late"); + for (const label of ["target", "late", "http"]) { + observer.consume(assistant(label, `mcp__${label}__hold`), "session"); + observer.consume(user(label, "Interrupted", true), "session"); + } + observer.assertComplete(); + observer.cancel(); + assert.deepEqual(observer.cancelledServers, ["late", "target"]); + profile.verifyReconnected(statuses); + assert.throws(() => profile.verifyReconnected(statuses.map(s => s.name === "target" ? {...s, tools:[{name:"replacement"}]} : s)), /inventory/); +}); + +test("failed local terminals retain only the current Turn's uncertain stdio ownership", () => { + const labels = ["timeout", "server_error", "healthy", "http"]; + const tools = new Map(labels.map(server => [`mcp__${server}__hold`, {server, name:"hold"}])); + const stdio = new Set(["timeout", "server_error", "healthy"]); + const current = new MCPObserver(tools, stdio); + for (const label of labels) { + const name = `mcp__${label}__hold`; + current.admit(label, name); + current.consume(assistant(label, name), "session"); + const failed = label !== "healthy"; + // The pinned CLI uses this same projection for local failures and a plain + // server isError reply, so neither string proves remote completion. + const result = {...user(label, failed ? "failed" : "finished", failed), tool_use_result:failed ? "Error: failed" : "finished"}; + assert.equal(current.consume(result, "session")[0].observation.status, failed ? "failed" : "completed"); + } + current.assertComplete(); + current.cancel(); + assert.deepEqual(current.cancelledServers, ["server_error", "timeout"]); + const successor = new MCPObserver(tools, stdio); + successor.cancel(); + successor.assertComplete(); + assert.deepEqual(successor.cancelledServers, []); +}); diff --git a/packages/claude-sdk-adapter/tests/startup_budget.test.mjs b/packages/claude-sdk-adapter/tests/startup_budget.test.mjs index 77b9c7f31..0259dbd86 100644 --- a/packages/claude-sdk-adapter/tests/startup_budget.test.mjs +++ b/packages/claude-sdk-adapter/tests/startup_budget.test.mjs @@ -105,7 +105,7 @@ test("the SDK accepts delayed initialization on both adapter startup paths", {ti else if(mode!=="required-mcp") {assert.ok(budget.remaining>0);assert.ok(Math.abs(budget.now+budget.remaining-deadline)<20);} if (!["short","expired","cancel"].includes(mode)) assert.deepEqual(observations.filter(event=>event.kind!=="budget").map(event=>event.kind),mode==="executor"?["native_spawned","initialized"]:["native_spawned","initialized","required_status","input"]); - if(mode==="executor")assert.deepEqual(events,[{type:"executor_ready",protocol:3}]); + if(mode==="executor")assert.deepEqual(events,[{type:"executor_ready",protocol:4}]); child.stdin.end(); assert.deepEqual(await closed,{code:0,signal:null},stderr); assert.equal(observations.filter(event=>event.kind==="native_closed").length,mode==="expired"?0:1); diff --git a/packages/codex-runtime/README.md b/packages/codex-runtime/README.md new file mode 100644 index 000000000..e44d974eb --- /dev/null +++ b/packages/codex-runtime/README.md @@ -0,0 +1,19 @@ +# Codex native MCP client invalidation + +The Harness catalog owns the Codex version. [source.json](source.json) identifies its exact upstream source. [invalidate-mcp.patch](invalidate-mcp.patch) adds the local client invalidation operation required by the Runtime–Harness cancelled stdio MCP lifecycle. It leaves Codex's model and tool loop, thread history and ordinary MCP reconciliation in their native owners. + +Run `python3 packages/codex-runtime/check-source.py /path/to/codex` against a clean checkout before applying the patch. The check verifies the source revision, catalog version and patch applicability and prints the patch digest for build provenance. Generated TypeScript and JSON schema exports are not native build inputs; the Rust protocol definitions and request macro compile the added operation directly. + +## Private app-server protocol + +The patched app-server declares `mcpServerInvalidation: true` in its `initialize` response. The Go adapter requires this declaration before preparing stdio MCP bindings; an upstream binary with the same version is insufficient. + +`mcpServer/invalidate` accepts `{ "threadId": "", "serverNames": [""] }`. Names must identify configured stdio servers. The method resolves the existing native spawn subtree, includes all its loaded threads, and invalidates the selected clients under each Session's existing MCP refresh semaphore. The operation is held by native owned tasks if its RPC caller disappears. Unloaded threads own no reusable runtime connection. Other labels and unrelated thread families remain untouched. Callers must first drain native work in the owned family and confirm closure of the selected host process scopes. + +Every captured thread's current configuration is checked before invalidation starts; a descendant may omit a selected binding, but a matching non-stdio binding rejects the operation. The supported owner family is Codex's native thread-spawn subtree. OAC's unattended approval profile does not enable Guardian review, and both upstream Guardian implementations explicitly clear their MCP server configuration; arbitrary internal sessions outside this subtree are not added to this adapter operation. + +Success returns `{ "serverNames": [""] }`, sorted and deduplicated, after selected client shutdown completes. Ready clients have entered the native `ClientState::Closed`; pending startup is cancelled. Existing prepared calls retain their original client and cannot redirect to a replacement. This receipt acknowledges local invalidation, not remote tool completion or replacement readiness. + +After the host `StopMCP` receipt and native invalidation receipt, the adapter calls maintained `config/mcpServer/reload`. Its dirty state applies through the existing native refresh flow before a subsequent Turn uses MCP. Reconciliation replaces closed clients and reuses unchanged healthy connections. No eager `mcpServerStatus/list` discovery, synthetic model Turn, polling delay or whole app-server restart establishes this fence. + +The patch's `targeted_invalidation_*` Rust tests exercise closed-client replacement and unaffected connection identity through native reconciliation using controlled in-process transports. The Go adapter tests exercise cancellation latching, late starts, native call identity, host closure ordering, explicit declaration, receipt failures and Executor reuse. Native compilation and focused Rust tests must run in the qualified source build; source applicability alone does not qualify the binary. diff --git a/packages/codex-runtime/check-source.py b/packages/codex-runtime/check-source.py new file mode 100644 index 000000000..d88e599fd --- /dev/null +++ b/packages/codex-runtime/check-source.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python3 +"""Check the pinned native build input without modifying it or running a model.""" + +import hashlib +import json +import pathlib +import re +import subprocess +import sys + + +def main(): + if len(sys.argv) != 2: + raise SystemExit("usage: check-source.py UPSTREAM_SOURCE_DIRECTORY") + package = pathlib.Path(__file__).resolve().parent + source = pathlib.Path(sys.argv[1]).resolve() + pin = json.loads((package / "source.json").read_text()) + catalog = json.loads((package.parents[1] / "internal/harnessconfig/builtin/catalog.json").read_text()) + version = next(item["version"] for item in catalog if item["kind"] == "codex") + + def git(*args): + return subprocess.check_output(["git", "-C", str(source), *args], text=True).strip() + + if git("rev-parse", "HEAD") != pin["revision"]: + raise SystemExit("Codex source revision does not match source.json") + if git("status", "--porcelain", "--untracked-files=no"): + raise SystemExit("Codex source has tracked modifications") + manifest = (source / "codex-rs/Cargo.toml").read_text() + package_section = re.search(r"(?ms)^\[workspace\.package\]\s*\n(.*?)(?=^\[|\Z)", manifest) + source_version = re.search(r'^version = "([^"]+)"$', package_section[1], re.M) if package_section else None + if source_version is None or source_version[1] != version: + raise SystemExit("Codex source version does not match the Harness catalog") + patch = package / "invalidate-mcp.patch" + subprocess.run(["git", "-C", str(source), "apply", "--check", str(patch)], check=True) + print(json.dumps({"repository": pin["repository"], "revision": pin["revision"], "version": version, + "patch_sha256": hashlib.sha256(patch.read_bytes()).hexdigest()}, sort_keys=True)) + + +if __name__ == "__main__": + main() diff --git a/packages/codex-runtime/invalidate-mcp.patch b/packages/codex-runtime/invalidate-mcp.patch new file mode 100644 index 000000000..c619a816a --- /dev/null +++ b/packages/codex-runtime/invalidate-mcp.patch @@ -0,0 +1,398 @@ +diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs +index 6c00f88..3d064b4 100644 +--- a/codex-rs/app-server-protocol/src/protocol/common.rs ++++ b/codex-rs/app-server-protocol/src/protocol/common.rs +@@ -1149,6 +1149,13 @@ client_request_definitions! { + response: v2::McpServerOauthLoginResponse, + }, + ++ /// Invalidates selected stdio clients in a loaded thread and its agent subtree. ++ McpServerInvalidate => "mcpServer/invalidate" { ++ params: v2::McpServerInvalidateParams, ++ serialization: global("mcp-registry"), ++ response: v2::McpServerInvalidateResponse, ++ }, ++ + McpServerRefresh => "config/mcpServer/reload" { + params: #[ts(type = "undefined")] #[serde(skip_serializing_if = "Option::is_none")] Option<()>, + serialization: global("mcp-registry"), +diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs +index 17013b5..57f3991 100644 +--- a/codex-rs/app-server-protocol/src/protocol/v1.rs ++++ b/codex-rs/app-server-protocol/src/protocol/v1.rs +@@ -68,6 +68,8 @@ pub struct InitializeCapabilities { + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] + #[serde(rename_all = "camelCase")] + pub struct InitializeResponse { ++ /// Supports mcpServer/invalidate for the complete owned agent subtree. ++ pub mcp_server_invalidation: bool, + pub user_agent: String, + /// Absolute path to the server's $CODEX_HOME directory. + pub codex_home: AbsolutePathBuf, +diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs +index 7ab179c..cc4ef6b 100644 +--- a/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs ++++ b/codex-rs/app-server-protocol/src/protocol/v2/mcp.rs +@@ -865,3 +865,21 @@ impl From for McpServerElicitationRequestResponse { + } + } + } ++ ++/// The caller must first settle native work and close the selected server scopes. ++#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] ++#[serde(rename_all = "camelCase")] ++#[ts(export_to = "v2/")] ++pub struct McpServerInvalidateParams { ++ /// Root of the owned agent subtree, including previously loaded descendants. ++ pub thread_id: String, ++ pub server_names: Vec, ++} ++ ++/// Acknowledges local invalidation, not remote tool completion or readiness. ++#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] ++#[serde(rename_all = "camelCase")] ++#[ts(export_to = "v2/")] ++pub struct McpServerInvalidateResponse { ++ pub server_names: Vec, ++} +diff --git a/codex-rs/app-server-transport/src/transport/mod.rs b/codex-rs/app-server-transport/src/transport/mod.rs +index c8b0e57..c6e707d 100644 +--- a/codex-rs/app-server-transport/src/transport/mod.rs ++++ b/codex-rs/app-server-transport/src/transport/mod.rs +@@ -370,6 +370,7 @@ mod tests { + id: RequestId::Integer(7), + result: Box::new(ClientResponsePayload::Initialize( + codex_app_server_protocol::InitializeResponse { ++ mcp_server_invalidation: true, + user_agent: "codex-test-agent".to_string(), + codex_home, + platform_family: "unix".to_string(), +diff --git a/codex-rs/app-server-transport/src/transport/remote_control/segment_tests.rs b/codex-rs/app-server-transport/src/transport/remote_control/segment_tests.rs +index 3b2406b..dd59f41 100644 +--- a/codex-rs/app-server-transport/src/transport/remote_control/segment_tests.rs ++++ b/codex-rs/app-server-transport/src/transport/remote_control/segment_tests.rs +@@ -136,6 +136,7 @@ fn invalid_response_becomes_remote_control_jsonrpc_error() { + message: Box::new(OutgoingMessage::Response(OutgoingResponse { + id: RequestId::Integer(7), + result: Box::new(ClientResponsePayload::Initialize(InitializeResponse { ++ mcp_server_invalidation: true, + user_agent: "codex-test-agent".to_string(), + codex_home, + platform_family: "unix".to_string(), +diff --git a/codex-rs/app-server-transport/src/transport/remote_control/tests.rs b/codex-rs/app-server-transport/src/transport/remote_control/tests.rs +index 0114a0b..222b05a 100644 +--- a/codex-rs/app-server-transport/src/transport/remote_control/tests.rs ++++ b/codex-rs/app-server-transport/src/transport/remote_control/tests.rs +@@ -1623,6 +1623,7 @@ async fn remote_control_http_mode_enrolls_before_connecting() { + result: Box::new( + codex_app_server_protocol::ClientResponsePayload::Initialize( + codex_app_server_protocol::InitializeResponse { ++ mcp_server_invalidation: true, + user_agent: "codex-test-agent".to_string(), + codex_home: codex_home.path().abs(), + platform_family: "test-family".to_string(), +diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs +index 8e78c92..c43d850 100644 +--- a/codex-rs/app-server/src/message_processor.rs ++++ b/codex-rs/app-server/src/message_processor.rs +@@ -1523,6 +1523,9 @@ impl MessageProcessor { + ClientRequest::McpServerOauthLogin { params, .. } => { + self.mcp_processor.mcp_server_oauth_login(params).await + } ++ ClientRequest::McpServerInvalidate { params, .. } => { ++ self.mcp_processor.mcp_server_invalidate(params).await ++ } + ClientRequest::McpServerRefresh { params, .. } => { + self.mcp_processor.mcp_server_refresh(params).await + } +diff --git a/codex-rs/app-server/src/request_processors/initialize_processor.rs b/codex-rs/app-server/src/request_processors/initialize_processor.rs +index 442b71f..3d58c7a 100644 +--- a/codex-rs/app-server/src/request_processors/initialize_processor.rs ++++ b/codex-rs/app-server/src/request_processors/initialize_processor.rs +@@ -140,6 +140,7 @@ impl InitializeRequestProcessor { + + let user_agent = get_codex_user_agent(); + let response = InitializeResponse { ++ mcp_server_invalidation: true, + user_agent, + codex_home, + platform_family: std::env::consts::FAMILY.to_string(), +diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs +index 405a1fe..a98bc1a 100644 +--- a/codex-rs/app-server/src/request_processors/mcp_processor.rs ++++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs +@@ -1,5 +1,7 @@ + use super::thread_input::ensure_direct_input_allowed; + use super::*; ++use codex_app_server_protocol::McpServerInvalidateParams; ++use codex_app_server_protocol::McpServerInvalidateResponse; + use codex_core::McpManager; + use codex_mcp::McpServerSource; + use codex_mcp::ReadResourceRequestParams; +@@ -44,6 +46,82 @@ impl McpRequestProcessor { + .map(|response| Some(response.into())) + } + ++ pub(crate) async fn mcp_server_invalidate( ++ &self, ++ params: McpServerInvalidateParams, ++ ) -> Result, JSONRPCErrorError> { ++ let (root_id, root) = self.load_thread(¶ms.thread_id).await?; ++ let mut names = params.server_names; ++ names.sort(); ++ names.dedup(); ++ if names.is_empty() || names.iter().any(String::is_empty) { ++ return Err(invalid_request("serverNames must contain nonempty names")); ++ } ++ let (config, _) = root.current_mcp_config_and_runtime_context().await; ++ let auth = self.auth_manager.auth().await; ++ let servers = codex_mcp::effective_mcp_servers(&config, auth.as_ref()); ++ for name in &names { ++ if !servers.get(name).is_some_and(|server| { ++ matches!( ++ server.config().transport, ++ McpServerTransportConfig::Stdio { .. } ++ ) ++ }) { ++ return Err(invalid_request(format!("unknown stdio MCP server: {name}"))); ++ } ++ } ++ let manager = Arc::clone(&self.thread_manager); ++ // This task owns the complete operation if the requesting connection disappears. ++ // No eager discovery or Turn is created by invalidation. ++ let task = tokio::spawn(async move { ++ let ids = manager ++ .list_agent_subtree_thread_ids(root_id) ++ .await ++ .map_err(|err| { ++ internal_error(format!("failed to resolve MCP owner subtree: {err}")) ++ })?; ++ let loaded = manager.list_thread_ids().await; ++ let mut threads = vec![root]; ++ for id in ids { ++ if id != root_id && loaded.contains(&id) { ++ threads.push(manager.get_thread(id).await.map_err(|err| { ++ internal_error(format!("MCP owner changed during invalidation: {err}")) ++ })?); ++ } ++ } ++ // Validate the entire captured family before closing any client. ++ // A descendant can omit a binding but cannot change its transport. ++ for thread in &threads { ++ let (config, _) = thread.current_mcp_config_and_runtime_context().await; ++ let servers = codex_mcp::effective_mcp_servers(&config, auth.as_ref()); ++ for name in &names { ++ if servers.get(name).is_some_and(|server| { ++ !matches!( ++ server.config().transport, ++ McpServerTransportConfig::Stdio { .. } ++ ) ++ }) { ++ return Err(invalid_request(format!( ++ "non-stdio MCP binding in owner subtree: {name}" ++ ))); ++ } ++ } ++ } ++ for thread in threads { ++ thread.invalidate_mcp_servers(&names).await.map_err(|err| { ++ internal_error(format!("failed to invalidate MCP clients: {err}")) ++ })?; ++ } ++ Ok::<_, JSONRPCErrorError>(McpServerInvalidateResponse { ++ server_names: names, ++ }) ++ }); ++ let response = task ++ .await ++ .map_err(|err| internal_error(format!("MCP invalidation task failed: {err}")))??; ++ Ok(Some(response.into())) ++ } ++ + pub(crate) async fn mcp_server_refresh( + &self, + params: Option<()>, +diff --git a/codex-rs/app-server/tests/suite/v2/initialize.rs b/codex-rs/app-server/tests/suite/v2/initialize.rs +index 923bb41..2fb1ffc 100644 +--- a/codex-rs/app-server/tests/suite/v2/initialize.rs ++++ b/codex-rs/app-server/tests/suite/v2/initialize.rs +@@ -59,12 +59,14 @@ async fn initialize_uses_client_info_name_as_originator() -> Result<()> { + codex_home: response_codex_home, + platform_family, + platform_os, ++ mcp_server_invalidation, + } = to_response::(response)?; + + assert!(user_agent.starts_with("codex_vscode/")); + assert_eq!(response_codex_home, expected_codex_home); + assert_eq!(platform_family, std::env::consts::FAMILY); + assert_eq!(platform_os, std::env::consts::OS); ++ assert!(mcp_server_invalidation); + Ok(()) + } + +@@ -171,12 +173,14 @@ async fn initialize_respects_originator_override_env_var() -> Result<()> { + codex_home: response_codex_home, + platform_family, + platform_os, ++ mcp_server_invalidation, + } = to_response::(response)?; + + assert!(user_agent.starts_with("codex_originator_via_env_var/")); + assert_eq!(response_codex_home, expected_codex_home); + assert_eq!(platform_family, std::env::consts::FAMILY); + assert_eq!(platform_os, std::env::consts::OS); ++ assert!(mcp_server_invalidation); + Ok(()) + } + +diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs +index f6d0c7e..4cd9fb8 100644 +--- a/codex-rs/codex-mcp/src/connection_manager.rs ++++ b/codex-rs/codex-mcp/src/connection_manager.rs +@@ -853,6 +853,17 @@ impl McpConnectionSet { + view.connection.client.ready_transport().is_some() || view.connection.client().await.is_ok() + } + ++ /// Cancel startup and close selected published clients. Prepared calls retain ++ /// their old client; reconciliation cannot reuse a closed transport. ++ /// Absent names are allowed because descendants can have a smaller catalog. ++ pub(crate) async fn invalidate_servers(&self, server_names: &[String]) { ++ for name in server_names { ++ if let Some(view) = self.servers.get(name) { ++ view.connection.shutdown().await; ++ } ++ } ++ } ++ + /// Stop all MCP clients owned by this manager and terminate stdio server processes. + pub async fn shutdown(&self) { + let connections = self +diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs +index 9c974ac..f1db362 100644 +--- a/codex-rs/codex-mcp/src/connection_manager_tests.rs ++++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs +@@ -5724,3 +5724,55 @@ async fn view_only_changes_reuse_connection_and_preserve_the_old_step() { + ); + assert_eq!(new_call.tool_approval_mode(), AppToolApproval::Approve); + } ++ ++#[tokio::test] ++async fn targeted_invalidation_retains_unaffected_connection_across_reconciliation() ++-> anyhow::Result<()> { ++ let context = reusable_server_runtime_context(); ++ let config = reusable_server_config("http://127.0.0.1:1"); ++ let mut previous = manager_with_reusable_ready_server( ++ &config, ++ &context, ++ vec![create_test_tool("docs", "search")], ++ ) ++ .await; ++ let mut target = manager_with_reusable_ready_server( ++ &config, ++ &context, ++ vec![create_test_tool("blocked", "wait")], ++ ) ++ .await; ++ let target_view = target.servers.remove("docs").expect("target client"); ++ let old_target = target_view.connection.client().await?.client; ++ let old_healthy = previous.servers["docs"].connection.client().await?.client; ++ previous.servers.insert("blocked".to_string(), target_view); ++ ++ previous.invalidate_servers(&["blocked".to_string()]).await; ++ ++ assert!(old_target.is_closed().await); ++ assert!(!old_healthy.is_closed().await); ++ let reconciled = reconcile_reusable_server(&previous, config, context).await; ++ assert!(previous.shares_test_connection_with(&reconciled, "docs")); ++ // A captured handle remains closed; invalidation never routes it to a successor. ++ assert!(old_target.list_tools(None, None).await.is_err()); ++ Ok(()) ++} ++ ++#[tokio::test] ++async fn targeted_invalidation_forces_replacement_without_waiting_for_eof() -> anyhow::Result<()> { ++ let context = reusable_server_runtime_context(); ++ let config = reusable_server_config("http://127.0.0.1:1"); ++ let previous = manager_with_reusable_ready_server( ++ &config, ++ &context, ++ vec![create_test_tool("docs", "search")], ++ ) ++ .await; ++ let old = previous.servers["docs"].connection.client().await?.client; ++ assert!(!old.is_closed().await); ++ previous.invalidate_servers(&["docs".to_string()]).await; ++ assert!(old.is_closed().await); ++ let reconciled = reconcile_reusable_server(&previous, config, context).await; ++ assert!(!previous.shares_test_connection_with(&reconciled, "docs")); ++ Ok(()) ++} +diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs +index dc30070..6c33119 100644 +--- a/codex-rs/codex-mcp/src/runtime.rs ++++ b/codex-rs/codex-mcp/src/runtime.rs +@@ -649,6 +649,13 @@ impl McpRuntime { + cancellation.retained_subscription_cancellation = Some(retained); + } + ++ /// Does not publish a replacement or modify unaffected connection identities. ++ pub async fn invalidate_servers(&self, server_names: &[String]) { ++ self.latest_connections() ++ .invalidate_servers(server_names) ++ .await; ++ } ++ + pub async fn shutdown(&self) { + self.latest_connections().shutdown().await; + } +diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs +index 172726f..803470e 100644 +--- a/codex-rs/core/src/codex_thread.rs ++++ b/codex-rs/core/src/codex_thread.rs +@@ -780,6 +780,11 @@ impl CodexThread { + self.session.get_config().await + } + ++ /// Invalidates only the selected clients without changing native history. ++ pub async fn invalidate_mcp_servers(&self, server_names: &[String]) -> anyhow::Result<()> { ++ self.session.invalidate_mcp_servers(server_names).await ++ } ++ + /// Observes this thread's published MCP connections that match the requested config. + pub async fn mcp_connection_statuses( + &self, +diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs +index 46af2d3..9753960 100644 +--- a/codex-rs/core/src/session/mcp.rs ++++ b/codex-rs/core/src/session/mcp.rs +@@ -89,6 +89,29 @@ impl ElicitationReviewer for GuardianMcpElicitationReviewer { + } + + impl Session { ++ pub(crate) async fn invalidate_mcp_servers( ++ self: &Arc, ++ server_names: &[String], ++ ) -> anyhow::Result<()> { ++ let session = Arc::clone(self); ++ let names = server_names.to_vec(); ++ // The owned task retains the refresh gate until old clients are invalidated. ++ tokio::spawn(async move { ++ let _refresh = session ++ .mcp_refresh ++ .acquire() ++ .await ++ .map_err(|_| anyhow::anyhow!("MCP runtime refresh semaphore closed"))?; ++ session ++ .services ++ .mcp_runtime ++ .invalidate_servers(&names) ++ .await; ++ Ok::<_, anyhow::Error>(()) ++ }) ++ .await? ++ } ++ + pub(crate) async fn runtime_mcp_config(&self, config: &Config) -> McpConfig { + self.runtime_mcp_config_and_context(config).await.0 + } diff --git a/packages/codex-runtime/source.json b/packages/codex-runtime/source.json new file mode 100644 index 000000000..676973d0f --- /dev/null +++ b/packages/codex-runtime/source.json @@ -0,0 +1,4 @@ +{ + "repository": "https://github.com/openai/codex", + "revision": "3d2ee51ca2d5db578f328aa75e20aa22c0197c9a" +} diff --git a/packages/mcode-harness/README.md b/packages/mcode-harness/README.md index 1f34ca32e..cc14f1dba 100644 --- a/packages/mcode-harness/README.md +++ b/packages/mcode-harness/README.md @@ -26,6 +26,8 @@ This standalone companion uses its own npm lock and is excluded from the root pn The bridge owns each launcher until exit. MCP cancellation and transport shutdown stop all owned workers before releasing the bridge. The outer Runtime owns the native process group. Both boundaries require real Docker cancellation tests. +For declared stdio MCP calls, the adapter captures the affected server identities before sending cancellation and retains identities from callbacks received while cancellation drains. A local failure remains unconfirmed within its Turn even if native reports the tool as finished. The native tool wrapper sets the private `details.oac_response_received` field only for a result returned by the MCP SDK, including a server's `isError` reply; the adapter validates the result identity before releasing that call's owner. It waits for the Runtime to close every selected server's process scope, including background descendants, then calls the private `oac/session/mcp/disconnect` ACP extension with the Session ID and those server names. The native owner disconnects only the selected Session connections and waits for their old transports' actual close events. Configurations remain installed for lazy reconnection on a later call; other MCP servers and the workspace bridge keep their connections. HTTP servers, unknown names and `oac_workspace` are rejected by this control operation. ACP initialization advertises `oac/mcp-lifecycle` version 2, which the adapter requires for declared stdio MCP. The request uses the existing native Session, MCP service and connection pool; it adds no model or tool execution loop. + The daemon sets the protected `protected-mcp-v1` tool policy independently of the concurrency limit. The native catalog applies it to root and child profiles, withholding direct native filesystem and process tools. The Session-private `oac_workspace` MCP server supplies the authorized workspace tools to workers. Other MCP servers keep their native selection rules. The same authored policy filters native prompt capabilities, so the ACP root and child templates name builtin workspace tools only when they are available; MCP tool names and schemas come from their declarations. Native Explore and Verifier profiles keep their stricter native capability ceiling. ACP initialization reports the applied policy and admission limit; enabled Subagents reject an unpatched CLI before accepting model input. Native tool schemas are retained. Text and image results use standard MCP content; video results are rejected. The pinned native CLI may add task and Skill utility tools, so qualification must inspect the actual inventory rather than assume exactly six. @@ -34,6 +36,8 @@ Native tool schemas are retained. Text and image results use standard MCP conten `make check-mcode-harness` runs the package's Node tests and syntax checks. The native prompt regression requires `MCODE_SOURCE` pointing to the pinned, patched native source with upstream dependencies installed; the companion build always runs it. It checks ordinary native guidance, disabled local tools and protected root and child capabilities without a model. Qualify changes with the [Harness acceptance checklist](../../contracts/agents-api/harness-onboarding.md#qualify-the-adapter); synthetic probes and native model runs do not complete public Files/Artifacts or independent Core acceptance. +With the same `MCODE_SOURCE`, `native-mcp-lifecycle.test.mjs` exercises the patched native Session configurations, connection pool and MCP SDK against controlled transports. It checks exact Session/server selection, delayed close events, connection attempts interrupted during initialization, and later lazy reconnection without closing unaffected services. + For the packaged Linux regression, provide an operator-owned private profile and artifact directory, then run `native.test.mjs` inside the qualified Docker Runtime: ```sh diff --git a/packages/mcode-harness/native-mcp-lifecycle.test.mjs b/packages/mcode-harness/native-mcp-lifecycle.test.mjs new file mode 100644 index 000000000..1cb644e82 --- /dev/null +++ b/packages/mcode-harness/native-mcp-lifecycle.test.mjs @@ -0,0 +1,141 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { join } from 'node:path'; +import { homedir } from 'node:os'; +import { pathToFileURL } from 'node:url'; + +const root = process.env.MCODE_SOURCE; +test('native MCP shutdown fences the old scoped transport and reconnects lazily', { skip: !root }, async t => { + const require = createRequire(join(root, 'package.json')); + const { build } = require('esbuild'); + const paths = JSON.parse(readFileSync(join(root, 'tsconfig.standalone.json'))).compilerOptions.paths; + // Keep the real native pool, Session configurations and SDK protocol. Only + // the transport is controlled, so close() can return before its close event. + const fixture = ` + export const transports = []; + export function createTransport(config) { + const previous = transports.filter(t => t.name === config.command).length; + const transport = { + name: config.command, closeRequested: false, + async start() {}, + async close() { this.closeRequested = true; }, + finishClose() { this.onclose?.(); }, + async send(message) { + if (!('id' in message)) return; + if (message.method === 'tools/call' && message.params.name === 'hold') return; + if (message.method === 'initialize' && this.name === 'connecting' && previous === 0) return; + const result = message.method === 'initialize' + ? { protocolVersion: '2025-11-25', capabilities: { tools: {} }, serverInfo: { name: this.name, version: '1' } } + : message.method === 'tools/list' + ? { tools: ['read', 'hold', 'server_error'].map(name => ({ name, inputSchema: { type: 'object' } })) } + : { content: [{ type: 'text', text: this.name }], + ...(message.params.name === 'server_error' ? { isError: true } : {}), + _meta: { oac_response_received: false }, details: { oac_response_received: false } }; + queueMicrotask(() => this.onmessage?.({ jsonrpc: '2.0', id: message.id, result })); + } + }; + transports.push(transport); + return transport; + } + `; + const result = await build({ + stdin: { contents: `export { McpConnectionPool } from '@mavis/mcp/runtime/connection-pool'; + export { SessionMcpServers } from ${JSON.stringify(join(root, 'packages/local-runtime-v2/src/service/mcp/runtime/session-servers.ts'))}; + export { LocalMcpService } from ${JSON.stringify(join(root, 'packages/local-runtime-v2/src/service/mcp/runtime/local-mcp.service.ts'))}; + export { transports } from 'lifecycle-transport';`, resolveDir: root }, + bundle: true, write: false, platform: 'node', format: 'esm', target: 'node22', + tsconfig: join(root, 'tsconfig.standalone.json'), nodePaths: [join(root, 'node_modules')], + banner: { js: `import { createRequire as lifecycleCreateRequire } from 'node:module'; const require = lifecycleCreateRequire(${JSON.stringify(join(root, 'package.json'))});` }, + plugins: [{ name: 'native-lifecycle-fixture', setup(builder) { + builder.onResolve({ filter: /^(lifecycle-transport|\.\/transport\/factory\.js)$/ }, () => ({ path: 'transport', namespace: 'fixture' })); + builder.onLoad({ filter: /.*/, namespace: 'fixture' }, () => ({ contents: fixture, loader: 'js' })); + // Omit unrelated barrel registrations; the selected native exports stay real. + builder.onResolve({ filter: /^@/ }, ({ path }) => paths[path] ? { path: join(root, paths[path][0]), sideEffects: false } : undefined); + } }], + }); + const artifacts = join(homedir(), '.oac', 'tests'); + mkdirSync(artifacts, { recursive: true }); + const directory = mkdtempSync(join(artifacts, 'mcode-lifecycle-')); + t.after(() => rmSync(directory, { recursive: true, force: true })); + const bundle = join(directory, 'lifecycle.mjs'); + writeFileSync(bundle, result.outputFiles[0].text); + const { McpConnectionPool, SessionMcpServers, LocalMcpService, transports } = await import(pathToFileURL(bundle).href); + const pool = new McpConnectionPool({ getResolvedServer() { throw new Error('Session overrides were lost'); } }); + const sessions = new SessionMcpServers(pool); + const service = new LocalMcpService(() => directory, { connectionPool: pool }); + const stdio = name => ({ name, config: { type: 'stdio', command: name, args: [] } }); + await sessions.configure('root', [stdio('target'), stdio('untouched'), stdio('oac_workspace'), stdio('connecting'), + { name: 'remote', config: { type: 'http', url: 'https://example.test/mcp' } }]); + await sessions.configure('other-session', [stdio('target')]); + const overrides = (session, name) => sessions.overrides(session, name, sessions.get(session)[name]); + try { + for (const [session, name] of [['root', 'target'], ['root', 'untouched'], ['root', 'oac_workspace'], ['other-session', 'target']]) + await pool.listTools(name, overrides(session, name)); + const original = [...transports]; + for (const name of ['unknown', 'remote', 'oac_workspace']) + await assert.rejects(sessions.disconnect('root', [name]), /Only declared Session stdio/); + assert.ok(transports.every(t => !t.closeRequested)); + // The whole selected server closes: both active requests sharing it fail. + const calls = [pool.callTool('target', 'hold', {}, overrides('root', 'target')), + pool.callTool('target', 'hold', {}, overrides('root', 'target'))]; + const failed = calls.map(call => assert.rejects(call, /Connection closed/)); + await new Promise(resolve => setImmediate(resolve)); + let settled = false; + const stop = sessions.disconnect('root', ['target']).then(() => { settled = true; }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(settled, false, 'SDK close return must not acknowledge transport closure'); + assert.equal(original[0].closeRequested, true); + assert.ok(original.slice(1).every(t => !t.closeRequested), 'another server or Session was closed'); + original[0].finishClose(); + await Promise.all([stop, ...failed]); + assert.equal(transports.length, 4, 'disconnect must not eagerly reconnect'); + await pool.callTool('target', 'read', {}, overrides('root', 'target')); + assert.equal(transports.length, 5, 'next call must use a fresh transport'); + for (const [session, name] of [['root', 'untouched'], ['root', 'oac_workspace'], ['other-session', 'target']]) + await pool.callTool(name, 'read', {}, overrides(session, name)); + assert.equal(transports.length, 5, 'unaffected connections must remain reusable'); + + const connecting = pool.listTools('connecting', overrides('root', 'connecting')); + const rejected = assert.rejects(connecting, /Failed to connect|disconnected/); + await new Promise(resolve => setImmediate(resolve)); + const pendingTransport = transports.at(-1); + let pendingSettled = false; + const pendingStop = sessions.disconnect('root', ['connecting']).then(() => { pendingSettled = true; }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(pendingSettled, false, 'invalidated connection attempts must also await close'); + pendingTransport.finishClose(); + await Promise.all([pendingStop, rejected]); + await pool.listTools('connecting', overrides('root', 'connecting')); + assert.notEqual(transports.at(-1), pendingTransport); + + // Exercise the real service call -> callLive -> pool -> SDK -> tool wrapper. + // The service turns a timeout into an MCP-shaped error, so only the wrapper's + // native provenance can distinguish it from a server's isError reply. + await service.configureSessionServers('root', [{ name: 'response', config: { + type: 'stdio', command: 'response', args: [], timeout: 20, + } }]); + const context = { sessionId: 'root', workspaceRoot: directory }; + const native = await service.listNativeTools(context); + const tools = service.runtimeToolsFromNative(native, context); + for (const [name, received, isError] of [['read', true, false], ['server_error', true, true], ['hold', false, true]]) { + const entry = native.find(entry => entry.toolName === name); + const tool = tools.find(tool => tool.def.name === entry.nativeName); + const result = await tool.impl.execute({}, {}); + assert.equal(result.details.oac_response_received, received, name); + assert.equal(result.details.mcp.isError, isError, name); + assert.equal(result.isError, isError, 'native tool error projection changed'); + assert.equal(result.details.server, 'response'); + assert.equal(result.details.tool, name); + if (received) + assert.equal(result.details.mcp._meta.oac_response_received, false, 'server metadata was changed'); + else + assert.match(result.details.mcp.content[0].text, /timed out/i); + } + } finally { + for (const transport of transports) transport.finishClose(); + await service.close(); + await pool.shutdown(); + } +}); diff --git a/packages/mcode-harness/patch-native.mjs b/packages/mcode-harness/patch-native.mjs index c0ddc1be2..9442cfac0 100644 --- a/packages/mcode-harness/patch-native.mjs +++ b/packages/mcode-harness/patch-native.mjs @@ -79,6 +79,134 @@ const projectGate = ' if (!context?.workspaceRoot || !context.sessionId) retu if (project.split(projectGate).length !== 2) throw new Error('Pinned native project MCP source changed'); writeFileSync(projectPath, project.replace(projectGate, " if (process.env.OAC_RUNTIME_MCODE_TOOL_POLICY === 'protected-mcp-v1' || !context?.workspaceRoot || !context.sessionId) return {};")); copyFileSync(join(here, 'native-subagent-admission.mjs'), join(root, 'packages/local-runtime/src/background-task/oac-subagent-admission.mjs')); +// Preserve the native owner and its connection keys. This control request is +// called only after the Runtime has closed the selected sandbox process scopes. +function replaceNative(file, before, after) { + const path = join(root, file); + const source = readFileSync(path, 'utf8'); + if (source.split(before).length !== 2) throw new Error('Pinned native MCP lifecycle source changed: ' + file); + writeFileSync(path, source.replace(before, after)); +} +const poolFile = 'packages/agent-modules/mcp/src/runtime/connection-pool.ts'; +replaceNative(poolFile, ' private connectionServerNames = new Map();', + ` private connectionServerNames = new Map(); + private transportClosures = new Map>>();`); +replaceNative(poolFile, ' const client = new Client(', ` // SDK close() may return after sending SIGKILL, before the child closes. + // Retain each old transport until its actual close event, including connects + // invalidated before they could enter the pool. + const closures = this.transportClosures.get(connectionKey) ?? new Set>(); + this.transportClosures.set(connectionKey, closures); + const closed = new Promise((resolve) => { + const previous = transport.onclose; + transport.onclose = () => { try { previous?.(); } finally { resolve(); } }; + }); + closures.add(closed); + void closed.then(() => { + closures.delete(closed); + if (closures.size === 0 && this.transportClosures.get(connectionKey) === closures) + this.transportClosures.delete(connectionKey); + }); + const client = new Client(`); +replaceNative(poolFile, ' async reconnect(\n', ` async disconnectAndWait(serverName: string, overrides: McpConnectionTokenOverrides): Promise { + if (!overrides.connectionKey || overrides.serverOverride?.transport.type !== 'stdio') + throw new Error('A scoped stdio MCP connection is required.'); + const key = this.resolveConnectionKey(serverName, overrides); + const closed = [...(this.transportClosures.get(key) ?? [])]; + await this.disconnect(serverName, overrides); + await Promise.all(closed); + } + + async reconnect( +`); +const sessionServersFile = 'packages/local-runtime-v2/src/service/mcp/runtime/session-servers.ts'; +replaceNative(sessionServersFile, ' async remove(sessionId: string): Promise {', ` async disconnect(sessionId: string, names: readonly string[]): Promise { + const id = requireSessionId(sessionId); + const current = this.servers.get(id); + if (names.length === 0 || new Set(names).size !== names.length || !this.pool) + throw new Error('Invalid MCP disconnect request.'); + const selected = names.map((name) => { + const config = current?.[name]; + if (name === 'oac_workspace' || !config || config.type !== 'stdio') + throw new Error('Only declared Session stdio MCP servers can be disconnected.'); + return { name, overrides: this.overrides(id, name, config)! }; + }); + for (const { name, overrides } of selected) + await this.pool.disconnectAndWait(name, overrides); + } + + async remove(sessionId: string): Promise {`); +replaceNative('packages/local-runtime-v2/src/service/mcp/runtime/local-mcp.service.ts', + ' async clearSessionServers(sessionId: string): Promise {', + ` disconnectSessionServers(sessionId: string, names: readonly string[]): Promise { + this.assertOpen(); + return this.enqueueMutation(() => this.sessionServers.disconnect(sessionId, names)); + } + + async clearSessionServers(sessionId: string): Promise {`); +const mcpServiceFile = 'packages/local-runtime-v2/src/service/mcp/runtime/local-mcp.service.ts'; +// callLive preserves local failures as MCP-shaped error results. Remember only +// SDK replies without changing those results or exposing a server-spoofable bit. +replaceNative(mcpServiceFile, ' private closed = false;', + ' private closed = false;\n private receivedResponses = new WeakSet();'); +replaceNative(mcpServiceFile, ' this.recordPublicRuntimeSuccess(server, context, config);\n return result;', + ' this.receivedResponses.add(result);\n this.recordPublicRuntimeSuccess(server, context, config);\n return result;'); +replaceNative(mcpServiceFile, ' details: { mcp: result, server: tool.server, tool: tool.toolName },', + ' details: { mcp: result, server: tool.server, tool: tool.toolName, oac_response_received: this.receivedResponses.has(result) },'); +const facadeFile = 'packages/local-runtime-v2/src/service/mcp/tools/public-facade.ts'; +replaceNative(facadeFile, " | 'clearSessionServers'", " | 'clearSessionServers'\n | 'disconnectSessionServers'"); +replaceNative(facadeFile, ' clearSessionServers(sessionId: string): Promise {', + ` disconnectSessionServers(input: { sessionId: string; servers: readonly string[] }): Promise { + return this.owner.disconnectSessionServers(input.sessionId, input.servers); + } + + clearSessionServers(sessionId: string): Promise {`); +replaceNative('packages/local-runtime-v2/src/application/session/process-local-application-contract.ts', + ' | "clearSessionServers"', ' | "clearSessionServers"\n | "disconnectSessionServers"'); +replaceNative('packages/local-runtime-v2/src/local/cli-service.ts', + ' configureSessionMcpServers(\n', + ` disconnectSessionMcpServers(sessionId: string, servers: readonly string[]): Promise { + return this.requireCapability("mcp", "MCP").disconnectSessionServers({ sessionId, servers }); + } + + configureSessionMcpServers( +`); +replaceNative('packages/tui/src/runtime/port.ts', + ' clearSessionMcpServers(sessionId: string): Promise;', + ' clearSessionMcpServers(sessionId: string): Promise;\n disconnectSessionMcpServers(sessionId: string, servers: readonly string[]): Promise;'); +for (const [file, owner] of [ + ['packages/tui/src/runtime/adapter.ts', 'sessionAccess'], + ['packages/tui/src/runtime/adapters/session-access.ts', 'cliService'], +]) { + replaceNative(file, ' clearSessionMcpServers(sessionId: string): Promise {', + ` disconnectSessionMcpServers(sessionId: string, servers: readonly string[]): Promise { + return this.${owner}.disconnectSessionMcpServers(sessionId, servers); + } + + clearSessionMcpServers(sessionId: string): Promise {`); +} +replaceNative('packages/tui/src/acp/agent.ts', " 'oac/subagents': {", " 'oac/mcp-lifecycle': { version: 2 },\n 'oac/subagents': {"); +replaceNative('packages/tui/src/acp/agent.ts', + ' app.onNotification(acp.methods.agent.session.cancel, async ({ params }) => {', + ` app.onRequest('oac/session/mcp/disconnect', (value: unknown) => { + const request = value as { sessionId?: unknown; servers?: unknown } | null; + if (!request || typeof request.sessionId !== 'string' || !request.sessionId || + !Array.isArray(request.servers) || request.servers.length === 0 || + request.servers.some((name: unknown) => typeof name !== 'string' || !name || name.trim() !== name)) + throw acp.RequestError.invalidParams(undefined, 'Invalid MCP disconnect request.'); + return { sessionId: request.sessionId, servers: request.servers as string[] }; + }, async ({ params }) => { + await runSessionMcpMutation(params.sessionId, async (signal) => { + assertLifecycleActive(signal); + const active = requireAttachedSession(sessions, params.sessionId); + if (active.activePrompt) + throw acp.RequestError.invalidParams(undefined, 'MCP disconnect requires a settled prompt.'); + await options.runtime.disconnectSessionMcpServers(params.sessionId, params.servers); + assertLifecycleActive(signal); + }); + return {}; + }); + + app.onNotification(acp.methods.agent.session.cancel, async ({ params }) => {`); const digest = name => createHash('sha256').update(readFileSync(join(here, name))).digest('hex'); writeFileSync(join(root, '.oac-native-patch.json'), JSON.stringify({ revision: pin.revision, files: { 'patch-native.mjs':digest('patch-native.mjs'), 'native-subagent-admission.mjs':digest('native-subagent-admission.mjs') } }, null, 2)+'\n'); diff --git a/scripts/build-codex-harness.sh b/scripts/build-codex-harness.sh new file mode 100755 index 000000000..ea3b8a906 --- /dev/null +++ b/scripts/build-codex-harness.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +package="$repo_root/packages/codex-runtime" +source="${CODEX_NATIVE_SOURCE:?Set CODEX_NATIVE_SOURCE to the pinned upstream checkout}" +root="${OAC_DEV_HOME:-$HOME/.oac}/build" +destination="${CODEX_HARNESS_BUILD_DIR:-$root/codex-harness-$(date +%Y%m%d%H%M%S)}" +for directory in "$source" "$root" "$destination"; do + [[ "$directory" == /* ]] || { printf 'Absolute build directories are required\n' >&2; exit 1; } +done +[[ "$(uname -s):$(uname -m)" == Linux:x86_64 ]] || { printf 'Build Codex on Linux x86_64\n' >&2; exit 1; } +[[ ! -e "$destination" ]] || { printf 'Codex artifact destination already exists\n' >&2; exit 1; } +mkdir -p "$root" "$(dirname "$destination")" +context="$(mktemp -d "$root/codex-build.XXXXXX")" +trap 'rm -rf "$context"' EXIT +python3 "$package/check-source.py" "$source" > "$context/source.json" +revision="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["revision"])' "$context/source.json")" +mkdir "$context/upstream" +git -C "$source" archive "$revision" | tar -x -C "$context/upstream" +git -C "$context/upstream" apply "$package/invalidate-mcp.patch" +target=x86_64-unknown-linux-musl +export CODEX_REPO_ROOT="$context/upstream" +# musl-gcc specs can inject an interpreter into Rust's static PIE. Keep the +# self-contained startup code authoritative instead of loading it twice. +export RUSTFLAGS="${RUSTFLAGS:-} -C link-arg=-Wl,--no-dynamic-linker" +( + cd "$context/upstream/codex-rs" + # The release tag updates workspace versions but leaves local lock entries at + # 0.0.0. Resolve that metadata before compiling; external dependencies stay pinned. + cargo fetch --target "$target" + python3 - "$source/codex-rs/Cargo.lock" Cargo.lock <<'PY' +import sys, tomllib +before, after = [tomllib.load(open(path, "rb")) for path in sys.argv[1:]] +def external(lock): + return [entry for entry in lock["package"] if "source" in entry] +assert external(before) == external(after), "Codex external dependency lock changed" +def local(lock): + return {entry["name"]: {k: v for k, v in entry.items() if k != "version"} + for entry in lock["package"] if "source" not in entry} +assert local(before) == local(after), "Codex workspace dependencies changed" +PY + # Match the CLI release feature union, including its vendored OpenSSL. + cargo test --locked --target "$target" --release -p codex-mcp -p codex-cli --lib targeted_invalidation_ + cargo test --locked --target "$target" --release -p codex-mcp -p codex-cli --lib prepared_call_does_not_reroute_after_captured_connection_closes + cargo build --locked --target "$target" --release --bin bwrap + output="${CARGO_TARGET_DIR:-$PWD/target}/$target/release" + strip --strip-debug --strip-unneeded "$output/bwrap" + export CODEX_BWRAP_SHA256="$(sha256sum "$output/bwrap" | cut -d' ' -f1)" + python3 "$context/upstream/scripts/build_codex_package.py" \ + --target "$target" --cargo-profile release --package-dir "$context/canonical" \ + --bwrap-bin "$output/bwrap" +) +mkdir "$context/artifact" +cp "$context/canonical/bin/codex" "$context/canonical/bin/codex-code-mode-host" "$context/artifact/" +strip --strip-debug --strip-unneeded "$context/artifact/codex" "$context/artifact/codex-code-mode-host" +python3 - "$context" <<'PY' +import hashlib, json, pathlib, subprocess, sys +context = pathlib.Path(sys.argv[1]) +artifact = context / "artifact" +pin = json.loads((context / "source.json").read_text()) +for name in ("codex", "codex-code-mode-host"): + executable = str(artifact / name) + headers = subprocess.check_output(["readelf", "-lW", executable], text=True) + dynamic = subprocess.check_output(["readelf", "-dW", executable], text=True) + assert not any(line.split()[0] == "INTERP" for line in headers.splitlines() if line.split()), "Codex payload requires an ELF interpreter: " + name + assert "(NEEDED)" not in dynamic, "Codex payload requires a shared library: " + name +assert subprocess.check_output([str(artifact / "codex"), "--version"], text=True).strip() == "codex-cli " + pin["version"] +pin["target"] = "x86_64-unknown-linux-musl" +pin["cargo_lock_sha256"] = hashlib.sha256((context / "upstream/codex-rs/Cargo.lock").read_bytes()).hexdigest() +pin["files"] = {p.name: hashlib.sha256(p.read_bytes()).hexdigest() for p in sorted(artifact.iterdir())} +(artifact / "provenance.json").write_text(json.dumps(pin, indent=2) + "\n") +PY +mv "$context/artifact" "$destination" +printf 'Codex Runtime artifact: %s\n' "$destination" diff --git a/scripts/build-codex-runtime.sh b/scripts/build-codex-runtime.sh index f0237229a..cc55553d9 100755 --- a/scripts/build-codex-runtime.sh +++ b/scripts/build-codex-runtime.sh @@ -4,30 +4,37 @@ set -euo pipefail repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" runtime_root="${OAC_DEV_HOME:-$HOME/.oac}" output_dir="${AGENTS_RUNTIME_BUILD_DIR:-$runtime_root/build/codex-runtime}" -# Extract the official pinned @openai/codex Linux x64 npm package here. -package_dir="${CODEX_CLI_DIR:?Set CODEX_CLI_DIR to the extracted pinned platform package}" +package_dir="${CODEX_HARNESS_BUILD_DIR:?Set CODEX_HARNESS_BUILD_DIR to the source-built pinned Codex artifact}" for directory in "$runtime_root" "$output_dir" "$package_dir"; do if [[ "$directory" != /* ]]; then printf 'Runtime build directories must be absolute: %s\n' "$directory" >&2 exit 1 fi done -python3 - "$package_dir/package.json" "$repo_root/internal/harnessconfig/builtin/catalog.json" <<'PY' -import json, sys -package = json.load(open(sys.argv[1])) -version = next(entry['version'] for entry in json.load(open(sys.argv[2])) if entry['kind'] == 'codex') -assert package['name'] == '@openai/codex' and package['version'] == version + '-linux-x64', 'Expected pinned official Linux x64 package' +python3 - "$package_dir" "$repo_root" <<'PY' +import hashlib, json, pathlib, sys +artifact, repository = map(pathlib.Path, sys.argv[1:]) +package = repository / "packages/codex-runtime" +pin = json.loads((package / "source.json").read_text()) +provenance = json.loads((artifact / "provenance.json").read_text()) +catalog = json.loads((repository / "internal/harnessconfig/builtin/catalog.json").read_text()) +version = next(entry["version"] for entry in catalog if entry["kind"] == "codex") +expected = {**pin, "version": version, "target": "x86_64-unknown-linux-musl", + "patch_sha256": hashlib.sha256((package / "invalidate-mcp.patch").read_bytes()).hexdigest()} +assert all(provenance.get(key) == value for key, value in expected.items()), "Codex artifact does not match the pinned patched source" +assert set(provenance["files"]) == {"codex", "codex-code-mode-host"}, "Incomplete Codex artifact" +for name, digest in provenance["files"].items(): + assert hashlib.sha256((artifact / name).read_bytes()).hexdigest() == digest, "Codex artifact checksum mismatch: " + name PY -native_dir="$package_dir/vendor/x86_64-unknown-linux-musl" -for executable in "$native_dir/bin/codex" "$native_dir/bin/codex-code-mode-host"; do +for executable in "$package_dir/codex" "$package_dir/codex-code-mode-host"; do test -x "$executable" || { printf 'Missing executable: %s\n' "$executable" >&2; exit 1; } done mkdir -p "$runtime_root/cache/oac-runtime-builds" context="$(mktemp -d "$runtime_root/cache/oac-runtime-builds/codex.XXXXXX")" trap 'rm -rf "$context"' EXIT -cp "$native_dir/bin/codex" "$native_dir/bin/codex-code-mode-host" "$context/" -cp "$package_dir/package.json" "$context/" +cp "$package_dir/codex" "$package_dir/codex-code-mode-host" "$package_dir/provenance.json" "$context/" # Preserve the previous payload if validation failed. mkdir -p "$output_dir" +rm -f "$output_dir/package.json" cp -R "$context/." "$output_dir/" printf 'Codex Harness payload: %s\n' "$output_dir" diff --git a/scripts/build-core-distribution.sh b/scripts/build-core-distribution.sh index ea6b1229c..1a8ba4c29 100755 --- a/scripts/build-core-distribution.sh +++ b/scripts/build-core-distribution.sh @@ -146,7 +146,7 @@ build_image web "$stage/web" CGO_ENABLED=0 go build -mod=readonly -trimpath -o "$stage/" \ ./apps/daemon/cmd/oac-daemon ./apps/daemon/cmd/oac-process-shim ./apps/sandboxio/cmd/oac-sandbox-io -: "${CODEX_CLI_DIR:?Set the extracted pinned Codex Linux x64 package directory}" +: "${CODEX_HARNESS_BUILD_DIR:?Set the existing source-built pinned Codex artifact directory}" : "${MCODE_HARNESS_BUILD_DIR:?Set the existing built pinned MiniMax Code companion directory}" export CLAUDE_SDK_BUILD_DIR="$stage/claude-sdk" scripts/build-claude-sdk-runtime.sh diff --git a/scripts/build-mcode-harness.sh b/scripts/build-mcode-harness.sh index b9b99e1ab..de1ca5cd4 100644 --- a/scripts/build-mcode-harness.sh +++ b/scripts/build-mcode-harness.sh @@ -47,7 +47,7 @@ test "$(node "$native/cli.js" --version)" = "$version" MCODE_SOURCE="$context/upstream" node patch-native.mjs cd upstream corepack pnpm install --frozen-lockfile - MCODE_SOURCE="$context/upstream" node --test "$context/native-prompt.test.mjs" + MCODE_SOURCE="$context/upstream" node --test "$context/native-prompt.test.mjs" "$context/native-mcp-lifecycle.test.mjs" node scripts/build.mjs ) ( diff --git a/scripts/check-claude-sdk-runtime.mjs b/scripts/check-claude-sdk-runtime.mjs index 16e079858..cfa4acbbd 100644 --- a/scripts/check-claude-sdk-runtime.mjs +++ b/scripts/check-claude-sdk-runtime.mjs @@ -22,7 +22,7 @@ const probe = spawnSync(process.execPath, [join(root, "dist/runtime_check.js"), assert.equal(probe.status, 0, "Exported runtime is unavailable"); const report = JSON.parse(probe.stdout); assert.equal(report.type, "runtime_ready"); -assert.equal(report.protocol, 3); +assert.equal(report.protocol, 4); assert.deepEqual(report.features, ["executor_reuse", ...(["linux", "darwin", "win32"].includes(process.platform) ? ["local_runtime_v2", "workspace_functions", "workspace_structured_output", "workspace_tool_search", "workspace_mcp_http"] : []), "message_images", "function_result_images", "tool_search", "structured_output", "subagent_resources", "mcp_http_tools", "mcp_http_bearer_auth", "mcp_http_required", "workspace_tools", "workspace_prepare", "workspace_command_observations"]); assert.equal(report.sdk, source.dependencies["@anthropic-ai/claude-agent-sdk"]); assert.equal(report.mcp, source.dependencies["@modelcontextprotocol/sdk"]); diff --git a/scripts/ci_plan.py b/scripts/ci_plan.py index 5288fcb58..fa0607df4 100644 --- a/scripts/ci_plan.py +++ b/scripts/ci_plan.py @@ -68,6 +68,7 @@ (("contracts/",), (*GO, ".json", ".yaml", ".yml"), ("backend", "api", "native", "web", "web-acceptance", "example", "distribution")), (("packages/agents-client/",), (*GO, *WEB), ("backend", "api", "web", "web-acceptance", "example")), (("packages/claude-sdk-adapter/", "packages/mcode-harness/"), WEB, ("harness", "native", "backend", "distribution")), + (("packages/codex-runtime/",), (".py", ".json", ".patch"), ("harness", "native", "backend", "distribution")), (("packages/tsconfig/",), (".json",), ("harness", "native")), (("deploy/node/", "scripts/acceptance/"), (".py", ".json", ".sh"), ("distribution",)), (("deploy/compose/",), (".yaml", ".toml", ".json"), ("distribution", "compose")), @@ -81,7 +82,7 @@ (("scripts/build-e2b-provider.sh",), (".sh",), ("backend", "api", "distribution")), (("scripts/build-claude", "scripts/check-claude", "scripts/build-mcode", "scripts/prepare-release-runtimes.sh"), SCRIPTS, ("harness", "native", "backend", "distribution")), - (("scripts/build-codex-runtime.sh",), (".sh",), ("backend", "native", "distribution")), + (("scripts/build-codex-",), (".sh",), ("harness", "backend", "native", "distribution")), (("scripts/build-agent-host-images.sh", "scripts/qualify-agent-host.sh"), (".sh",), ("distribution",)), (("scripts/generate-harness-catalog", "scripts/harness-catalog/", "scripts/openapi-split/", "scripts/patch-agents-openapi.py", "scripts/generate-public-api"), SCRIPTS, JOBS), diff --git a/scripts/core-distribution-manifest.test.py b/scripts/core-distribution-manifest.test.py index 4cb143fc5..a6ab58165 100644 --- a/scripts/core-distribution-manifest.test.py +++ b/scripts/core-distribution-manifest.test.py @@ -141,7 +141,7 @@ def test_mcode_payload_rejects_stale_companion_at_the_same_version(self): self.assertEqual((output / "mcode-harness" / name).read_bytes(), current) (companion / name).write_bytes(current) - def test_codex_payload_uses_catalog_pin_and_preserves_package_identity(self): + def test_codex_payload_requires_current_source_patch_and_binary_hashes(self): repository = self.stage / "repository" scripts = repository / "scripts" scripts.mkdir(parents=True) @@ -150,29 +150,49 @@ def test_codex_payload_uses_catalog_pin_and_preserves_package_identity(self): catalog = repository / "internal/harnessconfig/builtin/catalog.json" catalog.parent.mkdir(parents=True) catalog.write_text(json.dumps([{"kind": "codex", "version": "9.8.7"}])) + source = repository / "packages/codex-runtime" + source.mkdir(parents=True) + pin = {"repository": "https://github.com/openai/codex", "revision": "a" * 40} + (source / "source.json").write_text(json.dumps(pin)) + (source / "invalidate-mcp.patch").write_text("current patch") package = self.stage / "package" - native = package / "vendor/x86_64-unknown-linux-musl/bin" - native.mkdir(parents=True) + package.mkdir() + files = {} for executable in ("codex", "codex-code-mode-host"): - (native / executable).write_text("#!/bin/sh\nexit 0\n") - (native / executable).chmod(0o755) - manifest = {"name": "@openai/codex", "version": "9.8.7-linux-x64"} - (package / "package.json").write_text(json.dumps(manifest)) + path = package / executable + path.write_text("#!/bin/sh\nexit 0\n") + path.chmod(0o755) + files[executable] = hashlib.sha256(path.read_bytes()).hexdigest() + manifest = {**pin, "version": "9.8.7", "target": "x86_64-unknown-linux-musl", + "patch_sha256": hashlib.sha256(b"current patch").hexdigest(), "files": files} + (package / "provenance.json").write_text(json.dumps(manifest)) output = self.stage / "payload" + output.mkdir() + (output / "package.json").write_text('{"name":"@openai/codex"}') + (output / "unrelated").write_text("preserved") environment = {**os.environ, "OAC_DEV_HOME": str(self.stage / "dev"), - "CODEX_CLI_DIR": str(package), "AGENTS_RUNTIME_BUILD_DIR": str(output)} + "CODEX_HARNESS_BUILD_DIR": str(package), "AGENTS_RUNTIME_BUILD_DIR": str(output)} command = ["bash", str(scripts / builder.name)] result = subprocess.run(command, env=environment, capture_output=True, text=True) self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(json.loads((output / "package.json").read_text()), manifest) - for invalid in ({**manifest, "version": "9.8.6-linux-x64"}, - {**manifest, "name": "unofficial-codex"}): - with self.subTest(package=invalid): - (package / "package.json").write_text(json.dumps(invalid)) + self.assertFalse((output / "package.json").exists()) + self.assertEqual((output / "unrelated").read_text(), "preserved") + self.assertEqual(json.loads((output / "provenance.json").read_text()), manifest) + for key, value in (("version", "9.8.6"), ("revision", "b" * 40), + ("patch_sha256", "0" * 64), ("target", "aarch64-unknown-linux-musl")): + with self.subTest(field=key): + (package / "provenance.json").write_text(json.dumps({**manifest, key: value})) result = subprocess.run(command, env=environment, capture_output=True, text=True) self.assertNotEqual(result.returncode, 0) - self.assertIn("Expected pinned official Linux x64 package", result.stderr) - self.assertEqual(json.loads((output / "package.json").read_text()), manifest) + self.assertIn("does not match the pinned patched source", result.stderr) + self.assertEqual(json.loads((output / "provenance.json").read_text()), manifest) + (package / "provenance.json").write_text(json.dumps(manifest)) + original = (package / "codex").read_bytes() + (package / "codex").write_bytes(original + b"changed") + result = subprocess.run(command, env=environment, capture_output=True, text=True) + self.assertNotEqual(result.returncode, 0) + self.assertIn("Codex artifact checksum mismatch: codex", result.stderr) + self.assertEqual((output / "codex").read_bytes(), original) def setUp(self): self.temporary = tempfile.TemporaryDirectory() diff --git a/scripts/prepare-release-runtimes.sh b/scripts/prepare-release-runtimes.sh index 8659df5e6..b140fa72f 100644 --- a/scripts/prepare-release-runtimes.sh +++ b/scripts/prepare-release-runtimes.sh @@ -4,16 +4,31 @@ set -euo pipefail # Prepare pinned upstream inputs once, then reuse the existing Runtime builders. repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" catalog="$repo_root/internal/harnessconfig/builtin/catalog.json" -codex_version="$(python3 -c 'import json,sys; print(next(entry["version"] for entry in json.load(open(sys.argv[1])) if entry["kind"] == "codex"))' "$catalog")" release_root="$HOME/.oac/build/release-inputs" if [[ -e "$release_root" ]]; then printf 'Release input directory already exists; use a fresh build host\n' >&2 exit 1 fi -mkdir -p "$release_root/codex" "$release_root/mcode-native" -cd "$release_root/codex" -npm pack --ignore-scripts --silent "@openai/codex@$codex_version-linux-x64" > package-name.txt -tar -xzf "$(cat package-name.txt)" +mkdir -p "$release_root/mcode-native" +pin="$repo_root/packages/codex-runtime/source.json" +source_repository="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["repository"])' "$pin")" +source_revision="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["revision"])' "$pin")" +git init --quiet "$release_root/codex-source" +git -C "$release_root/codex-source" remote add origin "$source_repository" +git -C "$release_root/codex-source" fetch --depth 1 origin "$source_revision" +git -C "$release_root/codex-source" checkout --detach FETCH_HEAD +( +cd "$release_root/codex-source/codex-rs" +# rustup reads the toolchain and components from the pinned upstream checkout. +rustup show active-toolchain +rustup target add x86_64-unknown-linux-musl +TARGET=x86_64-unknown-linux-musl GITHUB_ENV="$release_root/codex-build.env" \ + bash "$release_root/codex-source/.github/scripts/install-musl-build-tools.sh" +while IFS= read -r assignment; do export "$assignment"; done < "$release_root/codex-build.env" +export AWS_LC_SYS_NO_JITTER_ENTROPY=1 AWS_LC_SYS_NO_JITTER_ENTROPY_x86_64_unknown_linux_musl=1 +CODEX_NATIVE_SOURCE="$release_root/codex-source" CODEX_HARNESS_BUILD_DIR="$release_root/codex" \ + bash "$repo_root/scripts/build-codex-harness.sh" | tee "$release_root/codex-build.log" +) pin="$repo_root/packages/mcode-harness/source.json" source_repository="$(python3 -c 'import json,sys; print(json.load(open(sys.argv[1]))["repository"])' "$pin")" @@ -34,7 +49,7 @@ python3 - "$release_root" "$companion" <<'PY' import json, pathlib, sys root = pathlib.Path(sys.argv[1]) (root / "inputs.json").write_text(json.dumps({ - "codex": str(root / "codex/package"), + "codex": str(root / "codex"), "mcode": sys.argv[2], }) + "\n") PY