Skip to content

Commit ee06667

Browse files
fix: rename the active-environment accessors so stale reads fail to build (#81) (#82)
Unexporting ActiveEnv behind an accessor of the same name made the break loud for writers and silent for readers: a method value is comparable to nil and assignable to interface{}, so `if svc.ActiveEnv == nil` compiles as dead code and `require.Nil(t, svc.ActiveEnv)` passes vacuously. Only the assignment form failed to build, and go vet catches just one of the two silent shapes. ActiveEnvironment/SetActiveEnvironment leave no pre-#79 spelling that compiles, so the compiler reports what review missed. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ea6b1f3 commit ee06667

8 files changed

Lines changed: 40 additions & 40 deletions

File tree

‎agent.codefly.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
publisher: codefly.dev
22
kind: codefly:service
33
name: python
4-
version: 0.0.60
4+
version: 0.0.61

‎pkg/code/callgraph.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ print(json.dumps({'calls': calls, 'implements': implements, 'module': workdir}))
137137
// so AST analysis uses the same interpreter as the running service.
138138
// Pre-Init falls through ResolveStandaloneEnvironment which picks Nix
139139
// if declared, native otherwise — keeps mode consistency when possible.
140-
env := c.Service.ActiveEnv()
140+
env := c.Service.ActiveEnvironment()
141141
if env == nil {
142142
var rctx *basev0.RuntimeContext
143143
if c.Service.Base != nil && c.Service.Base.Runtime != nil {

‎pkg/code/code.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (c *Code) fixPython(ctx context.Context, input corecode.FixInput) (corecode
7878
// One snapshot for the whole request: a specialization publishing or
7979
// clearing its environment mid-fix must not send check and format to
8080
// two different interpreters.
81-
active := c.Service.ActiveEnv()
81+
active := c.Service.ActiveEnvironment()
8282

8383
checkArgs := []string{"check", "--fix-only"}
8484
if input.Mode == basev0.FixMode_FIX_MODE_AGGRESSIVE {

‎pkg/code/fix_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ func TestCodeIsSafeWhileTheActiveEnvironmentIsRepublished(t *testing.T) {
119119
case <-done:
120120
return
121121
default:
122-
svc.SetActiveEnv(env)
123-
svc.SetActiveEnv(nil)
122+
svc.SetActiveEnvironment(env)
123+
svc.SetActiveEnvironment(nil)
124124
}
125125
}
126126
}()

‎pkg/runtime/commands.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
// (stdin ◀── code ── stdin)
2222
// (stdout ──▶ sentinel-reader ──▶ string)
2323
//
24-
// Mode-consistent: the REPL uses the plugin's ActiveEnv. A Docker-mode
24+
// Mode-consistent: the REPL uses the plugin's active environment. A Docker-mode
2525
// plugin opens the REPL inside its container (same venv, same Python
2626
// version as the running service). Nix-mode plugins get the REPL
2727
// inside the flake's devShell.
@@ -142,10 +142,10 @@ func (s *Runtime) getOrStartRepl(ctx context.Context) (*PythonRepl, error) {
142142
return r, nil
143143
}
144144

145-
// resolveReplEnv returns the plugin's ActiveEnv or a standalone one
145+
// resolveReplEnv returns the plugin's active environment or a standalone one
146146
// when Runtime.Init hasn't populated it. Same pattern as Code/Tooling.
147147
func (s *Runtime) resolveReplEnv(ctx context.Context) runners.RunnerEnvironment {
148-
if env := s.Service.ActiveEnv(); env != nil {
148+
if env := s.Service.ActiveEnvironment(); env != nil {
149149
return env
150150
}
151151
var rctx *basev0.RuntimeContext

‎pkg/runtime/runtime_internal_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,8 +450,8 @@ func TestResolveReplEnvIsSafeWhileTheActiveEnvironmentIsRepublished(t *testing.T
450450
case <-done:
451451
return
452452
default:
453-
service.SetActiveEnv(env)
454-
service.SetActiveEnv(nil)
453+
service.SetActiveEnvironment(env)
454+
service.SetActiveEnvironment(nil)
455455
}
456456
}
457457
}()

‎pkg/service/service.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,33 +46,33 @@ type Service struct {
4646
// Load). Specializations may override at Load time.
4747
SourceLocation string
4848

49-
// activeEnv is a specialization's active RunnerEnvironment, consumed by
50-
// Code / Tooling / the REPL so every spawn routes through the same mode
51-
// (native / docker / nix). The generic runtime leaves it nil; call sites
52-
// fall back to a fresh NativeEnvironment.
49+
// activeEnvironment is a specialization's active RunnerEnvironment,
50+
// consumed by Code / Tooling / the REPL so every spawn routes through the
51+
// same mode (native / docker / nix). The generic runtime leaves it nil;
52+
// call sites fall back to a fresh NativeEnvironment.
5353
//
5454
// A specialization publishes it from Init and clears it from Stop, while
5555
// Code, Tooling and the REPL read it on their own gRPC goroutines, so it
56-
// is reachable only through ActiveEnv / SetActiveEnv.
57-
activeEnvMu sync.RWMutex
58-
activeEnv runners.RunnerEnvironment
56+
// is reachable only through ActiveEnvironment / SetActiveEnvironment.
57+
activeEnvironmentMu sync.RWMutex
58+
activeEnvironment runners.RunnerEnvironment
5959
}
6060

61-
// ActiveEnv returns the environment a specialization published, or nil when
62-
// none is active — before Init, or after the specialization tore it down.
63-
func (s *Service) ActiveEnv() runners.RunnerEnvironment {
64-
s.activeEnvMu.RLock()
65-
defer s.activeEnvMu.RUnlock()
66-
return s.activeEnv
61+
// ActiveEnvironment returns the environment a specialization published, or
62+
// nil when none is active — before Init, or after it tore the environment down.
63+
func (s *Service) ActiveEnvironment() runners.RunnerEnvironment {
64+
s.activeEnvironmentMu.RLock()
65+
defer s.activeEnvironmentMu.RUnlock()
66+
return s.activeEnvironment
6767
}
6868

69-
// SetActiveEnv publishes the specialization's active RunnerEnvironment.
69+
// SetActiveEnvironment publishes the specialization's active RunnerEnvironment.
7070
// Pass nil once the environment is shut down: readers then fall back to a
7171
// standalone environment instead of spawning into a dead one.
72-
func (s *Service) SetActiveEnv(env runners.RunnerEnvironment) {
73-
s.activeEnvMu.Lock()
74-
defer s.activeEnvMu.Unlock()
75-
s.activeEnv = env
72+
func (s *Service) SetActiveEnvironment(env runners.RunnerEnvironment) {
73+
s.activeEnvironmentMu.Lock()
74+
defer s.activeEnvironmentMu.Unlock()
75+
s.activeEnvironment = env
7676
}
7777

7878
// New builds a generic Python Service bound to the given agent manifest.

‎pkg/service/service_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,34 +116,34 @@ func TestGetAgentInformationGeneric(t *testing.T) {
116116
}
117117
}
118118

119-
// TestActiveEnvPublishAndClear locks in the contract specializations use:
119+
// TestActiveEnvironmentPublishAndClear locks in the contract specializations use:
120120
// what Init publishes is what Code, Tooling and the REPL read, and what Stop
121121
// clears sends them back to the standalone fallback.
122-
func TestActiveEnvPublishAndClear(t *testing.T) {
122+
func TestActiveEnvironmentPublishAndClear(t *testing.T) {
123123
svc := pythonservice.New(&resources.Agent{Kind: "codefly:service", Name: "python"})
124-
if svc.ActiveEnv() != nil {
124+
if svc.ActiveEnvironment() != nil {
125125
t.Fatal("a Service with no specialization environment must read nil")
126126
}
127127

128128
env, err := runners.NewNativeEnvironment(context.Background(), t.TempDir())
129129
if err != nil {
130130
t.Fatalf("native environment: %v", err)
131131
}
132-
svc.SetActiveEnv(env)
133-
if svc.ActiveEnv() != env {
132+
svc.SetActiveEnvironment(env)
133+
if svc.ActiveEnvironment() != env {
134134
t.Fatal("published environment is not the one readers get")
135135
}
136136

137-
svc.SetActiveEnv(nil)
138-
if svc.ActiveEnv() != nil {
137+
svc.SetActiveEnvironment(nil)
138+
if svc.ActiveEnvironment() != nil {
139139
t.Fatal("cleared environment must read nil, not a shut-down one")
140140
}
141141
}
142142

143-
// TestActiveEnvUnderConcurrentPublishAndRead is the -race guard for the
143+
// TestActiveEnvironmentUnderConcurrentPublishAndRead is the -race guard for the
144144
// field itself: a specialization publishes from Init and clears from Stop
145145
// while Code, Tooling and the REPL read on their own gRPC goroutines.
146-
func TestActiveEnvUnderConcurrentPublishAndRead(t *testing.T) {
146+
func TestActiveEnvironmentUnderConcurrentPublishAndRead(t *testing.T) {
147147
svc := pythonservice.New(&resources.Agent{Kind: "codefly:service", Name: "python"})
148148
env, err := runners.NewNativeEnvironment(context.Background(), t.TempDir())
149149
if err != nil {
@@ -155,16 +155,16 @@ func TestActiveEnvUnderConcurrentPublishAndRead(t *testing.T) {
155155
go func() {
156156
defer wg.Done()
157157
for i := 0; i < 1000; i++ {
158-
svc.SetActiveEnv(env)
159-
svc.SetActiveEnv(nil)
158+
svc.SetActiveEnvironment(env)
159+
svc.SetActiveEnvironment(nil)
160160
}
161161
}()
162162
for readers := 0; readers < 3; readers++ {
163163
wg.Add(1)
164164
go func() {
165165
defer wg.Done()
166166
for i := 0; i < 1000; i++ {
167-
if got := svc.ActiveEnv(); got != nil && got != env {
167+
if got := svc.ActiveEnvironment(); got != nil && got != env {
168168
t.Errorf("read an environment that was never published: %#v", got)
169169
return
170170
}

0 commit comments

Comments
 (0)