sneak/dnswatcher
1
0
Fork 0
Code Issues 37 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

go.mod is untidy: golang.org/x/sync listed both direct and indirect, so script/bootstrap mutates a tracked file #132

New Issue
Open
opened 2026-08-09 16:56:48 +02:00 by clawbot · 0 comments
clawbot commented 2026-08-09 16:56:48 +02:00
Collaborator
Copy Link

Surfaced while implementing #117, where the implementer noticed script/bootstrap rewriting go.mod and correctly reverted it rather than carrying an unrelated change. The underlying cause is more specific than "bootstrap mutates the tree".

The actual defect

go.mod on main lists the same module twice:

line 16:  golang.org/x/sync v0.19.0              (direct require block)
line 43:  golang.org/x/sync v0.19.0 // indirect

It is genuinely a direct dependency — internal/portcheck/portcheck.go:15 imports golang.org/x/sync/errgroup. So the // indirect entry is redundant and stale, and go mod tidy would collapse it.

REPO_POLICIES.md is explicit: "Always run go mod tidy before committing." That did not happen for whichever commit introduced this.

Why it is worth fixing rather than ignoring

script/bootstrap ends with go mod download, which updates the main module's requirements and therefore drops the redundant line as a side effect. Consequences:

  • Running script/bootstrap on a clean checkout leaves git status dirty. Every agent on this repo runs bootstrap in a fresh worktree, so every one of them starts against a modified tracked file.
  • That is a trap for exactly the mistake this repo has been careful about elsewhere: an implementer staging files by name may still commit a go.mod change they did not make and cannot explain, or waste time investigating a diff that is not theirs.
  • It is also a small integrity problem in its own right. go.mod is a dependency manifest, and one that does not survive a no-op tooling run is not in a settled state.

This is not a security issue — the module hash is in go.sum either way, so the pinning rule is satisfied.

Definition of done

  1. go.mod is tidy: run go mod tidy and commit the result. Expect the redundant golang.org/x/sync v0.19.0 // indirect line to disappear and the direct requirement at line 16 to remain.
  2. go.sum is consistent afterwards.
  3. A clean checkout stays clean across bootstrap. Verify: fresh worktree, git status --porcelain empty, run script/bootstrap, confirm git status --porcelain is still empty. Run it a second time and confirm the same. This is the check that fails today, so it is a real test rather than a formality.
  4. make check green; TODO.md updated in the same commit.
  5. Confirm no dependency is actually removed — go mod tidy dropping something still imported would break the build, so check make build succeeds and the import in internal/portcheck/portcheck.go still resolves.

Commit title ends with (closes #N).

Constraints

  • Do not change any dependency version. This is a tidiness fix, not an upgrade. golang.org/x/sync stays at v0.19.0; nothing else moves. If go mod tidy wants to change a version, stop and report rather than committing it.
  • Do not modify .golangci.yml (sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb) or the golangci-lint pin.
  • Do not "fix" this by removing go mod download from script/bootstrap — bootstrap should download dependencies. The manifest is what is wrong.
  • DNS is never mocked in this repository; nothing here touches test behaviour.
  • make targets and script/ entrypoints only. go mod tidy is the exception and is expected here — the policy names it directly.

Environment hazards

  • Lint runs can be VOID (#121): void if output contains parallel golangci-lint is running, or names any path beginning with ../, or any absolute path outside your worktree. Fix is in unmerged PR #128; isolate manually meanwhile.
  • make hooks fails in a linked worktree (#129).

Sequencing

Independent of the open PRs — touches go.mod/go.sum only, which none of #97, #112, #113, #118, #122, #128 or #131 modifies. Only TODO.md will conflict.

Surfaced while implementing [#117](https://git.eeqj.de/sneak/dnswatcher/issues/117), where the implementer noticed `script/bootstrap` rewriting `go.mod` and correctly reverted it rather than carrying an unrelated change. The underlying cause is more specific than "bootstrap mutates the tree". ## The actual defect `go.mod` on `main` lists the same module twice: ``` line 16: golang.org/x/sync v0.19.0 (direct require block) line 43: golang.org/x/sync v0.19.0 // indirect ``` It is genuinely a **direct** dependency — `internal/portcheck/portcheck.go:15` imports `golang.org/x/sync/errgroup`. So the `// indirect` entry is redundant and stale, and `go mod tidy` would collapse it. `REPO_POLICIES.md` is explicit: "Always run `go mod tidy` before committing." That did not happen for whichever commit introduced this. ## Why it is worth fixing rather than ignoring `script/bootstrap` ends with `go mod download`, which updates the main module's requirements and therefore **drops the redundant line as a side effect**. Consequences: - Running `script/bootstrap` on a clean checkout leaves `git status` dirty. Every agent on this repo runs bootstrap in a fresh worktree, so every one of them starts against a modified tracked file. - That is a trap for exactly the mistake this repo has been careful about elsewhere: an implementer staging files by name may still commit a `go.mod` change they did not make and cannot explain, or waste time investigating a diff that is not theirs. - It is also a small integrity problem in its own right. `go.mod` is a dependency manifest, and one that does not survive a no-op tooling run is not in a settled state. This is not a security issue — the module hash is in `go.sum` either way, so the pinning rule is satisfied. ## Definition of done 1. `go.mod` is tidy: run `go mod tidy` and commit the result. Expect the redundant `golang.org/x/sync v0.19.0 // indirect` line to disappear and the direct requirement at line 16 to remain. 2. `go.sum` is consistent afterwards. 3. **A clean checkout stays clean across bootstrap.** Verify: fresh worktree, `git status --porcelain` empty, run `script/bootstrap`, confirm `git status --porcelain` is *still* empty. Run it a second time and confirm the same. This is the check that fails today, so it is a real test rather than a formality. 4. `make check` green; `TODO.md` updated in the same commit. 5. Confirm no dependency is actually removed — `go mod tidy` dropping something still imported would break the build, so check `make build` succeeds and the import in `internal/portcheck/portcheck.go` still resolves. Commit title ends with ` (closes #N)`. ## Constraints - **Do not change any dependency version.** This is a tidiness fix, not an upgrade. `golang.org/x/sync` stays at `v0.19.0`; nothing else moves. If `go mod tidy` wants to change a version, stop and report rather than committing it. - Do not modify `.golangci.yml` (sha256 `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`) or the golangci-lint pin. - Do not "fix" this by removing `go mod download` from `script/bootstrap` — bootstrap should download dependencies. The manifest is what is wrong. - DNS is never mocked in this repository; nothing here touches test behaviour. - `make` targets and `script/` entrypoints only. `go mod tidy` is the exception and is expected here — the policy names it directly. ## Environment hazards - Lint runs can be VOID ([#121](https://git.eeqj.de/sneak/dnswatcher/issues/121)): void if output contains `parallel golangci-lint is running`, or names any path beginning with `../`, or any absolute path outside your worktree. Fix is in unmerged [PR #128](https://git.eeqj.de/sneak/dnswatcher/pulls/128); isolate manually meanwhile. - `make hooks` fails in a linked worktree ([#129](https://git.eeqj.de/sneak/dnswatcher/issues/129)). ## Sequencing Independent of the open PRs — touches `go.mod`/`go.sum` only, which none of [#97](https://git.eeqj.de/sneak/dnswatcher/pulls/97), [#112](https://git.eeqj.de/sneak/dnswatcher/pulls/112), [#113](https://git.eeqj.de/sneak/dnswatcher/pulls/113), [#118](https://git.eeqj.de/sneak/dnswatcher/pulls/118), [#122](https://git.eeqj.de/sneak/dnswatcher/pulls/122), [#128](https://git.eeqj.de/sneak/dnswatcher/pulls/128) or [#131](https://git.eeqj.de/sneak/dnswatcher/pulls/131) modifies. Only `TODO.md` will conflict.
clawbot added this to the 1.0 milestone 2026-08-09 16:56:48 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No Label
Milestone
No items
No Milestone 1.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/dnswatcher#132
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?