From 1079e5f5d99a1305449803a5ffea1f7761a66b6d Mon Sep 17 00:00:00 2001 From: korya <148461+korya@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:54:44 -0400 Subject: [PATCH 1/4] chore: add golangci-lint to check and check-ci MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enables the standard set (errcheck, govet, ineffassign, staticcheck, unused) plus five chosen because they catch classes of bug this codebase can actually produce: rowserrcheck and sqlclosecheck because it runs raw SQL and a partially-iterated result set looks identical to a complete one; errorlint because control flow leans on errors.Is and a stray `==` would silently unfence a run; bodyclose for the HTTP clients; misspell because user-facing strings are a product surface here. Style linters are deliberately absent. A linter that mostly fires on taste gets suppressed within a week, and that teaches people to reach for //nolint reflexively — so nolintlint is on and demands a reason. The version is pinned and CI installs the same one via `just install-lint`, so a lint failure on a PR always reproduces locally. Formatting stays with gofmt in the justfile rather than moving into golangci-lint's formatters: two tools rewriting the same files is a way to conflict with yourself. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CiRDTC3E7HGD6Cbq8QeFXM --- .github/workflows/ci.yml | 13 +++++++-- .golangci.yml | 62 ++++++++++++++++++++++++++++++++++++++++ justfile | 29 +++++++++++++++++-- 3 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ed57a1..85f5deb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,12 +37,21 @@ jobs: | bash -s -- --to "$HOME/.local/bin" echo "$HOME/.local/bin" >> "$GITHUB_PATH" + # Same pinned version the justfile installs, so a lint failure here is + # always reproducible with `just check` locally. PATH is set explicitly + # rather than assuming setup-go exports GOPATH/bin. + - name: Install golangci-lint + run: | + just install-lint + echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" + - 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 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..9b1711d --- /dev/null +++ b/.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] diff --git a/justfile b/justfile index 8518651..694c96f 100644 --- a/justfile +++ b/justfile @@ -11,6 +11,9 @@ set shell := ["bash", "-c"] data := "./data" +# Pinned so local and CI lint with identical rules; see `just install-lint`. +golangci_version := "2.12.2" + [doc("List available recipes")] default: @just --list @@ -61,13 +64,14 @@ check: check-go check-ts [doc("Verify everything without writing a file — the CI gate")] check-ci: check-go-ci check-ts-ci -[doc("Go: format, tidy, vet — fixing in place")] -check-go: +[doc("Go: format, tidy, vet, lint — fixing what it can in place")] +check-go: lint-go-fix gofmt -w . go mod tidy go vet ./... + golangci-lint run ./... -[doc("Go: verify only — fails if gofmt or go mod tidy would change anything")] +[doc("Go: verify only — fails if gofmt, tidy, vet, or golangci-lint object")] check-go-ci: #!/usr/bin/env bash # `go mod tidy -diff` (Go 1.23+) prints what tidy would change and exits @@ -82,6 +86,25 @@ check-go-ci: fi go vet ./... go mod tidy -diff + golangci-lint run ./... + +# Applies the autofixes golangci-lint can make on its own, so `check` stays a +# fix-in-place recipe. Findings it cannot fix are reported by check-go's own +# `golangci-lint run` immediately afterwards. +[private] +lint-go-fix: + golangci-lint run --fix ./... || true + +[doc("Install the pinned golangci-lint into GOPATH/bin (same version as CI)")] +install-lint: + #!/usr/bin/env bash + # Pinned: an unpinned linter turns someone else's release into your red CI. + # If a locally installed golangci-lint (e.g. from Homebrew) disagrees with + # CI, run this — the pin here is the version that gates merges. + set -euo pipefail + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh \ + | sh -s -- -b "$(go env GOPATH)/bin" v{{ golangci_version }} + echo "installed golangci-lint v{{ golangci_version }} to $(go env GOPATH)/bin" [doc("TypeScript: format, lint, type-check via Vite+ — fixing in place")] check-ts: From 6d73f39797e2e09c2bcaa22957110b5da3ed0e14 Mon Sep 17 00:00:00 2001 From: korya <148461+korya@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:55:04 -0400 Subject: [PATCH 2/4] fix: stop export and DB close from hiding failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real bugs the new linter surfaced. Export finalised the zip from a deferred Close and ignored both that error and every read error mid-stream. Because zip.Writer.Close writes the central directory, an abandoned export still produced a *valid* file — just missing pages. A user would open a plausible, incomplete copy of their site and never learn otherwise, which is precisely the failure AC-15 exists to prevent. The response is already committed by then, so this cannot become an HTTP error; it now logs and leaves the stream torn rather than tidily wrong. DB.Close discarded the read pool's error while returning the writer's, so a failed close reported success. Both are joined now. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CiRDTC3E7HGD6Cbq8QeFXM --- internal/api/api.go | 54 +++++++++++++++++++++++++++++------------ internal/store/store.go | 10 +++++--- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index f402408..96a8a98 100644 --- a/internal/api/api.go +++ b/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) { diff --git a/internal/store/store.go b/internal/store/store.go index bd99ed9..b9bca6f 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -6,6 +6,7 @@ import ( "context" "database/sql" "embed" + "errors" "fmt" "io/fs" "sort" @@ -30,13 +31,13 @@ func Open(path string) (*DB, error) { w.SetMaxOpenConns(1) r, err := sql.Open("sqlite", dsn) if err != nil { - w.Close() + _ = w.Close() return nil, err } r.SetMaxOpenConns(4) db := &DB{W: w, R: r} if err := db.migrate(); err != nil { - db.Close() + _ = db.Close() return nil, fmt.Errorf("migrate: %w", err) } return db, nil @@ -84,7 +85,8 @@ func (db *DB) Write(ctx context.Context, fn func(tx *sql.Tx) error) error { return tx.Commit() } +// Close shuts both pools down and reports either failure. Returning only the +// writer's error would let a failed read-pool close pass for success. func (db *DB) Close() error { - db.R.Close() - return db.W.Close() + return errors.Join(db.R.Close(), db.W.Close()) } From d50a7db154a1cd89f8d72847f5f9f8fb0ac258c1 Mon Sep 17 00:00:00 2001 From: korya <148461+korya@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:55:04 -0400 Subject: [PATCH 3/4] chore: surface or mark every ignored error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Errcheck found 51 unchecked returns. Three kinds, handled differently. Silent holes now log: a failed session revoke leaves someone signed in who believes they signed out; a failing recovery scan quietly stops rescuing abandoned runs; a failed Complete leaves a run stuck until its lease expires; a cut-short file serve means the content store is damaged and a visitor got half a page. Genuinely unactionable ones are now `_ =` at the call site with a reason where it isn't obvious — so "I meant to ignore this" is visible in review instead of indistinguishable from an oversight. Conventional no-ops (deferred Close, fmt.Fprint to a terminal, fs.Parse under ExitOnError) are excluded in config rather than annotated 50 times. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CiRDTC3E7HGD6Cbq8QeFXM --- cmd/creo/main.go | 17 +++++++++-------- internal/harness/harness.go | 7 ++++--- internal/identity/identity.go | 4 +++- internal/model/anthropic.go | 2 +- internal/server/server.go | 31 +++++++++++++++++++++---------- internal/serving/serving.go | 10 ++++++++-- 6 files changed, 46 insertions(+), 25 deletions(-) diff --git a/cmd/creo/main.go b/cmd/creo/main.go index 6b06a99..f301f17 100644 --- a/cmd/creo/main.go +++ b/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 { diff --git a/internal/harness/harness.go b/internal/harness/harness.go index 61e00b7..5768a0b 100644 --- a/internal/harness/harness.go +++ b/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 } diff --git a/internal/identity/identity.go b/internal/identity/identity.go index c7c18da..0388ee9 100644 --- a/internal/identity/identity.go +++ b/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 }) diff --git a/internal/model/anthropic.go b/internal/model/anthropic.go index c92dab1..3116f6c 100644 --- a/internal/model/anthropic.go +++ b/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 diff --git a/internal/server/server.go b/internal/server/server.go index 8cb1628..6d9d566 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -93,19 +93,19 @@ func New(cfg Config) (*Server, error) { } gw, err := buildGateway(cfg.Model) if err != nil { - db.Close() + _ = db.Close() // constructor is already failing; this is cleanup return nil, err } elog := eventlog.New(db) coord := run.New(db, cfg.LeaseTTL) ps, err := project.New(db, filepath.Join(cfg.DataDir, "cas")) if err != nil { - db.Close() + _ = db.Close() // constructor is already failing; this is cleanup return nil, err } wp, err := workspace.NewProvider(filepath.Join(cfg.DataDir, "workspaces")) if err != nil { - db.Close() + _ = db.Close() // constructor is already failing; this is cleanup return nil, err } tenants := tenant.New(db) @@ -140,11 +140,11 @@ func New(cfg Config) (*Server, error) { // deployment and static login refuses to serve it ambiguously. boundTenant, userCount, err := staticTenant(db) if err != nil { - db.Close() + _ = db.Close() // constructor is already failing; this is cleanup return nil, err } if err := checkExposure(cfg.Addr, userCount > 0, cfg.AllowUnsecured, net.InterfaceAddrs); err != nil { - db.Close() + _ = db.Close() // constructor is already failing; this is cleanup return nil, err } unsecured := false @@ -169,7 +169,7 @@ func New(cfg Config) (*Server, error) { pub := publish.New(db) web, err := webui.Handler(cfg.WebDir) if err != nil { - db.Close() + _ = db.Close() // constructor is already failing; this is cleanup return nil, err } s.http = &http.Server{ @@ -286,7 +286,12 @@ func (s *Server) Run(ctx context.Context) error { for { select { case <-t.C: - s.coord.RecoverOrphans(ctx) + // Recovery is how abandoned runs come back (RC-5). If the scan + // keeps failing, runs quietly stop being rescued — silence + // would make that invisible. + if _, err := s.coord.RecoverOrphans(ctx); err != nil { + slog.Warn("recovery scan failed", "err", err) + } case <-ctx.Done(): return } @@ -315,8 +320,10 @@ func (s *Server) Run(ctx context.Context) error { case <-ctx.Done(): shutdownCtx, stop := context.WithTimeout(context.Background(), 3*time.Second) defer stop() - s.http.Shutdown(shutdownCtx) - s.serving.Shutdown(shutdownCtx) + // Both listeners are closing on the way out; a shutdown error changes + // nothing we can act on, and the drain below is the part that matters. + _ = s.http.Shutdown(shutdownCtx) + _ = s.serving.Shutdown(shutdownCtx) // Give workers a brief, bounded window to relinquish in-flight runs to // the recoverable pool before the DB closes. This waits for the // relinquish *write*, not the build — on timeout, runs are still @@ -412,7 +419,11 @@ func (s *Server) executeRun(ctx context.Context, workerID string, r *run.Run) { // Genuine failure. slog.Warn("run failed", "run", r.ID, "err", err) s.h.EmitFailure(context.WithoutCancel(ctx), r, err) - s.coord.Complete(context.WithoutCancel(ctx), r.Lease, run.StatusFailed, err.Error()) + if e := s.coord.Complete(context.WithoutCancel(ctx), r.Lease, run.StatusFailed, err.Error()); e != nil { + // The run stays claimable until its lease expires, so this + // self-heals — but only recovery, not the user, will notice. + slog.Warn("could not mark the run failed", "run", r.ID, "err", e) + } s.emitState(context.WithoutCancel(ctx), r, run.StatusFailed) } return diff --git a/internal/serving/serving.go b/internal/serving/serving.go index 2abb269..0443d93 100644 --- a/internal/serving/serving.go +++ b/internal/serving/serving.go @@ -9,6 +9,7 @@ package serving import ( "io" + "log/slog" "mime" "net/http" "path" @@ -30,7 +31,7 @@ func New(projects *project.Store, pub *publish.Store, csp string) *Gateway { func (g *Gateway) Routes() http.Handler { mux := http.NewServeMux() - mux.HandleFunc("GET /healthz", func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, "ok") }) + mux.HandleFunc("GET /healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = io.WriteString(w, "ok") }) // {path...} matches the empty remainder too, so these also serve the // directory root (which resolves to index.html in serveFile). mux.HandleFunc("GET /preview/{project}/{secret}/{version}/{path...}", g.servePreview) @@ -92,5 +93,10 @@ func (g *Gateway) serveFile(w http.ResponseWriter, r *http.Request, projectID, v if ct := mime.TypeByExtension(path.Ext(reqPath)); ct != "" { w.Header().Set("Content-Type", ct) } - io.Copy(w, blob) + // Headers are already out, so a failure here cannot become a status code. + // It still matters: a read error means the content store is damaged, and a + // visitor just received a half-rendered page. + if _, err := io.Copy(w, blob); err != nil { + slog.Warn("serving a site file was cut short", "path", reqPath, "err", err) + } } From 076b1203c25fdef963558e78b65c42a404f0932f Mon Sep 17 00:00:00 2001 From: korya <148461+korya@users.noreply.github.com> Date: Thu, 6 Aug 2026 12:59:44 -0400 Subject: [PATCH 4/4] chore: install golangci-lint via Brewfile and the official action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the hand-rolled `just install-lint` recipe. Locally, macOS gets the toolchain from `brew bundle`; in CI, golangci-lint-action installs and caches the pinned binary. install-only is the key flag: the action installs but does not run, so `just check-ci` remains what actually lints. CI has to exercise the same recipe a developer does, or the two drift and CI stops predicting local results. The Brewfile pins node@24 rather than tracking `node`, which is now 26 — building the client on a different major than CI is a divergence you find out about from a red build. It deliberately omits vite-plus and vitest: those are pinned in web/package.json, and pinning a dependency in two places is a way to have two answers to the same question. One honest gap noted in both files: Homebrew tracks the latest golangci-lint while CI pins v2.12.2, so a new release can fail CI on code that linted clean locally. The Brewfile says how to match the pin when that happens. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CiRDTC3E7HGD6Cbq8QeFXM --- .github/workflows/ci.yml | 18 +++++++++++------- AGENTS.md | 7 +++++++ Brewfile | 40 ++++++++++++++++++++++++++++++++++++++++ justfile | 13 ------------- 4 files changed, 58 insertions(+), 20 deletions(-) create mode 100644 Brewfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 85f5deb..767d459 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,13 +37,17 @@ jobs: | bash -s -- --to "$HOME/.local/bin" echo "$HOME/.local/bin" >> "$GITHUB_PATH" - # Same pinned version the justfile installs, so a lint failure here is - # always reproducible with `just check` locally. PATH is set explicitly - # rather than assuming setup-go exports GOPATH/bin. - - name: Install golangci-lint - run: | - just install-lint - echo "$(go env GOPATH)/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 diff --git a/AGENTS.md b/AGENTS.md index 4d10d49..7882a2d 100644 --- a/AGENTS.md +++ b/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 diff --git a/Brewfile b/Brewfile new file mode 100644 index 0000000..c3b9db7 --- /dev/null +++ b/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" diff --git a/justfile b/justfile index 694c96f..47d978d 100644 --- a/justfile +++ b/justfile @@ -11,9 +11,6 @@ set shell := ["bash", "-c"] data := "./data" -# Pinned so local and CI lint with identical rules; see `just install-lint`. -golangci_version := "2.12.2" - [doc("List available recipes")] default: @just --list @@ -95,16 +92,6 @@ check-go-ci: lint-go-fix: golangci-lint run --fix ./... || true -[doc("Install the pinned golangci-lint into GOPATH/bin (same version as CI)")] -install-lint: - #!/usr/bin/env bash - # Pinned: an unpinned linter turns someone else's release into your red CI. - # If a locally installed golangci-lint (e.g. from Homebrew) disagrees with - # CI, run this — the pin here is the version that gates merges. - set -euo pipefail - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh \ - | sh -s -- -b "$(go env GOPATH)/bin" v{{ golangci_version }} - echo "installed golangci-lint v{{ golangci_version }} to $(go env GOPATH)/bin" [doc("TypeScript: format, lint, type-check via Vite+ — fixing in place")] check-ts: