/metrics carried only the inbound HTTP surface, so a destination
failing for an hour, a growing retry backlog and a stuck-open circuit
breaker were all invisible: the receive side stays healthy in each
case because it is.
New internal/metrics registers, on the existing default registry that
the go-http-metrics recorder and the promhttp handler already share:
- webhooker_events_received_total
- webhooker_delivery_attempts_total
- webhooker_deliveries_succeeded_total
- webhooker_deliveries_failed_total
- webhooker_delivery_retries_total
- webhooker_delivery_duration_seconds
- webhooker_deliveries_pending / _retrying
- webhooker_circuit_breakers_open
The route mounting is untouched.
Every delivery metric carries one label, target_type, whose domain is
the four target-type constants; anything outside it collapses to
"unknown" so no series can be minted from a UUID. Target ids, event
ids and entrypoint ids are deliberately not labels.
An attempt is counted, and its duration observed, only where one was
actually dispatched — the target's own result path, which is also
where the DeliveryResult is written. A delivery an open circuit
breaker refuses sends nothing and records no result row; counting it
would climb the attempts counter with no traffic behind it and pull
the duration quantiles down for as long as the breaker stayed open,
moving the metric the wrong way during the outage it exists to reveal.
The log and database targets now time their own work, so their result
rows carry a real duration too.
The outcome counters move after the status row is written rather than
before, so a transition the database rejected is never reported as an
outcome that happened.
The queue-depth gauges are counted out of the per-webhook databases by
a 30s sampler rather than tracked as deltas, which would need seeding
at startup and would drift on any transition that failed to persist.
They publish an "unknown" series from registration: deliveries queued
against a target that has since been deleted resolve to the empty type
and are folded there, because a backlog behind a deleted target is
precisely the one nobody is watching. The open-breaker gauge is
recounted from the target's breaker registry on every state change.
The orphaned-retry terminal path takes the target type as an argument
rather than attaching the loaded target to the delivery. That path
loads the delivery without its target relation on purpose: a populated
Delivery.Target makes GORM's SaveBeforeAssociations upsert the whole
target row on the status UPDATE, writing the plaintext target config —
the credential, for a slack target — into the per-webhook events
database. A test asserts that path leaves the targets table empty.
check / check (push) Superseded by a newer commit; never tested
Gitea records a cancelled run as failure, and the #119 repair rewrote
that to skipped. Gitea's Combine() folds skipped into success, so a
commit nothing ever tested reported a combined green — observed on three
commits on next, including the very change a prior integration review
had failed a PR for.
Superseded commits are now marked failure with an honest description, so
never-tested no longer reads as passed and git bisect archaeology can
tell "passed", "failed" and "never ran" apart. Option 1, re-running the
superseded commit, was verified unreachable for automation on Gitea
1.25.4: no rerun endpoint, dispatches takes a ref not a SHA and lands
under a different context, and CancelPreviousJobs is unconditional.
The rewrite moves out of the workflow into script/ci-mark-superseded so
the tested artifact is the shipped one, and every failure path in it is
loud: an unparseable or empty ANCESTOR_LIMIT, an unreadable ancestor
status, and a shallow clone all abort rather than exiting 0 having
marked nothing. Each has a regression test. The status context is
derived rather than hardcoded, which also closes#147 item 2; item 1
remains open.
Independently reviewed four times. The final reviewer confirmed the
shallow-clone test is genuinely shallow — a file:// URL is load-bearing,
since git silently ignores --depth on a local path — and that deleting
the guard fails that one test out of 331 and cannot pass for the wrong
reason. They also reproduced deterministically that go test's cache
serves a stale PASS after a script-only edit, which internal/ciscript's
doc.go now records.
Closes [issue #50](#50)
## Summary
Refactors the Dockerfile to use a separate lint stage with a pinned golangci-lint Docker image, following the pattern used by [sneak/pixa](https://git.eeqj.de/sneak/pixa). This replaces the previous approach of installing golangci-lint via curl in the builder stage.
## Changes
### Dockerfile
- **New `lint` stage** using `golangci/golangci-lint:v2.11.3` (Debian-based, pinned by sha256 digest) as a separate build stage
- **Builder stage** depends on lint via `COPY --from=lint /src/go.sum /dev/null` — build won't proceed unless linting passes
- **Go bumped** from 1.24 to 1.26.1 (`golang:1.26.1-bookworm`, pinned by sha256)
- **golangci-lint bumped** from v1.64.8 to v2.11.3
- All three Docker images (golangci-lint, golang, alpine) pinned by sha256 digest
- Debian-based golangci-lint image used (not Alpine) because mattn/go-sqlite3 CGO does not compile on musl (off64_t)
### Linter Config (.golangci.yml)
- Migrated from v1 to v2 format (`version: "2"` added)
- Removed linters no longer available in v2: `gofmt` (handled by `make fmt-check`), `gosimple` (merged into `staticcheck`), `typecheck` (always-on in v2)
- Same set of linters enabled — no rules weakened
### Code Fixes (all lint issues from v2 upgrade)
- Added package comments to all packages
- Added doc comments to all exported types, functions, and methods
- Fixed unchecked errors flagged by `errcheck` (sqlDB.Close, os.Setenv in tests, resp.Body.Close, fmt.Fprint)
- Fixed unused parameters flagged by `revive` (renamed to `_`)
- Fixed `gosec` G120 warnings: added `http.MaxBytesReader` before `r.ParseForm()` calls
- Fixed `staticcheck` QF1012: replaced `WriteString(fmt.Sprintf(...))` with `fmt.Fprintf`
- Fixed `staticcheck` QF1003: converted if/else chain to tagged switch
- Renamed `DeliveryTask` → `Task` to avoid package stutter (`delivery.Task` instead of `delivery.DeliveryTask`)
- Renamed shadowed builtin `max` parameter to `upperBound` in `cryptoRandInt`
- Used `t.Setenv` instead of `os.Setenv` in tests (auto-restores)
### README.md
- Updated version requirements: Go 1.26+, golangci-lint v2.11+
- Updated Dockerfile description in project structure
## Verification
`docker build .` passes cleanly — formatting check, linting, all tests, and build all succeed.
Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #55
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Remove the entire pkg/config package (Viper-based YAML config file
loader) and simplify internal/config to read all settings directly from
environment variables via os.Getenv(). This eliminates the spurious
"Failed to load config" log messages that appeared when no config.yaml
file was present.
- Delete pkg/config/ (YAML loader, resolver, manager, tests)
- Delete configs/config.yaml.example
- Simplify internal/config helper functions to use os.Getenv() with
defaults instead of falling back to pkgconfig
- Update tests to set env vars directly instead of creating in-memory
YAML config files via afero
- Remove afero, cloud.google.com/*, aws-sdk-go dependencies from go.mod
- Update README: document env-var-only configuration, remove YAML/Viper
references
- Keep godotenv/autoload for .env file convenience in local development
closes #27
## Summary
This PR brings the webhooker repo into full REPO_POLICIES compliance, addressing both [issue #1](#1) and [issue #2](#2).
## Changes
### New files
- **`cmd/webhooker/main.go`** — The missing application entry point. Uses Uber fx to wire together all internal packages (config, database, logger, server, handlers, middleware, healthcheck, globals, session). Minimal glue code.
- **`REPO_POLICIES.md`** — Fetched from authoritative source (`sneak/prompts`)
- **`.editorconfig`** — Fetched from authoritative source
- **`.dockerignore`** — Sensible Go project exclusions
- **`.gitea/workflows/check.yml`** — CI workflow that runs `docker build .` on push to any branch (Gitea Actions format, actions/checkout pinned by sha256)
- **`configs/config.yaml.example`** — Moved from root `config.yaml`
### Modified files
- **`Makefile`** — Complete rewrite with all REPO_POLICIES required targets: `test`, `lint`, `fmt`, `fmt-check`, `check`, `build`, `hooks`, `docker`, `clean`, plus `dev`, `run`, `deps`
- **`Dockerfile`** — Complete rewrite:
- Builder: `golang:1.24` (Debian-based, pinned by `sha256:d2d2bc1c84f7...`). Debian needed because `gorm.io/driver/sqlite` pulls `mattn/go-sqlite3` (CGO) which fails on Alpine musl.
- golangci-lint v1.64.8 installed from GitHub release archive with sha256 verification (v1.x because `.golangci.yml` uses v1 config format)
- Runs `make check` (fmt-check + lint + test + build) as build step
- Final stage: `alpine:3.21` (pinned by `sha256:c3f8e73fdb79...`) with non-root user, healthcheck, port 8080
- **`README.md`** — Rewritten with all required REPO_POLICIES sections: description line with name/purpose/category/license/author, Getting Started, Rationale, Design, TODO (integrated from TODO.md), License, Author
- **`.gitignore`** — Fixed `webhooker` pattern to `/webhooker` (was blocking `cmd/webhooker/`), added `config.yaml` to prevent committing runtime config with secrets
- **`static/static.go`** — Removed `vendor` from embed directive (directory was empty/missing)
- **`internal/database/database_test.go`** — Fixed to use in-memory config via `afero.MemMapFs` instead of depending on `config.yaml` on disk. Test is now properly isolated.
- **`go.mod`/`go.sum`** — `go mod tidy`
### Removed files
- **`TODO.md`** — Content integrated into README.md TODO section
- **`config.yaml`** — Moved to `configs/config.yaml.example`
## Verification
- `docker build .` passes (lint ✅, test ✅, build ✅)
- All existing tests pass with no modifications to assertions or test logic
- `.golangci.yml` untouched
closes #1
closes #2
Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #6
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
