Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7674efd2b1 | ||
|
|
ecf47c37f5 |
@@ -1,55 +1,73 @@
|
||||
# AGENTS.md — Rules for AI Agents
|
||||
# Rules for coding agents
|
||||
|
||||
## Before Every Commit
|
||||
This file holds the rules for coding agents working in this repository.
|
||||
`CONVENTIONS.md` defines the architecture. `REPO_POLICIES.md` defines the
|
||||
repository standards. Both are copies of shared documents. Do not edit them
|
||||
here.
|
||||
|
||||
1. **Format**: `make fmt`
|
||||
2. **Check**: `make check` — the tests and the linter, which run in Docker, and
|
||||
the format check: all passing, zero issues
|
||||
## Before every commit
|
||||
|
||||
Never run `go test`, `golangci-lint` or `gofmt` directly; use the `make`
|
||||
targets.
|
||||
1. Run `make fmt`.
|
||||
2. Run `make check`. It runs the tests and the linter in Docker, then the format
|
||||
check. All three must pass with no findings.
|
||||
|
||||
No commit lands on main with lint errors, test failures, or formatting issues.
|
||||
Never run `go test`, `golangci-lint` or `gofmt` directly. Use the `make` targets
|
||||
instead, such as `make test` and `make lint`.
|
||||
|
||||
## Code Style
|
||||
## Branches and pull requests
|
||||
|
||||
- Follow CONVENTIONS.md exactly — it defines the architecture
|
||||
- All exported types, functions, methods, and vars must have doc comments
|
||||
- All packages must have a package doc comment
|
||||
- JSON tags use camelCase (not snake_case)
|
||||
- No magic numbers — use named constants
|
||||
- No inline error handling (`if err := ...; err != nil`) — use plain assignment
|
||||
- Blank line before every `return` statement
|
||||
- Blank line separating embedded fields from regular fields in structs
|
||||
- Blank imports must have a justification comment
|
||||
- Never commit to `main` or `next` directly. Every change is a pull request from
|
||||
a feature branch into `next`.
|
||||
- Only the owner merges `next` into `main`.
|
||||
- A pull request that closes an issue has the title `Description (closes #N)`.
|
||||
- One logical change per commit.
|
||||
- Commit messages are in the imperative mood and short.
|
||||
|
||||
## Git Workflow
|
||||
## Code style
|
||||
|
||||
- **Never commit directly to main** — all changes go on feature branches
|
||||
- Merge to main only when ready and passing all checks
|
||||
- PR titles: `Description (closes #N)` when closing an issue
|
||||
- One logical change per commit
|
||||
- Commit messages: imperative mood, concise
|
||||
- Follow `CONVENTIONS.md`.
|
||||
- Outside test files, every package has a package doc comment.
|
||||
- Outside test files, every exported type, function and variable has a doc
|
||||
comment.
|
||||
- JSON tags are camelCase. The HTTP API's `last_id`, `pow_token` and
|
||||
`hashcash_bits` stay as they are, each with a `//nolint:tagliatelle` comment.
|
||||
- Prefer named constants to magic numbers. The `mnd` linter checks for them
|
||||
outside test files.
|
||||
- No inline error handling such as `if err := f(); err != nil`. Assign the error
|
||||
on its own line, then test it.
|
||||
- A `return` that follows another statement in its block has a blank line before
|
||||
it.
|
||||
- A blank line between the embedded fields of a struct and its other fields.
|
||||
- Outside test files, every blank import has a comment saying why it is there.
|
||||
|
||||
## Architecture
|
||||
|
||||
- Dependency injection via Uber fx — all components are fx providers
|
||||
- `internal/` for all application packages — one package per concern
|
||||
- Handlers return `http.HandlerFunc` using the closure pattern
|
||||
- Request/response structs defined inside handler closures
|
||||
- Models embed `models.Base` for database access to relation methods
|
||||
- Database migrations in `internal/db/schema/` — embedded via `go:embed`
|
||||
- SQLite by default (pure Go driver: modernc.org/sqlite)
|
||||
- The server uses `go.uber.org/fx` for dependency injection.
|
||||
`cmd/neoircd/main.go` lists its `fx` providers, such as `db.New` and
|
||||
`handlers.New`.
|
||||
- Application packages live under `internal/`, one package per concern.
|
||||
- `pkg/irc` holds the IRC command names and numeric reply codes.
|
||||
- The `web` package embeds the built web client.
|
||||
- Each exported `Handle…` method of `Handlers` returns an `http.HandlerFunc`.
|
||||
- A request struct is defined inside the method that decodes it.
|
||||
- The server uses the database only through the methods of `db.Database`.
|
||||
- The database migrations live in `internal/db/schema/` and are embedded with
|
||||
`go:embed`.
|
||||
- The database is SQLite, through the pure Go driver `modernc.org/sqlite`.
|
||||
|
||||
## Testing
|
||||
|
||||
- Table-driven tests preferred
|
||||
- Use `testing.T` helpers, not assertion libraries
|
||||
- Test files live next to the code they test
|
||||
- `_test.go` suffix, same package for white-box testing
|
||||
- Prefer table-driven tests.
|
||||
- Use `testing.T` helpers, not assertion libraries.
|
||||
- A test file lives next to the code it tests and ends in `_test.go`.
|
||||
- The linter requires each test file to be in the package's external test
|
||||
package, such as `handlers_test`. It allows `export_test.go` and files ending
|
||||
in `_internal_test.go` in the package itself.
|
||||
- An `export_test.go` file gives the external tests access to the package's
|
||||
unexported code.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Required libraries are listed in CONVENTIONS.md §1
|
||||
- No new dependencies without justification
|
||||
- Pure Go preferred (no CGO)
|
||||
- The required libraries are listed in `CONVENTIONS.md` §1.
|
||||
- No new dependency without a reason given in the pull request.
|
||||
- Prefer dependencies written in pure Go, which build without cgo.
|
||||
|
||||
@@ -137,20 +137,6 @@ func newTestEnvWithResolver(
|
||||
|
||||
brk := broker.New()
|
||||
|
||||
var listenConfig net.ListenConfig
|
||||
|
||||
listener, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0")
|
||||
if err != nil {
|
||||
t.Fatalf("listen: %v", err)
|
||||
}
|
||||
|
||||
addr := listener.Addr().String()
|
||||
|
||||
err = listener.Close()
|
||||
if err != nil {
|
||||
t.Fatalf("close listener: %v", err)
|
||||
}
|
||||
|
||||
log := slog.New(slog.NewTextHandler(
|
||||
os.Stderr,
|
||||
&slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct
|
||||
@@ -160,7 +146,9 @@ func newTestEnvWithResolver(
|
||||
log, cfg, database, brk, interval, deadline, resolve,
|
||||
)
|
||||
|
||||
err = srv.Start(addr)
|
||||
// Port 0 makes the system pick a free port. dial reads it
|
||||
// back from the server's listener.
|
||||
err = srv.Start("127.0.0.1:0")
|
||||
if err != nil {
|
||||
t.Fatalf("start irc server: %v", err)
|
||||
}
|
||||
|
||||
@@ -15,6 +15,12 @@ func NewTestSPAServer() *Server {
|
||||
return srv
|
||||
}
|
||||
|
||||
// Listener returns the listener the start hook bound for the
|
||||
// HTTP server.
|
||||
func (srv *Server) Listener() net.Listener {
|
||||
return srv.listener
|
||||
}
|
||||
|
||||
// ServeUntilShutdown runs serveUntilShutdown, which serves the
|
||||
// HTTP server on listener, and returns when it does.
|
||||
func (srv *Server) ServeUntilShutdown(listener net.Listener) {
|
||||
|
||||
@@ -50,6 +50,7 @@ type Server struct {
|
||||
sentryEnabled bool
|
||||
log *slog.Logger
|
||||
httpServer *http.Server
|
||||
listener net.Listener
|
||||
router *chi.Mux
|
||||
params Params
|
||||
mw *middleware.Middleware
|
||||
@@ -179,6 +180,8 @@ func (srv *Server) startHTTP(ctx context.Context) error {
|
||||
return fmt.Errorf("http listen: %w", err)
|
||||
}
|
||||
|
||||
srv.listener = listener
|
||||
|
||||
srv.httpServer = &http.Server{ //nolint:exhaustruct // optional fields
|
||||
Addr: listenAddr,
|
||||
ReadTimeout: httpReadTimeout,
|
||||
|
||||
@@ -45,10 +45,9 @@ const (
|
||||
// for a long poll the HTTP server is serving to finish, and
|
||||
// that the poll gets its response.
|
||||
func TestStopWaitsForRequestInProgress(t *testing.T) {
|
||||
port := freePort(t)
|
||||
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(port))
|
||||
var srv *server.Server
|
||||
|
||||
app, brk, database := startApp(t, port)
|
||||
app, brk, database := startApp(t, fx.Populate(&srv))
|
||||
|
||||
sessionID, _, token, err := database.CreateSession(
|
||||
t.Context(), "poller", "poller", "", "",
|
||||
@@ -57,9 +56,12 @@ func TestStopWaitsForRequestInProgress(t *testing.T) {
|
||||
t.Fatalf("create session: %v", err)
|
||||
}
|
||||
|
||||
waitUntil(t, "the HTTP server to listen", func() bool {
|
||||
return canConnect(t, addr)
|
||||
})
|
||||
tcpAddr, ok := srv.Listener().Addr().(*net.TCPAddr)
|
||||
if !ok {
|
||||
t.Fatalf("listener address %v is not TCP", srv.Listener().Addr())
|
||||
}
|
||||
|
||||
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(tcpAddr.Port))
|
||||
|
||||
statuses := make(chan int, 1)
|
||||
|
||||
@@ -145,7 +147,7 @@ func TestStartFailsWhenHTTPPortIsTaken(t *testing.T) {
|
||||
func TestServeFailureStopsApp(t *testing.T) {
|
||||
var srv *server.Server
|
||||
|
||||
app, _, _ := startApp(t, freePort(t), fx.Populate(&srv))
|
||||
app, _, _ := startApp(t, fx.Populate(&srv))
|
||||
|
||||
shutdowns := app.Wait()
|
||||
|
||||
@@ -168,7 +170,7 @@ func TestServeFailureStopsApp(t *testing.T) {
|
||||
func TestServeAfterStopDoesNotStopApp(t *testing.T) {
|
||||
var srv *server.Server
|
||||
|
||||
app, _, _ := startApp(t, freePort(t), fx.Populate(&srv))
|
||||
app, _, _ := startApp(t, fx.Populate(&srv))
|
||||
|
||||
shutdowns := app.Wait()
|
||||
|
||||
@@ -186,16 +188,15 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// startApp starts the app newApp builds, with the HTTP server
|
||||
// listening on port.
|
||||
// startApp starts the app newApp builds, with the HTTP port
|
||||
// set to 0, so the system gives the server a free port.
|
||||
func startApp(
|
||||
t *testing.T,
|
||||
port int,
|
||||
options ...fx.Option,
|
||||
) (*fxtest.App, *broker.Broker, *db.Database) {
|
||||
t.Helper()
|
||||
|
||||
app, brk, database := newApp(t, port, options...)
|
||||
app, brk, database := newApp(t, 0, options...)
|
||||
|
||||
app.RequireStart()
|
||||
|
||||
@@ -254,21 +255,6 @@ func newApp(
|
||||
return app, brk, database
|
||||
}
|
||||
|
||||
// freePort returns a TCP port that nothing was listening on a
|
||||
// moment ago.
|
||||
func freePort(t *testing.T) int {
|
||||
t.Helper()
|
||||
|
||||
listener := closedListener(t)
|
||||
|
||||
tcpAddr, ok := listener.Addr().(*net.TCPAddr)
|
||||
if !ok {
|
||||
t.Fatalf("listener address %v is not TCP", listener.Addr())
|
||||
}
|
||||
|
||||
return tcpAddr.Port
|
||||
}
|
||||
|
||||
// closedListener returns a TCP listener that has already been
|
||||
// closed, so that accepting a connection on it fails.
|
||||
func closedListener(t *testing.T) net.Listener {
|
||||
@@ -291,22 +277,6 @@ func closedListener(t *testing.T) net.Listener {
|
||||
return listener
|
||||
}
|
||||
|
||||
// canConnect reports whether addr accepts a TCP connection.
|
||||
func canConnect(t *testing.T, addr string) bool {
|
||||
t.Helper()
|
||||
|
||||
var dialer net.Dialer
|
||||
|
||||
conn, err := dialer.DialContext(t.Context(), "tcp", addr)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
|
||||
_ = conn.Close()
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// waitUntil calls done every checkStep until it returns true,
|
||||
// and fails the test if it does not within testTimeout.
|
||||
func waitUntil(t *testing.T, what string, done func() bool) {
|
||||
|
||||
Reference in New Issue
Block a user