1
0
forked from wrenn/wrenn

Fix expandEnv regex, init script crash, healthcheck deadline, and test issues

- Fix envRegex: remove spurious (\$)? group that swallowed $$$, handle ${}
- wrenn-init.sh: add || true to networking commands under set -e, remove dead code
- waitForHealthcheck: use context deadline for unlimited retries instead of implicit 100 cap
- Make parseSandboxEnv a package-level function (unused receiver)
- Fix WrappedCommand test: map iteration order dependency, pre-expand env values
- Fix error wrapping: %v → %w per project conventions
- test-jupyter-kernel.py: move import to top-level, fix misleading comment
This commit is contained in:
2026-04-08 02:14:38 +06:00
parent 11e08e5b96
commit 0ea0e7cc70
5 changed files with 51 additions and 53 deletions

View File

@ -13,10 +13,10 @@ type ExecContext struct {
}
// This regex matches:
// 1. $$
// 2. ${ANY_WORD}
// 3. $ANY_WORD
var envRegex = regexp.MustCompile(`\$\$(\$)?|\$\{([a-zA-Z0-9_]+)\}|\$([a-zA-Z0-9_]+)`)
// 1. $$ (escaped dollar)
// 2. ${VAR} or ${} (braced variable, possibly empty)
// 3. $VAR (bare variable)
var envRegex = regexp.MustCompile(`\$\$|\$\{([a-zA-Z0-9_]*)\}|\$([a-zA-Z0-9_]+)`)
// WrappedCommand returns the full shell command for a RUN step with context
// applied. The result is passed as the argument to /bin/sh -c.
@ -77,7 +77,7 @@ func expandEnv(s string, vars map[string]string) string {
}
var name string
if match[1] == '{' {
if len(match) > 1 && match[1] == '{' {
name = match[2 : len(match)-1]
} else {
name = match[1:]

View File

@ -4,10 +4,11 @@ import "testing"
func TestExecContext_WrappedCommand(t *testing.T) {
tests := []struct {
name string
ctx ExecContext
cmd string
want string
name string
ctx ExecContext
cmd string
want string
wantOneOf []string
}{
{
name: "no context",
@ -46,19 +47,34 @@ func TestExecContext_WrappedCommand(t *testing.T) {
want: "MSG='it'\\''s fine' /bin/sh -c 'echo $MSG'",
},
{
name: "env expansion with dollar sign PATH",
name: "env expansion with pre-expanded PATH",
ctx: ExecContext{
EnvVars: map[string]string{"PATH": "/usr/bin", "FOO": "/opt/venv/bin:$PATH"},
EnvVars: map[string]string{"PATH": "/usr/bin", "FOO": "/opt/venv/bin:/usr/bin"},
},
cmd: "make build",
// Map iteration order is non-deterministic; accept either ordering.
wantOneOf: []string{
"FOO='/opt/venv/bin:/usr/bin' PATH='/usr/bin' /bin/sh -c 'make build'",
"PATH='/usr/bin' FOO='/opt/venv/bin:/usr/bin' /bin/sh -c 'make build'",
},
cmd: "make build",
want: "FOO='/opt/venv/bin:/usr/bin' PATH='/usr/bin' /bin/sh -c 'make build'",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := tc.ctx.WrappedCommand(tc.cmd)
if got != tc.want {
if len(tc.wantOneOf) > 0 {
matched := false
for _, w := range tc.wantOneOf {
if got == w {
matched = true
break
}
}
if !matched {
t.Errorf("WrappedCommand(%q)\n got %q\n want one of %q", tc.cmd, got, tc.wantOneOf)
}
} else if got != tc.want {
t.Errorf("WrappedCommand(%q)\n got %q\n want %q", tc.cmd, got, tc.want)
}
})

View File

@ -464,15 +464,18 @@ func (s *BuildService) executeBuild(ctx context.Context, buildIDStr string) {
// Returns nil on the first successful check, or an error if retries are
// exhausted, the deadline passes, or the context is cancelled.
func (s *BuildService) waitForHealthcheck(ctx context.Context, agent buildAgentClient, sandboxIDStr string, hc recipe.HealthcheckConfig) error {
maxAttempts := 100
if hc.Retries > 0 {
maxAttempts = hc.Retries
}
deadline := time.NewTimer(hc.StartPeriod + time.Duration(maxAttempts+1)*hc.Interval)
defer deadline.Stop()
ticker := time.NewTicker(hc.Interval)
defer ticker.Stop()
// When retries > 0, set a deadline based on the retry budget.
// When retries == 0 (unlimited), rely solely on the parent context deadline.
var deadlineCh <-chan time.Time
if hc.Retries > 0 {
deadline := time.NewTimer(hc.StartPeriod + time.Duration(hc.Retries+1)*hc.Interval)
defer deadline.Stop()
deadlineCh = deadline.C
}
startedAt := time.Now()
failCount := 0
@ -480,7 +483,7 @@ func (s *BuildService) waitForHealthcheck(ctx context.Context, agent buildAgentC
select {
case <-ctx.Done():
return ctx.Err()
case <-deadline.C:
case <-deadlineCh:
return fmt.Errorf("healthcheck timed out: exceeded %d attempts over %s", failCount, time.Since(startedAt))
case <-ticker.C:
execCtx, cancel := context.WithTimeout(ctx, hc.Timeout)
@ -497,7 +500,7 @@ func (s *BuildService) waitForHealthcheck(ctx context.Context, agent buildAgentC
if time.Since(startedAt) >= hc.StartPeriod {
failCount++
if hc.Retries > 0 && failCount >= hc.Retries {
return fmt.Errorf("healthcheck failed after %d retries: exec error: %v", failCount, err)
return fmt.Errorf("healthcheck failed after %d retries: exec error: %w", failCount, err)
}
}
continue
@ -574,14 +577,14 @@ func (s *BuildService) fetchSandboxEnv(ctx context.Context,
resp.Msg.ExitCode)
}
return s.parseSandboxEnv(string(resp.Msg.Stdout)), nil
return parseSandboxEnv(string(resp.Msg.Stdout)), nil
}
// parseSandboxEnv converts the raw newline-separated output of an 'env'
// command into a map.
// It skips empty lines and malformed entries, and correctly handles value
// It skips empty lines and malformed entries, and correctly handles values
// containing '='.
func (s *BuildService) parseSandboxEnv(raw string) map[string]string {
func parseSandboxEnv(raw string) map[string]string {
envVars := make(map[string]string)
for line := range strings.SplitSeq(raw, "\n") {