Use typed atomics (atomic.Int64, atomic.Int32, etc.) in test files to prevent
mixing atomic and non-atomic access on the same value, guarantee 64-bit
alignment on 32-bit platforms, and provide a cleaner API.
## Changes
- **Commit 1**: Remove 17 unnecessary `//nolint` directives:
- `//nolint:varnamelen` — linter not active
- `//nolint:unused` on exported `SlimUnsupported`
- `//nolint:govet` in `coderd/httpmw/csrf` — no longer fires
- `//nolint:revive` on functions refactored since the nolint was added
- `//nolint:paralleltest` citing Go 1.22 loop variable capture
(obsolete)
- Bare `//nolint` narrowed to specific `//nolint:gocritic` with
justification
- **Commit 2**: Fix root causes behind 5 dangerous nolint suppressions:
- Add `MinVersion: tls.VersionTLS12` to TLS client config (removes
`gosec` G402)
- Delete trivial unexported wrappers `apiKey()`/`normalizeProvider()` in
chatprovider (removes `revive` confusing-naming)
- Add doc comments to `StartWithAssert` and `Router` (removes `revive`
exported)
- Rename unused parameters to `_` in integration test helpers
> 🤖 This PR was created using Coder Agents and reviewed by me.
The `--parameter-default` value is now used to pre-select the default option for a coder parameter
with option blocks when prompting interactively in CLI.
Related to: https://github.com/coder/coder/issues/22078
Fixes three bugs that caused `coder update` to always re-prompt for
multi-select (`list(string)`) parameters instead of reusing previous
build values:
1. **`isValidTemplateParameterOption` failed for multi-select values**
(`cli/parameterresolver.go`): It compared the entire JSON array string
(e.g. `["vim","emacs"]`) against individual option values, which never
matched. Now parses the JSON array and validates each element
separately.
2. **`RichParameter` ignored previous build value for multi-select**
(`cli/cliui/parameter.go`): The `list(string)` branch always used the
template's default value instead of the `defaultValue` argument (which
carries the previous build's value). Now uses `defaultValue` when
available, falling back to the template default.
3. **Pre-existing crash when `list(string)` has no default value**
(`cli/cliui/parameter.go`): `json.Unmarshal` on an empty string caused
`unexpected end of JSON input`. Now skips unmarshaling when the default
source is empty.
Fixes#19956
Fixes: https://github.com/coder/internal/issues/560
"Select" CLI UI component should ignore "space" when `+Add custom value`
is highlighted. Otherwise it interprets that as a potential option...
and panics.
Fixes all our Go file imports to match the preferred spec that we've _mostly_ been using. For example:
```
import (
"context"
"time"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/xerrors"
"gopkg.in/natefinch/lumberjack.v2"
"cdr.dev/slog/v3"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/serpent"
)
```
3 groups: standard library, 3rd partly libs, Coder libs.
This PR makes the change across the codebase. The PR in the stack above modifies our formatting to maintain this state of affairs, and is a separate PR so it's possible to review that one in detail.
Verifies that streamLogs properly returns ctx.Err() when the context is
cancelled while waiting for logs. This covers the case where a user
interrupts an SSH connection (e.g., Ctrl+C) during startup script
execution.
Refs #21104
When users pass --wait=no or set CODER_SSH_WAIT=no, startup logs are no
longer dumped to stderr. The stage indicator is still shown, just not
the log content.
Fixes#13580
The Agent function had complex nested control flow and cross-case state sharing
via the showStartupLogs flag. This made the code hard to follow and maintain.
This change extract an agentWaiter struct with self-contained methods:
- wait: main state machine loop
- waitForConnection: handles Connecting/Timeout states
- handleConnected: handles Connected state and startup scripts
- streamLogs: handles log streaming/fetching
- waitForReconnection: handles Disconnected state
- pollWhile: helper to consolidate polling loops
Each handler is now self-contained with no cross-method state sharing and the
showStartupLogs flag is replaced by return values and the waitedForConnection
tracking variable.
This changes makes it so that we output the empty string for Format
when there is no data. It turns out there are many places in the code
where we have such handling, but in a way that would break the JSON
formatter (since we'd output nothing on stdout or text rather than
`[]`/`null`).
## Summary
In this pull request we're adding a simple backoff to the workspace
agent polling. This backoff is being added to address seemingly random
cases of elevated number of calls that we've seen to the
`api/v2/workspaceagent/{agent_id}` endpoint.
For more information on the investigation, see:
https://github.com/coder/internal/issues/725
### Changes
- Updated the polling to use predefined progressive intervals for
polling instead of continuously polling every 500ms
### Testing
- Added a test for the function used to calculate the progressive
polling interval
Co-authored-by: Spike Curtis <spike@coder.com>
- Update go.mod to use Go 1.24.1
- Update GitHub Actions setup-go action to use Go 1.24.1
- Fix linting issues with golangci-lint by:
- Updating to golangci-lint v1.57.1 (more compatible with Go 1.24.1)
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <claude@anthropic.com>
Closes https://github.com/coder/coder/issues/16312
We intend to modify the behavior of the CLI handler based on the
specified output format. However, the output format is currently only
accessible within the `OutputFormatter` structure. Therefore, I propose
extending `OutputFormatter` by introducing a public `FormatID` method,
which will allow us to retrieve the format identifier and use it to
customize the behavior of the CLI handler accordingly.
Whilst the `networking-troubleshooting` docs page already mentions that
a direct connection can be established over a private network, even if
there are no STUN servers, it's worth this is the case at the end of the
ping output.
This also removes a print statement that was dirtying up the diagnostic
output, and corrects the name of the `--disable-direct-connections`
flag.
* chore: add command for showing colors
* fix: use ANSI color codes instead of RGB
* feat: add '--no-color' flag
* fix: revert colors
* chore: change colors
* fix: update golden files
* fix: replace blue with brightBlue
* fix: drop '> ' for unfocused prompts
* fix: run 'make fmt'
* chore: allow disabling color with env flags
* fix: apply fixes from feedback
* fix: run 'make gen'
* fix: refactor janky code
* fix: re-add public function
* fix: re-add init for non-color tests
* fix: move styles to 'init' that can be
* fix: stop overwriting entire DefaultStyles
* fix: make code and field obey --no-color
* fix: rip out '--no-color' due to race condition
We still support `NO_COLOR` env variable through termenv's
`EnvColorProfile`. The reason for the race condition is that
`DefaultStyles` is a global that we shouldn't mutate after `init`
is called, but we have to mutate it after `init` has ran to have
serpent collect the cli flags and env vars for us.
* fix: apply nit
* fix: simplify code && hide command
* fix: newline shouldn't be themed
* fix: appease the linter
First PR to address #14244.
Adds common potential reasons as to why a direct connection to the workspace agent couldn't be established to `coder ping`:
- If the Coder deployment administrator has blocked direction connections (`CODER_BLOCK_DIRECT`).
- If the client has no STUN servers within it's DERP map.
- If the client or agent appears to be behind a hard NAT, as per Tailscale `netInfo.MappingVariesByDestIP`
Also adds a warning if the client or agent has a network interface below the 'safe' MTU for tailnet. This warning is always displayed at the end of a `coder ping`.
Zach