Stopping this process takes drain + server + cleanup seconds: eight out of the
box, and more for anyone who configures a drain window. Three places decide
whether it gets that long, and none of them was written with it in mind.
The release workflow stopped the previous container with the default deadline,
which docker sets at ten seconds. The compose file - which the Makefile calls
the first way to run this - set no stop_grace_period, so it took the same ten.
Under either, a drain window over two seconds would have been cut off by
SIGKILL part-way through the cleanup callbacks: this project's own deployments
could not have run the capability it ships.
The third was worse. `make run` removed the previous container with `docker rm
-f`, and the force flag kills a running container outright - "uses SIGKILL", in
docker's own words - with no grace at all. Restarting locally cut every
shutdown short, so the drain window would never once have been reached on a
developer's machine. It now stops with a deadline and then removes, which
leaves what gets removed unchanged: on a container that has already stopped,
stop is a no-op.
So: --timeout 30 in the workflow, stop_grace_period: 30s on the compose
service, and stop --timeout 30 before the removal in the Makefile. --timeout
rather than --time, which docker still honours but has deprecated - it prints a
warning on every use, and a deploy log that always carries a warning is one
nobody reads.
The three remaining `docker rm -f` calls in the workflow are left alone. Two
remove containers that have already been stopped and one is the rollback path,
and nothing static can tell those apart from a container that is still running -
which is also why the check added next does not look at `rm -f` at all: a forced
removal has no deadline to compare against. What keeps that path honest is the
line above it, not a check.
Thirty will drift the first time somebody raises a budget. The next commit is
what notices, which is also why these comments name a check that does not exist
yet.
The repository has 19 test files and nothing was running any of them. Both
workflows build with go build, which does not compile _test.go, the Makefile's
test target was commented out, and there is no pre-commit hook. Every test in
the tree, including the schema guards that exist precisely to catch a silent
breakage, only ran when someone remembered to type go test.
Enables the commented-out target and calls it from go.yml, the one workflow
that fires on every push and pull request. build.yml is left alone: it skips
documentation-only changes and deploys on master, so it is the wrong place for
a gate that should never be skipped.
Runs with -race. common/actions reuses model instances across concurrent
requests, so a Generate() that returns in place rather than a copy leaks data
between them, which a single-threaded run cannot see.
Verified locally: the suite passes under CGO_ENABLED=0 and under -race, and
make test exits non-zero when a test fails, so the step actually gates.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
zhangwenjian