Add LLM coding instruction
Signed-off-by: Bryan Frimin <bryan@getprobo.com>
This commit is contained in:
Bryan Frimin
376
AGENTS.md
Normal file
376
AGENTS.md
Normal file
@@ -0,0 +1,376 @@
|
||||
# AGENTS.md
|
||||
|
||||
## Project
|
||||
|
||||
- Module: `go.probo.inc/probo`
|
||||
- Router: `github.com/go-chi/chi/v5`
|
||||
- Database: `go.gearno.de/kit/pg` — all raw SQL lives in `pkg/coredata`, never elsewhere
|
||||
- HTTP server: `go.gearno.de/kit/httpserver`
|
||||
- HTTP client: `go.gearno.de/kit/httpclient`
|
||||
- Logging: `go.gearno.de/kit/log`
|
||||
- Tracing: OpenTelemetry (`go.opentelemetry.io/otel`)
|
||||
- UUID: `go.gearno.de/crypto/uuid` (never use `github.com/google/uuid`)
|
||||
- Pointers: `go.gearno.de/x/ref` for pointer helpers (`ref.UnrefOrZero`, etc.)
|
||||
- Tests: `github.com/stretchr/testify` (`require` for fatal, `assert` for non-fatal)
|
||||
- Go version: 1.26 — use `new(expr)` to create pointers to values (e.g. `new(1)`, `new("foo")`, `new(time.Now())`) instead of helper functions or temporary variables
|
||||
|
||||
## Go Style
|
||||
|
||||
### Grouped declarations
|
||||
|
||||
Use `type ()`, `const ()`, and `var ()` blocks to group related declarations. Use explicit typed values for string enums, not `iota`.
|
||||
|
||||
```go
|
||||
type (
|
||||
CreateFooRequest struct {
|
||||
Name string
|
||||
Active bool
|
||||
}
|
||||
|
||||
UpdateFooRequest struct {
|
||||
ID gid.GID
|
||||
Name *string
|
||||
Active *bool
|
||||
}
|
||||
)
|
||||
|
||||
const (
|
||||
NameMaxLength = 100
|
||||
ContentMaxLength = 5000
|
||||
)
|
||||
|
||||
var (
|
||||
_ Reader = (*FileReader)(nil)
|
||||
_ Writer = (*FileWriter)(nil)
|
||||
)
|
||||
```
|
||||
|
||||
### One argument per line
|
||||
|
||||
A function call is either entirely on one line or fully expanded with one argument per line. Never mix the two styles.
|
||||
|
||||
```go
|
||||
// Good — short enough to fit on one line
|
||||
id := gid.New(tenantID, "Foo")
|
||||
|
||||
// Good — multiple arguments, one per line
|
||||
svc, err := foo.NewService(
|
||||
ctx,
|
||||
db,
|
||||
logger,
|
||||
foo.Config{
|
||||
Interval: 10 * time.Second,
|
||||
MaxRetry: 3,
|
||||
},
|
||||
)
|
||||
|
||||
// Bad — mixed inline and multiline
|
||||
svc, err := foo.NewService(ctx, db, logger, foo.Config{
|
||||
Interval: 10 * time.Second,
|
||||
})
|
||||
```
|
||||
|
||||
### License header
|
||||
|
||||
Every `.go` file starts with the ISC license header. Use the **current year** for new files. When editing an existing file whose header has a different year, update it to a range (e.g. `2025-2026`). Never overwrite the original year.
|
||||
|
||||
```
|
||||
// Copyright (c) <YEAR> Probo Inc <hello@getprobo.com>.
|
||||
//
|
||||
// Permission to use, copy, modify, and/or distribute this software for any
|
||||
// purpose with or without fee is hereby granted, provided that the above
|
||||
// copyright notice and this permission notice appear in all copies.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
|
||||
// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
|
||||
// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
|
||||
// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
|
||||
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
|
||||
// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
|
||||
// PERFORMANCE OF THIS SOFTWARE.
|
||||
```
|
||||
|
||||
### Import ordering
|
||||
|
||||
Two groups separated by a blank line: stdlib, then everything else (third-party and internal sorted together alphabetically).
|
||||
|
||||
```go
|
||||
import (
|
||||
"errors"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"go.gearno.de/kit/httpserver"
|
||||
"go.gearno.de/kit/log"
|
||||
"go.probo.inc/probo/pkg/iam"
|
||||
"go.probo.inc/probo/pkg/probo"
|
||||
"go.probo.inc/probo/pkg/trust"
|
||||
)
|
||||
```
|
||||
|
||||
### Receiver names
|
||||
|
||||
Short receivers: usually single-letter matching the type (`s` for Service, `c` for Client, `p` for Provider).
|
||||
|
||||
### Error handling
|
||||
|
||||
Wrap errors with `fmt.Errorf` using lowercase messages starting with `cannot`:
|
||||
|
||||
```go
|
||||
return nil, fmt.Errorf("cannot load trust center: %w", err)
|
||||
return nil, fmt.Errorf("cannot create SAML service: %w", err)
|
||||
```
|
||||
|
||||
Sentinel errors in grouped `var ()` blocks. Custom error types implement `Unwrap() error`. Use `errors.Is` / `errors.As` for checks.
|
||||
|
||||
### Naming
|
||||
|
||||
- Constructors: `New*` (e.g. `NewService`, `NewServer`, `NewBridge`)
|
||||
- Config structs: `*Config` suffix (e.g. `APIConfig`, `PgConfig`, `TrustCenterConfig`)
|
||||
- Request structs: `*Request` suffix (e.g. `UpdateTrustCenterRequest`)
|
||||
- Unexported types for internal data: lowercase (e.g. `vendorInfo`, `ctxKey`)
|
||||
|
||||
### Tests
|
||||
|
||||
Black-box test packages (`package foo_test`). White-box (`package foo`) only when testing unexported functions. Test names follow `TestFunctionName_Scenario`. Use `t.Run` for subtests with lowercase descriptive names. Table-driven tests for parameterized cases.
|
||||
|
||||
**Parallel tests:** Always call `t.Parallel()` at both the top-level test and each subtest.
|
||||
|
||||
**`require` vs `assert`:**
|
||||
- `require` — stops the test immediately on failure. Use for **preconditions** that would make subsequent assertions meaningless: `require.NoError`, `require.Error`, `require.ErrorAs`, `require.Len`, `require.NotNil`, `require.True` (as a guard).
|
||||
- `assert` — logs failure but continues the test. Use for the **actual values** being verified: `assert.Equal`, `assert.Contains`, `assert.True`, `assert.False`, `assert.Nil`.
|
||||
|
||||
Rule of thumb: if a failure would cause a nil-pointer panic or make every following assertion nonsensical, use `require`; otherwise use `assert`.
|
||||
|
||||
```go
|
||||
func TestRun_Handoff(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run(
|
||||
"handoff with custom tool name and description",
|
||||
func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// ... setup ...
|
||||
|
||||
result, err := triage.Run(
|
||||
context.Background(),
|
||||
[]llm.Message{
|
||||
userMessage("How much is my invoice?"),
|
||||
},
|
||||
)
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "Your invoice is $42.", result.FinalMessage().Text())
|
||||
assert.Equal(t, "billing", result.LastAgent.Name())
|
||||
},
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Helpers and mocks:** Define mock types and helper functions (e.g. `stopResponse`, `userMessage`) at the top of the test file, not inline in each test.
|
||||
|
||||
### Functional options and Config structs
|
||||
|
||||
Use `Config` structs when a constructor has many required parameters. Use functional options (`With*` functions) for optional configuration.
|
||||
|
||||
```go
|
||||
type Option func(*Bridge)
|
||||
|
||||
func WithDryRun(dryRun bool) Option {
|
||||
return func(s *Bridge) {
|
||||
s.dryRun = dryRun
|
||||
}
|
||||
}
|
||||
|
||||
func NewBridge(provider provider.Provider, client *scimclient.Client, opts ...Option) *Bridge {
|
||||
s := &Bridge{provider: provider, scimClient: client}
|
||||
for _, opt := range opts {
|
||||
opt(s)
|
||||
}
|
||||
return s
|
||||
}
|
||||
```
|
||||
|
||||
### Interfaces
|
||||
|
||||
Define interfaces in the consumer package. Keep them small. Verify satisfaction at compile time:
|
||||
|
||||
```go
|
||||
var (
|
||||
_ unit.Configurable = (*Implm)(nil)
|
||||
_ unit.Runnable = (*Implm)(nil)
|
||||
)
|
||||
```
|
||||
|
||||
### Context
|
||||
|
||||
Always first parameter. Private struct keys for context values:
|
||||
|
||||
```go
|
||||
type ctxKey struct{ name string }
|
||||
var trustCenterIDKey = &ctxKey{name: "trust_center_id"}
|
||||
```
|
||||
|
||||
### Logging
|
||||
|
||||
Named, context-aware structured logging with typed fields. **Never log PII, PHI, or other sensitive data** (e.g. emails, names, passwords, tokens, health records). Log opaque identifiers (IDs, request IDs) instead.
|
||||
|
||||
```go
|
||||
l.InfoCtx(ctx, "HTTP request to trust center custom domain, redirecting to HTTPS",
|
||||
log.String("domain", domain),
|
||||
log.String("path", r.URL.Path),
|
||||
log.String("to", httpsURL),
|
||||
)
|
||||
```
|
||||
|
||||
### Service `Run()` orchestration
|
||||
|
||||
A top-level `Run` method starts child subsystems (workers, servers) as goroutines via `sync.WaitGroup.Go`. Each child gets its own cancellable context created with `context.WithCancel(context.WithoutCancel(ctx))` so that a parent cancellation does not kill in-flight work — the parent explicitly calls each `stop*` function and then `wg.Wait()` for a controlled shutdown.
|
||||
|
||||
When a child crashes, it calls `cancel(fmt.Errorf("… crashed: %w", err))` to signal the parent.
|
||||
|
||||
```go
|
||||
func (impl *Implm) Run(ctx context.Context, l *log.Logger) error {
|
||||
wg := sync.WaitGroup{}
|
||||
ctx, cancel := context.WithCancelCause(ctx)
|
||||
defer cancel(context.Canceled)
|
||||
|
||||
// Start a worker
|
||||
workerCtx, stopWorker := context.WithCancel(context.WithoutCancel(ctx))
|
||||
worker := NewFooWorker(pgClient, l.Named("foo-worker"))
|
||||
wg.Go(
|
||||
func() {
|
||||
if err := worker.Run(workerCtx); err != nil {
|
||||
cancel(fmt.Errorf("foo worker crashed: %w", err))
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
// Start a server
|
||||
serverCtx, stopServer := context.WithCancel(context.WithoutCancel(ctx))
|
||||
defer stopServer()
|
||||
wg.Go(
|
||||
func() {
|
||||
if err := impl.runServer(serverCtx, l); err != nil {
|
||||
cancel(fmt.Errorf("server crashed: %w", err))
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
<-ctx.Done()
|
||||
|
||||
stopServer()
|
||||
stopWorker()
|
||||
|
||||
wg.Wait()
|
||||
|
||||
return context.Cause(ctx)
|
||||
}
|
||||
```
|
||||
|
||||
### Workers
|
||||
|
||||
Background workers follow a poll-based pattern with bounded concurrency. The struct holds a `*pg.Client`, a `*log.Logger`, and tuning knobs (`interval`, `staleAfter`, `maxConcurrency`). Use functional options (`With*` functions) for the tuning knobs with sensible defaults.
|
||||
|
||||
The `Run(ctx context.Context) error` method loops with a `select` on `ctx.Done()` and `time.After(interval)`. On each tick it recovers stale rows, then drains available work via `processNext`. Work items are claimed inside a transaction with `FOR UPDATE SKIP LOCKED`, marked as processing, then handled concurrently in goroutines bounded by a semaphore channel. Use `context.WithoutCancel` for work that must complete even after shutdown, and `sync.WaitGroup` with `defer wg.Wait()` to ensure in-flight goroutines finish before `Run` returns.
|
||||
|
||||
```go
|
||||
type (
|
||||
FooWorker struct {
|
||||
pg *pg.Client
|
||||
logger *log.Logger
|
||||
interval time.Duration
|
||||
staleAfter time.Duration
|
||||
maxConcurrency int
|
||||
}
|
||||
|
||||
FooWorkerOption func(*FooWorker)
|
||||
)
|
||||
|
||||
func NewFooWorker(
|
||||
pgClient *pg.Client,
|
||||
logger *log.Logger,
|
||||
opts ...FooWorkerOption,
|
||||
) *FooWorker {
|
||||
w := &FooWorker{
|
||||
pg: pgClient,
|
||||
logger: logger,
|
||||
interval: 10 * time.Second,
|
||||
staleAfter: 5 * time.Minute,
|
||||
maxConcurrency: 5,
|
||||
}
|
||||
for _, opt := range opts {
|
||||
opt(w)
|
||||
}
|
||||
return w
|
||||
}
|
||||
|
||||
func (w *FooWorker) Run(ctx context.Context) error {
|
||||
var (
|
||||
wg sync.WaitGroup
|
||||
sem = make(chan struct{}, w.maxConcurrency)
|
||||
)
|
||||
defer wg.Wait()
|
||||
|
||||
LOOP:
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
case <-time.After(w.interval):
|
||||
nonCancelableCtx := context.WithoutCancel(ctx)
|
||||
w.recoverStaleRows(nonCancelableCtx)
|
||||
for {
|
||||
if err := w.processNext(ctx, sem, &wg); err != nil {
|
||||
if !errors.Is(err, coredata.ErrResourceNotFound) {
|
||||
w.logger.ErrorCtx(nonCancelableCtx, "cannot claim item", log.Error(err))
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
goto LOOP
|
||||
}
|
||||
}
|
||||
|
||||
func (w *FooWorker) processNext(ctx context.Context, sem chan struct{}, wg *sync.WaitGroup) error {
|
||||
select {
|
||||
case sem <- struct{}{}:
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
var (
|
||||
item coredata.FooItem
|
||||
now = time.Now()
|
||||
nonCancelableCtx = context.WithoutCancel(ctx)
|
||||
)
|
||||
|
||||
if err := w.pg.WithTx(
|
||||
nonCancelableCtx,
|
||||
func(tx pg.Conn) error {
|
||||
if err := item.LoadNextPendingForUpdateSkipLocked(nonCancelableCtx, tx); err != nil {
|
||||
return err
|
||||
}
|
||||
item.Status = coredata.FooStatusProcessing
|
||||
item.UpdatedAt = now
|
||||
return item.Update(nonCancelableCtx, tx, coredata.NewNoScope())
|
||||
},
|
||||
); err != nil {
|
||||
<-sem
|
||||
return err
|
||||
}
|
||||
|
||||
wg.Add(1)
|
||||
go func(item coredata.FooItem) {
|
||||
defer wg.Done()
|
||||
defer func() { <-sem }()
|
||||
|
||||
if err := w.handle(nonCancelableCtx, &item); err != nil {
|
||||
w.logger.ErrorCtx(nonCancelableCtx, "cannot process item", log.Error(err))
|
||||
}
|
||||
}(item)
|
||||
|
||||
return nil
|
||||
}
|
||||
```
|
||||
Reference in New Issue
Block a user