fix(mcp): clean process group before reaping leader; redact clipped HTTP bodies; mark clipped fields

commit: 1fb1dbf210cf6ce9cece687603cff330193a82be
author: brice · 7d ago · 2026-10-15 14:06 UTC
parent: ef9ee0c
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

10 files · +525 -26

M packages/agent/mcp/http.go +28 -11


@@ -144,8 +144,11 @@ func (t httpTransport) post(ctx context.Context, frame []byte) (http.Response,
144	144		}
145	145		if resp.StatusCode >= 300 {
146	146		defer resp.Body.Close()
147		-	snippet, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBody))
148		-	return nil, t.statusFailure(req, resp.StatusCode, snippet)
	147	+	// Read past the display limit by the longest secret's length, so a
	148	+	// secret straddling the cut is seen whole and can be redacted.
	149	+	limit := maxErrorBody + t.red.overlap()
	150	+	snippet, _ := io.ReadAll(io.LimitReader(resp.Body, int64(limit)))
	151	+	return nil, t.statusFailure(req, resp.StatusCode, snippet, len(snippet) >= limit)
149	152		}
150	153		return resp, nil
151	154		}

@@ -154,8 +157,8 @@ func (t httpTransport) post(ctx context.Context, frame []byte) (http.Response,
154	157		// request that carried a session id means the server forgot the session
155	158		// (expired or restarted): the transport becomes unusable so the manager
156	159		// reconnects with a fresh initialize. The failed call is not replayed.
157		-	func (t httpTransport) statusFailure(req http.Request, status int, body []byte) error {
158		-	err := t.red.statusError(status, body)
	160	+	func (t httpTransport) statusFailure(req http.Request, status int, body []byte, truncated bool) error {
	161	+	err := t.red.statusErrorBody(status, body, truncated)
159	162		if status == http.StatusNotFound && req.Header.Get(headerSessionID) != "" {
160	163		err = fmt.Errorf("mcp session expired: %w", err)
161	164		t.mu.Lock()

@@ -187,17 +190,31 @@ func httpStatusError(status int, body []byte) error {
187	190		return (*redactor)(nil).statusError(status, body)
188	191		}
189	192
190		-	// statusError builds an HTTPStatusError whose body is redacted, stripped of
191		-	// terminal control sequences, and shortened for display.
	193	+	// statusError builds an HTTPStatusError from a complete body.
192	194		func (r *redactor) statusError(status int, body []byte) error {
193		-	// Redact before truncating so a secret cut at the limit is not half shown.
194		-	text := strings.TrimSpace(r.scrubOneLine(string(body)))
195		-	if len(text) > 300 {
196		-	cut := 300
	195	+	return r.statusErrorBody(status, body, false)
	196	+	}
	197	+
	198	+	// maxStatusBodyDisplay is how much of an error body is shown.
	199	+	const maxStatusBodyDisplay = 300
	200	+
	201	+	// statusErrorBody builds an HTTPStatusError whose body is redacted, stripped of
	202	+	// terminal control sequences, and shortened for display. truncated says the
	203	+	// body was cut by the reader at an arbitrary byte, so it may end inside a
	204	+	// secret; whatever fragment of one remains at the very end is removed. The
	205	+	// final display cut gets the same protection.
	206	+	func (r *redactor) statusErrorBody(status int, body []byte, truncated bool) error {
	207	+	text := r.scrubOneLine(string(body))
	208	+	if truncated {
	209	+	text = r.trimPartialSecret(text)
	210	+	}
	211	+	text = strings.TrimSpace(text)
	212	+	if len(text) > maxStatusBodyDisplay {
	213	+	cut := maxStatusBodyDisplay
197	214		for cut > 0 && !utf8.RuneStart(text[cut]) {
198	215		cut--
199	216		}
200		-	text = text[:cut] + "…"
	217	+	text = r.trimPartialSecret(text[:cut]) + "…"
201	218		}
202	219		return &HTTPStatusError{Status: status, Body: text}
203	220		}


@@ -144,8 +144,11 @@ func (t httpTransport) post(ctx context.Context, frame []byte) (http.Response,
144		}	144		}
145		if resp.StatusCode >= 300 {	145		if resp.StatusCode >= 300 {
146		defer resp.Body.Close()	146		defer resp.Body.Close()
147	-	snippet, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBody))	147	+	// Read past the display limit by the longest secret's length, so a
148	-	return nil, t.statusFailure(req, resp.StatusCode, snippet)	148	+	// secret straddling the cut is seen whole and can be redacted.
			149	+	limit := maxErrorBody + t.red.overlap()
			150	+	snippet, _ := io.ReadAll(io.LimitReader(resp.Body, int64(limit)))
			151	+	return nil, t.statusFailure(req, resp.StatusCode, snippet, len(snippet) >= limit)
149		}	152		}
150		return resp, nil	153		return resp, nil
151		}	154		}

@@ -154,8 +157,8 @@ func (t httpTransport) post(ctx context.Context, frame []byte) (http.Response,
154		// request that carried a session id means the server forgot the session	157		// request that carried a session id means the server forgot the session
155		// (expired or restarted): the transport becomes unusable so the manager	158		// (expired or restarted): the transport becomes unusable so the manager
156		// reconnects with a fresh initialize. The failed call is not replayed.	159		// reconnects with a fresh initialize. The failed call is not replayed.
157	-	func (t httpTransport) statusFailure(req http.Request, status int, body []byte) error {	160	+	func (t httpTransport) statusFailure(req http.Request, status int, body []byte, truncated bool) error {
158	-	err := t.red.statusError(status, body)	161	+	err := t.red.statusErrorBody(status, body, truncated)
159		if status == http.StatusNotFound && req.Header.Get(headerSessionID) != "" {	162		if status == http.StatusNotFound && req.Header.Get(headerSessionID) != "" {
160		err = fmt.Errorf("mcp session expired: %w", err)	163		err = fmt.Errorf("mcp session expired: %w", err)
161		t.mu.Lock()	164		t.mu.Lock()

@@ -187,17 +190,31 @@ func httpStatusError(status int, body []byte) error {
187		return (*redactor)(nil).statusError(status, body)	190		return (*redactor)(nil).statusError(status, body)
188		}	191		}
189			192
190	-	// statusError builds an HTTPStatusError whose body is redacted, stripped of	193	+	// statusError builds an HTTPStatusError from a complete body.
191	-	// terminal control sequences, and shortened for display.
192		func (r *redactor) statusError(status int, body []byte) error {	194		func (r *redactor) statusError(status int, body []byte) error {
193	-	// Redact before truncating so a secret cut at the limit is not half shown.	195	+	return r.statusErrorBody(status, body, false)
194	-	text := strings.TrimSpace(r.scrubOneLine(string(body)))	196	+	}
195	-	if len(text) > 300 {	197	+
196	-	cut := 300	198	+	// maxStatusBodyDisplay is how much of an error body is shown.
			199	+	const maxStatusBodyDisplay = 300
			200	+
			201	+	// statusErrorBody builds an HTTPStatusError whose body is redacted, stripped of
			202	+	// terminal control sequences, and shortened for display. truncated says the
			203	+	// body was cut by the reader at an arbitrary byte, so it may end inside a
			204	+	// secret; whatever fragment of one remains at the very end is removed. The
			205	+	// final display cut gets the same protection.
			206	+	func (r *redactor) statusErrorBody(status int, body []byte, truncated bool) error {
			207	+	text := r.scrubOneLine(string(body))
			208	+	if truncated {
			209	+	text = r.trimPartialSecret(text)
			210	+	}
			211	+	text = strings.TrimSpace(text)
			212	+	if len(text) > maxStatusBodyDisplay {
			213	+	cut := maxStatusBodyDisplay
197		for cut > 0 && !utf8.RuneStart(text[cut]) {	214		for cut > 0 && !utf8.RuneStart(text[cut]) {
198		cut--	215		cut--
199		}	216		}
200	-	text = text[:cut] + "…"	217	+	text = r.trimPartialSecret(text[:cut]) + "…"
201		}	218		}
202		return &HTTPStatusError{Status: status, Body: text}	219		return &HTTPStatusError{Status: status, Body: text}
203		}	220		}

M packages/agent/mcp/proc_unix.go +51 -10


@@ -19,17 +19,33 @@ func isolateProcess(cmd *exec.Cmd) {
19	19
20	20		// processGuard owns the process group of one stdio server.
21	21		//
22		-	// A numeric process-group id is only safe to signal while the group still has
23		-	// members: once the last member is gone the kernel may hand the number to an
24		-	// unrelated process. The guard therefore (a) cleans the group exactly once,
25		-	// (b) forgets the id as soon as the group is observed empty or cleaned, and
26		-	// (c) is cleaned as soon as the direct child is reaped, so no stale id stays
27		-	// signalable for the life of the transport.
	22	+	// A process-group id may be reused by an unrelated process as soon as no
	23	+	// process uses the number any more. A group stays in use while ANY member
	24	+	// exists, and that includes a zombie: the leader's pid (which is the pgid) is
	25	+	// not freed until the leader is reaped. So the guard keeps the leader
	26	+	// unreaped, as a zombie, until it has cleaned the group, wherever the platform
	27	+	// can wait for a child without reaping it (waitLeaderExited; Linux). Only then
	28	+	// is the leader reaped by cmd.Wait.
	29	+	//
	30	+	// Where that is not available (darwin, the BSDs) cleanup runs after the leader
	31	+	// was reaped; the id is then only guaranteed to be ours while other group
	32	+	// members live, which is the best that can be done and is documented as a
	33	+	// residual race. In every case the guard
	34	+	//
	35	+	// (a) cleans the group exactly once,
	36	+	// (b) forgets the id as soon as the group is observed empty or cleaned,
	37	+	// (c) is cleaned as soon as the direct child exits, so no stale id stays
	38	+	// signalable for the life of the transport.
28	39		type processGuard struct {
29		-	mu sync.Mutex
30		-	pgid int // 0 once cleaned up or observed gone
31		-	proc *os.Process
32		-	kill func(pid int, sig syscall.Signal) error // syscall.Kill; replaced in tests
	40	+	mu sync.Mutex
	41	+	pgid int // 0 once cleaned up or observed gone
	42	+	leader int // pid of the group leader (== original pgid)
	43	+	proc *os.Process
	44	+	kill func(pid int, sig syscall.Signal) error // syscall.Kill; replaced in tests
	45	+	// zombie is set once the leader has exited but not been reaped. A probe
	46	+	// with signal 0 cannot tell an empty group from one whose only member is
	47	+	// that zombie, so liveness is then read from the process table.
	48	+	zombie bool
33	49
34	50		once sync.Once
35	51		}

@@ -39,11 +55,28 @@ func newProcessGuard(cmd exec.Cmd) (processGuard, error) {
39	55		g := &processGuard{kill: func(pid int, sig syscall.Signal) error { return syscall.Kill(pid, sig) }}
40	56		if cmd != nil && cmd.Process != nil {
41	57		g.pgid = cmd.Process.Pid
	58	+	g.leader = cmd.Process.Pid
42	59		g.proc = cmd.Process
43	60		}
44	61		return g, nil
45	62		}
46	63
	64	+	// awaitLeaderExit blocks until the group leader has exited, without reaping
	65	+	// it. It reports false when the platform cannot do that, in which case it
	66	+	// returns immediately and the caller must Wait first.
	67	+	func (g *processGuard) awaitLeaderExit() bool {
	68	+	if g == nil \|\| g.leader == 0 {
	69	+	return false
	70	+	}
	71	+	if !waitLeaderExited(g.leader) {
	72	+	return false
	73	+	}
	74	+	g.mu.Lock()
	75	+	g.zombie = true
	76	+	g.mu.Unlock()
	77	+	return true
	78	+	}
	79	+
47	80		// terminate signals the whole group, including descendants that outlive the
48	81		// direct child. It does nothing once the group is cleaned up.
49	82		func (g *processGuard) terminate(force bool) {

@@ -81,6 +114,14 @@ func (g *processGuard) alive() bool {
81	114		if g.pgid == 0 {
82	115		return false
83	116		}
	117	+	if g.zombie {
	118	+	if live, known := liveGroupMembers(g.pgid); known {
	119	+	if !live {
	120	+	g.pgid = 0
	121	+	}
	122	+	return live
	123	+	}
	124	+	}
84	125		err := g.kill(-g.pgid, 0)
85	126		if errors.Is(err, syscall.ESRCH) {
86	127		g.pgid = 0


@@ -19,17 +19,33 @@ func isolateProcess(cmd *exec.Cmd) {
19			19
20		// processGuard owns the process group of one stdio server.	20		// processGuard owns the process group of one stdio server.
21		//	21		//
22	-	// A numeric process-group id is only safe to signal while the group still has	22	+	// A process-group id may be reused by an unrelated process as soon as no
23	-	// members: once the last member is gone the kernel may hand the number to an	23	+	// process uses the number any more. A group stays in use while ANY member
24	-	// unrelated process. The guard therefore (a) cleans the group exactly once,	24	+	// exists, and that includes a zombie: the leader's pid (which is the pgid) is
25	-	// (b) forgets the id as soon as the group is observed empty or cleaned, and	25	+	// not freed until the leader is reaped. So the guard keeps the leader
26	-	// (c) is cleaned as soon as the direct child is reaped, so no stale id stays	26	+	// unreaped, as a zombie, until it has cleaned the group, wherever the platform
27	-	// signalable for the life of the transport.	27	+	// can wait for a child without reaping it (waitLeaderExited; Linux). Only then
			28	+	// is the leader reaped by cmd.Wait.
			29	+	//
			30	+	// Where that is not available (darwin, the BSDs) cleanup runs after the leader
			31	+	// was reaped; the id is then only guaranteed to be ours while other group
			32	+	// members live, which is the best that can be done and is documented as a
			33	+	// residual race. In every case the guard
			34	+	//
			35	+	// (a) cleans the group exactly once,
			36	+	// (b) forgets the id as soon as the group is observed empty or cleaned,
			37	+	// (c) is cleaned as soon as the direct child exits, so no stale id stays
			38	+	// signalable for the life of the transport.
28		type processGuard struct {	39		type processGuard struct {
29	-	mu sync.Mutex	40	+	mu sync.Mutex
30	-	pgid int // 0 once cleaned up or observed gone	41	+	pgid int // 0 once cleaned up or observed gone
31	-	proc *os.Process	42	+	leader int // pid of the group leader (== original pgid)
32	-	kill func(pid int, sig syscall.Signal) error // syscall.Kill; replaced in tests	43	+	proc *os.Process
			44	+	kill func(pid int, sig syscall.Signal) error // syscall.Kill; replaced in tests
			45	+	// zombie is set once the leader has exited but not been reaped. A probe
			46	+	// with signal 0 cannot tell an empty group from one whose only member is
			47	+	// that zombie, so liveness is then read from the process table.
			48	+	zombie bool
33			49
34		once sync.Once	50		once sync.Once
35		}	51		}

@@ -39,11 +55,28 @@ func newProcessGuard(cmd exec.Cmd) (processGuard, error) {
39		g := &processGuard{kill: func(pid int, sig syscall.Signal) error { return syscall.Kill(pid, sig) }}	55		g := &processGuard{kill: func(pid int, sig syscall.Signal) error { return syscall.Kill(pid, sig) }}
40		if cmd != nil && cmd.Process != nil {	56		if cmd != nil && cmd.Process != nil {
41		g.pgid = cmd.Process.Pid	57		g.pgid = cmd.Process.Pid
			58	+	g.leader = cmd.Process.Pid
42		g.proc = cmd.Process	59		g.proc = cmd.Process
43		}	60		}
44		return g, nil	61		return g, nil
45		}	62		}
46			63
			64	+	// awaitLeaderExit blocks until the group leader has exited, without reaping
			65	+	// it. It reports false when the platform cannot do that, in which case it
			66	+	// returns immediately and the caller must Wait first.
			67	+	func (g *processGuard) awaitLeaderExit() bool {
			68	+	if g == nil \|\| g.leader == 0 {
			69	+	return false
			70	+	}
			71	+	if !waitLeaderExited(g.leader) {
			72	+	return false
			73	+	}
			74	+	g.mu.Lock()
			75	+	g.zombie = true
			76	+	g.mu.Unlock()
			77	+	return true
			78	+	}
			79	+
47		// terminate signals the whole group, including descendants that outlive the	80		// terminate signals the whole group, including descendants that outlive the
48		// direct child. It does nothing once the group is cleaned up.	81		// direct child. It does nothing once the group is cleaned up.
49		func (g *processGuard) terminate(force bool) {	82		func (g *processGuard) terminate(force bool) {

@@ -81,6 +114,14 @@ func (g *processGuard) alive() bool {
81		if g.pgid == 0 {	114		if g.pgid == 0 {
82		return false	115		return false
83		}	116		}
			117	+	if g.zombie {
			118	+	if live, known := liveGroupMembers(g.pgid); known {
			119	+	if !live {
			120	+	g.pgid = 0
			121	+	}
			122	+	return live
			123	+	}
			124	+	}
84		err := g.kill(-g.pgid, 0)	125		err := g.kill(-g.pgid, 0)
85		if errors.Is(err, syscall.ESRCH) {	126		if errors.Is(err, syscall.ESRCH) {
86		g.pgid = 0	127		g.pgid = 0

A packages/agent/mcp/proc_wait_linux.go +71 -0

@@ -0,0 +1,71 @@
	1	+	//go:build linux
	2	+
	3	+	package mcp
	4	+
	5	+	import (
	6	+	"errors"
	7	+	"os"
	8	+	"strconv"
	9	+	"strings"
	10	+
	11	+	"golang.org/x/sys/unix"
	12	+	)
	13	+
	14	+	// waitLeaderExited blocks until pid (our direct child) has exited, leaving it
	15	+	// unreaped (a zombie) so its pid, and with it the process-group id, cannot be
	16	+	// reused. It reports false if the wait failed.
	17	+	func waitLeaderExited(pid int) bool {
	18	+	var info unix.Siginfo
	19	+	for {
	20	+	err := unix.Waitid(unix.P_PID, pid, &info, unix.WEXITED\|unix.WNOWAIT, nil)
	21	+	if errors.Is(err, unix.EINTR) {
	22	+	continue
	23	+	}
	24	+	return err == nil
	25	+	}
	26	+	}
	27	+
	28	+	// liveGroupMembers reports whether any non-zombie process is in process group
	29	+	// pgid, reading /proc. known is false when /proc cannot be read.
	30	+	func liveGroupMembers(pgid int) (live, known bool) {
	31	+	entries, err := os.ReadDir("/proc")
	32	+	if err != nil {
	33	+	return false, false
	34	+	}
	35	+	sawProc := false
	36	+	for _, e := range entries {
	37	+	if _, err := strconv.Atoi(e.Name()); err != nil {
	38	+	continue
	39	+	}
	40	+	sawProc = true
	41	+	raw, err := os.ReadFile("/proc/" + e.Name() + "/stat")
	42	+	if err != nil {
	43	+	continue // exited meanwhile
	44	+	}
	45	+	state, group, ok := parseProcStat(string(raw))
	46	+	if !ok \|\| group != pgid \|\| state == 'Z' \|\| state == 'X' {
	47	+	continue
	48	+	}
	49	+	return true, true
	50	+	}
	51	+	return false, sawProc
	52	+	}
	53	+
	54	+	// parseProcStat extracts the state and process-group fields of a
	55	+	// /proc/<pid>/stat line. The command name is parenthesised and may itself
	56	+	// contain spaces and parentheses, so fields are counted after the last ')'.
	57	+	func parseProcStat(stat string) (state byte, pgrp int, ok bool) {
	58	+	i := strings.LastIndexByte(stat, ')')
	59	+	if i < 0 {
	60	+	return 0, 0, false
	61	+	}
	62	+	fields := strings.Fields(stat[i+1:]) // state ppid pgrp ...
	63	+	if len(fields) < 3 \|\| len(fields[0]) != 1 {
	64	+	return 0, 0, false
	65	+	}
	66	+	pg, err := strconv.Atoi(fields[2])
	67	+	if err != nil {
	68	+	return 0, 0, false
	69	+	}
	70	+	return fields[0][0], pg, true
	71	+	}

@@ -0,0 +1,71 @@
	1	+	//go:build linux
	2	+
	3	+	package mcp
	4	+
	5	+	import (
	6	+	"errors"
	7	+	"os"
	8	+	"strconv"
	9	+	"strings"
	10	+
	11	+	"golang.org/x/sys/unix"
	12	+	)
	13	+
	14	+	// waitLeaderExited blocks until pid (our direct child) has exited, leaving it
	15	+	// unreaped (a zombie) so its pid, and with it the process-group id, cannot be
	16	+	// reused. It reports false if the wait failed.
	17	+	func waitLeaderExited(pid int) bool {
	18	+	var info unix.Siginfo
	19	+	for {
	20	+	err := unix.Waitid(unix.P_PID, pid, &info, unix.WEXITED\|unix.WNOWAIT, nil)
	21	+	if errors.Is(err, unix.EINTR) {
	22	+	continue
	23	+	}
	24	+	return err == nil
	25	+	}
	26	+	}
	27	+
	28	+	// liveGroupMembers reports whether any non-zombie process is in process group
	29	+	// pgid, reading /proc. known is false when /proc cannot be read.
	30	+	func liveGroupMembers(pgid int) (live, known bool) {
	31	+	entries, err := os.ReadDir("/proc")
	32	+	if err != nil {
	33	+	return false, false
	34	+	}
	35	+	sawProc := false
	36	+	for _, e := range entries {
	37	+	if _, err := strconv.Atoi(e.Name()); err != nil {
	38	+	continue
	39	+	}
	40	+	sawProc = true
	41	+	raw, err := os.ReadFile("/proc/" + e.Name() + "/stat")
	42	+	if err != nil {
	43	+	continue // exited meanwhile
	44	+	}
	45	+	state, group, ok := parseProcStat(string(raw))
	46	+	if !ok \|\| group != pgid \|\| state == 'Z' \|\| state == 'X' {
	47	+	continue
	48	+	}
	49	+	return true, true
	50	+	}
	51	+	return false, sawProc
	52	+	}
	53	+
	54	+	// parseProcStat extracts the state and process-group fields of a
	55	+	// /proc/<pid>/stat line. The command name is parenthesised and may itself
	56	+	// contain spaces and parentheses, so fields are counted after the last ')'.
	57	+	func parseProcStat(stat string) (state byte, pgrp int, ok bool) {
	58	+	i := strings.LastIndexByte(stat, ')')
	59	+	if i < 0 {
	60	+	return 0, 0, false
	61	+	}
	62	+	fields := strings.Fields(stat[i+1:]) // state ppid pgrp ...
	63	+	if len(fields) < 3 \|\| len(fields[0]) != 1 {
	64	+	return 0, 0, false
	65	+	}
	66	+	pg, err := strconv.Atoi(fields[2])
	67	+	if err != nil {
	68	+	return 0, 0, false
	69	+	}
	70	+	return fields[0][0], pg, true
	71	+	}

A packages/agent/mcp/proc_wait_linux_test.go +111 -0

@@ -0,0 +1,111 @@
	1	+	//go:build linux
	2	+
	3	+	package mcp
	4	+
	5	+	import (
	6	+	"os"
	7	+	"strconv"
	8	+	"testing"
	9	+	"time"
	10	+	)
	11	+
	12	+	func after5s() <-chan time.Time { return time.After(5 * time.Second) }
	13	+
	14	+	func TestParseProcStat(t *testing.T) {
	15	+	cases := []struct {
	16	+	name, in string
	17	+	state byte
	18	+	pgrp int
	19	+	ok bool
	20	+	}{
	21	+	{"plain", "123 (bash) S 1 123 123 0 -1 4194560", 'S', 123, true},
	22	+	{"zombie", "5 (fake-server) Z 1 77 77 0 -1 4", 'Z', 77, true},
	23	+	{"spaces and parens", "9 (a) b (c)) R 2 42 42 0", 'R', 42, true},
	24	+	{"truncated", "9 (x) S 1", 0, 0, false},
	25	+	{"no comm", "garbage", 0, 0, false},
	26	+	}
	27	+	for _, c := range cases {
	28	+	state, pg, ok := parseProcStat(c.in)
	29	+	if state != c.state \|\| pg != c.pgrp \|\| ok != c.ok {
	30	+	t.Errorf("%s: got (%c,%d,%v), want (%c,%d,%v)", c.name, state, pg, ok, c.state, c.pgrp, c.ok)
	31	+	}
	32	+	}
	33	+	}
	34	+
	35	+	func TestLiveGroupMembersSeesSelfOnly(t *testing.T) {
	36	+	// Our own process group has live members (us); a made-up one does not.
	37	+	live, known := liveGroupMembers(os.Getpid())
	38	+	if !known {
	39	+	t.Skip("/proc not readable")
	40	+	}
	41	+	_ = live // our pgid is usually not our pid; only the empty case is stable
	42	+	if live, known := liveGroupMembers(1 << 30); !known \|\| live {
	43	+	t.Errorf("unused group reported live=%v known=%v", live, known)
	44	+	}
	45	+	}
	46	+
	47	+	// ---- GO-020: group cleanup happens while the leader is still a zombie ----
	48	+
	49	+	func TestStdioGroupCleanedBeforeLeaderIsReaped(t *testing.T) {
	50	+	c, _ := connectFake(t, nil)
	51	+	st := c.t.(*stdioTransport)
	52	+	leader := st.cmd.Process.Pid
	53	+
	54	+	childPID, err := strconv.Atoi(callText(t, c, "spawn", `{"mode":"sleeper-stubborn"}`))
	55	+	if err != nil {
	56	+	t.Fatal(err)
	57	+	}
	58	+	t.Cleanup(func() { killPID(childPID) })
	59	+	if !processAlive(childPID) {
	60	+	t.Fatal("descendant not running")
	61	+	}
	62	+
	63	+	type observation struct {
	64	+	state byte
	65	+	childAlive bool
	66	+	pgidKept int
	67	+	}
	68	+	obs := make(chan observation, 1)
	69	+	hook := func(pid int) {
	70	+	if pid != leader {
	71	+	return
	72	+	}
	73	+	var o observation
	74	+	if raw, err := os.ReadFile("/proc/" + strconv.Itoa(leader) + "/stat"); err == nil {
	75	+	o.state, _, _ = parseProcStat(string(raw))
	76	+	}
	77	+	o.childAlive = processAlive(childPID)
	78	+	st.guard.mu.Lock()
	79	+	o.pgidKept = st.guard.pgid
	80	+	st.guard.mu.Unlock()
	81	+	select {
	82	+	case obs <- o:
	83	+	default:
	84	+	}
	85	+	}
	86	+	beforeReapHook.Store(&hook)
	87	+	t.Cleanup(func() { beforeReapHook.Store(nil) })
	88	+
	89	+	// The server exits on its own.
	90	+	_, _ = c.CallTool(t.Context(), "exit", []byte(`{}`))
	91	+	var o observation
	92	+	select {
	93	+	case o = <-obs:
	94	+	case <-after5s():
	95	+	t.Fatal("cleanup hook never ran: the leader was reaped before group cleanup")
	96	+	}
	97	+	if o.state != 'Z' {
	98	+	t.Errorf("leader state at cleanup = %q, want Z (exited but not reaped, so its pid/pgid cannot be reused)", o.state)
	99	+	}
	100	+	if o.childAlive {
	101	+	t.Error("descendant still alive when the leader was about to be reaped")
	102	+	}
	103	+	if o.pgidKept != 0 {
	104	+	t.Errorf("process-group id %d retained after cleanup", o.pgidKept)
	105	+	}
	106	+	// And the leader is reaped afterwards.
	107	+	eventually(t, "leader to be reaped", func() bool {
	108	+	_, err := os.Stat("/proc/" + strconv.Itoa(leader))
	109	+	return os.IsNotExist(err)
	110	+	})
	111	+	}

@@ -0,0 +1,111 @@
	1	+	//go:build linux
	2	+
	3	+	package mcp
	4	+
	5	+	import (
	6	+	"os"
	7	+	"strconv"
	8	+	"testing"
	9	+	"time"
	10	+	)
	11	+
	12	+	func after5s() <-chan time.Time { return time.After(5 * time.Second) }
	13	+
	14	+	func TestParseProcStat(t *testing.T) {
	15	+	cases := []struct {
	16	+	name, in string
	17	+	state byte
	18	+	pgrp int
	19	+	ok bool
	20	+	}{
	21	+	{"plain", "123 (bash) S 1 123 123 0 -1 4194560", 'S', 123, true},
	22	+	{"zombie", "5 (fake-server) Z 1 77 77 0 -1 4", 'Z', 77, true},
	23	+	{"spaces and parens", "9 (a) b (c)) R 2 42 42 0", 'R', 42, true},
	24	+	{"truncated", "9 (x) S 1", 0, 0, false},
	25	+	{"no comm", "garbage", 0, 0, false},
	26	+	}
	27	+	for _, c := range cases {
	28	+	state, pg, ok := parseProcStat(c.in)
	29	+	if state != c.state \|\| pg != c.pgrp \|\| ok != c.ok {
	30	+	t.Errorf("%s: got (%c,%d,%v), want (%c,%d,%v)", c.name, state, pg, ok, c.state, c.pgrp, c.ok)
	31	+	}
	32	+	}
	33	+	}
	34	+
	35	+	func TestLiveGroupMembersSeesSelfOnly(t *testing.T) {
	36	+	// Our own process group has live members (us); a made-up one does not.
	37	+	live, known := liveGroupMembers(os.Getpid())
	38	+	if !known {
	39	+	t.Skip("/proc not readable")
	40	+	}
	41	+	_ = live // our pgid is usually not our pid; only the empty case is stable
	42	+	if live, known := liveGroupMembers(1 << 30); !known \|\| live {
	43	+	t.Errorf("unused group reported live=%v known=%v", live, known)
	44	+	}
	45	+	}
	46	+
	47	+	// ---- GO-020: group cleanup happens while the leader is still a zombie ----
	48	+
	49	+	func TestStdioGroupCleanedBeforeLeaderIsReaped(t *testing.T) {
	50	+	c, _ := connectFake(t, nil)
	51	+	st := c.t.(*stdioTransport)
	52	+	leader := st.cmd.Process.Pid
	53	+
	54	+	childPID, err := strconv.Atoi(callText(t, c, "spawn", `{"mode":"sleeper-stubborn"}`))
	55	+	if err != nil {
	56	+	t.Fatal(err)
	57	+	}
	58	+	t.Cleanup(func() { killPID(childPID) })
	59	+	if !processAlive(childPID) {
	60	+	t.Fatal("descendant not running")
	61	+	}
	62	+
	63	+	type observation struct {
	64	+	state byte
	65	+	childAlive bool
	66	+	pgidKept int
	67	+	}
	68	+	obs := make(chan observation, 1)
	69	+	hook := func(pid int) {
	70	+	if pid != leader {
	71	+	return
	72	+	}
	73	+	var o observation
	74	+	if raw, err := os.ReadFile("/proc/" + strconv.Itoa(leader) + "/stat"); err == nil {
	75	+	o.state, _, _ = parseProcStat(string(raw))
	76	+	}
	77	+	o.childAlive = processAlive(childPID)
	78	+	st.guard.mu.Lock()
	79	+	o.pgidKept = st.guard.pgid
	80	+	st.guard.mu.Unlock()
	81	+	select {
	82	+	case obs <- o:
	83	+	default:
	84	+	}
	85	+	}
	86	+	beforeReapHook.Store(&hook)
	87	+	t.Cleanup(func() { beforeReapHook.Store(nil) })
	88	+
	89	+	// The server exits on its own.
	90	+	_, _ = c.CallTool(t.Context(), "exit", []byte(`{}`))
	91	+	var o observation
	92	+	select {
	93	+	case o = <-obs:
	94	+	case <-after5s():
	95	+	t.Fatal("cleanup hook never ran: the leader was reaped before group cleanup")
	96	+	}
	97	+	if o.state != 'Z' {
	98	+	t.Errorf("leader state at cleanup = %q, want Z (exited but not reaped, so its pid/pgid cannot be reused)", o.state)
	99	+	}
	100	+	if o.childAlive {
	101	+	t.Error("descendant still alive when the leader was about to be reaped")
	102	+	}
	103	+	if o.pgidKept != 0 {
	104	+	t.Errorf("process-group id %d retained after cleanup", o.pgidKept)
	105	+	}
	106	+	// And the leader is reaped afterwards.
	107	+	eventually(t, "leader to be reaped", func() bool {
	108	+	_, err := os.Stat("/proc/" + strconv.Itoa(leader))
	109	+	return os.IsNotExist(err)
	110	+	})
	111	+	}

A packages/agent/mcp/proc_wait_other.go +10 -0

@@ -0,0 +1,10 @@
	1	+	//go:build !linux && !windows
	2	+
	3	+	package mcp
	4	+
	5	+	// waitLeaderExited is unavailable here: the portable APIs reap the child. The
	6	+	// guard then cleans up after the leader was reaped (see processGuard).
	7	+	func waitLeaderExited(int) bool { return false }
	8	+
	9	+	// liveGroupMembers cannot inspect the process table portably.
	10	+	func liveGroupMembers(int) (live, known bool) { return false, false }

@@ -0,0 +1,10 @@
	1	+	//go:build !linux && !windows
	2	+
	3	+	package mcp
	4	+
	5	+	// waitLeaderExited is unavailable here: the portable APIs reap the child. The
	6	+	// guard then cleans up after the leader was reaped (see processGuard).
	7	+	func waitLeaderExited(int) bool { return false }
	8	+
	9	+	// liveGroupMembers cannot inspect the process table portably.
	10	+	func liveGroupMembers(int) (live, known bool) { return false, false }

M packages/agent/mcp/proc_windows.go +5 -0


@@ -112,6 +112,11 @@ func resumeProcess(pid uint32) error {
112	112		return nil
113	113		}
114	114
	115	+	// awaitLeaderExit is not needed on Windows: the Job Object, not a reusable
	116	+	// numeric id, identifies the group. It reports false so the caller waits for
	117	+	// the process first.
	118	+	func (g *processGuard) awaitLeaderExit() bool { return false }
	119	+
115	120		// terminate kills every process in the job. Windows has no graceful group
116	121		// signal, so force is ignored.
117	122		func (g *processGuard) terminate(_ bool) {


@@ -112,6 +112,11 @@ func resumeProcess(pid uint32) error {
112	return nil	112		return nil
113	}	113		}
114		114
		115	+	// awaitLeaderExit is not needed on Windows: the Job Object, not a reusable
		116	+	// numeric id, identifies the group. It reports false so the caller waits for
		117	+	// the process first.
		118	+	func (g *processGuard) awaitLeaderExit() bool { return false }
		119	+
115	// terminate kills every process in the job. Windows has no graceful group	120		// terminate kills every process in the job. Windows has no graceful group
116	// signal, so force is ignored.	121		// signal, so force is ignored.
117	func (g *processGuard) terminate(_ bool) {	122		func (g *processGuard) terminate(_ bool) {

A packages/agent/mcp/round3_test.go +183 -0

@@ -0,0 +1,183 @@
	1	+	package mcp
	2	+
	3	+	import (
	4	+	"context"
	5	+	"errors"
	6	+	"net/http"
	7	+	"net/http/httptest"
	8	+	"strings"
	9	+	"sync/atomic"
	10	+	"testing"
	11	+	"time"
	12	+	)
	13	+
	14	+	// Regression tests for the third review round (platform-independent parts).
	15	+
	16	+	// ---- GO-004: HTTP error bodies ----
	17	+
	18	+	func TestHTTPErrorBodySecretStraddlingDisplayCut(t *testing.T) {
	19	+	// End to end: secrets straddling the display cut and split by escapes.
	20	+	const secret = "HTTPSECRET-0123456789abcdef"
	21	+	t.Setenv("MCP_T_BEARER", secret)
	22	+	// The secret straddles the 300-byte display cut, and a second copy is
	23	+	// split by escape sequences so it only matches after stripping.
	24	+	body := strings.Repeat("x", maxStatusBodyDisplay-len(secret)/2) + secret + strings.Repeat("y", 2000) +
	25	+	"HTTPSE\x1b[0mCRET-0123456789abcdef"
	26	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	27	+	http.Error(w, body, http.StatusInternalServerError)
	28	+	}))
	29	+	defer srv.Close()
	30	+	cfg := ServerConfig{Type: "http", URL: srv.URL, BearerTokenEnvVar: "MCP_T_BEARER"}
	31	+	tr, err := newHTTPTransport("x", cfg, nil)
	32	+	if err != nil {
	33	+	t.Fatal(err)
	34	+	}
	35	+	defer tr.close()
	36	+	if got := tr.red.overlap(); got < len(secret) {
	37	+	t.Fatalf("overlap %d does not cover the secret length %d", got, len(secret))
	38	+	}
	39	+	_, err = tr.request(context.Background(), 1, "tools/list", nil)
	40	+	if err == nil {
	41	+	t.Fatal("want an error")
	42	+	}
	43	+	for _, frag := range []string{secret, "HTTPSECR", "HTTPS", "SECRET"} {
	44	+	if strings.Contains(err.Error(), frag) {
	45	+	t.Errorf("secret fragment %q leaked: %q", frag, err)
	46	+	}
	47	+	}
	48	+	}
	49	+
	50	+	func TestStatusErrorBodyTrimsPartialSecrets(t *testing.T) {
	51	+	red := newRedactor(ServerConfig{Env: map[string]string{"K": "TOPSECRET-99"}})
	52	+	// Body cut by the reader in the middle of the secret.
	53	+	err := red.statusErrorBody(500, []byte("failed with token TOPSECR"), true)
	54	+	if strings.Contains(err.Error(), "TOP") {
	55	+	t.Errorf("reader-cut prefix survived: %q", err)
	56	+	}
	57	+	// The same text, complete: a fragment that is not the secret stays.
	58	+	err = red.statusErrorBody(500, []byte("failed with token TOPSECR"), false)
	59	+	if !strings.Contains(err.Error(), "TOPSECR") {
	60	+	t.Errorf("complete bodies must not be trimmed: %q", err)
	61	+	}
	62	+	// A secret hidden by control sequences is matched after stripping, so the
	63	+	// 300-byte display cut never lands inside it. (The trim after that cut is
	64	+	// defense in depth: redaction runs first, so a whole secret is already the
	65	+	// marker when the cut happens and no behavioural input reaches the trim.)
	66	+	long := strings.Repeat("a", 296) + "TOPS" + "\x1b[0mECRET-99" + strings.Repeat("b", 50)
	67	+	err = red.statusErrorBody(500, []byte(long), false)
	68	+	if strings.Contains(err.Error(), "TOPS") \|\| strings.Contains(err.Error(), "SECRET") {
	69	+	t.Errorf("display cut exposed a secret prefix: %q", err)
	70	+	}
	71	+	if !strings.Contains(err.Error(), "…") {
	72	+	t.Errorf("long body not marked cut: %q", err)
	73	+	}
	74	+	if n := len(err.(*HTTPStatusError).Body); n > maxStatusBodyDisplay+len("…") {
	75	+	t.Errorf("body not bounded: %d", n)
	76	+	}
	77	+	}
	78	+
	79	+	// ---- GO-021: writer-side withdrawal ----
	80	+
	81	+	// lateCtx reports an error from Err before Done closes, which lets a test
	82	+	// order the writer's and the caller's observations of the cancellation.
	83	+	type lateCtx struct {
	84	+	context.Context
	85	+	done chan struct{}
	86	+	err atomic.Bool
	87	+	}
	88	+
	89	+	func newLateCtx() *lateCtx { return &lateCtx{Context: context.Background(), done: make(chan struct{})} }
	90	+
	91	+	func (c *lateCtx) Done() <-chan struct{} { return c.done }
	92	+	func (c *lateCtx) Err() error {
	93	+	if c.err.Load() {
	94	+	return context.Canceled
	95	+	}
	96	+	return nil
	97	+	}
	98	+
	99	+	func TestWriteFrameWriterSideSkipDoesNotFailTransport(t *testing.T) {
	100	+	c, _ := connectFake(t, nil)
	101	+	st := c.t.(*stdioTransport)
	102	+	ctx := newLateCtx()
	103	+	ctx.err.Store(true) // the writer will see an ended context at dequeue...
	104	+
	105	+	skipped := make(chan struct{})
	106	+	release := make(chan struct{})
	107	+	st.hookSkipped = func() {
	108	+	close(ctx.done) // ...and only now does the caller learn of it
	109	+	close(skipped)
	110	+	<-release // keep the writer from reporting, so the caller takes the Done branch
	111	+	}
	112	+	type result struct {
	113	+	sent bool
	114	+	err error
	115	+	}
	116	+	res := make(chan result, 1)
	117	+	go func() {
	118	+	sent, err := st.writeFrame(ctx, []byte(`{"jsonrpc":"2.0","method":"notifications/x"}`))
	119	+	res <- result{sent, err}
	120	+	}()
	121	+	select {
	122	+	case <-skipped:
	123	+	case <-time.After(5 * time.Second):
	124	+	t.Fatal("writer never skipped the frame")
	125	+	}
	126	+	var r result
	127	+	select {
	128	+	case r = <-res:
	129	+	case <-time.After(5 * time.Second):
	130	+	t.Fatal("caller did not return")
	131	+	}
	132	+	close(release)
	133	+	if r.sent \|\| !errors.Is(r.err, context.Canceled) {
	134	+	t.Errorf("sent=%v err=%v, want withdrawn", r.sent, r.err)
	135	+	}
	136	+	select {
	137	+	case <-st.done():
	138	+	t.Fatalf("a frame withdrawn by the writer failed the transport: %v", st.failure())
	139	+	default:
	140	+	}
	141	+	// The transport still works.
	142	+	if got := callText(t, c, "echo", `{"text":"ok"}`); got != "echo: ok" {
	143	+	t.Errorf("after skip: %q", got)
	144	+	}
	145	+	}
	146	+
	147	+	// ---- GO-022: budget marker after rune rounding ----
	148	+
	149	+	func TestBudgetedFieldMarksTruncationAfterRuneRounding(t *testing.T) {
	150	+	b := &budgetedText{}
	151	+	b.write(strings.Repeat("a", maxResultTextBytes-4)) // remaining = 4
	152	+	// remaining+1 = 5 bytes lands inside the second 3-byte rune; rounding
	153	+	// down gives 3 bytes, which is within budget, yet text was dropped.
	154	+	got := b.field("日日日日")
	155	+	if got != "日" {
	156	+	t.Fatalf("field = %q", got)
	157	+	}
	158	+	if !b.clipped {
	159	+	t.Fatal("clipping not recorded")
	160	+	}
	161	+	b.write("- " + got)
	162	+	if !b.full() {
	163	+	t.Error("truncation flag not set although text was clipped")
	164	+	}
	165	+	if !strings.HasSuffix(b.String(), "[truncated at 51200 bytes]") {
	166	+	t.Errorf("marker missing: %q", b.String()[len(b.String())-40:])
	167	+	}
	168	+	// Later writes are ignored.
	169	+	before := b.String()
	170	+	b.write("more")
	171	+	if b.String() != before {
	172	+	t.Error("write accepted after truncation")
	173	+	}
	174	+	// A field that fits is neither clipped nor marked.
	175	+	c := &budgetedText{}
	176	+	if got := c.field("日本"); got != "日本" \|\| c.clipped {
	177	+	t.Errorf("field = %q clipped=%v", got, c.clipped)
	178	+	}
	179	+	c.write("- 日本\n")
	180	+	if c.full() \|\| strings.Contains(c.String(), "truncated") {
	181	+	t.Error("unclipped field must not be marked")
	182	+	}
	183	+	}

@@ -0,0 +1,183 @@
	1	+	package mcp
	2	+
	3	+	import (
	4	+	"context"
	5	+	"errors"
	6	+	"net/http"
	7	+	"net/http/httptest"
	8	+	"strings"
	9	+	"sync/atomic"
	10	+	"testing"
	11	+	"time"
	12	+	)
	13	+
	14	+	// Regression tests for the third review round (platform-independent parts).
	15	+
	16	+	// ---- GO-004: HTTP error bodies ----
	17	+
	18	+	func TestHTTPErrorBodySecretStraddlingDisplayCut(t *testing.T) {
	19	+	// End to end: secrets straddling the display cut and split by escapes.
	20	+	const secret = "HTTPSECRET-0123456789abcdef"
	21	+	t.Setenv("MCP_T_BEARER", secret)
	22	+	// The secret straddles the 300-byte display cut, and a second copy is
	23	+	// split by escape sequences so it only matches after stripping.
	24	+	body := strings.Repeat("x", maxStatusBodyDisplay-len(secret)/2) + secret + strings.Repeat("y", 2000) +
	25	+	"HTTPSE\x1b[0mCRET-0123456789abcdef"
	26	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	27	+	http.Error(w, body, http.StatusInternalServerError)
	28	+	}))
	29	+	defer srv.Close()
	30	+	cfg := ServerConfig{Type: "http", URL: srv.URL, BearerTokenEnvVar: "MCP_T_BEARER"}
	31	+	tr, err := newHTTPTransport("x", cfg, nil)
	32	+	if err != nil {
	33	+	t.Fatal(err)
	34	+	}
	35	+	defer tr.close()
	36	+	if got := tr.red.overlap(); got < len(secret) {
	37	+	t.Fatalf("overlap %d does not cover the secret length %d", got, len(secret))
	38	+	}
	39	+	_, err = tr.request(context.Background(), 1, "tools/list", nil)
	40	+	if err == nil {
	41	+	t.Fatal("want an error")
	42	+	}
	43	+	for _, frag := range []string{secret, "HTTPSECR", "HTTPS", "SECRET"} {
	44	+	if strings.Contains(err.Error(), frag) {
	45	+	t.Errorf("secret fragment %q leaked: %q", frag, err)
	46	+	}
	47	+	}
	48	+	}
	49	+
	50	+	func TestStatusErrorBodyTrimsPartialSecrets(t *testing.T) {
	51	+	red := newRedactor(ServerConfig{Env: map[string]string{"K": "TOPSECRET-99"}})
	52	+	// Body cut by the reader in the middle of the secret.
	53	+	err := red.statusErrorBody(500, []byte("failed with token TOPSECR"), true)
	54	+	if strings.Contains(err.Error(), "TOP") {
	55	+	t.Errorf("reader-cut prefix survived: %q", err)
	56	+	}
	57	+	// The same text, complete: a fragment that is not the secret stays.
	58	+	err = red.statusErrorBody(500, []byte("failed with token TOPSECR"), false)
	59	+	if !strings.Contains(err.Error(), "TOPSECR") {
	60	+	t.Errorf("complete bodies must not be trimmed: %q", err)
	61	+	}
	62	+	// A secret hidden by control sequences is matched after stripping, so the
	63	+	// 300-byte display cut never lands inside it. (The trim after that cut is
	64	+	// defense in depth: redaction runs first, so a whole secret is already the
	65	+	// marker when the cut happens and no behavioural input reaches the trim.)
	66	+	long := strings.Repeat("a", 296) + "TOPS" + "\x1b[0mECRET-99" + strings.Repeat("b", 50)
	67	+	err = red.statusErrorBody(500, []byte(long), false)
	68	+	if strings.Contains(err.Error(), "TOPS") \|\| strings.Contains(err.Error(), "SECRET") {
	69	+	t.Errorf("display cut exposed a secret prefix: %q", err)
	70	+	}
	71	+	if !strings.Contains(err.Error(), "…") {
	72	+	t.Errorf("long body not marked cut: %q", err)
	73	+	}
	74	+	if n := len(err.(*HTTPStatusError).Body); n > maxStatusBodyDisplay+len("…") {
	75	+	t.Errorf("body not bounded: %d", n)
	76	+	}
	77	+	}
	78	+
	79	+	// ---- GO-021: writer-side withdrawal ----
	80	+
	81	+	// lateCtx reports an error from Err before Done closes, which lets a test
	82	+	// order the writer's and the caller's observations of the cancellation.
	83	+	type lateCtx struct {
	84	+	context.Context
	85	+	done chan struct{}
	86	+	err atomic.Bool
	87	+	}
	88	+
	89	+	func newLateCtx() *lateCtx { return &lateCtx{Context: context.Background(), done: make(chan struct{})} }
	90	+
	91	+	func (c *lateCtx) Done() <-chan struct{} { return c.done }
	92	+	func (c *lateCtx) Err() error {
	93	+	if c.err.Load() {
	94	+	return context.Canceled
	95	+	}
	96	+	return nil
	97	+	}
	98	+
	99	+	func TestWriteFrameWriterSideSkipDoesNotFailTransport(t *testing.T) {
	100	+	c, _ := connectFake(t, nil)
	101	+	st := c.t.(*stdioTransport)
	102	+	ctx := newLateCtx()
	103	+	ctx.err.Store(true) // the writer will see an ended context at dequeue...
	104	+
	105	+	skipped := make(chan struct{})
	106	+	release := make(chan struct{})
	107	+	st.hookSkipped = func() {
	108	+	close(ctx.done) // ...and only now does the caller learn of it
	109	+	close(skipped)
	110	+	<-release // keep the writer from reporting, so the caller takes the Done branch
	111	+	}
	112	+	type result struct {
	113	+	sent bool
	114	+	err error
	115	+	}
	116	+	res := make(chan result, 1)
	117	+	go func() {
	118	+	sent, err := st.writeFrame(ctx, []byte(`{"jsonrpc":"2.0","method":"notifications/x"}`))
	119	+	res <- result{sent, err}
	120	+	}()
	121	+	select {
	122	+	case <-skipped:
	123	+	case <-time.After(5 * time.Second):
	124	+	t.Fatal("writer never skipped the frame")
	125	+	}
	126	+	var r result
	127	+	select {
	128	+	case r = <-res:
	129	+	case <-time.After(5 * time.Second):
	130	+	t.Fatal("caller did not return")
	131	+	}
	132	+	close(release)
	133	+	if r.sent \|\| !errors.Is(r.err, context.Canceled) {
	134	+	t.Errorf("sent=%v err=%v, want withdrawn", r.sent, r.err)
	135	+	}
	136	+	select {
	137	+	case <-st.done():
	138	+	t.Fatalf("a frame withdrawn by the writer failed the transport: %v", st.failure())
	139	+	default:
	140	+	}
	141	+	// The transport still works.
	142	+	if got := callText(t, c, "echo", `{"text":"ok"}`); got != "echo: ok" {
	143	+	t.Errorf("after skip: %q", got)
	144	+	}
	145	+	}
	146	+
	147	+	// ---- GO-022: budget marker after rune rounding ----
	148	+
	149	+	func TestBudgetedFieldMarksTruncationAfterRuneRounding(t *testing.T) {
	150	+	b := &budgetedText{}
	151	+	b.write(strings.Repeat("a", maxResultTextBytes-4)) // remaining = 4
	152	+	// remaining+1 = 5 bytes lands inside the second 3-byte rune; rounding
	153	+	// down gives 3 bytes, which is within budget, yet text was dropped.
	154	+	got := b.field("日日日日")
	155	+	if got != "日" {
	156	+	t.Fatalf("field = %q", got)
	157	+	}
	158	+	if !b.clipped {
	159	+	t.Fatal("clipping not recorded")
	160	+	}
	161	+	b.write("- " + got)
	162	+	if !b.full() {
	163	+	t.Error("truncation flag not set although text was clipped")
	164	+	}
	165	+	if !strings.HasSuffix(b.String(), "[truncated at 51200 bytes]") {
	166	+	t.Errorf("marker missing: %q", b.String()[len(b.String())-40:])
	167	+	}
	168	+	// Later writes are ignored.
	169	+	before := b.String()
	170	+	b.write("more")
	171	+	if b.String() != before {
	172	+	t.Error("write accepted after truncation")
	173	+	}
	174	+	// A field that fits is neither clipped nor marked.
	175	+	c := &budgetedText{}
	176	+	if got := c.field("日本"); got != "日本" \|\| c.clipped {
	177	+	t.Errorf("field = %q clipped=%v", got, c.clipped)
	178	+	}
	179	+	c.write("- 日本\n")
	180	+	if c.full() \|\| strings.Contains(c.String(), "truncated") {
	181	+	t.Error("unclipped field must not be marked")
	182	+	}
	183	+	}

M packages/agent/mcp/sanitize.go +9 -0


@@ -163,6 +163,15 @@ func (r *redactor) scrub(s string) string {
163	163		return scrubURLs(s)
164	164		}
165	165
	166	+	// overlap is how many extra bytes to read past a display limit so a secret
	167	+	// straddling the limit is captured whole. It is bounded by maxStderrOverlap.
	168	+	func (r *redactor) overlap() int {
	169	+	if r == nil {
	170	+	return 0
	171	+	}
	172	+	return min(r.maxLen, maxStderrOverlap)
	173	+	}
	174	+
166	175		// redact replaces configured secret values. A nil redactor changes nothing.
167	176		func (r *redactor) redact(s string) string {
168	177		if r == nil {


@@ -163,6 +163,15 @@ func (r *redactor) scrub(s string) string {
163	return scrubURLs(s)	163		return scrubURLs(s)
164	}	164		}
165		165
		166	+	// overlap is how many extra bytes to read past a display limit so a secret
		167	+	// straddling the limit is captured whole. It is bounded by maxStderrOverlap.
		168	+	func (r *redactor) overlap() int {
		169	+	if r == nil {
		170	+	return 0
		171	+	}
		172	+	return min(r.maxLen, maxStderrOverlap)
		173	+	}
		174	+
166	// redact replaces configured secret values. A nil redactor changes nothing.	175		// redact replaces configured secret values. A nil redactor changes nothing.
167	func (r *redactor) redact(s string) string {	176		func (r *redactor) redact(s string) string {
168	if r == nil {	177		if r == nil {

M packages/agent/mcp/stdio.go +35 -3


@@ -56,6 +56,10 @@ var defaultEnvVars = func() []string {
56	56		}
57	57		}()
58	58
	59	+	// beforeReapHook, when set (tests), runs after the group was cleaned and
	60	+	// before the leader is reaped.
	61	+	var beforeReapHook atomic.Pointer[func(pid int)]
	62	+
59	63		// writeReq is one frame queued for the writer goroutine.
60	64		type writeReq struct {
61	65		ctx context.Context

@@ -105,6 +109,11 @@ type stdioTransport struct {
105	109		// cancelsSent counts notifications/cancelled sends started (diagnostics
106	110		// and tests).
107	111		cancelsSent atomic.Int64
	112	+	// Test hooks for the writer (nil in production): hookDequeued runs when the
	113	+	// writer takes a frame off the queue, hookSkipped after it claimed a frame
	114	+	// whose context had ended and before it reports that to the caller.
	115	+	hookDequeued func()
	116	+	hookSkipped func()
108	117		// writesStarted counts frames the writer has begun writing (diagnostics
109	118		// and tests).
110	119		writesStarted atomic.Int64

@@ -201,6 +210,16 @@ func startStdio(name string, cfg ServerConfig, cwd string) (*stdioTransport, err
201	210		go t.readStderr(errR)
202	211		go t.readLoop(outR)
203	212		go func() {
	213	+	// Wait for the leader to exit WITHOUT reaping it, clean the group
	214	+	// while the leader is a zombie (its pid, the pgid, cannot be reused
	215	+	// yet), and only then reap it.
	216	+	leaderGone := t.guard.awaitLeaderExit()
	217	+	if leaderGone {
	218	+	t.guard.cleanup()
	219	+	if h := beforeReapHook.Load(); h != nil {
	220	+	(*h)(cmd.Process.Pid)
	221	+	}
	222	+	}
204	223		waitErr := cmd.Wait()
205	224		close(t.exited)
206	225		if waitErr == nil {

@@ -213,9 +232,10 @@ func startStdio(name string, cfg ServerConfig, cwd string) (*stdioTransport, err
213	232		case <-time.After(250 * time.Millisecond):
214	233		}
215	234		t.fail(fmt.Errorf("server process ended: %w", waitErr))
216		-	// The leader is gone: stop whatever it left behind, once, while the
217		-	// process-group id is still guaranteed to belong to our group.
218		-	t.guard.cleanup()
	235	+	if !leaderGone {
	236	+	// No wait-without-reap on this platform: clean up after the fact.
	237	+	t.guard.cleanup()
	238	+	}
219	239		}()
220	240		return t, nil
221	241		}

@@ -377,7 +397,13 @@ func (t *stdioTransport) writeLoop() {
377	397		case <-t.doneCh:
378	398		return
379	399		case req := <-t.writeQ:
	400	+	if t.hookDequeued != nil {
	401	+	t.hookDequeued()
	402	+	}
380	403		if err := req.ctx.Err(); err != nil && req.state.CompareAndSwap(reqQueued, reqSkipped) {
	404	+	if t.hookSkipped != nil {
	405	+	t.hookSkipped()
	406	+	}
381	407		req.res <- err // never started: nothing partial on the wire
382	408		continue
383	409		}

@@ -427,6 +453,12 @@ func (t *stdioTransport) writeFrame(ctx context.Context, frame []byte) (sent boo
427	453		if req.state.CompareAndSwap(reqQueued, reqSkipped) {
428	454		return false, ctx.Err()
429	455		}
	456	+	if req.state.Load() == reqSkipped {
	457	+	// The writer already withdrew the frame itself (its context had
	458	+	// ended when it was dequeued): nothing was written, so the
	459	+	// transport is untouched.
	460	+	return false, ctx.Err()
	461	+	}
430	462		select {
431	463		case err := <-req.res:
432	464		return req.state.Load() == reqWriting, err

 	}
 }()
+// beforeReapHook, when set (tests), runs after the group was cleaned and
+// before the leader is reaped.
+var beforeReapHook atomic.Pointer[func(pid int)]
 // writeReq is one frame queued for the writer goroutine.
 type writeReq struct {
 	ctx   context.Context
 	// cancelsSent counts notifications/cancelled sends started (diagnostics
 	// and tests).
 	cancelsSent atomic.Int64
+	// Test hooks for the writer (nil in production): hookDequeued runs when the
+	// writer takes a frame off the queue, hookSkipped after it claimed a frame
+	// whose context had ended and before it reports that to the caller.
+	hookDequeued func()
+	hookSkipped  func()
 	// writesStarted counts frames the writer has begun writing (diagnostics
 	// and tests).
 	writesStarted atomic.Int64
 	go t.readStderr(errR)
 	go t.readLoop(outR)
 	go func() {
+		// Wait for the leader to exit WITHOUT reaping it, clean the group
+		// while the leader is a zombie (its pid, the pgid, cannot be reused
+		// yet), and only then reap it.
+		leaderGone := t.guard.awaitLeaderExit()
+		if leaderGone {
+			t.guard.cleanup()
+			if h := beforeReapHook.Load(); h != nil {
+				(*h)(cmd.Process.Pid)
+			}
+		}
 		waitErr := cmd.Wait()
 		close(t.exited)
 		if waitErr == nil {
 		case <-time.After(250 * time.Millisecond):
 		}
 		t.fail(fmt.Errorf("server process ended: %w", waitErr))
-		// The leader is gone: stop whatever it left behind, once, while the
+		if !leaderGone {
-		// process-group id is still guaranteed to belong to our group.
+			// No wait-without-reap on this platform: clean up after the fact.
-		t.guard.cleanup()
+			t.guard.cleanup()
+		}
 	}()
 	return t, nil
 }
 		case <-t.doneCh:
 			return
 		case req := <-t.writeQ:
+			if t.hookDequeued != nil {
+				t.hookDequeued()
+			}
 			if err := req.ctx.Err(); err != nil && req.state.CompareAndSwap(reqQueued, reqSkipped) {
+				if t.hookSkipped != nil {
+					t.hookSkipped()
+				}
 				req.res <- err // never started: nothing partial on the wire
 				continue
 			}
 		if req.state.CompareAndSwap(reqQueued, reqSkipped) {
 			return false, ctx.Err()
 		}
+		if req.state.Load() == reqSkipped {
+			// The writer already withdrew the frame itself (its context had
+			// ended when it was dequeued): nothing was written, so the
+			// transport is untouched.
+			return false, ctx.Err()
+		}
 		select {
 		case err := <-req.res:
 			return req.state.Load() == reqWriting, err

M packages/agent/mcp/tools.go +22 -2


@@ -111,6 +111,13 @@ func (b *textBudget) take(s string) string {
111	111		return s[:keep] + fmt.Sprintf("\n... [truncated at %d bytes]", maxResultTextBytes)
112	112		}
113	113
	114	+	// forceTruncate ends the budget now and returns the truncation marker.
	115	+	func (b *textBudget) forceTruncate() string {
	116	+	b.used = maxResultTextBytes
	117	+	b.truncated = true
	118	+	return fmt.Sprintf("\n... [truncated at %d bytes]", maxResultTextBytes)
	119	+	}
	120	+
114	121		func convertCallResult(result CallResult) core.ToolResult {
115	122		out := core.ToolResult{IsError: result.IsError}
116	123		budget := &textBudget{}

@@ -359,6 +366,9 @@ func (t *listResourcesTool) Execute(ctx context.Context, args json.RawMessage, _
359	366		type budgetedText struct {
360	367		b strings.Builder
361	368		budget textBudget
	369	+	// clipped is set by field when it shortened a string; the next write then
	370	+	// ends the output with the truncation marker.
	371	+	clipped bool
362	372		}
363	373
364	374		func (t *budgetedText) full() bool { return t.budget.truncated }

@@ -368,9 +378,15 @@ func (t *budgetedText) remaining() int { return max(0, maxResultTextBytes-t.budg
368	378
369	379		// field prepares one server-supplied string for output: it is cut to the
370	380		// remaining budget first (so a multi-megabyte description is never fully
371		-	// processed) and then stripped of control sequences and line breaks.
	381	+	// processed) and then stripped of control sequences and line breaks. When the
	382	+	// cut removes anything, the next write is marked truncated even if rounding to
	383	+	// a rune boundary left the text within budget.
372	384		func (t *budgetedText) field(s string) string {
373		-	return sanitizeText(clipUTF8(s, t.remaining()+1), false)
	385	+	clipped := clipUTF8(s, t.remaining()+1)
	386	+	if len(clipped) < len(s) {
	387	+	t.clipped = true
	388	+	}
	389	+	return sanitizeText(clipped, false)
374	390		}
375	391
376	392		// clipUTF8 shortens s to at most n bytes without splitting a rune.

@@ -389,6 +405,10 @@ func (t *budgetedText) write(s string) {
389	405		return
390	406		}
391	407		t.b.WriteString(t.budget.take(s))
	408	+	if t.clipped && !t.budget.truncated {
	409	+	t.b.WriteString(t.budget.forceTruncate())
	410	+	}
	411	+	t.clipped = false
392	412		}
393	413
394	414		func (t *budgetedText) printf(format string, args ...any) {

 	return s[:keep] + fmt.Sprintf("\n... [truncated at %d bytes]", maxResultTextBytes)
 }
+// forceTruncate ends the budget now and returns the truncation marker.
+func (b *textBudget) forceTruncate() string {
+	b.used = maxResultTextBytes
+	b.truncated = true
+	return fmt.Sprintf("\n... [truncated at %d bytes]", maxResultTextBytes)
+}
 func convertCallResult(result CallResult) core.ToolResult {
 	out := core.ToolResult{IsError: result.IsError}
 	budget := &textBudget{}
 type budgetedText struct {
 	b      strings.Builder
 	budget textBudget
+	// clipped is set by field when it shortened a string; the next write then
+	// ends the output with the truncation marker.
+	clipped bool
 }
 func (t *budgetedText) full() bool { return t.budget.truncated }
 // field prepares one server-supplied string for output: it is cut to the
 // remaining budget first (so a multi-megabyte description is never fully
-// processed) and then stripped of control sequences and line breaks.
+// processed) and then stripped of control sequences and line breaks. When the
+// cut removes anything, the next write is marked truncated even if rounding to
+// a rune boundary left the text within budget.
 func (t *budgetedText) field(s string) string {
-	return sanitizeText(clipUTF8(s, t.remaining()+1), false)
+	clipped := clipUTF8(s, t.remaining()+1)
+	if len(clipped) < len(s) {
+		t.clipped = true
+	}
+	return sanitizeText(clipped, false)
 }
 // clipUTF8 shortens s to at most n bytes without splitting a rune.
 		return
 	}
 	t.b.WriteString(t.budget.take(s))
+	if t.clipped && !t.budget.truncated {
+		t.b.WriteString(t.budget.forceTruncate())
+	}
+	t.clipped = false
 }
 func (t *budgetedText) printf(format string, args ...any) {