Found by the independent review of #159, which proved it with a probe but correctly declined to file it as a defect of that PR. Deliberately NOT milestoned 1.0.0: the pre-change behaviour was strictly worse (no bound at all, so SIGKILL at 10s), and SQLite is crash-safe.
#134 clamped the Sentry flush to the remaining stop budget less TailHookReserve (2s). The HTTP drain is not clamped the same way: cleanShutdown uses context.WithTimeout(ctx, ShutdownTimeout), i.e. min(3s, whatever is left), with no reserve subtracted. So the 2s reserve is only guaranteed when the hooks that run BEFORE the server are fast.
Demonstrated: patching the ArchiveSweeper stop hook to take 2.2s and holding a request open gives exit 1 with the tail hooks, database close included, skipped — while the server hook itself stays inside its own 3s drain budget. The budget gets eaten jointly rather than by any single hook.
That is inside the family the README discloses, but the disclosure is worded as a wedged reaper consuming "the whole budget on its own", and 2.2s out of 5s already suffices. So the README is optimistic about the size of the wedge required.
Definition of done
cleanShutdown derives the drain deadline as min(ShutdownTimeout, remaining - TailHookReserve), the same shape already applied to the flush, so the reserve survives slow preceding hooks.
TestStopTimeout_LeavesHeadroomForTailHooks is extended to model a non-zero preceding-hook consumption, not just a drain against a full budget.
The README caveat is tightened: a slow ArchiveSweeper or RetentionReaper is enough, not only one that consumes the whole budget.
The probe above exits 0 with the database close running.
Worth deciding while you are in there
TailHookReserve = 2s is currently the residual of stopTimeout - ShutdownTimeout, but the code and README treat it as a named constant chosen on its own merits. Measured tail hooks are 55.101us, 395ns, 2.168us and 1.43ms — about a thousandth of the reserve. Either pick the value deliberately and say why, or document that it is derived. A smaller reserve would buy the flush more room, though the 2s nominal cap is the binding constraint for drains under 1s anyway.
Implementation requirements
Branch from next, PR based on next, single commit, title ending (closes #N).
Run make bootstrap (or make assets) in a fresh clone before gating — browser assets are fetched at build time and not committed.
Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
Found by the independent review of https://git.eeqj.de/sneak/webhooker/pulls/159, which proved it with a probe but correctly declined to file it as a defect of that PR. Deliberately NOT milestoned 1.0.0: the pre-change behaviour was strictly worse (no bound at all, so SIGKILL at 10s), and SQLite is crash-safe.
https://git.eeqj.de/sneak/webhooker/issues/134 clamped the Sentry flush to the remaining stop budget less `TailHookReserve` (2s). The HTTP drain is not clamped the same way: `cleanShutdown` uses `context.WithTimeout(ctx, ShutdownTimeout)`, i.e. `min(3s, whatever is left)`, with no reserve subtracted. So the 2s reserve is only guaranteed when the hooks that run BEFORE the server are fast.
Demonstrated: patching the `ArchiveSweeper` stop hook to take 2.2s and holding a request open gives **exit 1 with the tail hooks, database close included, skipped** — while the server hook itself stays inside its own 3s drain budget. The budget gets eaten jointly rather than by any single hook.
That is inside the family the README discloses, but the disclosure is worded as a wedged reaper consuming "the whole budget on its own", and 2.2s out of 5s already suffices. So the README is optimistic about the size of the wedge required.
## Definition of done
- `cleanShutdown` derives the drain deadline as `min(ShutdownTimeout, remaining - TailHookReserve)`, the same shape already applied to the flush, so the reserve survives slow preceding hooks.
- `TestStopTimeout_LeavesHeadroomForTailHooks` is extended to model a non-zero preceding-hook consumption, not just a drain against a full budget.
- The README caveat is tightened: a slow `ArchiveSweeper` or `RetentionReaper` is enough, not only one that consumes the whole budget.
- The probe above exits 0 with the database close running.
## Worth deciding while you are in there
`TailHookReserve = 2s` is currently the residual of `stopTimeout - ShutdownTimeout`, but the code and README treat it as a named constant chosen on its own merits. Measured tail hooks are 55.101us, 395ns, 2.168us and 1.43ms — about a thousandth of the reserve. Either pick the value deliberately and say why, or document that it is derived. A smaller reserve would buy the flush more room, though the 2s nominal cap is the binding constraint for drains under 1s anyway.
## Implementation requirements
- Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`.
- Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112).
- Run `make bootstrap` (or `make assets`) in a fresh clone before gating — browser assets are fetched at build time and not committed.
- Gate on `make check` plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
clawbot
self-assigned this 2026-08-18 00:13:14 +02:00
Plan. Check against next first: the stop sequence changed with fx's logger (#443).
cleanShutdown takes the HTTP drain deadline as the shorter of ShutdownTimeout and the time left less TailHookReserve, as the Sentry flush already does, so slow hooks before the server cannot eat the reserve the tail hooks need.
TestStopTimeout_LeavesHeadroomForTailHooks also covers a preceding hook that uses part of the budget, and fails against the old deadline.
The README says a slow archive sweeper or retention reaper is enough to squeeze the shutdown, not only one that uses the whole budget.
TailHookReserve: one comment says how its value was chosen (it is the stop timeout less the drain timeout), or a value is picked and the reason given.
Model: opus-5-5
Plan. Check against `next` first: the stop sequence changed with fx's logger (https://git.eeqj.de/sneak/webhooker/pulls/443).
- `cleanShutdown` takes the HTTP drain deadline as the shorter of `ShutdownTimeout` and the time left less `TailHookReserve`, as the Sentry flush already does, so slow hooks before the server cannot eat the reserve the tail hooks need.
- `TestStopTimeout_LeavesHeadroomForTailHooks` also covers a preceding hook that uses part of the budget, and fails against the old deadline.
- The README says a slow archive sweeper or retention reaper is enough to squeeze the shutdown, not only one that uses the whole budget.
- `TailHookReserve`: one comment says how its value was chosen (it is the stop timeout less the drain timeout), or a value is picked and the reason given.
Model: opus-5-5
The HTTP drain now waits at most the shorter of ShutdownTimeout and what is left of the stop budget less TailHookReserve, so time the archive sweeper or retention reaper spent comes out of the drain, not out of the reserve the hooks after the server need. The headroom test also covers earlier hooks having spent part of the budget, and a new test holds a request open with only the reserve left. The reserve is documented as derived (the stop timeout less the drain timeout), not tuned.
Judgement call: the README caveat describes the fixed behaviour (a slow sweeper or reaper shortens the drain and only past 3 seconds eats the reserve), not the old one the definition of done described.
Model: opus-5-5
Built in https://git.eeqj.de/sneak/webhooker/pulls/454.
The HTTP drain now waits at most the shorter of `ShutdownTimeout` and what is left of the stop budget less `TailHookReserve`, so time the archive sweeper or retention reaper spent comes out of the drain, not out of the reserve the hooks after the server need. The headroom test also covers earlier hooks having spent part of the budget, and a new test holds a request open with only the reserve left. The reserve is documented as derived (the stop timeout less the drain timeout), not tuned.
- Judgement call: the README caveat describes the fixed behaviour (a slow sweeper or reaper shortens the drain and only past 3 seconds eats the reserve), not the old one the definition of done described.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found by the independent review of #159, which proved it with a probe but correctly declined to file it as a defect of that PR. Deliberately NOT milestoned 1.0.0: the pre-change behaviour was strictly worse (no bound at all, so SIGKILL at 10s), and SQLite is crash-safe.
#134 clamped the Sentry flush to the remaining stop budget less
TailHookReserve(2s). The HTTP drain is not clamped the same way:cleanShutdownusescontext.WithTimeout(ctx, ShutdownTimeout), i.e.min(3s, whatever is left), with no reserve subtracted. So the 2s reserve is only guaranteed when the hooks that run BEFORE the server are fast.Demonstrated: patching the
ArchiveSweeperstop hook to take 2.2s and holding a request open gives exit 1 with the tail hooks, database close included, skipped — while the server hook itself stays inside its own 3s drain budget. The budget gets eaten jointly rather than by any single hook.That is inside the family the README discloses, but the disclosure is worded as a wedged reaper consuming "the whole budget on its own", and 2.2s out of 5s already suffices. So the README is optimistic about the size of the wedge required.
Definition of done
cleanShutdownderives the drain deadline asmin(ShutdownTimeout, remaining - TailHookReserve), the same shape already applied to the flush, so the reserve survives slow preceding hooks.TestStopTimeout_LeavesHeadroomForTailHooksis extended to model a non-zero preceding-hook consumption, not just a drain against a full budget.ArchiveSweeperorRetentionReaperis enough, not only one that consumes the whole budget.Worth deciding while you are in there
TailHookReserve = 2sis currently the residual ofstopTimeout - ShutdownTimeout, but the code and README treat it as a named constant chosen on its own merits. Measured tail hooks are 55.101us, 395ns, 2.168us and 1.43ms — about a thousandth of the reserve. Either pick the value deliberately and say why, or document that it is derived. A smaller reserve would buy the flush more room, though the 2s nominal cap is the binding constraint for drains under 1s anyway.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make bootstrap(ormake assets) in a fresh clone before gating — browser assets are fetched at build time and not committed.make checkplus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.Plan. Check against
nextfirst: the stop sequence changed with fx's logger (#443).cleanShutdowntakes the HTTP drain deadline as the shorter ofShutdownTimeoutand the time left lessTailHookReserve, as the Sentry flush already does, so slow hooks before the server cannot eat the reserve the tail hooks need.TestStopTimeout_LeavesHeadroomForTailHooksalso covers a preceding hook that uses part of the budget, and fails against the old deadline.TailHookReserve: one comment says how its value was chosen (it is the stop timeout less the drain timeout), or a value is picked and the reason given.Model: opus-5-5
Built in #454.
The HTTP drain now waits at most the shorter of
ShutdownTimeoutand what is left of the stop budget lessTailHookReserve, so time the archive sweeper or retention reaper spent comes out of the drain, not out of the reserve the hooks after the server need. The headroom test also covers earlier hooks having spent part of the budget, and a new test holds a request open with only the reserve left. The reserve is documented as derived (the stop timeout less the drain timeout), not tuned.Model: opus-5-5