From b5571e50f3027873a8565fcbbd2360e544c1342b Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Tue, 25 Aug 2026 17:35:03 -0400 Subject: [PATCH 1/3] feat: add NewBuf opt-out for the OpenAPI/TypeScript stage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit proto.(*Buf).Generate runs an OpenAPI post-generation stage after buf generate: it promotes a generated Swagger document to the canonical OpenAPI path and derives TypeScript types from every openapi/*.swagger.json. That is correct for services that want TS types from their REST API, but a non-TypeScript service that owns a proto tree for gRPC stubs and also ships an unrelated openapi/ REST contract incurs it too — dropping a stray openapi/api.ts on every Sync. Add a fluent WithoutOpenAPI opt-out, mirroring the existing With* builder methods, that short-circuits the stage. buf dep update / buf generate and the generated-dir cleanup are unchanged, and the default still runs the OpenAPI pipeline. The stage is extracted verbatim into a guarded emitOpenAPIArtifacts method so the opt-out is unit-testable offline. Co-Authored-By: Claude Opus 4.8 --- companions/proto/openapi_optout_test.go | 89 +++++++++++++++++++++++++ companions/proto/proto.go | 45 ++++++++++++- 2 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 companions/proto/openapi_optout_test.go diff --git a/companions/proto/openapi_optout_test.go b/companions/proto/openapi_optout_test.go new file mode 100644 index 00000000..e3905a69 --- /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 (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 (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..f26ac0be 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,16 @@ 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). +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 +193,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 +276,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 } From 9883468108bb2c2b67873b4ef68d697a8dc17b3e Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Tue, 25 Aug 2026 22:13:34 -0400 Subject: [PATCH 2/3] test(runners): deterministically spawn reused-owner in legacy reaper test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestReaperReapsLegacyGroupWhenOwnerPidReused gated the owner spawn on wall-clock time.Now, but the reaper authenticates owners through processStartUnixSeconds, which derives the start second from /proc and truncates the kernel's clock-tick start time to whole seconds. A process launched a wall-clock second past the record's spawn second could still read back as that same second, so legacyOwnerAlive treated the recycled PID as the live owner and preserved the group — leaving it alive and failing the test on Linux CI. Spawn the owner in a loop that verifies processStartUnixSeconds reports a start second strictly greater than the record's, killing and retrying until it does. This gates on the reaper's own clock, removing the wall-clock/proc skew race. Co-Authored-By: Claude Opus 4.8 --- runners/base/pgid_legacy_test.go | 55 +++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 12 deletions(-) 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() From 7519d52e82be7b167221b948c832d7b373ca61a3 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Wed, 26 Aug 2026 09:26:24 -0400 Subject: [PATCH 3/3] docs(proto): state WithoutOpenAPI's one-time cleanup contract; fix test message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WithoutOpenAPI opt-out documented that it skips the OpenAPI stage and leaves inputs untouched, but was silent on what happens to an artifact a prior opted-in run already generated and committed (e.g. openapi/api.ts): it lingers as a stale orphan with no guidance. Auto-deleting it on the skip path was rejected as the fix — the stage emits .ts next to .swagger.json, so a name-matched delete cannot tell a stale generated file from one a teammate later authored at that path, and per-Sync deletion would be silent data loss that contradicts the method's 'leaves inputs untouched' contract. The real fix is to make the one-time manual cleanup explicit in the method doc. Also reword two opt-out test failure messages that printed the observed bad value as if it were the requirement, so a failing run reads forward. Co-Authored-By: Claude Opus 4.8 --- companions/proto/openapi_optout_test.go | 4 ++-- companions/proto/proto.go | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/companions/proto/openapi_optout_test.go b/companions/proto/openapi_optout_test.go index e3905a69..ad32bd75 100644 --- a/companions/proto/openapi_optout_test.go +++ b/companions/proto/openapi_optout_test.go @@ -17,13 +17,13 @@ func TestWithoutOpenAPILeavesFlagUnsetByDefault(t *testing.T) { t.Fatalf("NewBuf: %v", err) } if generator.skipOpenAPI { - t.Fatal("NewBuf must run the OpenAPI stage by default (skipOpenAPI = true)") + 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 (skipOpenAPI = false)") + t.Fatal("WithoutOpenAPI must opt out of the OpenAPI stage (got skipOpenAPI = false)") } } diff --git a/companions/proto/proto.go b/companions/proto/proto.go index f26ac0be..9f685933 100644 --- a/companions/proto/proto.go +++ b/companions/proto/proto.go @@ -105,6 +105,13 @@ func (g *Buf) WithGeneratedDirs(dirs ...string) *Buf { // 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