Skip to content

Commit 2c71003

Browse files
fix: honor StartRequest in Runtime.Start (#17)
Start received a StartRequest and only logged it, so `codefly run --set` overrides, the fixture selector and dependency network mappings were accepted and silently dropped. Fold all three into the environment manager before the uvicorn process is assembled, and re-apply the overrides last so a `--set` beats a configuration value naming the same key — the manager emits overrides ahead of configurations, which would otherwise shadow the flag. The hot-reload fast path now compares the incoming request against the one the running process was launched with: identical inputs keep the process (uvicorn --reload covers source edits), changed inputs replace it, since a running process keeps the environment it was given. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3b75d78 commit 2c71003

3 files changed

Lines changed: 264 additions & 8 deletions

File tree

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/codefly-dev/service-python v0.0.52
88
github.com/stretchr/testify v1.11.1
99
google.golang.org/grpc v1.83.0
10+
google.golang.org/protobuf v1.36.12
1011
gopkg.in/yaml.v3 v3.0.1
1112
)
1213

@@ -125,7 +126,6 @@ require (
125126
golang.org/x/text v0.41.0 // indirect
126127
google.golang.org/genproto/googleapis/api v0.0.0-20260817212433-ac3dfec99bb1 // indirect
127128
google.golang.org/genproto/googleapis/rpc v0.0.0-20260817212433-ac3dfec99bb1 // indirect
128-
google.golang.org/protobuf v1.36.12 // indirect
129129
gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
130130
gopkg.in/warnings.v0 v0.1.2 // indirect
131131
k8s.io/kube-openapi v0.0.0-20260721132016-d427ff9ee9ad // indirect

‎runtime.go‎

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package main
33
import (
44
"context"
55
"fmt"
6+
"maps"
67
"os"
78
"path"
9+
"slices"
810
"strings"
911

1012
"github.com/codefly-dev/core/agents/helpers/code"
@@ -20,6 +22,8 @@ import (
2022
"github.com/codefly-dev/core/wool"
2123

2224
pythonruntime "github.com/codefly-dev/service-python/pkg/runtime"
25+
26+
"google.golang.org/protobuf/proto"
2327
)
2428

2529
// Runtime is the FastAPI specialization of the generic Python Runtime.
@@ -54,6 +58,10 @@ type Runtime struct {
5458
runnerEnvironment runners.RunnerEnvironment
5559
runner runners.Proc
5660

61+
// startRequest is what the running process was launched with, kept so a
62+
// later Start can tell whether the inputs it carries still match.
63+
startRequest *runtimev0.StartRequest
64+
5765
port uint16
5866

5967
// grpcPort is the mapped port for the service-owned gRPC listener, passed
@@ -354,7 +362,22 @@ func (s *Runtime) Start(ctx context.Context, req *runtimev0.StartRequest) (*runt
354362
s.Base.Runtime.LogStartRequest(req)
355363

356364
if s.runner != nil && s.FastAPI.Settings.HotReload {
357-
return s.Base.Runtime.StartResponse()
365+
if proto.Equal(req, s.startRequest) {
366+
return s.Base.Runtime.StartResponse()
367+
}
368+
// Hot reload only covers source: uvicorn --reload re-executes the app
369+
// inside the process it was launched in, so that process keeps the
370+
// environment it was given. A request carrying different inputs
371+
// therefore needs a new process, or it would be accepted and never
372+
// take effect.
373+
s.Infof("start inputs changed, restarting fastapi app")
374+
if err := s.runner.Stop(ctx); err != nil {
375+
return s.Base.Runtime.StartError(err)
376+
}
377+
s.runner = nil
378+
// The restart registers a new watcher below, which would otherwise
379+
// leave this one running with no way to reach it.
380+
s.Base.StopWatcher()
358381
}
359382

360383
if s.runnerEnvironment == nil {
@@ -363,6 +386,11 @@ func (s *Runtime) Start(ctx context.Context, req *runtimev0.StartRequest) (*runt
363386
return s.Base.Runtime.StartError(s.Wool.NewError("runner environment not initialized (Init must run before Start)"))
364387
}
365388

389+
startEnvs, err := s.startEnvironment(ctx, req)
390+
if err != nil {
391+
return s.Base.Runtime.StartErrorf(err, "getting environment variables")
392+
}
393+
366394
proc, err := s.runnerEnvironment.NewProcess(
367395
"uv", "run", "uvicorn", "src.main:app",
368396
"--reload", "--host", "0.0.0.0", "--port", fmt.Sprintf("%d", s.port))
@@ -373,12 +401,6 @@ func (s *Runtime) Start(ctx context.Context, req *runtimev0.StartRequest) (*runt
373401
proc.WithOutput(s.Logger)
374402
proc.WithDir(s.Service.SourceLocation)
375403

376-
s.EnvironmentVariables.SetRunning()
377-
378-
startEnvs, err := s.EnvironmentVariables.All()
379-
if err != nil {
380-
return s.Base.Runtime.StartErrorf(err, "getting environment variables")
381-
}
382404
proc.WithEnvironmentVariables(ctx, startEnvs...)
383405
proc.WithEnvironmentVariables(ctx, s.EnvironmentVariables.Secrets()...)
384406
if s.grpcPort != 0 {
@@ -389,6 +411,7 @@ func (s *Runtime) Start(ctx context.Context, req *runtimev0.StartRequest) (*runt
389411
}
390412

391413
s.runner = proc
414+
s.startRequest = req
392415

393416
if s.FastAPI.Settings.HotReload {
394417
conf := services.NewWatchConfiguration(requirements)
@@ -408,6 +431,55 @@ func (s *Runtime) Start(ctx context.Context, req *runtimev0.StartRequest) (*runt
408431
return s.Base.Runtime.StartResponse()
409432
}
410433

434+
// startEnvironment folds the StartRequest into the environment manager and
435+
// returns what the uvicorn process runs with: the per-service overrides of
436+
// `codefly run --set`, the fixture selector, and the resolved addresses of the
437+
// endpoints this service depends on. req.Specs carries agent-specific runtime
438+
// knobs and fastapi defines none, so nothing is read from it.
439+
func (s *Runtime) startEnvironment(ctx context.Context, req *runtimev0.StartRequest) ([]*resources.EnvironmentVariable, error) {
440+
overrides := req.GetOverrides()
441+
s.EnvironmentVariables.AddOverrides(overrides)
442+
443+
if fixture := req.GetFixture(); fixture != "" {
444+
s.EnvironmentVariables.SetFixture(fixture)
445+
}
446+
447+
if err := s.EnvironmentVariables.AddEndpoints(ctx, req.GetDependenciesNetworkMappings(),
448+
resources.NetworkAccessFromRuntimeContext(s.Base.Runtime.RuntimeContext)); err != nil {
449+
return nil, err
450+
}
451+
452+
s.EnvironmentVariables.SetRunning()
453+
454+
envs, err := s.EnvironmentVariables.All()
455+
if err != nil {
456+
return nil, err
457+
}
458+
return withOverridesLast(envs, overrides), nil
459+
}
460+
461+
// withOverridesLast re-appends the overrides at the end of envs and collapses
462+
// the result to a single entry per key, keeping the last value. The
463+
// environment manager emits overrides ahead of the configuration-derived
464+
// variables, so without the reordering a configuration would shadow a `--set`
465+
// naming the same key.
466+
func withOverridesLast(envs []*resources.EnvironmentVariable, overrides map[string]string) []*resources.EnvironmentVariable {
467+
for _, key := range slices.Sorted(maps.Keys(overrides)) {
468+
envs = append(envs, resources.Env(key, overrides[key]))
469+
}
470+
seen := make(map[string]bool, len(envs))
471+
resolved := make([]*resources.EnvironmentVariable, 0, len(envs))
472+
for i := len(envs) - 1; i >= 0; i-- {
473+
if seen[envs[i].Key] {
474+
continue
475+
}
476+
seen[envs[i].Key] = true
477+
resolved = append(resolved, envs[i])
478+
}
479+
slices.Reverse(resolved)
480+
return resolved
481+
}
482+
411483
// Test is INHERITED from *pythonruntime.Runtime (uv run pytest).
412484
// Lint is INHERITED from *pythonruntime.Runtime (uv run ruff check).
413485
// Build is INHERITED (no-op for Python).

‎start_request_test.go‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"io"
6+
"testing"
7+
8+
basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
9+
runtimev0 "github.com/codefly-dev/core/generated/go/codefly/services/runtime/v0"
10+
"github.com/codefly-dev/core/resources"
11+
runners "github.com/codefly-dev/core/runners/base"
12+
"github.com/codefly-dev/core/standards"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
// fakeProc records what Start hands to the service process, standing in for
17+
// the uvicorn process so these tests need no python, docker or network.
18+
type fakeProc struct {
19+
envs []*resources.EnvironmentVariable
20+
started bool
21+
stopped bool
22+
}
23+
24+
func (p *fakeProc) Start(context.Context) error { p.started = true; return nil }
25+
func (p *fakeProc) Run(context.Context) error { return nil }
26+
func (p *fakeProc) Stop(context.Context) error { p.stopped = true; return nil }
27+
func (p *fakeProc) Wait(context.Context) error { return nil }
28+
func (p *fakeProc) IsRunning(context.Context) (bool, error) { return p.started && !p.stopped, nil }
29+
func (p *fakeProc) WaitOn(string) {}
30+
func (p *fakeProc) WithDir(string) {}
31+
func (p *fakeProc) WithOutput(io.Writer) {}
32+
func (p *fakeProc) StdinPipe() (io.WriteCloser, error) { return nil, nil }
33+
func (p *fakeProc) StdoutPipe() (io.ReadCloser, error) { return nil, nil }
34+
35+
func (p *fakeProc) WithEnvironmentVariables(_ context.Context, envs ...*resources.EnvironmentVariable) {
36+
p.envs = append(p.envs, envs...)
37+
}
38+
39+
func (p *fakeProc) WithEnvironmentVariablesAppend(context.Context, *resources.EnvironmentVariable, string) {
40+
}
41+
42+
type fakeRunnerEnvironment struct {
43+
procs []*fakeProc
44+
}
45+
46+
func (e *fakeRunnerEnvironment) Init(context.Context) error { return nil }
47+
func (e *fakeRunnerEnvironment) Stop(context.Context) error { return nil }
48+
func (e *fakeRunnerEnvironment) Shutdown(context.Context) error { return nil }
49+
func (e *fakeRunnerEnvironment) WithBinary(string) error { return nil }
50+
51+
func (e *fakeRunnerEnvironment) WithEnvironmentVariables(context.Context, ...*resources.EnvironmentVariable) {
52+
}
53+
54+
func (e *fakeRunnerEnvironment) NewProcess(string, ...string) (runners.Proc, error) {
55+
proc := &fakeProc{}
56+
e.procs = append(e.procs, proc)
57+
return proc, nil
58+
}
59+
60+
// startTestRuntime builds a Runtime already past Init: a runner environment is
61+
// in place, so Start goes straight to assembling the process.
62+
func startTestRuntime(t *testing.T) (*Runtime, *fakeRunnerEnvironment) {
63+
t.Helper()
64+
runtime := NewRuntime(NewService())
65+
runtime.Location = t.TempDir()
66+
env := &fakeRunnerEnvironment{}
67+
runtime.runnerEnvironment = env
68+
t.Cleanup(runtime.Base.StopWatcher)
69+
return runtime, env
70+
}
71+
72+
// processEnv returns every value the process received for key, in the order
73+
// they were applied.
74+
func processEnv(proc *fakeProc, key string) []string {
75+
var values []string
76+
for _, env := range proc.envs {
77+
if env.Key == key {
78+
values = append(values, env.ValueAsString())
79+
}
80+
}
81+
return values
82+
}
83+
84+
func dependencyMapping() *basev0.NetworkMapping {
85+
return &basev0.NetworkMapping{
86+
Endpoint: &basev0.Endpoint{Module: "mod", Service: "store", Name: "rest", Api: standards.REST},
87+
Instances: []*basev0.NetworkInstance{{
88+
Access: resources.NewNativeNetworkAccess(),
89+
Hostname: "localhost",
90+
Port: 9876,
91+
Address: "http://localhost:9876",
92+
}},
93+
}
94+
}
95+
96+
// TestStartHonorsStartRequest pins the three StartRequest inputs that shape the
97+
// process environment: `codefly run --set` overrides, the fixture selector, and
98+
// the resolved addresses of the endpoints this service depends on.
99+
func TestStartHonorsStartRequest(t *testing.T) {
100+
ctx := context.Background()
101+
runtime, env := startTestRuntime(t)
102+
103+
dependency := dependencyMapping()
104+
resp, err := runtime.Start(ctx, &runtimev0.StartRequest{
105+
Overrides: map[string]string{"LOG_LEVEL": "debug"},
106+
Fixture: "seed",
107+
DependenciesNetworkMappings: []*basev0.NetworkMapping{dependency},
108+
})
109+
require.NoError(t, err)
110+
require.Equal(t, runtimev0.StartStatus_STARTED, resp.GetStatus().GetState(), resp.GetStatus().GetMessage())
111+
require.Len(t, env.procs, 1)
112+
proc := env.procs[0]
113+
require.True(t, proc.started)
114+
115+
require.Equal(t, []string{"debug"}, processEnv(proc, "LOG_LEVEL"))
116+
require.Equal(t, []string{"seed"}, processEnv(proc, resources.FixturePrefix))
117+
118+
dependencyKey := resources.EndpointAsEnvironmentVariableKey(resources.EndpointInformationFromProto(dependency.Endpoint))
119+
require.Equal(t, []string{"http://localhost:9876"}, processEnv(proc, dependencyKey))
120+
}
121+
122+
// TestStartOverrideBeatsConfiguration pins the precedence between the two
123+
// sources that can name the same variable. The environment manager emits
124+
// overrides ahead of configuration-derived values, so a `--set` aimed at a
125+
// configuration key only takes effect because Start reorders them.
126+
func TestStartOverrideBeatsConfiguration(t *testing.T) {
127+
ctx := context.Background()
128+
runtime, env := startTestRuntime(t)
129+
130+
require.NoError(t, runtime.EnvironmentVariables.AddConfigurations(ctx, &basev0.Configuration{
131+
Origin: "mod/store",
132+
Infos: []*basev0.ConfigurationInformation{{
133+
Name: "postgres",
134+
ConfigurationValues: []*basev0.ConfigurationValue{{Key: "url", Value: "from-configuration"}},
135+
}},
136+
}))
137+
138+
key := resources.ServiceConfigurationKeyFromUnique("mod/store", "postgres", "url")
139+
resp, err := runtime.Start(ctx, &runtimev0.StartRequest{Overrides: map[string]string{key: "from-override"}})
140+
require.NoError(t, err)
141+
require.Equal(t, runtimev0.StartStatus_STARTED, resp.GetStatus().GetState(), resp.GetStatus().GetMessage())
142+
require.Len(t, env.procs, 1)
143+
144+
require.Equal(t, []string{"from-override"}, processEnv(env.procs[0], key))
145+
}
146+
147+
// TestStartHotReloadKeepsProcessOnIdenticalRequest proves the hot-reload
148+
// fast path still avoids pointless restarts when nothing about the request
149+
// changed — uvicorn --reload already covers source edits.
150+
func TestStartHotReloadKeepsProcessOnIdenticalRequest(t *testing.T) {
151+
ctx := context.Background()
152+
runtime, env := startTestRuntime(t)
153+
runtime.FastAPI.Settings.HotReload = true
154+
155+
req := &runtimev0.StartRequest{Overrides: map[string]string{"LOG_LEVEL": "debug"}}
156+
_, err := runtime.Start(ctx, req)
157+
require.NoError(t, err)
158+
require.Len(t, env.procs, 1)
159+
160+
_, err = runtime.Start(ctx, &runtimev0.StartRequest{Overrides: map[string]string{"LOG_LEVEL": "debug"}})
161+
require.NoError(t, err)
162+
require.Len(t, env.procs, 1, "an unchanged request must reuse the running process")
163+
require.False(t, env.procs[0].stopped)
164+
}
165+
166+
// TestStartHotReloadReplacesProcessOnChangedRequest is the counterpart: a
167+
// running process keeps the environment it was launched with, so new inputs
168+
// only reach the service through a fresh process.
169+
func TestStartHotReloadReplacesProcessOnChangedRequest(t *testing.T) {
170+
ctx := context.Background()
171+
runtime, env := startTestRuntime(t)
172+
runtime.FastAPI.Settings.HotReload = true
173+
174+
_, err := runtime.Start(ctx, &runtimev0.StartRequest{Overrides: map[string]string{"LOG_LEVEL": "debug"}})
175+
require.NoError(t, err)
176+
require.Len(t, env.procs, 1)
177+
178+
resp, err := runtime.Start(ctx, &runtimev0.StartRequest{Overrides: map[string]string{"LOG_LEVEL": "trace"}})
179+
require.NoError(t, err)
180+
require.Equal(t, runtimev0.StartStatus_STARTED, resp.GetStatus().GetState(), resp.GetStatus().GetMessage())
181+
require.Len(t, env.procs, 2)
182+
require.True(t, env.procs[0].stopped, "the process carrying the old environment must be torn down")
183+
require.Equal(t, []string{"trace"}, processEnv(env.procs[1], "LOG_LEVEL"))
184+
}

0 commit comments

Comments
 (0)