fdb566a209db4bd49973085d843a1a7b3cc4d793
26 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
clawbot
|
fdb566a209 |
Reflow the merged .golangci.yml bullet with prettier
All checks were successful
check / check (push) Successful in 11s
|
||
|
clawbot
|
d709d88e79 |
Merge main into next
Some checks failed
check / check (push) Has been cancelled
Reconciles next with three merges it predated: the 60s/90s test budget, thin cmd/ entrypoints, and the .golangci.yml prohibition scoped to vendored copies. Conflicts: - prompts/REPO_POLICIES.md frontmatter: took main's later last_modified. - prompts/NEW_REPO_CHECKLIST.md: main's test-budget line, next's containerised-lint line. The two edits touch adjacent list items only. - prompts/REPO_POLICIES.md .golangci.yml bullet: both sides rewrote it. Took main's vendored-copy scoping and the change-by-re-vendoring sentence, and kept next's digest pin in Dockerfile.lint, which supersedes the host go install that main's text still describes. The test-budget text is main's verbatim on both sides of every conflict. |
||
| f77d785bed |
Scope the .golangci.yml agent prohibition to vendored copies (#49)
All checks were successful
check / check (push) Successful in 9s
SPECULATIVE and ahead of your ruling — nothing here is urgent and closing it costs nothing. One sentence of policy prose changed; no config file is touched. ## The contradiction `REPO_POLICIES.md` line 266 currently reads: > `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only manually by the user. Stated unqualified, that forbids an agent from modifying `.golangci.yml` **anywhere** — including the canonical copy in this repo, which is the only place it can ever be fixed. An agent that wants to remediate a linter problem must either violate the rule or leave the problem standing. A rule that cannot be complied with and satisfied at the same time gets resolved ad hoc, differently by each reader, which is the worst of both outcomes it was trying to produce. This is not hypothetical. It has already cost real time: - The `gomodguard` deprecation (#25) has been open since 2026-08-07 and still prints on every lint run in every consuming Go repo. - One agent read the rule as binding here and **declined to open even a speculative branch**, so the fix was not written at all on that pass. - #47 exists only because a later request was explicit enough to override the reading, and its lead comment asks for exactly this ruling before the PR itself can be judged on its merits. - The same warning is refiled downstream as sneak/homoicon#4, where it is correctly marked owner-only and correctly punted upstream. Each new agent that meets the rule reruns this whole argument. ## The change Scope the prohibition to the vendored copy, and name the one legitimate path by which the config can change: ``` - `.golangci.yml` is standardized. The vendored copy in a consuming repo must _NEVER_ be modified by an agent: fetch it from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml` and keep it byte-identical, so that no repo can quietly loosen its own linting. Linter configuration changes are made to the canonical copy in the `prompts` repo and reach consuming repos by re-vendoring; an agent may open a PR against canonical, which only the user merges. ``` The version pin sentence that followed is unchanged. This keeps the property the rule exists for — no repo silently weakens its own linting, and divergence from canonical stays detectable — while removing the reading that freezes canonical itself. Your control is not reduced: an agent may open a PR here, and only you merge it. ## Scope of the wording sweep I grepped every `.md` in the repo for the absolute phrasing. It appears in **exactly one place**, `prompts/REPO_POLICIES.md` lines 266-267. `EXISTING_REPO_CHECKLIST.md` (line 39) and `NEW_REPO_CHECKLIST.md` (line 63) both mention `.golangci.yml`, but only as "fetch from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`" — an instruction to vendor canonical verbatim, which is exactly what the scoped rule says. Neither carries a prohibition, so neither needs changing and neither is left contradicting the other. `REPO_POLICIES.md` line 414 lists `.golangci.yml` as a required file, also unaffected. Note that the repo-root `REPO_POLICIES.md` is a symlink to `prompts/REPO_POLICIES.md`, so the single edit covers both paths. ## Deliberately NOT included This PR does **not** change `.golangci.yml`. The `gomodguard` fix stays in #47 so the two can be judged separately — the policy question is worth settling on its own terms regardless of what you decide about that config change, and merging them would collapse two decisions into one. ## Unrelated observation, for the record While verifying #47 against a scratch `sneak/homoicon` clone, I found that homoicon's vendored `.golangci.yml` is sha256 `391ea68e637432980f1db0776076f51578fa58193bbd17f4b40ad725975e21f8`, while canonical `main` is `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. The whole difference is a three-line comment recording a one-time agent edit you authorized on 2026-08-07; the config is functionally identical. That matters only if you ever want a hash-based drift guard against canonical, which #25 floats as an offline alternative to `golangci-lint config verify`: such a guard would already report homoicon as drifted on day one. Worth knowing before building one. No change proposed here. ## Validation `make check` passes (`prettier --check '**/*.md' --tab-width 4 --prose-wrap always`: all matched files clean). `make fmt` produced no further changes. `last_modified` in the front matter updated to 2026-08-19 per this file's own rule. Co-authored-by: sneak <sneak@sneak.berlin> Co-authored-by: Jeffrey Paul <sneak@noreply.example.org> Reviewed-on: #49 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| 3f8201d532 |
Require thin cmd/ entrypoints: all logic in internal/ or pkg/ (#54)
All checks were successful
check / check (push) Successful in 13s
Codifies your ruling (2026-08-30, filed as homoicon issue 555): no project logic outside `internal/` or `pkg/`; each `cmd/<name>/` is a single `main.go` whose body is one call into library code. - `CODE_STYLEGUIDE_GO.md`: strengthens the "keep `main` small" rule to the single-call form and adds the no-logic-outside-`internal/`-or-`pkg/` rule. - `REPO_POLICIES.md`: annotates `cmd/` in the canonical subdirectory list accordingly. (Root copy is a symlink; one edit covers both.) Co-authored-by: sneak <sneak@sneak.berlin> Reviewed-on: #54 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| a8686891ba |
Raise org-wide make test cap to 60s, backstop timeout to 90s (#42)
All checks were successful
check / check (push) Successful in 4s
## The question this answers Is the 60-second test-time cap the new **org-wide** ceiling, or an approved **dnswatcher-only** divergence? - Question: #41 - Origin: sneak/dnswatcher#93 This PR implements **org-wide**. ## The ruling On sneak/dnswatcher#93 (comment) (2026-08-09), verbatim: > make the cap 60s in both and never use mocking, always use live resolvers and > assume the build and run environments have full unmodified unrestricted > internet access. it is ok if they fail due to a bad build environment that > alters dns packets. And on #41 (comment), disambiguating the scope: > org wide. the hard cap is 60 for ci/green, but over 20s should be filed as an > improvement bug. That second comment landed after this work was started, and it confirms the option implemented here. The two-tier shape it describes (60s hard, 20s target, overage filed as a bug) is encoded in the policy text. ## What changed, and the number chosen The backstop moves from `30s` to **`90s`**. The old pairing was incoherent under the new cap: a 60-second ceiling with a 30-second `-timeout` means the timeout kills the suite long before the ceiling is reached, so the ceiling would never be the thing that fails. The backstop has to sit above the cap, where it does its actual job of catching a hung test rather than a merely slow one. `90s` preserves the 1.5x backstop-to-cap ratio the old `20s`/`30s` pair already had, so the relationship between the two numbers is unchanged and only the scale moves. Every place a number changed: | File | What | | --- | --- | | `prompts/REPO_POLICIES.md` | Prose ceiling: `20 seconds` to `60 seconds`, plus the new 20s target / improvement-bug tier | | `prompts/REPO_POLICIES.md` | Backstop prose: `30-second timeout` to `90-second timeout`, with the rationale for why it exceeds the cap | | `prompts/REPO_POLICIES.md` | Go example Makefile snippet, first run: `go test -timeout 30s` to `-timeout 90s` | | `prompts/REPO_POLICIES.md` | Go example Makefile snippet, verbose rerun: `go test -timeout 30s` to `-timeout 90s` | | `prompts/EXISTING_REPO_CHECKLIST.md` | `make test` has a `30-second` timeout, to `90-second` timeout plus the 60s hard cap and the 20s filing rule | | `prompts/NEW_REPO_CHECKLIST.md` | `script/test` / `make test` entrypoint line: `30-second timeout` to `90-second timeout, 60-second hard cap on wall time` | I swept the whole repo for `20 second`, `30-second`, `20s`, `30s`, `timeout 20`, `timeout 30`, and `under 20`/`under 30`. After this change the only remaining occurrences of `20` in a test-timing context are the two intentional references to the new 20-second target. The other `timeout` hits in the repo are unrelated (`.golangci.yml` lint timeout, HTTP server `ReadTimeout`/`WriteTimeout` examples, `middleware.Timeout`) and were left alone. One deliberate non-change: the Python example Makefile snippet in `prompts/REPO_POLICIES.md` carries no timeout flag today and still carries none. `pytest` has no built-in timeout, so adding one would mean mandating the `pytest-timeout` plugin org-wide, which is a new dependency requirement rather than a renumbering, and outside what was ruled on. It is a pre-existing gap between the prose and that snippet, not one this PR introduces. Happy to file it separately or add `--timeout=90` here if you want it in scope. ## The alternative that was not implemented **Keep canonical at 20s and let `sneak/dnswatcher` carry a documented per-repo divergence.** That was the recommendation originally written up in #41, on the reasoning that the 20s ceiling is doing real work in repos with fast deterministic suites and only dnswatcher needs the headroom. Org-wide was chosen instead for two reasons. First, the pressure is not specific to DNS: any repo whose tests exercise real infrastructure over the network inherits the same variance, and there is no principled line that admits dnswatcher and excludes the next such repo. Second, and more decisively, `REPO_POLICIES.md` is a vendored file. A sanctioned per-repo divergence in a vendored file is indistinguishable, on inspection, from a stale vendored copy: the next re-vendoring silently reverts the divergence, and nobody reading a consuming repo can tell whether the number they are looking at is an intentional exception or drift. That is the bidirectional-drift problem already tracked in #31. The two-tier cap gets the same outcome without the drift, since a fast repo that regresses from 4s to 45s still generates an improvement bug. ## Status This PR was opened speculatively, ahead of a decision, on the standing "open it rather than wait" instruction. The scope question has since been answered org-wide in the issue, but the specific backstop value of `90s` and the two-tier wording are still my proposals rather than anything ruled on, so closing this or sending it back for a different number is a perfectly fine outcome. `make check` passes; `make fmt` was run and the result is included (it was a no-op, the edits were already prettier-conformant). `sneak/dnswatcher` is landing the matching 60s edit to its vendored copy in parallel, and will match whichever way this is decided. Co-authored-by: clawbot <clawbot@eeqj.de> Reviewed-on: #42 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| cc6a5a00e7 |
Run every lint in a container via Dockerfile.lint (closes #40)
All checks were successful
check / check (push) Successful in 9s
script/lint runs the linter directly when it is already inside a container and otherwise builds Dockerfile.lint, so the linter never runs on a developer host. That closes three host-only mechanisms: the result cache golangci-lint keys on file content rather than location, which produced a confirmed false green and findings reported against other checkouts; the host-global $TMPDIR/golangci-lint.lock, which fails a run in a way no caller can distinguish from findings; and host/container version skew, which hid thirteen findings on one repo. Detection is on LINT_IN_CONTAINER=1, set by every Dockerfile, and on nothing else. The two directions are not symmetric: a false negative inside a container attempts a nested docker build, finds no daemon and fails loudly, while a false positive on a host silently lints there, which is the defect this issue exists to kill. /.dockerenv is therefore rejected even as a fallback -- measured absent inside BuildKit RUN steps and present on any host that is itself a container, so it fails in both directions and one of them is the dangerous one. Nothing else changes shape. The Dockerfile still runs make check, script/check still runs test, lint and fmt-check, script/cibuild is still a single docker build with CHECK_EPOCH and VERSION, and the Go multistage lint stage and its COPY --from=lint ordering dependency survive with ENV LINT_IN_CONTAINER=1 added. Dockerfile.lint is the standalone developer-host path and carries the same CHECK_EPOCH guard, with the ARG below the dependency layer so only the lint re-runs. The script/bootstrap golangci-lint install and the per-checkout GOLANGCI_LINT_CACHE/TMPDIR wrapper are deleted as superseded. Neither has a caller left. A JS repo's yarn install stays: the rule is that no lint verdict may come from a host invocation, not that no linter binary may exist there, and in a repo whose formatter is its linter the formatter necessarily runs on the host. golangci-lint config verify is kept, on measurement. Under the pinned v2.12.2 a bogus top-level key and a bogus key under linters.settings.lll both pass `golangci-lint run` with exit 0 and `0 issues` while config verify exits 3 and names them; an unknown linter name fails run and passes config verify. It needs no network: every case reproduced byte-identically under `docker run --network none`, in a container where `getent hosts golangci-lint.run` exits 2. Comment blocks were cut hard across every file this unit touches. .dockerignore drops from 67 comment lines to 28, script/cibuild from 17 to 12, script/docker from 18 to 12, and prompts/REPO_POLICIES.md from 1182 lines to 907. What remains says why a line is load-bearing; the discovery narratives are gone. config verify lives in script/lint's native branch rather than in a Dockerfile, so every path that lints inherits it: the lint stage of the main image, which is what CI runs, as well as Dockerfile.lint. Putting it in one Dockerfile is how the other path silently loses it. |
|||
|
clawbot
|
0620416869 |
Give golangci-lint per-checkout cache and lock state (closes #30)
All checks were successful
check / check (push) Successful in 7s
A golangci-lint result on a host running many concurrent workers does not reliably belong to the tree that asked for it. Two independent mechanisms, which have repeatedly been mistaken for one: The result cache is keyed on file content, not location, so two checkouts of the same commit hold byte-identical files, share cache entries, and one tree's findings are served for the other under the other tree's path. This produced a confirmed false green as well as the loud false reds. Moving workers from shared worktrees to their own clones does not address it — two clones collide exactly as two worktrees did — and removes only the foreign-path artefact that made the defect noticeable. The concurrency lock is $TMPDIR/golangci-lint.lock (pkg/commands/run.go, acquireFileLock), host-global and independent of GOLANGCI_LINT_CACHE, with a five-second acquire timeout, so it fails when the host is busiest. A private cache directory does not isolate it. Setting only the cache closes the contamination half and leaves runs failing red on a condition that is not a result at all. REPO_POLICIES.md now carries the canonical Go script/lint: both variables scoped into a .lint-cache/ directory inside the checkout, above any container-versus-host branch so every path reaching the linter gets them; --allow-serial-runners, which keeps the mutual-exclusion guard and queues rather than aborting, for the same-checkout overlap TMPDIR scoping cannot cover, with --allow-parallel-runners rejected because it deletes the guard; and a bounded retry that treats the lock error as VOID rather than as findings, exiting 75 on exhaustion so it is neither a pass nor a failure. Detection is on the stderr stream and never on exit status: findings go to stdout, so a finding quoting the lock message in source cannot be retried away, and the exit status is not a stable discriminator anyway. The interim void rule is recorded with the ../ clause that the original filter missed, and with its limit stated — it catches contamination that names foreign files, not contamination that suppresses findings. Both checklists gained the corresponding items, since a half-fix that sets only the cache reads as complete. GOCACHE was measured rather than assumed and does not need isolating: with the two variables scoped per checkout and GOCACHE shared at the host default, each checkout reported its own paths. Verified with the snippet extracted from the committed document and executed as a consuming repo would adopt it, each control paired against the pre-fix form: contamination reproduced on the pre-fix script and absent on the adopted one; a stub linter colliding twice then clearing, with the retry engaging and succeeding; exhaustion exiting 75 with a VOID message; a genuine finding whose text quotes the lock message reported as findings with no retry; and a real held lock failing the pre-fix script with exit 3 while the adopted script, inheriting the same environment, completed in one second. |
||
| 3a218497b8 |
Keep in-repo agent scratch out of the build context and out of git (closes #27)
All checks were successful
check / check (push) Successful in 17s
The canonical .dockerignore and .gitignore both omitted the in-repo agent scratch directory. On this fleet that directory holds one worktree per in-flight agent -- an entire additional checkout of the repo each -- so under `COPY . .` all of it reached the build context and the image. Measured on this repo before the change: five planted scratch files, at every depth beneath the directory, all present inside a probe image built from the real context. Three consequences, only the first of which is about size. The context inflates by a multiple of the repo. Another session's unreviewed and sometimes uncommitted work is copied into a build artifact. And the directory is created and destroyed constantly by tooling, so it invalidates `COPY . .` for reasons that have nothing to do with this repo's content -- which is the accidental cache protection described at length in the issue thread, and the reason this change was sequenced behind the CHECK_EPOCH bust rather than landed alongside the rest of the .dockerignore work. The two entries are deliberately different shapes, because the two files have different semantics and neither is derived from the other. In .dockerignore the entry is anchored, `.claude`, with no `**/` prefix: the directory occurs exactly once, at the context root, and the prefixed form additionally matches any nested directory of that name. Measured rather than argued -- the `**/`-prefixed control was built and enumerated too, and it removes prompts/.claude/ from the context as well, which in a repo with a legitimately named nested directory would silently delete it from the build. In .gitignore the entry is unanchored, `.claude/`, because a .gitignore pattern already matches at every depth; `git check-ignore -v` confirms it covering both .claude/ and prompts/.claude/, so a `**/` prefix there would be redundant at best, and on an anchored pattern it would be actively wrong. Anchoring buys that at the cost of a residual exposure, and the vendored files now say so rather than only asserting the reason to anchor. "Occurs exactly once, at the context root" is a property of how agents are run, not of the tooling: the directory is created in the agent's working directory, so a monorepo running a per-service agent in services/api/ still ships services/api/.claude/ into the context and the image -- the exact exposure this change exists to close, left open in the repo shape where it is likeliest. The .dockerignore header block, the REPO_POLICIES.md bullet and both checklists state the gap and the remedy (anchored entries for the subdirectories that have one, or `**/.claude` once no legitimately named nested directory would be caught). Consuming repos receive the files and not the tracker, so a caveat that lives only in a PR body is not a caveat. It is not case-folded the way the neighbouring secret patterns are: tooling creates the directory in exactly one spelling, so a folded pattern would add no coverage. The earlier justification -- that a miss costs bloat rather than exposure -- is gone, because it contradicted this issue's own framing, in which the cost of a miss is unreviewed work in an image layer. The second half of this change is the consequence that ships broken silently. Excluding .git means `git describe` cannot run in any build stage, and it fails quietly there rather than erroring: `-X main.Version=` comes out empty, the binary reports no version, and the build still exits 0. The Go template in REPO_POLICIES.md had `ARG VERSION=dev` and never said where VERSION came from, which is precisely the gap a reader fills in with `git describe` inside the build. It now says: computed on the host, threaded in with `--build-arg VERSION=...`, shown as a complete command rather than as two rules each documenting half of one. script/docker and script/cibuild do it, with the same discipline the epoch already has -- assignment on its own line, because a failing command substitution inside an argument does not trip `set -e`, plus a non-empty fallback so a build from an export with no .git reports `unknown` rather than an empty string that reads as a successful version. That fallback is applied in exactly one place, and that place can actually execute. `git describe ... || true` leaves the value empty when it fails, and the `[ -n "$version" ]` line is what substitutes `unknown`. Folding the fallback into the substitution as `|| echo unknown` would have left the guard unreachable -- harmless in itself, but a guard that cannot fire is indistinguishable from one that works to every repo that copies it, and canonical text should not carry a check that is decorative. Measured firing in both sh and dash: not a git repository -> unknown, repository with no commits yet -> unknown, this repository -> the describe output. The checklist now carries the guard line as well, so the vendored guidance and the vendored script no longer disagree. The scripts pass VERSION unconditionally rather than growing a per-repo variant. This repo's Dockerfile declares no `ARG VERSION`, and BuildKit was measured accepting the unconsumed arg silently -- no warning, no cache effect, confirmed by the paired runs below in which the bootstrap layer still caches. The alternative, leaving it to each repo, reintroduces the trap: a repo that needs a version and finds no VERSION in its scripts writes `git describe` into the Dockerfile, which is the failure being closed. The same correction reaches the two Go documents that carry the GOLDFLAGS pattern, since a `$(shell git describe)` evaluated inside a build stage is exactly this empty version. Both are now `?=`, and their comments say precisely when that matters: where a build stage compiles by invoking make, `ARG VERSION` puts the value in the environment and `?=` defers to it, whereas the canonical Go template compiles with `go build` directly and uses the Makefile on the host only -- still `?=`, so that a repo which later moves its build behind make does not silently start shipping an empty version. Leaving them as `:=` would have left the corpus telling a reader one thing in the policy and the opposite in the styleguide. Both repo checklists gain the entries too. They are what an agent reads while writing these files, so they are where the wrong shape actually gets written: the .gitignore item is the one an existing repo never re-fetches, and it now names `.claude/` explicitly along with the warning not to prefix it. Verification, by enumerating a probe image rather than by reading the patterns. Standalone minimal Dockerfile held outside the context, `--no-cache` scoped to that one image, no prune of any kind. Before: all five planted scratch files in the image, 42 files total. After: zero, 37 files total, with README.md, script/check, prompts/NEW_REPO_CHECKLIST.md and a planted probe_src/app.md all still present as positive controls, so the exclusion is a real exclusion and not a COPY that stopped copying. Transferred context fell from 161.86kB to 68.82kB, recorded as corroboration only: BuildKit reports a delta, not a total, and an earlier run in this repo transferred 2.18kB while shipping 43 files. The CHECK_EPOCH verification was re-run under the changed context, because the context moved underneath the earlier measurement. Two consecutive script/cibuild runs on an unchanged tree: run 1 in 17.07s, run 2 in 5.73s, both executing the check layer with a distinct epoch and real prettier output from both lint and fmt-check. The `RUN script/bootstrap` layer is CACHED in run 2, which is the validity control -- it proves no concurrent prune landed between the runs and that no --no-cache path was taken, so the check layer executing is the bust working rather than a cold cache. Planted files were removed afterwards and their absence confirmed against the filesystem with `find`, not against `git status`, which cannot see them once .gitignore covers the directory -- the same blind spot that made the earlier secret exposure invisible. |
|||
| fd78aeb003 |
Keep secrets out of the Docker build context at every depth (closes #29)
All checks were successful
check / check (push) Successful in 21s
The canonical .dockerignore was three lines -- .git, node_modules, .DS_Store -- while the canonical Dockerfile does `COPY . .`, so a developer's local .env, *.pem or *.key was shipped into the build context and could land in an image layer. Nothing surfaced it because .gitignore covers those patterns, so the files are invisible to every git-based check. The obvious repair, copying .gitignore's secret patterns across, is worse than the gap it closes. .dockerignore does not use .gitignore semantics: Docker matches with moby/patternmatcher, which is filepath.Match semantics plus a `**` extension compiled to a regexp. `*` does not cross `/`, and a pattern without a leading `**/` is anchored at the build-context root. A file listing .env, *.pem and *.key therefore reads as solved, reviews as solved, and protects only the repository root, while config/.env and certs/server.key still ship. The three-line file at least invited scrutiny; the transplanted form manufactures confidence and stops anyone looking. So every depth-independent pattern here carries the `**/` prefix and only genuinely root-anchored entries stay unprefixed. `**/node_modules` fixes a defect the three-line file had today for any nested node_modules, independently of the secret exposure. Coverage is not limited to the three patterns the issue names, because the enumeration found more shapes reaching the image. `**/*.env` covers the prod.env / local.env convention, which the .env and .env.* spellings miss entirely; `.envrc` is a secrets file by direnv convention; *.p12 and *.pfx are bundles carrying private keys; and id_rsa, id_dsa, id_ecdsa and id_ed25519 are the extensionless SSH keys that were covered before only when someone happened to append a .key suffix. Matching is case-sensitive, so `**/*.key` does not match certs/SERVER.KEY, which is reachable on the case-insensitive filesystems most laptops use. Doubling each pattern with an ALL-CAPS twin is not the fix: measured against the planted set it still ships certs/Server.Key and certs/Ca.Pem while reading as though case were handled, which is this issue's failure mode restated in a new place. The matcher supports character ranges, so every secret name is written that way -- `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`, `**/.[eE][nN][vV][rR][cC]`, `**/[iI][dD]_[rR][sS][aA]` and the rest. The extensionless SSH keys and .envrc are folded for the same reason the extensions are: on the very filesystems that make SERVER.KEY reachable, direnv reads .ENVRC and ssh reads ID_RSA, so exempting them would have contradicted the rule that justifies the folding. Since `*` matches the empty string, `**/*.[eE][nN][vV]` already covers a bare .ENV and no separate literal .env entry is needed; the literal one is gone rather than left to imply that case is unhandled there. Deliberately not covered: bare `key` and `pem` filenames, which no tool produces and which collide with legitimate paths (a `**/key` pattern would delete an internal/key/ package directory from the context); `**/id_*`, which would match ordinary source such as id_generator.go and ID_MAP.go; and *.crt and *.cer, which are public certificates rather than secrets and are sometimes a legitimate build input. Each of those is planted as a positive control and verified present in the image after the change. The one predictable false positive is a committed env template: `**/*.env` excludes example.env. The remedy travels with the file rather than living in a review thread -- the header comment and the policy both say to re-include it with a negation, `!docs/example.env`, and never to delete the pattern, which would reopen the exposure for everything else it covers. The OS and editor patterns are included on their own merits rather than by mirroring .gitignore. None of them is ever a build input, and editor state in particular churns under a developer's hands, so each one is a source of `COPY . .` invalidation carrying no information about the source tree. Now that the checks are keyed on CHECK_EPOCH rather than on accidental context churn, there is no reason left to keep churn in the context. Language build artifacts are deliberately absent: they are per-repo, and the file's header comment tells consuming repos to add their own host-built binaries, which is the case that actually bites -- a host `make build` drops a multi-megabyte artifact into the context where .gitignore hides it from every git-based check. That comment gives the anchored form explicitly, `/myapp` rather than `**/myapp`, because the prefixed spelling also matches cmd/myapp/ and deletes the package directory. The header comment is the only part of this guidance a consuming repo actually receives, since it is vendored with the file. .gitignore is untouched. Its semantics are the inverse: an unanchored pattern already matches at any depth, so `**/`-prefixing it produces a file that is wrong in a way that looks careful. That asymmetry is why "derive one from the other" was the wrong instruction, and it is now written down in REPO_POLICIES.md in both directions, together with the case-sensitivity rule, the negation remedy, and the requirement to verify by enumerating the image rather than by reading the patterns. Every consuming repo inherits .dockerignore by copy, so the trap has to live where the next person looks, not only be fixed once here. Both repo checklists gain the same requirements. Verified by planting 130 secret files -- twenty-six name shapes, including every capitalisation of .env, .env.*, prod.env, .envrc, id_rsa, id_ed25519, ca.pem, server.key, bundle.p12 and bundle.pfx, at five depths from the context root to a/b/c -- alongside nine positive controls, then building a standalone probe image doing `COPY . .` and listing what actually landed inside it. The patterns as first written leaked 35 of the 130: every .ENVRC, .Envrc, ID_RSA, Id_Rsa, ID_ED25519, .ENV.PRODUCTION and .Env.Local, at all five depths. A lowercase-only control leaks 80, so the probe is not vacuous. After this change: zero, with all nine controls still present, including internal/ID_MAP.go and certs/CA.CRT. Transferred-context size is recorded but load-bearing on nothing: an earlier run reported 2.18kB transferred while 43 files, five of them secrets, were in the image. BuildKit transfers only the delta from the previous build, so the number describes the transfer and not the contents. Planted files were removed and their absence confirmed against the filesystem rather than against `git status`, which could not have seen them. `make docker` re-run after the change: the check layer executed rather than being served from cache, so the CHECK_EPOCH verification still holds under the altered build context. |
|||
|
clawbot
|
d173e69f85 |
Make the pinned golangci-lint actually reach the host (closes #28)
All checks were successful
check / check (push) Successful in 9s
REPO_POLICIES.md now carries the canonical script/bootstrap snippet for Go repos alongside the .golangci.yml bullet, where the pinned linter version already lives. The guard it replaces, `if missing golangci-lint; then go install ...; fi`, tests PATH presence and never version, so on any already-provisioned machine the pin is inert and a version bump is a no-op. The Dockerfile installs unconditionally into a clean image, so CI and local then disagree about what the linter is: a local `make check` green while `make docker` rejects the same commit, and a container run surfacing findings the host run cannot see. Comparing versions alone is not enough. `go install` writes to GOBIN (or GOPATH/bin) while callers resolve through PATH, so a shadowing binary earlier in PATH lets the install succeed and change nothing a caller ever sees, while bootstrap prints success. The canonical form therefore compares the installed version against the pin, re-resolves through PATH after installing and asserts the pin, and treats any unparseable --version output as a mismatch so the failure direction is a redundant install rather than a skipped one. The comparison is exact over the whole version token. A parser that stops at the first `-` reports 2.12.2 for a host running 2.12.2-rc1, which compares equal to a 2.12.2 pin and skips the install -- the original defect, reachable through the comparison meant to close it, and not caught by requiring the pin to be tagged, since a pre-release is tagged too. When the assert fails the diagnosis is derived from the resolved path rather than asserted: a path outside the install directory is shadowing and the operator is told to remove it or reorder PATH; a path inside it is not, and saying so would send them after a fault that does not exist; no resolution at all means the install directory is simply absent from PATH. A trailing slash on GOBIN is normalised away, since it would otherwise make the inside-the- directory test miss and misreport shadowing. The snippet ends with a call site, and both success paths print a confirmation naming the version. Two definitions with no invocation are a silent no-op with exactly the shape this change exists to close, and a success path that prints nothing is byte-identical to that no-op: same exit status, same empty output. The version helper ends in `|| true` so a --version that exits non-zero cannot kill the script through `set -e` under `set -o pipefail` before the diagnostic is printed, which the styleguide's bash form would otherwise do. The policy text states each of those as a requirement rather than leaving them implicit in the code, and records why the commit-pinned `go install` ref satisfies the hash-pinning rule: a commit hash is not a mutable tag, and the checksum database verifies the fetch, with no repo go.sum consulted, since `go install pkg@version` ignores the go.mod in the current directory or any parent. Whether a go.mod tool dependency should replace that is an open decision and is linked rather than settled here. The two version strings must be kept in sync and must match exactly what --version prints; tagged pins are preferred because the expected string is then derivable from the ref, rather than because the comparison cannot handle a pseudo-version. Verification runs the controls against the block as a consuming repo would adopt it, pasted into a script/bootstrap-shaped file and executed, rather than sourcing it and calling the function directly. The node and yarn handling described earlier in the document is untouched. |
||
|
clawbot
|
51c394552e |
Bust the Docker check-layer cache with a per-invocation CHECK_EPOCH (closes #26)
All checks were successful
check / check (push) Successful in 7s
script/cibuild was a plain `docker build .`, and the Dockerfile does `COPY . .` followed by `RUN make check`. Docker invalidates a COPY layer only when the copied content changes, so on an unchanged tree the check layer was served from cache, the suite never ran, and the build still exited 0. Measured here: run 1 took 18.5s and ran the suite; run 2 on a byte-identical tree took 0.286s with `RUN make check` CACHED. script/cibuild and script/docker now assign a per-invocation nonce on its own line and pass it as --build-arg CHECK_EPOCH. The Dockerfile declares ARG CHECK_EPOCH, guards it with `[ -n "$CHECK_EPOCH" ] || exit 1`, and expands it into the check command. Post-fix, two consecutive runs both execute make check (17.4s / 8.1s) with `RUN script/bootstrap` still CACHED, so dependency layers are untouched and the build ceiling is not at risk. The guard is what makes a bare `docker build .` — the command REPO_POLICIES named verbatim — fail closed rather than reuse the empty and therefore stable cache key; verified failing in 0.455s. Holding the epoch constant restores the false green (run 2 fully CACHED), which pins the varying value as the operative mechanism rather than a coincidence. Because the guard references $CHECK_EPOCH it is itself value-keyed: BuildKit renders the epoch into that layer's description and re-runs the layer when the value changes. Each stage therefore has two independent invalidation points, the guard and the expansion, and the guard always precedes the check RUN. Both are kept and the prose now records this; the expansion remains defence in depth and is what puts the epoch in the build log. The false guarantee was org-canonical text in more than one document, so it is corrected everywhere it appeared rather than only where the issue first found it. REPO_POLICIES.md carried it in two places, and its Go multistage template had check steps in two stages; ARG is stage-scoped, so both stages get the treatment or the fleet inherits the half-fixed shape. CODE_STYLEGUIDE_GO.md restated the guarantee for the bare command this change makes fail closed. NEW_REPO_CHECKLIST.md specified the pre-fix script/cibuild verbatim, so every new repo would have been born with the false green, and EXISTING_REPO_CHECKLIST.md ended on a `docker build` acceptance item that the guard makes unsatisfiable by design — an agent working that checklist would have been led to delete the guard to tick the last box. Both checklists' Dockerfile criteria were also satisfiable by a Dockerfile whose check layers are still frozen, and now require the ARG and guard in every check-running stage. REPO_POLICIES.md's own Dockerfile criterion carried that same incomplete form; it is tightened by cross-reference to the CHECK_EPOCH rule rather than by duplicating the canonical block. The Go template's Key points gain a caveat that the cache-bust turns the `COPY --from=lint` no-op into a content-cache hit, so a repo using a file-dependency trick for stage ordering must re-prove that ordering on a warm cache after adopting it. That was re-proved in another repo in the org which uses the trick with a marker file, where the ordering held; the caveat states explicitly that it was not verified here, this repo being single-stage with no lint stage to order against. |
||
| 0f8efafe68 |
Set canonical .golangci.yml to the org-standard v2 config (golangci-lint v2.12.2) (#24)
Some checks failed
check / check (push) Has been cancelled
Requested by sneak. Sets the canonical `.golangci.yml` to the org-standard v2-schema config already deployed byte-identical across the org's Go repos (vaultik, sfdupes, attrsum, upaas, simplelog, mfer, secret, rgoue, bsfirehose). sha256: `021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb`. Why: settings live under `linters.settings`, so the lll/funlen/cyclop/dupl thresholds actually apply under golangci-lint v2. The old canonical file kept them under top-level `linters-settings`, which v2 ignores. Version note: golangci-lint v2.12.2 tag = commit `c0d3ddc9cf3faa61a4e378e879ece580256d76e5`, recorded for consuming repos. This repo itself has no version pins; the `v2.x.x` / `@sha256:...` strings in `prompts/REPO_POLICIES.md` are intentional placeholders and are unchanged. Known informational note: this config does not disable the deprecated `gomodguard` linter, so golangci-lint 2.12.x prints a deprecation warning. Harmless, accepted. Matches dnswatcher PR #96: sneak/dnswatcher#96 Verification: `make check` green (prettier fmt-check on all markdown) after `make fmt`; `.golangci.yml` sha256 verified as `021cc83f...` matching the org-standard file. Co-authored-by: sneak <sneak@sneak.berlin> Co-authored-by: clawbot <clawbot@eeqj.de> Reviewed-on: #24 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| 3d8d1c0600 |
Adopt scripts-to-rule-them-all: script/ entrypoints, Makefile shims, policy update (#23)
All checks were successful
check / check (push) Successful in 5s
Reviewed-on: #23 Co-authored-by: sneak <sneak@sneak.berlin> Co-committed-by: sneak <sneak@sneak.berlin> |
|||
| 777822e50e |
docs: document conditional -v test rerun pattern in REPO_POLICIES.md (#21)
All checks were successful
check / check (push) Successful in 8s
## Summary Adds the conditional verbose test rerun pattern as a policy recommendation in REPO_POLICIES.md. Per sneak's request from [sneak/chat PR #82](sneak/chat#82): document the pattern where `make test` runs tests without `-v` first, then automatically reruns with `-v` on failure for full diagnostic output. ## Changes **`prompts/REPO_POLICIES.md`** (root `REPO_POLICIES.md` is a symlink to this): - Added new policy bullet after the `make test` timeout rule - Explains the rationale: clean CI/Docker build logs on success, full verbose output on failure - Includes a generic shell pattern template - Includes concrete Go and Python examples - Documents that `exit 1` ensures the target always fails after a rerun (the rerun is solely for diagnostic output) - Updated `last_modified` from 2026-03-12 to 2026-03-18 ## The Pattern ```makefile test: @go test -timeout 30s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ go test -timeout 30s -race -v ./...; exit 1; } ``` - **On success**: concise package summaries only, no per-test noise - **On failure**: automatic verbose rerun shows every test case and assertion - **Always fails**: `exit 1` ensures the build fails regardless of second run's exit code closes #20 Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de> Reviewed-on: #21 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| 1c84344978 |
docs: document fail-fast lint stage pattern for Dockerfiles (#18)
All checks were successful
check / check (push) Successful in 5s
Documents the multistage Docker build pattern we now use across repos (chat, pixa, etc.) where a separate `lint` stage runs `make fmt-check` and `make lint` independently from the build stage. Key additions to REPO_POLICIES.md: - Full Dockerfile template showing the lint → build → runtime stage pattern - Explanation of `COPY --from=lint /src/go.sum /dev/null` as the BuildKit dependency trick - Handling `//go:embed` placeholders in the lint stage - CGO/system library notes for the lint stage - Clarification that tests run in the build stage, not the lint stage Reference implementations: `sneak/chat`, `sneak/pixa`. Co-authored-by: user <user@Mac.lan guest wan> Reviewed-on: #18 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| 41005ecbe5 |
Add HTTP service hardening policy for 1.0 releases (#17)
All checks were successful
check / check (push) Successful in 8s
Closes #16 Adds a comprehensive HTTP/web service security hardening policy to `REPO_POLICIES.md` that must be satisfied before tagging 1.0. The policy covers all items sneak specified (without limitation): **Security headers** — HSTS (min 1 year, includeSubDomains), CSP (restrictive `default-src 'self'` baseline), X-Frame-Options / frame-ancestors, X-Content-Type-Options: nosniff, Referrer-Policy, Permissions-Policy. **Request/response limits** — max request body size on all endpoints, max response size for paginated APIs, ReadTimeout + ReadHeaderTimeout (slowloris defense), WriteTimeout, IdleTimeout, per-handler execution time limits. **Authentication & session security** — rate limiting on password-based auth (API keys exempt as high-entropy), CSRF tokens on state-mutating forms (header-auth APIs exempt), bcrypt/scrypt/argon2 for passwords, session cookies with HttpOnly + Secure + SameSite. **Reverse proxy awareness** — true client IP detection via X-Forwarded-For/X-Real-IP with trusted proxy allowlist (never trust unconditionally). **CORS** — explicit origin allowlist for authenticated endpoints; wildcard only for public unauthenticated read-only APIs. **Error handling** — no leaking stack traces, SQL queries, file paths, or implementation details to clients. **TLS** — HSTS and secure cookie flags required regardless of whether the service terminates TLS directly or sits behind a reverse proxy. The policy is explicitly non-exhaustive (defense-in-depth: "when in doubt, harden"). Also adds corresponding checklist sections to `EXISTING_REPO_CHECKLIST.md` and `NEW_REPO_CHECKLIST.md` so that HTTP hardening is verified during repo setup and 1.0 preparation. Co-authored-by: user <user@Mac.lan guest wan> Co-authored-by: clawbot <clawbot@eeqj.de> Reviewed-on: #17 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
| eb6b11ee23 |
policy: no build artifacts in repos (#15)
All checks were successful
check / check (push) Successful in 5s
Add policy rule: build artifacts and code-derived data must not be committed to repos if they can be generated during the build process. Notable exception: Go protobuf-generated files (`.pb.go`) may be committed because `go get` downloads source but does not execute build steps. This addresses feedback from sneak/chat PR [#61](sneak/chat#61). Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de> Reviewed-on: #15 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org> |
|||
user
|
a2dd953601 |
fmt: format REPO_POLICIES.md per prettier
All checks were successful
check / check (push) Successful in 8s
|
||
clawbot
|
699f97d093 | REPO_POLICIES: expand pre-1.0 schema migration rule (closes #5) | ||
| cb5d630158 |
add note about makefile being authoritative docs
All checks were successful
check / check (push) Successful in 8s
|
|||
| e97b48eea4 |
Fix review issues: front matter, headings, consistency, typos
All checks were successful
check / check (push) Successful in 9s
- Move title and last_modified to YAML front matter (all policy docs) - Make all document sections H1, subsections H2 - Update version rule to reference front matter format - Fix "our" → "your" typo in Go styleguide - Fix Python styleguide numbering (2. → 1.) - Fix README: "flat collection" → accurate description, remove stale TODO - Remove Makefile items from code styleguides (repo stuff, not code), add note linking to Repository Policies - Change zerolog → slog in Go styleguide - Fix JS styleguide npm reference: both work, but use make targets - Drop .json from healthcheck path, add JSON content-type requirement - Add Author/License to Go HTTP Server Conventions - Convert hyperlinks to backtick URLs in checklists for consistency - Add version/front matter to both checklists |
|||
| 3768b8ca02 |
Add rule: all software repos must have tests
All checks were successful
check / check (push) Successful in 7s
Require at least minimal tests (e.g. import/compile check) using the platform-standard test framework. make test must never be a no-op. |
|||
| 03bf0b8445 |
Add authoritative URLs to checklists and copy .golangci.yml
All checks were successful
check / check (push) Successful in 7s
- Add .golangci.yml from upaas as authoritative copy in this repo - Update REPO_POLICIES.md to reference .golangci.yml by URL - Add fetch URLs for all template files in both checklists: .gitignore, .editorconfig, Makefile, .prettierrc, .prettierignore, REPO_POLICIES.md, .golangci.yml, check.yml |
|||
| 00c21cc5c5 |
Fix heading, scope, version placement, and consistency across policy docs
- Rename REPO_POLICIES.md heading from "Development Policies" to "Repository Policies" to distinguish from code styleguides - Move version line above heading per convention - Add scope statement and links to code styleguide documents - Add missing Makefile and LICENSE to minimum files list - Add version lines to all cross-project docs (CODE_STYLEGUIDE*.md, GO_HTTP_SERVER_CONVENTIONS.md) - Clean up CODE_STYLEGUIDE.md heading (was old repo name) - Update EXISTING_REPO_CHECKLIST.md link text to match new heading |
|||
| f43445caea |
Add CI policy, strengthen hash-pinning rule, add Gitea Actions workflow
All checks were successful
check / check (push) Successful in 16s
- All Dockerfiles must run make check as a build step - Every repo needs a Gitea Actions workflow running docker build on push - Greatly strengthen the hash-pinning rule: explicitly list all reference types, ban curl|bash installs, mark as most important rule in document - Add model .gitea/workflows/check.yml pinned by commit hash |
|||
| 2ab09985e0 |
Move prompt markdown files into prompts/ subdirectory
Update all internal URLs to reflect new paths. |
