diff --git a/CLAUDE.md b/CLAUDE.md index fe60c5c..de44516 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,6 +29,8 @@ Standard Go toolchain (Go 1.25.5 per `go.mod`). All commands run from the repo r There is no `Makefile`, no `golangci-lint` config, and no test files yet — do not assume they exist. +Cross-compile outputs are gitignored via the pattern `spark-mcp-*-*` (see `.gitignore`); do not commit stray build artifacts. + ## Key Pinned Dependencies `go.mod` declares these as `// indirect` (they will become direct once code is added that imports them): @@ -56,3 +58,71 @@ The codebase is empty enough that a few decisions are not yet pinned down and wi ## Memory This is a fresh repo with no `MEMORY.md` or project-level memory file. Decisions made during early development (transport choice, Spark backend interface, persistence schema) are worth recording once they are made — see the `claude-md-management` skill conventions. + +--- + +# Working in this Repo + +The rules in `~/.claude/CLAUDE.md` apply (Codex-first routing, mandatory Codex MCP invocation parameters, Linus three-question review, 4-phase workflow, Codex prompt template, auto-confirm vs pause boundaries). This section adds the **project-specific calibration** for applying them here. + +## Routing: Codex vs CC + +| Task in this repo | Executor | Why | +|---|---|---| +| Edit `main.go` to wire `cfg.GinMode` | Codex | One-line code change in production path | +| Add `loadDotenv` to `internal/config/config.go` | Codex | New function, real logic | +| Add a new test file | Codex | Even small, code generation is the strength | +| Edit `internal/httpclient/ssrf.go` to silence S1017 | Codex | Behavior must stay identical — needs verification | +| Update `.gitignore` line | CC | Single-line, no logic, mechanical | +| Update this `CLAUDE.md` | CC | Pure docs | +| Fix typo in a comment | CC | <20 lines, no logic | +| Update `.env.example` header comment | CC | Single line of prose | + +**Threshold for "trivial enough for CC" in this repo:** purely textual, no code generation, <20 lines, no risk of subtle behavior change. When in doubt, default to Codex — the cost difference is small and Codex is more consistent at preserving invariants. + +## Codebase Patterns to Reuse + +When working in a package, **read the package's existing test file first** to see the established style before writing new code: + +- **HTTP client mocks** — `internal/httpclient/redirect_test.go` uses `httptest.NewServer` with path-switch handlers. For SSRF / redirect tests, pass `"127.0.0.1"` as the allowed host. +- **slog capture in tests** — `internal/logging/tool_call_test.go` shows the pattern: `bytes.Buffer` + `slog.NewJSONHandler`, decode each line with `json.Unmarshal`, assert on `rec["msg"]`, `rec["level"]`, `rec["tool"]`. Reuse this for any new logger test. +- **MCP tool registration** — every tool lives in `internal/mcp/tools/.go`. Schema is the first arg, handler is a method on the package's client struct. The tool's Go test file goes in the same directory. + +## Pre-Commit Checklist (project-specific) + +In addition to the global acceptance list, this repo's convention is: + +- [ ] `go build ./...` — zero errors +- [ ] `go vet ./...` — zero warnings (catches the stringsseq / S1017 family) +- [ ] `go test ./...` — all green; new tests added for new behavior +- [ ] `gofmt -l .` — empty output (no diffs pending) +- [ ] No stray `spark-mcp-*-*` binary in repo root +- [ ] If behavior gap is intentionally deferred: add a deferred test that **locks the current behavior** (so a future fix must update the test deliberately) and a `TODO()` comment in the source with rationale +- [ ] If a public function signature changed: explain backward compatibility in the commit body + +## Commit Message Convention + +Observed in this repo's recent history: + +``` +: + + +" trailer for intentionally uncommitted artifacts> + +Co-Authored-By: Claude +``` + +- Subject line: `:` prefix (e.g. `httpclient:`, `rm:`, `admin:`, `config:`, `logging:`, `gitignore:`), then a short verb-led summary in Chinese. +- Body in Chinese; technical English terms (RFC numbers, status codes, test names, package paths) are fine in code form. +- Always end with the `Co-Authored-By` trailer. +- Body should answer: *what was the gap, why this fix, what could it have broken, what tests lock it in.* This is what makes the commit history a useful design record. + +## Project-Specific Gotchas + +- **`.env` is uncommitted and gitignored** — never try to read it, and never claim to know what's in it. Treat its existence as opaque. +- **Cross-compile artifacts** — `go build -o spark-mcp-linux-amd64 .` and similar produce files covered by the `spark-mcp-*-*` gitignore. Don't try to `git add` them. +- **`internal/` is enforced** — Go's `internal/` rule means sub-packages of `spark-mcp-go/internal/...` are importable only by `spark-mcp-go/...`. Cross-repo imports will fail; design packages accordingly. +- **Deferred-behavior tests are intentional** — `TestDoWithRedirect_DeferredRFC9110Gaps` is a *gap-locking* test, not a regression test. The assertions verify the gap, not the fix. Read the comment block above the test before "fixing" what looks like a bug. +- **YARN / SHS API surface is the source of truth** — the upstream REST API behavior (e.g. amContainerLogs being a 307 redirect, not a 303) drives the design. Don't invent redirect semantics; check what the cluster actually returns. +- **`log/slog` uses a `multiHandler` for dual terminal+file writing** — the `base *slog.Logger` passed to `StartToolCall` already routes through the main logger. Adding a new event is one `base.Info(...)` call; do not add a separate handler.