diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..81c8fc8 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,154 @@ +# Working in solution-runtime-go + +This repository owns the generic Go runtime for **codefly solutions** — modules +deployed independently of the host they extend, with no build-time coupling. It +owns what every solution needs identically: configuration resolution through the +codefly SDK, self-registration with the host frontend and the gateway (the +credential exchanges and their heartbeats), CORS, Module Federation asset +serving, the capability handshake, the manifest, and the gateway client a +handler uses to read composed modules on the viewer's behalf. + +It owns none of the counterparts it talks to. The registration surfaces and +their admission rules belong to the host (`codefly-dev/module-saas-starter`) and +the gateway; endpoint, port and secret resolution to `codefly-dev/sdk-go`; +workspace and manifest types to `codefly-dev/core`; running a composition and +provisioning its secrets to `codefly-dev/cli`. Everything here is a library — +one Go package at the repository root, consumed as the Go module +`github.com/codefly-dev/solution-runtime-go` at a `vX.Y.Z` tag. + +## Boundaries + +- Read [README.md](README.md) first. It is the contract this runtime offers its + consumers and it is kept current with the code, down to the env-override + table. A behaviour change the README describes is unfinished until the README + describes the new one. +- Everything `go doc -short .` prints is pinned at a tag by repos you cannot see + from here — the types a solution author writes (`Manifest`, `Handler`, + `RequestHandler`) as much as the calls it makes. Read that list before you + change a signature, a struct field or an exported constant; a change that + breaks a consumer carries a `!` in the commit type and says so in the PR body. +- Boot configuration is resolved in one place, `loadConfig`, and checked in one + place, `validate()`: resolved through the SDK, with an explicit env override, + refused loudly when unresolved. New configuration follows both rather than + reading the environment where it is used. +- The exception is the consumed-API projection, read at serve time in + `registerConsumedAPIs` and never seen by `validate()`. A malformed one is not + refused: the runtime logs that the federation is disabled and serves on, so + every consumed facade 404s at the gateway while the solution looks healthy. + Anything you add on that path inherits that property — it degrades silently + unless you make it loud, and a log line is the only signal there is. Both + halves of this bullet are pinned by `environment_boundary_test.go`, so a + second exception fails the suite until this file describes it. +- Validate at the boundary — boot, and the headers a request arrives with — not + between internal callers. `validate()` is the model: each refusal names the + variable or the provisioning path that fixes it. +- A registration that cannot attest which publisher it speaks for is not a + registration. There is no fallback to the shared cluster-internal token and no + downgrade path when an exchange answers `404`. +- Tests live beside the code in the same package. Every behaviour change brings + one, and the counterpart it exercises is an `httptest` server, never a real + host. + +## How to behave when something does not work + +Fleet standard, governed by the handbook's agent-context track +([obin-ai/handbook#68](https://github.com/obin-ai/handbook/issues/68)). These +are not style preferences; each is a rule an agent broke at real cost, and this +repository is the runtime that cost was measured against. + +1. **A gap in the tooling is a bug in the tooling** — never a reason to reach + around it. Not as a "workaround", not "just this once", not "until the verb + lands". The failure that produced this standard was exactly that: unable to + run three solutions under one `codefly run`, an agent hand-wrote the + environment these runtimes exist to resolve — internal token, endpoint + carriers, credential paths, pinned ports. It worked, and a sibling runtime + with a missing credential skipped registration *silently*, so a solution + booted, served, and was simply absent. The real cause was a missing CLI + capability (codefly-dev/cli#717). If the CLI cannot express a composition, + that is a CLI bug; if the SDK cannot resolve a value, that is an SDK bug. + Neither is a reason to add a default here. The env overrides in the README + exist for an operator with a deployment the resolver cannot see — they are + not a way to fill a gap in resolution. +2. **Never hack. Always provide the best fix, even when it spans repos.** + Almost nothing that breaks here is owned here. Resolving the host by service + role rather than by module name needed a change in `codefly-dev/core` + (core#382); loading a composed module's workspace path needed another + (core#365). Both were fixed at the owner and arrived here as a `go.mod` pin — + which is why this module carries no `replace` directive. The right fix living + in someone else's repo is not a reason to work around it in this one. If it + genuinely cannot be fixed now, the deliverable is a precise issue against that + owner plus an explicitly labelled stopgap, never an unlabelled one. +3. **Classify every change that makes something work**, in the PR body: a *fix* + at the place that owns the behaviour, or a *hack*. A hack does not become a + fix by working, by being small, by being local, or by the real fix belonging + to core, the SDK or the host. +4. **Never hardcode what the system resolves.** This package resolves every + address, port and secret through the SDK; the single `localhost` literal in + it is the public URL built from the *resolved* port. Both token-exchange URLs + are derived from their register URLs by swapping the path suffix, which is + why an override that drops the documented suffix cannot be paired and is + refused at boot rather than guessed at. If you are typing an address, a port + or a credential, you are encoding something true only on your machine for the + next ten minutes. +5. **Diagnose, do not pattern-match.** "It started working when I set X" is not + a diagnosis — set X back and confirm it breaks. Do not trust an error message + before checking it: in the session above, *"the provisioned secret does not + match its digest"* actually meant a missing internal token. `validate()` + already carries that lesson — it separates "the SDK resolved nothing" from + "loading the injected environment failed first" from "your own override + cannot be paired", because the single generic message sent an operator to + inspect endpoint resolution over a variable they had broken themselves. Keep + new refusals that specific. +6. **Say what you did not verify.** Unverified is not the same as working, and + this suite makes that easy to get wrong: it is hermetic. Every host, gateway + and accounts counterpart is an `httptest` server on `127.0.0.1`, so a green + run proves the wire shapes and refusals this package produces — never that a + real host admits a registration, that accounts mints a Work Context, or that + a composed module accepts one. If you did not run it against a real + composition, the PR says so. + +## Build and test + +Derived from [`.github/workflows/go.yml`](.github/workflows/go.yml), the only +gate. It is four commands, and they are the whole local loop: + +```sh +go mod download && go mod verify +go test -race ./... -count=1 -timeout=5m +go vet ./... +test -z "$(gofmt -l ./*.go)" +``` + +Go comes from `go.mod` (1.27). Nothing else is needed: no Docker, no +credentials, no running composition. + +- The suite takes ~20s, most of it two heartbeat tests that wait on real wall + time (10s and 5s). That is not a hang. +- The formatting gate globs `./*.go`, so it covers the root package and only + the root package. A package added in a subdirectory is vetted and tested by + the commands above but not format-checked; widen the glob in the same PR. +- Dependency pins are how fixes from `core` and `sdk-go` reach consumers: + `go get github.com/codefly-dev/core@vX.Y.Z && go mod tidy`, then the four + commands above. `go mod tidy` is a no-op on a clean tree, so a diff it + produces is part of your change. Say in the commit *why* the pin moves and + reference the owning issue — "bump" is not a reason. +- Releases are plain `vX.Y.Z` tags on `main`; there is no release workflow, and + the one GitHub Release object (v0.0.2) was not kept up — it names no current + version and nothing reads it. The tag is what consumers pin, so it is cut + from a commit whose gate is green. + +## Where the depth is + +Keep this file short; add depth to the file that owns the subject. + +- [README.md](README.md) — the consumer-facing contract: handler errors, the + full configuration and env-override table, self-registration and its + provisioning, consumed-module federation, and reading a + Work-Context-authenticated module. +- The package doc comment and the comments in [`solution.go`](solution.go) — + why a refusal, a backoff cap or a cache lifetime is what it is. Several + record a failure mode that is not obvious from the code; read the one next to + what you are changing before you change it. +- `.claude/skills/` — procedures an agent repeats, loaded only when relevant. + None exist yet, because nothing here repeats beyond the four commands above. + Add one instead of growing this file. diff --git a/environment_boundary_test.go b/environment_boundary_test.go new file mode 100644 index 0000000..1d56323 --- /dev/null +++ b/environment_boundary_test.go @@ -0,0 +1,177 @@ +package solution + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +// serveTimeEnvironmentRead is the one function that reads the environment +// outside boot resolution: the consumed-API projection is parsed when the +// server starts, never reaches validate(), and a malformed value disables the +// federation with a log line instead of refusing the boot. +const serveTimeEnvironmentRead = "registerConsumedAPIs" + +// AGENTS.md tells an agent that configuration is resolved in one place and +// refused at boot, naming the single serve-time exception and the silent +// failure it produces. Prose cannot notice a second exception appearing +// underneath it, and an agent-context file is followed literally, so the claim +// is pinned here: every environment read must sit in loadConfig's call tree, +// where validate() governs the result, or be the documented exception. +func TestEnvironmentIsReadOnlyWhereAgentsFileSaysItIs(t *testing.T) { + pkg := parsePackage(t) + boot := pkg.reachableFrom("loadConfig") + + var undocumented []string + for _, fn := range pkg.environmentReaders() { + if !boot[fn] && fn != serveTimeEnvironmentRead { + undocumented = append(undocumented, fn) + } + } + if len(undocumented) > 0 { + t.Errorf("these functions read the environment outside loadConfig's call tree: %s\n"+ + "AGENTS.md states that boot configuration is resolved in loadConfig and refused by validate(), with %q as the only exception. "+ + "Either resolve the value in loadConfig, or document the new exception in AGENTS.md — a value read after boot is never refused, it degrades while the solution serves.", + strings.Join(undocumented, ", "), serveTimeEnvironmentRead) + } +} + +// The documented exception has to keep being one. If it stops reading the +// environment, or starts being called during boot resolution, AGENTS.md +// describes a hazard that no longer exists where it says it does. +func TestServeTimeEnvironmentReadIsStillTheException(t *testing.T) { + pkg := parsePackage(t) + + reads := false + for _, fn := range pkg.environmentReaders() { + if fn == serveTimeEnvironmentRead { + reads = true + } + } + if !reads { + t.Errorf("%s no longer reads the environment, so AGENTS.md documents an exception that does not exist: remove it there and here", + serveTimeEnvironmentRead) + } + if pkg.reachableFrom("loadConfig")[serveTimeEnvironmentRead] { + t.Errorf("%s is now reachable from loadConfig, so its value is resolved at boot: AGENTS.md describes it as read at serve time and unseen by validate()", + serveTimeEnvironmentRead) + } + + agents, err := os.ReadFile("AGENTS.md") + if err != nil { + t.Fatalf("read AGENTS.md: %v", err) + } + if !strings.Contains(string(agents), serveTimeEnvironmentRead) { + t.Errorf("AGENTS.md does not name %q, the one configuration read that is not refused at boot", serveTimeEnvironmentRead) + } +} + +// pkgFuncs is the package's functions, the calls between them, and which of +// them read the process environment. +type pkgFuncs struct { + calls map[string][]string + reads map[string]bool +} + +func (p pkgFuncs) environmentReaders() []string { + names := make([]string, 0, len(p.reads)) + for name := range p.reads { + names = append(names, name) + } + sort.Strings(names) + return names +} + +func (p pkgFuncs) reachableFrom(root string) map[string]bool { + seen := map[string]bool{root: true} + queue := []string{root} + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + for _, callee := range p.calls[current] { + if !seen[callee] { + seen[callee] = true + queue = append(queue, callee) + } + } + } + return seen +} + +func parsePackage(t *testing.T) pkgFuncs { + t.Helper() + + entries, err := os.ReadDir(".") + if err != nil { + t.Fatalf("read package directory: %v", err) + } + + fset := token.NewFileSet() + decls := map[string]*ast.FuncDecl{} + for _, entry := range entries { + name := entry.Name() + if filepath.Ext(name) != ".go" || strings.HasSuffix(name, "_test.go") { + continue + } + file, err := parser.ParseFile(fset, name, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", name, err) + } + for _, decl := range file.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Body != nil { + decls[fn.Name.Name] = fn + } + } + } + + pkg := pkgFuncs{calls: map[string][]string{}, reads: map[string]bool{}} + for name, fn := range decls { + ast.Inspect(fn.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + if readsEnvironment(call) { + pkg.reads[name] = true + } + if callee, ok := calleeName(call); ok && decls[callee] != nil { + pkg.calls[name] = append(pkg.calls[name], callee) + } + return true + }) + } + if len(pkg.reads) == 0 { + t.Fatal("no environment reads found at all: this guard is inert") + } + return pkg +} + +func readsEnvironment(call *ast.CallExpr) bool { + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + pkg, ok := selector.X.(*ast.Ident) + if !ok || pkg.Name != "os" { + return false + } + return selector.Sel.Name == "Getenv" || selector.Sel.Name == "LookupEnv" || selector.Sel.Name == "Environ" +} + +// calleeName is the called function's own name, whether it is called plainly +// or as a method on a receiver. Names are unique enough in this package to +// identify the declaration. +func calleeName(call *ast.CallExpr) (string, bool) { + switch fn := call.Fun.(type) { + case *ast.Ident: + return fn.Name, true + case *ast.SelectorExpr: + return fn.Sel.Name, true + } + return "", false +}