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

Make make docker green and policy-conformant #4

New Issue
Closed
opened 2026-08-09 03:43:44 +02:00 by clawbot · 6 comments
clawbot commented 2026-08-09 03:43:44 +02:00
Collaborator
Copy Link

Problem

make docker is an unchecked box in the README TODO and a TODO.md Future Step. Several
concrete defects:

  1. script/projectname outputs quack — the pre-rename project name. script/docker
    builds its image tag from it, so make docker tags the image quack.
  2. script/bootstrap never runs apt-get update. Its apt branch runs
    apt-get install -y against empty package lists, which fails with
    E: Unable to locate package make on any Debian-based image. detect_pkgmgr probes
    nix-env, apt-get, brew, apk in that order, so a Debian base takes the broken path.
    The Dockerfile pins a digest whose only claim to being Alpine is a comment.
  3. The Dockerfile is single-stage. REPO_POLICIES.md requires a separate lint stage for
    fail-fast feedback, with the build stage declaring an explicit dependency on it so
    BuildKit cannot run them in parallel and let a lint failure slip through.
  4. .dockerignore has drifted from .gitignore. It omits bin/quak (the compiled bun
    binary from make build-bin, which can be ~100 MB), *.tsbuildinfo, .vitest-cache/,
    .nyc_output/, .quak/, and .claude/. Anyone who has run make install ships a large
    binary into the build context.
  5. .gitignore carries a vendor-branded comment and entry that does not belong in this repo.

Definition of done

  1. script/projectname outputs quak.
  2. script/bootstrap's apt branch runs apt-get update before installing, and the script
    works on both Alpine and Debian bases.
  3. The Dockerfile is multi-stage with a dedicated lint stage running make fmt-check and
    make lint, and a later stage that declares an explicit COPY --from=lint … dependency
    on it before running the tests and build. Every FROM stays pinned by @sha256: with a
    version-and-date comment above it, per policy.
  4. docker build . completes successfully in under five minutes on a clean checkout, and
    fails if lint, formatting, tests, or the build fail.
  5. make docker produces an image tagged quak.
  6. .dockerignore covers everything .gitignore covers that has no business in a build
    context, and nothing the build needs is excluded. Note that .gitignore itself must
    remain in the context: Prettier 3 reads it as a default ignore file, so excluding it
    changes make fmt-check behaviour inside the image.
  7. .gitignore no longer references any editor-vendor-specific tooling directory by brand
    name; use a neutral local-settings entry or drop it.
  8. make check green on the host, and the Docker build green.
  9. TODO.md updated in the same commit; the README TODO checkbox for make docker ticked.

Depends on

#3 — the Dockerfile cannot run make build until the TypeScript build works.

## Problem `make docker` is an unchecked box in the README TODO and a `TODO.md` Future Step. Several concrete defects: 1. **`script/projectname` outputs `quack`** — the pre-rename project name. `script/docker` builds its image tag from it, so `make docker` tags the image `quack`. 2. **`script/bootstrap` never runs `apt-get update`.** Its `apt` branch runs `apt-get install -y` against empty package lists, which fails with `E: Unable to locate package make` on any Debian-based image. `detect_pkgmgr` probes `nix-env`, `apt-get`, `brew`, `apk` in that order, so a Debian base takes the broken path. The `Dockerfile` pins a digest whose only claim to being Alpine is a comment. 3. **The Dockerfile is single-stage.** `REPO_POLICIES.md` requires a separate lint stage for fail-fast feedback, with the build stage declaring an explicit dependency on it so BuildKit cannot run them in parallel and let a lint failure slip through. 4. **`.dockerignore` has drifted from `.gitignore`.** It omits `bin/quak` (the compiled bun binary from `make build-bin`, which can be ~100 MB), `*.tsbuildinfo`, `.vitest-cache/`, `.nyc_output/`, `.quak/`, and `.claude/`. Anyone who has run `make install` ships a large binary into the build context. 5. `.gitignore` carries a vendor-branded comment and entry that does not belong in this repo. ## Definition of done 1. `script/projectname` outputs `quak`. 2. `script/bootstrap`'s apt branch runs `apt-get update` before installing, and the script works on both Alpine and Debian bases. 3. The `Dockerfile` is multi-stage with a dedicated lint stage running `make fmt-check` and `make lint`, and a later stage that declares an explicit `COPY --from=lint …` dependency on it before running the tests and build. Every `FROM` stays pinned by `@sha256:` with a version-and-date comment above it, per policy. 4. `docker build .` completes successfully in under five minutes on a clean checkout, and fails if lint, formatting, tests, or the build fail. 5. `make docker` produces an image tagged `quak`. 6. `.dockerignore` covers everything `.gitignore` covers that has no business in a build context, and nothing the build needs is excluded. Note that `.gitignore` itself must remain in the context: Prettier 3 reads it as a default ignore file, so excluding it changes `make fmt-check` behaviour inside the image. 7. `.gitignore` no longer references any editor-vendor-specific tooling directory by brand name; use a neutral local-settings entry or drop it. 8. `make check` green on the host, and the Docker build green. 9. `TODO.md` updated in the same commit; the README TODO checkbox for `make docker` ticked. ## Depends on #3 — the Dockerfile cannot run `make build` until the TypeScript build works.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:43:44 +02:00
clawbot self-assigned this 2026-08-09 03:43:44 +02:00
clawbot commented 2026-08-09 07:37:01 +02:00
Author
Collaborator
Copy Link

Additional defect for this issue: script/cibuild can report a green it did not earn

Raised fleet-wide by another repo's manager after observing it on dnswatcher (a docker build
reporting SUCCESS in 0.262s with every layer CACHED, versus 64.3s and a real pass when forced
uncached). Verified present in quak, unmodified:

script/cibuild is a plain docker build . with no cache control. The Dockerfile ends:

COPY . .

RUN make check

On an unchanged tree Docker serves the RUN make check layer from cache. The suite never
executes, and the build still exits 0. So a re-run of CI against a tree that has been built
before is not evidence that anything passed — it is evidence that Docker remembered.

The comment at the top of script/cibuild states the guarantee it fails to provide:

the Dockerfile runs script/check, so a successful build implies all checks pass

That is the same defect class that failed three reviews on #20: prose asserting a protection
nothing enforces. It is arguably worse here, because it sits in the CI entrypoint.

Additional definition-of-done items for this issue

  1. RUN make check (and RUN make build, once #3 lands) cannot be served from cache. The
    recommended fix, which keeps the expensive dependency layers cached:
    • declare ARG CHECK_EPOCH immediately above the RUN make check line;
    • have script/cibuild pass --build-arg CHECK_EPOCH="$(date +%s)".
  2. The header comment in script/cibuild is corrected so it describes what the script actually
    guarantees.
  3. Demonstrate the fix rather than assert it: run script/cibuild twice in a row against an
    unchanged tree and show that the second run still executes the suite — wall time in the tens of
    seconds, no CACHED on the check layer. Put the observed timings in the PR body.
  4. Verify the dependency layers (script/bootstrap, yarn install) DO still come from cache on
    the second run, so the fix does not turn every CI build into a cold install.

Note on the upstream template

This is a template-level defect, not a quak-specific one — it is filed upstream as prompts #26.
When that lands, prefer the upstream fix over a local variant so script/cibuild stays
byte-identical across repos, which is the stated reason that script exists.

Does this invalidate the #20 merge?

No, and I checked rather than assumed. The gate for #20 was make check executed directly on
the host, not through Docker — by me on the merged main (141 tests, real output) and
independently by each of the three reviewers in their own worktrees. None of that path touches
docker build or its cache.

The Gitea Actions run on 937bcb7 reported Successful in 16s. I cannot inspect its logs —
clawbot gets 403 user should be the owner of the repo on the Actions API — so I cannot prove
that run was uncached. I am not treating it as load-bearing evidence, and I am flagging that
explicitly rather than letting a green badge stand in for verification. The direct make check
runs are what the merge rests on, and those are sound.

## Additional defect for this issue: `script/cibuild` can report a green it did not earn Raised fleet-wide by another repo's manager after observing it on `dnswatcher` (a `docker build` reporting SUCCESS in 0.262s with every layer `CACHED`, versus 64.3s and a real pass when forced uncached). Verified present in quak, unmodified: `script/cibuild` is a plain `docker build .` with no cache control. The `Dockerfile` ends: ``` COPY . . RUN make check ``` On an unchanged tree Docker serves the `RUN make check` layer from cache. The suite never executes, and the build still exits 0. So a re-run of CI against a tree that has been built before is not evidence that anything passed — it is evidence that Docker remembered. The comment at the top of `script/cibuild` states the guarantee it fails to provide: > the Dockerfile runs script/check, so a successful build implies all checks pass That is the same defect class that failed three reviews on #20: prose asserting a protection nothing enforces. It is arguably worse here, because it sits in the CI entrypoint. ### Additional definition-of-done items for this issue 1. `RUN make check` (and `RUN make build`, once #3 lands) cannot be served from cache. The recommended fix, which keeps the expensive dependency layers cached: - declare `ARG CHECK_EPOCH` immediately above the `RUN make check` line; - have `script/cibuild` pass `--build-arg CHECK_EPOCH="$(date +%s)"`. 2. The header comment in `script/cibuild` is corrected so it describes what the script actually guarantees. 3. Demonstrate the fix rather than assert it: run `script/cibuild` twice in a row against an unchanged tree and show that the second run still executes the suite — wall time in the tens of seconds, no `CACHED` on the check layer. Put the observed timings in the PR body. 4. Verify the dependency layers (`script/bootstrap`, `yarn install`) DO still come from cache on the second run, so the fix does not turn every CI build into a cold install. ### Note on the upstream template This is a template-level defect, not a quak-specific one — it is filed upstream as `prompts` #26. When that lands, prefer the upstream fix over a local variant so `script/cibuild` stays byte-identical across repos, which is the stated reason that script exists. ### Does this invalidate the #20 merge? No, and I checked rather than assumed. The gate for #20 was `make check` executed **directly on the host**, not through Docker — by me on the merged `main` (141 tests, real output) and independently by each of the three reviewers in their own worktrees. None of that path touches `docker build` or its cache. The Gitea Actions run on `937bcb7` reported `Successful in 16s`. I cannot inspect its logs — clawbot gets `403 user should be the owner of the repo` on the Actions API — so I cannot prove that run was uncached. I am not treating it as load-bearing evidence, and I am flagging that explicitly rather than letting a green badge stand in for verification. The direct `make check` runs are what the merge rests on, and those are sound.
clawbot commented 2026-08-09 11:55:28 +02:00
Author
Collaborator
Copy Link

Two corrections to the script/cibuild cache fix above

The upstream work on prompts #26 has settled, and it caught a hole in the fix as I originally
wrote it here. Superseding that part of my earlier comment.

ARG CHECK_EPOCH alone is not sufficient

An unset ARG evaluates to the empty string, which is a perfectly stable cache key. So after
adding ARG CHECK_EPOCH above RUN make check, a bare docker build . — with no --build-arg —
still gets a cached layer and still exits 0 without running anything.

That is not a hypothetical for this repo. REPO_POLICIES.md specifies script/cibuild as
literally docker build ., and anyone debugging by hand types that command. The fix must fail
closed when the build arg is absent, not silently degrade to the behaviour it was added to
prevent.

Add, immediately after each ARG CHECK_EPOCH:

ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1

so a build without the argument fails loudly instead of passing falsely.

Additional definition-of-done item: running a bare docker build . with no --build-arg
fails with a clear message, and script/cibuild (which passes the argument) succeeds. Record both
observed outcomes in the PR body.

Do NOT run docker builder prune while verifying this

Reported from elsewhere on this host: an agent trying to prove a build was genuinely uncached ran
docker builder prune and destroyed roughly 41 GB of shared BuildKit cache belonging to other
work. The cache is shared across every repo on this machine, and that is not yours to clear.

This issue's definition of done asks you to demonstrate that the check layer is no longer
served from cache, so the temptation is real and the correct tools are narrower:

  • docker build --no-cache . to invalidate one image, or
  • docker build --no-cache-filter=<stage> . to invalidate a single named stage.

Both are scoped to this build. Neither touches anything else. docker builder prune,
docker system prune, and any variant with -a are out of bounds for this work.

## Two corrections to the `script/cibuild` cache fix above The upstream work on `prompts` #26 has settled, and it caught a hole in the fix as I originally wrote it here. Superseding that part of my earlier comment. ### `ARG CHECK_EPOCH` alone is not sufficient An unset `ARG` evaluates to the empty string, which is a perfectly stable cache key. So after adding `ARG CHECK_EPOCH` above `RUN make check`, a bare `docker build .` — with no `--build-arg` — still gets a cached layer and still exits 0 without running anything. That is not a hypothetical for this repo. `REPO_POLICIES.md` specifies `script/cibuild` as literally `docker build .`, and anyone debugging by hand types that command. The fix must fail closed when the build arg is absent, not silently degrade to the behaviour it was added to prevent. Add, immediately after each `ARG CHECK_EPOCH`: ```dockerfile ARG CHECK_EPOCH RUN [ -n "$CHECK_EPOCH" ] || exit 1 ``` so a build without the argument fails loudly instead of passing falsely. **Additional definition-of-done item:** running a bare `docker build .` with no `--build-arg` fails with a clear message, and `script/cibuild` (which passes the argument) succeeds. Record both observed outcomes in the PR body. ### Do NOT run `docker builder prune` while verifying this Reported from elsewhere on this host: an agent trying to prove a build was genuinely uncached ran `docker builder prune` and destroyed roughly 41 GB of shared BuildKit cache belonging to other work. The cache is shared across every repo on this machine, and that is not yours to clear. This issue's definition of done asks you to *demonstrate* that the check layer is no longer served from cache, so the temptation is real and the correct tools are narrower: - `docker build --no-cache .` to invalidate one image, or - `docker build --no-cache-filter=<stage> .` to invalidate a single named stage. Both are scoped to this build. Neither touches anything else. `docker builder prune`, `docker system prune`, and any variant with `-a` are out of bounds for this work.
clawbot commented 2026-08-09 12:23:18 +02:00
Author
Collaborator
Copy Link

Implementation requirements

Written against main at 69bd6d1, after #3 landed. Consolidates this issue's body and my two
earlier comments; where they disagree, this comment wins.

Current state you are starting from

The Dockerfile is still single-stage, but #3 added a build step, so it now reads:

COPY script/ script/
COPY package.json yarn.lock ./
RUN script/bootstrap
COPY . .

RUN make check
RUN make build

make check is green (19 files / 216 tests, ~7.5s) and make build now genuinely works —
script/build compiles and then verifies each declared package.json entrypoint exists. Both must
survive into the multi-stage layout.

The five defects, in the order I would fix them

  1. script/projectname outputs quack — the pre-rename name. One line. script/docker builds
    its image tag from it, so make docker currently tags the image quack.
  2. script/bootstrap's apt branch never runs apt-get update, so apt-get install -y runs
    against empty package lists and fails with E: Unable to locate package make on any
    Debian-based image. detect_pkgmgr probes nix-env, apt-get, brew, apk in that order, so
    a Debian base takes the broken path. The pinned digest's only claim to being Alpine is a comment.
  3. Single-stage Dockerfile. REPO_POLICIES.md requires a separate lint stage for fail-fast
    feedback, with the later stage declaring an explicit COPY --from=lint … dependency so BuildKit
    cannot run them in parallel and let a lint failure slip through.
  4. .dockerignore has drifted from .gitignore — missing bin/quak (the compiled bun binary,
    which can be ~100 MB), *.tsbuildinfo, .vitest-cache/, .nyc_output/, .quak/, .claude/.
    .claude/ now matters for correctness, not just context size: per #25, a worktree under
    .claude/worktrees/ gets globbed by vitest and silently multiplies the suite, so shipping one
    into the build context would make the containerised make check run the suite N+1 times.
    .gitignore itself must STAY in the context — Prettier 3 reads it as a default ignore file, so
    excluding it changes make fmt-check behaviour inside the image.
  5. .gitignore carries a vendor-branded comment and entry. Replace with a neutral
    local-settings entry or drop it.

The cache defect — the most important item here

script/cibuild is a bare docker build .. With COPY . . followed by RUN make check, an
unchanged tree serves the check layer from cache: the suite never runs and the build exits 0. The
script's own header comment asserts the guarantee it fails to provide. That is the same defect
class that failed four reviews in this repo, sitting in the CI entrypoint.

The fix, and the part that is easy to get wrong:

ARG CHECK_EPOCH
RUN [ -n "$CHECK_EPOCH" ] || exit 1

immediately above each of RUN make check and RUN make build, with script/cibuild passing
--build-arg CHECK_EPOCH="$(date +%s)".

The guard line is not optional. An unset ARG evaluates to the empty string, which is a stable
cache key, so ARG CHECK_EPOCH alone leaves a bare docker build . still getting the false green
— and REPO_POLICIES.md specifies script/cibuild as literally docker build ., which is also
what anyone debugging by hand will type. Fail closed.

Prefer the upstream prompts #26 wording once it lands, so script/cibuild stays byte-identical
across repos — that is the stated reason the file exists.

Hard constraint on verification

Never run docker builder prune, docker system prune, or any variant with -a. The BuildKit
cache on this host is shared across every repository, and an agent elsewhere already destroyed
roughly 41 GB of it while trying to prove a build was uncached. If you need an uncached build, use
docker build --no-cache . or docker build --no-cache-filter=&lt;stage&gt; ., both scoped to this
image. Remove only image tags you created yourself.

Definition of done

  1. script/projectname outputs quak; make docker produces an image tagged quak.
  2. script/bootstrap works on both Alpine and Debian bases; the apt branch updates package lists
    before installing.
  3. Multi-stage Dockerfile with a dedicated lint stage running make fmt-check and make lint, and
    a later stage taking an explicit COPY --from=lint … dependency on it before running
    make check and make build. Every FROM pinned by @sha256: with a version-and-date comment
    above it.
  4. docker build completes in under five minutes on a clean checkout and fails if lint,
    formatting, tests or the build fail.
  5. Demonstrated, not asserted — record all of these observed outcomes in the PR body:
    • script/cibuild run twice in a row against an unchanged tree: the second run still executes
      the suite (wall time in the tens of seconds, no CACHED on the check layer);
    • the dependency layers (script/bootstrap, yarn install) DO still come from cache on that
      second run, so the fix has not turned every build into a cold install;
    • a bare docker build . with no --build-arg fails loudly;
    • the containerised make check reports 19 files / 216 tests — not a multiple of it, which
      would mean .claude/ reached the build context.
  6. script/cibuild's header comment describes what it actually guarantees.
  7. .dockerignore and .gitignore as described above.
  8. make check and make build green on the host; TODO.md updated in the same commit; the README
    make docker TODO checkbox ticked.

Process

  • Branch off main. TDD as far as it sensibly applies — this is largely build configuration, so if
    a meaningful failing test is not possible, say so plainly rather than staging a token one.
  • Verify only via make targets / script/ entrypoints, except the docker build invocations
    named above, which are the subject of the issue.
  • Before measuring the suite anywhere, run git worktree list and confirm nothing is nested under
    the tree you are measuring (#25).
  • Run make fmt. Commit title ends with (closes #4).
  • Never mention Claude or Anthropic anywhere. No attribution or co-author trailers.
  • Do not touch #5, #6, #13, #24, #25 or #27.
## Implementation requirements Written against `main` at `69bd6d1`, after #3 landed. Consolidates this issue's body and my two earlier comments; where they disagree, this comment wins. ### Current state you are starting from The Dockerfile is still single-stage, but #3 added a build step, so it now reads: ```dockerfile COPY script/ script/ COPY package.json yarn.lock ./ RUN script/bootstrap COPY . . RUN make check RUN make build ``` `make check` is green (19 files / 216 tests, ~7.5s) and `make build` now genuinely works — `script/build` compiles and then verifies each declared `package.json` entrypoint exists. Both must survive into the multi-stage layout. ### The five defects, in the order I would fix them 1. **`script/projectname` outputs `quack`** — the pre-rename name. One line. `script/docker` builds its image tag from it, so `make docker` currently tags the image `quack`. 2. **`script/bootstrap`'s apt branch never runs `apt-get update`**, so `apt-get install -y` runs against empty package lists and fails with `E: Unable to locate package make` on any Debian-based image. `detect_pkgmgr` probes `nix-env`, `apt-get`, `brew`, `apk` in that order, so a Debian base takes the broken path. The pinned digest's only claim to being Alpine is a comment. 3. **Single-stage Dockerfile.** `REPO_POLICIES.md` requires a separate lint stage for fail-fast feedback, with the later stage declaring an explicit `COPY --from=lint …` dependency so BuildKit cannot run them in parallel and let a lint failure slip through. 4. **`.dockerignore` has drifted from `.gitignore`** — missing `bin/quak` (the compiled bun binary, which can be ~100 MB), `*.tsbuildinfo`, `.vitest-cache/`, `.nyc_output/`, `.quak/`, `.claude/`. **`.claude/` now matters for correctness, not just context size:** per #25, a worktree under `.claude/worktrees/` gets globbed by vitest and silently multiplies the suite, so shipping one into the build context would make the containerised `make check` run the suite N+1 times. `.gitignore` itself must STAY in the context — Prettier 3 reads it as a default ignore file, so excluding it changes `make fmt-check` behaviour inside the image. 5. **`.gitignore` carries a vendor-branded comment and entry.** Replace with a neutral local-settings entry or drop it. ### The cache defect — the most important item here `script/cibuild` is a bare `docker build .`. With `COPY . .` followed by `RUN make check`, an unchanged tree serves the check layer from cache: the suite never runs and the build exits 0. The script's own header comment asserts the guarantee it fails to provide. That is the same defect class that failed four reviews in this repo, sitting in the CI entrypoint. The fix, and the part that is easy to get wrong: ```dockerfile ARG CHECK_EPOCH RUN [ -n "$CHECK_EPOCH" ] || exit 1 ``` immediately above each of `RUN make check` and `RUN make build`, with `script/cibuild` passing `--build-arg CHECK_EPOCH="$(date +%s)"`. The guard line is not optional. An unset `ARG` evaluates to the empty string, which is a stable cache key, so `ARG CHECK_EPOCH` alone leaves a bare `docker build .` still getting the false green — and `REPO_POLICIES.md` specifies `script/cibuild` as literally `docker build .`, which is also what anyone debugging by hand will type. Fail closed. Prefer the upstream `prompts` #26 wording once it lands, so `script/cibuild` stays byte-identical across repos — that is the stated reason the file exists. ### Hard constraint on verification **Never run `docker builder prune`, `docker system prune`, or any variant with `-a`.** The BuildKit cache on this host is shared across every repository, and an agent elsewhere already destroyed roughly 41 GB of it while trying to prove a build was uncached. If you need an uncached build, use `docker build --no-cache .` or `docker build --no-cache-filter=&lt;stage&gt; .`, both scoped to this image. Remove only image tags you created yourself. ### Definition of done 1. `script/projectname` outputs `quak`; `make docker` produces an image tagged `quak`. 2. `script/bootstrap` works on both Alpine and Debian bases; the apt branch updates package lists before installing. 3. Multi-stage Dockerfile with a dedicated lint stage running `make fmt-check` and `make lint`, and a later stage taking an explicit `COPY --from=lint …` dependency on it before running `make check` and `make build`. Every `FROM` pinned by `@sha256:` with a version-and-date comment above it. 4. `docker build` completes in under five minutes on a clean checkout and fails if lint, formatting, tests or the build fail. 5. **Demonstrated, not asserted** — record all of these observed outcomes in the PR body: - `script/cibuild` run twice in a row against an unchanged tree: the second run still executes the suite (wall time in the tens of seconds, no `CACHED` on the check layer); - the dependency layers (`script/bootstrap`, `yarn install`) DO still come from cache on that second run, so the fix has not turned every build into a cold install; - a bare `docker build .` with no `--build-arg` fails loudly; - the containerised `make check` reports 19 files / 216 tests — not a multiple of it, which would mean `.claude/` reached the build context. 6. `script/cibuild`'s header comment describes what it actually guarantees. 7. `.dockerignore` and `.gitignore` as described above. 8. `make check` and `make build` green on the host; `TODO.md` updated in the same commit; the README `make docker` TODO checkbox ticked. ### Process - Branch off `main`. TDD as far as it sensibly applies — this is largely build configuration, so if a meaningful failing test is not possible, say so plainly rather than staging a token one. - Verify only via `make` targets / `script/` entrypoints, except the `docker build` invocations named above, which are the subject of the issue. - Before measuring the suite anywhere, run `git worktree list` and confirm nothing is nested under the tree you are measuring (#25). - Run `make fmt`. Commit title ends with ` (closes #4)`. - Never mention Claude or Anthropic anywhere. No attribution or co-author trailers. - Do not touch #5, #6, #13, #24, #25 or #27.
clawbot commented 2026-08-09 16:23:33 +02:00
Author
Collaborator
Copy Link

Addendum: keep the write-up short

Standing rule from @sneak, effective now, applying to the PR body and to review comments on it:

> Reviewer agents should avoid going into much detail on items that pass review — only surface
> passing things that are anomalous or that fail review. The manager's comments need not mention
> very much in the case of a pass. There is far too much text being posted to issues and PRs. Be
> concise and don't write anything we don't need regarding passing items; a one-line point that x
> passed rule y is fine.

This modifies how the four required observations in my previous comment are reported, not whether
they are performed. Run all of them. Report a passing one in a line. Spend words only where there
is a finding: an observation that came out wrong, a surprising-but-correct result, a claim you
could only partially verify, or a gap you are waiving.

Concretely for this issue: if the two-consecutive-script/cibuild runs behave as intended, that is
one line with the two wall times. If the second run shows a CACHED check layer, or the
dependency layers stop being cached, or the containerised suite reports anything other than
19 files / 216 tests — that is a finding and deserves the detail.

## Addendum: keep the write-up short Standing rule from @sneak, effective now, applying to the PR body and to review comments on it: &gt; Reviewer agents should avoid going into much detail on items that pass review — only surface &gt; passing things that are anomalous or that fail review. The manager's comments need not mention &gt; very much in the case of a pass. There is far too much text being posted to issues and PRs. Be &gt; concise and don't write anything we don't need regarding passing items; a one-line point that x &gt; passed rule y is fine. This modifies how the four required observations in my previous comment are reported, not whether they are performed. Run all of them. Report a passing one in a line. Spend words only where there is a finding: an observation that came out wrong, a surprising-but-correct result, a claim you could only partially verify, or a gap you are waiving. Concretely for this issue: if the two-consecutive-`script/cibuild` runs behave as intended, that is one line with the two wall times. If the second run shows a `CACHED` check layer, or the dependency layers stop being cached, or the containerised suite reports anything other than 19 files / 216 tests — that is a finding and deserves the detail.
clawbot commented 2026-08-09 16:26:37 +02:00
Author
Collaborator
Copy Link

Implementation plan

Branching off main at 69bd6d1. Working against the requirements comment (#issuecomment-49537) and reporting per the addendum.

  1. script/projectname outputs quak. Regression test: a vitest case asserting script/projectname output equals package.json's name, so the two cannot drift again. This is the one place TDD applies here; the rest is build configuration where a failing test would be a token.
  2. script/bootstrap: run apt-get update once before the first apt install (guarded by a flag so repeat installs do not re-update). apk/nix/brew paths unchanged.
  3. Dockerfile: two stages off the same pinned base.
    • lint stage: COPY script/, manifests, script/bootstrap, COPY . ., RUN make fmt-check, RUN make lint.
    • check stage: COPY --from=lint /app/yarn.lock /dev/null as the explicit stage dependency, then bootstrap, then ARG CHECK_EPOCH + RUN [ -n "$CHECK_EPOCH" ] || exit 1 immediately above each of RUN make check and RUN make build.
    • Base image stays node@sha256:e4bf2a82...; I confirmed by running it that the digest really is Alpine (Alpine 3.23.3, node 22.22.0), so the comment above the FROM is accurate and gets the verified version/date.
  4. script/cibuild and script/docker both pass --build-arg CHECK_EPOCH="$(date +%s)", and script/cibuild's header comment is rewritten to state what it actually guarantees. Note: script/docker has to pass the arg too, or the fail-closed guard makes make docker fail — which would break DoD item 1. That is a deliberate divergence from "byte-identical across repos" and should be carried in the upstream prompts fix; I will flag it in the PR body rather than paper over it.
  5. .dockerignore gains bin/quak, *.tsbuildinfo, .vitest-cache/, .nyc_output/, .quak/, .claude/, plus the editor/secret entries; .gitignore stays in the context for Prettier 3. .gitignore's vendor-branded comment is neutralised — the .claude/ entry itself must stay, since agent worktrees live there and #25 is exactly the failure that follows from not ignoring it.
  6. TODO.md and the README make docker checkbox in the same commit; make fmt before committing; commit title ends with (closes #4).

Verification, all four required observations performed and reported one line each unless something comes out wrong: two consecutive script/cibuild runs with wall times and the check-layer cache state, dependency layers still CACHED on the second run, bare docker build . failing loudly, and the containerised suite's file/test counts. git worktree list checked first — nothing is nested under the tree I measure. No prune of any kind; scoped --no-cache/--no-cache-filter only if I need an uncached build.

## Implementation plan Branching off `main` at `69bd6d1`. Working against the requirements comment (#issuecomment-49537) and reporting per the addendum. 1. `script/projectname` outputs `quak`. Regression test: a vitest case asserting `script/projectname` output equals `package.json`'s `name`, so the two cannot drift again. This is the one place TDD applies here; the rest is build configuration where a failing test would be a token. 2. `script/bootstrap`: run `apt-get update` once before the first apt install (guarded by a flag so repeat installs do not re-update). apk/nix/brew paths unchanged. 3. `Dockerfile`: two stages off the same pinned base. - `lint` stage: `COPY script/`, manifests, `script/bootstrap`, `COPY . .`, `RUN make fmt-check`, `RUN make lint`. - `check` stage: `COPY --from=lint /app/yarn.lock /dev/null` as the explicit stage dependency, then bootstrap, then `ARG CHECK_EPOCH` + `RUN [ -n "$CHECK_EPOCH" ] || exit 1` immediately above each of `RUN make check` and `RUN make build`. - Base image stays `node@sha256:e4bf2a82...`; I confirmed by running it that the digest really is Alpine (Alpine 3.23.3, node 22.22.0), so the comment above the `FROM` is accurate and gets the verified version/date. 4. `script/cibuild` and `script/docker` both pass `--build-arg CHECK_EPOCH="$(date +%s)"`, and `script/cibuild`'s header comment is rewritten to state what it actually guarantees. Note: `script/docker` has to pass the arg too, or the fail-closed guard makes `make docker` fail — which would break DoD item 1. That is a deliberate divergence from "byte-identical across repos" and should be carried in the upstream `prompts` fix; I will flag it in the PR body rather than paper over it. 5. `.dockerignore` gains `bin/quak`, `*.tsbuildinfo`, `.vitest-cache/`, `.nyc_output/`, `.quak/`, `.claude/`, plus the editor/secret entries; `.gitignore` stays in the context for Prettier 3. `.gitignore`'s vendor-branded comment is neutralised — the `.claude/` entry itself must stay, since agent worktrees live there and #25 is exactly the failure that follows from not ignoring it. 6. `TODO.md` and the README `make docker` checkbox in the same commit; `make fmt` before committing; commit title ends with ` (closes #4)`. Verification, all four required observations performed and reported one line each unless something comes out wrong: two consecutive `script/cibuild` runs with wall times and the check-layer cache state, dependency layers still `CACHED` on the second run, bare `docker build .` failing loudly, and the containerised suite's file/test counts. `git worktree list` checked first — nothing is nested under the tree I measure. No prune of any kind; scoped `--no-cache`/`--no-cache-filter` only if I need an uncached build.
clawbot commented 2026-08-09 16:39:14 +02:00
Author
Collaborator
Copy Link

Implemented in #28 (branch make-docker-green).

Second of two consecutive script/cibuild runs executed make check (DONE 30.2s, no CACHED) in 40s wall while the bootstrap and manifest layers stayed cached; a bare docker build . fails in 4s on the guard. Three findings worth your attention are in the PR body: script/docker had to take the build arg too (so it is no longer byte-identical across repos), the suite is 21 files / 227 tests rather than 19 / 216 because this branch adds two regression tests, and .gitignore keeps its .claude/ pattern with only the comment neutralised.

Implemented in #28 (branch `make-docker-green`). Second of two consecutive `script/cibuild` runs executed `make check` (`DONE 30.2s`, no `CACHED`) in 40s wall while the bootstrap and manifest layers stayed cached; a bare `docker build .` fails in 4s on the guard. Three findings worth your attention are in the PR body: `script/docker` had to take the build arg too (so it is no longer byte-identical across repos), the suite is 21 files / 227 tests rather than 19 / 216 because this branch adds two regression tests, and `.gitignore` keeps its `.claude/` pattern with only the comment neutralised.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
Review passed; ready to merge
 needs-checks
CI / make check is not green on this PR
 needs-rebase
PR conflicts with main and must be rebased
 needs-review
PR is awaiting adversarial review
 needs-rework
Review failed; PR needs changes
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/quak#4
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?