refactor: drop comments that restate the code

cmd/devtool/main.go

@@ -1,8 +1,3 @@
-// Command devtool is the repo's CI/dev helper, replacing the former scripts/*.sh.
-// Its commands live in internal/devtools (devtools.Main), which wires stdin/stdout,
-// git, `go tool`, and docker around the pure, unit-tested logic there. Invoked
-// from the Taskfile and the CI workflow (e.g. `go run ./cmd/devtool coverage-gate
-// coverage.out TOTAL=48`).
 package main
 
 import (

cmd/devtool/main_test.go

@@ -8,16 +8,13 @@ import (
 	"testing"
 )
 
-// TestMain runs the command itself, instead of the tests, in the child process
-// TestTheCommandIsWired starts.
 func TestMain(m *testing.M) {
 	if os.Getenv("DEVTOOL_RUN_MAIN") == "1" {
 		main()
 	}
 	os.Exit(m.Run())
 }
 
-// devtool runs this test binary as the devtool command, with args and stdin.
 func devtool(t *testing.T, stdin string, args ...string) (code int, stdout, stderr string) {
 	t.Helper()
 	cmd := exec.Command(os.Args[0], args...)
@@ -35,8 +32,6 @@ func devtool(t *testing.T, stdin string, args ...string) (code int, stdout, stde
 	return cmd.ProcessState.ExitCode(), o.String(), e.String()
 }
 
-// TestTheCommandIsWired holds what main adds to devtools.Main: the process's
-// arguments, standard streams and exit code.
 func TestTheCommandIsWired(t *testing.T) {
 	if code, out, errOut := devtool(t, "internal/cli/exec.go\n", "parity-affected"); code != 0 || out != "exec\n" || errOut != "" {
 		t.Errorf("parity-affected: code=%d stdout=%q stderr=%q, want 0 \"exec\\n\" \"\"", code, out, errOut)

internal/cli/build_auth.go

@@ -7,14 +7,7 @@ import (
 	"github.com/devcontainers/cli/internal/oci"
 )
 
-// bridgeBuildAuth resolves credentials for every registry a build touches (base
-// image, push tags, cache import/export) via the CLI's credential chain and, when
-// any resolve, returns the subprocess env (DOCKER_CONFIG=<tmp>) plus a cleanup
-// func. It returns (nil, no-op) when there is nothing to bridge, leaving the
-// build's ambient auth untouched. Errors are swallowed (best-effort): a build
-// that would have worked anonymously must not fail because auth bridging did.
 func bridgeBuildAuth(logger log.Logger, baseImage string, tags, cacheFrom []string, cacheTo string) (env []string, cleanup func()) {
-	// With no registry to bridge, ResolveBuildAuth resolves nothing and is not ok.
 	regs := collectBuildRegistries(baseImage, tags, cacheFrom, cacheTo)
 	dir, clean, ok, err := oci.ResolveBuildAuth(osEnvMap(), regs, logger)
 	if err != nil || !ok {
@@ -23,7 +16,6 @@ func bridgeBuildAuth(logger log.Logger, baseImage string, tags, cacheFrom []stri
 	return []string{"DOCKER_CONFIG=" + dir}, clean
 }
 
-// collectBuildRegistries returns the deduped registry hosts referenced by a build.
 func collectBuildRegistries(baseImage string, tags, cacheFrom []string, cacheTo string) []string {
 	var out []string
 	seen := map[string]bool{}
@@ -64,9 +56,6 @@ func registryHostFromImageRef(ref string) string {
 	return ""
 }
 
-// registryCacheRef pulls the image reference out of a buildx cache spec. A
-// registry cache is written as "type=registry,ref=<image>[,...]"; anything else
-// (type=local/inline/gha, or a bare image) is returned as-is.
 func registryCacheRef(spec string) string {
 	spec = strings.TrimSpace(spec)
 	if spec == "" || !strings.Contains(spec, "=") {

internal/cli/build_auth_held_test.go

@@ -9,8 +9,6 @@ import (
 	"github.com/devcontainers/cli/internal/log"
 )
 
-// isolateBuildAuth points the credential chain at an empty docker config, so
-// only DEVCONTAINERS_OCI_AUTH can supply credentials.
 func isolateBuildAuth(t *testing.T, ociAuth string) {
 	t.Helper()
 	dir := t.TempDir()
@@ -22,8 +20,6 @@ func isolateBuildAuth(t *testing.T, ociAuth string) {
 	t.Setenv("DEVCONTAINERS_OCI_AUTH", ociAuth)
 }
 
-// A build whose registries have no credentials keeps its ambient auth: no
-// DOCKER_CONFIG is handed to the build.
 func TestBridgeBuildAuthWithoutCredentials(t *testing.T) {
 	isolateBuildAuth(t, "")
 	env, cleanup := bridgeBuildAuth(log.Null, "reg.example.com/base:1", nil, nil, "")
@@ -33,7 +29,6 @@ func TestBridgeBuildAuthWithoutCredentials(t *testing.T) {
 	}
 }
 
-// Credentials that resolve are bridged through a temporary DOCKER_CONFIG.
 func TestBridgeBuildAuthWithCredentials(t *testing.T) {
 	isolateBuildAuth(t, "reg.example.com|alice|secret")
 	env, cleanup := bridgeBuildAuth(log.Null, "reg.example.com/base:1", nil, nil, "")

internal/cli/build_auth_test.go

@@ -12,12 +12,11 @@ func TestRegistryHostFromImageRef(t *testing.T) {
 		"myreg.example.com:5000/team/app":               "myreg.example.com:5000",
 		"localhost:5000/app":                            "localhost:5000",
 		"123456.dkr.ecr.us-east-1.amazonaws.com/repo:1": "123456.dkr.ecr.us-east-1.amazonaws.com",
-		// Docker Hub forms → no explicit registry → "" (ambient auth applies).
-		"ubuntu:22.04":   "",
-		"library/ubuntu": "",
-		"org/img:tag":    "",
-		"mcr":            "",
-		"":               "",
+		"ubuntu:22.04":                                  "",
+		"library/ubuntu":                                "",
+		"org/img:tag":                                   "",
+		"mcr":                                           "",
+		"":                                              "",
 	}
 	for ref, want := range cases {
 		if got := registryHostFromImageRef(ref); got != want {
@@ -30,9 +29,9 @@ func TestRegistryCacheRef(t *testing.T) {
 	cases := map[string]string{
 		"type=registry,ref=ghcr.io/org/cache:main": "ghcr.io/org/cache:main",
 		"type=registry,ref=myreg.com/c,mode=max":   "myreg.com/c",
-		"type=local,dest=/tmp/c":                   "", // not a registry cache
+		"type=local,dest=/tmp/c":                   "",
 		"type=inline":                              "",
-		"ghcr.io/org/cache:main":                   "ghcr.io/org/cache:main", // bare ref
+		"ghcr.io/org/cache:main":                   "ghcr.io/org/cache:main",
 		"":                                         "",
 	}
 	for spec, want := range cases {
@@ -44,10 +43,10 @@ func TestRegistryCacheRef(t *testing.T) {
 
 func TestCollectBuildRegistries(t *testing.T) {
 	got := collectBuildRegistries(
-		"ghcr.io/base/img:1", // base
-		[]string{"ghcr.io/base/img:1", "reg.example.com/app:2", "ubuntu"},            // tags (dup base, hub)
-		[]string{"type=registry,ref=cache.example.com/c:main", "type=local,dest=/x"}, // cacheFrom
-		"type=registry,ref=cache.example.com/c:main",                                 // cacheTo (dup of cacheFrom)
+		"ghcr.io/base/img:1",
+		[]string{"ghcr.io/base/img:1", "reg.example.com/app:2", "ubuntu"},
+		[]string{"type=registry,ref=cache.example.com/c:main", "type=local,dest=/x"},
+		"type=registry,ref=cache.example.com/c:main",
 	)
 	want := []string{"ghcr.io", "reg.example.com", "cache.example.com"}
 	if !reflect.DeepEqual(got, want) {
@@ -56,7 +55,6 @@ func TestCollectBuildRegistries(t *testing.T) {
 }
 
 func TestCollectBuildRegistriesEmpty(t *testing.T) {
-	// Only Docker Hub / local caches → nothing to bridge.
 	if got := collectBuildRegistries("ubuntu:22.04", []string{"org/app"}, []string{"type=inline"}, ""); len(got) != 0 {
 		t.Fatalf("expected no registries, got %v", got)
 	}

internal/cli/build_cachefrom_test.go

@@ -7,9 +7,6 @@ import (
 	"github.com/devcontainers/cli/internal/config"
 )
 
-// TestCacheFromForDockerfileBuild pins the merge of the --cache-from flag with
-// devcontainer.json build.cacheFrom: flag values first, then config values, in
-// order — matching TS singleContainer (additionalCacheFroms, then config.build.cacheFrom).
 func TestCacheFromForDockerfileBuild(t *testing.T) {
 	cfgWith := func(cf ...string) *config.DevContainer {
 		return &config.DevContainer{Build: &config.Build{CacheFrom: config.StringOrStrings(cf)}}
@@ -38,9 +35,6 @@ func TestCacheFromForDockerfileBuild(t *testing.T) {
 	}
 }
 
-// TestCacheFromForDockerfileBuildDoesNotMutateFlag guards against the helper
-// aliasing/appending into the caller's flag slice (which extendImageWithFeatures
-// still passes verbatim for the feature layers).
 func TestCacheFromForDockerfileBuildDoesNotMutateFlag(t *testing.T) {
 	flagBacking := []string{"flag1", "sentinel"}
 	flag := flagBacking[:1]

internal/cli/build_compose_path_test.go

@@ -9,13 +9,6 @@ import (
 	"github.com/devcontainers/cli/internal/log"
 )
 
-// TestBuildComposePathWiring verifies that the `build` command threads
-// --docker-compose-path into docker.NewComposeClient. When the call site
-// hardcodes "" instead, the operator-supplied path could never appear in the
-// compose-client detection error ("neither 'docker compose' nor '<path>'
-// found"). The test is hermetic: dockerPath points at a nonexistent binary so
-// the `docker compose` (v2) probe always fails regardless of the host, forcing
-// NewComposeClient down the custom-path branch.
 func TestBuildComposePathWiring(t *testing.T) {
 	tests := []struct {
 		name              string
@@ -39,8 +32,6 @@ func TestBuildComposePathWiring(t *testing.T) {
 			r := &buildRunner{
 				log: log.New(log.Options{Writer: io.Discard}),
 				opts: &buildOpts{
-					// Force the `docker compose` (v2) probe to fail hermetically,
-					// independent of whether Docker is installed on the host.
 					dockerPath:        "/nonexistent/docker-binary",
 					dockerComposePath: tt.dockerComposePath,
 				},

internal/cli/build_held_test.go

@@ -17,8 +17,6 @@ import (
 
 func discardLogger() log.Logger { return log.New(log.Options{Writer: io.Discard}) }
 
-// The deprecated --experimental-frozen-lockfile and --frozen-lockfile both
-// make the Feature build verify the lockfile, as TS effectiveFrozenLockfile.
 func TestBuildFrozenLockfileReachesTheFeatureBuild(t *testing.T) {
 	for _, tc := range []struct {
 		name                 string
@@ -187,9 +185,6 @@ func TestBuildxSecrets(t *testing.T) {
 	}
 }
 
-// --buildkit never skips BuildKit detection, so the build runs as if buildx
-// were absent even when the Docker CLI has it. The fake --docker-path reports
-// buildx, and both cases stop at the flag check before any Docker build.
 func TestRunBuildBuildKitNeverIgnoresBuildx(t *testing.T) {
 	bin := t.TempDir()
 	docker := filepath.Join(bin, "docker")
@@ -227,9 +222,6 @@ func TestRunBuildBuildKitNeverIgnoresBuildx(t *testing.T) {
 	}
 }
 
-// The Dockerfile build targets the config's target when it names one, else
-// the final stage, named when it has no name; a Dockerfile without FROM is
-// TS's error.
 func TestPrepareDockerfileBuildStage(t *testing.T) {
 	ws := writeConfigFixture(t, map[string]string{
 		".devcontainer/Dockerfile": "FROM alpine:3 AS base\nFROM base\n",
@@ -257,9 +249,6 @@ func TestPrepareDockerfileBuildStage(t *testing.T) {
 	}
 }
 
-// A Compose build without --image-name reports its image as a string, as TS
-// doBuild does (imageNameResult = originalImageName); with --image-name, and
-// for any other configuration, the names are an array.
 func TestBuildResultImageName(t *testing.T) {
 	for _, tc := range []struct {
 		name     string

internal/cli/build_nodocker_held_test.go

@@ -7,9 +7,6 @@ import (
 	"testing"
 )
 
-// A build whose image or Dockerfile step fails before any Docker call reports
-// the failure on stdout and exits 1, never the success envelope: a legacy
-// Feature id as the bare message, any other failure as a container error.
 func TestRunBuildReportsAFailedBuildStep(t *testing.T) {
 	for name, tc := range map[string]struct {
 		config string

internal/cli/build_repo2_held_test.go

@@ -10,9 +10,6 @@ import (
 	"github.com/devcontainers/cli/internal/config"
 )
 
-// The Dockerfile written to name the final stage sits next to the user's with
-// a copy of its .dockerignore, and Cleanup removes both and leaves the user's
-// files alone.
 func TestPrepareDockerfileBuildCleanupRemovesTheWrittenFiles(t *testing.T) {
 	dir := t.TempDir()
 	dockerfile := filepath.Join(dir, "Dockerfile")

internal/cli/build_repo_held_test.go

@@ -10,8 +10,6 @@ import (
 	"github.com/devcontainers/cli/internal/config"
 )
 
-// buildConfigNotFound runs build with args and returns the path its "config
-// not found" error names.
 func buildConfigNotFound(t *testing.T, args ...string) string {
 	t.Helper()
 	cmd := newBuildCmd()
@@ -31,8 +29,6 @@ func buildConfigNotFound(t *testing.T, args ...string) string {
 	return got.Message
 }
 
-// build [path] builds the workspace the argument names, not the current
-// directory; --workspace-folder wins over the argument.
 func TestBuildPositionalPathIsTheWorkspace(t *testing.T) {
 	ws := t.TempDir()
 	t.Chdir(t.TempDir())
@@ -45,8 +41,6 @@ func TestBuildPositionalPathIsTheWorkspace(t *testing.T) {
 	}
 }
 
-// A build target names the stage to build, and the Dockerfile is used as it
-// is, with no stage name added.
 func TestPrepareDockerfileBuildUsesTheConfiguredTarget(t *testing.T) {
 	dir := t.TempDir()
 	dockerfile := filepath.Join(dir, "Dockerfile")
@@ -67,9 +61,6 @@ func TestPrepareDockerfileBuildUsesTheConfiguredTarget(t *testing.T) {
 	}
 }
 
-// Without a target, the final stage gets a name to build, in a Dockerfile
-// written next to the user's, whether the Dockerfile is named under build or
-// at the top level.
 func TestPrepareDockerfileBuildNamesTheFinalStageWithoutATarget(t *testing.T) {
 	for name, cfg := range map[string]config.DevContainer{
 		"build":     {Build: &config.Build{Dockerfile: "Dockerfile"}},

internal/cli/cache_key.go

@@ -11,18 +11,11 @@ import (
 	"github.com/devcontainers/cli/internal/features"
 )
 
-// proxyEnvKeys are the proxy-related environment variables folded into the cache
-// key: they materially change what a build produces (proxies baked into package
-// installs), so two otherwise-identical configs built behind different proxies
-// must get different keys.
 var proxyEnvKeys = []string{
 	"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "FTP_PROXY", "ALL_PROXY",
 	"http_proxy", "https_proxy", "no_proxy", "ftp_proxy", "all_proxy",
 }
 
-// cacheKeyInput is the canonical, deterministic pre-image of the cache key. Field
-// order is fixed and every nested map is marshaled by encoding/json with sorted
-// keys, so the same inputs always hash to the same value.
 type cacheKeyInput struct {
 	Config     json.RawMessage   `json:"config"`
 	Dockerfile string            `json:"dockerfile,omitempty"`
@@ -31,14 +24,6 @@ type cacheKeyInput struct {
 	Proxy      map[string]string `json:"proxy,omitempty"`
 }
 
-// computeCacheKey returns a deterministic, content-addressed cache key for a
-// resolved configuration: sha256 over the normalized devcontainer.json, the
-// Dockerfile (when Dockerfile-based), the features lockfile (when present, which
-// pins resolved digests), the build context path, and the proxy environment.
-//
-// It is hermetic — no network, no registry resolution — so the same inputs on
-// any host yield the same key. Feature refs are hashed as declared; pin them (via
-// @sha256 or a committed lockfile) for the key to track the exact feature bits.
 func computeCacheKey(result *config.LoadResult, env map[string]string) (string, error) {
 	cfg := result.Config
 	cfgJSON, err := json.Marshal(cfg)
@@ -48,21 +33,13 @@ func computeCacheKey(result *config.LoadResult, env map[string]string) (string,
 	in := cacheKeyInput{Config: cfgJSON, Context: cfg.BuildContext()}
 
 	if cfg.Dockerfile() != "" {
-		// The Dockerfile build reads (prepareDockerfileBuild). One that cannot
-		// be read fails the key, which would otherwise not track the image's
-		// recipe.
 		content, err := os.ReadFile(dockerfileConfigPath(cfg))
 		if err != nil {
 			return "", fmt.Errorf("read Dockerfile: %w", err)
 		}
 		in.Dockerfile = string(content)
 	}
 
-	// ReadLockfile's bool reports an empty marker file, which pins nothing; a
-	// lockfile with content comes back as a non-nil lock. A lockfile that
-	// cannot be read or parsed fails the key, as it fails every reference
-	// command that reads it (lockfile.ts readLockfile rethrows all but ENOENT):
-	// a key computed without it would not track the pinned digests.
 	lock, _, err := features.ReadLockfile(cfg.ConfigFilePath)
 	if err != nil {
 		return "", fmt.Errorf("read features lockfile %s: %w", features.LockfilePath(cfg.ConfigFilePath), err)
@@ -93,8 +70,6 @@ func computeCacheKey(result *config.LoadResult, env map[string]string) (string,
 	return "sha256:" + hex.EncodeToString(sum[:]), nil
 }
 
-// proxyEnvFromEnviron snapshots only the proxy-related variables from the process
-// environment (keeps computeCacheKey injectable in tests).
 func proxyEnvFromEnviron() map[string]string {
 	m := map[string]string{}
 	for _, k := range proxyEnvKeys {

internal/cli/cache_key_repo_held_test.go

@@ -9,9 +9,6 @@ import (
 	"github.com/devcontainers/cli/internal/config"
 )
 
-// A committed features lockfile pins resolved digests, so changing it changes
-// the cache key (docs/go-only-features.md, read-configuration --cache-key); an
-// empty lockfile, which pins nothing, keys like no lockfile at all.
 func TestComputeCacheKeyFoldsInTheLockfile(t *testing.T) {
 	dir := t.TempDir()
 	cfgPath := filepath.Join(dir, "devcontainer.json")
@@ -51,8 +48,6 @@ func TestComputeCacheKeyFoldsInTheLockfile(t *testing.T) {
 	}
 }
 
-// A proxy variable that is set but empty configures no proxy, so it keys like
-// an unset one.
 func TestComputeCacheKeyIgnoresEmptyProxy(t *testing.T) {
 	cfgPath := filepath.Join(t.TempDir(), "devcontainer.json")
 	base, err := computeCacheKey(imageResult("ubuntu:22.04", cfgPath), nil)
@@ -64,9 +59,6 @@ func TestComputeCacheKeyIgnoresEmptyProxy(t *testing.T) {
 	}
 }
 
-// A lockfile that is not JSON fails the key instead of keying as if no
-// lockfile were committed: the reference's readLockfile rethrows the parse
-// error, and a key without the lockfile would not track its pinned digests.
 func TestComputeCacheKeyFailsOnAMalformedLockfile(t *testing.T) {
 	dir := t.TempDir()
 	cfgPath := filepath.Join(dir, "devcontainer.json")
@@ -83,8 +75,6 @@ func TestComputeCacheKeyFailsOnAMalformedLockfile(t *testing.T) {
 	}
 }
 
-// A lockfile that exists but cannot be read fails the key too: only a missing
-// lockfile (ENOENT) means none is committed.
 func TestComputeCacheKeyFailsOnAnUnreadableLockfile(t *testing.T) {
 	dir := t.TempDir()
 	cfgPath := filepath.Join(dir, "devcontainer.json")
@@ -97,8 +87,6 @@ func TestComputeCacheKeyFailsOnAnUnreadableLockfile(t *testing.T) {
 	}
 }
 
-// A Dockerfile-based configuration whose Dockerfile cannot be read fails the
-// key instead of keying as if it had no Dockerfile, naming the file.
 func TestComputeCacheKeyFailsOnAMissingDockerfile(t *testing.T) {
 	dir := t.TempDir()
 	r := &config.LoadResult{Config: &config.DevContainer{