[linyows]

The leaf packages at the module root that only probe itself imports move under internal/, so the compiler says they are not API. While moving, the problems earlier reviews found in them are fixed, each in its own commit.

Moved and removed

Before	After	Why
jsonutil	internal/jsonutil	Imported only by the root, expr and the grpc action
mapping	internal/mapping	Imported only by the built-in actions
safefile	internal/safefile	Imported only by the root and report
truncate	internal/truncate	Imported only by actionrpc and expr
envfile	internal/envfile	Imported only by cmd/probe
oas	internal/oas	Imported only by cmd/probe and the grpc action
procgroup	internal/procgroup	Imported only by the root
pb	internal/pb	The gRPC code between the runner and an action; an action is written against actionrpc.Serve, never pb
binary	internal/body	It reads a response body as text or writes it to a temporary file
binary.ProcessHttpBody	body.TextOrFile	Names the choice it makes
binary.IsJSONMediaType	body.IsJSON
binary.SaveBinaryToTempFile	unexported (saveTemp)	Only TextOrFile uses it
mapping.AnyToString	scalar.String in internal/scalar	jsonutil imported mapping only for it
mapping.HeaderToStringValue	removed; mapping.StringMap	See below
actions/grpc.NewCoverage	protocoverage.New in internal/protocoverage	Only probe coverage used it; cmd/probe no longer imports actions/grpc

truncate.Map, MapString and MaxLength named in the review were already gone. mask and actionref stay at the root, as the root's exported API uses them.

The proto keeps package pb, so the service is still pb.Actions on the wire and an action built with an earlier probe talks to this one. The code is regenerated from the moved proto: only the file name the descriptor records (internal/pb/actions.proto) and go_package, now the full import path, change. make code now runs buf generate --path internal/pb/actions.proto, since a bare buf generate also picks up the e2e and test server protos, which define the same messages and already refused to build together. actionrpc.Server.Run and Cleanup keep their *pb parameters; they are the gRPC handlers, and an action reaches them through Serve.

For external actions

External actions such as probe-browser and probe-agent imported mapping. What they use of it is offered by actionrpc, the package an external action is written against, as functions that call the same code, so they read parameters exactly as the built-in actions do. The guide for external actions (docs/content/guide/concepts/actions.md, en and ja) shows them.

Before	After
mapping.MapToStructByTags(with, &req)	actionrpc.DecodeParams(with, &req)
mapping.StructToMapByTags(v)	actionrpc.EncodeResult(v)
mapping.FieldTags(v)	actionrpc.ParamNames(v)
mapping.ParseTimeout(v)	actionrpc.ParseTimeout(v)

No other package this PR moves is imported by them.

Behavior changes

• A number or a bool in any map[string]string parameter is passed as its text instead of dropped: the gRPC action's metadata, imap's headers, and HTTP's headers (which already got this from HeaderToStringValue).
• A nil value in such a map, as YAML reads X-Empty: with nothing after it, is an empty string: an empty header, where HTTP sent <nil> and the others dropped it.
• A list or a map value in such a map is an error naming the key and the field, such as cannot convert a list to a string for 'x-list' in field 'headers', where HTTP sent its Go text and the others dropped it silently.
• truncate.String backs off to the start of a rune, so a truncated log of Japanese text no longer ends in a broken byte. It backs off at most three bytes, so bytes that are not UTF-8 are cut where the limit falls rather than all left out.
• A template whose output passes the expression engine's length cap is cut with truncate.String, so it no longer splits a UTF-8 character either.
• jsonutil.Decode is Parse with its error put in error_message, which now reads invalid JSON: … rather than mustMarshalJSON error: …. A body of null, which LooksLikeJSON never hands to it, would now be an error_message rather than a nil map. parse_json errors read as before; the prefix moves from jsonutil.Parse to the expression function.
• scalar.String writes nil as "nil", as AnyToString did; its doc comment now says so.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X6CYBxNCxQyo1rbdg9hQ42

[linyows]

Adversarial pass over the diff against refactor/structure. What I checked, and what held:

Wire compatibility (OK). Beyond the file_pb_… → file_internal_pb_… identifier renames, the only changes in the regenerated .pb.go files are the descriptor's file name (pb/actions.proto → internal/pb/actions.proto), the go_package option bytes, and ServiceDesc.Metadata. The proto package is still pb, ServiceName is still pb.Actions, and the full method names are still /pb.Actions/Run and /pb.Actions/Cleanup, with no message or field changes. The file path only matters to the protobuf registry inside one process, and runner and action are separate processes. Nothing in probe resolves its own descriptor by file path; the grpc action's reflection client only talks to the user's target server. So an action built against an older probe still talks to this runner.

Makefile/buf (OK). buf.gen.yaml uses paths=source_relative, so --path internal/pb/actions.proto writes next to the .proto file. Scoping the path also stops a bare buf generate from regenerating actions/grpc/testserver/pb (which is generated by protoc with its own relative source path) and e2e/proto. There are no go:generate directives.

External actions (OK). testdata/*-action and the docs only import actionrpc/actions. Nothing outside the module path refers to the moved packages any more. Server.Run/Cleanup have internal/pb types in their signatures, but actionrpc.Serve users never name them.

Decode/Parse (OK). parse_json errors are unchanged (parse_json: empty string, parse_json: <json err>, parse_json: input is not a JSON object or array). match_json/diff_json don't go through Parse. Every Decode caller except grpc's readResponses is guarded by LooksLikeJSON, and that one gets protojson output or "", so the null → nil-map and "mustMarshalJSON error:" → "invalid JSON:" changes are not reachable in a way that matters. Neither string appears in the docs.

StringMap (OK, one nit). I went through every map[string]string field filled by MapToStructByTags: http headers/cookies, grpc metadata, imap headers, shell env/ready, ssh env. Numbers and bools are now kept instead of dropped (grpc metadata and imap headers gain this too, which is fine). shell.ready is still checked on the raw value by checkReady, and env goes through EnvToStringValue first. HeaderToStringValue only ever touched the headers key, so removing it from embedded/ssh/grpc was a no-op there.

truncate. The … (N more bytes) markers live in internal/dump/detail.go, not in truncate, so their counts are unaffected. The masker runs before truncate.Value in actionrpc, so a cut can't expose a partly masked secret.

Inline below: expr still cuts by bytes; the rune back-off is unbounded on non-UTF-8 input; the module's CHANGELOG has no entry for packages that left the public API; and a minor note on nil headers.

[linyows]

should-fix: the template output cap at expr.go:544-546 still does outputStr[:maxStringLength] + truncate.Message(). That is a byte slice, so it can split a multibyte rune — exactly what this PR fixes in truncate.String. It's the same operation, so outputStr = truncate.String(outputStr, maxStringLength) would close the gap and leave one place that cuts text.

[linyows]

nit: the back-off has no bound. On a string that isn't valid UTF-8 (e.g. Latin-1 or binary text in a logged value, where long runs of 0x80–0xBF bytes are possible), it can walk all the way to 0 and log nothing but the marker. Before, it kept 200 bytes. A valid rune is at most utf8.UTFMax bytes, so stopping after utf8.UTFMax-1 steps (for cut > 0 && maxLen-cut < utf8.UTFMax-1 && !utf8.RuneStart(s[cut])) keeps the fix for real text and keeps the old behaviour for garbage.

[linyows]

should-fix (or track for the series): CHANGELOG.md says it records "changes that break code importing probe as a library". Its v1.27.0 entry documents implementing pb.ActionsServer/pb.ActionsClient directly, and names procgroup.Tracker.AfterStop as new API. After this PR pb, procgroup, mapping, jsonutil, truncate, safefile, envfile, oas, binary and grpc.NewCoverage can't be imported at all, and neither this PR nor refactor/structure adds a Breaking Changes entry for that. Fine to do it once at the end of the series, but it needs to land before the release.

[linyows]

nit: dropping nil is better than the old "<nil>". But YAML X-Empty: (a key with no value) decodes to nil, and a user who writes it most likely means an empty header (e.g. Accept-Encoding: to switch off compression). A list such as Accept: [a, b] now also disappears silently, where it used to be sent as [a b]. Two options: map nil to "", or for http headers return an error on non-scalars so the header isn't lost without a word. Up to you; it isn't a regression.