diff --git a/companions/proto/openapi_optout_test.go b/companions/proto/openapi_optout_test.go new file mode 100644 index 00000000..ad32bd75 --- /dev/null +++ b/companions/proto/openapi_optout_test.go @@ -0,0 +1,89 @@ +package proto + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +// TestWithoutOpenAPILeavesFlagUnsetByDefault locks in that the OpenAPI +// post-generation stage remains opt-out: a freshly constructed Buf runs it, and +// WithoutOpenAPI is the only thing that suppresses it. The builder is fluent, so +// it must return the same receiver for chaining alongside the other With* calls. +func TestWithoutOpenAPILeavesFlagUnsetByDefault(t *testing.T) { + generator, err := NewBuf(context.Background(), t.TempDir()) + if err != nil { + t.Fatalf("NewBuf: %v", err) + } + if generator.skipOpenAPI { + t.Fatal("NewBuf must run the OpenAPI stage by default (got skipOpenAPI = true)") + } + if got := generator.WithoutOpenAPI(); got != generator { + t.Fatal("WithoutOpenAPI must return the receiver for fluent chaining") + } + if !generator.skipOpenAPI { + t.Fatal("WithoutOpenAPI must opt out of the OpenAPI stage (got skipOpenAPI = false)") + } +} + +// TestEmitOpenAPIArtifactsOptOutLeavesSwaggerUntouched is the behavioral guard +// for the downstream consumer: service-python-fastapi generates gRPC stubs from +// its service root, which also holds a FastAPI openapi/ REST contract. Without +// the opt-out, every Sync would run npx openapi-typescript and drop a stray .ts +// into the Python service. With WithoutOpenAPI the stage short-circuits before +// touching the swagger inputs or spawning any process — so a nil runner is +// safe here precisely because it is never reached. +func TestEmitOpenAPIArtifactsOptOutLeavesSwaggerUntouched(t *testing.T) { + root := t.TempDir() + openapiDir := filepath.Join(root, "openapi") + if err := os.MkdirAll(openapiDir, 0o755); err != nil { + t.Fatalf("mkdir openapi directory: %v", err) + } + + const canonicalDoc = `{"swagger":"2.0","info":{"title":"api"}}` + const extraDoc = `{"swagger":"2.0","info":{"title":"extra"}}` + canonical := filepath.Join(openapiDir, "api.swagger.json") + extra := filepath.Join(openapiDir, "extra.swagger.json") + if err := os.WriteFile(canonical, []byte(canonicalDoc), 0o644); err != nil { + t.Fatalf("write canonical swagger: %v", err) + } + if err := os.WriteFile(extra, []byte(extraDoc), 0o644); err != nil { + t.Fatalf("write extra swagger: %v", err) + } + + generator, err := NewBuf(context.Background(), root) + if err != nil { + t.Fatalf("NewBuf: %v", err) + } + generator.WithoutOpenAPI() + + // A nil runner would panic the moment the TypeScript loop tried to spawn a + // process; reaching this without a panic proves the stage short-circuited. + if err := generator.emitOpenAPIArtifacts(context.Background(), nil); err != nil { + t.Fatalf("emitOpenAPIArtifacts with opt-out: %v", err) + } + + for path, want := range map[string]string{canonical: canonicalDoc, extra: extraDoc} { + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("swagger input %q was disturbed: %v", path, err) + } + if string(got) != want { + t.Fatalf("swagger input %q content = %q, want %q", path, got, want) + } + } + + entries, err := os.ReadDir(openapiDir) + if err != nil { + t.Fatalf("read openapi directory: %v", err) + } + for _, entry := range entries { + if filepath.Ext(entry.Name()) == ".ts" { + t.Fatalf("opt-out still produced a TypeScript file: %s", entry.Name()) + } + if filepath.Ext(entry.Name()) == ".json" && entry.Name() != "api.swagger.json" && entry.Name() != "extra.swagger.json" { + t.Fatalf("opt-out produced an unexpected intermediate file: %s", entry.Name()) + } + } +} diff --git a/companions/proto/proto.go b/companions/proto/proto.go index 1a59764e..9f685933 100644 --- a/companions/proto/proto.go +++ b/companions/proto/proto.go @@ -41,6 +41,13 @@ type Buf struct { // directory. It defaults to Dir, but a service whose protocol tree lives // below the service root can widen this boundary explicitly. generatedRoot string + + // skipOpenAPI opts the generator out of the OpenAPI post-generation stage. + // A service that owns a proto tree purely for gRPC stubs but also ships an + // openapi/ REST contract (e.g. a Python FastAPI service) would otherwise + // have TypeScript types generated into it on every Sync. WithoutOpenAPI + // suppresses that stage; buf code generation is unaffected. + skipOpenAPI bool } func NewBuf(ctx context.Context, dir string) (*Buf, error) { @@ -93,6 +100,23 @@ func (g *Buf) WithGeneratedDirs(dirs ...string) *Buf { return g } +// WithoutOpenAPI opts out of the OpenAPI post-generation stage: the canonical +// OpenAPI move and the OpenAPI→TypeScript derivation are both skipped, leaving +// any openapi/*.swagger.json inputs untouched. buf generate still runs. This +// suits non-TypeScript services that own a proto tree for gRPC stubs yet keep +// an unrelated openapi/ REST contract alongside it (e.g. service-python-fastapi). +// +// The opt-out only stops future emission; it deliberately does not delete +// artifacts a prior (opted-in) run already produced. A service that adopts this +// after previously syncing with the stage enabled must remove the now-stale +// generated files (e.g. a committed openapi/api.ts) once by hand — the stage +// cannot tell a stale generated .ts from one someone later authored at the same +// path, so it never removes them. +func (g *Buf) WithoutOpenAPI() *Buf { + g.skipOpenAPI = true + return g +} + // Generate runs buf in a companion (golden wrapper) to regenerate code from local proto files. func (g *Buf) Generate(ctx context.Context) error { w := wool.Get(ctx).In("proto.Generate") @@ -176,6 +200,31 @@ func (g *Buf) Generate(ctx context.Context) error { return w.Wrapf(err, "cannot generate with buf") } + if err = g.emitOpenAPIArtifacts(ctx, runner); err != nil { + return w.Wrapf(err, "cannot emit OpenAPI artifacts") + } + + if err = g.updateGenerationCache(ctx); err != nil { + return w.Wrapf(err, "cannot update cache") + } + return nil +} + +// emitOpenAPIArtifacts runs the OpenAPI post-generation stage that follows buf +// generate: it promotes a generated Swagger document to Codefly's canonical +// OpenAPI path, then derives TypeScript types from every openapi/*.swagger.json +// via the Swagger 2.0 → OpenAPI 3.0 → TypeScript pipeline. Callers that own a +// proto tree purely for gRPC stubs but keep an unrelated openapi/ REST contract +// opt out with WithoutOpenAPI, which short-circuits this stage and leaves those +// swagger inputs untouched. +func (g *Buf) emitOpenAPIArtifacts(ctx context.Context, runner companion.CompanionRunner) error { + w := wool.Get(ctx).In("proto.emitOpenAPIArtifacts") + + if g.skipOpenAPI { + w.Debug("skipping OpenAPI post-generation stage (opted out)") + return nil + } + // Deal with OpenAPI if exists openapi := path.Join(g.Dir, "openapi/api.swagger.json") if ok, err := shared.FileExists(ctx, openapi); err == nil && ok { @@ -234,9 +283,6 @@ func (g *Buf) Generate(ctx context.Context) error { } } - if err = g.updateGenerationCache(ctx); err != nil { - return w.Wrapf(err, "cannot update cache") - } return nil } diff --git a/runners/base/pgid_legacy_test.go b/runners/base/pgid_legacy_test.go index 4f1336d9..fe981755 100644 --- a/runners/base/pgid_legacy_test.go +++ b/runners/base/pgid_legacy_test.go @@ -70,18 +70,13 @@ func TestReaperReapsLegacyGroupWhenOwnerPidReused(t *testing.T) { } // A live owner that started strictly after the record was written cannot be // the process that spawned the leader — it is a recycled PID, so the group - // is orphaned and must still be reaped. - for time.Now().Unix() <= leaderStart { - time.Sleep(50 * time.Millisecond) - } - owner := exec.Command("sleep", "30") - if err := owner.Start(); err != nil { - t.Fatal(err) - } - defer func() { - _ = owner.Process.Kill() - _ = owner.Wait() - }() + // is orphaned and must still be reaped. The reaper compares the owner's + // start second as reported by processStartUnixSeconds (derived from /proc, + // which truncates the kernel's clock-tick start time to whole seconds), so + // gate the spawn on that same clock rather than wall-clock time.Now: a + // wall-clock second past leaderStart can still read back as leaderStart + // from /proc, which would make the reaper preserve the group. + owner := spawnOwnerAfterSecond(t, leaderStart) legacyPath := legacyRecordPath(t, pid, ".pgid") writeLegacyRecord(t, legacyPath, pid, owner.Process.Pid, leaderStart) @@ -199,6 +194,42 @@ func spawnOrphanedStubbornLeader(t *testing.T) int { return pid } +// spawnOwnerAfterSecond starts a live helper process whose start second, as +// observed through processStartUnixSeconds (the reaper's own clock), is +// strictly greater than after. Gating on that clock — rather than wall-clock +// time.Now — keeps the "reused owner PID" scenario deterministic: /proc +// truncates the kernel's clock-tick start time to whole seconds, so a process +// launched a wall-clock second past `after` can still read back as `after`. +func spawnOwnerAfterSecond(t *testing.T, after int64) *exec.Cmd { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for { + owner := exec.Command("sleep", "30") + if err := owner.Start(); err != nil { + t.Fatal(err) + } + start, err := processStartUnixSeconds(owner.Process.Pid) + if err != nil { + _ = owner.Process.Kill() + _ = owner.Wait() + t.Fatal(err) + } + if start > after { + t.Cleanup(func() { + _ = owner.Process.Kill() + _ = owner.Wait() + }) + return owner + } + _ = owner.Process.Kill() + _ = owner.Wait() + if time.Now().After(deadline) { + t.Fatalf("owner process start second %d never advanced past %d", start, after) + } + time.Sleep(50 * time.Millisecond) + } +} + func legacyRecordPath(t *testing.T, pgid int, suffix string) string { t.Helper() dir, err := pgidStateDir()