textmachine/backend/internal/archguard/archguard.go

475 lines
23 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

// Package archguard holds the architectural invariants as go/analysis analyzers, run by `go vet
// -vettool` from `make battery` and CI.
//
// The defect they exist for was never "somebody forgot a key" — it was the SHAPE of the code. Engine
// -internal calls each hand-built their own config.Stage, and a Go literal silently zero-fills every
// field it does not name, so adding the reasoning knob left three sites riding the vendor default
// (D39.87). Adding the field in three places would have left the fourth site, tomorrow, just as broken.
// The literals are gone; these analyzers are what keeps them gone.
//
// WHY TYPES AND NOT SYNTAX. The predecessors walked the AST and resolved types by IMPORT PATH, which
// works but has to enumerate the ways a type can be SPELLED — and each enumeration was found by planting
// a bypass and watching the guard report ok (a byte scan was defeated five ways, the first AST version
// two more). types.Info closes that axis: an alias import, a dot-import, a `type X = config.Stage` and
// the elided element type in `[]config.Stage{{…}}` all resolve to the same types.Type.
//
// WHAT IS NOT CAUGHT. These guards are a floor, not a proof. Twice now the thing that broke them was a
// COMMENT claiming closure the code did not have, so the rule here is: a gap is named below and carries
// a fixture in testdata with no expectation comment, which makes analysistest go red the day it starts
// being reported. The list is what has been planted and measured — not a proof of exhaustiveness.
//
// - Node kind, both seams: a Stage or Request obtained by struct EMBEDDING (`type w struct{
// config.Stage }`), by `make([]config.Stage, 1)` then assigning fields, or through a generic zero
// value (`zeroOf[config.Stage]()`) is not reported. Each yields the zero value with Reasoning
// empty, which is exactly the D39.87 defect. Fixtures: stagesites.go, "documented misses". The
// construction that ENDS this class is a type change — making the zero value unusable outside its
// package — not a longer switch, which is why the switch is not being extended further.
// - Type reach, egress: typeReaches walks type CONSTRUCTORS, and stops at a named type. A signature
// that hides the pair inside one — `type batch struct{ reqs []llm.LLMRequest }`, then
// `Complete(b batch) error` — is not reported. Descending into every named type's underlying would
// reach half the module through context.Context and make the reports hard to predict.
// - Name and shape, egress: a provider call is recognised either by carrying the llm.LLMRequest →
// llm.LLMResponse pair (name-independent) or by being spelled `Complete` against internal/llm. A
// helper that is called something else AND launders the pair through its own types — `func Ask(ctx,
// prompt string) (string, error)` building the request inside — is invisible to both halves.
// - Reflection, egress: a call made through reflect.Value.Call is spelled `Call` and typed
// func([]reflect.Value) []reflect.Value, so neither half can see the provider behind it. No static
// analyzer closes this one; it is named so that nobody has to find it twice.
//
// WHAT WAS GIVEN UP.
//
// The predecessors walked the whole REPOSITORY, reaching eval/ — Go files outside the module, where the
// original defect's fourth call site lived. go/analysis cannot: it needs a package the loader can build,
// and eval/ has no go.mod (`go build` there fails with "cannot find main module"). Measured before
// dropping it: eval/ holds three programs, one of which imports config and pipeline, and NONE constructs
// a Stage or a Request. So the reach guarded nothing, and the polygon is scratch space by design
// (CLAUDE.md) — gating it is not worth a constraint on the guard's form. If a harness there ever needs
// to be covered, the honest fix is a go.mod in eval/, not a syntax-only walk here.
//
// They also read every .go file regardless of BUILD TAGS, which go/analysis cannot do either: a pass
// sees only the files its build configuration selects. `make battery` therefore vets two configurations
// — default and `live` — and a file behind any third tag is not covered by anything. That is a real
// reach the byte scan had; a new tag in this module needs a matching vet line in the Makefile.
package archguard
import (
"go/ast"
"go/types"
"path/filepath"
"strings"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"
)
const (
configPkg = "textmachine/backend/internal/config"
pipelinePkg = "textmachine/backend/internal/pipeline"
llmPkg = "textmachine/backend/internal/llm"
// The provider-neutral pair every real egress carries: a request goes in, a response comes out.
llmRequestType = "LLMRequest"
llmResponseType = "LLMResponse"
)
// Analyzers is every invariant this package enforces. Both drivers — cmd/tmvet and the
// shipping-tree test — take the whole slice, and that test asserts its length, so an analyzer
// dropped from here fails loudly instead of silently enforcing nothing.
func Analyzers() []*analysis.Analyzer {
return []*analysis.Analyzer{StageSeam, RequestSeam, EgressSeam}
}
// exempt reports whether the file holding n is one of the named exceptions, identified by IMPORT PATH
// plus base name and compared exactly.
//
// The obvious spelling — HasSuffix over the absolute filename — was wrong in both directions, and both
// were reproduced: any nested directory or symlink whose tail happened to replay
// ".../backend/internal/pipeline/stagerun.go" INHERITED the exemption, while the intended files LOST it
// on any checkout where the module directory is not literally named "backend". An import path is
// canonical, so neither can happen. PositionFor(…, false) is deliberate: the adjusted position honours
// //line directives, which would let a generated file claim another file's identity.
func exempt(pass *analysis.Pass, n ast.Node, files []string) bool {
id := strings.TrimSuffix(pass.Pkg.Path(), "_test") + "/" +
filepath.Base(pass.Fset.PositionFor(n.Pos(), false).Filename)
for _, f := range files {
if f == id {
return true
}
}
return false
}
// inPackage reports whether the pass is analyzing pkgPath, counting its EXTERNAL test package (path
// "<pkgPath>_test") as the same package.
//
// Callers use this to SKIP a package, so the TrimSuffix widens an exemption rather than closing a hole
// — an earlier comment here claimed the opposite, and the measurement is unambiguous: with a
// `package config_test` file building a Stage, dropping the TrimSuffix makes it report (exit 1) and
// keeping it makes it silent (exit 0). That is intended — a package's external tests are the seam's own
// test surface — but it is an exemption, and the cost is that any file declaring `package config_test`
// or `package llm_test` may build Stages and call providers without a word.
func inPackage(pass *analysis.Pass, pkgPath string) bool {
return strings.TrimSuffix(pass.Pkg.Path(), "_test") == pkgPath
}
// isConversion reports whether call is a type conversion T(x) rather than a function call: the callee
// expression denotes a TYPE, which go/types records in Types[fun].IsType().
func isConversion(pass *analysis.Pass, call *ast.CallExpr) bool {
tv, ok := pass.TypesInfo.Types[call.Fun]
return ok && tv.IsType()
}
// isNamed reports whether t is exactly pkgPath.name, seeing through aliases. It does NOT unwrap
// pointers, slices or maps: `[]config.Stage` as a TYPE is a legitimate field, while each ELEMENT
// literal inside it types as config.Stage on its own and is caught there.
//
// The pointer unwrap this used to do was load-bearing in ONE place and a false positive everywhere
// else (measured both ways): `[]*config.Stage{{…}}` types its elided element as *config.Stage, so a
// literal needs the deref — but `var s *config.Stage` and `(*config.Stage)(p)` construct no Stage at
// all, and go vet has no per-line suppression, so reporting them made the gate unarguable. Only the
// literal branch derefs now.
func isNamed(t types.Type, pkgPath, name string) bool {
named, ok := types.Unalias(t).(*types.Named)
if !ok {
return false
}
obj := named.Obj()
return obj != nil && obj.Pkg() != nil && obj.Pkg().Path() == pkgPath && obj.Name() == name
}
// deref removes one pointer level, for the composite-literal branch only.
func deref(t types.Type) types.Type {
if p, ok := types.Unalias(t).(*types.Pointer); ok {
return p.Elem()
}
return t
}
// typeReaches reports whether t, or any type reachable from it through the type CONSTRUCTORS — pointer,
// slice, array, map, channel, signature, tuple, anonymous struct/interface, generic type argument —
// is a named type satisfying want.
//
// The recursion is what the flat version lacked: `Complete(ctx, reqs []llm.LLMRequest)
// ([]*llm.LLMResponse, error)` unwrapped to a slice and stopped, so the batch form of our own client
// passed the egress guard in silence (D39.96 §3в-2). It stops at a NAMED type — see the package doc.
func typeReaches(t types.Type, want func(*types.Named) bool) bool {
return reaches(t, want, map[types.Type]bool{})
}
func reaches(t types.Type, want func(*types.Named) bool, seen map[types.Type]bool) bool {
t = types.Unalias(t)
if t == nil || seen[t] {
return false // a recursive type (`type node struct{ next *node }`) would not terminate
}
seen[t] = true
switch v := t.(type) {
case *types.Named:
if want(v) {
return true
}
// Type ARGUMENTS are still constructors: `Result[llm.LLMResponse]` carries the response.
for args, i := v.TypeArgs(), 0; args != nil && i < args.Len(); i++ {
if reaches(args.At(i), want, seen) {
return true
}
}
case *types.Pointer:
return reaches(v.Elem(), want, seen)
case *types.Slice:
return reaches(v.Elem(), want, seen)
case *types.Array:
return reaches(v.Elem(), want, seen)
case *types.Chan:
return reaches(v.Elem(), want, seen)
case *types.Map:
return reaches(v.Key(), want, seen) || reaches(v.Elem(), want, seen)
case *types.Tuple:
for i := 0; i < v.Len(); i++ {
if reaches(v.At(i).Type(), want, seen) {
return true
}
}
case *types.Signature:
return reaches(v.Params(), want, seen) || reaches(v.Results(), want, seen)
case *types.Struct: // reachable only while ANONYMOUS — a named struct stops the walk above
for i := 0; i < v.NumFields(); i++ {
if reaches(v.Field(i).Type(), want, seen) {
return true
}
}
case *types.Interface: // likewise anonymous: `interface{ Complete(llm.LLMRequest) error }`
for i := 0; i < v.NumMethods(); i++ {
if reaches(v.Method(i).Type(), want, seen) {
return true
}
}
for i := 0; i < v.NumEmbeddeds(); i++ {
if reaches(v.EmbeddedType(i), want, seen) {
return true
}
}
case *types.TypeParam:
// Inside a generic body the request can travel as the parameter itself; what pins it down is
// the CONSTRAINT. Underlying() is deliberate and is the one place this walk goes THROUGH a
// named type: a constraint is usually named (`type reqBound interface{ … }`), and stopping
// there — the rule everywhere else — reaches nothing at all. It is safe here precisely because
// a constraint is a type-level bound rather than a data structure: it cannot drag in the
// module the way descending into every named struct would.
return reaches(v.Constraint().Underlying(), want, seen)
case *types.Union: // the term list of such a constraint
for i := 0; i < v.Len(); i++ {
if reaches(v.Term(i).Type(), want, seen) {
return true
}
}
}
return false
}
func fromPkg(pkgPath string) func(*types.Named) bool {
return func(n *types.Named) bool {
o := n.Obj()
return o != nil && o.Pkg() != nil && o.Pkg().Path() == pkgPath
}
}
func namedIs(pkgPath, name string) func(*types.Named) bool {
return func(n *types.Named) bool {
o := n.Obj()
return o != nil && o.Pkg() != nil && o.Pkg().Path() == pkgPath && o.Name() == name
}
}
// seamRule is one "there is exactly ONE place this type may be built" invariant. Stage and Request
// differ only in the type protected, the files allowed to build it and the sentence printed — sharing
// the walk is what stopped the two from drifting apart: Request used to inspect ONE node kind against
// Stage's four, so `var r pipeline.Request` and `new(pipeline.Request)` passed both generations of the
// guard (D39.96 §3б).
type seamRule struct {
pkgPath string // package declaring the protected type
name string // its name
ownPkg string // a package that may build it freely ("" — none; exempt files carry it instead)
exempt []string // file path suffixes allowed to build it
msg string
}
func (s seamRule) run(pass *analysis.Pass) (any, error) {
if s.ownPkg != "" && inPackage(pass, s.ownPkg) {
return nil, nil // the type's OWN package: the seam lives here
}
ins := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
ins.Preorder([]ast.Node{
(*ast.CompositeLit)(nil), (*ast.ValueSpec)(nil), (*ast.CallExpr)(nil), (*ast.TypeSpec)(nil),
(*ast.FuncType)(nil),
}, func(n ast.Node) {
if exempt(pass, n, s.exempt) {
return
}
is := func(t types.Type) bool { return isNamed(t, s.pkgPath, s.name) }
hit := false
switch v := n.(type) {
case *ast.CompositeLit:
// TypeOf resolves the ELIDED element type too, so []T{{…}} and map[k]T{k:{…}} are caught
// on their elements; []*T{{…}} types that element as *T, which is why this one derefs.
hit = is(deref(pass.TypesInfo.TypeOf(v)))
case *ast.ValueSpec: // var s T — then assign field by field, no literal anywhere
hit = v.Type != nil && is(pass.TypesInfo.TypeOf(v.Type))
case *ast.FuncType:
// A NAMED RESULT is the same zero-value declaration as `var s T`, one keystroke apart:
// `func f() (s config.Stage) { s.Name = …; return s }`. Params are not declarations —
// the caller supplies them — so only results count.
if v.Results != nil {
for _, f := range v.Results.List {
if len(f.Names) > 0 && is(pass.TypesInfo.TypeOf(f.Type)) {
hit = true
}
}
}
case *ast.CallExpr:
if id, ok := v.Fun.(*ast.Ident); ok && len(v.Args) == 1 &&
pass.TypesInfo.Uses[id] == types.Universe.Lookup("new") {
hit = is(pass.TypesInfo.TypeOf(v.Args[0])) // new(T)
} else if isConversion(pass, v) {
// T(twin{…}) — a conversion from a structurally identical type builds a T just as
// a literal does, and drops exactly the same fields.
hit = is(pass.TypesInfo.TypeOf(v))
}
case *ast.TypeSpec: // type myT = T / type myT T
hit = is(pass.TypesInfo.TypeOf(v.Type))
}
if hit {
pass.Reportf(n.Pos(), "%s", s.msg)
}
})
return nil, nil
}
// StageSeam: outside the config package there is exactly ONE way to obtain a Stage for an engine-internal
// call — config.InternalCall.Stage() — so a fifth call site inherits every decision the seam took instead
// of re-taking them by omission.
var StageSeam = &analysis.Analyzer{
Name: "stageseam",
Doc: "config.Stage must be built only through config.InternalCall.Stage()",
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: stageSeam.run,
}
var stageSeam = seamRule{
pkgPath: configPkg,
name: "Stage",
ownPkg: configPkg,
exempt: []string{
"textmachine/backend/internal/pipeline/fewshot_test.go", // pure predicate test: no wire, no ledger, no checkpoint
// Its subject IS stage inheritance: it builds a PARENT stage to assert what repair derives from it.
"textmachine/backend/internal/pipeline/synthetic_stage_seam_test.go",
},
msg: "an engine-internal call must derive its stage through config.InternalCall.Stage(), " +
"not by building one — a hand-built stage silently drops reasoning/temperature (D39.87)",
}
// RequestSeam: the tuple RequestHash addresses a checkpoint by is assembled in exactly ONE place
// (Runner.attemptRequest). Two assemblies agreeing is not a property anyone maintains by eye — it held
// only while the omitted fields happened to be zero, and the effort knob makes them non-zero. The guard
// is "no Request BUILT outside the seam", not "no RequestHash(Request{…})": the second form is one
// refactor away from invisible (hoist the literal into a variable), and the construction is the hazard.
var RequestSeam = &analysis.Analyzer{
Name: "requestseam",
Doc: "pipeline.Request must be assembled only in Runner.attemptRequest",
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: requestSeam.run,
}
var requestSeam = seamRule{
pkgPath: pipelinePkg,
name: "Request",
// No ownPkg: pipeline is where Request lives AND where every drifting re-assembly would live, so
// the exemption is per-file rather than per-package.
exempt: []string{
"textmachine/backend/internal/pipeline/stagerun.go", // owns attemptRequest — the seam itself
// render.go — where Request is DEFINED and hashed — used to be exempt too. Measured: with the
// exemption removed the tree is still clean, because declaring the type and hashing a
// parameter construct nothing. An exemption that covers no existing code is not neutral; it
// silently pre-approves the next literal somebody adds there.
// The tests OF the hashing contract legitimately build Requests — that is their subject.
"textmachine/backend/internal/pipeline/render_test.go",
"textmachine/backend/internal/pipeline/render_memory_test.go",
},
msg: "a call's request identity must come from Runner.attemptRequest, not a re-assembled " +
"Request literal — a probe that drifts from the attempt turns the role sub-budget off",
}
// EgressSeam: exactly ONE production call site may reach a provider (stagerun.go runAttempt, behind
// clientFor's label assert), plus the live-gated conformance probe. A new egress path — a Ф2 judge, an
// annotator — must extend the routing assert, not slip past it.
//
// The predecessor matched the BYTES `.Complete(`, which cannot tell our client's method from any other
// type's method of that name and had to exempt this guard's own source from matching its own needle.
// Two halves replace it, and a call is reported if EITHER fires:
//
// - SHAPE (isEgressShaped) — the callee carries the llm.LLMRequest → llm.LLMResponse pair. Name
// -independent, so a helper called Do or Ask is caught exactly like Complete, and it reads through
// slices and pointers, so the batch form is caught too.
// - PROVENANCE (isCompleteAgainstLLM) — the callee is spelled Complete and either is declared in
// internal/llm or mentions an internal/llm type at all. This is the parity half: the byte scan
// matched every `.Complete(`, and a client whose request/response types get renamed would slip
// past the shape half alone.
var EgressSeam = &analysis.Analyzer{
Name: "egressseam",
Doc: "provider egress must stay in one place (stagerun.go runAttempt)",
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: runEgressSeam,
}
var egressExempt = []string{
"textmachine/backend/internal/pipeline/stagerun.go", // runAttempt — the single production egress
"textmachine/backend/internal/pipeline/live_conformance_test.go", // live-gated probe: calls the real catalog on purpose
}
func runEgressSeam(pass *analysis.Pass) (any, error) {
if inPackage(pass, llmPkg) {
return nil, nil // the adapters ARE the implementation of Complete — the seam, not a bypass
}
const msg = "provider egress must stay in ONE place (stagerun.go runAttempt, behind clientFor's " +
"label assert) — route this through runAttempt or extend the routing assert deliberately"
ins := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
ins.Preorder([]ast.Node{(*ast.CallExpr)(nil)}, func(n ast.Node) {
call := n.(*ast.CallExpr)
if exempt(pass, n, egressExempt) {
return
}
tv, ok := pass.TypesInfo.Types[call.Fun]
if !ok || tv.IsType() {
return // a conversion calls nothing
}
// The callee's signature is NOT always available: through a type PARAMETER, Underlying() is
// the constraint interface, and a guard that required a signature returned right here — a
// whole generic provider path was invisible (found by planting `runJudge[F providerFn](…,
// ask F, …)` and watching `go vet -vettool` exit 0). The shape half therefore reads the CALL
// SITE, which go/types resolves in every spelling; the signature is only what provenance
// needs, and it may be nil.
sig, _ := tv.Type.Underlying().(*types.Signature)
if isEgressShaped(pass, call, sig) || isCompleteAgainstLLM(pass, call.Fun, sig) {
pass.Reportf(call.Pos(), "%s", msg)
}
})
return nil, nil
}
// isEgressShaped reports whether this call has the shape every provider call has: an llm.LLMRequest
// goes in and an llm.LLMResponse comes out. A wrapper declared OUTSIDE internal/llm cannot launder
// this: to present its own types it has to call the real client somewhere, and that inner call carries
// the pair. A wrapper declared INSIDE internal/llm can, because this analyzer skips that package.
//
// Arguments and the call's own result type are read from the call site, so the answer does not depend
// on how the callee is spelled — a plain func, a method, a func-typed field, a type parameter. The
// declared signature is consulted as well, which covers the one case the call site cannot express: a
// request arriving as a variadic tail that this call happens to leave empty.
func isEgressShaped(pass *analysis.Pass, call *ast.CallExpr, sig *types.Signature) bool {
wantsRequest := func() bool {
for _, a := range call.Args {
if typeReaches(pass.TypesInfo.TypeOf(a), namedIs(llmPkg, llmRequestType)) {
return true
}
}
return sig != nil && typeReaches(sig.Params(), namedIs(llmPkg, llmRequestType))
}
yieldsResponse := func() bool {
if typeReaches(pass.TypesInfo.TypeOf(call), namedIs(llmPkg, llmResponseType)) {
return true
}
return sig != nil && typeReaches(sig.Results(), namedIs(llmPkg, llmResponseType))
}
return wantsRequest() && yieldsResponse()
}
// isCompleteAgainstLLM reports whether the callee is spelled Complete and belongs to internal/llm by
// declaration or by signature.
//
// Resolving the callee through Uses is what the previous version got wrong: it read Selections, which
// go/types does not populate for a QUALIFIED IDENTIFIER, so a package-level `llm.Complete(…)` — a second
// egress reachable from anywhere, since internal/llm is skipped wholesale — returned nil and the
// analyzer walked away (D39.96 §3в-1). Uses is populated for both forms, and taking the callee ident
// covers a dot-imported `Complete(…)` as well.
func isCompleteAgainstLLM(pass *analysis.Pass, fun ast.Expr, sig *types.Signature) bool {
id := calleeIdent(fun)
if id == nil || id.Name != "Complete" {
return false
}
obj := pass.TypesInfo.Uses[id]
declaredInLLM := obj != nil && obj.Pkg() != nil && obj.Pkg().Path() == llmPkg
return declaredInLLM ||
typeReaches(sig.Params(), fromPkg(llmPkg)) || typeReaches(sig.Results(), fromPkg(llmPkg))
}
// calleeIdent is the name a call is written under: the selector's tail for x.f() and llm.f(), the
// identifier itself for a dot-imported or local f().
func calleeIdent(fun ast.Expr) *ast.Ident {
switch v := fun.(type) {
case *ast.Ident:
return v
case *ast.SelectorExpr:
return v.Sel
}
return nil
}