From 7cbb12cb2204f85b4b3efd94e6ca1089b0b43dd5 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 08:45:38 +1100 Subject: [PATCH 01/16] test: daemon-starting suites refuse on a developer machine; make verify-full-container (PLAN-526) Co-Authored-By: Claude Opus 5.5 --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- AGENTS.md | 4 +- CHANGELOG.md | 17 +++ CONTRIBUTING.md | 32 +++-- Makefile | 21 +++- build/docker/verify.Dockerfile | 35 ++++++ cmd/clientsmoke/harness_test.go | 4 + cmd/smoke/main_test.go | 18 +++ docs/contributing.md | 5 +- internal/arch/layers.go | 1 + internal/separateenv/separateenv.go | 111 +++++++++++++++++ internal/separateenv/separateenv_test.go | 131 +++++++++++++++++++++ internal/separateenv/testdata/vectors.json | 23 ++++ scripts/pre-commit | 4 +- scripts/verify-full-container.sh | 88 ++++++++++++++ 15 files changed, 472 insertions(+), 24 deletions(-) create mode 100644 build/docker/verify.Dockerfile create mode 100644 cmd/smoke/main_test.go create mode 100644 internal/separateenv/separateenv.go create mode 100644 internal/separateenv/separateenv_test.go create mode 100644 internal/separateenv/testdata/vectors.json create mode 100755 scripts/verify-full-container.sh diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4fad67df..e2a54dbf 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -13,7 +13,7 @@ See CONTRIBUTING.md for the full guide. ## Checklist -- [ ] `make verify` is green; `make verify-full` if the change touches `//go:build integration` code +- [ ] `make verify` is green; `make verify-full-container` if the change touches `//go:build integration` code - [ ] `CHANGELOG.md` entry added - [ ] Prose is in Australian English (-ise/-isation, behaviour, …); spec identifiers left canonical - [ ] New/changed behaviour is covered by tests (integration tests gated with `//go:build integration`) diff --git a/AGENTS.md b/AGENTS.md index fd9c0fbb..8b30d6b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,12 +76,12 @@ Detailed rules and exceptions live in [`docs/contributing.md`](docs/contributing - Keep tests beside code and table-driven where appropriate. `internal/lsp`, `internal/cache`, and `internal/tools` require meaningful coverage; write-tool tests use `WriteDeps{}` and session-isolation tests stay with the package they protect. - Internal tool-to-tool `Execute` calls use canonical parameter names because aliases resolve only at the MCP dispatch boundary. - Gate tests needing external binaries with `//go:build integration`; do not chase TUI coverage. -- Use `topology_affected` during the edit loop. Before delivery run `make verify-full`; run `make lint-cross` after platform-constrained or linter-config changes. `make cover` and `make vuln` remain separate CI/on-demand gates. +- Use `topology_affected` during the edit loop. Before delivery run `make verify-full-container` (the full gate in Docker: its daemon-starting suites refuse on a developer machine); run `make lint-cross` after platform-constrained or linter-config changes. `make cover` and `make vuln` remain separate CI/on-demand gates. - `make test` puts `t.TempDir()` under repository `.testcache`; tests must not assume a system-temp ancestry. ## Delivery and risk -- `make install-hooks` is required after cloning; `make verify-full` (verify + the integration suite) defines ready to commit. `make help` and [`docs/contributing.md`](docs/contributing.md) own the full command matrix. +- `make install-hooks` is required after cloning; `make verify-full` (verify + the integration suite), run via `make verify-full-container` or CI, defines ready to commit. `make help` and [`docs/contributing.md`](docs/contributing.md) own the full command matrix. - Version resolution is exact tag → `VERSION` → short commit. After rebuilding, run `plumb restart`; source changes do not activate in an old daemon. - Use conventional commit types (`feat`, `fix`, `refactor`, `test`, `docs`, `ci`, `chore`). Keep changes bisectable and add a `CHANGELOG.md` entry for each discrete change. - Treat concurrency, the rate limiter, read tracking, and the stats schema as high-risk invariants; inspect their tests and architecture before editing. diff --git a/CHANGELOG.md b/CHANGELOG.md index 21df68d8..21a3bd6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -233,6 +233,23 @@ the project also counts as a peer for the wake watcher's longer window. No read path runs git. +### Changed + +- **The daemon-starting test suites no longer run on a developer machine.** + `cmd/smoke` (the `integration` tag) and `cmd/clientsmoke` (the `clients`, + `clients_e2e` and `clients_conformance` tags) start plumb daemons of their + own, and a developer machine should run exactly one: the daemon its sessions + use. Their `TestMain` now refuses, exit status 2, before building or starting + anything, unless it finds `GITHUB_ACTIONS=true`, a Linux container marker + (`/.dockerenv`, `/run/.containerenv`), or `PLUMB_SEPARATE_ENV=vm` on Linux + (`internal/separateenv`). This guards against running them by accident; it is + not a security boundary. CI is unaffected. The new + `make verify-full-container` runs `make verify-full` in a Linux container + with pinned Go, golangci-lint and language servers. It streams in the working + tree and fails if a plumb daemon is still running after the suite. It replaces + a host `make verify-full` as the pre-submit gate. `make docker-integration` + now runs with `--init`. + ### Fixed - **A revoked resume credential no longer breaks continuity for good.** A diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 48cbf3e4..9eba6630 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,18 +43,24 @@ would have fixed. ## The definition of "ready to commit" ```sh -make verify-full # build + test + lint + the integration suite — run before every push +make verify-full-container # the full gate, in Docker — run before every push ``` `make verify-full` is the full gate: the same checks as `make verify` plus -`make integration-test`, with the unit tests run once rather than twice. -`make verify` alone is the fast gate CI runs on every pull request; it COMPILES -the `//go:build integration` suite without running it, printing that on success, -and a failing integration test does not turn it red. `make integration-test` -SKIPS tests whose language server is absent, so install gopls and pyright if you -want the whole suite to execute. Run `make verify-full` before submitting, and -`make lint-cross` after platform-constrained or linter-config changes. Other -useful targets: `make test`, `make test-race`, `make lint`, `make tidy`. +`make integration-test`, with the unit tests run once rather than twice. Its +`cmd/smoke` suite starts plumb daemons of its own, and a developer machine must +run exactly one plumb daemon (the one its sessions use), so that suite refuses +to run outside CI, a container or a declared VM. `make verify-full-container` +runs the whole gate in a Linux container with pinned tools, streams in your +working tree, and fails if a plumb daemon outlives it. +`make verify` alone is the fast gate CI runs on every pull request, and the one +to run on your machine; it COMPILES the `//go:build integration` suite without +running it, printing that on success, and a failing integration test does not +turn it red. `make integration-test` SKIPS tests whose language server is +absent; the container has gopls, pyright and the npm language servers. Run +`make verify-full-container` before submitting, and `make lint-cross` after +platform-constrained or linter-config changes. Other useful targets: `make test`, +`make test-race`, `make lint`, `make tidy`. **Formatting note:** apply formatting via `golangci-lint run --fix ./...`, never the standalone `gofumpt -w` binary — the two can pin different versions and produce phantom @@ -101,6 +107,10 @@ real-client tier also gates releases and runs nightly. make install-clients # or: ./scripts/install-clients.sh ``` +These tiers start plumb daemons of their own, so like `cmd/smoke` they refuse on a +developer machine: run them in CI (the client-conformance workflow), a container, or a +disposable VM with `PLUMB_SEPARATE_ENV=vm`. + Prerequisites: Node 20+ (npm clients), Python 3.8+ (hermes), `curl` (script-installed clients). Some installers drop binaries in `~/.local/bin` or `~/.npm-global/bin` — add those to `PATH` if a binary isn't found. @@ -182,7 +192,7 @@ From a fork, the flow is: ```sh git checkout -b my-change # branch off an up-to-date main # … make your change, then: -make verify-full # must be green +make verify-full-container # must be green git push origin my-change # push to YOUR fork ``` @@ -190,7 +200,7 @@ Then open a pull request from your fork's branch against `plumbkit/plumb`'s `mai refresh a long-running branch, `git fetch upstream && git rebase upstream/main`. - Keep PRs focused and reviewable. -- Ensure `make verify-full` is green and a `CHANGELOG.md` entry is included. +- Ensure `make verify-full-container` is green and a `CHANGELOG.md` entry is included. - Fill out the PR template — it asks the questions that speed up review. **What to expect on `main`.** `main` is a protected branch: every PR must pass the full diff --git a/Makefile b/Makefile index c38158dd..91c8b66f 100644 --- a/Makefile +++ b/Makefile @@ -56,7 +56,7 @@ UNAME_S := $(shell uname -s) CODESIGN_ID := $(if $(CODESIGN_IDENTITY),$(CODESIGN_IDENTITY),-) CODESIGN_BUNDLE := com.plumbkit.plumb -.PHONY: build tool-sizes web-ui web-ui-audit test test-race integration-test fuzz build-integration lint lint-tags lint-cross check-size check-brief check-changelog check-site-claims check-verify-disclosure check-changelog-placement check-changelog-placement-test check-pre-commit cover cover-report vuln tidy-check verify verify-full run clean tidy install install-hooks hooks codesign ts-wasm swift-wasm install-clients clients-test clients-test-auth clients-test-conformance build-clients docker-integration docker-cleanroom site blog demo-gif +.PHONY: build tool-sizes web-ui web-ui-audit test test-race integration-test fuzz build-integration lint lint-tags lint-cross check-size check-brief check-changelog check-site-claims check-verify-disclosure check-changelog-placement check-changelog-placement-test check-pre-commit cover cover-report vuln tidy-check verify verify-full run clean tidy install install-hooks hooks codesign ts-wasm swift-wasm install-clients clients-test clients-test-auth clients-test-conformance build-clients docker-integration docker-cleanroom verify-full-container site blog demo-gif $(TESTCACHE): mkdir -p $(TESTCACHE) @@ -190,12 +190,21 @@ DOCKER_PLATFORM_FLAG := $(if $(DOCKER_PLATFORM),--platform $(DOCKER_PLATFORM),) # working tree; named volumes cache the Go build + module caches across reruns. docker-integration: docker build $(DOCKER_PLATFORM_FLAG) -f build/docker/integration.Dockerfile -t plumb-integration build/docker - docker run --rm $(DOCKER_PLATFORM_FLAG) \ + docker run --rm --init $(DOCKER_PLATFORM_FLAG) \ -v "$(CURDIR)":/src \ -v plumb-gocache:/root/.cache/go-build \ -v plumb-gomod:/go/pkg/mod \ plumb-integration +# verify-full-container runs `make verify-full` in a Linux container, the local +# way to run the whole gate. The gate's daemon-starting suites (cmd/smoke, +# cmd/clientsmoke) refuse on a developer machine, which must run exactly one +# plumb daemon; they run only in CI, a container or a declared VM +# (internal/separateenv). The working tree streams in as git sees it; no plumb +# daemon may outlive the suite. See scripts/verify-full-container.sh. +verify-full-container: + DOCKER_PLATFORM=$(DOCKER_PLATFORM) ./scripts/verify-full-container.sh + # docker-cleanroom proves a fresh Debian with NO toolchain can install and run # plumb end-to-end: a multi-stage build compiles the binary, then a slim runtime # (bash + python3 only) drives the two-agents MCP demo. The demo's exit code is @@ -434,9 +443,9 @@ verify: build test $(VERIFY_CHECKS) @printf '\n%s\n%s\n%s\n%s\n%s\n\n' \ 'verify: PASSED — but the //go:build integration suite was COMPILED, not RUN.' \ ' A failing integration test does not fail this target.' \ - ' Run it before submitting: make integration-test' \ - ' (skips tests whose language server is absent; install gopls and pyright)' \ - ' Whole gate in one command: make verify-full' + ' It (make integration-test) starts plumb daemons of its own, so it runs' \ + ' in CI or a container, never beside the plumb daemon you are using.' \ + ' Whole gate, in Docker: make verify-full-container' # verify-full is verify's gates plus the integration suite that verify only # compiles. It runs the suite once, without verify's separate `test` step, because @@ -454,4 +463,4 @@ install-hooks: mkdir -p "$$hooks"; \ cp scripts/pre-commit "$$hooks/pre-commit"; \ chmod +x "$$hooks/pre-commit"; \ - echo "Pre-commit hook installed at $$hooks/pre-commit. Run 'make verify' before every push — and 'make verify-full' when the change touches integration-tagged code." + echo "Pre-commit hook installed at $$hooks/pre-commit. Run 'make verify' before every push — and 'make verify-full-container' when the change touches integration-tagged code." diff --git a/build/docker/verify.Dockerfile b/build/docker/verify.Dockerfile new file mode 100644 index 00000000..3bfbe14c --- /dev/null +++ b/build/docker/verify.Dockerfile @@ -0,0 +1,35 @@ +# verify.Dockerfile — the toolchain for `make verify-full-container`. +# +# `make verify-full` runs suites that start plumb daemons of their own +# (cmd/smoke, cmd/clientsmoke), and a developer machine must run exactly one +# plumb daemon, so the full gate runs here instead (or in CI). The image holds +# only tools, never the source: scripts/verify-full-container.sh streams the +# working tree in at run time. +# +# Every tool is pinned, so a green run means the same thing tomorrow. Keep Go in +# step with go.mod's toolchain line, golangci-lint with GOLANGCI_LINT_VERSION in +# .github/workflows/ci.yml, and the language servers with the integration job. +FROM golang:1.27.2-bookworm + +# Node only to install the npm-published language servers. +COPY --from=node:22.23.3-bookworm /usr/local/bin/node /usr/local/bin/node +COPY --from=node:22.23.3-bookworm /usr/local/lib/node_modules /usr/local/lib/node_modules +RUN ln -s ../lib/node_modules/npm/bin/npm-cli.js /usr/local/bin/npm \ + && ln -s ../lib/node_modules/npm/bin/npx-cli.js /usr/local/bin/npx + +ARG GOPLS_VERSION=v0.23.0 +ARG GOLANGCI_LINT_VERSION=v2.14.0 +RUN go install "golang.org/x/tools/gopls@${GOPLS_VERSION}" \ + && go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}" \ + && go clean -cache -modcache + +# typescript@5: plain `typescript` resolves to 7, which ships no tsserver (see +# the integration job in ci.yml). +RUN npm install -g --no-audit --no-fund \ + pyright@1.1.414 typescript-language-server@6.0.2 typescript@5.9.3 vscode-langservers-extracted@4.10.0 + +ENV PATH="/go/bin:${PATH}" + +# Fail the image, not the run, when a tool is broken. +RUN gopls version && golangci-lint --version && pyright --version \ + && typescript-language-server --version && command -v vscode-html-language-server diff --git a/cmd/clientsmoke/harness_test.go b/cmd/clientsmoke/harness_test.go index 5e7260d8..dd65df86 100644 --- a/cmd/clientsmoke/harness_test.go +++ b/cmd/clientsmoke/harness_test.go @@ -51,6 +51,7 @@ import ( "testing" "time" + "github.com/plumbkit/plumb/internal/separateenv" "github.com/plumbkit/plumb/internal/sqlitex" ) @@ -65,6 +66,9 @@ var plumbBin string var realHome string func TestMain(m *testing.M) { + // The clients start plumb daemons of their own: refuse on a developer + // machine before anything is built or created (internal/separateenv). + separateenv.RequireForTestMain("cmd/clientsmoke") realHome = os.Getenv("HOME") root, err := findRepoRoot() if err != nil { diff --git a/cmd/smoke/main_test.go b/cmd/smoke/main_test.go new file mode 100644 index 00000000..7effa46b --- /dev/null +++ b/cmd/smoke/main_test.go @@ -0,0 +1,18 @@ +//go:build integration + +package smoke_test + +import ( + "os" + "testing" + + "github.com/plumbkit/plumb/internal/separateenv" +) + +// TestMain refuses on a developer machine before any test builds plumb or +// starts a `plumb serve`, whose daemon would sit beside the developer's own: +// the suite runs in CI or `make verify-full-container` (internal/separateenv). +func TestMain(m *testing.M) { + separateenv.RequireForTestMain("cmd/smoke") + os.Exit(m.Run()) +} diff --git a/docs/contributing.md b/docs/contributing.md index 1650ea30..868e70db 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -46,14 +46,15 @@ both behaviours against a stub linter. | `make lint` | `golangci-lint run` via `scripts/lint-with-retry.sh` — retries with bounded backoff on the shared-cache lock ("parallel golangci-lint is running"), so a peer agent's lint does not read as a failure of this one. | | `make lint-tags` | Lint and format-check the files behind custom build tags (`integration`, `clients*`, `parity`, `race`), which `make lint` never analyses. `scripts/build-tags.sh` reads the tags from the `//go:build` lines of every Go file that is not ignored, so a new tag is covered without a config change. Part of `make verify`. It lints this OS's files only; `make lint-cross` covers the other's. | | `make verify` | Build, test, lint (including `make lint-tags`), compile integration/client binaries, check file/brief/changelog limits, and verify `go.mod` tidiness — the fast gate CI runs on every pull request. It does not RUN the integration suite, and prints that on success. | -| `make verify-full` | The same checks as `make verify` plus `make integration-test` — the full **"ready to commit"** gate, with the unit tests run once. `integration-test` SKIPS tests whose language server is absent, so install gopls and pyright to run all of it. | +| `make verify-full` | The same checks as `make verify` plus `make integration-test` — the full **"ready to commit"** gate, with the unit tests run once. Its `cmd/smoke` suite starts plumb daemons of its own and refuses on a developer machine (`internal/separateenv`), so run it in CI or through `make verify-full-container`. `integration-test` SKIPS tests whose language server is absent. | +| `make verify-full-container` | `make verify-full` in a Linux container with pinned Go, golangci-lint and language servers: streams in the working tree as git sees it, runs with `--init` and `--rm`, and fails if a plumb daemon is left after the suite. The local way to run the full gate. | | `make lint-cross` | Statically lint and vet the other supported OS, its tagged files included; required after platform-constrained or linter-config changes. | | `make cover` | Enforce the whole-tree statement floor from `scripts/check-coverage.sh`. | | `make vuln` | Run `govulncheck`; requires network access. | | `make tidy` | `go mod tidy` | | `make clean` | Remove build artefacts. | | `make install-clients` | Install every supported client CLI, for local client-integration testing. | -| `make clients-test` / `make clients-test-auth` | On-demand connection/auth tiers that drive each installed client CLI headless (own build tags, never part of `make verify`) — see the comments above these targets in the `Makefile`. | +| `make clients-test` / `make clients-test-auth` | On-demand connection/auth tiers that drive each installed client CLI headless (own build tags, never part of `make verify`) — see the comments above these targets in the `Makefile`. Like `cmd/smoke` they start plumb daemons, so they refuse outside CI, a container or a declared VM. | > **Formatting:** always format via `golangci-lint run --fix ./...` (the fix the > hook names when it fails), not a standalone `gofumpt` binary — the two can pin diff --git a/internal/arch/layers.go b/internal/arch/layers.go index 2aa14c16..3f48c396 100644 --- a/internal/arch/layers.go +++ b/internal/arch/layers.go @@ -94,6 +94,7 @@ var Layers = map[string]Layer{ "internal/textfmt": LayerFoundation, // stdlib-only text primitives, no lipgloss "internal/toolerror": LayerFoundation, // stdlib-only structured tool-error contract "internal/redact": LayerFoundation, + "internal/separateenv": LayerFoundation, // where a test suite may start its own plumb daemon "internal/theme": LayerFoundation, // UI-agnostic hex palettes, no lipgloss "internal/render": LayerFoundation, // pure presentation helpers "internal/quality": LayerFoundation, // post-write analyser interface diff --git a/internal/separateenv/separateenv.go b/internal/separateenv/separateenv.go new file mode 100644 index 00000000..7d8e714e --- /dev/null +++ b/internal/separateenv/separateenv.go @@ -0,0 +1,111 @@ +// Package separateenv tells a test harness whether it runs somewhere a plumb +// daemon of its own may be started. +// +// A developer machine runs exactly one plumb daemon, the one its sessions use. +// Test suites that start, kill or restart daemons (cmd/smoke, cmd/clientsmoke) +// therefore run only in a separate environment: a CI runner, a container or a +// disposable VM. Their TestMain calls RequireForTestMain, which refuses on +// anything else before the suite creates a directory, builds a binary or +// starts a process. +// +// This guards against running such a suite on a developer machine by accident +// (an agent, a habit, a stale recipe); it is not a security boundary, since +// anyone can fake the evidence. It accepts exactly three kinds: +// - GITHUB_ACTIONS=true, on any OS: a hosted CI runner; +// - Linux with a readable regular-file container marker, /.dockerenv +// (Docker) or /run/.containerenv (Podman); +// - Linux with PLUMB_SEPARATE_ENV=vm: an explicit declaration that this is a +// disposable VM. +// +// Nothing else counts: no CI=true, no override, no case folding, and a marker +// that cannot be read counts as absent. testdata/vectors.json is byte-identical +// to the vectors of the same rule in the dsh-plumb-identity e2e. +package separateenv + +import ( + "fmt" + "os" + "runtime" + "strings" +) + +// The kinds of separate environment Detect reports. +const ( + GitHubActions = "github-actions" + Container = "container" + VM = "vm" +) + +// Probe is what Detect reads, injectable for tests. +type Probe struct { + Getenv func(string) string + GOOS string + Readable func(path string) bool +} + +// ContainerMarkers are the files a container runtime creates at the root. +func ContainerMarkers() []string { + return []string{"/.dockerenv", "/run/.containerenv"} +} + +// Detect returns the kind of separate environment p describes, or "" when it +// may be a developer machine. +func Detect(p Probe) string { + if p.Getenv("GITHUB_ACTIONS") == "true" { + return GitHubActions + } + if p.GOOS != "linux" { + return "" + } + for _, marker := range ContainerMarkers() { + if p.Readable(marker) { + return Container + } + } + if p.Getenv("PLUMB_SEPARATE_ENV") == "vm" { + return VM + } + return "" +} + +// Current is Detect for this process. +func Current() string { + return Detect(Probe{Getenv: os.Getenv, GOOS: runtime.GOOS, Readable: ReadableFile}) +} + +// ReadableFile reports whether path is a regular file this process can open +// for reading. Anything else, including an error, is false. +func ReadableFile(path string) bool { + info, err := os.Stat(path) + if err != nil || !info.Mode().IsRegular() { + return false + } + f, err := os.Open(path) //nolint:gosec // a fixed marker path, opened only to prove it is readable + if err != nil { + return false + } + _ = f.Close() + return true +} + +// Refusal is the message a suite named what prints when Current is "". +func Refusal(what string) string { + return strings.Join([]string{ + what + ": refusing to run here: it starts plumb daemons of its own, and a developer", + "machine must run exactly one plumb daemon (the one its sessions use).", + "Run it in a separate environment instead: `make verify-full-container`", + "(Docker), or CI. Accepted evidence: GITHUB_ACTIONS=true; Linux with a readable", + "/.dockerenv or /run/.containerenv; Linux with PLUMB_SEPARATE_ENV=vm (a disposable VM).", + }, "\n") +} + +// RequireForTestMain exits the test binary with status 2 and Refusal(what) +// unless this process runs in a separate environment. Call it first in +// TestMain, before anything is created or started. +func RequireForTestMain(what string) { + if Current() != "" { + return + } + fmt.Fprintln(os.Stderr, Refusal(what)) + os.Exit(2) +} diff --git a/internal/separateenv/separateenv_test.go b/internal/separateenv/separateenv_test.go new file mode 100644 index 00000000..20dfa78a --- /dev/null +++ b/internal/separateenv/separateenv_test.go @@ -0,0 +1,131 @@ +package separateenv + +import ( + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" +) + +type vector struct { + Name string `json:"name"` + Platform string `json:"platform"` + Env map[string]string `json:"env"` + Readable []string `json:"readable"` + Expect *string `json:"expect"` +} + +func TestDetect_SharedVectors(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("testdata", "vectors.json")) + if err != nil { + t.Fatal(err) + } + var doc struct { + Cases []vector `json:"cases"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + t.Fatal(err) + } + if len(doc.Cases) == 0 { + t.Fatal("no vectors") + } + // The vectors are shared with the Node copy of this rule, which names + // platforms as process.platform does. + nodePlatformToGOOS := map[string]string{"win32": "windows"} + for _, c := range doc.Cases { + t.Run(c.Name, func(t *testing.T) { + goos := c.Platform + if mapped, ok := nodePlatformToGOOS[goos]; ok { + goos = mapped + } + got := Detect(Probe{ + Getenv: func(k string) string { return c.Env[k] }, + GOOS: goos, + Readable: func(p string) bool { return slices.Contains(c.Readable, p) }, + }) + want := "" + if c.Expect != nil { + want = *c.Expect + } + if got != want { + t.Fatalf("Detect = %q, want %q", got, want) + } + }) + } +} + +func TestContainerMarkers_MatchTheVectors(t *testing.T) { + if got, want := ContainerMarkers(), []string{"/.dockerenv", "/run/.containerenv"}; !slices.Equal(got, want) { + t.Fatalf("ContainerMarkers = %v, want %v", got, want) + } +} + +func TestReadableFile(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "marker") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + if !ReadableFile(file) { + t.Error("an empty regular file (Docker writes /.dockerenv empty) must be readable") + } + if ReadableFile(dir) { + t.Error("a directory is not a marker") + } + if ReadableFile(filepath.Join(dir, "missing")) { + t.Error("a missing path is not a marker") + } + dangling := filepath.Join(dir, "dangling") + if err := os.Symlink(filepath.Join(dir, "missing"), dangling); err != nil { + t.Fatal(err) + } + if ReadableFile(dangling) { + t.Error("a dangling symlink is not a marker") + } + locked := filepath.Join(dir, "locked") + if err := os.WriteFile(locked, nil, 0o000); err != nil { + t.Fatal(err) + } + if os.Geteuid() != 0 && ReadableFile(locked) { + t.Error("a file this process cannot read is not a marker") + } +} + +// TestRequireForTestMain_RefusesOnAHost runs this test binary again as a child +// that calls RequireForTestMain with the CI and VM evidence removed, so only a +// container marker could let it through. +func TestRequireForTestMain_RefusesOnAHost(t *testing.T) { + if os.Getenv("SEPARATEENV_CHILD") == "1" { + RequireForTestMain("separateenv-child") + os.Exit(0) + } + child := Probe{Getenv: func(string) string { return "" }, GOOS: "linux", Readable: ReadableFile} + if Detect(child) != "" { + t.Skip("this process runs in a container, so the child would be let through") + } + cmd := exec.Command(os.Args[0], "-test.run=^TestRequireForTestMain_RefusesOnAHost$") //nolint:gosec // re-runs this test binary + cmd.Env = append(withoutKeys(os.Environ(), "GITHUB_ACTIONS", "PLUMB_SEPARATE_ENV"), "SEPARATEENV_CHILD=1") + out, err := cmd.CombinedOutput() + var exit *exec.ExitError + if !errors.As(err, &exit) || exit.ExitCode() != 2 { + t.Fatalf("child: err=%v, want exit status 2\n%s", err, out) + } + if !strings.Contains(string(out), "separateenv-child: refusing to run here") { + t.Fatalf("child output lacks the refusal:\n%s", out) + } +} + +func withoutKeys(env []string, keys ...string) []string { + out := env[:0:0] + for _, kv := range env { + key, _, _ := strings.Cut(kv, "=") + if !slices.Contains(keys, key) { + out = append(out, kv) + } + } + return out +} diff --git a/internal/separateenv/testdata/vectors.json b/internal/separateenv/testdata/vectors.json new file mode 100644 index 00000000..ffdcc7cf --- /dev/null +++ b/internal/separateenv/testdata/vectors.json @@ -0,0 +1,23 @@ +{ + "comment": "Shared vectors for the separate-environment rule (PLAN-526). Byte-identical copies live in dsh-plumb-identity test/fixtures/separate-env-vectors.json and plumb internal/separateenv/testdata/vectors.json; every implementation must give `expect` for each case. `readable` lists the container markers that are readable regular files; whether a path qualifies is tested separately against the real filesystem.", + "cases": [ + { "name": "darwin, no evidence", "platform": "darwin", "env": {}, "readable": [], "expect": null }, + { "name": "darwin, GitHub Actions", "platform": "darwin", "env": { "GITHUB_ACTIONS": "true" }, "readable": [], "expect": "github-actions" }, + { "name": "linux, GitHub Actions", "platform": "linux", "env": { "GITHUB_ACTIONS": "true" }, "readable": [], "expect": "github-actions" }, + { "name": "GITHUB_ACTIONS=1 is not exact", "platform": "linux", "env": { "GITHUB_ACTIONS": "1" }, "readable": [], "expect": null }, + { "name": "GITHUB_ACTIONS=TRUE is not exact", "platform": "darwin", "env": { "GITHUB_ACTIONS": "TRUE" }, "readable": [], "expect": null }, + { "name": "CI=true alone is not evidence", "platform": "linux", "env": { "CI": "true" }, "readable": [], "expect": null }, + { "name": "linux, no evidence", "platform": "linux", "env": {}, "readable": [], "expect": null }, + { "name": "linux, /.dockerenv", "platform": "linux", "env": {}, "readable": ["/.dockerenv"], "expect": "container" }, + { "name": "linux, /run/.containerenv", "platform": "linux", "env": {}, "readable": ["/run/.containerenv"], "expect": "container" }, + { "name": "darwin ignores container markers", "platform": "darwin", "env": {}, "readable": ["/.dockerenv"], "expect": null }, + { "name": "linux, declared VM", "platform": "linux", "env": { "PLUMB_SEPARATE_ENV": "vm" }, "readable": [], "expect": "vm" }, + { "name": "VM declaration is case-sensitive", "platform": "linux", "env": { "PLUMB_SEPARATE_ENV": "VM" }, "readable": [], "expect": null }, + { "name": "VM declaration is not trimmed", "platform": "linux", "env": { "PLUMB_SEPARATE_ENV": "vm " }, "readable": [], "expect": null }, + { "name": "PLUMB_SEPARATE_ENV=1 is not a declaration", "platform": "linux", "env": { "PLUMB_SEPARATE_ENV": "1" }, "readable": [], "expect": null }, + { "name": "darwin cannot declare a VM", "platform": "darwin", "env": { "PLUMB_SEPARATE_ENV": "vm" }, "readable": [], "expect": null }, + { "name": "windows, no evidence", "platform": "win32", "env": {}, "readable": [], "expect": null }, + { "name": "GitHub Actions wins over a VM declaration", "platform": "linux", "env": { "GITHUB_ACTIONS": "true", "PLUMB_SEPARATE_ENV": "vm" }, "readable": [], "expect": "github-actions" }, + { "name": "container marker wins over a VM declaration", "platform": "linux", "env": { "PLUMB_SEPARATE_ENV": "vm" }, "readable": ["/.dockerenv"], "expect": "container" } + ] +} diff --git a/scripts/pre-commit b/scripts/pre-commit index cd6fc6e3..a2f1992e 100644 --- a/scripts/pre-commit +++ b/scripts/pre-commit @@ -1,6 +1,6 @@ #!/bin/sh # Pre-commit: build + format + lint. -# For the full sweep (including tests) run: make verify-full +# For the full sweep (including tests) run: make verify-full-container set -e go build ./... @@ -26,7 +26,7 @@ fi if [ -z "$GOLANGCI_LINT" ]; then echo "pre-commit: golangci-lint not found on PATH, in \$GOBIN, or in \$GOPATH/bin." >&2 echo " install it (https://golangci-lint.run/welcome/install/), or set GOBIN," >&2 - echo " or commit with --no-verify if you must and run 'make verify-full' before pushing." >&2 + echo " or commit with --no-verify if you must and run 'make verify-full-container' before pushing." >&2 exit 1 fi diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh new file mode 100755 index 00000000..826a93d4 --- /dev/null +++ b/scripts/verify-full-container.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# verify-full-container.sh — run `make verify-full` in a Linux container. +# +# The full gate includes suites that start plumb daemons of their own +# (cmd/smoke, cmd/clientsmoke), and a developer machine must run exactly one +# plumb daemon: those suites refuse to run outside CI, a container or a declared +# VM (internal/separateenv). This is the local way to run the whole gate. +# +# The working tree is streamed in as git sees it (tracked and untracked files, +# not ignored ones), so uncommitted changes are verified and host build output +# is not. Inside, it becomes a fresh one-commit git repository, which the checks +# that call git need. The container runs with --init (exited processes are +# reaped, not counted) and --rm: its exit ends every process it started. +# Afterwards no plumb daemon may be left: it is looked for for 30 s, and the run +# fails if one is seen or the process table cannot be read. +# +# make verify-full-container (DOCKER_PLATFORM=linux/amd64 for amd64) + +set -uo pipefail + +root=$(git rev-parse --show-toplevel) || exit 1 +platform=() +[ -n "${DOCKER_PLATFORM:-}" ] && platform=(--platform "$DOCKER_PLATFORM") +image=plumb-verify + +docker build ${platform[@]+"${platform[@]}"} -q -f "$root/build/docker/verify.Dockerfile" -t "$image" "$root/build/docker" > /dev/null || exit 1 + +name=plumb-verify-$(date +%s)-$$ +trap 'docker kill "$name" > /dev/null 2>&1' INT TERM HUP + +inside=' +set -uo pipefail +mkdir -p /work && tar -C /work -xf - && cd /work || exit 70 +git init -q && git add -A && git -c user.name=verify -c user.email=verify@localhost commit -qm "working tree" || exit 70 + +# The plumb daemons alive now, by their own argv; "?" when the process table +# cannot be read (a process that exited mid-scan is skipped). +daemons() { + local n=0 dir argv + for dir in /proc/[0-9]*; do + if ! argv=$(tr "\0" " " < "$dir/cmdline" 2> /dev/null); then + [ -d "$dir" ] && { echo "?"; return; } + continue + fi + [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] && n=$((n + 1)) + done + echo "$n" +} + +make verify-full +rc=$? +echo "verify-full-container: make verify-full exited $rc" + +seen=0 +for _ in $(seq 1 60); do + n=$(daemons) + if [ "$n" = "?" ]; then echo "verify-full-container: cannot read the process table"; exit 1; fi + [ "$n" -gt "$seen" ] && seen=$n + sleep 0.5 +done +if [ "$seen" -gt 0 ]; then + echo "verify-full-container: $seen plumb daemon(s) still running after the suite (watched 30 s)" + [ "$rc" -eq 0 ] && rc=1 +else + echo "verify-full-container: no plumb daemon left after the suite (watched 30 s)" +fi +exit "$rc" +' + +( + cd "$root" || exit 1 + git ls-files -z --cached --others --exclude-standard | + while IFS= read -r -d '' f; do [ -e "$f" ] && printf '%s\0' "$f"; done | + tar --null -T - -cf - +) | docker run -i --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ + -v plumb-verify-gocache:/root/.cache/go-build \ + -v plumb-verify-gomod:/go/pkg/mod \ + -v plumb-verify-lintcache:/root/.cache/golangci-lint \ + "$image" bash -c "$inside" +rc=$? + +if docker container inspect "$name" > /dev/null 2>&1; then + echo "verify-full-container: container $name still exists" >&2 + rc=1 +else + echo "verify-full-container: container $name is gone, so every process it started has ended" +fi +exit "$rc" From 247b3e9302505d36cc2eb6c9dc91ceaf47dfc1dd Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 08:52:59 +1100 Subject: [PATCH 02/16] test: verify-full-container supervises the daemon peak and fails closed on an unreadable process table (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 36 +++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index 826a93d4..8c1388e5 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -11,8 +11,9 @@ # is not. Inside, it becomes a fresh one-commit git repository, which the checks # that call git need. The container runs with --init (exited processes are # reaped, not counted) and --rm: its exit ends every process it started. -# Afterwards no plumb daemon may be left: it is looked for for 30 s, and the run -# fails if one is seen or the process table cannot be read. +# While the suite runs, the plumb daemons are counted every 200 ms: at most one +# may run at a time. Afterwards none may be left: they are looked for for 30 s. +# The run fails if either rule breaks, or if the process table cannot be read. # # make verify-full-container (DOCKER_PLATFORM=linux/amd64 for amd64) @@ -34,22 +35,47 @@ mkdir -p /work && tar -C /work -xf - && cd /work || exit 70 git init -q && git add -A && git -c user.name=verify -c user.email=verify@localhost commit -qm "working tree" || exit 70 # The plumb daemons alive now, by their own argv; "?" when the process table -# cannot be read (a process that exited mid-scan is skipped). +# cannot be read. A process that exited mid-scan is skipped, but an empty +# listing is not "none": this shell is a process, so /proc must list one. daemons() { - local n=0 dir argv + local n=0 dir argv listed=0 for dir in /proc/[0-9]*; do + [ -d "$dir" ] || continue + listed=1 if ! argv=$(tr "\0" " " < "$dir/cmdline" 2> /dev/null); then [ -d "$dir" ] && { echo "?"; return; } continue fi [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] && n=$((n + 1)) done + [ "$listed" -eq 1 ] || { echo "?"; return; } echo "$n" } +# The peak during the suite, written to /tmp/peak ("?" sticks once seen). +supervise() { + local peak=0 n + while :; do + n=$(daemons) + if [ "$n" = "?" ]; then peak="?"; elif [ "$peak" != "?" ] && [ "$n" -gt "$peak" ]; then peak=$n; fi + echo "$peak" > /tmp/peak + sleep 0.2 + done +} +supervise & +supervisor=$! + make verify-full rc=$? echo "verify-full-container: make verify-full exited $rc" +kill "$supervisor" 2> /dev/null +peak=$(cat /tmp/peak 2> /dev/null || echo "?") +case "$peak" in + 0) echo "verify-full-container: no plumb daemon was seen during the suite" ;; + 1) echo "verify-full-container: at most 1 plumb daemon ran at a time during the suite" ;; + "?") echo "verify-full-container: the process table could not be read during the suite"; [ "$rc" -eq 0 ] && rc=1 ;; + *) echo "verify-full-container: $peak plumb daemons ran at once during the suite (at most 1)"; [ "$rc" -eq 0 ] && rc=1 ;; +esac seen=0 for _ in $(seq 1 60); do @@ -70,7 +96,7 @@ exit "$rc" ( cd "$root" || exit 1 git ls-files -z --cached --others --exclude-standard | - while IFS= read -r -d '' f; do [ -e "$f" ] && printf '%s\0' "$f"; done | + while IFS= read -r -d '' f; do { [ -e "$f" ] || [ -L "$f" ]; } && printf '%s\0' "$f"; done | tar --null -T - -cf - ) | docker run -i --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ -v plumb-verify-gocache:/root/.cache/go-build \ From 53483409a9318426854f6513ec99704026523f43 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 08:54:02 +1100 Subject: [PATCH 03/16] test: verify-full-container calls the container gone only on the engine's answer (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index 8c1388e5..a7b6a30c 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -105,7 +105,12 @@ exit "$rc" "$image" bash -c "$inside" rc=$? -if docker container inspect "$name" > /dev/null 2>&1; then +# Container exit is what ends a SIGKILL'd run's processes, so "gone" must be +# an answer from the engine, never the failure to get one. +if ! listed=$(docker ps -a --filter "name=^/$name\$" --format '{{.Names}}'); then + echo "verify-full-container: could not ask docker whether container $name is gone" >&2 + rc=1 +elif [ -n "$listed" ]; then echo "verify-full-container: container $name still exists" >&2 rc=1 else From 9820422d9d6182a6625ccaaae1f13a088a17be30 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 08:56:17 +1100 Subject: [PATCH 04/16] test: verify-full-container trusts the daemon peak only from a watcher that lived through the suite (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 43 ++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index a7b6a30c..4487eeab 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -52,30 +52,51 @@ daemons() { echo "$n" } -# The peak during the suite, written to /tmp/peak ("?" sticks once seen). +# The peak during the suite and the number of samples, rewritten atomically to +# /tmp/peak after every sample ("?" sticks once the table could not be read). supervise() { - local peak=0 n + local peak=0 samples=0 n while :; do n=$(daemons) + samples=$((samples + 1)) if [ "$n" = "?" ]; then peak="?"; elif [ "$peak" != "?" ] && [ "$n" -gt "$peak" ]; then peak=$n; fi - echo "$peak" > /tmp/peak + echo "$peak $samples" > /tmp/peak.new && mv /tmp/peak.new /tmp/peak sleep 0.2 done } +rm -f /tmp/peak /tmp/peak.new supervise & supervisor=$! make verify-full rc=$? echo "verify-full-container: make verify-full exited $rc" -kill "$supervisor" 2> /dev/null -peak=$(cat /tmp/peak 2> /dev/null || echo "?") -case "$peak" in - 0) echo "verify-full-container: no plumb daemon was seen during the suite" ;; - 1) echo "verify-full-container: at most 1 plumb daemon ran at a time during the suite" ;; - "?") echo "verify-full-container: the process table could not be read during the suite"; [ "$rc" -eq 0 ] && rc=1 ;; - *) echo "verify-full-container: $peak plumb daemons ran at once during the suite (at most 1)"; [ "$rc" -eq 0 ] && rc=1 ;; -esac + +# Trust the peak only from a watcher that ran for the whole suite: alive now, +# stopped by us (TERM, so it exits 143), with a readable, numeric record. +watched=yes +if kill -0 "$supervisor" 2> /dev/null; then + kill -TERM "$supervisor" + wait "$supervisor" + status=$? + [ "$status" -eq 143 ] || { watched=no; echo "verify-full-container: the daemon watcher exited $status, not 143 on our TERM"; } +else + watched=no + echo "verify-full-container: the daemon watcher died during the suite" +fi +peak="?" samples=0 +read -r peak samples < /tmp/peak 2> /dev/null +[[ "$samples" =~ ^[0-9]+$ ]] && [ "$samples" -gt 0 ] || { watched=no; echo "verify-full-container: the daemon watcher left no samples"; } +if [ "$watched" = no ]; then + [ "$rc" -eq 0 ] && rc=1 +else + case "$peak" in + 0) echo "verify-full-container: no plumb daemon was seen during the suite ($samples samples)" ;; + 1) echo "verify-full-container: at most 1 plumb daemon ran at a time during the suite ($samples samples)" ;; + "?") echo "verify-full-container: the process table could not be read during the suite"; [ "$rc" -eq 0 ] && rc=1 ;; + *) echo "verify-full-container: $peak plumb daemons ran at once during the suite (at most 1)"; [ "$rc" -eq 0 ] && rc=1 ;; + esac +fi seen=0 for _ in $(seq 1 60); do From 90cfcd72ebe8e30f88c84702294cbbee6bc88558 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 09:01:14 +1100 Subject: [PATCH 05/16] test: verify-full-container's watcher stops on a failed checkpoint write (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index 4487eeab..d4f680e5 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -60,7 +60,9 @@ supervise() { n=$(daemons) samples=$((samples + 1)) if [ "$n" = "?" ]; then peak="?"; elif [ "$peak" != "?" ] && [ "$n" -gt "$peak" ]; then peak=$n; fi - echo "$peak $samples" > /tmp/peak.new && mv /tmp/peak.new /tmp/peak + # A checkpoint that cannot be written ends the watcher (status 3), which the + # checks after the suite reject, rather than leave an older record trusted. + { echo "$peak $samples" > /tmp/peak.new && mv /tmp/peak.new /tmp/peak; } || exit 3 sleep 0.2 done } From 1a703c5e0bfc86b750ae59c4535e81308d7716ca Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 09:11:40 +1100 Subject: [PATCH 06/16] fix: verify-full-container streams no macOS AppleDouble or xattr entries (found by pure-antelope) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index d4f680e5..b786450a 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -120,7 +120,7 @@ exit "$rc" cd "$root" || exit 1 git ls-files -z --cached --others --exclude-standard | while IFS= read -r -d '' f; do { [ -e "$f" ] || [ -L "$f" ]; } && printf '%s\0' "$f"; done | - tar --null -T - -cf - + COPYFILE_DISABLE=1 tar --no-xattrs --null -T - -cf - # no macOS ._* AppleDouble entries or xattr headers ) | docker run -i --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ -v plumb-verify-gocache:/root/.cache/go-build \ -v plumb-verify-gomod:/go/pkg/mod \ From 617dab3c27b9af0d4a5e3576e89cd079e2f23924 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 13:55:37 +1100 Subject: [PATCH 07/16] test: verify-full-container does not count a daemon's forked pre-exec child as a second daemon (PLAN-526) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index b786450a..f7923042 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -37,8 +37,12 @@ git init -q && git add -A && git -c user.name=verify -c user.email=verify@localh # The plumb daemons alive now, by their own argv; "?" when the process table # cannot be read. A process that exited mid-scan is skipped, but an empty # listing is not "none": this shell is a process, so /proc must list one. +# A child of a daemon, in the instant between fork and exec, still shows the +# argv of the daemon, so a match whose parent is itself a match is not counted: +# plumb never starts a daemon from a daemon (serve and `plumb restart` do). daemons() { - local n=0 dir argv listed=0 + local dir argv stat listed=0 pid + local -A ppid_of=() for dir in /proc/[0-9]*; do [ -d "$dir" ] || continue listed=1 @@ -46,9 +50,21 @@ daemons() { [ -d "$dir" ] && { echo "?"; return; } continue fi - [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] && n=$((n + 1)) + [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] || continue + if ! stat=$(< "$dir/stat") 2> /dev/null; then + [ -d "$dir" ] && { echo "?"; return; } + continue + fi + stat=${stat##*) } # after the last ")": "state ppid ..." + stat=${stat#* } + [[ "${stat%% *}" =~ ^[0-9]+$ ]] || { echo "?"; return; } + ppid_of[${dir#/proc/}]=${stat%% *} done [ "$listed" -eq 1 ] || { echo "?"; return; } + local n=0 + for pid in "${!ppid_of[@]}"; do + [ -n "${ppid_of[${ppid_of[$pid]}]+x}" ] || n=$((n + 1)) + done echo "$n" } From e99f219d4a81d950b191b67bb63902b7857f81b1 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 13:59:17 +1100 Subject: [PATCH 08/16] test: verify-full-container tells a forked, not-yet-exec'd lookalike from a daemon by PF_FORKNOEXEC (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index f7923042..8e4916c7 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -37,34 +37,31 @@ git init -q && git add -A && git -c user.name=verify -c user.email=verify@localh # The plumb daemons alive now, by their own argv; "?" when the process table # cannot be read. A process that exited mid-scan is skipped, but an empty # listing is not "none": this shell is a process, so /proc must list one. -# A child of a daemon, in the instant between fork and exec, still shows the -# argv of the daemon, so a match whose parent is itself a match is not counted: -# plumb never starts a daemon from a daemon (serve and `plumb restart` do). +# Until exec, a forked child still shows its parent argv, so a daemon starting +# gopls or git briefly looks like a second daemon. The kernel marks that state +# with PF_FORKNOEXEC (0x40) in the stat flags (field 9), so stat is read FIRST +# and a flagged process is skipped; only then is cmdline read, which after exec +# is the process own. A real plumb daemon counts, whatever its parent. daemons() { - local dir argv stat listed=0 pid - local -A ppid_of=() + local n=0 dir stat argv listed=0 rest for dir in /proc/[0-9]*; do [ -d "$dir" ] || continue listed=1 - if ! argv=$(tr "\0" " " < "$dir/cmdline" 2> /dev/null); then + if ! stat=$(< "$dir/stat") 2> /dev/null; then [ -d "$dir" ] && { echo "?"; return; } continue fi - [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] || continue - if ! stat=$(< "$dir/stat") 2> /dev/null; then + rest=${stat##*) } # "state ppid pgrp session tty tpgid flags ..." + read -r _ _ _ _ _ _ flags _ <<< "$rest" + [[ "$flags" =~ ^[0-9]+$ ]] || { echo "?"; return; } + (( flags & 0x40 )) && continue # forked, not yet exec-ed + if ! argv=$(tr "\0" " " < "$dir/cmdline" 2> /dev/null); then [ -d "$dir" ] && { echo "?"; return; } continue fi - stat=${stat##*) } # after the last ")": "state ppid ..." - stat=${stat#* } - [[ "${stat%% *}" =~ ^[0-9]+$ ]] || { echo "?"; return; } - ppid_of[${dir#/proc/}]=${stat%% *} + [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] && n=$((n + 1)) done [ "$listed" -eq 1 ] || { echo "?"; return; } - local n=0 - for pid in "${!ppid_of[@]}"; do - [ -n "${ppid_of[${ppid_of[$pid]}]+x}" ] || n=$((n + 1)) - done echo "$n" } From bad29016dea37439387b6e5c6813766ba81cd7b4 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 14:02:00 +1100 Subject: [PATCH 09/16] fix: verify-full-container keeps history and tags, uses a git with merge-tree -X, and runs as a non-root user (found by pure-antelope) On main the container run failed 7 tests for environmental reasons: no history or tags in the one-commit snapshot (TestLifecycleSaveCost, the v0.0.0 version in TestSmoke_ServeReplacementResumesByName), bookworm's git 2.39 lacking merge-tree -X, and root defeating the chmod-unwritable tests. Co-Authored-By: Claude Opus 5.5 --- build/docker/verify.Dockerfile | 15 ++++++++++++- scripts/verify-full-container.sh | 38 +++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/build/docker/verify.Dockerfile b/build/docker/verify.Dockerfile index 3bfbe14c..b90f3a2d 100644 --- a/build/docker/verify.Dockerfile +++ b/build/docker/verify.Dockerfile @@ -9,7 +9,9 @@ # Every tool is pinned, so a green run means the same thing tomorrow. Keep Go in # step with go.mod's toolchain line, golangci-lint with GOLANGCI_LINT_VERSION in # .github/workflows/ci.yml, and the language servers with the integration job. -FROM golang:1.27.2-bookworm +# trixie, not bookworm: its git (2.47) has `merge-tree -X`, which the git tool's +# tests exercise; bookworm's 2.39 does not. +FROM golang:1.27.2-trixie # Node only to install the npm-published language servers. COPY --from=node:22.23.3-bookworm /usr/local/bin/node /usr/local/bin/node @@ -30,6 +32,17 @@ RUN npm install -g --no-audit --no-fund \ ENV PATH="/go/bin:${PATH}" +# The suite runs as an ordinary user: tests that make a directory unwritable +# with chmod prove nothing as root, who can write it anyway. Its caches live in +# its home, created here so the named cache volumes mounted over them start out +# owned by it. +RUN useradd --create-home --uid 1000 verify \ + && mkdir -p /work /home/verify/.cache/go-build /home/verify/.cache/golangci-lint /home/verify/go/pkg/mod \ + && chown -R verify:verify /work /home/verify +USER verify +ENV HOME=/home/verify GOPATH=/home/verify/go GOCACHE=/home/verify/.cache/go-build \ + GOMODCACHE=/home/verify/go/pkg/mod GOLANGCI_LINT_CACHE=/home/verify/.cache/golangci-lint + # Fail the image, not the run, when a tool is broken. RUN gopls version && golangci-lint --version && pyright --version \ && typescript-language-server --version && command -v vscode-html-language-server diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index 8e4916c7..d42c20cf 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -6,10 +6,11 @@ # plumb daemon: those suites refuse to run outside CI, a container or a declared # VM (internal/separateenv). This is the local way to run the whole gate. # -# The working tree is streamed in as git sees it (tracked and untracked files, -# not ignored ones), so uncommitted changes are verified and host build output -# is not. Inside, it becomes a fresh one-commit git repository, which the checks -# that call git need. The container runs with --init (exited processes are +# The container gets the repository's history (a git bundle of every ref, tags +# included, which the version and history-reading tests need) checked out at +# HEAD, and over it the working tree as git sees it (tracked and untracked +# files, not ignored ones, minus deleted ones), so uncommitted changes are +# verified and host build output is not. It runs as a non-root user. The container runs with --init (exited processes are # reaped, not counted) and --rm: its exit ends every process it started. # While the suite runs, the plumb daemons are counted every 200 ms: at most one # may run at a time. Afterwards none may be left: they are looked for for 30 s. @@ -27,12 +28,13 @@ image=plumb-verify docker build ${platform[@]+"${platform[@]}"} -q -f "$root/build/docker/verify.Dockerfile" -t "$image" "$root/build/docker" > /dev/null || exit 1 name=plumb-verify-$(date +%s)-$$ -trap 'docker kill "$name" > /dev/null 2>&1' INT TERM HUP inside=' set -uo pipefail -mkdir -p /work && tar -C /work -xf - && cd /work || exit 70 -git init -q && git add -A && git -c user.name=verify -c user.email=verify@localhost commit -qm "working tree" || exit 70 +cd /work || exit 70 +git -c init.defaultBranch=main clone -q --no-checkout /in/repo.bundle . && git checkout -q --detach "$(cat /in/head)" || exit 70 +tar -xf /in/tree.tar || exit 70 +while IFS= read -r -d "" f; do rm -f -- "$f"; done < /in/deleted || exit 70 # The plumb daemons alive now, by their own argv; "?" when the process table # cannot be read. A process that exited mid-scan is skipped, but an empty @@ -129,17 +131,27 @@ fi exit "$rc" ' +in=$(mktemp -d "${TMPDIR:-/tmp}/plumb-verify-in.XXXXXX") || exit 1 +trap 'docker kill "$name" > /dev/null 2>&1; rm -rf "$in"' INT TERM HUP ( cd "$root" || exit 1 - git ls-files -z --cached --others --exclude-standard | + git bundle create "$in/repo.bundle" --all 2> /dev/null && + git rev-parse HEAD > "$in/head" && + git ls-files -z --deleted > "$in/deleted" && + git ls-files -z --cached --others --exclude-standard | while IFS= read -r -d '' f; do { [ -e "$f" ] || [ -L "$f" ]; } && printf '%s\0' "$f"; done | - COPYFILE_DISABLE=1 tar --no-xattrs --null -T - -cf - # no macOS ._* AppleDouble entries or xattr headers -) | docker run -i --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ - -v plumb-verify-gocache:/root/.cache/go-build \ - -v plumb-verify-gomod:/go/pkg/mod \ - -v plumb-verify-lintcache:/root/.cache/golangci-lint \ + COPYFILE_DISABLE=1 tar --no-xattrs --null -T - -cf "$in/tree.tar" # no macOS ._* AppleDouble entries or xattr headers +) || { echo "verify-full-container: could not package the repository" >&2; rm -rf "$in"; exit 1; } +chmod -R a+rX "$in" + +docker run --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ + -v "$in:/in:ro" \ + -v plumb-verify-v2-gocache:/home/verify/.cache/go-build \ + -v plumb-verify-v2-gomod:/home/verify/go/pkg/mod \ + -v plumb-verify-v2-lintcache:/home/verify/.cache/golangci-lint \ "$image" bash -c "$inside" rc=$? +rm -rf "$in" # Container exit is what ends a SIGKILL'd run's processes, so "gone" must be # an answer from the engine, never the failure to get one. From e4708b84ae132afe55b6ad85762e0b108ad6c95e Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 14:03:03 +1100 Subject: [PATCH 10/16] docs(changelog): one Changed section under 0.24.1; describe verify-full-container as it now is Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21a3bd6f..174d1ef7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -232,9 +232,6 @@ `workspace_sessions`, and `plumb mail`. A live session in another checkout of the project also counts as a peer for the wake watcher's longer window. No read path runs git. - -### Changed - - **The daemon-starting test suites no longer run on a developer machine.** `cmd/smoke` (the `integration` tag) and `cmd/clientsmoke` (the `clients`, `clients_e2e` and `clients_conformance` tags) start plumb daemons of their @@ -245,10 +242,11 @@ (`internal/separateenv`). This guards against running them by accident; it is not a security boundary. CI is unaffected. The new `make verify-full-container` runs `make verify-full` in a Linux container - with pinned Go, golangci-lint and language servers. It streams in the working - tree and fails if a plumb daemon is still running after the suite. It replaces - a host `make verify-full` as the pre-submit gate. `make docker-integration` - now runs with `--init`. + with pinned Go, git, golangci-lint and language servers, as a non-root user, + on the repository's history with the working tree laid over it. It fails if + two plumb daemons run at once during the suite or one is still running after + it. It replaces a host `make verify-full` as the pre-submit gate. + `make docker-integration` now runs with `--init`. ### Fixed From 27cb69344f1db6150f278e0b4ff98158ac80b13f Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 21:35:03 +1100 Subject: [PATCH 11/16] test: verify-full-container traces the daemons when more than one runs, for diagnosis (PLAN-526) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index d42c20cf..32fb11ef 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -45,7 +45,7 @@ while IFS= read -r -d "" f; do rm -f -- "$f"; done < /in/deleted || exit 70 # and a flagged process is skipped; only then is cmdline read, which after exec # is the process own. A real plumb daemon counts, whatever its parent. daemons() { - local n=0 dir stat argv listed=0 rest + local n=0 dir stat argv listed=0 rest matched="" for dir in /proc/[0-9]*; do [ -d "$dir" ] || continue listed=1 @@ -61,9 +61,10 @@ daemons() { [ -d "$dir" ] && { echo "?"; return; } continue fi - [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] && n=$((n + 1)) + [[ "$argv" =~ ^([^\ ]*/)?plumb\ daemon(\ |$) ]] && { n=$((n + 1)); matched="$matched ${dir#/proc/}"; } done [ "$listed" -eq 1 ] || { echo "?"; return; } + echo "$matched" > /tmp/daemons.now 2> /dev/null # diagnostics only echo "$n" } @@ -75,13 +76,20 @@ supervise() { n=$(daemons) samples=$((samples + 1)) if [ "$n" = "?" ]; then peak="?"; elif [ "$peak" != "?" ] && [ "$n" -gt "$peak" ]; then peak=$n; fi + # Diagnostics only (never the verdict): who the daemons were when more than + # one ran, so an overlap can be traced to the tests that caused it. + if [ "$n" != "?" ] && [ "$n" -gt 1 ] && [ "$({ wc -l < /tmp/overlaps; } 2> /dev/null || echo 0)" -lt 40 ]; then + for pid in $(cat /tmp/daemons.now 2> /dev/null); do + echo "$(date +%T.%N) pid=$pid ppid=$(awk "{print \$4}" /proc/$pid/stat 2> /dev/null) home=$(tr "\0" "\n" < /proc/$pid/environ 2> /dev/null | sed -n "s/^HOME=//p") started=$(ps -o lstart= -p $pid 2> /dev/null)" >> /tmp/overlaps + done + fi # A checkpoint that cannot be written ends the watcher (status 3), which the # checks after the suite reject, rather than leave an older record trusted. { echo "$peak $samples" > /tmp/peak.new && mv /tmp/peak.new /tmp/peak; } || exit 3 sleep 0.2 done } -rm -f /tmp/peak /tmp/peak.new +rm -f /tmp/peak /tmp/peak.new /tmp/overlaps supervise & supervisor=$! @@ -111,7 +119,8 @@ else 0) echo "verify-full-container: no plumb daemon was seen during the suite ($samples samples)" ;; 1) echo "verify-full-container: at most 1 plumb daemon ran at a time during the suite ($samples samples)" ;; "?") echo "verify-full-container: the process table could not be read during the suite"; [ "$rc" -eq 0 ] && rc=1 ;; - *) echo "verify-full-container: $peak plumb daemons ran at once during the suite (at most 1)"; [ "$rc" -eq 0 ] && rc=1 ;; + *) echo "verify-full-container: $peak plumb daemons ran at once during the suite (at most 1)"; [ "$rc" -eq 0 ] && rc=1 + sed "s/^/verify-full-container: overlap: /" /tmp/overlaps 2> /dev/null ;; esac fi From 369d5b1923d87fd55bceb9e1c9f8861bf15eaaf1 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 21:36:52 +1100 Subject: [PATCH 12/16] test: verify-full-container's census keeps a vanished process's read error out of the log Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index 32fb11ef..ff371e5e 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -49,7 +49,7 @@ daemons() { for dir in /proc/[0-9]*; do [ -d "$dir" ] || continue listed=1 - if ! stat=$(< "$dir/stat") 2> /dev/null; then + if ! stat=$({ cat "$dir/stat"; } 2> /dev/null); then [ -d "$dir" ] && { echo "?"; return; } continue fi @@ -57,7 +57,7 @@ daemons() { read -r _ _ _ _ _ _ flags _ <<< "$rest" [[ "$flags" =~ ^[0-9]+$ ]] || { echo "?"; return; } (( flags & 0x40 )) && continue # forked, not yet exec-ed - if ! argv=$(tr "\0" " " < "$dir/cmdline" 2> /dev/null); then + if ! argv=$({ tr "\0" " " < "$dir/cmdline"; } 2> /dev/null); then [ -d "$dir" ] && { echo "?"; return; } continue fi From 98165f6d4e23c6990d04664324240c5d2c0dfbfe Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 21:43:43 +1100 Subject: [PATCH 13/16] test: verify-full-container's census rejects a stat line without its pid (comm) header (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index ff371e5e..6025df43 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -53,7 +53,12 @@ daemons() { [ -d "$dir" ] && { echo "?"; return; } continue fi - rest=${stat##*) } # "state ppid pgrp session tty tpgid flags ..." + # "pid (comm) state ppid pgrp session tty tpgid flags ...": comm may hold + # spaces or ")", so the greedy match ends at the last ") ". A line without + # that header, or naming another pid, is not trusted. + [[ "$stat" =~ ^([0-9]+)\ [\(](.*)[\)]\ (.*)$ ]] && [ "${BASH_REMATCH[1]}" = "${dir#/proc/}" ] || { echo "?"; return; } + rest=${BASH_REMATCH[3]} + flags="" read -r _ _ _ _ _ _ flags _ <<< "$rest" [[ "$flags" =~ ^[0-9]+$ ]] || { echo "?"; return; } (( flags & 0x40 )) && continue # forked, not yet exec-ed From 7d816426de67e0cd619c930553ec822c5d9d5aa3 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 21:59:07 +1100 Subject: [PATCH 14/16] test: verify-full-container runs every test rather than replaying cached results (PLAN-526) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index 6025df43..cb8c0b65 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -98,7 +98,9 @@ rm -f /tmp/peak /tmp/peak.new /tmp/overlaps supervise & supervisor=$! -make verify-full +# Every test runs: the build cache volume is reused between runs, and a test +# result replayed from it would prove nothing, least of all about daemons. +GOFLAGS=-count=1 make verify-full rc=$? echo "verify-full-container: make verify-full exited $rc" From 4a939bba397c48d1a929c75f01ab981c91ae1c49 Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:22:39 +1100 Subject: [PATCH 15/16] test: fold the independent review's should-fixes (PLAN-526) - verify-full-container fails when its watcher sees no daemon at all (cmd/smoke always starts one), reads any non-count as an unread table, keeps the packaging loop's status from its last file, cleans its input dir on any exit, and runs docker in the background so its trap can fire; - a go/parser test pins separateenv.RequireForTestMain as the first statement of each daemon-starting TestMain, which CI and the container, both separate environments, could never notice missing; - docs: the verify message names cmd/smoke, the CHANGELOG no longer claims a pinned git, CONTRIBUTING says the verify image has no client CLIs. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- CONTRIBUTING.md | 5 +-- Makefile | 2 +- internal/separateenv/wiring_test.go | 56 +++++++++++++++++++++++++++++ scripts/verify-full-container.sh | 24 ++++++++----- 5 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 internal/separateenv/wiring_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 174d1ef7..812a0c84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -242,7 +242,8 @@ (`internal/separateenv`). This guards against running them by accident; it is not a security boundary. CI is unaffected. The new `make verify-full-container` runs `make verify-full` in a Linux container - with pinned Go, git, golangci-lint and language servers, as a non-root user, + with pinned Go, golangci-lint and language servers (and Debian trixie's git), + as a non-root user, on the repository's history with the working tree laid over it. It fails if two plumb daemons run at once during the suite or one is still running after it. It replaces a host `make verify-full` as the pre-submit gate. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9eba6630..f4a0ba65 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -108,8 +108,9 @@ make install-clients # or: ./scripts/install-clients.sh ``` These tiers start plumb daemons of their own, so like `cmd/smoke` they refuse on a -developer machine: run them in CI (the client-conformance workflow), a container, or a -disposable VM with `PLUMB_SEPARATE_ENV=vm`. +developer machine: run them in CI (the client-conformance workflow), in a container you +have installed the client CLIs into (the `make verify-full-container` image has none), or +in a disposable VM with `PLUMB_SEPARATE_ENV=vm`. Prerequisites: Node 20+ (npm clients), Python 3.8+ (hermes), `curl` (script-installed clients). Some installers drop binaries in `~/.local/bin` or `~/.npm-global/bin` — add diff --git a/Makefile b/Makefile index 91c8b66f..aceaef7a 100644 --- a/Makefile +++ b/Makefile @@ -443,7 +443,7 @@ verify: build test $(VERIFY_CHECKS) @printf '\n%s\n%s\n%s\n%s\n%s\n\n' \ 'verify: PASSED — but the //go:build integration suite was COMPILED, not RUN.' \ ' A failing integration test does not fail this target.' \ - ' It (make integration-test) starts plumb daemons of its own, so it runs' \ + ' Its cmd/smoke suite (make integration-test) starts plumb daemons, so it runs' \ ' in CI or a container, never beside the plumb daemon you are using.' \ ' Whole gate, in Docker: make verify-full-container' diff --git a/internal/separateenv/wiring_test.go b/internal/separateenv/wiring_test.go new file mode 100644 index 00000000..a61506ca --- /dev/null +++ b/internal/separateenv/wiring_test.go @@ -0,0 +1,56 @@ +package separateenv + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "testing" +) + +// TestGuardIsFirstInEachDaemonStartingTestMain pins the wiring the library +// tests cannot: CI and the verify container both count as separate +// environments, so deleting a guard call would leave every run green. Each +// suite that starts plumb daemons must call RequireForTestMain as the first +// statement of its TestMain, before anything is built or created. +func TestGuardIsFirstInEachDaemonStartingTestMain(t *testing.T) { + for _, file := range []string{ + filepath.Join("..", "..", "cmd", "smoke", "main_test.go"), + filepath.Join("..", "..", "cmd", "clientsmoke", "harness_test.go"), + } { + t.Run(file, func(t *testing.T) { + f, err := parser.ParseFile(token.NewFileSet(), file, nil, parser.SkipObjectResolution) + if err != nil { + t.Fatal(err) + } + var main *ast.FuncDecl + for _, decl := range f.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Recv == nil && fn.Name.Name == "TestMain" { + main = fn + } + } + if main == nil || main.Body == nil || len(main.Body.List) == 0 { + t.Fatal("no TestMain with a body") + } + stmt, ok := main.Body.List[0].(*ast.ExprStmt) + if !ok { + t.Fatal("the first statement of TestMain is not a call") + } + call, ok := stmt.X.(*ast.CallExpr) + if !ok { + t.Fatal("the first statement of TestMain is not a call") + } + sel, ok := call.Fun.(*ast.SelectorExpr) + pkg, pkgOK := func() (*ast.Ident, bool) { + if !ok { + return nil, false + } + id, ok := sel.X.(*ast.Ident) + return id, ok + }() + if !ok || !pkgOK || pkg.Name != "separateenv" || sel.Sel.Name != "RequireForTestMain" { + t.Fatalf("the first statement of TestMain must be separateenv.RequireForTestMain(...)") + } + }) + } +} diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index cb8c0b65..ba946e34 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -12,8 +12,10 @@ # files, not ignored ones, minus deleted ones), so uncommitted changes are # verified and host build output is not. It runs as a non-root user. The container runs with --init (exited processes are # reaped, not counted) and --rm: its exit ends every process it started. -# While the suite runs, the plumb daemons are counted every 200 ms: at most one -# may run at a time. Afterwards none may be left: they are looked for for 30 s. +# While the suite runs, the plumb daemons are counted about every 200 ms (plus +# the time a scan takes): at most one may run at a time, and at least one must +# be seen, since cmd/smoke always starts one. Afterwards none may be left: they +# are looked for for 30 s or more. # The run fails if either rule breaks, or if the process table cannot be read. # # make verify-full-container (DOCKER_PLATFORM=linux/amd64 for amd64) @@ -79,6 +81,7 @@ supervise() { local peak=0 samples=0 n while :; do n=$(daemons) + [[ "$n" =~ ^[0-9]+$ ]] || n="?" # anything but a count is an unread table samples=$((samples + 1)) if [ "$n" = "?" ]; then peak="?"; elif [ "$peak" != "?" ] && [ "$n" -gt "$peak" ]; then peak=$n; fi # Diagnostics only (never the verdict): who the daemons were when more than @@ -123,7 +126,7 @@ if [ "$watched" = no ]; then [ "$rc" -eq 0 ] && rc=1 else case "$peak" in - 0) echo "verify-full-container: no plumb daemon was seen during the suite ($samples samples)" ;; + 0) echo "verify-full-container: no plumb daemon was seen during the suite ($samples samples), though cmd/smoke starts one: the watcher saw nothing"; [ "$rc" -eq 0 ] && rc=1 ;; 1) echo "verify-full-container: at most 1 plumb daemon ran at a time during the suite ($samples samples)" ;; "?") echo "verify-full-container: the process table could not be read during the suite"; [ "$rc" -eq 0 ] && rc=1 ;; *) echo "verify-full-container: $peak plumb daemons ran at once during the suite (at most 1)"; [ "$rc" -eq 0 ] && rc=1 @@ -134,7 +137,7 @@ fi seen=0 for _ in $(seq 1 60); do n=$(daemons) - if [ "$n" = "?" ]; then echo "verify-full-container: cannot read the process table"; exit 1; fi + [[ "$n" =~ ^[0-9]+$ ]] || { echo "verify-full-container: cannot read the process table"; exit 1; } [ "$n" -gt "$seen" ] && seen=$n sleep 0.5 done @@ -148,16 +151,17 @@ exit "$rc" ' in=$(mktemp -d "${TMPDIR:-/tmp}/plumb-verify-in.XXXXXX") || exit 1 -trap 'docker kill "$name" > /dev/null 2>&1; rm -rf "$in"' INT TERM HUP +trap 'rm -rf "$in"' EXIT +trap 'docker kill "$name" > /dev/null 2>&1; exit 130' INT TERM HUP ( cd "$root" || exit 1 git bundle create "$in/repo.bundle" --all 2> /dev/null && git rev-parse HEAD > "$in/head" && git ls-files -z --deleted > "$in/deleted" && git ls-files -z --cached --others --exclude-standard | - while IFS= read -r -d '' f; do { [ -e "$f" ] || [ -L "$f" ]; } && printf '%s\0' "$f"; done | + while IFS= read -r -d '' f; do { [ -e "$f" ] || [ -L "$f" ]; } && printf '%s\0' "$f"; :; done | COPYFILE_DISABLE=1 tar --no-xattrs --null -T - -cf "$in/tree.tar" # no macOS ._* AppleDouble entries or xattr headers -) || { echo "verify-full-container: could not package the repository" >&2; rm -rf "$in"; exit 1; } +) || { echo "verify-full-container: could not package the repository (git bundle, ls-files or tar failed; see above)" >&2; exit 1; } chmod -R a+rX "$in" docker run --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ @@ -165,9 +169,11 @@ docker run --rm --init --name "$name" ${platform[@]+"${platform[@]}"} \ -v plumb-verify-v2-gocache:/home/verify/.cache/go-build \ -v plumb-verify-v2-gomod:/home/verify/go/pkg/mod \ -v plumb-verify-v2-lintcache:/home/verify/.cache/golangci-lint \ - "$image" bash -c "$inside" + "$image" bash -c "$inside" & +child=$! # in the background and waited for, so the trap can run meanwhile +wait "$child" rc=$? -rm -rf "$in" +kill -0 "$child" 2> /dev/null && { wait "$child"; rc=$?; } # Container exit is what ends a SIGKILL'd run's processes, so "gone" must be # an answer from the engine, never the failure to get one. From 56a80d5aeab9cdcdc541309631bd7fc7a9a83eab Mon Sep 17 00:00:00 2001 From: Atlas from Plumb Date: Mon, 12 Oct 2026 22:39:46 +1100 Subject: [PATCH 16/16] test: verify-full-container's signal trap kills the container and still ends on the engine's gone answer (PLAN-526 review) Co-Authored-By: Claude Opus 5.5 --- scripts/verify-full-container.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/verify-full-container.sh b/scripts/verify-full-container.sh index ba946e34..4b252a77 100755 --- a/scripts/verify-full-container.sh +++ b/scripts/verify-full-container.sh @@ -152,7 +152,10 @@ exit "$rc" in=$(mktemp -d "${TMPDIR:-/tmp}/plumb-verify-in.XXXXXX") || exit 1 trap 'rm -rf "$in"' EXIT -trap 'docker kill "$name" > /dev/null 2>&1; exit 130' INT TERM HUP +# A signal kills the container but does not exit: the run still ends with the +# engine's answer on whether the container is gone, and then fails. +interrupted=0 +trap 'interrupted=1; docker kill "$name" > /dev/null 2>&1' INT TERM HUP ( cd "$root" || exit 1 git bundle create "$in/repo.bundle" --all 2> /dev/null && @@ -174,6 +177,7 @@ child=$! # in the background and waited for, so the trap can run meanwhile wait "$child" rc=$? kill -0 "$child" 2> /dev/null && { wait "$child"; rc=$?; } +[ "$interrupted" -eq 1 ] && { echo "verify-full-container: interrupted" >&2; rc=130; } # Container exit is what ends a SIGKILL'd run's processes, so "gone" must be # an answer from the engine, never the failure to get one.