sneak/quak
1
0
Fork 0
Code Issues 23 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Revert the 16 commits pushed to next on 2026-09-04 by an agent outside the managed fleet
All checks were successful
check / check (push) Successful in 1m40s
Details

Browse Source 
sneak, 2026-09-05: "inference instance stopped. undo its rogue work." The
reverted commits stay in history; nothing else on next is touched.

Model: fable-5-1
This commit is contained in:
clawbot
2026-09-05 09:33:23 +00:00
parent 48db9b438a
commit d1d6cdd4f0
2 changed files with 45 additions and 753 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
README.md
View File

@@ -108,14 +108,11 @@ alpine. We provide:
### Linting
Linting runs in a container, one way, in every path that produces a lint verdict
for `make check` or the pre-commit hook. `script/lint` builds `Dockerfile.lint`,
which copies the repo into a digest-pinned node image and runs eslint and
prettier as build steps, so a successful build is a clean lint. No lint path
reachable from `script/check` or `script/precommit` runs on the host: every lint
verdict those two produce comes from the container. Docker is therefore required
to lint, and the container route also works where the docker daemon is remote
and bind mounts are impossible.
Linting runs in a container, one way, everywhere. `script/lint` builds
`Dockerfile.lint`, which copies the repo into a digest-pinned node image and
runs eslint and prettier as build steps, so a successful build is a clean lint.
There is no host lint path: docker is required to lint, and that also works
where the docker daemon is remote and bind mounts are impossible.
The formatting check is part of that, not a step beside it. `script/check` and
`script/precommit` therefore call `script/lint` and stop; neither calls
@@ -127,20 +124,15 @@ in each. `test/packaging/lint-once.test.ts` asserts that count by walking the
invocation graph, so a second pass cannot creep back in unnoticed.
`script/fmt-check` remains as a standalone entrypoint for asking the formatting
question on its own, without docker and without the rest of lint. It is not the
last formatting path on the host — `script/fmt` writes with the host prettier,
and `package.json` exposes `fmt` and `fmt-check` as direct prettier calls — it
is the one of them that the check graph could plausibly have reached, and
nothing reaches it: neither `script/check` nor `script/precommit` calls it, so
it never contributes to their verdicts. Its own verdict cannot drift from the
container's: prettier is pinned to an exact version, installed from `yarn.lock`
under `--frozen-lockfile` in both places, and reads `.gitignore` as its default
ignore file — which is why `.dockerignore` deliberately keeps `.gitignore` in
the build context.
question on its own, without docker and without the rest of lint. Its verdict
cannot drift from the container's: prettier is pinned to an exact version,
installed from `yarn.lock` under `--frozen-lockfile` in both places, and reads
`.gitignore` as its default ignore file — which is why `.dockerignore`
deliberately keeps `.gitignore` in the build context.
Lint happens in exactly one place in that graph, which constrains the rest of
the build. `script/check` calls `script/lint`, so `make check` cannot run inside
a container without asking for docker inside docker. The image built from
Lint happens in exactly one place, which constrains the rest of the build.
`script/check` calls `script/lint`, so `make check` cannot run inside a
container without asking for docker inside docker. The image built from
`Dockerfile` therefore runs `make test` and `make build` and does not lint;
`script/cibuild` builds `Dockerfile.lint` first and that image second, so CI
gets both verdicts.
@@ -534,8 +526,8 @@ documents:
runs the tests) must pass before merging to `main`. `make fmt-check` is
available for a host-side formatting check on its own, but it is not a
separate requirement: `make lint` already covers it, and running both would
check formatting twice. Never invoke eslint or prettier directly; every lint
verdict `make check` and the pre-commit hook produce comes from the container.
check formatting twice. Never invoke eslint or prettier directly; linting runs
in the container only.
- **Formatting:** prettier with 4-space indents and `proseWrap: always` for
markdown. Use `make fmt` to format. Use `yarn` not `npm`.

760
test/packaging/lint-once.test.ts
View File

@@ -16,56 +16,10 @@
// against one file. Starting from an entrypoint, it follows every edge the repo
// actually uses to reach another command — `run:` steps in the CI workflow,
// `"$SCRIPT_DIR/<name>"` and `script/<name>` into other scripts, `make <target>`
// and `$(MAKE) <target>` through the Makefile shims, the prerequisites of a
// Makefile target, `yarn run <name>` through the `package.json` scripts, and
// `docker build` into the `RUN` steps of the Dockerfile it names, heredoc
// bodies included — and counts the prettier invocations it finds.
//
// What that covers is worth stating exactly rather than as "anywhere", because
// a comment claiming more than the code delivers is the same defect as an
// assertion that cannot fail — and an earlier draft of this header did claim
// it, while `check: fmt-check` and `@$(MAKE) fmt-check` both walked straight
// past it. Command names are read as `[a-z][a-z0-9_-]*` for Makefile targets
// and `script/` files, and `[A-Za-z][A-Za-z0-9_:-]*` for `package.json`
// scripts. A command reached by some other means — `sh -c`, a shell alias, an
// `include`d makefile, a name outside those charsets, a generated file — is
// not followed. Recipe lines reached through a make conditional are followed,
// without evaluating the condition, and so is a recipe written on the target
// line after a `;`; a recipe built by an `include`d makefile, a pattern rule,
// or a target name outside that charset is not. A variable given a literal
// value on one line — `VAR := ...`, `VAR = ...`, `VAR ?= ...` — is substituted
// wherever the Makefile spells it `$(VAR)` or `${VAR}`, because this Makefile
// writes its recipes that way (`@$(YARN) tsc --watch`); the substitution is
// one pass over literals, so a value that itself names another variable, a
// value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the
// body of a `define`/`endef` block are not expanded and not followed. A
// build is recognised as `docker build`, as its three management-command
// spellings (`docker buildx build`, `docker image build`, `docker builder
// build`), and as any of those reached through global flags standing between
// the command and the subcommand (`docker --context ci build`, `docker -H
// tcp://h:2375 buildx build`), a flag's separate argument being stepped over;
// `docker compose build` is deliberately not, since it builds services out of
// a compose file this walk does not read, and neither is any other non-flag
// word in that position. Matching only a literal `docker build` was worse than a
// wrong file: `docker buildx build -f <file> .` emitted no edge at all, not
// even the default one. A recognised build resolves to the file named by
// `-f`, `-f=`, `-f<file>` with the value attached to the flag, `--file` or
// `--file=`, wherever in that invocation the flag sits, and to `Dockerfile`
// when it names none; a Dockerfile chosen some other way — a bundled short
// flag cluster (`-qf <file>`), a value this walk cannot expand to a literal —
// resolves to the default rather than to the real file. Each build on a line
// is followed separately, and the flag search for one is bounded to the slice
// running from that command to the next `&&`, `||`, `;` or `|`, so a second
// build on the same line is not swallowed by the first and a later command's
// `-f` is not read as the build's. That bounding is a split on those four
// separators, not a shell parse: a separator appearing inside quotes or a
// command substitution still ends the slice, so a flag standing after one —
// `docker build --build-arg MSG="a;b" -f Dockerfile.lint .` — is not read.
// Inside a Dockerfile the `RUN` keyword is matched case-insensitively and may
// be followed by any whitespace, because Docker executes `run ...` and a
// tab-separated `RUN\t...` exactly as it executes `RUN ...`; an instruction
// indented from the left margin is read too. Within those edges, a prettier
// call is caught wherever it is added.
// through the Makefile shims, `yarn run <name>` through the `package.json`
// scripts, and `docker build -f <file>` into that Dockerfile's `RUN` steps — and
// counts the prettier invocations it finds. A prettier call added anywhere in
// that graph is therefore caught, wherever it is added.
//
// Two entrypoints are walked, because they cover different graphs: `make check`
// is what a developer runs, and `.gitea/workflows/check.yml` is what CI runs.
@@ -78,9 +32,8 @@
// Undercounting is the failure mode that would make this test worthless. Three
// things guard against it: the walk is asserted to have reached the nodes that
// matter, an unresolvable or empty node is a thrown error rather than a quiet
// zero, and both the counting and the edge-following are per occurrence rather
// than per line, so two prettier calls or two `docker build`s chained with `&&`
// on one line cannot read as one.
// zero, and prettier is counted per occurrence rather than per line, so two
// invocations chained with `&&` cannot read as one.
import { describe, expect, it } from "vitest";
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
@@ -130,189 +83,26 @@ const countPrettier = (line: string): number =>
// Makefile targets are thin shims (`check:` / tab / `@script/check`), so a
// `make <target>` edge has to resolve through them to keep "per `make check`"
// meaning what it says. Recipe lines are the tab-indented ones.
//
// The prerequisite list is read as well, and it is not decoration: make runs a
// target's prerequisites before its recipe, so `check: fmt-check` invokes
// fmt-check every bit as much as a `@script/fmt-check` line in the recipe
// would. An earlier version of this parser captured the target name and threw
// the rest of the line away, which made `check: fmt-check` — a one-token edit,
// and the most natural way for someone to wire the host formatting check back
// into `make check` — a second prettier pass that this file scored as one. The
// shipped Makefile already relies on prerequisites (`install: build-bin`), so
// this is the file's own house style rather than a contrived evasion.
interface MakeTarget {
prerequisites: string[];
recipe: string[];
}
// Names are lowercase-with-dashes here, plus digits and underscores. The
// leading character is deliberately not uppercase: `YARN := yarn run` is a
// variable assignment, not a rule, and `(?!=)` alone would not say so on a
// line spelled `YARN: yarn run`.
const MAKE_TARGET_NAME = "[a-z][a-z0-9_-]*";
// Conditional directives are not target lines, and they are not recipe lines
// either — they are parse-time structure wrapped around the recipe that
// encloses them. Treating one as an ordinary non-tab line ends the current
// recipe, and every tab-indented line after it is discarded, so
//
// check:
// \t@script/check
// ifeq (1,1)
// \t@script/fmt-check
// endif
//
// reads as a recipe of one command while `make -n check` prints two. The
// condition is deliberately not evaluated: that would mean evaluating make
// variables, and the conservative reading — every branch is reachable, so
// every branch's lines are recipe lines — is the one that cannot lose an
// invocation. Counting a command from a branch make would skip is a false
// alarm someone fixes; missing one is the failure this file exists to prevent.
const MAKE_CONDITIONAL = /^\s*(?:ifeq|ifneq|ifdef|ifndef|else|endif)\b/;
// `@$(FMT)` is one token away from being a second prettier pass, and until the
// variable is resolved it reads as a line that invokes nothing: `make -n check`
// printed both `script/check` and `script/fmt-check` while this file scored
// one. `$(MAKE)` was already special-cased below, which is this same rule
// half-applied to a single name; this is the general form of it.
//
// Deliberately only the simple case: a name assigned a literal on one line,
// optionally through an `export`/`override` prefix. `:=` and `=` differ in
// when make expands them, which does not change the single value a literal can
// take, so those two are read the same way and the last one wins. `?=` differs
// in whether it assigns at all — it is skipped when the name already has a
// value — so among assignments to one name the first `?=` wins, and reading it
// as last-wins would resolve a reference to the string make discards. What is
// not read is anything needing evaluation — a value containing another
// `$(...)`, a make function, or a `define` body —
// because expanding those means implementing make, and a half-implementation
// that resolves a variable to the wrong string would count invocations that do
// not happen. An unresolved reference is left standing verbatim instead, which
// is the same dead end as any other unfollowed edge rather than a wrong answer.
const MAKE_ASSIGNMENT = new RegExp(
`^(?:(?:export|override)\\s+)*([A-Za-z_][A-Za-z0-9_]*)\\s*(:=|\\?=|=)\\s*(.*)$`,
);
const MAKE_VARIABLE_REFERENCE = /\$[({]([A-Za-z_][A-Za-z0-9_]*)[)}]/g;
// `define`/`endef` bodies are skipped rather than parsed: the body is a
// multi-line value, so reading its lines as assignments would take whatever
// `=` they happen to contain and call it a variable.
const makeVariables = (text: string): Map<string, string> => {
const variables = new Map<string, string>();
let inDefine = false;
for (const raw of text.split("\n")) {
if (/^\s*endef\b/.test(raw)) {
inDefine = false;
continue;
}
if (/^\s*define\b/.test(raw)) {
inDefine = true;
continue;
}
if (inDefine || raw.startsWith("\t")) continue;
const assignment = MAKE_ASSIGNMENT.exec(raw.replace(/#.*$/, "").trim());
if (assignment === null) continue;
const [, name, operator, value] = assignment;
if (name === undefined || operator === undefined || value === undefined)
continue;
// `?=` assigns only when the name has no value yet, so the first one
// wins where `:=` and `=` let the last one win. Overwriting here would
// resolve the reference to a string make never uses.
if (operator === "?=" && variables.has(name)) continue;
// A value that is itself a reference is the recursive case, and is not
// resolved. Recording it would hand the substitution below a string it
// cannot finish expanding.
if (/\$[({]/.test(value)) continue;
variables.set(name, value.trim());
}
return variables;
};
// One pass, and only over names that were assigned a literal: the result is
// never re-scanned for further references, so this cannot recurse or diverge.
const expandMakeVariables = (
line: string,
variables: Map<string, string>,
): string =>
line.replace(
MAKE_VARIABLE_REFERENCE,
(reference, name: string) => variables.get(name) ?? reference,
);
const parseMakefile = (text: string): Map<string, MakeTarget> => {
const recipes = new Map<string, MakeTarget>();
// Collected in a pass of their own because make reads the whole file
// before it runs anything: a recipe may spell a variable that the Makefile
// assigns further down, and a single pass would leave that one unexpanded
// purely because of where its author put it.
const variables = makeVariables(text);
const makeRecipes = (): Map<string, string[]> => {
const recipes = new Map<string, string[]>();
let current: string | null = null;
for (const line of text.split("\n")) {
// Expanded before parsing rather than only on recipe lines, so a
// prerequisite written `check: $(FMT)` resolves the same way a recipe
// line does. Assignment lines are expanded too and are unaffected: a
// literal value has nothing to substitute.
const raw = expandMakeVariables(line, variables);
for (const raw of read("Makefile").split("\n")) {
if (raw.startsWith("\t")) {
if (current !== null) {
recipes
.get(current)
?.recipe.push(raw.trim().replace(/^[@-]+/, ""));
recipes.get(current)?.push(raw.trim().replace(/^[@-]+/, ""));
}
continue;
}
if (MAKE_CONDITIONAL.test(raw)) continue;
const target = new RegExp(`^(${MAKE_TARGET_NAME})\\s*:(?!=)(.*)$`).exec(
raw,
);
current = target === null ? null : (target[1] ?? null);
if (target === null || current === null) continue;
// `target: prereqs ; command` puts the first recipe line on the target
// line itself. Read as prerequisites the whole of it, `;` and the
// command split into tokens that name no target and are filtered out,
// and the invocation disappears — `check: ; @script/fmt-check` was a
// second prettier pass this parser scored as none.
const rest = target[2] ?? "";
const semicolon = rest.indexOf(";");
const inlineRecipe =
semicolon === -1 ? "" : rest.slice(semicolon + 1).trim();
// Trailing `# comment` is not a prerequisite; neither is the empty
// string a split leaves behind.
const prerequisites = (
semicolon === -1 ? rest : rest.slice(0, semicolon)
)
.replace(/#.*$/, "")
.trim()
.split(/\s+/)
.filter((name) => name !== "");
const existing = recipes.get(current);
if (existing === undefined) {
recipes.set(current, { prerequisites, recipe: [] });
} else {
existing.prerequisites.push(...prerequisites);
}
if (inlineRecipe !== "") {
recipes
.get(current)
?.recipe.push(inlineRecipe.replace(/^[@-]+/, ""));
const target = /^([a-z][a-z-]*)\s*:(?!=)/.exec(raw);
current = target === null ? null : target[1];
if (current !== null && !recipes.has(current)) {
recipes.set(current, []);
}
}
return recipes;
};
const recipes = parseMakefile(read("Makefile"));
// Only prerequisites that name a target of this Makefile are followed: the
// rest are filenames, or expansions of variables this parser does not
// evaluate, and neither is an invocation of anything it could read.
const prerequisitesOf = (node: string): string[] => {
if (!node.startsWith("make:")) return [];
const target = recipes.get(node.slice("make:".length));
if (target === undefined) return [];
return target.prerequisites
.filter((name) => recipes.has(name))
.map((name) => `make:${name}`);
};
const recipes = makeRecipes();
const packageScripts = (): Record<string, string> => {
const pkg = JSON.parse(read("package.json")) as {
@@ -323,63 +113,19 @@ const packageScripts = (): Record<string, string> => {
const scripts = packageScripts();
// A `RUN <<EOF` body is a command the image executes, and BuildKit runs it on
// the default frontend with no `# syntax=` directive, so keeping only the
// lines that begin `RUN ` would let a whole shell script hide one line below
// one. The body is returned alongside the `RUN` line that opened it. An
// unterminated heredoc is a thrown error rather than a silent truncation of
// the rest of the file, for the same reason every other dead end here is.
const HEREDOC_OPEN = /<<-?\s*(['"]?)([A-Za-z_][A-Za-z0-9_]*)\1/;
// Dockerfile instruction keywords are case-insensitive and are separated from
// their arguments by any run of whitespace, so `run yarn run prettier ...` and
// a tab-separated `RUN\tyarn run prettier ...` are both instructions the image
// really executes. A literal `line.startsWith("RUN ")` saw neither, and either
// one appended to the Dockerfile put a second prettier pass into the image
// that `script/cibuild` builds while this file still reported green. The
// separator is consumed by the match rather than by a fixed-width `slice`, so
// the captured command is the same string for a space and for a tab, and the
// heredoc terminator is read off that command exactly as before.
//
// Leading whitespace before the keyword — which a Dockerfile also permits — is
// already gone by this point: `joinContinuations` trims every line, so an
// indented instruction arrives here flush and needs nothing further.
const DOCKER_RUN = /^RUN\s+(.*)$/i;
const dockerRunCommands = (text: string): string[] => {
const commands: string[] = [];
let terminator: string | null = null;
for (const line of executable(text)) {
if (terminator !== null) {
if (line === terminator) terminator = null;
else commands.push(line);
continue;
}
const run = DOCKER_RUN.exec(line);
if (run === null) continue;
const command = run[1] as string;
terminator = HEREDOC_OPEN.exec(command)?.[2] ?? null;
commands.push(command);
}
if (terminator !== null) {
throw new Error(`unterminated heredoc: ${terminator}`);
}
return commands;
};
// Node keys: `script/<name>`, `docker:<Dockerfile>`, `make:<target>`,
// `yarn:<package.json script>`, `workflow:<CI workflow file>`.
const resolve = (node: string): string[] => {
if (node.startsWith("script/")) return executable(read(node));
if (node.startsWith("docker:")) {
return dockerRunCommands(read(node.slice("docker:".length)));
return executable(read(node.slice("docker:".length)))
.filter((line) => line.startsWith("RUN "))
.map((line) => line.slice("RUN ".length));
}
// The `run:` steps of a workflow, in file order. `uses:` steps are actions,
// not commands, and have no edges into this repo's graph. A `run: |` block
// resolves to the bare `|`, which reaches nothing — so the count is exactly
// what such a step does NOT move, and it is the pinned resolved list, not
// the count, that turns it red. See the exact-equality test at the bottom
// of this file.
// would resolve to the bare `|`, which reaches nothing and therefore fails
// the count rather than passing quietly.
if (node.startsWith("workflow:")) {
return executable(read(node.slice("workflow:".length)))
.filter((line) => /^-?\s*run:\s*\S/.test(line))
@@ -394,7 +140,7 @@ const resolve = (node: string): string[] => {
if (recipe === undefined) {
throw new Error(`no such Makefile target: ${target}`);
}
return recipe.recipe;
return recipe;
}
if (node.startsWith("yarn:")) {
const name = node.slice("yarn:".length);
@@ -410,128 +156,42 @@ const resolve = (node: string): string[] => {
// Same reasoning as the missing-target error, applied to every node kind: a
// node that resolves to no commands contributes zero prettier invocations and
// zero edges, which is indistinguishable from a clean result. Fail instead.
//
// A Makefile target with prerequisites and an empty recipe is the one case
// that is genuinely not vacuous — it runs its prerequisites — so it is not
// caught here.
const commandsOf = (node: string): string[] => {
const commands = resolve(node);
if (commands.length === 0 && prerequisitesOf(node).length === 0) {
if (commands.length === 0) {
throw new Error(`node resolved to no commands: ${node}`);
}
return commands;
};
// Which invocation shapes read as a build, and which do not.
//
// RECOGNISED: `docker build`, the three management-command spellings of the
// same thing — `docker buildx build`, `docker image build`, `docker builder
// build` — and any of those reached through global flags standing between the
// command and the subcommand: `docker --context ci build`, `docker -H
// tcp://host:2375 build`, `docker --debug buildx build`. A global flag's
// separate argument is stepped over (`--context ci`), and an `=`-joined one is
// a single token (`--context=ci`). All of these read `-f` the same way and
// build the Dockerfile it names, so all of them are edges.
//
// `docker buildx build` was the concrete miss: `\bdocker\s+build\b` cannot
// reach across `buildx`, so `docker buildx build -f Dockerfile.extra .` in
// script/cibuild emitted no edge at all — not even the default `Dockerfile`
// one, which the old file-flag bug at least still produced — and a tree
// running prettier twice reported green.
//
// NOT RECOGNISED, deliberately: `docker compose build`, which builds services
// named in a compose file this parser does not read, so resolving it against a
// Dockerfile path would be a wrong answer rather than a missing one. Only
// `-`-prefixed tokens (plus their arguments) and those three literal
// subcommand words are stepped over, so no other non-flag word between
// `docker` and `build` matches, and `docker run -f build` is not a build.
const DOCKER_BUILD =
/\bdocker(?:\s+-{1,2}[A-Za-z][\w-]*(?:=\S+)?(?:\s+[^-\s]\S*)?)*(?:\s+(?:buildx|image|builder))?\s+build\b/g;
const edgesOf = (line: string): string[] => {
const edges: string[] = [];
// `"$SCRIPT_DIR/lint"`, `"$ROOT/script/lint"` and a bare `script/lint` are
// all the same edge.
for (const match of line.matchAll(
new RegExp(
`(?:\\$SCRIPT_DIR|\\$\\{SCRIPT_DIR\\}|script)/(${MAKE_TARGET_NAME})`,
"g",
),
/(?:\$SCRIPT_DIR|\$\{SCRIPT_DIR\}|script)\/([a-z][a-z-]*)/g,
)) {
edges.push(`script/${match[1]}`);
}
// Only real targets: `pkg_install gnumake make make make` in
// script/bootstrap is a package name, not an invocation of this Makefile.
//
// `$(MAKE)` and `${MAKE}` count as `make`. Recursive make is spelled that
// way by convention rather than as a literal `make`, and this Makefile
// already expands variables into recipes (`@$(YARN) tsc --watch`), so a
// case-sensitive literal-only match left `@$(MAKE) fmt-check` — the other
// one-token way to put the host prettier pass back into `make check` —
// unfollowed. Valueless flags between the command and the target (`make
// -n check`, `make -j4 check`) are skipped. A flag that takes a separate
// argument is not: `make -C sub build` runs `sub/Makefile`'s target, not
// this one's, and resolving it against these recipes would be wrong
// rather than merely incomplete.
for (const match of line.matchAll(
new RegExp(
`(?:\\$\\(MAKE\\)|\\$\\{MAKE\\}|\\bmake)(?:\\s+-\\S+)*\\s+(${MAKE_TARGET_NAME})`,
"g",
),
)) {
for (const match of line.matchAll(/\bmake\s+([a-z][a-z-]*)/g)) {
if (recipes.has(match[1] ?? "")) edges.push(`make:${match[1]}`);
}
// Same rule for yarn: `yarn run prettier` is the linter itself (counted,
// not followed), `yarn run fmt-check` would be a package.json script that
// runs it indirectly.
//
// The name charset is wider than the Makefile's because `package.json`
// script names conventionally carry colons, digits and underscores
// (`lint:fmt`, `test:e2e`). Under the narrower charset `yarn run lint:fmt`
// read as `yarn run lint`, which is not a script, so it produced no edge
// and no count.
for (const match of line.matchAll(
/\byarn(?:\s+run)?\s+([A-Za-z][A-Za-z0-9_:-]*)/g,
)) {
for (const match of line.matchAll(/\byarn(?:\s+run)?\s+([a-z][a-z-]*)/g)) {
if ((match[1] ?? "") in scripts) edges.push(`yarn:${match[1]}`);
}
// The container lint pass lives behind a `docker build`; without following
// it the count would miss the one invocation that is supposed to survive.
//
// Every spelling of the file flag is read, not just `-f <file>`. Docker
// accepts `--file <file>`, `--file=<file>`, `-f=<file>` and the value
// attached to the short flag as one token (`-f<file>`) for the same thing,
// and matching `-f ` alone resolved all of them to the default
// `Dockerfile` edge — so `docker build --file=Dockerfile.lint .` in a
// recipe was followed into the wrong file, counted nothing, and reported
// green. The flag is searched for anywhere in the invocation, as `-f`
// already was, because a real build line wraps it in `--build-arg` and
// other flags. A leading `\s` is required so that a longer flag ending in
// the same letters cannot supply the match, and the attached form is
// allowed only for the short flag, so that `--force-rm` — a long flag that
// merely starts with the same letters — still does not read as one.
//
// Every `docker build` on the line produces an edge, and each one looks for
// its file flag only within its own invocation — the slice from the command
// to the next `&&`, `||`, `;` or `|`. One `test` and one `exec` over the
// whole line got both halves of that wrong: `docker build -f Dockerfile.lint
// . && docker build -f Dockerfile.extra .` followed the first file and
// dropped the second, so a tree running prettier twice reported green, and a
// bare `docker build . && cp -f Dockerfile.lint /tmp/x` read `cp`'s `-f` as
// the build's, counting an invocation that never happens and losing the
// default `Dockerfile` edge. The bounding is a split on those four
// separators and nothing more: a separator inside quotes or a `$(...)`
// substitution ends the slice anyway, and a newline-separated command list
// is already one line per invocation by the time it gets here.
for (const match of line.matchAll(DOCKER_BUILD)) {
const invocation = line
.slice(match.index)
.split(/&&|\|\||;|\|/)[0] as string;
const file = /\s(?:--file[=\s]+|-f=?\s*)(\S+)/.exec(invocation);
if (/\bdocker\s+build\b/.test(line)) {
const file = /\s-f\s+(\S+)/.exec(line);
edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`);
}
@@ -557,11 +217,6 @@ const walk = (node: string, path: string[] = [], into?: Walk): Walk => {
}
result.reached.add(node);
// Before the recipe, exactly as make does.
for (const edge of prerequisitesOf(node)) {
walk(edge, [...path, node], result);
}
for (const line of commandsOf(node)) {
result.prettier += countPrettier(line);
for (const edge of edgesOf(line)) {
@@ -678,13 +333,9 @@ const installBranches = (): { withoutYarn: string[]; withYarn: string[] } => {
}
const close = lines.indexOf("}", open);
const body = lines.slice(open + 1, close === -1 ? undefined : close);
// The guard has to be a plain positive `missing yarn` test at the front of
// the condition. `if ! missing yarn ...` is the same shape to a looser
// regex but swaps which branch is which, and since the two branches are
// asserted separately below, that would silently relabel them — a failing
// test would then name the wrong branch. Anything else throws the named
// error below, which is the loud failure this file prefers.
const guard = body.findIndex((line) => /^if\s+missing yarn\b/.test(line));
const guard = body.findIndex((line) =>
/^if\b.*\bmissing yarn\b/.test(line),
);
const otherwise = body.indexOf("else", guard);
const end = body.indexOf("fi", otherwise);
if (guard === -1 || otherwise === -1 || end === -1) {
@@ -837,244 +488,6 @@ describe("the resolver reads what the shell would run", () => {
).toEqual(["script/lint", "script/test", "script/fmt"]);
});
// The two edges the previous version of this file could not see, each
// pinned directly as well as through the graph. Both were reproduced as
// Makefile mutations that gave `make check` two prettier passes with the
// whole suite green.
it("follows a Makefile prerequisite as an invocation", () => {
// Not hypothetical: this is the shipped Makefile's own `install`.
expect(prerequisitesOf("make:install")).toEqual(["make:build-bin"]);
expect(prerequisitesOf("make:check")).toEqual([]);
});
it("treats $(MAKE) and ${MAKE} as make", () => {
expect(edgesOf("$(MAKE) fmt-check")).toContain("make:fmt-check");
expect(edgesOf("${MAKE} fmt-check")).toContain("make:fmt-check");
expect(edgesOf("make fmt-check")).toContain("make:fmt-check");
expect(edgesOf("make -n fmt-check")).toContain("make:fmt-check");
});
// Two more constructs that gave `make check` a second prettier pass with
// the whole suite green, each reproduced as a Makefile mutation before
// being pinned here.
it("keeps recipe lines inside a make conditional", () => {
const recipes = parseMakefile(
[
"check:",
"\t@script/check",
"ifeq (1,1)",
"\t@script/fmt-check",
"endif",
].join("\n"),
);
// What `make -n check` prints, in order.
expect(recipes.get("check")?.recipe).toEqual([
"script/check",
"script/fmt-check",
]);
});
it("reads a recipe written on the target line after a semicolon", () => {
const recipes = parseMakefile(
["check: ; @script/fmt-check", "\t@script/check"].join("\n"),
);
expect(recipes.get("check")?.recipe).toEqual([
"script/fmt-check",
"script/check",
]);
// And the `;` and the command are not mistaken for prerequisites.
expect(recipes.get("check")?.prerequisites).toEqual([]);
expect(
parseMakefile("check: lint ; @script/fmt-check").get("check"),
).toEqual({
prerequisites: ["lint"],
recipe: ["script/fmt-check"],
});
});
// The fifth construct that gave `make check` two prettier passes with the
// whole suite green: `FMT := script/fmt-check` and a recipe of `@$(FMT)`.
// The shipped Makefile writes recipe lines this way already
// (`@$(YARN) tsc --watch`), so an unexpanded variable was a gap in the
// house style rather than in an exotic corner of make.
it("expands a variable spelled into a recipe line", () => {
const recipes = parseMakefile(
[
"FMT := script/fmt-check",
"check:",
"\t@$(FMT)",
"\t@${FMT}",
"\t@script/check",
].join("\n"),
);
// What `make -n check` prints, in order.
expect(recipes.get("check")?.recipe).toEqual([
"script/fmt-check",
"script/fmt-check",
"script/check",
]);
});
it("expands a variable spelled into a prerequisite list", () => {
expect(
parseMakefile("FMT = fmt-check\ncheck: $(FMT)").get("check")
?.prerequisites,
).toEqual(["fmt-check"]);
});
// A recipe may name a variable the Makefile assigns further down.
it("expands a variable assigned after the recipe that uses it", () => {
const recipes = parseMakefile(
["check:", "\t@$(FMT)", "FMT ?= script/fmt-check"].join("\n"),
);
expect(recipes.get("check")?.recipe).toEqual(["script/fmt-check"]);
});
// `export FMT := script/fmt-check` is an assignment make honours and the
// pattern anchored at the name, so the whole line read as neither an
// assignment nor a target and `@$(FMT)` stood unresolved: `make -n check`
// printed `script/check` and `script/fmt-check` while this file scored
// one. `override` reaches the same place by the same route.
it("expands a variable assigned through an export or override prefix", () => {
expect(
parseMakefile(
[
"export FMT := script/fmt-check",
"check:",
"\t@$(FMT)",
"\t@script/check",
].join("\n"),
).get("check")?.recipe,
).toEqual(["script/fmt-check", "script/check"]);
expect(
parseMakefile(
["override FMT = script/fmt-check", "check:", "\t@$(FMT)"].join(
"\n",
),
).get("check")?.recipe,
).toEqual(["script/fmt-check"]);
// `export` on its own line names a variable without assigning one, and
// is not read as an assignment of the empty string.
expect(
parseMakefile(
[
"FMT := script/fmt-check",
"export FMT",
"check:",
"\t@$(FMT)",
].join("\n"),
).get("check")?.recipe,
).toEqual(["script/fmt-check"]);
});
// `?=` assigns only when the name has no value yet, so among assignments
// to one name the first `?=` wins where `:=` and `=` let the last one win.
// Reading every operator as last-wins resolved `@$(FMT)` to the value make
// discards: the file counted an invocation that never happens and missed
// the one that does, with the suite green either way.
it("keeps the first value when a later assignment is conditional", () => {
expect(
parseMakefile(
[
"FMT := script/fmt-check",
"FMT ?= script/build",
"check:",
"\t@$(FMT)",
].join("\n"),
).get("check")?.recipe,
).toEqual(["script/fmt-check"]);
// A `?=` that is itself the first assignment does assign, and a `?=`
// after it does not.
expect(
parseMakefile(
[
"FMT ?= script/fmt-check",
"FMT ?= script/build",
"check:",
"\t@$(FMT)",
].join("\n"),
).get("check")?.recipe,
).toEqual(["script/fmt-check"]);
// `:=` and `=` stay last-wins, which is what make does.
expect(
parseMakefile(
[
"FMT := script/build",
"FMT := script/fmt-check",
"check:",
"\t@$(FMT)",
].join("\n"),
).get("check")?.recipe,
).toEqual(["script/fmt-check"]);
});
// The edges of the expansion, asserted so the header's not-followed list
// is the code's behaviour rather than a claim about it. Each of these
// leaves the reference standing verbatim, which reaches nothing — the same
// dead end as any other unfollowed edge, and not a wrong resolution.
it("leaves a value it cannot resolve to a literal unexpanded", () => {
// A make function: resolving it would mean running it.
expect(
parseMakefile(
[
"FMT := $(shell echo script/fmt-check)",
"check:",
"\t@$(FMT)",
].join("\n"),
).get("check")?.recipe,
).toEqual(["$(FMT)"]);
// A `define` body is a multi-line value, not a single-line assignment.
expect(
parseMakefile(
[
"define RUNFMT",
"@script/fmt-check",
"endef",
"check:",
"\t$(RUNFMT)",
].join("\n"),
).get("check")?.recipe,
).toEqual(["$(RUNFMT)"]);
// An undefined name is not silently emptied.
expect(
parseMakefile(["check:", "\t@$(NOPE)"].join("\n")).get("check")
?.recipe,
).toEqual(["$(NOPE)"]);
});
it("reads a package.json script name that contains a colon", () => {
// The lookup against `scripts` is what turns this into an edge, and no
// script in this repo is colon-named, so the charset is asserted on
// the extraction itself.
expect(
[
..."yarn run lint:fmt".matchAll(
/\byarn(?:\s+run)?\s+([A-Za-z][A-Za-z0-9_:-]*)/g,
),
].map((match) => match[1]),
).toEqual(["lint:fmt"]);
});
it("reads the body of a BuildKit heredoc RUN step", () => {
expect(
dockerRunCommands(
[
"FROM scratch",
"RUN <<EOF",
"yarn run prettier --check .",
"EOF",
"RUN echo done",
].join("\n"),
),
).toContain("yarn run prettier --check .");
});
it("refuses a heredoc that is never terminated", () => {
expect(() =>
dockerRunCommands("FROM scratch\nRUN <<EOF\nyarn run prettier .\n"),
).toThrow(/unterminated heredoc/);
});
it("follows a bare docker build to Dockerfile and -f to its file", () => {
expect(edgesOf("docker build .")).toContain("docker:Dockerfile");
expect(edgesOf("docker build -f Dockerfile.lint .")).toContain(
@@ -1082,119 +495,6 @@ describe("the resolver reads what the shell would run", () => {
);
});
// `-f` is not the only spelling of the flag. `--file=Dockerfile.lint` and
// `--file Dockerfile.lint` mean exactly what `-f Dockerfile.lint` means,
// and matching only `-f` sent both to the default `Dockerfile` edge
// instead. That is the false-green shape this whole file exists to
// prevent: a second prettier pass added as
// `docker build --file=Dockerfile.lint .` was followed into the wrong
// Dockerfile, counted nothing, and left every build green. The long form
// is an ordinary thing for a human to write, so it is followed rather than
// merely documented as unfollowed.
//
// The equality rather than containment matters: resolving to the right
// file is only half of it, the default edge must not also be emitted.
it("follows --file in all three spellings as it does -f", () => {
expect(edgesOf("docker build --file=Dockerfile.lint .")).toEqual([
"docker:Dockerfile.lint",
]);
expect(edgesOf("docker build --file Dockerfile.lint .")).toEqual([
"docker:Dockerfile.lint",
]);
expect(edgesOf("docker build -f=Dockerfile.lint .")).toEqual([
"docker:Dockerfile.lint",
]);
});
// The value may be attached to the short flag with no separator at all:
// `-fDockerfile.lint` is a single token, and the flag parser reads it as
// `-f` naming that file, exactly as `-f Dockerfile.lint` does. Requiring a
// separator sent it to the default `Dockerfile` edge instead — the same
// false green as the long spellings above, and one the header promised was
// followed: a second prettier pass added as `docker build
// -fDockerfile.lint .` was resolved into the wrong file, counted nothing,
// and left the build green. `-fFILE` is an ordinary thing for a human to
// write, so it is followed rather than merely documented as unfollowed.
it("follows a short file flag with its value attached", () => {
expect(edgesOf("docker build -fDockerfile.lint .")).toEqual([
"docker:Dockerfile.lint",
]);
});
// The flag is found anywhere in the invocation rather than at a fixed
// position, matching how `-f` was already read, since a real build line
// carries `--build-arg` and friends around it.
it("finds the file flag after other flags and build args", () => {
expect(
edgesOf(
'docker build --build-arg LINT_EPOCH="1" --file=Dockerfile.lint --progress=plain .',
),
).toEqual(["docker:Dockerfile.lint"]);
});
// A different flag that merely begins with the same letters is not the
// file flag: `--force-rm` must not read as one, or its next token would be
// resolved as a Dockerfile and thrown on as unreadable.
it("does not mistake --force-rm for the file flag", () => {
expect(edgesOf("docker build --force-rm .")).toEqual([
"docker:Dockerfile",
]);
});
// The header says a bundled short flag cluster resolves to the default
// rather than to the file it names, which is a limitation and so has to be
// pinned: an unpinned limitation is how the header starts overclaiming
// again. If someone teaches the resolver to split clusters, this test is
// the one that tells them to correct the header too.
it("resolves a bundled -qf cluster to the default Dockerfile", () => {
expect(edgesOf("docker build -qf Dockerfile.lint .")).toEqual([
"docker:Dockerfile",
]);
});
// The false green this file exists to prevent, in the one shape it still
// had: `script/lint` chaining a second `docker build` after the lint image
// built prettier twice and counted once, because a single `test` and a
// single `exec` over the line produced exactly one edge and resolved it to
// the first file named. That is the same failure as the original duplicate
// pass — two invocations, one verdict — reached through the walker instead
// of through the repo.
it("follows both docker builds when one line chains two", () => {
expect(
edgesOf(
'docker build --build-arg LINT_EPOCH="$(date +%s)" -f Dockerfile.lint . && docker build -f Dockerfile.extra .',
),
).toEqual(["docker:Dockerfile.lint", "docker:Dockerfile.extra"]);
});
// The other half: a flag search over the whole line reads a *later*
// command's `-f` as the build's own. A bare `docker build .` followed by
// `cp -f` resolved to `Dockerfile.lint`, which both counts an invocation
// that never happens and drops the `Dockerfile` edge — the half of the CI
// graph the second entrypoint exists to cover. Each separator that bounds
// an invocation is pinned, since the bounding is a split rather than a
// shell parse and dropping one of them would be silent.
it("does not read a later command's -f as the build's", () => {
expect(
edgesOf("docker build . && cp -f Dockerfile.lint /tmp/x"),
).toEqual(["docker:Dockerfile"]);
expect(
edgesOf("docker build . || cp -f Dockerfile.lint /tmp/x"),
).toEqual(["docker:Dockerfile"]);
expect(
edgesOf("docker build . ; cp -f Dockerfile.lint /tmp/x"),
).toEqual(["docker:Dockerfile"]);
expect(edgesOf("docker build . | tee -f Dockerfile.lint")).toEqual([
"docker:Dockerfile",
]);
});
// This exact-equality assertion is also the block-scalar guard, which is
// not obvious from its name: a `run: |` step resolves to the bare `|`,
// which reaches nothing, so the prettier count would stay 1 no matter what
// the block contains. Pinning the resolved list is what makes such a step
// red. The cost is that any legitimate second `run:` step — a cache step,
// an `echo` — fails this test for a reason unrelated to prettier.
it("reads the run steps of the CI workflow and not its uses steps", () => {
expect(commandsOf("workflow:.gitea/workflows/check.yml")).toEqual([
"script/cibuild",