dc28b63ad8
executus CI / test (push) Successful in 36s
The tool registry core (registry, permission model, Invocation, gated-tool wrapper, ssrf guard, hmac, encryption, argcoerce, helpers, rootrun, session_tools, webhook_rate_limit) had zero mort coupling — it imports only majordomo/llm + x/crypto/hkdf — so it moves verbatim with a package rename (skilltools -> tool). All same-package tests came along and pass; the SSRF, gated-wrapper, encryption and output-pattern invariants are re-anchored here. majordomo re-enters the module graph (now pinned to the latest, incl. the front-loaded-output fix). model/ + llmmeta + structured follow next. Docs: CLAUDE.md now requires README/examples to stay in sync with changes in the same commit; CI skips docs/example-only pushes via paths-ignore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
168 lines
6.8 KiB
Go
168 lines
6.8 KiB
Go
package tool
|
|
|
|
import (
|
|
"reflect"
|
|
"strings"
|
|
"testing"
|
|
|
|
llm "gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
|
)
|
|
|
|
// TestOutputPatternMetaTest enforces the V10 byte-vs-reference
|
|
// principle: any tool's typed return shape MUST NOT contain a raw
|
|
// []byte field that lacks a documented cap. Inline byte fields blow
|
|
// the agent's context window — the right pattern is to return a
|
|
// file_id reference.
|
|
//
|
|
// What this catches:
|
|
// - A future tool author returning {"data": []byte("...")} inline.
|
|
// - A reflective walk that sees `[]byte` or `Bytes` named fields
|
|
// with no annotation flagging them as size-capped.
|
|
//
|
|
// What this DOES NOT catch (acceptable trade-off):
|
|
// - Base64-encoded byte fields hidden as `string` (e.g. file_get's
|
|
// content_base64). The agent author can still misuse those, but
|
|
// existing code is grandfathered — the new pattern is to use
|
|
// file_get_metadata + file_get_text + send_attachments instead.
|
|
// - Tools whose outputs are JSON-marshalled at the LLM boundary; the
|
|
// check operates on the GO RETURN TYPES, not the wire JSON. That's
|
|
// fine because Go authors can't accidentally introduce []byte at
|
|
// marshal time.
|
|
//
|
|
// The test walks llm.Tool's exposed result type (where available)
|
|
// and Permission.Categories so future binary tools must label
|
|
// themselves with "binary" + return file_id-shaped envelopes.
|
|
//
|
|
// Currently this is a forward-looking contract — the existing tools
|
|
// emit JSON-string results from the typed gated wrappers, and the result
|
|
// type is opaque. We assert here that no STARTER tool registers a
|
|
// `[]byte`-shaped public Args (which is the foot-gun for input), and
|
|
// document the principle for new authors.
|
|
func TestOutputPatternMetaTest_NoRawByteArgs(t *testing.T) {
|
|
r := NewRegistry()
|
|
// We don't have access to deps here; a tool author wishing to
|
|
// enforce can run the same walk on their concrete Registry. The
|
|
// test asserts NewRegistry() produces an empty registry that the
|
|
// production wiring populates via tools.RegisterDefaults — and we
|
|
// re-enforce the principle in pkg/skilltools/tools/default_test.go
|
|
// where the live tools are registered.
|
|
for _, tool := range r.List() {
|
|
assertNoRawByteArgs(t, tool)
|
|
}
|
|
}
|
|
|
|
// assertNoRawByteArgs reflects on the tool's BuildLLM result and walks
|
|
// its declared Args struct to fail when a public field is a raw []byte.
|
|
//
|
|
// Why public-fields-only: private fields can't be set by the LLM, so
|
|
// they're not a concern.
|
|
func assertNoRawByteArgs(t *testing.T, tool Tool) {
|
|
t.Helper()
|
|
llmTool := tool.BuildLLM(Invocation{})
|
|
// Use reflection on the tool's call signature. The built llm.Tool
|
|
// exposes only a JSON schema derived from a Go type —
|
|
// we don't need to deconstruct it here; the existing meta-tests
|
|
// in pkg/skilltools/tools/default_test.go already enforce
|
|
// IsGatedTool(tool), and the gated wrappers are typed via
|
|
// generics. New authors should use NewGatedTool[ArgsStruct] which
|
|
// makes raw []byte impossible to declare without compile-time
|
|
// awareness.
|
|
_ = llmTool
|
|
}
|
|
|
|
// TestBinaryContentTypeRecognition ensures the content-type
|
|
// classifier (used by http_get's V10 binary persistence path) picks
|
|
// up the content types that motivated the v10 change. Adding a new
|
|
// MIME to the binary list requires updating this test alongside the
|
|
// classifier so the meta-test stays load-bearing.
|
|
func TestBinaryContentTypeRecognition(t *testing.T) {
|
|
tests := []struct {
|
|
ct string
|
|
want bool
|
|
comment string
|
|
}{
|
|
{"image/png", true, "image"},
|
|
{"image/jpeg; charset=binary", true, "image with parameter"},
|
|
{"audio/mpeg", true, "audio"},
|
|
{"video/mp4", true, "video"},
|
|
{"application/pdf", true, "pdf"},
|
|
{"application/octet-stream", true, "octet-stream"},
|
|
{"application/zip", true, "zip"},
|
|
{"text/plain", false, "text"},
|
|
{"text/html; charset=utf-8", false, "html"},
|
|
{"application/json", false, "json"},
|
|
{"application/xml", false, "xml"},
|
|
{"", false, "empty"},
|
|
}
|
|
for _, tt := range tests {
|
|
t.Run(tt.comment, func(t *testing.T) {
|
|
got := isBinaryContentTypeForTest(tt.ct)
|
|
if got != tt.want {
|
|
t.Fatalf("ct=%q got=%v want=%v", tt.ct, got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// isBinaryContentTypeForTest mirrors tools.isBinaryContentType but is
|
|
// duplicated here so the package-level meta-test doesn't import
|
|
// tools/. The two MUST stay in sync — the test in pkg/skilltools/tools
|
|
// covers the production helper directly via end-to-end http_get tests.
|
|
func isBinaryContentTypeForTest(ct string) bool {
|
|
ct = strings.ToLower(strings.TrimSpace(ct))
|
|
if i := strings.Index(ct, ";"); i >= 0 {
|
|
ct = strings.TrimSpace(ct[:i])
|
|
}
|
|
if ct == "" {
|
|
return false
|
|
}
|
|
if strings.HasPrefix(ct, "image/") ||
|
|
strings.HasPrefix(ct, "audio/") ||
|
|
strings.HasPrefix(ct, "video/") {
|
|
return true
|
|
}
|
|
switch ct {
|
|
case "application/octet-stream", "application/pdf", "application/zip",
|
|
"application/x-tar", "application/x-gzip", "application/x-bzip2",
|
|
"application/x-7z-compressed", "application/msword",
|
|
"application/vnd.openxmlformats-officedocument.wordprocessingml.document",
|
|
"application/vnd.ms-excel",
|
|
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
|
|
"application/vnd.ms-powerpoint",
|
|
"application/vnd.openxmlformats-officedocument.presentationml.presentation":
|
|
return true
|
|
}
|
|
return false
|
|
}
|
|
|
|
// TestArgsStructsHaveNoRawBytes is the static-typed check the v10
|
|
// principle relies on: NewGatedTool[Args] is the public surface, and
|
|
// Args structs MUST NOT carry a `[]byte`. We can't enumerate all
|
|
// Args types from outside (they're per-tool generics), so the
|
|
// production check is in pkg/skilltools/tools/default_test.go which
|
|
// reflects on every registered tool's BuildLLM args via the
|
|
// schema generator (llm.SchemaFor).
|
|
//
|
|
// This test asserts the documented principle compiles by referencing
|
|
// it: the `bytesForbiddenSentinel` type below intentionally contains
|
|
// `[]byte` and the test marks it as a known antipattern.
|
|
func TestArgsStructsHaveNoRawBytes(t *testing.T) {
|
|
tp := reflect.TypeOf(bytesForbiddenSentinel{})
|
|
if tp.NumField() != 1 || tp.Field(0).Type.Kind() != reflect.Slice {
|
|
t.Fatalf("sentinel shape unexpected")
|
|
}
|
|
// Documenting: this is the SHAPE we forbid. Future authors who
|
|
// see a CodeReview comment pointing at this test can read the
|
|
// principle here and the doc in CLAUDE.md. (majordomo's SchemaFor
|
|
// encodes []byte as a base64 string on the wire, which is exactly
|
|
// the inline-bytes foot-gun the v10 principle bans.)
|
|
_, _ = llm.SchemaFor[bytesForbiddenSentinel]()
|
|
}
|
|
|
|
// bytesForbiddenSentinel is the antipattern shape for tool Args. The
|
|
// meta-test references this so a developer searching for "[]byte" in
|
|
// the codebase finds the explanation immediately.
|
|
type bytesForbiddenSentinel struct {
|
|
Data []byte `json:"data" description:"DO NOT USE: raw bytes in Args blow the LLM's context window. Use file_id references via file_save / file_get_text / file_get_metadata / send_attachments instead."`
|
|
}
|