Skip to content

Commit 804e09f

Browse files
fix(shellwrap): set docker source on stat-probe cache upgrade (Codex P2)
ResolveDockerPath's MCP-2744 stat-probe override (upgrading a live cached negative to a permanent success when the well-known-path probe finds docker) updated dockerPath/dockerPathErr but not dockerPathSource. A stale 'absent' from the prior failed resolution then leaked into docker_cli_source telemetry (schema v5) even though docker WAS resolved via the bundled path. Set dockerPathSource = DockerSourceBundled in that branch. Regression test asserts a cached negative upgraded via the stat probe reports 'bundled', not 'absent' (fails without the one-line fix). Related #696 Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent 5aa843d commit 804e09f

2 files changed

Lines changed: 46 additions & 0 deletions

File tree

internal/shellwrap/shellwrap.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,11 @@ func ResolveDockerPath(logger *zap.Logger) (string, error) {
245245
!dockerPathExpires.IsZero() && time.Now().Before(dockerPathExpires) {
246246
if p := probeWellKnownDocker(logger); p != "" {
247247
dockerPath = p
248+
// The override resolves via the well-known-path probe, so the
249+
// source is "bundled". Must be set here too, else a stale "absent"
250+
// from the prior failed resolution leaks into docker_cli_source
251+
// telemetry on the next ResolveDockerSource call (schema v5).
252+
dockerPathSource = DockerSourceBundled
248253
dockerPathErr = nil
249254
dockerPathExpires = time.Time{}
250255
return p, nil

internal/shellwrap/shellwrap_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,47 @@ func TestResolveDockerSource(t *testing.T) {
352352
})
353353
}
354354

355+
// TestResolveDockerSource_StatProbeUpgradeReportsBundled is the regression
356+
// guard for the MCP-2744 cache-upgrade path: when a live cached NEGATIVE is
357+
// overridden by the well-known-path stat probe, the cached source must be
358+
// upgraded to "bundled" too — otherwise the stale "absent" leaks into the
359+
// schema-v5 docker_cli_source telemetry even though docker WAS resolved.
360+
func TestResolveDockerSource_StatProbeUpgradeReportsBundled(t *testing.T) {
361+
if runtime.GOOS == "windows" {
362+
t.Skip("unix-only fixture")
363+
}
364+
resetDockerPathCacheForTest()
365+
t.Cleanup(resetDockerPathCacheForTest)
366+
367+
// FIRST resolution fails everywhere → caches a live negative whose source
368+
// is "absent". Keep the negative TTL at its default so it stays live.
369+
prevPaths := wellKnownDockerPathsFn
370+
wellKnownDockerPathsFn = func() []string { return nil }
371+
t.Cleanup(func() { wellKnownDockerPathsFn = prevPaths })
372+
373+
t.Setenv("PATH", t.TempDir())
374+
shellDir := t.TempDir()
375+
t.Setenv("SHELL", writeFakeShell(t, shellDir, ""))
376+
377+
_, err := ResolveDockerPath(nil)
378+
require.Error(t, err, "first call should cache a negative (docker absent everywhere)")
379+
require.Equal(t, DockerSourceAbsent, ResolveDockerSource(nil), "cached negative reports absent")
380+
381+
// Docker now appears at a well-known path. The stat-probe override must
382+
// upgrade BOTH the path and the source on the next call.
383+
dockerDir := t.TempDir()
384+
want := writeFakeDocker(t, dockerDir)
385+
wellKnownDockerPathsFn = func() []string { return []string{want} }
386+
387+
got, err := ResolveDockerPath(nil)
388+
require.NoError(t, err, "stat probe must override the live negative")
389+
require.Equal(t, want, got)
390+
// The bug: this returned "absent" before the fix because the upgrade path
391+
// never updated dockerPathSource.
392+
require.Equal(t, DockerSourceBundled, ResolveDockerSource(nil),
393+
"source must upgrade to bundled when the stat probe overrides the cached negative")
394+
}
395+
355396
func TestMinimalEnv_DropsSecrets(t *testing.T) {
356397
t.Setenv("AWS_ACCESS_KEY_ID", "AKIA_test_dummy_value_00000000")
357398
t.Setenv("GITHUB_TOKEN", "ghp_dummy_test_token_1234567890abcdef")

0 commit comments

Comments
 (0)