diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 520035b5..a9017fcd 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -45,6 +45,16 @@ jobs: - name: Install Dependencies run: go mod tidy && go mod download + - name: CGO-free surface guard + # cgo-ness is a link-time property of the import graph: a new import can + # silently drag a cgo-only package (the tree-sitter grammars under + # code/semantic) into a surface that must stay CGO-free, and nothing + # fails until a downstream CGO_ENABLED=0 build (companion publish, an + # alpine image) tries to link — weeks later, one repo away. This builds + # every package except the documented cgo allowlist with CGO_ENABLED=0 + # so creep fails HERE, at the PR that introduces it. See docs/cgo.md. + run: ./scripts/check_cgo_free.sh + - name: Install apt deps (bubblewrap, ripgrep) # bubblewrap → core/runners/sandbox tests (default + sandbox_e2e) # ripgrep → core/code search tests (real-repo grep coverage) diff --git a/CLAUDE.md b/CLAUDE.md index 56249263..13d9d02a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -151,3 +151,4 @@ grpcEP, _ := resources.FindGRPCEndpoint(ctx, endpoints) - **Readiness checks must use gRPC health checks**, not raw TCP connects. A port being open does not mean the service is ready. - **`resources/` is the source of truth** for all type definitions. When in doubt about how something is modeled, look there first. - **Companion containers** are built separately and used at runtime. If a companion is broken, fix it — we own all of this. +- **Keep the CGO-free surface CGO-free.** cgo lives only in `code/semantic` (tree-sitter). Consumers get a build-tag-aware server from `code/codeserver.New` (`-tags codefly_nosemantic` for CGO-free builds). `make check-cgo-free` (also a CI step) fails if a new dependency drags cgo into the CGO-free surface. See `docs/cgo.md`. diff --git a/Makefile b/Makefile index 5a6b863c..396032c3 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,13 @@ GOBIN ?= $$(go env GOPATH)/bin # silently ignored by go build/vet, so this is safe to scope to all commands. export GOFLAGS ?= -timeout=300s +# CGO-free surface guard: asserts every package builds with CGO_ENABLED=0 +# except the documented cgo allowlist, so cgo creep fails here rather than at a +# downstream companion publish. See docs/cgo.md. +.PHONY: check-cgo-free +check-cgo-free: + ./scripts/check_cgo_free.sh + .PHONY: install-go-test-coverage install-go-test-coverage: go install github.com/vladopajic/go-test-coverage/v2@latest diff --git a/code/codeserver/codeserver_test.go b/code/codeserver/codeserver_test.go new file mode 100644 index 00000000..cbe023c9 --- /dev/null +++ b/code/codeserver/codeserver_test.go @@ -0,0 +1,38 @@ +//go:build !codefly_nosemantic + +package codeserver_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/core/code/codeserver" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + codev0 "github.com/codefly-dev/core/generated/go/codefly/services/code/v0" + "google.golang.org/protobuf/encoding/protojson" +) + +// TestNewInstallsSemanticAnalyzer verifies the default (CGO) build wires the +// tree-sitter analyzer into the server: a non-Go source tree produces a complete +// semantic index instead of the unsupported-operation failure a plain +// DefaultCodeServer returns. The codefly_nosemantic variant is exercised by the +// CGO-free build guard (scripts/check_cgo_free.sh), not here. +func TestNewInstallsSemanticAnalyzer(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "app.py"), []byte("import requests\nclass Client: pass\n"), 0o644); err != nil { + t.Fatal(err) + } + + response, err := codeserver.New(root).Execute(t.Context(), &codev0.CodeRequest{ + Operation: &codev0.CodeRequest_GetSemanticIndex{GetSemanticIndex: &codev0.GetSemanticIndexRequest{}}, + }) + if err != nil { + t.Fatal(err) + } + index := response.GetGetSemanticIndex() + if index == nil || index.GetState() != basev0.SemanticIndexState_SEMANTIC_INDEX_STATE_COMPLETE { + encoded, _ := protojson.Marshal(index) + t.Fatalf("semantic index = %s, failure = %#v", encoded, response.GetFailure()) + } +} diff --git a/code/codeserver/doc.go b/code/codeserver/doc.go new file mode 100644 index 00000000..a5055e0d --- /dev/null +++ b/code/codeserver/doc.go @@ -0,0 +1,15 @@ +// Package codeserver assembles the DefaultCodeServer that in-process consumers +// (the CLI, the gateway) run, deciding at build time whether to link the +// tree-sitter semantic analyzer. +// +// New is the single entry point, so consumers never reinvent the cgo/no-cgo +// split themselves. On a normal build it installs core/code/semantic — the +// tree-sitter CGO stack — so the server answers source-semantics operations. +// Built with -tags codefly_nosemantic it returns a plain DefaultCodeServer that +// never imports semantic, keeping the binary CGO-free (linkable with +// CGO_ENABLED=0 for alpine/static targets such as companion publish). +// +// codefly_nosemantic is the canonical, cross-repo build tag for this switch. +// See docs/cgo.md for the CGO surface contract and the CI guard that enforces +// it. +package codeserver diff --git a/code/codeserver/semantics_cgo.go b/code/codeserver/semantics_cgo.go new file mode 100644 index 00000000..42bd07e5 --- /dev/null +++ b/code/codeserver/semantics_cgo.go @@ -0,0 +1,17 @@ +//go:build !codefly_nosemantic + +package codeserver + +import ( + "github.com/codefly-dev/core/code" + "github.com/codefly-dev/core/code/semantic" +) + +// New returns a DefaultCodeServer with the tree-sitter semantic analyzer +// installed. Importing core/code/semantic pulls the tree-sitter CGO stack, so +// this variant cannot link under CGO_ENABLED=0 — an accidental CGO-free build +// fails loudly at link time rather than silently dropping source semantics. +// Build with -tags codefly_nosemantic to select the CGO-free variant. +func New(root string, opts ...code.ServerOption) *code.DefaultCodeServer { + return code.NewDefaultCodeServer(root, append(opts, code.WithSemanticAnalyzer(semantic.New()))...) +} diff --git a/code/codeserver/semantics_nosemantic.go b/code/codeserver/semantics_nosemantic.go new file mode 100644 index 00000000..39923b9f --- /dev/null +++ b/code/codeserver/semantics_nosemantic.go @@ -0,0 +1,13 @@ +//go:build codefly_nosemantic + +package codeserver + +import "github.com/codefly-dev/core/code" + +// New returns a plain DefaultCodeServer with no semantic analyzer. Selected by +// the codefly_nosemantic build tag, this variant never imports +// core/code/semantic, so the binary links with CGO_ENABLED=0. Source-semantics +// operations report an unsupported-operation failure. +func New(root string, opts ...code.ServerOption) *code.DefaultCodeServer { + return code.NewDefaultCodeServer(root, opts...) +} diff --git a/code/semantic.go b/code/semantic.go index 2f41744b..9b7eb477 100644 --- a/code/semantic.go +++ b/code/semantic.go @@ -46,6 +46,11 @@ type SemanticAnalyzer interface { // WithSemanticAnalyzer installs the source-language analyzer. Without it the // server stays free of the tree-sitter CGO stack and semantic operations report // unsupported. +// +// Consumers that need a build-tag-aware server — analyzer installed on normal +// builds, CGO-free under -tags codefly_nosemantic — should call +// code/codeserver.New instead of wiring this option (and the build-tag split) +// themselves. See docs/cgo.md. func WithSemanticAnalyzer(analyzer SemanticAnalyzer) ServerOption { return func(s *DefaultCodeServer) { s.semantic = analyzer } } diff --git a/docs/cgo.md b/docs/cgo.md new file mode 100644 index 00000000..b160cafb --- /dev/null +++ b/docs/cgo.md @@ -0,0 +1,99 @@ +# CGO Surface Contract + +Core is the shared library for the entire codefly ecosystem, and several +downstream binaries must link **statically** (`CGO_ENABLED=0`) for alpine/scratch +targets — notably `codefly companion publish` and `codefly self build +--os/--arch`. cgo-ness is a property of the **import graph**, resolved at link +time: any binary that transitively imports a cgo-only package cannot be built +CGO-free, no matter how cleanly that package is separated. + +cgo dependencies are insidious because they are **transitive and silent**. A new +plugin or a bumped dependency can pull a cgo-only package into the import graph, +and nothing fails until someone tries a `CGO_ENABLED=0` build far downstream — +long after the offending change merged. This document defines the CGO-free +surface as an **enforced contract in core**, so creep fails at the core PR that +introduces it, not at a downstream publish weeks later. + +## The cgo surface + +The entire cgo surface of the module is confined to **one package**: + +| Package | What pulls in cgo | Reachable from the CGO-free surface? | +|---|---|---| +| `code/semantic` | tree-sitter runtime + grammar bindings (`github.com/tree-sitter/*`, `github.com/tree-sitter-grammars/*`, `github.com/dekobon/tree-sitter-groovy`); each grammar's `bindings/go` compiles C via cgo | No — only `code/codeserver` imports it, and only on the default (cgo) build; the `codefly_nosemantic` build drops the import | + +There are **no other cgo dependencies**: no sqlite, no cgo language servers. The +only literal `import "C"` elsewhere is a runner test fixture +(`runners/golang/testdata/mod_cgo`), which is not part of the module build. + +The base `code` package and its per-language servers (`NewGoCodeServer`, +`NewPythonCodeServer`, `NewRustCodeServer`, the TypeScript server) are CGO-free. +The tree-sitter analyzer is reached only through the `code.SemanticAnalyzer` +interface seam, installed via `code.WithSemanticAnalyzer(semantic.New())`. + +## Ownership of the cgo/no-cgo switch + +Package isolation is necessary but **not sufficient** to make a consumer +CGO-free: the consumer still needs a build-time switch to conditionally import +(or not import) `code/semantic`. Core owns that switch so consumers do not each +reinvent it: + +```go +import "github.com/codefly-dev/core/code/codeserver" + +srv := codeserver.New(root) // + optional code.ServerOption values +``` + +- **Default build** — `codeserver.New` installs the tree-sitter analyzer. The + server answers source-semantics operations. The binary links with cgo. +- **`-tags codefly_nosemantic`** — `codeserver.New` returns a plain + `DefaultCodeServer` that never imports `code/semantic`. The binary links with + `CGO_ENABLED=0`. Source-semantics operations report an unsupported-operation + failure. + +The switch cannot live in package `code` itself: `code/semantic` imports `code`, +so a `code` → `code/semantic` import would be a cycle. `code/codeserver` is the +package that depends on both. + +### Canonical build tag + +`codefly_nosemantic` is the **canonical, cross-repo tag name** for this switch. +Downstream consumers (e.g. the CLI's static builds) pass `-tags +codefly_nosemantic` on CGO-free builds; they do not define their own tag. + +**Loud-fail on accidental `CGO_ENABLED=0`.** The default (untagged) build still +imports `code/semantic`, so building it with `CGO_ENABLED=0` fails at link time +(`build constraints exclude all Go files` for the tree-sitter bindings). This is +deliberate: an accidental CGO-free build of the full server fails loudly rather +than silently dropping semantics. Dropping semantics is only ever an explicit +choice, expressed by the tag. + +## Guardrail against creep + +`scripts/check_cgo_free.sh` (Makefile: `make check-cgo-free`; CI: the "CGO-free +surface guard" step) builds every package in the module with `CGO_ENABLED=0 +-tags codefly_nosemantic`, **except** an explicit allowlist. If a package outside +the allowlist stops linking CGO-free, the check fails in core with a clear +message. + +### cgo allowlist + +Packages permitted to require cgo, kept in sync with `CGO_ALLOWLIST` in +`scripts/check_cgo_free.sh`: + +- `github.com/codefly-dev/core/code/semantic` — the tree-sitter semantic + analyzer. + +Adding an entry is a deliberate, reviewed decision. A new cgo dependency does +not belong in the CGO-free surface unless there is no alternative; when it is +genuinely optional, gate it behind `codefly_nosemantic` (as `code/codeserver` +does) instead of allowlisting the package. + +## Out of scope + +Moving tree-sitter semantics out-of-process (a separate cgo helper/agent so +core-linked consumers are fully CGO-free with no split anywhere) was considered +and deferred. It contradicts the in-process source-only path and adds a +process/IPC hop; the build-tag split delivers the CGO-free contract without that +cost. Revisit if the gateway moves per-service semantics out-of-process for +other reasons. diff --git a/scripts/check_cgo_free.sh b/scripts/check_cgo_free.sh new file mode 100755 index 00000000..eebb81e4 --- /dev/null +++ b/scripts/check_cgo_free.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Enforces core's CGO-free surface contract: every package in the module MUST +# build with CGO_ENABLED=0, except a small, explicit allowlist of packages that +# are permitted to require cgo. cgo-ness is a property of the import graph +# resolved at link time — a single new import can silently drag a cgo-only +# package (today: the tree-sitter grammars under code/semantic) into the graph, +# and nothing fails until a downstream CGO_ENABLED=0 build (companion publish, an +# alpine image) tries to link, weeks later and one repo away. +# +# This check fails IN CORE, at the PR that introduces the creep, with a clear +# message. Builds are done with -tags codefly_nosemantic so the supported split +# constructor (code/codeserver.New) selects its CGO-free variant. +# +# See docs/cgo.md for the contract and the ownership boundary. + +set -euo pipefail + +# Packages permitted to require cgo. Adding an entry is a deliberate, reviewed +# decision — a new cgo dependency does not belong in the CGO-free surface unless +# there is no alternative. Keep this list and docs/cgo.md in sync. +CGO_ALLOWLIST=( + "github.com/codefly-dev/core/code/semantic" +) + +cd "$(git rev-parse --show-toplevel)" + +grep_args=() +for pkg in "${CGO_ALLOWLIST[@]}"; do + grep_args+=(-e "$pkg") +done + +# .GoFiles excludes test-only packages, which `go build` rejects with "no +# non-test Go files" — those carry no linkable surface anyway. +pkgs=() +while IFS= read -r line; do + pkgs+=("$line") +done < <(go list -f '{{if .GoFiles}}{{.ImportPath}}{{end}}' ./... | grep -vxF "${grep_args[@]}") + +echo "Checking CGO-free surface: CGO_ENABLED=0 go build -tags codefly_nosemantic" +echo "Allowlist (permitted to require cgo): ${CGO_ALLOWLIST[*]}" + +if CGO_ENABLED=0 go build -tags codefly_nosemantic "${pkgs[@]}"; then + echo "OK: the CGO-free surface links without cgo." + exit 0 +fi + +cat >&2 <<'EOF' + +============================================================================ +CGO creep detected: a package outside the cgo allowlist no longer builds with +CGO_ENABLED=0. + +A "build constraints exclude all Go files" error above means a cgo-only package +(e.g. the tree-sitter grammars under code/semantic) has entered the import +graph of a package that is supposed to stay CGO-free. Downstream CGO-free builds +(companion publish, alpine images) will fail to link. + +Fix one of: + * Remove the new cgo dependency from the CGO-free surface. If it is optional, + gate it behind the codefly_nosemantic build tag (see code/codeserver for the + pattern) so the default build keeps it and the tagged build drops it. + * If the package legitimately must require cgo, add it to CGO_ALLOWLIST in + this script and document why in docs/cgo.md — a conscious, reviewed choice. +============================================================================ +EOF +exit 1