diff --git a/.claude/skills/changelog/SKILL.md b/.claude/skills/changelog/SKILL.md index 2e97a682554..3062486380b 100644 --- a/.claude/skills/changelog/SKILL.md +++ b/.claude/skills/changelog/SKILL.md @@ -84,6 +84,28 @@ problem or technique first, the way someone outside the project would recognize Structure the body `## The Problem` / `## The Fix` / `## How to Use It` / `## Get Involved`. +### Rule 1a — Open on the real reason, at the scope it actually applies to + +Find the actual motivating reason for the change (PR description, linked issue, commit messages) before +writing the intro, and open on *that* — not a plausible-sounding scenario constructed to fit it, and not +narrowed to the one path you happened to notice it through when the real gap is broader. Both are the same +mistake: substituting a specific, contrived framing for the real, general one. + +- **Correct** — `2026-07-13-atmos-stack-schema-command.mdx`: "Editors, CI pipelines, and offline + environments that want to validate stack manifests locally have had one option: fetch the JSON Schema + from `atmos.tools`... and hope it matches." A real, checkable limitation, not an anecdote. +- **Violation (invented)** — `2026-08-06-toolchain-lockfile-default.mdx` opened with a fabricated "a + teammate's laptop and CI don't quite match" vignette, when the real reason (stated correctly two + paragraphs later) was simpler: the fix already existed but was undocumented, so nobody enabled it. +- **Violation (over-narrowed)** — `2026-08-05-taskfile-convergence.mdx` opens "If you've ever tried to move + a `Taskfile.yml` over to Atmos, you've hit the gap..." — framing a general task-runner deficiency (no + dependency ordering, no incremental builds — table-stakes features nearly every task runner has) as if it + only matters to people migrating from one specific competitor. The real problem, stated correctly under + `## The Problem`, is category-general: Atmos was missing it as a task runner, full stop. + +If you can't find the real reason, ask rather than invent one — and state it at the scope it actually +applies to. + ## Rule 2 — Never open prose with a backtick Prose (a sentence, paragraph, or the post intro) must start with a word, not an inline code span or fence. @@ -130,6 +152,8 @@ implementation structure — describe behavior only in CLI/config/output terms. ## Pre-publish checklist - [ ] Intro opens on the problem, not the feature, and doesn't open with a backtick +- [ ] The opening problem is the real, specific reason this change happened (checked against the PR + description/issue/commits) — not a generic scenario invented to justify it - [ ] Body follows Problem → Fix → How to Use It → Get Involved (no `## What Changed` opener) - [ ] Tag(s) exist in `website/blog/tags.yml` - [ ] Author exists in `website/blog/authors.yml` (added in this PR if new) diff --git a/.github/actions/go-mod-download-retry/action.yml b/.github/actions/go-mod-download-retry/action.yml new file mode 100644 index 00000000000..52e96207083 --- /dev/null +++ b/.github/actions/go-mod-download-retry/action.yml @@ -0,0 +1,38 @@ +name: Go Mod Download With Retry +description: > + Runs `go mod download`, retrying on transient proxy.golang.org failures. A + request that gets reset mid-stream (e.g. "stream error: stream ID ; + INTERNAL_ERROR; received from peer") is not retried by `go mod download` + itself, and GOPROXY's `|direct` fallback only helps when the proxy is + unreachable outright, not a mid-stream reset. Makes up to three attempts + with a cooldown between each, matching the convention already used for + artifact downloads (.github/actions/download-artifact-retry) and + magefiles/build.go's runGoModDownload. See + docs/fixes/2026-08-25-build-atmos-go-mod-download-retry.md. + +inputs: + backoff-seconds: + description: 'Seconds to wait before each retry' + required: false + default: '15' + +runs: + using: composite + steps: + - name: go mod download (with retry) + shell: bash + env: + BACKOFF_SECONDS: ${{ inputs.backoff-seconds }} + run: | + set -euo pipefail + attempt=1 + max_attempts=3 + until go mod download; do + if [ "$attempt" -ge "$max_attempts" ]; then + echo "go mod download failed after $max_attempts attempts" >&2 + exit 1 + fi + echo "go mod download failed (attempt $attempt/$max_attempts), retrying in ${BACKOFF_SECONDS}s..." >&2 + sleep "$BACKOFF_SECONDS" + attempt=$((attempt + 1)) + done diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 32953e15b7f..e2facb9ebcf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -238,8 +238,20 @@ jobs: # cache save (tar + zstd) runs after that and is much slower on Windows, # so a flat 20m job budget can expire mid-save even though the test # itself already passed (see the Acceptance tests step's timeout comment - # below for the same Windows-is-slower pattern). - timeout-minutes: ${{ matrix.flavor.target == 'windows' && 30 || 20 }} + # below for the same Windows-is-slower pattern). 30m (raised from 20 in + # #2959) was still not enough: GitHub Job ID 99475416637 hit it at + # 30m27s even though every step had genuinely completed or nearly + # completed -- "Get dependencies" (normally ~5s) took ~5m, the test step + # itself (normally ~1m, well under its own 15m budget) took ~10.5m, and + # "Post Set up Go" (normally near-instant) took ~10.5m before finishing + # successfully; only the next step, "Post Cache Atmos toolchain" + # (normally ~42s), was still running when the job clock ran out. That + # uniform ~10x slowdown across three unrelated operations (dependency + # download, test execution, cache save) is the signature of a + # degraded/throttled runner that day, not a code regression -- see + # docs/fixes/2026-08-31-terraform-registry-cache-windows-runner-degradation.md. + # Raised to 45m for real headroom above that worst case. + timeout-minutes: ${{ matrix.flavor.target == 'windows' && 45 || 20 }} runs-on: ${{ matrix.flavor.os }} steps: - name: Harden Runner diff --git a/.github/workflows/website-deploy-prod.yml b/.github/workflows/website-deploy-prod.yml index 13e580452c6..825deca60cc 100644 --- a/.github/workflows/website-deploy-prod.yml +++ b/.github/workflows/website-deploy-prod.yml @@ -65,6 +65,13 @@ jobs: with: go-version-file: "go.mod" + # `go run .` below has no prior module cache warm-up, so a single + # mid-stream proxy.golang.org reset (e.g. "stream error: ... + # INTERNAL_ERROR; received from peer") aborts it outright -- see + # docs/fixes/2026-08-25-build-atmos-go-mod-download-retry.md. + - name: Download Go modules (with retry) + uses: ./.github/actions/go-mod-download-retry + - name: Generate atmos-manifest schema # website/static/schemas/atmos/atmos-manifest/ is gitignored (see .gitignore) — it is # never hand-maintained or committed, only generated here from the embedded schema diff --git a/.github/workflows/website-preview-build.yml b/.github/workflows/website-preview-build.yml index 4598946f809..8b171df58bf 100644 --- a/.github/workflows/website-preview-build.yml +++ b/.github/workflows/website-preview-build.yml @@ -51,6 +51,13 @@ jobs: with: go-version-file: "go.mod" + # `go run .` below has no prior module cache warm-up, so a single + # mid-stream proxy.golang.org reset (e.g. "stream error: ... + # INTERNAL_ERROR; received from peer") aborts it outright -- see + # docs/fixes/2026-08-25-build-atmos-go-mod-download-retry.md. + - name: Download Go modules (with retry) + uses: ./.github/actions/go-mod-download-retry + - name: Generate atmos-manifest schema # website/static/schemas/atmos/atmos-manifest/ is gitignored (see .gitignore) — it is # never hand-maintained or committed, only generated here from the embedded schema diff --git a/.gitignore b/.gitignore index 1d3a7c8416f..4b153c65eda 100644 --- a/.gitignore +++ b/.gitignore @@ -144,9 +144,9 @@ TEST_QUALITY_*.md /lintroller /.lintroller tools/lintroller/.lintroller +tools/gomodcheck/.gomodcheck .golangci/lintroller/lintroller /custom-gcl -tools/gomodcheck/.gomodcheck # golangci-lint per-worktree cache + lock isolation (see # magefiles/mage_lint_golangci_run.go). The cache is large (100MB+); .golangci-tmp diff --git a/agent-skills/AGENTS.md b/agent-skills/AGENTS.md index 841c3c4a4aa..b00d78aa7ff 100644 --- a/agent-skills/AGENTS.md +++ b/agent-skills/AGENTS.md @@ -141,7 +141,7 @@ When a task involves Atmos, activate the matching skill for detailed guidance. | AWS ECR: registry login, ECR auth integrations, Docker credential writes | `atmos-aws-ecr` | `agent-skills/skills/atmos-aws-ecr/SKILL.md` | | AWS compliance: Security Hub standards, compliance reports, CIS AWS, PCI DSS, SOC2, HIPAA, NIST | `atmos-aws-compliance` | `agent-skills/skills/atmos-aws-compliance/SKILL.md` | | AWS security: analyze findings, map to components/stacks, structured remediation | `atmos-aws-security` | `agent-skills/skills/atmos-aws-security/SKILL.md` | -| Migrating to Atmos from native Terraform/OpenTofu, Terraform Workspaces, or Terramate: layout, workspace mapping, remote-state bridge, generate_hcl/script decomposition | `atmos-migration` | `agent-skills/skills/atmos-migration/SKILL.md` | +| Migrating to Atmos from native Terraform/OpenTofu, Terraform Workspaces, or Terramate (layout, workspace mapping, remote-state bridge, generate_hcl/script decomposition), or migrating CLI tool-version management from mise or Aqua CLI to the Atmos toolchain | `atmos-migration` | `agent-skills/skills/atmos-migration/SKILL.md` | | Atmos Modernization: replace deprecated patterns with current Atmos naming, CI, Pro, auth, secrets, and dependencies | `atmos-modernization` | `agent-skills/skills/atmos-modernization/SKILL.md` | ## Common Patterns diff --git a/agent-skills/skills/atmos-migration/SKILL.md b/agent-skills/skills/atmos-migration/SKILL.md index ff01d3ecbcb..e6142a82484 100644 --- a/agent-skills/skills/atmos-migration/SKILL.md +++ b/agent-skills/skills/atmos-migration/SKILL.md @@ -1,6 +1,6 @@ --- name: atmos-migration -description: "Migrating to Atmos from existing IaC: techniques, tactics, and design patterns for native Terraform, Terraform Workspaces, and Terramate — minimum-disruption paths, file-layout options, workspace mapping, tag/generate_hcl/script decomposition, and the remote-state bridge for progressive migration" +description: "Migrating to Atmos from existing IaC: techniques, tactics, and design patterns for native Terraform, Terraform Workspaces, and Terramate — minimum-disruption paths, file-layout options, workspace mapping, tag/generate_hcl/script decomposition, and the remote-state bridge for progressive migration; also covers migrating CLI tool-version management from mise or Aqua CLI to the Atmos toolchain" metadata: copyright: Copyright Cloud Posse, LLC 2026 version: "1.0.0" @@ -11,6 +11,8 @@ references: - references/remote-state-bridge.md - references/from-terramate.md - references/from-component-updater.md + - references/from-mise.md + - references/from-aqua.md --- # Migrating to Atmos @@ -22,6 +24,10 @@ Atmos is designed to **adopt an existing repo without forcing a reorganization** `components/terraform/` layout is a recommendation, not a requirement. Lead with the minimum change that delivers value, then escalate only as the user's needs grow. +This skill also covers migrating CLI tool-version management from mise or Aqua CLI to the Atmos +toolchain -- see [from-mise.md](references/from-mise.md) and +[from-aqua.md](references/from-aqua.md) in the routing table below. + For full prose tutorials aimed at end users, link to: - [Migrating from Native Terraform](https://atmos.tools/migration/native-terraform) @@ -80,6 +86,8 @@ different reference: | `.tm.hcl` files, `stack.tm.hcl`, `generate_hcl` blocks (Terramate project) | [from-terramate.md](references/from-terramate.md) | | Need to read outputs from un-migrated TF (legacy or another repo) | [remote-state-bridge.md](references/remote-state-bridge.md) | | `cloudposse/github-action-atmos-component-updater` | [from-component-updater.md](references/from-component-updater.md) | +| mise config (`mise.toml`, `.mise.toml`, `.mise/config.toml`, `.tool-versions`) for tool versions | [from-mise.md](references/from-mise.md) | +| `aqua.yaml` (Aqua CLI) for tool versions | [from-aqua.md](references/from-aqua.md) | The remote-state-bridge pattern is what makes **progressive, component-by-component migration** possible. Without it, a team is forced into a big-bang cutover. Cover it any time the user has @@ -196,3 +204,7 @@ Things to push back on if a user (or another agent) proposes them during migrati - [References/from-terramate.md](references/from-terramate.md) -- construct-by-construct mapping from Terramate (`stack.tm.hcl`, globals, `generate_hcl`, `script{}`, tags/labels) to Atmos, including the one remaining known gap (`.tmtriggers`) +- [References/from-mise.md](references/from-mise.md) -- migrating tool versions, tasks, and env + vars from mise to the Atmos toolchain +- [References/from-aqua.md](references/from-aqua.md) -- migrating tool versions from Aqua CLI's + `aqua.yaml` to the Atmos toolchain diff --git a/agent-skills/skills/atmos-migration/references/from-aqua.md b/agent-skills/skills/atmos-migration/references/from-aqua.md new file mode 100644 index 00000000000..3a3982c6e92 --- /dev/null +++ b/agent-skills/skills/atmos-migration/references/from-aqua.md @@ -0,0 +1,235 @@ +# Migrating from Aqua CLI + +[Aqua CLI](https://aquaproj.github.io/) is a tool-version manager. It reads a config file named +`aqua.yaml`. + +Read this distinction before you start: Aqua CLI is not the same thing as the Aqua registry. +Atmos does not run the Aqua CLI tool. Atmos does reuse the Aqua **registry** data format -- +the same package listings that Aqua CLI reads. Atmos reimplemented the registry parser itself, +because the Aqua project states that its own Go modules are for internal use, not for external +tools to depend on. This means the registry ecosystem carries over. The `aqua.yaml` file, the +checksum lockfile, and the policy file do not carry over as-is. See "Common Gotchas" below. + +This reference is a scenario-keyed decision guide. It covers only tool-version management: the +migration of `aqua.yaml` to the Atmos toolchain. It does not cover the migration of Terraform code +itself. For that task, use the other references in this skill. + +## Identifying the User's Shape + +Read the `aqua.yaml` file, or ask the user to show it to you. + +| Shape | Recipe | +|---|---| +| `aqua.yaml` uses the standard registry only | [Shape A](#shape-a-standard-registry-only) | +| `aqua.yaml` uses a custom registry, a checksum block, or a policy file | [Shape B](#shape-b-custom-registry-checksums-or-policy) | + +## Shape A: Standard Registry Only + +**Before:** +```text +project/ +├── aqua.yaml +└── main.tf +``` +```yaml +# aqua.yaml +registries: + - type: standard + ref: v4.0.0 + +packages: + - name: hashicorp/terraform@v1.10.3 + - name: jqlang/jq@jq-1.7.1 + - name: kubernetes/kubectl@v1.28.0 +``` + +**Recipe:** + +1. Add the `toolchain:` block to `atmos.yaml`. A `registries:` entry with `type: standard` maps + to a `toolchain.registries[]` entry with `type: aqua`. Carry over the `ref:` pin from + `aqua.yaml` -- an unpinned registry can change under you the same way an unpinned `ref: main` + would. Do not put a `tree/` segment in `source`; Atmos treats everything after + `github.com//` as a literal file path, so `source` must end at the repository + (plus an optional subpath like `pkgs`) and the ref belongs in the separate `ref:` field. + ```yaml + toolchain: + versions_file: .tool-versions + registries: + - name: aqua + type: aqua + source: https://github.com/aquaproj/aqua-registry/pkgs + ref: v4.0.0 + priority: 10 + ``` +2. Convert each `packages:` entry to a `.tool-versions` line. The common `name: owner/repo@version` + form splits into a short tool name and a version: + ```text + # .tool-versions + terraform 1.10.3 + jq 1.7.1 + kubectl 1.28.0 + ``` + If the short name in `.tool-versions` does not match the Aqua `owner/repo` name, add an alias: + ```yaml + toolchain: + aliases: + terraform: hashicorp/terraform + jq: jqlang/jq + kubectl: kubernetes/kubectl + ``` +3. Check the migration. Run `atmos toolchain install`, then `atmos toolchain list`, then + `atmos toolchain which ` for each tool. Confirm each version matches what Aqua CLI + reported. + +## Shape B: Custom Registry, Checksums, or Policy + +This shape builds on Shape A. Do the Shape A recipe first, then add the steps below. + +**Before:** +```yaml +# aqua.yaml +registries: + - type: standard + ref: v4.0.0 + - type: github_content + repo_owner: myorg + repo_name: my-registry + ref: main + path: registry.yaml + +packages: + - name: hashicorp/terraform@v1.10.3 + - name: myorg/internal-tool@v2.0.0 + +checksum: + enabled: true + require_checksum: true +``` +```yaml +# aqua-policy.yaml +registries: + - type: standard +policies: + - myorg/my-registry/registry.yaml +``` + +**Recipe:** + +1. Add the custom registry as a second `toolchain.registries[]` entry. Use `source` for the + registry location, and `ref` to pin a version. Atmos accepts `ref` only when `source` is a + `github.com` URL. Pin `ref` to a tag or commit SHA, not a branch -- a branch like `main` is + mutable and can change what gets installed without any change to `atmos.yaml`. Do not encode + the ref as a `tree/` path segment in `source`; that breaks tool lookups against this + registry (see "Common Gotchas" below). Keep `source` at the repository, plus an optional + subpath, and put the ref in `ref:`. + ```yaml + toolchain: + registries: + - name: internal + type: aqua + source: https://github.com/myorg/my-registry + ref: v1.2.0 + priority: 100 + - name: aqua + type: aqua + source: https://github.com/aquaproj/aqua-registry/pkgs + ref: v4.0.0 + priority: 10 + ``` +2. Replace the `checksum:` block with `toolchain.verification`: + ```yaml + toolchain: + verification: + checksums: required + signatures: when_available + verifier_install: auto + ``` +3. Drop `aqua-policy.yaml`. Atmos has no trust or policy step. See "Common Gotchas" below. +4. Do not migrate `aqua-checksums.json`. Atmos manages its own lockfile, `toolchain.lock.yaml`. + Whether it writes that lockfile automatically depends on the project's edition: unpinned or + newer projects get it by default, with no configuration needed. A project whose `atmos.yaml` + pins an edition dated before `2026-08-05` keeps the old opt-in behavior and must set + `toolchain.use_lock_file: true` explicitly to get the same automatic lockfile. +5. Check the migration the same way as Shape A. + +## CLI Command Mapping + +| Aqua CLI command | Atmos equivalent | Notes | +|---|---|---| +| `aqua init` | Create the `toolchain:` block in `atmos.yaml` by hand | There is no init command. | +| `aqua install` | `atmos toolchain install` | Installs all tools from `.tool-versions` | +| `aqua g` / `aqua generate` | `atmos toolchain search `, then add a `.tool-versions` line | Atmos has no interactive picker. | +| `aqua cp` | No direct equivalent | `toolchain.install_path` gives a predictable local directory, for caching, not for vendoring binaries into another image. | +| `aqua exec -- ` | `atmos toolchain exec @ -- ` | Runs one command with a pinned tool version | +| `aqua which ` | `atmos toolchain which ` | Shows the path to the tool binary | +| `aqua list` | `atmos toolchain list` | Lists installed tools | +| `aqua update-checksum` | No action needed | Atmos manages `toolchain.lock.yaml` on its own. | +| `aqua policy allow [file]` | No equivalent | Atmos has no trust or policy step. | +| `aqua info` | `atmos toolchain info ` | Shows registry metadata for one tool, not full environment diagnostics | + +## Common Gotchas + +### The registry format is shared. The CLI tool is not. + +Atmos reads Aqua registry package listings. Atmos does not read `aqua.yaml` directly, and Atmos +does not run the Aqua CLI. Translate `aqua.yaml` by hand, using the recipes above. Do not expect +Atmos to parse `aqua.yaml` as config. + +### No policy or trust-gating step + +Aqua CLI uses `aqua-policy.yaml` and the `AQUA_POLICY_CONFIG` variable to control which configs +and registries a user trusts, because some Aqua package types run arbitrary build steps or +scripts. Atmos supports only the two safest package types, `github_release` and `http`. Atmos +never runs an install script. This removes the need for a trust step. Do not look for a policy +equivalent. + +### No layered global config + +Aqua CLI supports `AQUA_GLOBAL_CONFIG`, a list of config file paths that apply everywhere, not +just in one project. Atmos has no matching variable. Use the project `.tool-versions` file for +tools every developer needs, or `toolchain.aliases`/`toolchain.registries` in `atmos.yaml` or in +`.atmos.d/`. + +### A broken custom registry fails silently for public tools + +If a custom `toolchain.registries[]` entry can't resolve a tool (wrong `source`, unreachable +host, tool not present at that path), Atmos does not error. It falls back to searching the +built-in public Aqua registry for that same tool. For a tool that also exists publicly, this +means a misconfigured custom registry produces no visible symptom at all -- the install succeeds, +just from the wrong source. The failure only becomes visible for a tool that exists *only* in the +custom registry, like the `myorg/internal-tool` example above, where it fails outright with "tool +not in registry." After adding a custom registry, verify it is actually being used: run +`atmos toolchain install` with `--reinstall` and `logs.level: Debug` in `atmos.yaml`, and confirm +the log shows `Tool found in configured registry`, not a `Searching builtin registry` fallback. + +### Shims are opt-in, not automatic + +Aqua CLI installs a shim for every declared tool automatically, then downloads the real binary +the first time the shim runs. Atmos has an equivalent, `toolchain.proxies`, but it is opt-in per +command, not automatic for every package in `.tool-versions`: add a `proxies:` entry (command +name -> tool) under the `toolchain:` block, then run `atmos toolchain env` to activate it in an +interactive shell -- the pinned tool installs on first use, the same lazy-install behavior as an +Aqua shim. See [Toolchain Proxies](https://atmos.tools/cli/configuration/toolchain/proxies). + +Without an explicit proxy entry, Atmos installs a tool when a component, workflow, or command +that declares it runs, or when the user runs `atmos toolchain install` directly. + +### Unsupported package types + +Atmos does not support these Aqua package types: `github_content`, `github_archive`, `go_build`, +`cargo`, `go_install`. If a package in `aqua.yaml` uses one of these types, find another source +for it, or define an inline `type: atmos` registry entry with a `github_release` or `http` +package type instead. See the [atmos-toolchain](../../atmos-toolchain/SKILL.md) skill for the +full list of supported and unsupported registry features. + +## What to NOT Do + +- Do not point Atmos at `aqua.yaml` directly. Atmos does not read this file. Translate it by + hand, using the recipes above. +- Do not look for an Atmos equivalent of `aqua-policy.yaml` or `AQUA_POLICY_CONFIG`. Atmos has no + trust step, by design. +- Do not try to migrate `aqua-checksums.json`. Atmos writes its own lockfile automatically. +- Do not use a naive `grep` on `name:` lines to convert `packages:` entries. This breaks on the + common single-line `name: owner/repo@version` form. Use the recipe above instead. +- Do not introduce Gomplate datasources for things YAML functions can express. See the Core + Principles in the [SKILL.md](../SKILL.md). diff --git a/agent-skills/skills/atmos-migration/references/from-mise.md b/agent-skills/skills/atmos-migration/references/from-mise.md new file mode 100644 index 00000000000..9c1285d21a9 --- /dev/null +++ b/agent-skills/skills/atmos-migration/references/from-mise.md @@ -0,0 +1,229 @@ +# Migrating from mise + +[mise](https://mise.jdx.dev/) is a tool-version manager. A mise config file has one of these +names: `mise.toml`, `.mise.toml`, `.mise/config.toml`. mise can also read a `.tool-versions` file. + +This reference is a scenario-keyed decision guide. It covers only tool-version management: the +migration of `[tools]`, `[tasks]`, `[env]`, and `[settings]` to the Atmos toolchain. It does not +cover the migration of Terraform code itself. For that task, use the other references in this +skill. + +## Identifying the User's Shape + +Read the mise config file, or ask the user to show it to you. + +| Shape | Recipe | +|---|---| +| Tool versions only: `[tools]`, or a `.tool-versions` file, with no `[tasks]` or `[env]` | [Shape A](#shape-a-tool-versions-only) | +| Tool versions plus tasks or environment variables: `[tasks]`, `[env]` | [Shape B](#shape-b-tool-versions-plus-tasks-and-env) | + +## Shape A: Tool Versions Only + +**Before:** +```text +project/ +├── mise.toml +└── main.tf +``` +```toml +# mise.toml +[tools] +terraform = "1.10.3" +jq = "1.7.1" +kubectl = "1.28.0" +``` + +**Recipe:** + +1. Check for a `.tool-versions` file. If mise already reads this file, do not change it. If mise + does not use this file, create it. Add one line for each tool in `[tools]`. +2. Find each tool in the Atmos toolchain. Run `atmos toolchain search `. If the tool is in + the Aqua registry, and the mise short name differs from the Aqua `owner/repo` name, add an + alias in `toolchain.aliases`. If the tool is not in the Aqua registry, add an inline registry + entry. +3. Add the `toolchain:` block to `atmos.yaml`. Do not put a `tree/` segment in `source`; + Atmos treats everything after `github.com//` as a literal file path, so this + breaks tool lookups. Keep `source` at the repository plus subpath, and pin a specific + revision with a separate `ref:` field if needed. + ```yaml + toolchain: + versions_file: .tool-versions + aliases: + terraform: hashicorp/terraform + jq: jqlang/jq + kubectl: kubernetes/kubectl + registries: + - name: aqua + type: aqua + source: https://github.com/aquaproj/aqua-registry/pkgs + priority: 10 + ``` + ```text + # .tool-versions + terraform 1.10.3 + jq 1.7.1 + kubectl 1.28.0 + ``` +4. Check the migration. Run `atmos toolchain install`, then `atmos toolchain list`, then + `atmos toolchain which ` for each tool. Confirm each version matches what mise reported. + +## Shape B: Tool Versions Plus Tasks and Env + +This shape builds on Shape A. Do the Shape A recipe first, then add the steps below. + +**Before:** +```text +project/ +├── mise.toml +└── main.tf +``` +```toml +# mise.toml +[tools] +terraform = "1.10.3" + +[env] +AWS_REGION = "us-east-1" + +[env.production] +AWS_PROFILE = "prod" + +[tasks] +fmt = "terraform fmt -recursive" + +[tasks.deploy] +description = "Deploy the app" +run = "terraform apply -auto-approve" +depends = ["fmt"] + +[settings] +experimental = true +``` + +**Recipe:** + +1. Migrate `[env]`. A mise env var maps to a stack `env:` block or a command `env:` block. + ```yaml + # atmos.yaml or stacks/_defaults.yaml -- env for the whole project + env: + AWS_REGION: us-east-1 + ``` + ```yaml + # stacks/prod.yaml -- env for one environment + env: + AWS_PROFILE: prod + ``` + A mise profile, for example `[env.production]`, maps to an Atmos stack. Each stack already + has its own `env:` block. You do not need a separate profile feature. If only one command + needs an env var, add the var to that command's own `env:` block instead of the global one. + + Precedence order, from lowest to highest: the system environment, then the global `env:` block + in `atmos.yaml`, then the `env:` block in a stack file (stack root, then component type, then + component). A command's own `env:` block is different. It is a list of key-value pairs, and it + applies only when that command runs. It is not part of the order above. See the + [atmos-settings](../../atmos-settings/SKILL.md) skill for full detail. +2. Migrate `[tasks]`. A simple mise task maps to an Atmos custom command. + ```yaml + # atmos.yaml + commands: + - name: fmt + description: Format Terraform code + steps: + - terraform fmt -recursive + + - name: deploy + description: Deploy the app + steps: + - atmos fmt + - terraform apply -auto-approve + ``` + mise `depends` has no direct equivalent inside one custom command. For a simple, linear + dependency, add a step that runs the other command, as shown above. For a task graph with + parallel steps or conditions, use an Atmos workflow instead. A workflow supports `depends_on` + and `when:`. See the [atmos-workflows](../../atmos-workflows/SKILL.md) skill. Tasks in a + `mise-tasks/` directory have no direct mapping. Convert each script to a step in a custom + command. +3. Drop `[settings]`. `[settings].experimental` has no equivalent. Most other `[settings]` + entries have no equivalent either. See "Common Gotchas" below. +4. Check the migration the same way as Shape A. + +## CLI Command Mapping + +| mise command | Atmos equivalent | Notes | +|---|---|---| +| `mise install` | `atmos toolchain install` | Installs all tools listed in `.tool-versions` | +| `mise install @` | `atmos toolchain install @` | Installs one tool | +| `mise use @` | `atmos toolchain set ` | Sets the default version in `.tool-versions` | +| `mise ls` / `mise ls --current` | `atmos toolchain list` | Lists installed tools | +| `mise ls-remote ` | `atmos toolchain info ` | Shows available versions | +| `mise current` | `atmos toolchain get [tool]` | Shows the version set in `.tool-versions` | +| `mise which ` | `atmos toolchain which ` | Shows the path to the tool binary | +| `mise uninstall @` | `atmos toolchain uninstall @` | Removes one installed version | +| `mise prune` | No direct equivalent | Atmos has no command that removes only unused versions. | +| `mise exec @ -- ` | `atmos toolchain exec @ -- ` | Runs one command with a pinned tool version | +| `mise run ` | `atmos ` | Runs the migrated custom command | +| `mise env` | `atmos toolchain env` | Prints PATH only. `mise env` also exports `[env]` table entries; migrate those to a stack or command `env:` block instead — see "What to NOT Do" below. `atmos env` is unrelated: it prints only atmos.yaml's own global `env:` section, not stack or command `env:` values. | +| `mise activate` | `eval "$(atmos toolchain env --format=bash)"` in the shell startup file | Atmos has no activate daemon. Other formats: `fish`, `powershell`, `github`. | +| `mise search ` | `atmos toolchain search ` | Searches all registries | +| `mise registry` | `atmos toolchain registry list` or `registry search` | Lists or searches one registry | +| `mise unuse ` | `atmos toolchain remove ` | Removes a tool from `.tool-versions` | + +These mise commands have no Atmos equivalent. Do not look for a match. Drop them during the +migration: `mise doctor`, `mise plugins`, `mise trust`/`untrust`, `mise settings`, `mise where` +(use `atmos toolchain which` for the binary path instead), `mise outdated`/`upgrade` (change the +version in `.tool-versions` and run `atmos toolchain install` instead), `mise implode` (closest +match is `atmos toolchain clean`), `mise tasks` (run `atmos --help` to list custom commands +instead). + +## Common Gotchas + +### No plugin system + +mise and asdf use plugins. A plugin is a script that installs a tool. Atmos does not run install +scripts. Atmos gets tools from the Aqua registry, or from an inline registry in `atmos.yaml`. Do +not look for a plugin equivalent. Find the tool in the Aqua registry instead, or add an inline +registry entry for it. See the [atmos-toolchain](../../atmos-toolchain/SKILL.md) skill for the +full registry reference. + +### `.tool-versions` format is shared + +Atmos and mise read the same asdf-compatible `.tool-versions` format. In most cases, this file +does not need to change. Only the config that wraps it changes: mise finds the file on its own, +but Atmos needs a `toolchain:` block in `atmos.yaml`. + +### `[settings]` mostly has no equivalent + +Settings such as `experimental`, `idiomatic_version_file_enable_tools`, and `jobs` are specific to +mise. Do not try to find an Atmos setting for each one. Drop them during the migration. + +### `atmos toolchain` is an experimental command + +Tell the user this before the migration starts. Do not let the user find this out partway +through the work. + +### Shims vs. `dependencies.tools` + +mise adds every declared tool to the shell PATH with shims automatically. Atmos has an +equivalent, `toolchain.proxies`, but it is opt-in per command, not automatic for every tool in +`.tool-versions`: add a `proxies:` entry (command name -> tool) under the `toolchain:` block, then +run `atmos toolchain env` to activate it in an interactive shell. See +[Toolchain Proxies](https://atmos.tools/cli/configuration/toolchain/proxies). + +Without an explicit proxy entry, Atmos adds a tool to PATH only for the command, workflow, or +component that declares it, through `dependencies.tools`. For an interactive shell without +proxies, run `atmos toolchain env` or `atmos toolchain path`. + +## What to NOT Do + +- Do not try to run mise install scripts through Atmos. Atmos has no plugin system. +- Do not leave tool versions only in `.tool-versions` when a specific component, workflow, or + command needs a pinned version. Use `dependencies.tools` for that case. See the + [atmos-toolchain](../../atmos-toolchain/SKILL.md) skill. +- Do not put `[tasks]` content inside the `toolchain:` block. Custom commands and workflows are + separate features. See the [atmos-custom-commands](../../atmos-custom-commands/SKILL.md) and + [atmos-workflows](../../atmos-workflows/SKILL.md) skills. +- Do not put `[env]` content inside the `toolchain:` block. Use stack or command `env:` instead. + See the [atmos-config](../../atmos-config/SKILL.md) and [atmos-stacks](../../atmos-stacks/SKILL.md) + skills. +- Do not introduce Gomplate datasources for things YAML functions can express. See the Core + Principles in the [SKILL.md](../SKILL.md). diff --git a/docs/fixes/2026-08-20-windows-go-test-unlinkat-retry.md b/docs/fixes/2026-08-20-windows-go-test-unlinkat-retry.md new file mode 100644 index 00000000000..72cb9de01a7 --- /dev/null +++ b/docs/fixes/2026-08-20-windows-go-test-unlinkat-retry.md @@ -0,0 +1,80 @@ +# Fix: retry the transient Windows `go: unlinkat ...` race in acceptance test shards + +**Date:** 2026-08-20 + +## Summary + +`Acceptance Tests (windows, shard 10/10)` (GitHub Job ID: 96582175875) failed even though every +one of its 39 packages reported `ok`. The actual failure was Go's own toolchain diagnostic: + +```text +go: unlinkat C:\Users\RUNNER~1\AppData\Local\Temp\go-build860437867\b2679\list.test.exe: The process cannot access the file because it is being used by another process. +``` + +This fires when `go test` tries to delete its own compiled temp test binary after the test run +has already completed and reported its real result, but another process (commonly Windows +Defender's real-time scanner) still briefly holds the file open. It is a documented Go-on-Windows +race, not a test failure. `internal/ci/acceptance.commandRunner.run` now retries (up to 2 times, +2s apart) when it detects exactly this diagnostic on stderr, and gives up immediately for any +other failure so a real test failure still fails the job on the first attempt. + +## Context + +Reading the attached job log showed every package in the shard's `pkgs` source-test group +(`cmd/auth`, `cmd/list`, ..., `pkg/web`) printed `ok` before the shard failed. `cmd/list` in +particular had already printed `ok github.com/cloudposse/atmos/cmd/list 19.301s`. The failure +surfaced only afterward, from `go test`'s own cleanup step, and propagated up through +`runSourceTestGroup` → `runWindowsShard` → `mage acceptance:run` as a generic `exit status 1`, +making the job (and the required `Acceptance Tests` gate) fail despite no actual test regression. + +`internal/ci/acceptance/run.go`'s `runWindowsShard` already runs three test groups concurrently +on Windows (`tests.test.exe`, `internal-exec.test.exe`, and the on-the-fly `go test ` for +everything else) specifically to save wall-clock time; that concurrency is the likely trigger for +the antivirus-scan race, since several test binaries are being written/executed/deleted around +the same time. + +## Changes + +- `internal/ci/acceptance/command.go`: + - `commandRunner.run` now captures stderr (via `io.MultiWriter`, so live CI log streaming is + unaffected) and retries the whole command up to `transientUnlinkRetries` (2) times, 2s apart, + when `isTransientWindowsUnlinkError` matches. Any other failure (including a real test + failure, which never emits this diagnostic) still returns immediately on the first attempt. + - Added `isTransientWindowsUnlinkError`, a pure string-match helper. + - Added a `retryDelay` field on `commandRunner` (defaulted to the real 2s in + `newCommandRunner`) so tests can zero it out instead of paying real wall-clock time. +- `internal/ci/acceptance/command_test.go`: added `TestIsTransientWindowsUnlinkError` (table + test over the pure matcher) and three end-to-end tests + (`TestRunRetriesTransientWindowsUnlinkError`, `TestRunGivesUpAfterExhaustingRetries`, + `TestRunDoesNotRetryUnrelatedFailures`) that drive `commandRunner.run` against a real child + process -- this test binary re-execing itself via a `TestMain` sentinel (this repo's + cross-platform convention for simulating subprocess behavior instead of relying on + platform-specific binaries like `false`), controlled by a counter file so the same binary can + simulate N transient failures followed by success, or an unrelated non-transient failure. + +## Verification + +- `go test ./internal/ci/acceptance/...` passes, including the new retry tests. +- `go build ./internal/ci/acceptance/...` and `go vet ./internal/ci/acceptance/...` clean. +- `gofumpt -l` clean on both changed files. + +## Follow-up: scope the retry to `go test` invocations only + +A PR review (CodeRabbit) correctly flagged that `commandRunner.run`'s retry, as first written, +applied to every call through the shared `run` helper -- including `go test -c` (which writes a +persistent `-o` binary Go never auto-deletes), `go tool covdata merge`/`textfmt`, and precompiled +`*.test.exe`/`cmd.test` binaries executed directly. None of those can hit the actual unlinkat +race (only a bare, on-the-fly `go test ` compiles-runs-deletes its own temp binary), so +retrying them on a coincidental stderr match risked rerunning a command with real side effects +(e.g. writing coverage data twice) or silently masking an unrelated failure that happened to +mention both substrings. + +Fixed by replacing `run`'s four positional parameters (`dir`, `env`, `retryTransient`, `name`) +with an explicit `runOptions{dir, env, retryTransient}` struct (also resolving a `revive` +`argument-limit` violation from the extra bool), passed as `true` only from the two call sites +that run bare `go test `: `runSourceTestGroup` (`run.go`) and `CollectCoverage` +(`coverage.go`). Every other call site (`go test -c`, `go tool covdata`, and the three precompiled +test binary executions) now explicitly passes `retryTransient: false`. Added +`TestRunDoesNotRetryWhenNotRetryTransient` to `command_test.go`, which drives the same transient +diagnostic through the helper subprocess with `retryTransient: false` and asserts `run` fails on +the first attempt without ever invoking the command a second time. diff --git a/docs/fixes/2026-08-31-floci-azure-health-check-race.md b/docs/fixes/2026-08-31-floci-azure-health-check-race.md new file mode 100644 index 00000000000..fb5fd5fdc63 --- /dev/null +++ b/docs/fixes/2026-08-31-floci-azure-health-check-race.md @@ -0,0 +1,55 @@ +# Fix: retry the Floci endpoint health check instead of checking once + +**Date:** 2026-08-31 + +## Summary + +`TestAzureSecretsFlociE2E` failed in the `[floci] go e2e` CI job (GitHub Job ID 99670860774) with +`Get "http://localhost:4577": context deadline exceeded` / `Floci HTTP endpoint is not reachable at +http://localhost:4577`, even though the TCP dial to that port had already succeeded. Every other +Floci test in the same run passed, including the Azure landing-zone scaffold test, which also +exercises the Azure emulator successfully. `tests/floci_harness_test.go`'s `requireFlociEndpoint` +checked the endpoint exactly once with a 2-second timeout; it now polls for up to +`flociStartupTimeout` (90s), reusing the same budget the local testcontainers auto-start path +already grants for exactly this kind of cold start. + +## Context + +CI runs Floci as three separate GitHub Actions `services:` containers (`floci`, `floci-gcp`, +`floci-az`), started via `.github/workflows/test.yml`. GitHub Actions only waits for a service +container's process to start, not for the application inside it to actually begin serving +requests -- none of the three service definitions have a `--health-cmd` configured. The Go test +suite's own `requireFlociEndpoint` is therefore the only readiness gate in CI, and it did a single +TCP dial plus a single HTTP GET, each with a 2-second timeout. + +The failure log shows the TCP dial succeeded (no "Floci is not reachable" error) but the +subsequent HTTP GET timed out -- the socket was accepting connections before the HTTP handler +inside the container was ready to respond, a well-known startup-race pattern for emulator/proxy +servers that bind their listener early. `tests/floci_containers_test.go`'s `startFlociContainer` +(used when tests auto-start Floci locally via testcontainers, not in CI where endpoints are +pre-supplied) already accounts for this: it waits via `wait.ForHTTP("/").WithStartupTimeout(flociStartupTimeout)` +with a 90-second budget, specifically for `floci-az`, which additionally sets +`FLOCI_AZ_TLS_ENABLED: "true"` and likely needs to generate a TLS certificate on startup, making it +plausibly slower to become ready than the other two emulators. The CI-path check never had an +equivalent tolerance. + +## Changes + +- `tests/floci_harness_test.go`: `requireFlociEndpoint` now polls the combined TCP-dial-then-HTTP-GET + check via a new `pollUntil` helper, bounded by the existing `flociStartupTimeout` (90s, declared in + `floci_containers_test.go`) instead of trying once. The final error message includes the elapsed + budget so a genuine misconfiguration (wrong port, service never starts) still fails loudly, just + with the same patience the local auto-start path already has. +- Added `pollUntil`, a small generic retry-until-success-or-timeout helper (500ms interval), + and three unit tests (`TestPollUntilSucceedsImmediately`, + `TestPollUntilRetriesUntilSuccessWithinBudget`, `TestPollUntilReturnsLastErrorOnTimeout`) + covering it directly, independent of any real Floci endpoint. + +## Verification + +- `go build ./tests/...` and `go vet ./tests/...` clean. +- `go test ./tests -run TestPollUntil` passes (3/3). +- `go test ./tests -run TestAzureSecretsFlociE2E` skips cleanly locally (no `ATMOS_TEST_FLOCI` + set), confirming the skip path is unaffected. +- `gofumpt -l` clean; patch-scoped `./custom-gcl run --new-from-rev=origin/main ./tests/...` + reports 0 issues. diff --git a/docs/fixes/2026-08-31-terraform-registry-cache-windows-runner-degradation.md b/docs/fixes/2026-08-31-terraform-registry-cache-windows-runner-degradation.md new file mode 100644 index 00000000000..d436b239023 --- /dev/null +++ b/docs/fixes/2026-08-31-terraform-registry-cache-windows-runner-degradation.md @@ -0,0 +1,56 @@ +# Fix: `Terraform registry cache test (windows)` CI job cancelled again despite the test passing + +**Date:** 2026-08-31 + +## Summary + +The `Terraform registry cache test (windows)` CI check was reported failing (`GitHub Job ID: +99475416637`), but the actual `TestTerraformRegistryCache` Go test had already passed. The job's +`timeout-minutes: 30` (raised from 20 in #2959, see +`docs/fixes/2026-08-19-terraform-registry-cache-windows-ci-timeout.md`) still wasn't enough this +time: three unrelated steps were all running roughly 10x slower than a normal run, and their +combined time exceeded even the raised budget. Raised `timeout-minutes` for this job's windows +leg from 30 to 45. + +## Context + +The attached failure log (`.context/attachments/lrwozZ/...log`) contained only Windows runner +diagnostic noise (`pid reused`, `existing process not stopped`, `Cleaning up orphan processes`) — +no test output at all, because the actual per-step job log had already scrolled past the +"last 1000 lines" window captured for the attachment. Reading the real per-step timing via +`gh api repos/cloudposse/atmos/actions/jobs/99475416637` showed the job's true conclusion was +`cancelled`, not a test failure: + +- `Get dependencies`: 11:43:59 → 11:49:02 (~5m) — a normal successful run of this same job + (`32400222717`, 2026-08-20) took ~5s for the equivalent step. +- `Terraform registry cache acceptance test`: 11:49:02 → 11:59:30 (~10.5m), **completed + successfully** — well inside its own 15m sub-step timeout, but ~10x the ~1m a normal run takes. +- `Post Set up Go` (the `actions/setup-go` cache-save step): 11:59:30 → 12:10:08 (~10.5m), + **also completed successfully** — a normal run finishes this step near-instantly. +- `Post Cache Atmos toolchain` (normally ~42s): started 12:10:08, was still running when the + job's 30-minute budget expired at 12:11:50 (job started 11:41:50), and got cancelled at + 12:11:54. + +Comparing against four other `Terraform registry cache test (windows)` runs from 2026-08-20 (all +`success`, total job duration 7-21 minutes, `Post Set up Go` consistently near-instant) confirmed +this was not a step regressing in isolation — every measured operation in the failing run, +regardless of what it actually does (module download, running a Go test binary, saving a build +cache), was uniformly ~10x slower than normal. That signature points to a degraded or throttled +runner/network that day, not a code-level regression in this repository: a real regression would +slow only the affected step, not dependency download, test execution, and cache save all equally. + +This is the second time this exact job has hit its own timeout after its test step had already +passed (see the 2026-08-19 fix above, which raised 20→30); 30 minutes evidently still doesn't +leave enough headroom for an occasional fully-degraded run. + +## Changes + +- `.github/workflows/test.yml`: `terraform-registry-cache` job's windows-leg `timeout-minutes` + raised from 30 to 45, with the comment updated to record this incident's measured timings + alongside the original 2026-08-19 incident. Linux/macOS remain at 20 (unaffected; both + historically finish this job comfortably under budget). + +## Verification + +- `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/test.yml'))"` parses cleanly. +- `actionlint .github/workflows/test.yml` reports no issues. diff --git a/docs/fixes/2026-08-31-website-workflows-go-mod-download-retry.md b/docs/fixes/2026-08-31-website-workflows-go-mod-download-retry.md new file mode 100644 index 00000000000..9802f887b38 --- /dev/null +++ b/docs/fixes/2026-08-31-website-workflows-go-mod-download-retry.md @@ -0,0 +1,61 @@ +# Fix: retry `go mod download` before `go run .` in the website workflows + +**Date:** 2026-08-31 + +## Summary + +The `website-deploy-preview` job (`.github/workflows/website-preview-build.yml`) failed with +`stream error: stream ID ; INTERNAL_ERROR; received from peer` on many unrelated modules +(`google.golang.org/genproto`, `github.com/jwalton/go-supportscolor`, +`github.com/jfrog/jfrog-client-go`, `github.com/updatecli/updatecli`, ...) during its "Generate +atmos-manifest schema" step, which runs `go run . stack schema ...`. This is the same transient +proxy.golang.org HTTP/2 mid-stream reset already fixed once for `go mod download` in +`scripts/build-atmos.sh`/`magefiles/build.go` (see +`docs/fixes/2026-08-25-build-atmos-go-mod-download-retry.md`), but the website workflows' `go run` +steps have no module cache warm-up before them and no retry protection at all -- a single reset +during any one of the ~1000+ transitive module downloads `go run .` triggers aborts the whole +step. + +## Context + +Reading the attached failure log (GitHub Job ID 99475416637): the "Generate atmos-manifest +schema" step ran `go run . stack schema website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json` +directly, with no prior `go mod download` step, so Go had to resolve and download the entire +module graph inline. Around 40 `go: downloading ...` lines in, a batch of `##[error]` lines fired +simultaneously across completely unrelated modules -- `cloud.google.com/go/iam`, +`cloud.google.com/go/storage`, `cloud.google.com/go/monitoring`, `cloud.google.com/go/secretmanager`, +`github.com/jwalton/go-supportscolor`, `github.com/jfrog/jfrog-client-go`, +`github.com/updatecli/updatecli` -- all with the identical `stream error: stream ID ; +INTERNAL_ERROR; received from peer` message, several sharing the same underlying +`google.golang.org/genproto` transitive dependency. That breadth (many unrelated modules failing +identically, at the same instant) is the same signature as the two prior incidents in the +2026-08-25 fix doc: a CDN-side proxy.golang.org hiccup, not a dependency or code problem. + +`.github/workflows/website-preview-build.yml` and `.github/workflows/website-deploy-prod.yml` both +run `go run . stack schema ...` / `go run . config schema ...` directly after `Set up Go`, with no +`go mod download` step first and no retry wrapper -- unlike the native CI build path, which now +goes through `magefiles/build.go`'s `runGoModDownload` (ported from `scripts/build-atmos.sh` per +`docs/fixes/2026-08-26-merge-main-go-mod-download-retry-port.md`). These two website workflows +never got that protection because they don't build the `atmos` binary via the mage target at all +-- they're standalone `go run` invocations that happened to predate the original fix. + +## Changes + +- `.github/actions/go-mod-download-retry/action.yml` (new): a composite action that runs + `go mod download`, retrying up to 3 times with a 15s cooldown, mirroring the exact convention + in `magefiles/build.go`'s `runGoModDownload` and `.github/actions/download-artifact-retry`. + Extracted as a reusable action (rather than duplicating the shell loop) since both website + workflows need it identically, and a future workflow doing a bare `go run`/`go build` without + going through the mage build target can reuse it too. +- `.github/workflows/website-preview-build.yml`: added a "Download Go modules (with retry)" step + right after "Set up Go", before the "Generate atmos-manifest schema" / "Generate atmos-config + schema" steps -- both `go run .` invocations in this job share the one module cache warm-up. +- `.github/workflows/website-deploy-prod.yml`: same addition, covering this job's four `go run .` + invocations (two schema kinds, each run twice for the `1.0` path and the version-specific path). + +## Verification + +- `python3 -c "import yaml; yaml.safe_load(open(...))"` parses all three changed/added files + cleanly. +- `actionlint .github/workflows/website-preview-build.yml .github/workflows/website-deploy-prod.yml` + reports no issues. diff --git a/internal/ci/acceptance/command.go b/internal/ci/acceptance/command.go index bf90f33ad75..9dee850c7e2 100644 --- a/internal/ci/acceptance/command.go +++ b/internal/ci/acceptance/command.go @@ -11,17 +11,29 @@ import ( "os/exec" "path/filepath" "strings" + "time" ) const ( directoryPermissions = 0o755 defaultTestTimeout = "40m" cgoDisabled = "CGO_ENABLED=0" + // Links Go's native FIPS 140-3 crypto module so acceptance-test binaries // default to FIPS-enforcing mode (GODEBUG=fips140=on) at runtime, matching // release builds. This is FIPS 140-3 mode, not a CMVP compliance // certification. See docs/prd/fips-140-mode.md. fips140Latest = "GOFIPS140=latest" + + // Retry bounds for the known-benign Windows race handled by isTransientWindowsUnlinkError. + transientUnlinkRetries = 2 + transientUnlinkDelay = 2 * time.Second + + // Bounds how much trailing stderr transientErrorDetector retains while looking + // for isTransientWindowsUnlinkError's diagnostic, so a verbose `go test` run + // can't grow that buffer without limit. The real diagnostic is one short line + // (well under this), so bounding it doesn't affect detection. + maxTransientMatchWindow = 4096 ) var ( @@ -29,16 +41,18 @@ var ( errCoverageData = errors.New("invalid coverage data") errShardPlan = errors.New("invalid acceptance shard plan") errRequiredArtifact = errors.New("required acceptance artifact") + errCommandFailed = errors.New("command failed") ) type commandRunner struct { - stdout io.Writer - stderr io.Writer - stdin io.Reader + stdout io.Writer + stderr io.Writer + stdin io.Reader + retryDelay time.Duration } func newCommandRunner() commandRunner { - return commandRunner{stdout: os.Stdout, stderr: os.Stderr, stdin: os.Stdin} + return commandRunner{stdout: os.Stdout, stderr: os.Stderr, stdin: os.Stdin, retryDelay: transientUnlinkDelay} } func environment(name string) string { @@ -58,17 +72,109 @@ func writeStatus(format string, args ...any) error { return nil } -func (r commandRunner) run(ctx context.Context, dir string, env []string, name string, args ...string) error { - cmd := exec.CommandContext(ctx, name, args...) // #nosec G702 -- CI executes only repository-selected tools and test binaries. - cmd.Dir = dir - cmd.Env = append(os.Environ(), env...) - cmd.Stdin = r.stdin - cmd.Stdout = r.stdout - cmd.Stderr = r.stderr - if err := cmd.Run(); err != nil { - return fmt.Errorf("run %s: %w", commandString(name, args), err) +// runOptions configures how commandRunner.run executes a command. +type runOptions struct { + // dir is the working directory for the command. + dir string + // env is appended to the current process environment for the command. + env []string + // retryTransient enables the known-benign Windows go-test unlinkat retry (see + // run's doc comment). Note: it must be false for anything other than a bare + // `go test ` invocation -- `go test -c` (which writes a persistent binary + // Go never deletes), `go tool covdata`, and precompiled *.test.exe binaries run + // directly cannot hit this specific race, and blindly retrying them on a + // coincidental stderr match could rerun a command with real side effects (e.g. + // writing coverage data) or mask a genuine, unrelated failure that happens to + // mention both substrings. + retryTransient bool +} + +// run executes name/args in opts.dir with opts.env appended to the current +// environment. When opts.retryTransient is true, it also retries on the known-benign +// Windows race where a bare `go test` invocation fails to delete its own temp binary +// after every test case has already reported ok/FAIL -- see isTransientWindowsUnlinkError. +// Retrying is safe there: by the time that error appears, the process under test has +// already exited and its actual pass/fail result was already written to stdout, so a +// retry re-runs already-cached work rather than masking a real failure. +func (r commandRunner) run(ctx context.Context, opts runOptions, name string, args ...string) error { + var lastErr error + for attempt := 0; attempt <= transientUnlinkRetries; attempt++ { + cmd := exec.CommandContext(ctx, name, args...) // #nosec G702 -- CI executes only repository-selected tools and test binaries. + cmd.Dir = opts.dir + cmd.Env = append(os.Environ(), opts.env...) + cmd.Stdin = r.stdin + cmd.Stdout = r.stdout + + // Only retry-enabled invocations need to watch stderr for the transient + // diagnostic; everything else forwards stderr directly instead of paying for + // a capture that's never inspected. + var detector *transientErrorDetector + cmd.Stderr = r.stderr + if opts.retryTransient { + detector = &transientErrorDetector{} + cmd.Stderr = io.MultiWriter(r.stderr, detector) + } + + err := cmd.Run() + if err == nil { + return nil + } + lastErr = fmt.Errorf("%w: run %s: %w", errCommandFailed, commandString(name, args), err) + if attempt == transientUnlinkRetries || !opts.retryTransient || !detector.matched() { + return lastErr + } + _, _ = fmt.Fprintf(r.stderr, "::warning::retrying %s after a transient Windows go-test cleanup race (attempt %d/%d): %v\n", + commandString(name, args), attempt+1, transientUnlinkRetries, err) + select { + case <-ctx.Done(): + return lastErr + case <-time.After(r.retryDelay): + } } - return nil + return lastErr +} + +// isTransientWindowsUnlinkError reports whether output is the Go toolchain's own +// "go: unlinkat ...: The process cannot access the file because it is being used by +// another process" diagnostic -- a documented Windows race (another process, commonly +// Windows Defender's real-time scanner, briefly holds the temp test binary open right +// as `go test` tries to delete it) that fires only after every test case in the run has +// already reported its real result. It is never emitted for an actual test failure. +func isTransientWindowsUnlinkError(output string) bool { + return strings.Contains(output, "unlinkat") && + strings.Contains(output, "cannot access the file because it is being used by another process") +} + +// transientErrorDetector is an io.Writer that reports whether +// isTransientWindowsUnlinkError has matched anywhere in everything written to it so +// far, without retaining unbounded output: it keeps only the trailing +// maxTransientMatchWindow bytes while unmatched, and drops that window entirely once +// matched, since the sticky matched flag is all run needs from then on. The real +// diagnostic is one short line, so bounding the window doesn't affect detection. +type transientErrorDetector struct { + window []byte + found bool +} + +// Write implements io.Writer. +func (d *transientErrorDetector) Write(p []byte) (int, error) { + if !d.found { + d.window = append(d.window, p...) + if len(d.window) > maxTransientMatchWindow { + d.window = d.window[len(d.window)-maxTransientMatchWindow:] + } + if isTransientWindowsUnlinkError(string(d.window)) { + d.found = true + d.window = nil + } + } + return len(p), nil +} + +// matched reports whether the diagnostic has been observed. A nil detector (the +// retryTransient=false path, where nothing is watching stderr) never matches. +func (d *transientErrorDetector) matched() bool { + return d != nil && d.found } func (r commandRunner) output(ctx context.Context, dir string, env []string, name string, args ...string) (string, error) { diff --git a/internal/ci/acceptance/command_test.go b/internal/ci/acceptance/command_test.go index a5a0cda46cc..cd48a4b1b09 100644 --- a/internal/ci/acceptance/command_test.go +++ b/internal/ci/acceptance/command_test.go @@ -1,11 +1,59 @@ package acceptance import ( + "bytes" + "context" + "fmt" "os" "path/filepath" + "strconv" + "strings" "testing" ) +// helperFailCountEnv names the counter file a TestMain-driven helper process reads to +// decide whether to simulate the transient Windows unlinkat race (see command.go) or +// succeed, letting TestRunRetriesTransientWindowsUnlinkError exercise commandRunner.run's +// retry loop against a real child process rather than a Windows-only race condition. +const helperFailCountEnv = "ATMOS_TEST_ACCEPTANCE_HELPER_FAIL_COUNT_FILE" + +// TestMain lets this test binary act as a controllable subprocess for +// TestRunRetriesTransientWindowsUnlinkError, per this repo's cross-platform convention +// of self-exec instead of relying on platform-specific binaries like `false`. +func TestMain(m *testing.M) { + if countFile := os.Getenv(helperFailCountEnv); countFile != "" { + os.Exit(runUnlinkRaceHelper(countFile)) + } + os.Exit(m.Run()) +} + +// runUnlinkRaceHelper decrements the count stored in countFile on every invocation: +// while positive, it prints the exact Go toolchain diagnostic isTransientWindowsUnlinkError +// matches and exits 1 (simulating the race); once the count reaches zero, it exits 0 +// (simulating the retry succeeding). +func runUnlinkRaceHelper(countFile string) int { + raw, err := os.ReadFile(countFile) + if err != nil { + fmt.Fprintf(os.Stderr, "read fail count: %v\n", err) + return 2 + } + remaining, err := strconv.Atoi(string(raw)) + if err != nil { + fmt.Fprintf(os.Stderr, "parse fail count: %v\n", err) + return 2 + } + if remaining <= 0 { + return 0 + } + if writeErr := os.WriteFile(countFile, []byte(strconv.Itoa(remaining-1)), 0o600); writeErr != nil { + fmt.Fprintf(os.Stderr, "write fail count: %v\n", writeErr) + return 2 + } + fmt.Fprintln(os.Stderr, "go: unlinkat C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\go-build0\\b0\\pkg.test.exe: "+ + "The process cannot access the file because it is being used by another process.") + return 1 +} + func TestEnvironment(t *testing.T) { t.Setenv("ATMOS_TEST_ACCEPTANCE_ENV_VAR", "value") if got := environment("ATMOS_TEST_ACCEPTANCE_ENV_VAR"); got != "value" { @@ -64,3 +112,244 @@ func TestCommandString(t *testing.T) { t.Fatalf("commandString() = %q, want %q", got, want) } } + +func TestIsTransientWindowsUnlinkError(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + output string + want bool + }{ + { + name: "real diagnostic", + output: `go: unlinkat C:\Users\RUNNER~1\AppData\Local\Temp\go-build860437867\b2679\list.test.exe: ` + + `The process cannot access the file because it is being used by another process.`, + want: true, + }, + {name: "empty output", output: "", want: false}, + { + name: "real test failure", + output: "--- FAIL: TestSomething (0.01s)\nFAIL\tgithub.com/cloudposse/atmos/cmd/list\t0.013s\n", + want: false, + }, + { + name: "unrelated file-in-use error", + output: "The process cannot access the file because it is being used by another process.", + want: false, // missing "unlinkat" -- not the Go toolchain's own cleanup diagnostic. + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := isTransientWindowsUnlinkError(tt.output); got != tt.want { + t.Fatalf("isTransientWindowsUnlinkError(%q) = %v, want %v", tt.output, got, tt.want) + } + }) + } +} + +func TestTransientErrorDetectorNilNeverMatches(t *testing.T) { + t.Parallel() + + var d *transientErrorDetector + if d.matched() { + t.Fatal("nil detector must never report a match") + } +} + +func TestTransientErrorDetectorMatchesWithinOneWrite(t *testing.T) { + t.Parallel() + + d := &transientErrorDetector{} + line := []byte("go: unlinkat C:\\pkg.test.exe: The process cannot access the file because it is being used by another process.\n") + n, err := d.Write(line) + if err != nil { + t.Fatalf("Write() error = %v", err) + } + if n != len(line) { + t.Fatalf("Write() n = %d, want %d (io.Writer contract: n == len(p))", n, len(line)) + } + if !d.matched() { + t.Fatal("expected a match after writing the diagnostic in one call") + } +} + +func TestTransientErrorDetectorMatchesAcrossWrites(t *testing.T) { + t.Parallel() + + d := &transientErrorDetector{} + if _, err := d.Write([]byte("go: unlinkat C:\\pkg.test.exe: ")); err != nil { + t.Fatalf("Write() error = %v", err) + } + if d.matched() { + t.Fatal("must not match on a partial write") + } + if _, err := d.Write([]byte("The process cannot access the file because it is being used by another process.\n")); err != nil { + t.Fatalf("Write() error = %v", err) + } + if !d.matched() { + t.Fatal("expected a match once both halves have been written") + } +} + +func TestTransientErrorDetectorDoesNotMatchUnrelatedOutput(t *testing.T) { + t.Parallel() + + d := &transientErrorDetector{} + for range 5 { + if _, err := d.Write([]byte("--- PASS: TestSomething (0.01s)\n")); err != nil { + t.Fatalf("Write() error = %v", err) + } + } + if d.matched() { + t.Fatal("expected no match for ordinary passing-test output") + } +} + +// TestTransientErrorDetectorBoundsMemory reproduces the risk CodeRabbit flagged: +// stderrCapture previously retained everything a verbose command wrote, unbounded, +// even when the diagnostic never appeared. The detector must cap its retained window +// at maxTransientMatchWindow regardless of how much unrelated output it sees. +func TestTransientErrorDetectorBoundsMemory(t *testing.T) { + t.Parallel() + + d := &transientErrorDetector{} + line := strings.Repeat("v", 512) + "\n" + for range 50 { // 50 * 513 bytes >> maxTransientMatchWindow (4096). + if _, err := d.Write([]byte(line)); err != nil { + t.Fatalf("Write() error = %v", err) + } + } + if d.matched() { + t.Fatal("expected no match for repeated unrelated output") + } + if len(d.window) > maxTransientMatchWindow { + t.Fatalf("retained window = %d bytes, want at most %d", len(d.window), maxTransientMatchWindow) + } +} + +// TestTransientErrorDetectorDropsWindowOnceMatched confirms the window is freed after +// a match, since run only needs the sticky bool from then on -- a long-running command +// that keeps writing after the diagnostic appears must not keep growing memory either. +func TestTransientErrorDetectorDropsWindowOnceMatched(t *testing.T) { + t.Parallel() + + d := &transientErrorDetector{} + if _, err := d.Write([]byte("go: unlinkat x: The process cannot access the file because it is being used by another process.\n")); err != nil { + t.Fatalf("Write() error = %v", err) + } + if !d.matched() { + t.Fatal("expected a match") + } + if d.window != nil { + t.Fatalf("window = %q, want nil once matched", d.window) + } + if _, err := d.Write([]byte(strings.Repeat("more output\n", 1000))); err != nil { + t.Fatalf("Write() error = %v", err) + } + if d.window != nil { + t.Fatalf("window = %d bytes after further writes, want it to stay nil once matched", len(d.window)) + } + if !d.matched() { + t.Fatal("match must remain sticky after further writes") + } +} + +// newHelperRunner builds a commandRunner whose run() re-execs this test binary via +// runUnlinkRaceHelper, with retryDelay zeroed so the test doesn't pay the real +// (Windows-only) retry backoff. +func newHelperRunner(t *testing.T) commandRunner { + t.Helper() + var stdout, stderr bytes.Buffer + return commandRunner{stdout: &stdout, stderr: &stderr, stdin: nil, retryDelay: 0} +} + +func TestRunRetriesTransientWindowsUnlinkError(t *testing.T) { + exePath, err := os.Executable() + if err != nil { + t.Fatalf("resolve test binary path: %v", err) + } + + countFile := filepath.Join(t.TempDir(), "fail-count") + if err := os.WriteFile(countFile, []byte("1"), 0o600); err != nil { + t.Fatalf("write initial fail count to %s: %v", countFile, err) + } + + runner := newHelperRunner(t) + env := []string{helperFailCountEnv + "=" + countFile} + if err := runner.run(context.Background(), runOptions{dir: t.TempDir(), env: env, retryTransient: true}, exePath); err != nil { + t.Fatalf("run() did not recover from a single transient failure: %v", err) + } +} + +func TestRunGivesUpAfterExhaustingRetries(t *testing.T) { + exePath, err := os.Executable() + if err != nil { + t.Fatalf("resolve test binary path: %v", err) + } + + countFile := filepath.Join(t.TempDir(), "fail-count") + // One more failure than run() will retry, so the final attempt still fails. + if err := os.WriteFile(countFile, []byte(strconv.Itoa(transientUnlinkRetries+1)), 0o600); err != nil { + t.Fatalf("write initial fail count to %s: %v", countFile, err) + } + + runner := newHelperRunner(t) + env := []string{helperFailCountEnv + "=" + countFile} + if err := runner.run(context.Background(), runOptions{dir: t.TempDir(), env: env, retryTransient: true}, exePath); err == nil { + t.Fatal("expected run() to fail once transient retries are exhausted") + } +} + +func TestRunDoesNotRetryUnrelatedFailures(t *testing.T) { + exePath, err := os.Executable() + if err != nil { + t.Fatalf("resolve test binary path: %v", err) + } + + // A missing count file makes the helper exit 2 immediately (an unrelated failure), + // never printing the transient diagnostic -- run() must not retry this. + countFile := filepath.Join(t.TempDir(), "fail-count-does-not-exist") + + runner := newHelperRunner(t) + env := []string{helperFailCountEnv + "=" + countFile} + if err := runner.run(context.Background(), runOptions{dir: t.TempDir(), env: env, retryTransient: true}, exePath); err == nil { + t.Fatal("expected run() to fail for an unrelated (non-transient) error") + } +} + +// TestRunDoesNotRetryWhenNotRetryTransient reproduces the risk CodeRabbit flagged: +// commandRunner.run must never retry a non-`go test` invocation (precompiled test +// binaries, `go test -c`, `go tool covdata`) even if its output happens to contain +// both substrings isTransientWindowsUnlinkError matches -- retrying those could rerun +// a command with real side effects or mask a genuine, unrelated failure. This drives +// the helper to print the exact transient diagnostic on its first invocation, but with +// retryTransient=false run() must still fail on that first attempt. +func TestRunDoesNotRetryWhenNotRetryTransient(t *testing.T) { + exePath, err := os.Executable() + if err != nil { + t.Fatalf("resolve test binary path: %v", err) + } + + countFile := filepath.Join(t.TempDir(), "fail-count") + if err := os.WriteFile(countFile, []byte("1"), 0o600); err != nil { + t.Fatalf("write initial fail count to %s: %v", countFile, err) + } + + runner := newHelperRunner(t) + env := []string{helperFailCountEnv + "=" + countFile} + if err := runner.run(context.Background(), runOptions{dir: t.TempDir(), env: env}, exePath); err == nil { + t.Fatal("expected run() to fail immediately for a non-go-test invocation, even with a matching transient diagnostic") + } + + // Confirm it was never retried: the helper only decrements the counter (and would + // have exited 0 on a second call) when actually invoked again. + remaining, err := os.ReadFile(countFile) + if err != nil { + t.Fatalf("read fail count from %s: %v", countFile, err) + } + if string(remaining) != "0" { + t.Fatalf("fail count = %q, want %q (run() must invoke the command exactly once)", remaining, "0") + } +} diff --git a/internal/ci/acceptance/coverage.go b/internal/ci/acceptance/coverage.go index 2b3cc960871..f59762fcb93 100644 --- a/internal/ci/acceptance/coverage.go +++ b/internal/ci/acceptance/coverage.go @@ -61,7 +61,7 @@ func CollectCoverage(ctx context.Context, options *CoverageOptions, packages, te args = append(args, "-cover", "-covermode=atomic", "-coverpkg=./...") args = append(args, testArgs...) args = append(args, "-timeout", defaultValue(options.Timeout, defaultTestTimeout), "-args", "-test.gocoverdir="+absUnit) - if err := runner.run(ctx, options.RepoRoot, goCommandEnvironment("GOCOVERDIR="+absIntegration), "go", args...); err != nil { + if err := runner.run(ctx, runOptions{dir: options.RepoRoot, env: goCommandEnvironment("GOCOVERDIR=" + absIntegration), retryTransient: true}, "go", args...); err != nil { return err } @@ -88,7 +88,7 @@ func MergeCoverage(ctx context.Context, repoRoot, dataOut, textOut string, input return err } runner := newCommandRunner() - if err := runner.run(ctx, repoRoot, nil, "go", "tool", "covdata", "merge", "-pcombine", "-i="+strings.Join(normalizedInputs, ","), "-o="+dataOut); err != nil { + if err := runner.run(ctx, runOptions{dir: repoRoot}, "go", "tool", "covdata", "merge", "-pcombine", "-i="+strings.Join(normalizedInputs, ","), "-o="+dataOut); err != nil { return err } if textOut != "" { @@ -172,7 +172,7 @@ func writeCoverageText(ctx context.Context, runner commandRunner, repoRoot, data return fmt.Errorf("close temporary coverage profile: %w", closeErr) } defer func() { _ = os.Remove(rawPath) }() - if err := runner.run(ctx, repoRoot, nil, "go", "tool", "covdata", "textfmt", "-i="+dataOut, "-o="+rawPath); err != nil { + if err := runner.run(ctx, runOptions{dir: repoRoot}, "go", "tool", "covdata", "textfmt", "-i="+dataOut, "-o="+rawPath); err != nil { return err } if err := filterCoverageProfile(rawPath, textOut); err != nil { diff --git a/internal/ci/acceptance/run.go b/internal/ci/acceptance/run.go index 2bb7c79142a..655e16bf89e 100644 --- a/internal/ci/acceptance/run.go +++ b/internal/ci/acceptance/run.go @@ -160,7 +160,7 @@ func runSourceTestGroup( args = append(args, group.packages...) args = append(args, group.testArgs...) args = append(args, "-timeout", defaultValue(options.GoTestTimeout, defaultTestTimeout)) - return "", runner.run(ctx, options.RepoRoot, goCommandEnvironment(), "go", args...) + return "", runner.run(ctx, runOptions{dir: options.RepoRoot, env: goCommandEnvironment(), retryTransient: true}, "go", args...) } func runWindowsTests(ctx context.Context, runner commandRunner, options *RunOptions) error { @@ -175,7 +175,7 @@ func runWindowsTests(ctx context.Context, runner commandRunner, options *RunOpti } assigned := windowsTestsForShard(tests, options.Shard) args := []string{"-test.run=" + testRunPattern(assigned), "-test.timeout=" + defaultValue(options.GoTestTimeout, defaultTestTimeout)} - return runner.run(ctx, dir, nil, binary, args...) + return runner.run(ctx, runOptions{dir: dir}, binary, args...) } func runWindowsExecTests(ctx context.Context, runner commandRunner, options *RunOptions) error { @@ -193,7 +193,7 @@ func runWindowsExecTests(ctx context.Context, runner commandRunner, options *Run return nil } args := []string{"-test.run=" + testRunPattern(assigned), "-test.timeout=" + defaultValue(options.GoTestTimeout, defaultTestTimeout)} - return runner.run(ctx, dir, nil, binary, args...) + return runner.run(ctx, runOptions{dir: dir}, binary, args...) } func runCmdTests(ctx context.Context, runner commandRunner, options *RunOptions) (string, error) { @@ -228,7 +228,7 @@ func runCmdTests(ctx context.Context, runner commandRunner, options *RunOptions) coverDir = temporary } absCoverDir := absoluteFromRoot(options.RepoRoot, coverDir) - if err := runner.run(ctx, dir, []string{"GOCOVERDIR=" + absCoverDir}, binary, args...); err != nil { + if err := runner.run(ctx, runOptions{dir: dir, env: []string{"GOCOVERDIR=" + absCoverDir}}, binary, args...); err != nil { return "", err } if options.Mode == ModeCoverage { @@ -247,7 +247,7 @@ func Precompile(ctx context.Context, repoRoot string, target Target, outputDir s if target == TargetWindows { extension = ".exe" } - if err := runner.run(ctx, repoRoot, goCommandEnvironment(), "go", "test", "-c", "-covermode=atomic", "-coverpkg=./...", + if err := runner.run(ctx, runOptions{dir: repoRoot, env: goCommandEnvironment()}, "go", "test", "-c", "-covermode=atomic", "-coverpkg=./...", "-o", filepath.Join(outputDir, "cmd.test"+extension), "./cmd"); err != nil { return err } @@ -261,7 +261,7 @@ func Precompile(ctx context.Context, repoRoot string, target Target, outputDir s {name: "tests.test.exe", pkgPath: "./tests"}, {name: "internal-exec.test.exe", pkgPath: "./internal/exec"}, } { - if err := runner.run(ctx, repoRoot, goCommandEnvironment(), "go", "test", "-c", + if err := runner.run(ctx, runOptions{dir: repoRoot, env: goCommandEnvironment()}, "go", "test", "-c", "-o", filepath.Join(outputDir, testBinary.name), testBinary.pkgPath); err != nil { return err } diff --git a/pkg/ai/tools/atmos/toolchain_set_test.go b/pkg/ai/tools/atmos/toolchain_set_test.go index 5f9b7114661..ddea51c7b7f 100644 --- a/pkg/ai/tools/atmos/toolchain_set_test.go +++ b/pkg/ai/tools/atmos/toolchain_set_test.go @@ -64,10 +64,13 @@ func TestToolchainSetTool_Execute(t *testing.T) { require.NoError(t, err) require.True(t, result.Success) + // Written under the raw "terraform" key (what was passed), not the resolved + // canonical "hashicorp/terraform" form -- see pkg/toolchain/set_test.go's + // TestSetToolVersion_WithValidVersion for the underlying contract. toolVersions, err := toolchain.LoadToolVersions(toolVersionsFile) require.NoError(t, err) - assert.Contains(t, toolVersions.Tools, "hashicorp/terraform") - assert.Contains(t, toolVersions.Tools["hashicorp/terraform"], "1.11.4") + assert.Contains(t, toolVersions.Tools, "terraform") + assert.Contains(t, toolVersions.Tools["terraform"], "1.11.4") }) t.Run("fails with missing version", func(t *testing.T) { diff --git a/pkg/config/default.go b/pkg/config/default.go index be9e1e50294..01be1fd97e8 100644 --- a/pkg/config/default.go +++ b/pkg/config/default.go @@ -125,6 +125,9 @@ var ( }, }, Initialized: true, + Toolchain: schema.Toolchain{ + UseLockFile: true, // Changed from false to true since PR toolchain-lockfile-default (journaled in pkg/edition). + }, Version: schema.Version{ Check: schema.VersionCheck{ Enabled: true, diff --git a/pkg/config/load.go b/pkg/config/load.go index 9142f79f652..ae67c1209e4 100644 --- a/pkg/config/load.go +++ b/pkg/config/load.go @@ -894,6 +894,11 @@ func setDefaultConfiguration(v *viper.Viper) { v.SetDefault("cast.recording.height", 36) v.SetDefault("docs.generate.readme.output", "./README.md") + // Toolchain lockfile is written by default for reproducible installs across + // platforms and CI (journaled in pkg/edition; previously opt-in via + // use_lock_file: true). + v.SetDefault("toolchain.use_lock_file", true) + // Atmos Pro defaults v.SetDefault("settings.pro.base_url", AtmosProDefaultBaseUrl) v.SetDefault("settings.pro.endpoint", AtmosProDefaultEndpoint) diff --git a/pkg/config/testdata/default-config-snapshot.yaml b/pkg/config/testdata/default-config-snapshot.yaml index a82e941c490..ffeda03a956 100644 --- a/pkg/config/testdata/default-config-snapshot.yaml +++ b/pkg/config/testdata/default-config-snapshot.yaml @@ -30,3 +30,4 @@ settings.terminal.no_color: false settings.terminal.pager: "false" settings.terminal.speed: 0 stacks.inherit.metadata: true +toolchain.use_lock_file: true diff --git a/pkg/edition/journal.go b/pkg/edition/journal.go index de7174d5cb8..96fbe4450be 100644 --- a/pkg/edition/journal.go +++ b/pkg/edition/journal.go @@ -154,6 +154,15 @@ var journal = []Entry{ Description: "Helmfile EKS integration is opt-in; kubeconfig is no longer downloaded automatically before Helmfile commands.", Ref: "https://github.com/cloudposse/atmos/pull/1903", }, + { + Date: "2026-08-05", + Key: "toolchain.use_lock_file", + Kind: KindValue, + Old: false, + New: true, + Description: "The toolchain writes toolchain.lock.yaml by default, pinning resolved tool versions and checksums for reproducible installs across platforms and CI.", + Ref: "https://atmos.tools/changelog/toolchain-lockfile-default", + }, } // Journal returns a copy of the journal sorted by date (oldest first), then key. diff --git a/pkg/toolchain/set.go b/pkg/toolchain/set.go index c5d819b84ee..0cf3a260ebe 100644 --- a/pkg/toolchain/set.go +++ b/pkg/toolchain/set.go @@ -351,13 +351,19 @@ func SetToolVersion(toolName, version string, scrollSpeed int) error { // Set the tool's default version. Always replace (not append) so `set` // matches its documented purpose and never produces a multi-version // .tool-versions line that leaves a stale version as the default. + // + // Write under toolName (the string the user passed), not spec.key (the + // resolved owner/repo form) -- matching AddToolVersion's (add.go) pattern. + // .tool-versions is keyed by whatever string is already on disk (often a + // short alias like "jq"); writing the canonical form here would create a + // second, disconnected entry instead of updating the existing one. filePath := GetToolVersionsFilePath() - err = AddToolToVersionsAsDefault(filePath, spec.key, version) + err = AddToolToVersionsAsDefault(filePath, toolName, version) if err != nil { return fmt.Errorf("failed to set version: %w", err) } - ui.Successf("Set %s@%s in %s", spec.key, version, filePath) + ui.Successf("Set %s@%s in %s", toolName, version, filePath) return nil } diff --git a/pkg/toolchain/set_test.go b/pkg/toolchain/set_test.go index cc6bc1fde04..75d2c521301 100644 --- a/pkg/toolchain/set_test.go +++ b/pkg/toolchain/set_test.go @@ -1941,10 +1941,14 @@ func TestSetToolVersion_WithValidVersion(t *testing.T) { err = SetToolVersion("terraform", "1.11.4", 3) assert.NoError(t, err) - // Verify the file was updated + // Verify the file was updated. Written under the raw "terraform" key (what the + // caller passed), not the resolved canonical "hashicorp/terraform" form -- + // matching AddToolVersion's (add.go) established contract, see + // TestAddCommand_ValidTool. content, err := os.ReadFile(tmpFile.Name()) require.NoError(t, err) - assert.Contains(t, string(content), "hashicorp/terraform") + assert.Contains(t, string(content), "terraform") + assert.NotContains(t, string(content), "hashicorp/terraform") assert.Contains(t, string(content), "1.11.4") } @@ -1986,6 +1990,48 @@ func TestSetToolVersion_ReplacesExistingDefault(t *testing.T) { assert.Equal(t, []string{"1.11.4"}, versions, "set on a single-version tool must not leave the old default (1.5.7) pinned as a stale second entry") } +// TestSetToolVersion_UpdatesExistingAliasKeyInPlace tests that SetToolVersion updates +// the existing entry when the file already tracks the tool under a configured short +// alias (e.g. "jq" for "jqlang/jq", as in the atmos-migration skill's from-mise.md +// recipe), instead of writing a second, disconnected entry under the resolved +// owner/repo form. Regression coverage for SetToolVersion writing spec.key (always +// the canonical form) instead of toolName (the string the user/on-disk file actually +// uses), which AddToolVersion (add.go) already gets right. +func TestSetToolVersion_UpdatesExistingAliasKeyInPlace(t *testing.T) { + setupTestIO(t) + + tmpFile, err := os.CreateTemp("", "tool-versions-*") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + // Seed the file with an existing default version under the short alias key, + // matching what a real .tool-versions file looks like per the migration recipe. + toolVersions := &ToolVersions{Tools: make(map[string][]string)} + AddVersionToTool(toolVersions, "jq", "1.7.1", false) + require.NoError(t, SaveToolVersions(tmpFile.Name(), toolVersions)) + + oldConfig := atmosConfig + defer func() { atmosConfig = oldConfig }() + atmosConfig = &schema.AtmosConfiguration{ + Toolchain: schema.Toolchain{ + VersionsFile: tmpFile.Name(), + Aliases: map[string]string{"jq": "jqlang/jq"}, + }, + } + + err = SetToolVersion("jq", "1.9.0", 3) + assert.NoError(t, err) + + loaded, err := LoadToolVersions(tmpFile.Name()) + require.NoError(t, err) + require.Len(t, loaded.Tools, 1, "should update the existing 'jq' entry, not add a second entry under the canonical form") + // set replaces (not merges into) the default, matching TestSetToolVersion_ReplacesExistingDefault -- + // the old 1.7.1 must not survive as a stale second entry under "jq". + assert.Equal(t, []string{"1.9.0"}, loaded.Tools["jq"], "set should replace the existing alias-keyed entry, not append to it") + _, hasCanonicalKey := loaded.Tools["jqlang/jq"] + assert.False(t, hasCanonicalKey, "must not create a second entry under the resolved owner/repo form") +} + // TestSetToolVersion_RejectsRangeSyntax reproduces a gap where SetToolVersion could write // invalid SemVer range/constraint syntax (e.g. "^1.7.0") straight into .tool-versions without // validation, unlike the add/install paths which already reject it via ValidateVersionSpec. diff --git a/pkg/toolchain/tool_versions.go b/pkg/toolchain/tool_versions.go index c4b9de28a65..662fd39bc75 100644 --- a/pkg/toolchain/tool_versions.go +++ b/pkg/toolchain/tool_versions.go @@ -184,7 +184,16 @@ func addToolToVersionsInternal(filePath, tool, version string, asDefault bool) e installer := NewInstaller() resolver := installer.GetResolver() - if wouldCreateDuplicate(toolVersions, tool, version, resolver) { + if duplicateKey := findDuplicateKey(toolVersions, tool, version, resolver); duplicateKey != "" { + // The version is already tracked under a different key (an alias vs. its + // canonical owner/repo form, or vice versa). Don't create a second, + // disconnected entry -- but when the caller wants this version to become + // the default, promote it within its existing key instead of silently + // doing nothing. + if asDefault { + AddVersionToTool(toolVersions, duplicateKey, version, true) + return saveToolVersionsUnlocked(filePath, toolVersions) + } return nil } @@ -207,52 +216,56 @@ func withToolVersionsSharedLock(filePath string, fn func() error) error { return filelock.New(filePath+".lock").WithShared(context.Background(), fn) } -// wouldCreateDuplicate checks if adding a tool/version combination would create a duplicate -// with an existing aliased version. For example, if "opentofu/opentofu 1.10.3" already exists, -// adding "opentofu 1.10.3" would create a duplicate. -func wouldCreateDuplicate(toolVersions *ToolVersions, tool, version string, resolver ToolResolver) bool { +// findDuplicateKey checks whether adding a tool/version combination would create a duplicate +// with an existing aliased version, and if so returns the key under which the version is +// already tracked. For example, if "opentofu/opentofu 1.10.3" already exists, adding +// "opentofu 1.10.3" would create a duplicate, and findDuplicateKey returns "opentofu/opentofu". +// Returns "" when there is no duplicate. +func findDuplicateKey(toolVersions *ToolVersions, tool, version string, resolver ToolResolver) string { // Check if the tool is an alias that conflicts with an existing full name. - if aliasConflictsWithFullName(toolVersions, tool, version, resolver) { - return true + if key := aliasConflictsWithFullName(toolVersions, tool, version, resolver); key != "" { + return key } // Check if the tool is a full name that conflicts with an existing alias. - if fullNameConflictsWithAlias(toolVersions, tool, version, resolver) { - return true + if key := fullNameConflictsWithAlias(toolVersions, tool, version, resolver); key != "" { + return key } - return false + return "" } // aliasConflictsWithFullName checks if an alias conflicts with an existing full name entry. -// For example, if "opentofu/opentofu 1.10.3" already exists, adding "opentofu 1.10.3" would be a duplicate. -func aliasConflictsWithFullName(toolVersions *ToolVersions, tool, version string, resolver ToolResolver) bool { +// For example, if "opentofu/opentofu 1.10.3" already exists, adding "opentofu 1.10.3" would be a +// duplicate. Returns the conflicting key ("opentofu/opentofu"), or "" if there is none. +func aliasConflictsWithFullName(toolVersions *ToolVersions, tool, version string, resolver ToolResolver) string { // Check if the tool is an alias (e.g., "opentofu"). owner, repo, err := resolver.Resolve(tool) if err != nil || owner == "" || repo == "" { - return false + return "" } // This is an alias, check if the full name already exists. aliasKey := owner + "/" + repo versions, ok := toolVersions.Tools[aliasKey] if !ok { - return false + return "" } // Check if any existing version matches. for _, v := range versions { if v == version { - return true // Duplicate found. + return aliasKey // Duplicate found. } } - return false + return "" } // fullNameConflictsWithAlias checks if a full name conflicts with an existing alias entry. -// For example, if "opentofu 1.10.3" already exists, adding "opentofu/opentofu 1.10.3" would be a duplicate. -func fullNameConflictsWithAlias(toolVersions *ToolVersions, tool, version string, resolver ToolResolver) bool { +// For example, if "opentofu 1.10.3" already exists, adding "opentofu/opentofu 1.10.3" would be a +// duplicate. Returns the conflicting key ("opentofu"), or "" if there is none. +func fullNameConflictsWithAlias(toolVersions *ToolVersions, tool, version string, resolver ToolResolver) string { // Check if this is a full name (e.g., "opentofu/opentofu") and if an alias exists // that resolves to this full name. for existingTool, versions := range toolVersions.Tools { @@ -276,12 +289,12 @@ func fullNameConflictsWithAlias(toolVersions *ToolVersions, tool, version string // Check if any version matches. for _, v := range versions { if v == version { - return true // Duplicate found. + return existingTool // Duplicate found. } } } - return false + return "" } // LookupToolVersion attempts to find the version for a tool, trying both the raw name and its resolved alias. diff --git a/pkg/toolchain/tool_versions_test.go b/pkg/toolchain/tool_versions_test.go index d0fb280a398..6e9625cb5dc 100644 --- a/pkg/toolchain/tool_versions_test.go +++ b/pkg/toolchain/tool_versions_test.go @@ -315,6 +315,55 @@ func TestAddToolToVersionsAsDefault(t *testing.T) { assert.ErrorIs(t, err, ErrInvalidToolSpec) }) + t.Run("Sets an already-tracked version as the sole default under the same key", func(t *testing.T) { + // Regression test: "atmos toolchain set jq 1.7.1" must make 1.7.1 the sole + // entry when .tool-versions already contains "jq 1.9.0 1.7.1" -- both + // versions tracked under the same "jq" key. AddVersionToTool's asDefault + // path always fully replaces (see its doc comment): the stale 1.9.0 must + // not survive as a second entry. + tempDir := t.TempDir() + filePath := filepath.Join(tempDir, DefaultToolVersionsFilePath) + + err := AddToolToVersions(filePath, "jq", "1.9.0") + require.NoError(t, err) + err = AddToolToVersions(filePath, "jq", "1.7.1") + require.NoError(t, err) + + err = AddToolToVersionsAsDefault(filePath, "jq", "1.7.1") + require.NoError(t, err) + + toolVersions, err := LoadToolVersions(filePath) + require.NoError(t, err) + assert.Equal(t, []string{"1.7.1"}, toolVersions.Tools["jq"]) + }) + + t.Run("Promotes an already-tracked version under a different (alias/canonical) key", func(t *testing.T) { + // Regression test: when the version is already tracked under a different + // key form than the one the caller passed (e.g. the file stores the + // canonical "opentofu/opentofu" entry but the caller asks to promote a + // version by the "opentofu" alias), findDuplicateKey finds the conflict. + // Setting asDefault=true must still promote the version within its + // existing key instead of silently doing nothing -- and, per + // AddVersionToTool's asDefault contract, fully replace the list there + // rather than leaving the old version pinned alongside it. + tempDir := t.TempDir() + filePath := filepath.Join(tempDir, DefaultToolVersionsFilePath) + + err := AddToolToVersions(filePath, "opentofu/opentofu", "1.10.3") + require.NoError(t, err) + err = AddToolToVersions(filePath, "opentofu/opentofu", "1.10.2") + require.NoError(t, err) + + // Promote 1.10.2 to default using the alias form of the tool name. + err = AddToolToVersionsAsDefault(filePath, "opentofu", "1.10.2") + require.NoError(t, err) + + toolVersions, err := LoadToolVersions(filePath) + require.NoError(t, err) + assert.Equal(t, []string{"1.10.2"}, toolVersions.Tools["opentofu/opentofu"]) + assert.NotContains(t, toolVersions.Tools, "opentofu", "should not create a second, disconnected alias entry") + }) + // TestAddToolToVersionsAsDefault/Single-version_tool_ends_up_with_exactly_one_version // reproduces the most common real-world case (a tool pinned to a single version, e.g. from // `add`, then bumped via `set`, `add --default`, or `update`). set's and update's own docs diff --git a/pkg/toolchain/which_test.go b/pkg/toolchain/which_test.go index 7774ea56517..b7e8cc940b8 100644 --- a/pkg/toolchain/which_test.go +++ b/pkg/toolchain/which_test.go @@ -194,7 +194,7 @@ func TestWhichCommand_CanonicalName(t *testing.T) { // TestWhichCommand_StoredAsCanonicalLookupByAlias reproduces a bug where // .tool-versions stores a tool under its canonical owner/repo key (e.g. -// "helm/helm") — which is what the write-side dedup (wouldCreateDuplicate) +// "helm/helm") — which is what the write-side dedup (findDuplicateKey) // keeps when install paths canonicalize — but findBinaryPath does a raw // map lookup and misses when the caller asks by alias (e.g. "helm"). // diff --git a/pkg/ui/markdown/custom_renderer_test.go b/pkg/ui/markdown/custom_renderer_test.go index 0b4fbef1055..ff8de6be5a4 100644 --- a/pkg/ui/markdown/custom_renderer_test.go +++ b/pkg/ui/markdown/custom_renderer_test.go @@ -202,6 +202,64 @@ func TestCustomRenderer_Render_Strikethrough(t *testing.T) { assert.Contains(t, stripped, "text") } +// TestCustomRenderer_Render_PackageRefLinkify is regression coverage for a bug where +// package-ref-shaped text (owner/repo@version, or bare tool@version) that goldmark's +// GFM autolink pass mistook for an email address and the strict-linkify extension +// un-linked, disappeared entirely from glamour's rendered ANSI output instead of +// rendering as plain text. TestStrictLinkifyExtension in +// pkg/ui/markdown/extensions/extensions_test.go only asserts on goldmark's +// intermediate HTML output, which does render the node type the old code produced -- +// only glamour's ANSI renderer, used here via the real CustomRenderer, did not. +func TestCustomRenderer_Render_PackageRefLinkify(t *testing.T) { + renderer, err := NewCustomRenderer(WithColorProfile(termenv.TrueColor)) + require.NoError(t, err) + + tests := []struct { + name string + input string + mustContain []string + wantCount map[string]int + }{ + { + name: "owner/repo@version in a success message", + input: "Set jqlang/jq@1.9.0 in .tool-versions", + mustContain: []string{"jqlang/jq@1.9.0"}, + }, + { + name: "bare tool@version", + input: "Set jq@1.9.0 in .tool-versions", + mustContain: []string{"jq@1.9.0"}, + }, + { + name: "two package refs in one message", + input: "Updated jq from jqlang/jq@1.7.1 to jqlang/jq@1.9.0", + mustContain: []string{"jqlang/jq@1.7.1", "jqlang/jq@1.9.0"}, + }, + { + // Regression coverage for an implementation that dedupes by label and + // always keeps only the first occurrence -- that would pass the "two + // different refs" case above but drop a genuinely repeated reference. + name: "same package ref twice", + input: "Updated jqlang/jq@1.9.0 then jqlang/jq@1.9.0", + wantCount: map[string]int{"jqlang/jq@1.9.0": 2}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := renderer.Render(tt.input) + assert.NoError(t, err) + stripped := stripANSIForTest(result) + for _, want := range tt.mustContain { + assert.Contains(t, stripped, want, "rendered output: %q", stripped) + } + for want, count := range tt.wantCount { + assert.Equal(t, count, strings.Count(stripped, want), "rendered output: %q", stripped) + } + }) + } +} + func TestCustomRenderer_Render_Highlight(t *testing.T) { renderer, err := NewCustomRenderer(WithColorProfile(termenv.TrueColor)) require.NoError(t, err) diff --git a/tests/floci_harness_test.go b/tests/floci_harness_test.go index 27efb9b22d7..1960999f5a8 100644 --- a/tests/floci_harness_test.go +++ b/tests/floci_harness_test.go @@ -201,22 +201,64 @@ func requireFlociEndpoint(t *testing.T, endpointEnvVar, defaultEndpoint string) address = net.JoinHostPort(parsed.Hostname(), port) } - conn, err := net.DialTimeout("tcp", address, 2*time.Second) - require.NoErrorf(t, err, "Floci is not reachable at %s", endpoint) - require.NoError(t, conn.Close()) + // Poll for up to flociStartupTimeout instead of checking once: CI pre-supplies + // these endpoints via GitHub Actions service containers, which are only + // guaranteed to have started the container process, not to have the app inside + // actually serving HTTP yet -- the same cold-start window the local + // testcontainers auto-start path already tolerates via + // wait.ForHTTP(...).WithStartupTimeout(flociStartupTimeout) in + // startFlociContainer. A single 2s check here previously failed + // TestAzureSecretsFlociE2E with the TCP port already accepting connections but + // the HTTP handler not yet ready to respond. + err = pollUntil(flociStartupTimeout, func() error { + conn, dialErr := net.DialTimeout("tcp", address, 2*time.Second) + if dialErr != nil { + return dialErr + } + defer func() { _ = conn.Close() }() - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) - require.NoError(t, err) - // #nosec G107 -- endpoint is an opt-in local Floci test target. - resp, err := http.DefaultClient.Do(req) - require.NoErrorf(t, err, "Floci HTTP endpoint is not reachable at %s", endpoint) - require.NoError(t, resp.Body.Close()) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + req, reqErr := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if reqErr != nil { + return reqErr + } + // #nosec G107 -- endpoint is an opt-in local Floci test target. + resp, respErr := http.DefaultClient.Do(req) + if respErr != nil { + return respErr + } + return resp.Body.Close() + }) + require.NoErrorf(t, err, "Floci HTTP endpoint is not reachable at %s within %s", endpoint, flociStartupTimeout) return endpoint } +// pollUntil retries fn every 500ms until it succeeds or timeout elapses, returning +// fn's last error on timeout. The deadline is checked before each call to fn so a +// slow fn can't push the total run time past timeout by an extra call, and the +// retry sleep is capped to the time remaining so it never oversleeps the deadline. +func pollUntil(timeout time.Duration, fn func() error) error { + deadline := time.Now().Add(timeout) + var lastErr error + for { + if time.Now().After(deadline) { + return lastErr + } + if lastErr = fn(); lastErr == nil { + return nil + } + if remaining := time.Until(deadline); remaining > 0 { + sleep := 500 * time.Millisecond + if remaining < sleep { + sleep = remaining + } + time.Sleep(sleep) + } + } +} + // resolveFlociEndpoint determines the Floci endpoint to use and reports whether it // came from an explicitly set environment variable (as opposed to a built-in // default). The resolution order mirrors the long-standing harness behavior: @@ -337,3 +379,43 @@ func uniqueFlociTestID(t *testing.T) string { name = strings.Trim(name, "-") return fmt.Sprintf("%d-%s", time.Now().UnixNano(), name) } + +func TestPollUntilSucceedsImmediately(t *testing.T) { + t.Parallel() + + calls := 0 + err := pollUntil(time.Second, func() error { + calls++ + return nil + }) + require.NoError(t, err) + require.Equal(t, 1, calls, "fn should not be retried once it succeeds") +} + +func TestPollUntilRetriesUntilSuccessWithinBudget(t *testing.T) { + t.Parallel() + + calls := 0 + err := pollUntil(5*time.Second, func() error { + calls++ + if calls < 3 { + return errors.New("not ready yet") + } + return nil + }) + require.NoError(t, err) + require.Equal(t, 3, calls, "should stop retrying as soon as fn succeeds") +} + +func TestPollUntilReturnsLastErrorOnTimeout(t *testing.T) { + t.Parallel() + + sentinel := errors.New("still not ready") + calls := 0 + err := pollUntil(600*time.Millisecond, func() error { + calls++ + return sentinel + }) + require.ErrorIs(t, err, sentinel) + require.Greater(t, calls, 1, "should have retried at least once before the budget elapsed") +} diff --git a/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden b/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden index d7132e9373e..9e1f8497dd1 100644 --- a/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden @@ -130,5 +130,12 @@ auth: container: runtime: auto_start: true +toolchain: + install_path: "" + file_path: "" + tools_dir: "" + versions_file: "" + use_tool_versions: false + use_lock_file: true list: error_mode: warn diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden index 1ab1d073791..47456a5d607 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden @@ -316,7 +316,7 @@ "tools_dir": "", "versions_file": "", "use_tool_versions": false, - "use_lock_file": false + "use_lock_file": true }, "git": { "list": {} diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden index e4f513b6d23..bd3e57f9a31 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden @@ -131,5 +131,12 @@ auth: container: runtime: auto_start: true +toolchain: + install_path: "" + file_path: "" + tools_dir: "" + versions_file: "" + use_tool_versions: false + use_lock_file: true list: error_mode: warn diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden index 7c1990eccdf..eeac2b700ab 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden @@ -149,5 +149,12 @@ auth: container: runtime: auto_start: true +toolchain: + install_path: "" + file_path: "" + tools_dir: "" + versions_file: "" + use_tool_versions: false + use_lock_file: true list: error_mode: warn diff --git a/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden index 7c9168c5db1..8004d804b33 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden @@ -145,5 +145,12 @@ auth: container: runtime: auto_start: true +toolchain: + install_path: "" + file_path: "" + tools_dir: "" + versions_file: "" + use_tool_versions: false + use_lock_file: true list: error_mode: warn diff --git a/tests/snapshots/TestCLICommands_indentation.stdout.golden b/tests/snapshots/TestCLICommands_indentation.stdout.golden index 7015b50bbde..69a541bc960 100644 --- a/tests/snapshots/TestCLICommands_indentation.stdout.golden +++ b/tests/snapshots/TestCLICommands_indentation.stdout.golden @@ -129,5 +129,12 @@ auth: container: runtime: auto_start: true +toolchain: + install_path: "" + file_path: "" + tools_dir: "" + versions_file: "" + use_tool_versions: false + use_lock_file: true list: error_mode: warn diff --git a/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden b/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden index 8593efc015c..e5619685021 100644 --- a/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden +++ b/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden @@ -325,7 +325,7 @@ "tools_dir": "", "versions_file": "", "use_tool_versions": false, - "use_lock_file": false + "use_lock_file": true }, "git": { "list": {} diff --git a/website/blog/2026-09-01-toolchain-lockfile-default.mdx b/website/blog/2026-09-01-toolchain-lockfile-default.mdx new file mode 100644 index 00000000000..c96ebcc4c00 --- /dev/null +++ b/website/blog/2026-09-01-toolchain-lockfile-default.mdx @@ -0,0 +1,55 @@ +--- +slug: toolchain-lockfile-default +title: "Toolchain Installs Are Reproducible by Default Now" +authors: [osterman] +tags: [experimental, enhancement] +--- + +Atmos's toolchain has had a lockfile for a while. The lockfile records the exact resolved artifact +and checksum for each platform. A package manager's lockfile pins a dependency tree the same way. +But the lockfile was opt-in. The setting was not documented anywhere a user could find it. Almost +nobody turned it on. Almost nobody's installs were actually reproducible. The installs only looked +reproducible, because the version string matched. + + + +## The Problem + +A version in `.tool-versions` pins what you asked for. It does not pin what Atmos actually +installed. Every `atmos toolchain install` command resolves that version against the live registry +again. The exact download URL, checksum, and platform artifact are not fixed anywhere. Only the +version number is fixed. + +## The Fix + +Toolchain installs now write a lockfile by default. You do not need to configure anything. Run +`atmos toolchain install`. Atmos records the exact resolved version, download URL, checksum, and +size for your platform. The next install resolves against that lockfile instead of asking the +registry again. This applies on a teammate's machine and in CI. Everyone on the same operating +system and architecture gets the same artifact, byte for byte. + +If a project's `atmos.yaml` pins an edition dated before this change, the project keeps the old +opt-in behavior. Nothing changes for a project that relied on the previous default. New and +unpinned projects get the lockfile from the start. + +## How to Use It + +You do not need to opt in. Install as usual: + +```shell +$ atmos toolchain install +``` + +Commit the resulting `toolchain.lock.yaml` file next to `.tool-versions`. After that, every install +resolves the exact pinned artifact. This applies on your machine, a teammate's machine, and a CI +runner. Atmos does not resolve the version against the registry again. + +The `atmos toolchain` command is still experimental. Its interface may change. The reproducibility +does not depend on that. + +## Get Involved + +If you already use the toolchain, check for `toolchain.lock.yaml` next to your `.tool-versions` +file after you run `atmos toolchain install`. Commit the lockfile. If your installs still do not +reproduce the same way across machines, [open an +issue](https://github.com/cloudposse/atmos/issues). That is exactly the gap this fix closes. diff --git a/website/src/data/roadmap.js b/website/src/data/roadmap.js index e0833ab1387..98005c4d074 100644 --- a/website/src/data/roadmap.js +++ b/website/src/data/roadmap.js @@ -230,6 +230,7 @@ export const roadmapConfig = { { label: 'macOS Gatekeeper/AMFI trust fix for verifier binary installs', status: 'shipped', quarter: 'q3-2026', pr: 2720, changelog: 'macos-toolchain-verifier-trust', docs: '/cli/configuration/toolchain/verification', description: 'On macOS, a toolchain-installed verifier binary (e.g. cosign, via `verifier_install: auto`) could pass checksum verification but still get killed by Gatekeeper/AMFI on first execution. Atmos now strips quarantine extended attributes and ad-hoc re-signs the binary, matching what Homebrew does, gated by a new `verifier_trust: auto|disabled` setting under `toolchain.verification` (default `auto`). No-op on Linux/Windows.', benefits: 'Toolchain-installed verifier binaries run on macOS without being killed by Gatekeeper. No manual `xattr -d`/`codesign` workaround required.' }, { label: 'Parallel toolchain installs with safe shared-state locking', status: 'shipped', quarter: 'q3-2026', pr: 2758, changelog: 'parallel-toolchain-installs', docs: '/cli/commands/toolchain/usage', description: 'Batch toolchain installation now runs independent packages with bounded concurrency (default four, configurable with `toolchain.max_concurrency` or `--max-concurrency`). Per-resource locks coordinate cache assets, extraction directories, version markers, and tool-version/lockfile updates across worker goroutines and separate Atmos processes. The terminal UI retains one row per active package, including right-aligned download bytes and a verifying state.', benefits: 'Project setup completes faster without interleaved terminal output, corrupted cache metadata, partially installed tool trees, or lost tool-version entries when multiple tools install at once.' }, { label: 'Script-friendly `--format=plain`/`json` for `atmos toolchain get`', status: 'shipped', quarter: 'q3-2026', changelog: 'toolchain-get-plain-json-output', docs: '/cli/commands/toolchain/toolchain-get', description: 'New `--format` flag on `atmos toolchain get` adds `plain` (bare version string) and `json` (structured output with installed status) alongside the existing human-readable table.', benefits: 'CI and scripts capture a tool version with a single flag instead of regex-scraping styled terminal output.' }, + { label: 'Toolchain lockfile written by default (`use_lock_file`)', status: 'shipped', quarter: 'q3-2026', changelog: 'toolchain-lockfile-default', docs: '/cli/configuration/toolchain', description: 'Toolchain installs now write `toolchain.lock.yaml` — pinning the exact resolved version, download URL, checksum, and size per platform — by default. Previously this required an undocumented `use_lock_file: true` setting, so most projects never actually enabled it despite declaring exact tool versions. Projects pinned to an edition dated before this change keep the previous opt-in default automatically.', benefits: 'Tool installs are byte-for-byte reproducible across machines and CI on the same operating system and architecture, out of the box, with no configuration required.', experimental: true }, { label: '`atmos toolchain update` to move pinned tools forward', status: 'shipped', quarter: 'q3-2026', changelog: 'toolchain-update-command', docs: '/cli/commands/toolchain/toolchain-update', description: 'New `atmos toolchain update [tool...]` resolves each tool\'s newest available version, replaces its pin, and reinstalls it, with `--dry-run` and bounded `--max-concurrency`. Tools pinned to a PR, commit SHA, or git ref are skipped with an explanation rather than silently left alone. Also adds `--format`/`--installed-only`/`--pending-only` to `atmos toolchain list`, `--dry-run`/`--cache-only`/`--force` to `atmos toolchain clean`, and `--dry-run` to `atmos toolchain exec`.', benefits: 'Move a pinned tool forward with one command instead of hand-editing `.tool-versions` and guessing the next version.' }, { label: 'Terminal themes', status: 'shipped', quarter: 'q4-2025', docs: '/cli/configuration/settings/terminal', changelog: 'terminal-themes', version: 'v1.198.0', description: 'Customizable terminal color themes with built-in presets and user-defined themes.', benefits: 'Match your terminal output to your preferences or brand. Switch themes without code changes.', demoId: 'theme' }, { label: 'Theme-aware help text', status: 'shipped', quarter: 'q4-2025', changelog: 'theme-aware-help', version: 'v1.200.0', description: 'Help text respects your terminal theme settings for consistent styling.', benefits: 'Help output matches your theme. Consistent look across all Atmos commands.' },