From 24723778bdcdae1b52d7886cab8cc5cc2c5a389e Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Wed, 12 Aug 2026 08:07:36 +0200 Subject: [PATCH] Keep an experiment's code out of the commands the gate runs (#122) Record 0009 decided that nothing which runs on its own builds or executes anything under experiments/. It named no mechanism, and every command in the guide and in the workflows walked the whole module, so the sentence was true of the intention and not of the tree. At 9807dc7 `go list ./...` returned the experiment's package next to the runner's seven, and six build entries, three suite entries, the vet job and the static analysis compiled it on every pull request. What that would have cost is the failure 0009 was written against. A prototype somebody abandoned half-written holds the board red on six platforms it was never about, and the two ways out of a red board are deleting the evidence or rewriting the answer. The commands now name the runner's two roots. gofmt takes cmd and internal, because it reads directories rather than a package pattern, and it is the one command here that fails on an experiment without compiling it. CodeQL moves from autobuild to a manual build of the same two roots: autobuild builds the module, and the first line of that file says its subject is the runner's own source. Record 0017 carries the mechanism, the three options it was chosen over and what each would have cost. The largest of those is measured rather than argued. An experiment as its own module, or behind a build constraint, stops `go run ./experiments/reading-a-tree-of-records` resolving from the root of a checkout, and that command is written twice in an answered record which gains lines rather than having them replaced. The command was run at this commit and still reports its three numbers. What nothing refuses is written in the record rather than left to be found: a Go file at the root, or under a root directory the layout names that is neither cmd/ nor internal/, is outside these patterns. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- .github/workflows/build.yml | 35 ++++-- .github/workflows/codeql.yml | 14 ++- .github/workflows/headless.yml | 7 +- CONTRIBUTING.md | 18 ++- ...17-what-the-gate-reads-of-an-experiment.md | 116 ++++++++++++++++++ 5 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 docs/decisions/0017-what-the-gate-reads-of-an-experiment.md diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index cd437bf..599dd8b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,10 +85,15 @@ jobs: cache: false - name: Build for ${{ matrix.goos }}/${{ matrix.goarch }} + # The two roots are the runner and nothing else. An experiment's code is + # in this module and is deliberately outside this pattern, which is + # record 0017 supplying the mechanism record 0009 decided the property + # for. Widening this back to ./... puts a prototype somebody abandoned + # half-written in front of six platforms it was never about. env: GOOS: ${{ matrix.goos }} GOARCH: ${{ matrix.goarch }} - run: go build ./... + run: go build ./cmd/... ./internal/... test: # One entry per platform record 0012 says the release ships a binary for and @@ -166,9 +171,11 @@ jobs: fi - name: Run the default suite - # The same suite on every entry. No platform gets a reduced set and no - # package is excluded here, because the platform whose difficult tests - # are left out is the platform the defect is on. + # The same suite on every entry. No platform gets a reduced set, because + # the platform whose difficult tests are left out is the platform the + # defect is on. The pattern is the runner's two roots on all three, and + # what it leaves out is an experiment's code rather than any of the + # runner's, which is record 0017. # # -count=1 rather than the cache, so a green entry is a suite that ran # on this machine rather than one whose stored output was replayed. @@ -185,7 +192,7 @@ jobs: PLATFORM: ${{ matrix.platform }} run: | set -euo pipefail - go test -count=1 -v ./... 2>&1 | tee suite.log + go test -count=1 -v ./cmd/... ./internal/... 2>&1 | tee suite.log executed=$(grep -cE '^--- (PASS|FAIL): ' suite.log || true) skipped=$(grep -cE '^--- SKIP: ' suite.log || true) echo "${PLATFORM}: executed ${executed} test(s), skipped ${skipped}" @@ -227,7 +234,10 @@ jobs: cache: false - name: Vet the runner - run: go vet ./... + # The step name and the pattern say the same thing now. Record 0017 + # keeps an experiment's code out of every command here, and vet is one + # of them. + run: go vet ./cmd/... ./internal/... format: name: format @@ -252,12 +262,19 @@ jobs: # so the job has to read the output to fail. Printing the names first # means a red run says which files rather than only that some file was # wrong. + # + # gofmt takes directories rather than a package pattern, so the two + # roots are spelled the way it reads them. This is the one command here + # that would fail on an experiment without compiling it: gofmt exits + # non-zero on a file it cannot parse, and half-written is the ordinary + # state of a prototype somebody abandoned. Record 0017 is where that is + # argued. run: | set -euo pipefail - unformatted=$(gofmt -l .) + unformatted=$(gofmt -l cmd internal) if [ -n "$unformatted" ]; then echo "$unformatted" - echo "::error::These files are not gofmt-formatted. Run 'gofmt -w .'." + echo "::error::These files are not gofmt-formatted. Run 'gofmt -w cmd internal'." exit 1 fi - echo "Every Go file in this tree is gofmt-formatted." + echo "Every Go file the runner is built from is gofmt-formatted." diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 094eb2e..16526da 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -71,7 +71,19 @@ jobs: uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 with: languages: go - build-mode: autobuild + # Manual rather than autobuild, because autobuild builds the module + # and the module holds the experiments as well. The subject of this + # workflow is the runner's own source, which is the first line of this + # file, and under autobuild that sentence was not true of what ran. + # Record 0017 is where the pattern below is fixed. + build-mode: manual + + - name: Build the runner + # The same two roots the build workflow, the suite and the guide name. + # A finding is reported against what this step compiled, so widening it + # would put an abandoned prototype in the code-scanning tab under the + # runner's name. + run: go build ./cmd/... ./internal/... - name: Analyse uses: github/codeql-action/analyze@5595ccaf912efad79be6eef63a5619ff05969be3 # v4 diff --git a/.github/workflows/headless.yml b/.github/workflows/headless.yml index 34f2d7b..b1cdec6 100644 --- a/.github/workflows/headless.yml +++ b/.github/workflows/headless.yml @@ -94,14 +94,17 @@ jobs: done - name: Run the default suite - # The same default run, with no opt-in and no reduced set. The count is + # The same default run, with no opt-in and no reduced set. The pattern + # is the runner's two roots, which is what the default run is: record + # 0017 keeps an experiment's code out of every command, and this job + # runs the same string the build workflow and the guide do. The count is # what separates a suite that ran everything and passed from one that # executed nothing and reported success, which look identical from # outside. Top-level results are counted: a subtest's line is indented, # so the anchored pattern does not read it twice. run: | set -euo pipefail - go test -count=1 -v ./... 2>&1 | tee suite.log + go test -count=1 -v ./cmd/... ./internal/... 2>&1 | tee suite.log executed=$(grep -cE '^--- (PASS|FAIL): ' suite.log || true) skipped=$(grep -cE '^--- SKIP: ' suite.log || true) echo "with no display and no elevation: executed ${executed} test(s), skipped ${skipped}" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6c9d07c..bc05eea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,12 +12,22 @@ difference is meant to be visible. Run these, in this order, from the root of a checkout: ``` -go build ./... -go vet ./... -gofmt -l . -go test -count=1 -v ./... +go build ./cmd/... ./internal/... +go vet ./cmd/... ./internal/... +gofmt -l cmd internal +go test -count=1 -v ./cmd/... ./internal/... ``` +Every one of them names the runner rather than the whole module, and an +experiment's code is outside all four. That is +[docs/decisions/0017-what-the-gate-reads-of-an-experiment.md](docs/decisions/0017-what-the-gate-reads-of-an-experiment.md), +which supplies the mechanism record +[docs/decisions/0009-the-checks-do-not-run-experiment-code.md](docs/decisions/0009-the-checks-do-not-run-experiment-code.md) +decided the property for. A prototype you abandon half-written is a thing this +board takes, and it holds nothing red. What follows from that is that nothing +compiles your experiment for you: run it yourself, from a checkout, at the +moment you want the answer. + `gofmt -l` prints the files it would change and exits zero either way, so read its output rather than its exit code. No output is the passing result. diff --git a/docs/decisions/0017-what-the-gate-reads-of-an-experiment.md b/docs/decisions/0017-what-the-gate-reads-of-an-experiment.md new file mode 100644 index 0000000..f041321 --- /dev/null +++ b/docs/decisions/0017-what-the-gate-reads-of-an-experiment.md @@ -0,0 +1,116 @@ +# 0017. What the gate reads of an experiment + +## What was decided + +Every command this repository is gated by names the runner's packages rather +than the whole module. Where a command takes a package pattern that is +`./cmd/... ./internal/...`, and where it takes directories it is `cmd` and +`internal`. It is one set of commands rather than two, so what CONTRIBUTING.md +asks somebody to run before they push and what a job runs are the same strings. + +Record `0009` decided the property this implements. It says nothing that runs on +its own builds or executes anything under `experiments/`, and it names no +mechanism, so the sentence was true of the intention and not of the tree. This +record supplies the mechanism and takes nothing away from that one. + +The state that made the gap visible. An experiment lives inside this module, so +its Go package was in every pattern the commands used, and the first experiment +to land put it there: + + git rev-parse HEAD + 9807dc783b9b981b652bdf00ac1e1467171a1051 + go list ./... + github.com/Flowfin/lab/cmd/lab + github.com/Flowfin/lab/cmd/pullrequest + github.com/Flowfin/lab/experiments/reading-a-tree-of-records + github.com/Flowfin/lab/internal/check + github.com/Flowfin/lab/internal/hardware + github.com/Flowfin/lab/internal/invariants + github.com/Flowfin/lab/internal/prose + github.com/Flowfin/lab/internal/pullrequest + +Six build entries, three suite entries, the vet job and the static analysis all +compiled that package on every pull request, and the formatting job read its +bytes. A prototype that does not compile for one of the six platforms would have +held the board red, and the failure would have named a platform the experiment +was never about. + +The two roots are record `0002`'s list read a second way. That record puts the +entry point in `cmd/lab/` and everything the runner is built from in +`internal/`, and says the runner imports nothing from `experiments/`. So the +question of what the gate covers is a question about the layout rather than +about a pattern somebody picked. + +One thing nothing refuses, written here rather than left to be found. A Go file +at the root of the tree, or under a root directory the layout names that is +neither `cmd/` nor `internal/`, is outside these patterns, and no command +reaches it. A new root directory is refused by +`root-holds-a-directory-the-layout-does-not-name`, so the shape that escapes is +a package added under a directory already named, and what stands behind that is +a reader. That is a smaller hole than the one this record closes and it is not +zero. + +## What it applies to + +Every command in CONTRIBUTING.md and every job that runs one, now and later. A +job added afterwards that walks this module with a Go toolchain takes the same +two patterns rather than the whole module. + +It does not apply to what a person runs in their own checkout. `go run +./experiments/reading-a-tree-of-records` is written in that experiment's record +and still runs from the root of a checkout, which is one of the reasons the +mechanism is this one and not another. + +It does not apply to the checks that read an experiment's record as text. +Record `0009` already separates reading a record from executing an experiment, +and nothing here narrows what the record checks walk. + +## What else was considered + +Giving each experiment its own Go module. + +Holding an experiment's Go files behind a build constraint. + +Holding experiments to the gate and writing that down as deliberate. + +Leaving the patterns alone and letting the first abandoned prototype decide. + +## What each rejected option would have cost + +A module per experiment costs a landed record its command. A directory holding +its own `go.mod` leaves the root module, which is the attraction, because every +pattern stops at it with nothing written down anywhere. The cost arrives at the +other end: `go run ./experiments/reading-a-tree-of-records` stops resolving from +the root of a checkout, and that command is in +`experiments/reading-a-tree-of-records/EXPERIMENT.md` twice, once in the method +and once inside the quoted answer. A record here gains lines and does not have +them replaced, so the repair would be an answered record whose method names a +command that does not run. It also puts a toolchain version in the tree per +experiment, on a board whose whole argument is that writing one should be cheap. + +A build constraint costs the same command for the same reason one level down. A +file every tag excludes is not run by `go run` either, so the record's command +would need a flag it does not carry. It costs one thing the module option does +not: the constraint is a line at the top of a file that somebody has to +remember, nothing refuses its absence, and the day it is forgotten is the day an +abandoned prototype reddens the board, which is the failure being prevented +here. + +Holding experiments to the gate is the option that would replace `0009` rather +than sit beside it, and nothing has happened to weaken the argument that record +makes. Its first reason is still the strongest: the code left behind by an +experiment that answered no is often exactly how it failed, and a gate that goes +red on that code leaves two ways out, deleting the evidence or rewriting the +answer. It would also make every toolchain any experiment ever wants a +dependency of this repository's checks. + +Leaving the patterns alone costs `0009` its meaning. A decision record that is +not true of the tree is worse than an absent one, because it is quoted by +somebody who has no reason to go and check. + +## The condition that reopens this + +Record `0009` carries the condition that reopens the property, and this record +follows it rather than holding one of its own. If experiments start carrying +code that is meant to keep working, the mechanism here is not what is wrong +first.