Add golangci-lint and fix the two real bugs it found

.github/workflows/ci.yml

@@ -37,12 +37,25 @@ jobs:
             | bash -s -- --to "$HOME/.local/bin"
           echo "$HOME/.local/bin" >> "$GITHUB_PATH"
 
+      # install-only: the action installs and caches the binary, but `just
+      # check-ci` is what runs it — CI must exercise the same recipe a
+      # developer does, or the two drift.
+      #
+      # This version is the one that gates merges. Homebrew tracks latest, so a
+      # local golangci-lint can be newer; when they disagree, this pin wins and
+      # the Brewfile says how to match it locally.
+      - uses: golangci/golangci-lint-action@v9
+        with:
+          version: v2.12.2
+          install-only: true
+
       - name: Install web dependencies
         working-directory: web
         run: npm ci
 
-      # Verify-only: fails if gofmt, oxfmt, oxlint, the type-checker, or
-      # `go mod tidy -diff` would change anything. Writes nothing.
+      # Verify-only: fails if gofmt, golangci-lint, oxfmt, oxlint, the
+      # type-checker, or `go mod tidy -diff` would change anything or object.
+      # Writes nothing.
       - name: Check (format, lint, types, module graph)
         run: just check-ci
 

.golangci.yml

@@ -0,0 +1,62 @@
+# golangci-lint configuration.
+#
+# Selection principle: every linter here must catch a class of bug this
+# codebase can actually produce. A linter that mostly fires on style gets
+# suppressed within a week, and suppressed linters are worse than absent ones
+# because they train people to add `//nolint` reflexively.
+#
+# Formatting is deliberately NOT here — `just check` runs gofmt directly, and
+# two tools rewriting the same files is a way to get a merge conflict with
+# yourself.
+version: "2"
+
+linters:
+  default: standard # errcheck, govet, ineffassign, staticcheck, unused
+  enable:
+    # Raw SQL everywhere: a result set whose iteration stopped early looks
+    # identical to a complete one unless rows.Err() is checked.
+    - rowserrcheck
+    - sqlclosecheck
+    # The platform leans hard on errors.Is/As for control flow (ErrStaleLease,
+    # ErrNotWaiting, ErrBudgetExceeded). A `==` comparison or a `%v` instead of
+    # `%w` silently breaks that, and the failure mode is a run that should have
+    # been fenced and wasn't.
+    - errorlint
+    # HTTP clients in the CLI, the model adapters, and the e2e harness.
+    - bodyclose
+    # User-facing strings are a product surface here (R-AGT-2), so typos in
+    # them are product bugs, not cosmetics.
+    - misspell
+    # Any suppression must say why. Keeps the escape hatch honest.
+    - nolintlint
+
+  settings:
+    errcheck:
+      # Cleanup calls whose error is genuinely unactionable. Everything not
+      # listed must be handled or explicitly assigned to `_` at the call site,
+      # which makes "I meant to ignore this" visible in review.
+      exclude-functions:
+        - (*database/sql.Tx).Rollback # the original error is already being returned
+        - (io.Closer).Close
+        - (*database/sql.Rows).Close
+        - (*os.File).Close
+        - (io.ReadCloser).Close
+    nolintlint:
+      require-explanation: true
+      require-specific: true
+      allow-unused: false
+
+  exclusions:
+    generated: lax
+    rules:
+      # Tests assert on outcomes, not on the error of every fixture call;
+      # requiring it there buys noise, not safety.
+      - path: _test\.go
+        linters: [errcheck, bodyclose]
+      # flag.ExitOnError means Parse can only return nil — it exits otherwise.
+      - path: cmd/creo/main\.go
+        text: "Error return value of `fs.Parse` is not checked"
+        linters: [errcheck]
+      # Writing to stdout/stderr in a CLI: nothing useful to do if it fails.
+      - text: "Error return value of `fmt.Fprint(f|ln)?` is not checked"
+        linters: [errcheck]

AGENTS.md

@@ -32,6 +32,13 @@ spikes/              throwaway experiment code; never imported by the core
 scripts/             demo and operational scripts
 ```
 
+## Getting set up (macOS)
+
+`brew bundle` installs the toolchain: Go, `just`, Node 24, golangci-lint, `gh`.
+The web client's own toolchain (vite-plus, vitest) is pinned in
+`web/package.json` and comes from `npm ci` — deliberately not duplicated in the
+Brewfile.
+
 ## Canonical commands
 
 `justfile` is the task runner — `just` on its own lists every recipe. It wraps

Brewfile

@@ -0,0 +1,40 @@
+# Developer tooling for Creo on macOS: `brew bundle` installs everything
+# needed to run `just check`, `just test-full`, and `just run`.
+#
+# Not listed here on purpose: the web client's toolchain (vite-plus, vitest,
+# jsdom) is pinned in web/package.json and installed by `npm ci`. Pinning a
+# JS dependency twice — once in a lockfile, once in a Brewfile — is a way to
+# have two answers to the same question.
+
+# The Go toolchain. The exact version CI uses is read from go.mod, so keep
+# this new enough to satisfy it (`go version` vs the `go` line in go.mod).
+brew "go"
+
+# Task runner. `just` on its own lists every recipe.
+brew "just"
+
+# Node for the web client, pinned to the major CI uses. Plain `node` tracks
+# latest (26 at the time of writing) — running the client on a different major
+# than CI is a divergence you only discover when the build breaks there.
+#
+# This is a keg-only formula: `brew install node@24` does not put it on PATH.
+# Either link it (`brew link --overwrite --force node@24`) or let the vite-plus
+# runtime provide node, which is what happens on this machine today — `node`
+# resolves to ~/.vite-plus/bin/node before anything Homebrew installs.
+brew "node@24"
+
+# Linter, wired into `just check` and `just check-ci`.
+#
+# Version note: CI pins v2.12.2 via golangci-lint-action. Homebrew tracks
+# latest, so the two can drift and a new release can fail CI on code that
+# linted clean locally. If that happens, pin locally to match CI:
+#   go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
+brew "golangci-lint"
+
+# GitHub CLI — PR creation and CI status from the terminal.
+brew "gh"
+
+# Optional: one e2e test reads the server's SQLite file directly to assert
+# that model usage was metered. Without it that single assertion skips; the
+# rest of the suite is unaffected.
+brew "sqlite"

cmd/creo/main.go

@@ -124,7 +124,7 @@ func cmdTenant(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		var limit *int64
 		if *daily > 0 {
 			limit = daily
@@ -146,7 +146,7 @@ func cmdTenant(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		tenants, err := tenant.New(db).List(context.Background())
 		if err != nil {
 			return err
@@ -187,7 +187,7 @@ func cmdToken(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		plaintext, id, err := tenant.New(db).CreateToken(context.Background(), tenantID, *name)
 		if err != nil {
 			return err
@@ -205,7 +205,7 @@ func cmdToken(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		if err := tenant.New(db).RevokeToken(context.Background(), tokenID); err != nil {
 			return err
 		}
@@ -238,7 +238,7 @@ func cmdAccount(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		u, err := identity.CreateUser(context.Background(), db, *tenantID, name, *color)
 		if err != nil {
 			return err
@@ -251,7 +251,7 @@ func cmdAccount(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		users, err := identity.ListUsers(context.Background(), db, *tenantID)
 		if err != nil {
 			return err
@@ -274,7 +274,7 @@ func cmdAccount(args []string) error {
 		if err != nil {
 			return err
 		}
-		defer db.Close()
+		defer func() { _ = db.Close() }()
 		if err := identity.DisableUser(context.Background(), db, userID); err != nil {
 			return err
 		}
@@ -611,7 +611,8 @@ func call(method, url string, body any, headers map[string]string, out any) erro
 		var e struct {
 			Error string `json:"error"`
 		}
-		json.NewDecoder(resp.Body).Decode(&e)
+		// Best-effort: a non-JSON error body still leaves the status worth reporting.
+		_ = json.NewDecoder(resp.Body).Decode(&e)
 		return fmt.Errorf("%s: %s", resp.Status, e.Error)
 	}
 	if out != nil {

internal/api/api.go

@@ -180,7 +180,12 @@ func (a *API) loginComplete(w http.ResponseWriter, r *http.Request) {
 
 func (a *API) logout(w http.ResponseWriter, r *http.Request) {
 	if c, err := r.Cookie(SessionCookie); err == nil && c.Value != "" {
-		a.Identity.RevokeSession(r.Context(), c.Value)
+		// A failed revoke leaves the session usable server-side while the
+		// browser believes it signed out — the one failure here worth an
+		// operator's attention.
+		if err := a.Identity.RevokeSession(r.Context(), c.Value); err != nil {
+			slog.Error("sign-out did not revoke the session", "err", err)
+		}
 	}
 	http.SetCookie(w, &http.Cookie{
 		Name: SessionCookie, Value: "", Path: "/", MaxAge: -1, HttpOnly: true, SameSite: http.SameSiteLaxMode,
@@ -247,7 +252,8 @@ func (a *API) publishProject(w http.ResponseWriter, r *http.Request) {
 	var body struct {
 		VersionID string `json:"versionId"`
 	}
-	json.NewDecoder(r.Body).Decode(&body)
+	// The body is optional here — no version means "the latest one".
+	_ = json.NewDecoder(r.Body).Decode(&body)
 	versionID, err := a.resolveVersion(r.Context(), projectID, body.VersionID)
 	if err != nil || versionID == "" {
 		httpError(w, http.StatusBadRequest, "There's nothing to put online yet — describe your site first.")
@@ -324,21 +330,36 @@ func (a *API) exportProject(w http.ResponseWriter, r *http.Request) {
 	}
 	w.Header().Set("Content-Type", "application/zip")
 	w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", projectID+".zip"))
+	// The response is already committed, so a mid-stream failure cannot become
+	// an HTTP error. What it must not do is finish quietly: zw.Close writes the
+	// central directory, so an abandoned export still yields a *valid* zip that
+	// is simply missing files. A user opening it would see a plausible,
+	// incomplete copy of their site and never know (AC-15).
 	zw := zip.NewWriter(w)
-	defer zw.Close()
 	for _, f := range files {
-		blob, err := a.Projects.Open(f.BlobSHA)
-		if err != nil {
-			return
-		}
-		fw, err := zw.Create(f.Path)
-		if err != nil {
-			blob.Close()
-			return
+		if err := copyBlobInto(zw, a, f.Path, f.BlobSHA); err != nil {
+			slog.Error("export truncated", "project", projectID, "version", versionID,
+				"path", f.Path, "err", err)
+			return // deliberately no zw.Close(): a torn stream beats a tidy lie
 		}
-		io.Copy(fw, blob)
-		blob.Close()
 	}
+	if err := zw.Close(); err != nil {
+		slog.Error("export could not be finalised", "project", projectID, "version", versionID, "err", err)
+	}
+}
+
+func copyBlobInto(zw *zip.Writer, a *API, path, sha string) error {
+	blob, err := a.Projects.Open(sha)
+	if err != nil {
+		return err
+	}
+	defer blob.Close()
+	fw, err := zw.Create(path)
+	if err != nil {
+		return err
+	}
+	_, err = io.Copy(fw, blob)
+	return err
 }
 
 func (a *API) appendProjectEvent(ctx context.Context, projectID, typ, userText, eventActor string, detail any) {
@@ -588,7 +609,7 @@ func (a *API) waitingRun(ctx context.Context, sessionID string) (runID, toolID s
 		return "", "", false
 	}
 	var d harness.InputRequestDetail
-	json.Unmarshal(evs[len(evs)-1].Detail, &d)
+	_ = json.Unmarshal(evs[len(evs)-1].Detail, &d) // our own payload; a zero value degrades gracefully
 	return runID, d.ToolID, true
 }
 
@@ -701,7 +722,7 @@ func (a *API) getSession(w http.ResponseWriter, r *http.Request) {
 	if _, _, ok := a.waitingRun(r.Context(), sessionID); ok {
 		if evs, err := a.Log.Read(r.Context(), sessionID, 0, []string{harness.EvInputRequested}); err == nil && len(evs) > 0 {
 			var d harness.InputRequestDetail
-			json.Unmarshal(evs[len(evs)-1].Detail, &d)
+			_ = json.Unmarshal(evs[len(evs)-1].Detail, &d) // our own payload; a zero value degrades gracefully
 			out["question"] = map[string]any{"text": d.Question, "choices": d.Choices}
 		}
 	}
@@ -784,7 +805,8 @@ func (a *API) streamEvents(w http.ResponseWriter, r *http.Request) {
 func writeJSON(w http.ResponseWriter, status int, v any) {
 	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(status)
-	json.NewEncoder(w).Encode(v)
+	// The status is already sent; a write failure here has no recovery path.
+	_ = json.NewEncoder(w).Encode(v)
 }
 
 func httpError(w http.ResponseWriter, status int, msg string) {

internal/harness/harness.go

@@ -251,7 +251,8 @@ func (h *Harness) emitQuestion(ctx context.Context, r *run.Run, lease *eventlog.
 		Choices  []string `json:"choices"`
 	}
 	if len(call.ToolInput) > 0 {
-		json.Unmarshal(call.ToolInput, &in)
+		// A malformed question falls through to the default prompt below.
+		_ = json.Unmarshal(call.ToolInput, &in)
 	}
 	if strings.TrimSpace(in.Question) == "" {
 		in.Question = "Could you tell me a bit more about what you'd like?"
@@ -288,7 +289,7 @@ func (h *Harness) EmitFailure(ctx context.Context, r *run.Run, cause error) {
 	case errors.Is(cause, tenant.ErrStorageExceeded):
 		text = "There's no room left to save more changes. Your site is safe as it is — ask whoever runs this server for more space, or remove a few images to free some up."
 	}
-	h.Log.Append(ctx, r.SessionID, []eventlog.NewEvent{{
+	_, _ = h.Log.Append(ctx, r.SessionID, []eventlog.NewEvent{{
 		Type: EvRunFailed, RunID: r.ID,
 		UserText: text,
 		Detail:   map[string]string{"error": cause.Error()},
@@ -303,7 +304,7 @@ func toolPath(call model.Block) string {
 	var in struct {
 		Path string `json:"path"`
 	}
-	json.Unmarshal(call.ToolInput, &in)
+	_ = json.Unmarshal(call.ToolInput, &in) // absent path just means no progress phrase
 	return in.Path
 }
 

internal/identity/identity.go

@@ -210,7 +210,9 @@ func (s *Service) Authenticate(ctx context.Context, token string) (Principal, er
 	}
 	if time.Until(expires) < renewBelow {
 		newExp := time.Now().UTC().Add(SessionTTL).Format(time.RFC3339Nano)
-		s.db.Write(ctx, func(tx *sql.Tx) error {
+		// Best-effort: a failed roll-forward only means this session expires on
+		// its original schedule, and the next request tries again.
+		_ = s.db.Write(ctx, func(tx *sql.Tx) error {
 			_, err := tx.Exec(`UPDATE web_sessions SET expires_at = ? WHERE id = ? AND revoked_at IS NULL`, newExp, sessID)
 			return err
 		})

internal/model/anthropic.go

@@ -34,7 +34,7 @@ func (a *Anthropic) Complete(ctx context.Context, req Request) (*Completion, err
 		params.System = []anthropic.TextBlockParam{{Text: req.System}}
 	}
 	for _, t := range req.Tools {
-		properties, _ := t.InputSchema["properties"]
+		properties := t.InputSchema["properties"]
 		var required []string
 		if r, ok := t.InputSchema["required"].([]string); ok {
 			required = r