Cordy/cairn-desktop

Fork 0

Task 2: Architecture guard — keep internal/sync pure #2

New issue

Closed

opened 2026-09-10 17:14:29 +00:00 by Cordy · 1 comment

Cordy commented

2026-09-10 17:14:29 +00:00

Owner

Depends on Task 1.

Goal

A test that fails the build if anyone imports the OS, the network, or a UI package into the sync engine. Written before there is anything to guard, so it can never regress.

Files

Create: internal/sync/arch_test.go

Why this matters

This single constraint is what lets the whole engine be tested on the Alpine/arm64 CI runner with no network and no disk, and what keeps the UI framework swappable (Wails v3 is beta — see docs/design-spec.md §3). If a later task appears to need os, the design is wrong. Stop and ask rather than deleting a line from forbidden.

Steps

Write the test:

package sync

import (
	"go/build"
	"strings"
	"testing"
)

// The engine must stay pure: no OS, no network, no UI. This is what lets it
// run on the Alpine/arm64 CI runner and makes the UI framework swappable.
var forbidden = []string{
	"os", "net/http", "path/filepath", "syscall",
	"github.com/wailsapp/wails",
}

func TestEngineHasNoForbiddenImports(t *testing.T) {
	pkg, err := build.ImportDir(".", 0)
	if err != nil {
		t.Fatalf("import dir: %v", err)
	}
	for _, imp := range pkg.Imports {
		for _, bad := range forbidden {
			if imp == bad || strings.HasPrefix(imp, bad+"/") {
				t.Errorf("internal/sync must not import %q (see docs/design-spec.md §3)", imp)
			}
		}
	}
}

Run it: go test ./internal/sync/ -run TestEngineHasNoForbiddenImports -v — expect PASS (nothing imported yet).
Commit: git commit -s -m "test: guard internal/sync against os, net and UI imports"

Note

build.ImportDir inspects non-test imports only, so test files in this package may use os freely.

Acceptance criteria

Test passes on an empty package.
Adding import "os" to any non-test file in internal/sync makes it fail.

Depends on Task 1. ## Goal A test that fails the build if anyone imports the OS, the network, or a UI package into the sync engine. Written before there is anything to guard, so it can never regress. ## Files - Create: `internal/sync/arch_test.go` ## Why this matters This single constraint is what lets the whole engine be tested on the Alpine/arm64 CI runner with no network and no disk, and what keeps the UI framework swappable (Wails v3 is beta — see `docs/design-spec.md` §3). If a later task appears to need `os`, the design is wrong. Stop and ask rather than deleting a line from `forbidden`. ## Steps - [ ] **Write the test:** ```go package sync import ( "go/build" "strings" "testing" ) // The engine must stay pure: no OS, no network, no UI. This is what lets it // run on the Alpine/arm64 CI runner and makes the UI framework swappable. var forbidden = []string{ "os", "net/http", "path/filepath", "syscall", "github.com/wailsapp/wails", } func TestEngineHasNoForbiddenImports(t *testing.T) { pkg, err := build.ImportDir(".", 0) if err != nil { t.Fatalf("import dir: %v", err) } for _, imp := range pkg.Imports { for _, bad := range forbidden { if imp == bad || strings.HasPrefix(imp, bad+"/") { t.Errorf("internal/sync must not import %q (see docs/design-spec.md §3)", imp) } } } } ``` - [ ] **Run it:** `go test ./internal/sync/ -run TestEngineHasNoForbiddenImports -v` — expect PASS (nothing imported yet). - [ ] **Commit:** `git commit -s -m "test: guard internal/sync against os, net and UI imports"` ## Note `build.ImportDir` inspects non-test imports only, so test files in this package may use `os` freely. ## Acceptance criteria - Test passes on an empty package. - Adding `import "os"` to any non-test file in `internal/sync` makes it fail.

Cordy added this to the phase-1-engine milestone

2026-09-10 17:14:29 +00:00

Cordy added the

engine

label

2026-09-10 17:14:29 +00:00

~~Cordy referenced this issue 2026-09-10 17:45:57 +00:00~~

P3-11: Private beta — internal gate, NOT the release #40

Cordy referenced this issue

2026-09-10 18:04:20 +00:00

P0-5: Generate the EdDSA update-signing key #20

Cordy referenced this issue

2026-09-10 20:01:40 +00:00

Task 1: Module scaffold, GPLv3 licence, CI on the arm64 runner #1

Cordy commented

2026-09-10 20:01:58 +00:00

Author

Owner

Done

fae5f26 test: guard internal/sync against os, net and UI imports
42fffb1 test: arch guard checks files excluded by build constraints
6257370 test: arch guard also forbids net, io/ioutil and x/sys

What was built

internal/sync/arch_test.go with TestEngineHasNoForbiddenImports, using go/build to inspect internal/sync's non-test imports against a forbidden list, exactly as written in the issue
Fixed a bypass found in review: build.ImportDir (mode 0) misses files excluded by GOOS/GOARCH suffix or a build tag; switched to build.Default with UseAllFiles = true so those files are seen too
Extended forbidden to also cover net, io/ioutil, and golang.org/x/sys (previously open next to net/http, path/filepath, syscall, wails)
Every change demonstrated with a real red run — a temporary probe file importing the forbidden package, observed FAIL, deleted, never committed — before going green

Tests

CI run 2864 (arm64/linux, Go 1.25.5): go vet ./... clean, go test -count=1 ./... → ok cairn.ch/desktop/internal/sync, job test succeeded — green.
Red steps captured directly: an os probe failed before the guard existed (R7); a probe_windows.go/os file and a //go:build integration/net/http file both passed the unfixed guard and failed after the UseAllFiles fix; net and io/ioutil probes passed before the forbidden-list extension and failed after; an x/sys/windows probe failed once both fixes were in place.

Acceptance criteria

"Test passes on an empty package" — met; internal/sync contained only the test file at the first commit.
"Adding import \"os\" to any non-test file makes it fail" — met for the plain case at fae5f26; the GOOS/build-tag bypass found in review was closed by 42fffb1.

Rulings

R7: show the red step by temporarily adding a non-test file importing os, observe it fail, then remove it (not committed).
R12: commit subjects verbatim, git commit -s, plus Co-Authored-By trailer.
F1: build.ImportDir(".",0) misses GOOS/build-tag-excluded files; switched to build.Default+UseAllFiles, verified with windows- and tag-suffixed probes.
F2: forbidden left net, io/ioutil, golang.org/x/sys open; added, verified with probes for each.
CV1: the open item was the combined push's CI result; satisfied by run 2864's green (R11).
CV2: the implementer's own red/green ran on go1.26.5/darwin; independently reproduced, and this Go 1.25.5/linux run is the real cross-version check.

Deferred

Minor: forbidden now contains both net and net/http; an import of net/http matches both the exact entry and the net prefix, so t.Errorf logs the same violation twice. Outcome (fail) is unchanged; deferred to the final review as harmless duplication.

Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI.

**Done** - [fae5f26](http://192.168.10.245/Cordy/cairn-desktop/commit/fae5f26bca99cee4f5a59865ced8120a20093cff) test: guard internal/sync against os, net and UI imports - [42fffb1](http://192.168.10.245/Cordy/cairn-desktop/commit/42fffb1c848f54ab33b803adf3fd3453da567af2) test: arch guard checks files excluded by build constraints - [6257370](http://192.168.10.245/Cordy/cairn-desktop/commit/6257370e4630d5fdc87da9524430b31afe0c5a7f) test: arch guard also forbids net, io/ioutil and x/sys **What was built** - `internal/sync/arch_test.go` with `TestEngineHasNoForbiddenImports`, using `go/build` to inspect `internal/sync`'s non-test imports against a forbidden list, exactly as written in the issue - Fixed a bypass found in review: `build.ImportDir` (mode 0) misses files excluded by GOOS/GOARCH suffix or a build tag; switched to `build.Default` with `UseAllFiles = true` so those files are seen too - Extended `forbidden` to also cover `net`, `io/ioutil`, and `golang.org/x/sys` (previously open next to `net/http`, `path/filepath`, `syscall`, wails) - Every change demonstrated with a real red run — a temporary probe file importing the forbidden package, observed FAIL, deleted, never committed — before going green **Tests** - CI run 2864 (**arm64/linux**, Go 1.25.5): `go vet ./...` clean, `go test -count=1 ./...` → `ok cairn.ch/desktop/internal/sync`, job `test` succeeded — green. - Red steps captured directly: an `os` probe failed before the guard existed (R7); a `probe_windows.go`/`os` file and a `//go:build integration`/`net/http` file both passed the unfixed guard and failed after the `UseAllFiles` fix; `net` and `io/ioutil` probes passed before the forbidden-list extension and failed after; an `x/sys/windows` probe failed once both fixes were in place. **Acceptance criteria** - "Test passes on an empty package" — met; `internal/sync` contained only the test file at the first commit. - "Adding `import \"os\"` to any non-test file makes it fail" — met for the plain case at fae5f26; the GOOS/build-tag bypass found in review was closed by 42fffb1. **Rulings** - R7: show the red step by temporarily adding a non-test file importing `os`, observe it fail, then remove it (not committed). - R12: commit subjects verbatim, `git commit -s`, plus Co-Authored-By trailer. - F1: `build.ImportDir(".",0)` misses GOOS/build-tag-excluded files; switched to `build.Default`+`UseAllFiles`, verified with windows- and tag-suffixed probes. - F2: `forbidden` left `net`, `io/ioutil`, `golang.org/x/sys` open; added, verified with probes for each. - CV1: the open item was the combined push's CI result; satisfied by run 2864's green (R11). - CV2: the implementer's own red/green ran on go1.26.5/darwin; independently reproduced, and this Go 1.25.5/linux run is the real cross-version check. **Deferred** - Minor: `forbidden` now contains both `net` and `net/http`; an import of `net/http` matches both the exact entry and the `net` prefix, so `t.Errorf` logs the same violation twice. Outcome (fail) is unchanged; deferred to the final review as harmless duplication. _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-10 20:02:02 +00:00