Coding Style — infrabroker¶

Rules enforced in this project. Every rule has a rationale and a mechanical check so "does this pass?" has an unambiguous answer.

1. Formatting — `gofmt` is non-negotiable¶

gofmt -l .          # must print nothing
gofmt -w .          # apply in-place before committing

gofmt is the Go community standard. Struct tag alignment, comment spacing, and indentation are handled by the tool; do not fight it manually. If a diff touches only whitespace, run gofmt -w before opening a PR.

What gofmt does NOT cover (see rules below for those): - Import grouping - Comment style - Function length - Error wrapping

2. Import grouping — three blocks, always¶

Imports are separated into three groups with a blank line between each:

import (
    // 1. Standard library
    "context"
    "fmt"
    "net/http"

    // 2. Third-party modules
    "golang.org/x/crypto/ssh"
    "github.com/modelcontextprotocol/go-sdk/mcp"

    // 3. Internal packages (same module)
    "github.com/luisgf/infrabroker/internal/audit"
    "github.com/luisgf/infrabroker/internal/signer"
)

goimports enforces this automatically. Run it as a pre-commit check if available; otherwise enforce it in code review.

3. Error handling¶

3a. Wrap with `%w` when there is an underlying error¶

// Correct — caller can use errors.Is / errors.As
return nil, fmt.Errorf("parsear certificado: %w", err)

// Wrong — loses the underlying error type
return nil, fmt.Errorf("parsear certificado: %v", err)

Use %w whenever you have an err value from a previous call. Use errors.New or fmt.Errorf (without %w) only when creating a new error with no underlying cause.

3b. Never silence errors with `_ =` in HTTP handlers¶

The pattern _ = json.NewEncoder(w).Encode(v) hides write failures. Use the writeJSON helper that logs on error:

func writeJSON(w http.ResponseWriter, status int, v any) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(status)
    if err := json.NewEncoder(w).Encode(v); err != nil {
        log.Printf("writeJSON: %v", err)
    }
}

3c. No panics in production code¶

panic is reserved for programmer errors caught at init time (e.g. invalid regex in a var block). Never panic in request-handling code.

4. `context.Context` — first parameter, always¶

Any function that performs network I/O, calls an external service, or may block must accept ctx context.Context as its first parameter:

// Correct
func (r *Remote) SignIntent(ctx context.Context, in Intent) (*Issued, error)
func (e *Engine) Execute(ctx context.Context, ...) (*Result, error)

// Wrong — no way to cancel from the caller
func (r *Remote) SignIntent(in Intent) (*Issued, error)

Pass context.Background() only in: - Long-lived background goroutines (e.g. host-refresh ticker) - main() startup paths

Never store a context in a struct field.

Nil-interface pitfall¶

When a function returns a concrete pointer type (*T) that may be nil, and the caller stores it in an interface field, assign the interface type directly to avoid the non-nil interface with nil value trap:

// Wrong — e.fetcher != nil is always true even when r == nil
func buildSigner(...) (signer.Signer, *signer.Remote, error) { ... }
e.fetcher = fetcher  // *signer.Remote(nil) stored as hostFetcher interface ≠ nil

// Correct — nil stays nil
func buildSigner(...) (signer.Signer, hostFetcher, error) { ... }

5. Interfaces — small and defined by the consumer¶

Prefer single-method interfaces (Signer, Notifier, hostFetcher).
Define interfaces in the package that consumes them, not the one that implements them.
Avoid interface pollution: only define an interface when you have (or anticipate) more than one implementation, or when you need to break a dependency cycle.

6. Function length — 80 lines max¶

No function body should exceed 80 lines (blank lines and comments included). When a function grows past this limit:

Identify a cohesive sub-task within the function.
Extract it into a named helper with a descriptive name.
The helper must be independently testable or at least independently readable.

Check with:

awk '/^func /{if(fname!="" && lines>80) printf "%s: %s (%d lines)\n",FILENAME,fname,lines
             fname=$0; fstart=NR; lines=0} {lines++}
     END     {if(fname!="" && lines>80) printf "%s: %s (%d lines)\n",FILENAME,fname,lines
}' $(find . -name "*.go" -not -name "*_test.go" -not -path "*/vendor/*")

Allowed exceptions: main() startup functions and generated code.

7. Tests¶

7a. `t.Parallel()` in pure unit tests¶

Every func TestXxx(t *testing.T) that does not share global state, open real network connections, or write to disk must call t.Parallel() as its first statement:

func TestRegistryCreateAndApprove(t *testing.T) {
    t.Parallel()
    // ...
}

This catches data races earlier and speeds up the test suite.

Exclude from parallelisation: - Tests that spin up real HTTP servers (httptest.NewServer) with shared state - Tests that write to the same temp directory - Integration tests in cmd/ that coordinate multiple components

7b. Race detector in CI¶

All test runs must pass with -race:

go test -race ./...

This is the gate that confirms concurrency correctness. A test that only passes without -race is a broken test.

7c. Table-driven tests for exhaustive cases¶

When testing more than three variants of the same behaviour, use a table-driven pattern:

tests := []struct {
    name    string
    input   string
    want    bool
}{
    {"empty", "", false},
    {"valid", "deploy", true},
    {"with-flag", "--rm", false},
}
for _, tc := range tests {
    tc := tc // capture range variable
    t.Run(tc.name, func(t *testing.T) {
        t.Parallel()
        got := sudoUserAllowed(nil, tc.input)
        if got != tc.want {
            t.Errorf("got %v, want %v", got, tc.want)
        }
    })
}

8. Naming¶

Thing	Convention	Example
Exported type	PascalCase noun	`PolicyTable`, `WireRequest`
Unexported type	camelCase noun	`liveSession`, `hostFetcher`
Interface	Noun or agent noun	`Signer`, `Notifier`, `hostFetcher`
Constructor	`New` prefix	`NewRegistry`, `NewEngine`
Boolean field/var	Positive assertion	`AllowSudo`, `IsError`
Error variable	`Err` prefix (sentinel)	`ErrUnknownHost`
Context parameter	Always `ctx`	`func f(ctx context.Context, ...)`
HTTP handler	verb + noun suffix	`handleSign`, `handleHosts`

Avoid stutter: signer.SignerConfig → signer.Config. Avoid generic names (data, info, result) unless the scope is tiny (< 5 lines).

9. Concurrency¶

9a. Keep mutexes unexported and close to the data they protect¶

type Registry struct {
    mu    sync.Mutex        // protects items
    items map[string]*Approval
    ttl   time.Duration
}

Never export a mutex. Never embed a mutex in a type that is copied by value.

9b. Prefer `sync.RWMutex` for read-heavy caches¶

Engine.mu and server.mu protect caches that are read on every request but written only during periodic reload. RWMutex avoids contention on the read path.

9c. Goroutines must have a clear lifetime¶

Every go func() must have an obvious termination condition (context cancellation, channel close, or ticker stop). Document it with a comment if it is not obvious:

// Runs until the ticker is stopped in Close().
go func() {
    t := time.NewTicker(interval)
    defer t.Stop()
    for range t.C { ... }
}()

10. Language¶

All source code must be in English. No exceptions for legacy code.

Artifact	Language
Commit messages	English
Go comments (all files, new and existing)	English
`*.md` docs (README, API, USAGE, CHANGELOG, ARCHITECTURE, OPERATIONS, THREAT_MODEL, SECURITY, CONTRIBUTING)	English
`HANDOFF.md`	Spanish (internal session-handoff document)

The rule is: do not mix languages within a single function or doc block. When editing a legacy function that still has Spanish comments, translate them in the same commit.

Quick reference checklist¶

Before opening a PR or pushing to main — make verify runs the mechanical checks (fmt, vet, build, race tests, and the docs anti-drift gate) in one shot:

[ ] gofmt -l . → no output
[ ] go vet ./... → no output
[ ] go test -race ./... → all pass
[ ] No function body > 80 lines (run awk check above)
[ ] All new I/O functions accept ctx context.Context as first param
[ ] No _ = json.NewEncoder(w).Encode(...) in handlers
[ ] New Test functions call t.Parallel() (if applicable)
[ ] Imports in three groups: stdlib / third-party / internal
[ ] fmt.Errorf uses %w when wrapping an existing error
[ ] CHANGELOG.md updated (see workflow in CONTRIBUTING.md)