CLAUDE.md
Rama main · réplica de lectura del repositorio original.
## Testing
Exactly ONE coverage condition can block a merge: SonarCloud's "Coverage on
New Code >= 80%" (configured in the SonarCloud UI). That is because
`SonarCloud scan` is one of the 16 checks the `main-protection` ruleset
lists as required, and GitHub blocks a merge only on checks in that list
(`.github/rulesets/main.json`, mirrored live; there is no catch-all
"all checks must pass" option and the ruleset has no bypass actors).
Codecov is a real signal but not a merge gate, and the two halves differ:
- `codecov/patch` (and the per-flag `codecov/patch/server` etc.) is
configured ENFORCING in `codecov.yml`: `target: 70%`, `threshold: 0%`,
`if_ci_failed: error`, and deliberately NOT `informational`. So it does go
RED below 70%. It is simply not in the required-check list, so red does
not block the merge.
- `codecov/project` and every per-flag project rollup ARE
`informational: true` (#227, its numbers proved unreliable here), so they
report success regardless.
Read a red Codecov accordingly: `codecov/project` red is noise, and
`codecov/patch` red is worth fixing on its merits (new lines lack tests, and
Sonar's 80% bar is stricter and WILL block), but neither is the thing
holding the merge button. Do not describe the patch gate as blocking, and do
not dismiss it as advisory either.
Measure patch coverage the way CI does, with `-tags=integration`
(`task test:go:server:coverage`). Without the tag the integration-only paths
report 0% locally and the number is not the one the gate will see.
Tests live in three layers:
1. **Per-package unit tests**: co-located with the code, default tag.
2. **Per-context integration tests**: `server/<context>/internal/tests/`,
`package tests`, real MySQL via `testdb/full.Open(t)`.
3. **Cross-context integration tests**: `test/integration/`,
`package integration`, multi-context scenarios.
### Test-style decision matrix
When adding tests, pick the style that matches the property under test:
| Style | Pick when | Library |
|---|---|---|
| Example-based + table-driven | Wire-format pinning, security regressions, named bug repros, HTTP handler error paths | `testify/assert` + `testify/require`, `t.Run` subtests |
| **Property-based (PBT)** | Algebraic invariants over an input space larger than a table can reasonably cover | `pgregory.net/rapid` |
| Fuzz | Untrusted input parsing (event JSON, policy diff, agent HTTP bodies) | `go test -fuzz` |
| Integration | DB-backed behaviour, multi-step workflows | `testdb/full.Open` + a real MySQL via docker-compose |
| **UI unit + component** | React component rendering, hooks, browser-facing logic (URL builders, fetch wrappers, state machines) | `vitest` + `@testing-library/react` + `@testing-library/jest-dom`, files at `ui/src/**/*.test.{ts,tsx}` |
| UI end-to-end | Spec-level user journeys (login + dashboard + policy editor flows) | `@playwright/test` at `test/e2e/tests/**/*.spec.ts`, `task test:e2e` |
### When to reach for PBT
PBT (`pgregory.net/rapid`) is the right choice when the property holds across
a larger input space than a table-driven test can enumerate. Concrete shapes
in this codebase that already use or should use PBT:
- **Serialization round-trips**: `Marshal ∘ Unmarshal == identity`,
`Scan ∘ Value == identity` for `NullRawJSON`, `JSONStringSlice`,
`wire.DecodeBatch ∘ wire.EncodeBatch`.
- **State-machine transitions**: `service.canTransition` over the full
`AlertStatus` enum cross-product, exhaustive-by-construction.
- **Graph invariants**: `graph.buildForest` - for any random fork batch, every
non-root node has its parent in the tree, no cycles, every input PID
appears exactly once.
- **Order-preserving filters**: `engine.filterSnapshotEvents` - the output is
exactly the input minus the snapshot:true exec events, in the same order.
- **Re-exec chain**: walking `previous_exec_id` from any leaf reaches the
chain root in ≤ chain length steps without revisiting nodes.
### UI testing convention
UI tests live under `ui/src/**` as `*.test.{ts,tsx}` files co-located with the source. Run via `cd ui && npm test` (or
`npm run test:coverage` for the LCOV that Sonar + codecov read). The conventions:
- **Components**: `@testing-library/react`'s `render` + `screen` + user-event idioms. Mock the API layer via `vi.spyOn(api,
...)` rather than stubbing global `fetch`; this keeps the test scoped to the component's behaviour, not the HTTP wire shape.
- **Hooks**: `renderHook` from `@testing-library/react`. The `act()` wrapper is required around state-setter calls.
- **Pure-logic modules** (URL builders, parsers, validators): plain vitest `describe` / `it` with no React dependency.
- **WebAuthn / browser globals**: stub via `vi.stubGlobal("navigator", { credentials: { create / get } })` or
`vi.spyOn(globalThis.location, "assign")`. Tear down in `afterEach(vi.restoreAllMocks)`.
`ui/src/**` was excluded from Sonar's new-code coverage gate until vitest tests landed. The exclusion is now lifted; every
new UI file must carry a `*.test.{ts,tsx}` sibling that exercises the non-trivial branches. The 80% new-code gate applies
identically to UI + server code. Coverage is captured by V8 via vitest's coverage provider (`@vitest/coverage-v8`); the
Playwright E2E run captures complementary V8 coverage of the running React bundle via `monocart-coverage-reports`, and
Codecov takes the union.
PBT does NOT replace example-based tests when:
- The bug is specific (you want the exact reproducer in the test name).
- You're pinning a wire shape (you want the literal bytes).
- The path is security-critical (you want the test to be auditable as
"this exact input produces this exact output").
### When PBT isn't a good fit, default to table-driven
Use `t.Run` subtests + a `cases := []struct{...}` slice. Keep test names
descriptive (`whitespace hostID`, `pending->completed must ack first`),
not numbered (`case3`).
## UAT and integration test layers
Full strategy: `docs/testing-strategy.md`. Seven test layers from unit tests up to detection-efficacy runs, with
reusable artefacts (fake agent library, headless agent binary on Linux, captured ESF corpus) and spec-to-test
traceability.
Minimum requirements per PR:
- **Behavior changes ship a change delta, not a hand-edit of the canonical spec.** Any observable behavior change (bug fix, feature, detection rule, wire/event shape, API, persistence semantics) MUST ship an `openspec/changes/<name>/` proposal in the same PR: `proposal.md`, `tasks.md`, and a delta `specs/<capability>/spec.md` written with `## ADDED / MODIFIED / REMOVED Requirements`. Do NOT hand-edit `openspec/specs/**` directly: that canonical tree is updated ONLY by `openspec archive`, which runs at release time and never per-merge (see the next bullet). spectrace treats scenario IDs declared in an in-flight delta as valid marker targets, so a test added in the same PR can reference a brand-new scenario before it reaches the canonical spec, no pre-merge needed. Two gates back this: `OpenSpec sync` (`.github/workflows/openspec-sync.yml`) enforces that a behavior-path change touches `openspec/` at all (the delta satisfies it) for the highest-signal paths (`server/rules/internal/catalog/`, `schema/events.json`, `server/detection/migrations/`); `OpenSpec validate` (`.github/workflows/openspec-validate.yml`, `openspec validate --all --strict`) enforces that every spec and delta is structurally well-formed (e.g. every requirement has a scenario). The sync gate fires on path, so a genuine no-behavior touch of those files (comment / refactor / gofmt / dep bump) asserts `no-behavior-change` (label or `[no-behavior-change]` in the title) to clear it. That opt-out is an auditable "this changes no behavior" claim a reviewer verifies; it is NEVER a way to skip the spec for a real behavior change.
- **Do NOT archive per-merge; archives are batched at release.** A merged change stays in `openspec/changes/<name>/` until the release archive step runs `openspec archive <name> -y` (NO `--skip-specs`) for every pending change, merging each delta into `openspec/specs/**` and moving the folder to `openspec/changes/archive/`. This is a release-checklist step (`docs/release-checklist.md`) and is enforced by the `openspec-archived` job in `.github/workflows/release.yml`, which every publishing job depends on (`needs:`) so a `v*` tag push fails before signing if any non-archive folder is left under `openspec/changes/`. Rationale for batching rather than archiving on the merging PR: archiving on a feature branch is a direct edit to `openspec/specs/**` (which the compliance bots flag) and collides with squash-merge rebases on stacked work; batching keeps the canonical tree moving only at release boundaries, where it is reviewed as one diff. A merged change that REMOVES a requirement does NOT need early archiving to stay green: spectrace exempts canonical scenarios whose requirement an in-flight delta marks `## REMOVED` from the `--strict` gate, so the requirement's tests can be deleted at merge and the gate stays honest until the release archive finalizes the removal. Use `--skip-specs` ONLY when the canonical spec was already updated by hand (legacy changes predating this rule) or for a tooling/doc-only change with no spec delta. Never move or rename a change folder into `archive/` by hand: that is how the malformed double-date names (`2026-06-09-2026-06-09-...`) got into the archive. Archived deltas are historical; spectrace skips the `archive/` subtree.
- **Keep each new or modified requirement's text under 500 characters.** State the normative behavior and put rationale, examples and edge cases in its scenarios. OpenSpec 1.14.1+ fails `validate --strict` on longer requirements; CI is pinned to 1.14.0 until the existing backlog is shortened (#1251).
- New wire-format struct or event field: PBT round-trip (`Marshal . Unmarshal == identity`).
- New detection rule under `server/rules/internal/catalog/`: ship `test/efficacy/corpus/T<MITRE-id>/scenario.yaml`
plus `expected.yaml`. Add `attack.sh` when system / VM coverage is needed.
- Agent or extension change touching ESF, XPC, or the event wire format: must be exercised on a live macOS VM
(the system / VM layer) before RC. Flag in the PR description.
- New or modified SHALL / MUST scenario (in `openspec/specs/` or an in-flight `openspec/changes/<name>/` delta): at least one test must reference the canonical scenario ID. ID scheme and marker forms in `docs/testing-strategy.md`; gated by `tools/spectrace`.
## Bounded contexts
ADR-0004 carved `server/` into bounded contexts: the original five
(`identity`, `endpoint`, `rules`, `response`, `detection`), since amended to add
`observability` (the sixth), `visibility` (the seventh, carved from
`detection` per ADR-0015), and `rulecontent` (the eighth, carved from `rules`
per ADR-0021). Cross-context calls go through the imported `api/`
package only. Internal packages are Go-compiler enforced via the `internal/`
rule. arch-go (`arch-go.yml`) layers an extra check.
## Dev environment
- Local MySQL: `task db:up` brings up `127.0.0.1:33306` (dev) and `:33307` (test).
Empty password (`MYSQL_ALLOW_EMPTY_PASSWORD=yes` in `docker-compose.yml`).
- Dev server: `task dev:server` serves HTTPS on `0.0.0.0:8088` (it binds all
interfaces so a VM agent on the bridge can reach it, override `EDR_LISTEN_ADDR=127.0.0.1:8088`
on an untrusted LAN) against `127.0.0.1:33306/edr`. Real-tool QA must use this; never fall
back to curl or unit tests when the user asks for a Chrome / VM / dev-server check.
- Test DSN: `EDR_TEST_DSN=root@tcp(127.0.0.1:33307)/edr_test?parseTime=true`.
## Coverage gates
CI uploads `coverage-server.out` from `task test:go:server:coverage`, which runs
`go test -race -tags=integration -coverpkg=./server/...,./internal/... -coverprofile=coverage-server.out ./server/... ./internal/... ./test/integration/... ./test/scale/... ./tools/...`.
The wide `-coverpkg` is mandatory: integration tests at
`server/<context>/internal/tests/` exercise other packages' symbols, and
without `-coverpkg` Sonar would report 0% on the bootstrap / service files
those tests fully cover.
## Code style
Layered on the global guide. Project-specific:
- Line wrap source code at 140 characters.
- Settings forms are capped at `$settings-form-max-width` (`ui/src/styles/var/_global.scss`), never at a hardcoded literal. A form left uncapped spans an ultrawide display and puts a field's label and its input a screen apart, which is how the Webhooks page came to look nothing like the SSO page beside it. The cap belongs on the form's own root, NOT on `.settings-layout__content`: Users and Service accounts live under that same layout and render wide tables that legitimately want the whole viewport.
- Markdown is NOT hard-wrapped: write each paragraph and bullet as one line and let it soft-wrap. Tables are Prettier-aligned (cells padded so column dividers line up). Both are enforced by Prettier (`proseWrap: never`, `.prettierrc.yaml`); run `task lint:md:prose:fix` to reflow, `task lint:md:prose` to check. markdownlint (`task lint:md`) owns Markdown structure; the two are scoped not to overlap.
- Sentence case for headings.
- `CHANGELOG.md` release notes (and the GitHub Release body, which the release workflow auto-generates with `gh release create --generate-notes` and which must then be manually replaced to match this section) are written for the security and IT professionals who read them, not for contributors: keep them brief and to the point. One tight claim per bullet, lead with what changed and what action the operator must take (put "action required" upgrade notes first), and cut engineer-facing internals (datastore mechanics, concurrency and race detail, exhaustive env-var or endpoint or error-code enumerations, per-route byte caps, package and module names). State the user-visible effect instead ("event history is not migrated on upgrade"). Fold intra-release refinements into the feature they refine rather than listing each PR. The `## [0.3.0]` section is the reference for density (~1,300 words for a feature release, not 4,000+); the release-checklist changelog step (`docs/release-checklist.md`) repeats this.
- Public-facing docs (operator docs under `docs/`, the OpenAPI spec, the rule pack files, and their source `Doc()` strings) describe the product as it is. Do not narrate how it got there ("until issue #N this rule also...", "earlier versions recommended..."): history belongs in `CHANGELOG.md`. Link an issue only while it is open, as a pointer to a known gap. Enforced by `task lint:docs:issues` (`tools/doc-issue-lint`), which fails on any reference to a closed issue or a pull request; ADRs, runbooks, and contributor docs are out of its scope.
- No em dashes, and no spaced hyphen (` - ` or ` -- `) standing in for one: reword the sentence (prefer shorter sentences) or use a colon. A hyphen is fine only unspaced inside a compound word (`per-IP`) or as a list marker. Insert a literal em dash (U+2014) only when explicitly asked. Enforced by `task lint:dashes`: `tools/lint-no-emdash.sh` bans the dash characters in any tracked text file, `tools/dash-lint` bans the spaced-hyphen stand-in (single ` - ` and double ` -- `) in Markdown prose, in code comments and string literals (`.go`/`.swift`/C-style), and in `#`-comments of `.yml`/`.yaml`/`.sh`. A legitimate ` -- ` (a GNU end-of-options separator in a CLI example) is exempt when wrapped in `code-span` backticks; the ESLint `-- description` and gosec `#nosec -- justification` separators are recognized and skipped. For the rare unwrappable case, a line carrying the literal `dash-lint:ignore` directive is skipped.
- Don't run `task db:reset` without explicit user permission.
- Stateless server (ADR-0010): the server holds no in-process state that survives a request lifetime and that a peer replica
would need to serve the next request. Durable cross-request state goes in MySQL; per-request state may ride in signed cookies;
a new in-process map / channel / queue holding shared state is a review defect unless it carries a "per-replica perf cache,
safe to lose" note. The app tier is multi-replica behind a load balancer (the server-availability arc).
- The project builds on Go 1.27+ (see `go.mod`); modern language and standard-library features through 1.27 are in-bounds.
That includes the stdlib `uuid` package, which replaced `github.com/google/uuid` here and is depguard-banned from returning.
Integer range expressions (`for i := range N` where `N` is an `int`) are valid project style and the modernize linter
prefers them over `for i := 0; i < N; i++`; the same applies to newer stdlib like `strings.SplitSeq` / `bytes.SplitSeq`.
Copilot and CodeRabbit have re-flagged these as "doesn't compile" or "breaks Go 1.22+" on multiple PRs;
the claim is false and the finding should be skipped.
- An audited state change commits its audit entry in the same transaction as the change (ADR-0011, 2026-10-10 amendment): directly into `audit_events` inside the identity context, through the context's outbox table and `server/auditoutbox` everywhere else. Recording the audit row after the change commits is a review defect: the change can land without its audit row.
- When you delete a symbol (function, type, command name, XPC message kind, config field),
scrub every doc comment that still references it before committing. Stale comments
in IPC-adjacent code are a recurring class of footgun in review. Treat the comment delta as part of the
deletion, not a follow-up.
## Reuse and simplicity
This codebase is largely AI-generated, which produces two failure modes no linter catches: code that re-implements helpers, types, or endpoints that already exist (because it was generated without searching the tree), and defensive branches for inputs no caller can produce. Two rules counter this.
- Search before you add. Before writing a new helper, type, validator, constant, SQL query, or HTTP route, search the owning bounded context and the shared `internal/` and `api/` packages for an existing equivalent, and extend it rather than cloning it. The `find-prior-art` skill automates that search; run it before generating non-trivial new code. SonarCloud gates new lexical duplication, but semantic duplication (the same logic renamed) is invisible to it and is the author's and reviewer's responsibility.
- No speculative edge cases. Every defensive branch and error path needs a real caller that reaches it. If a state is already ruled out by a type or an upstream validator, do not re-handle it: validate once at the boundary and trust the types inward. A guard you cannot produce an input for is dead code, not robustness. New-code cognitive complexity and unused parameters are gated by `task lint:go:newcode`; the existing backlog is paid down by the monthly `consolidation-pass` maintenance task.