From 7d80f6a5bf773b7fbacbbbc87ea787ab3cdde527 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:00:52 +0000 Subject: [PATCH 1/6] chore(agents): move topic rules out of the root agents file The root AGENTS.md loads in full in every agent session. Move the architecture, code style, CI, feature usage tracking, and agent automation rules into .agents/.md guides that agents read on demand, and leave one pointer line under each original heading. The user-facing copy bullets already live in /writing-user-facing-copy. Point the enforcement-tag invariant test, its CI path filter, CODEOWNERS, the stamphog agent_steering category, and every reference at the new guides. Root file: 46,010 bytes to 22,404 bytes. Co-Authored-By: Claude Opus 5.5 --- .agents/agent-automation.md | 12 ++ .agents/architecture.md | 56 ++++++++ .agents/ci.md | 8 ++ .agents/code-style.md | 40 ++++++ .agents/feature-usage-tracking.md | 8 ++ .../building-product-empty-states/SKILL.md | 2 +- .agents/skills/editing-agents-md/SKILL.md | 10 +- .../SKILL.md | 2 +- .agents/skills/writing-ui-components/SKILL.md | 2 +- .../skills/writing-user-facing-copy/SKILL.md | 2 +- .claude/agents/code-reviewer.md | 4 +- .claude/rules/agents-md.md | 9 +- .depot/workflows/ci-backend.yml | 5 +- .github/CODEOWNERS | 5 + .github/scripts/check-claude-hooks.sh | 2 +- .github/workflows/ci-dagster.yml | 2 +- .greptile/files.json | 14 +- .../devex/tuple-return-prefer-dataclass.yaml | 4 +- .stamphog/README.md | 2 +- .stamphog/policy.yml | 1 + AGENTS.md | 129 +----------------- docs/internal/ci-things-already-tried.md | 2 +- frontend/src/AGENTS.md | 10 +- .../test_agents_md_enforcement_tags.py | 35 +++-- 24 files changed, 201 insertions(+), 165 deletions(-) create mode 100644 .agents/agent-automation.md create mode 100644 .agents/architecture.md create mode 100644 .agents/ci.md create mode 100644 .agents/code-style.md create mode 100644 .agents/feature-usage-tracking.md diff --git a/.agents/agent-automation.md b/.agents/agent-automation.md new file mode 100644 index 000000000000..820696988e59 --- /dev/null +++ b/.agents/agent-automation.md @@ -0,0 +1,12 @@ +# Agent automation + +When automating a convention, try these in order — only fall back to the next if the previous isn't suitable: + +1. **Linters** (ruff, oxlint, semgrep) — code pattern enforcement, always paired with CI +2. **lint-staged / husky** — file-level validation or warnings at commit time +3. **Skills** (`.agents/skills/`) — scaffold with `hogli init:skill` +4. **AGENTS.md / CLAUDE.md instructions** — when automated enforcement isn't suitable + +Claude Code hooks are reserved for environment bootstrapping (`SessionStart` only) — do not add `PreToolUse`, `PostToolUse`, or `Notification` hooks as they add latency and are fragile. +Changes to `.claude/hooks/` trigger a warning from the `pre-commit` hook; lint-staged allows only repo-wide keys in `.claude/settings.json`, because personal settings belong in the gitignored `.claude/settings.local.json`. +A warn-only check belongs in the `pre-commit` hook body rather than in a lint-staged task, because lint-staged discards the output of every task that exits 0. diff --git a/.agents/architecture.md b/.agents/architecture.md new file mode 100644 index 000000000000..cf374bef750e --- /dev/null +++ b/.agents/architecture.md @@ -0,0 +1,56 @@ +# Architecture guidelines + +Each rule is tagged with what catches a violation. +`[lint: ]` means a linter, semgrep rule, or invariant test blocks it, so CI is the check and this text is only the reasoning. +`[review]` means nothing catches it automatically — a reader is the only control. + +## Entrypoints and layering + +- **Keep entrypoints thin.** `[review]` The runtime calls the entrypoint first: a DRF view, a management command `handle()`, a Celery task body, an HTTP handler, a Rust `main()`, a React component. An entrypoint reads the input, connects the dependencies, and formats the output. All other logic goes in a function the entrypoint calls, so a test can call it directly. If the area already has a place for that logic, use it rather than making a new one. +- **A product under `products/` is scaffolded, not hand-rolled.** `[lint: hogli product:lint --all]` Create one with `hogli product:bootstrap `, because the scaffold emits a product that is isolated from its first commit. See "New products must be isolated" in [products/README.md](../products/README.md) for the isolation rules, and [products/architecture.md](../products/architecture.md) for facade and contract design. +- **Code a single product owns belongs under `products//`** — backend, frontend, scripts, CLIs, services, packages, MCP tools, skills. `[review]` Reserve top-level `tools/`, `services/`, `packages/`, `cli/` and `hog/` for cross-product things. `product:lint` only scans `products/`, so a single-product feature parked in a top-level directory is invisible to it. + +## Tenancy and scoping + +- **Every tenant-data model must have `team_id`**, as a FK to `posthog.Team` or a plain `BigIntegerField` for multi-DB products. `[lint: idor-lookup-without-team, check-idor-model-coverage.py]` This is the primary tenant isolation boundary. New models inherit `TeamScopedRootMixin` (main DB) or `ProductTeamModel` (separate DB) so they start fail-closed — see [posthog/models/scoping/README.md](../posthog/models/scoping/README.md). +- **Classifying a model as unscoped is a review decision, not a checked one.** `[review]` A model without `team_id` must be org-scoped, user-scoped, or instance-global, never silently unscoped. `check-idor-model-coverage.py` only warns when a name lands in `NEEDS_TEAM_ID`, and passes silently for `LEGITIMATELY_UNSCOPED`, so a wrongly exempted tenant model reaches master unless a reader catches it. +- **Reading a fail-closed model outside request context** — Temporal activities, Celery tasks, management commands — uses `Model.objects.for_team(team_id)`. `[review]` Not `Model.all_teams.filter(team_id=...)` and not `objects.unscoped().filter(...)`; those are for genuinely cross-team access and Django internals. In serializers the team comes from `self.context["get_team"]()`. Caveat: `for_team(...).get_or_create(...)` and `.create(...)` still need `team_id` passed explicitly, because a queryset filter does not propagate into row creation. +- **Do not add domain-specific fields to the `Team` model.** `[review]` Use a Team Extension model — see [posthog/models/team/README.md](../posthog/models/team/README.md) for the pattern and helpers. + +## Person and group data + +- **All person/group access goes through the personhog client.** `[review]` Never query these via the Django ORM or raw SQL: `posthog_person`, `posthog_persondistinctid`, `posthog_cohortpeople`, `posthog_group`, `posthog_grouptypemapping`, and the override tables (`posthog_personoverride`, `posthog_pendingpersonoverride`, `posthog_flatpersonoverride`, `posthog_featureflaghashkeyoverride`, `posthog_personlessdistinctid`, `posthog_personoverridemapping`). Use the routed helpers in `posthog/models/person/util.py` and `posthog/models/group_type_mapping.py`. New access follows the same `_personhog_routed()` pattern: a `personhog_fn` using `get_personhog_client()`, plus an `orm_fn` fallback. See [posthog/personhog_client/README.md](../posthog/personhog_client/README.md). +- **Personhog answers identity questions only.** `[review]` Use it for resolving distinct IDs, point lookups by person id or UUID, and lifecycle writes. Person properties, list hydration, and search come from ClickHouse (HogQL over `persons`, `ActorsQueryRunner`). Avoid reading `properties` from personhog — the API will stop surfacing it. Single-person lookups that need properties are exempt only while no ClickHouse primitive covers them, never in bulk. See [docs/internal/person-data-access.md](../docs/internal/person-data-access.md). + +## Transactions and locking + +- **There is no implicit per-request transaction** — PostHog does not enable `ATOMIC_REQUESTS`. `[review]` Every operation runs in autocommit unless wrapped. Put `with transaction.atomic():` around the specific writes that must succeed or fail together, never around a whole view method. Keep irreversible side effects out of the block: an email sent or an API called before a rollback has already happened. Schedule them after the commit, or dispatch Celery tasks with `transaction.on_commit()`. +- **Do not use `Team` or `Organization` rows as mutexes.** `[lint: hot-parent-row-select-for-update]` PostgreSQL takes `KEY SHARE` locks on parent rows for foreign-key checks, and `FOR UPDATE` conflicts with them, so an unrelated child-row write can wait behind a transaction holding a hot parent row. Lock the product's `TeamConfig` extension row instead, reached through `get_or_create_team_extension` so the row exists first. For a per-team count limit or number allocation, use a unique constraint, a conditional update, or a dedicated allocation row. Otherwise lock the quiet child row, or take a transaction-scoped advisory lock. Lock `Team` or `Organization` only when the transaction updates or deletes that row, and keep it short — no network calls, no input-sized loops. Each exception needs a short `nosemgrep` justification. + +## API schemas and generated types + +- **API views declare request/response schemas.** `[review]` Prefer `@validated_request` from `posthog.api.mixins`, or `@extend_schema` from drf-spectacular. A plain `ViewSet` method that validates manually needs `@extend_schema(request=YourSerializer)`; without it drf-spectacular cannot discover the request body and generated code gets an empty schema. Serializer fields need `help_text`. These flow into both frontend types and MCP tool schemas. +- **Django serializers are the source of truth for frontend API types.** `[lint: build:openapi CI gate, prefer-codegen-api, test_generated_files_are_registered.py]` `hogli build:openapi` generates TypeScript via drf-spectacular and Orval into `frontend/src/generated/core/` and `products/{product}/frontend/generated/`. Never hand-edit `api.schemas.ts`, `api.ts` or `api.zod.ts` — change the serializer and regenerate. A value list the frontend needs comes from a typed serializer field, not from a side script; other checked-in projections are entries in `tools/hogli-commands/hogli_commands/projections.py`. [Type system guide](../docs/published/handbook/engineering/type-system.md) has the full pipeline. +- MCP tools and MCP UI apps are generated from the same OpenAPI spec. `[review]` See [implementing MCP tools](../docs/published/handbook/engineering/ai/implementing-mcp-tools.md) covers the YAML config and codegen. MCP UI apps live in `products/*/mcp/tools.yaml` under `ui_apps` — see [services/mcp/CONTRIBUTING.md](../services/mcp/CONTRIBUTING.md) or `/implementing-mcp-ui-apps`. + +## Async, storage and outbound calls + +- **Do not use `posthoganalytics.capture()` in a Celery task** — events are silently lost. `[review]` Use `ph_scoped_capture` from `posthog.ph_client`; its docstring explains why. +- **Temporal activity payloads cap at ~2 MiB — pass large data by reference.** `[review]` Activity inputs and outputs cross a gRPC boundary the server rejects above that (`blobSizeLimitError`). As a field-level rule: if a field could exceed ~256 KB serialized (query results, exported file contents, LLM context, rendered HTML, image bytes, unbounded `list[dict[str, Any]]`), write it to Postgres or object storage from inside the activity and return only the row ID or S3 key. The workflow already has any ID created earlier in the run. Shuttling large data through the workflow produces `PayloadSizeError` (`TMPRL1103`) as soon as the data crosses the limit. +- **A Python `requests` call to GitHub or Slack under `common/`, `ee/`, `posthog/` or `products/` goes through `posthog/egress/`.** `[lint: github-api-calls-go-through-egress, slack-api-calls-go-through-egress]` Route it through the gated, recorded transport. +- **Every other call to those hosts goes through `posthog/egress/` too.** `[lint: github-api-calls-go-through-egress-wide, slack-api-calls-go-through-egress-wide]` The wide rules catch a URL bound to a variable or constant, `requests.Session`, `httpx`, `aiohttp`, `urllib.request`, and an aliased Slack SDK client. Another language, another vendor SDK, or a caller under `tools/` still passes CI, so a reader is the only control there. +- **Any other third-party API that needs rate-limiting or egress telemetry belongs there too.** `[review]` Add a `/` incarnation (GitHub is the reference). No semgrep rule covers a new domain, so a raw client for one reaches master unless a reader catches it. `/routing-outbound-api-calls` decides the route and names the reference domain to copy. +- **Object storage is SeaweedFS — do not add new MinIO dependencies.** `[review]` Both S3-compatible stores are SeaweedFS: `objectstorage` (`:19000`, `OBJECT_STORAGE_*`) for general storage, `seaweedfs` (`:8333`, `SESSION_RECORDING_V2_S3_*`) for session replay v2. MinIO survives only as one-off salvage tooling: `bin/migrate-storage-hobby` starts a temporary container to read an old hobby MinIO volume. Do not add compose services, scripts, tests or docs that stand up a `minio/minio` container. Talk to storage through the existing config and a standard S3 client, never a hardcoded endpoint. Note `objectstorage` registers credentials at runtime and returns `InvalidAccessKeyId` until that finishes, so wait for its readiness sentinel rather than the container start. +- **A new DNS-resolution or outbound-host entry point calls the shared out-of-band domain check.** `[lint: test_oob_denylist_coverage.py]` Every host guard here is IP-based, and an out-of-band interaction domain resolves to an ordinary public IP, so a resolver that skips the check restores the gap in silence. The list and the edit procedure live in `posthog/security/crosslang/`. + +## Configuration and secrets + +- **A key in `posthog/settings/dynamic_settings.py` exists only because operators must change its value live, without a deploy.** `[review]` These instance settings are Postgres rows that staff edit at runtime, and a stored row overrides the environment variable. Kill switches and tuning knobs for incidents qualify. Secrets and static config are environment variables read in `posthog/settings/`, with values from the deployment secret store and charts. Never store a secret as an instance setting, because the database keeps it as plain text. A per-team rollout or a team allowlist is a feature flag. + +## Django admin + +- **A new `ForeignKey`/`OneToOneField` on an admin-exposed model needs explicit widget config.** `[review]` List the field in `autocomplete_fields`, `raw_id_fields`, or `readonly_fields` on **every** admin class that renders the model, including inlines attached to a related admin. Otherwise the default `` loads the entire target table per row on each change-page render. Declare it on a shared base inline so per-parent variants inherit it. - -### LLM gateway - -- **`services/llm-gateway` is under an unofficial code freeze** while callers move to [`PostHog/ai-gateway`](https://github.com/PostHog/ai-gateway). `[review]` If a task asks you to change it, tell the user about the freeze in your reply. New callers and features belong on the Go gateway. A Python gateway change needs a documented parity blocker for an active caller and stays limited to it — read [`services/llm-gateway/PARITY.md`](services/llm-gateway/PARITY.md). Its Postgres role reads only allowlisted tables: a new table read needs the SELECT grant landed in posthog-cloud-infra for every environment first, then a declaration in `required_tables.py`. The readiness probe checks every declared grant on every probe, so a missing grant holds a rollout instead of serving 500s. Use `/auditing-llm-gateway-parity` for contract changes, `/finding-llm-gateway-migration-candidates` to pick the next caller, `/migrating-llm-gateway-callers` to move one. +Read [.agents/architecture.md](.agents/architecture.md) before you add or change application code: entrypoints, `team_id` scoping, personhog, transactions, API schemas, Celery and Temporal, outbound calls, storage, settings, Django admin, and the `services/llm-gateway` freeze. ## Code Style -Same tags as above: `[lint: ]` is machine-enforced, `[review]` is not. - -### Python - -- **Write as if mypy `--strict` were on.** `[review]` Annotate every signature, avoid `Any`, put type-only references under `TYPE_CHECKING`. The config is not fully strict yet; new code should be. When a change is type-risky run mypy the way CI does — `uv run mypy --cache-fine-grained .`, repo-wide, never a file subset. It follows imports, so a subset misses reverse-dependency breakage. -- **Imports stay at module level.** `[review]` ruff's `PLC0415` catches this, but `posthog/**`, `ee/**`, `common/**`, `tools/**` and most of `products/` are on grandfathered exemption lists, so CI does not block it in the trees you are most likely to be editing. Defer one only to break a true unavoidable circular import, to reference types under `TYPE_CHECKING`, or to keep a heavy or optional dependency off the import path. For the last case add a justified `# noqa: PLC0415 — keeps the heavy dep off the import path` on the line. Never blanket-suppress the rule. -- **A stdlib `@dataclass` must declare `frozen=` explicitly.** `[lint: prefer-frozen-dataclasses, test_dataclass_defaults.py]` A bare `@dataclass` fails the ratchet; `@dataclass(frozen=False)` passes it, so intentional mutability is a choice you state. -- **The house decorator is `@frozen` from `posthog.dataclasses`.** `[review]` Neither check prefers it over a declared stdlib `@dataclass`. Invoke `/writing-dataclasses` before adding or changing one, before returning or passing several values together, and before passing a dataclass through layers. -- **`product:lint` and the pytest collision test say where an `__init__.py` is required, and print the fix.** `[lint: product:lint, test_pytest_module_collisions.py]` Run them rather than reasoning about it. -- **Do not add or delete an `__init__.py` speculatively.** `[review]` Whether a directory needs one depends on what sits above it. Neither check objects to a needless one outside `products/` that creates no module-name collision, so only a reader catches that. -- Prefer classes over loose functions unless a class genuinely does not fit. `[review]` Use `pathlib` over `os.path`. Order functions and methods so a definition appears before its first call. - -### Frontend - -- **Follow [frontend/src/AGENTS.md](frontend/src/AGENTS.md) for all frontend work** — the main app and `products/*/frontend/` alike, since they share components and generated types. `[review]` It covers reusing Lemon/quill components instead of hand-rolling tables, badges and labels, importing generated `*Api` types instead of handwriting them, and when to run typecheck and typegen. -- **TypeScript with explicit return types. Business logic goes in the kea logic file, not a React hook.** `[review]` -- **Guard every network-triggering button against double submission.** `[review]` Disable it and show a loading state (`loading` / `disabledReason` on `LemonButton`, or equivalent) while the request is in flight, and reset in both the success and error paths. Applies to `
`, any `onClick` that calls `api.*`, and any kea `listener` that issues a request. Wire in the in-flight state from a loader `*Loading` selector, a reducer, or local `useState`. -- **Every surface must hold up narrow.** `[review]` The nav sidebar plus an open side panel leave a 1280px window about 520px of scene. Break on container queries, not `md:`/`lg:`/`xl:`. Wrap or truncate rather than clip, and stack halves that no longer fit. Render at a few widths before calling it done. We do not support mobile — no phone-width layouts, no touch-sized targets. See "Rule 6" in [frontend/src/AGENTS.md](frontend/src/AGENTS.md). -- **quill is for MCP apps, the desktop app, and the web app UI behind the `today-rail-nav` feature flag; LemonUI is for the rest of the web app.** `[review]` `today-rail-nav` gates a quill redesign of the whole web app UI, built alongside the current UI and kept behind the flag for months. Build or change UI on that path with quill, and read [frontend/src/design.md](frontend/src/design.md) first: it holds the components, tokens and decision rules. The current UI stays on LemonUI while the flag exists: do not add quill to it or convert it in place. On that flag-off path in `frontend/src/` and `products/*/frontend/` use LemonUI, including menus — `LemonMenu` with a `LemonButton` trigger. `lib/ui/DropdownMenu` (Radix) is legacy; do not add new ones. Where quill is right, do not mix the two inside one component, and remember quill uses Base UI's `render` prop, not Radix's `asChild`. Read [primitives/AGENTS.md](packages/quill/packages/primitives/AGENTS.md) before importing quill — it covers component choice and spacing. Charts: [/working-with-charts](.agents/skills/working-with-charts/SKILL.md) for consumers, [charts/AGENTS.md](packages/quill/packages/charts/AGENTS.md) for library changes. DataTable and DateTimePicker: [components/AGENTS.md](packages/quill/packages/components/AGENTS.md). -- Use tailwind utility classes over inline styles, and `lib/dayjs` over a direct dayjs import. `[review]` oxlint warns on a `style` prop through `react/forbid-dom-props`, but a warning does not block. - -### Comments - -- **Explain _why_, not _what_,** and only where a future reader with no access to this PR or chat would otherwise be confused. `[review]` Default to one line. Python tests get no doc comments. -- **Never log change history or chat context in code.** `[review]` No "previously did X, now does Y", no "per ", no "changed because…", no "AI:" or "agent:" notes. That belongs in the commit message and PR description. -- **When refactoring or moving code, keep the existing comments** unless the change actually makes them obsolete. `[review]` -- Use ASD-STE100 Simplified Technical English: active voice, simple tenses, one idea per sentence, consistent terms. `[review]` - -### Tests - -- **Every new test must catch a realistic regression no existing test catches.** `[review]` If you cannot name that regression, do not add the test. Assert observable behavior through the public interface, not implementation details, and keep it cheap — deterministic, isolated, at the lowest level that catches the bug. See `/writing-tests`. -- **Extend a relevant existing test rather than adding a standalone one** where practical, and parameterize variations of the same behavior (`parameterized` in Python, `test.each` in Jest). `[review]` -- Jest: one top-level `describe` per file. `[review]` Node.js Jest: `.test.ts` by default, `.serial.test.ts` only when shared mutable infrastructure cannot be isolated (a test that resets a shared database). - -### Markdown and prose - -- Semantic line breaks, no hard wrapping. American English spelling. `[review]` +Read [.agents/code-style.md](.agents/code-style.md) before you write Python, frontend code, comments, tests, or Markdown, or choose between quill and LemonUI. ## User-facing copy -For any text a person reads (UI labels, tooltips, empty/error states, notifications, docs, support replies). Invoke `/writing-user-facing-copy` before writing or editing it — that skill carries the full voice, em-dash, and feature-naming rules. When unsure whether copy reads well, ask a human. - -- Sentence case, not Title Case: capitalize only the first word and proper nouns ('Product analytics', 'Save as view'). -- Avoid the tells of AI-generated text: em dashes (—), "not just X, but Y", rule-of-three padding, hedging preambles. Write like a person typed it; if you can't tell, ask a human. -- Plain language, no jargon. Use the labels users see, not internal names (`surveyPopupDelaySeconds` becomes "Delay the survey popup"). -- Be direct and friendly: short sentences, consistent tone across surfaces. -- Errors and empty states guide, don't dead-end: say what happened and the next action. +Invoke `/writing-user-facing-copy` before you write or edit text a person reads. It carries the voice, sentence-case, em-dash, and feature-naming rules. When unsure whether copy reads well, ask a human. ## Feature usage tracking -- **Every feature is tracked end to end.** A user's path through a feature, from entry to outcome, emits events that show whether people start it, finish it, and where they drop off. -- **Features adjacent to your work get tracking too.** A feature is adjacent when it shares a scene or a user flow with your change. When an adjacent feature has no tracking, add it in the same change. -- **Check usage of tracked adjacent features.** Run a subagent that queries each tracked adjacent feature's usage over the last 6 months. - - Fewer than 100 interactions a week counts as low usage. Zero events in 6 months is the extreme case of low usage. - - Before you act on low usage, confirm the tracking fires: trigger the feature locally and check that the events arrive. Missing events can mean broken tracking, not an unused feature. - - Post each low-usage feature and its usage numbers in the platform UX Slack channel (Channel ID: `C08499A7REU`). The decision to keep or remove the feature is made there. Do not remove a feature before that decision. +Read [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md) before you add or change a feature a user interacts with, or remove one for low usage. ## Agent automation -When automating a convention, try these in order — only fall back to the next if the previous isn't suitable: - -1. **Linters** (ruff, oxlint, semgrep) — code pattern enforcement, always paired with CI -2. **lint-staged / husky** — file-level validation or warnings at commit time -3. **Skills** (`.agents/skills/`) — scaffold with `hogli init:skill` -4. **AGENTS.md / CLAUDE.md instructions** — when automated enforcement isn't suitable - -Claude Code hooks are reserved for environment bootstrapping (`SessionStart` only) — do not add `PreToolUse`, `PostToolUse`, or `Notification` hooks as they add latency and are fragile. -Changes to `.claude/hooks/` trigger a warning from the `pre-commit` hook; lint-staged allows only repo-wide keys in `.claude/settings.json`, because personal settings belong in the gitignored `.claude/settings.local.json`. -A warn-only check belongs in the `pre-commit` hook body rather than in a lint-staged task, because lint-staged discards the output of every task that exits 0. +Read [.agents/agent-automation.md](.agents/agent-automation.md) before you automate a convention, or add a Claude Code hook, a lint-staged task, or a pre-commit check. ### Mandatory skill invocation @@ -321,7 +206,7 @@ ALWAYS invoke the matching skill **first** — do not skip it, and do not attemp - `/modifying-taxonomic-filter` — any TaxonomicFilter change - `/profiling-slow-api-endpoints` — an endpoint, list, picker, or scene is slow, a p95 latency number needs explaining, or a Postgres query plan needs checking against production rather than a local database - `/placing-product-frontend-code` — adding a frontend file or directory for a product, or deciding between `products//frontend/` and `frontend/src/scenes//` -- [`products/conversations/skills/organizing-conversations-code/SKILL.md`](products/conversations/skills/organizing-conversations-code/SKILL.md) — adding, moving, renaming, or reviewing files under `products/conversations/` +- `/organizing-conversations-code` — adding, moving, renaming, or reviewing files under `products/conversations/` - `/integrating-with-posthog-ai` — making a product surface work with PostHog AI: injecting scene context or custom instructions, reacting to the agent's tool calls, or rendering your product's tool cards in a thread - `/sending-notifications` — adding notification support - `/routing-outbound-api-calls` — adding or changing Python code that calls a third-party HTTP API, or changing any domain under `posthog/egress/` (budget, lanes, identity, metrics, headers) diff --git a/docs/internal/ci-things-already-tried.md b/docs/internal/ci-things-already-tried.md index ae607e2aa1e4..27ee9a461728 100644 --- a/docs/internal/ci-things-already-tried.md +++ b/docs/internal/ci-things-already-tried.md @@ -695,7 +695,7 @@ One narrow PR from the same window did merge. [#41620](https://github.com/PostHo The setting is in `posthog/settings/data_stores.py`, and `Person.Meta.db_table` reads it. It defaults to `posthog_person`, so the cutover is off. The switch exists. The eight PRs above failed at what surrounds it: the test setup, the dual reads, and the partition guard. `posthog_person_new` comes from the sqlx migrations in `rust/persons_migrations/`. Three Dagster jobs read it, and one carries a comment about a future name swap. -Person and group data now goes through the gRPC client in `posthog/personhog_client/`. `AGENTS.md` makes that client the required interface and prohibits new ORM queries against the person tables. +Person and group data now goes through the gRPC client in `posthog/personhog_client/`. `.agents/architecture.md` makes that client the required interface and prohibits new ORM queries against the person tables. Read this entry before you propose a Django-level cutover. The setting is already there. What is missing is everything that must be true before a person changes its value. diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md index 5be7cb0fa07b..31b32cfd3a13 100644 --- a/frontend/src/AGENTS.md +++ b/frontend/src/AGENTS.md @@ -1,6 +1,6 @@ # Frontend agent guide (`frontend/src`) -Applies to any change under `frontend/src`. This is a **discovery + cadence** guide: the rules below exist because agents tend to generate before they look. The root `AGENTS.md` and the quill package guides remain authoritative, except that [`design.md`](./design.md) wins over a quill package guide for UI behind `today-rail-nav` — this file does not repeat them, it points at them. +Applies to any change under `frontend/src`. This is a **discovery + cadence** guide: the rules below exist because agents tend to generate before they look. The root `AGENTS.md`, [`.agents/code-style.md`](../../.agents/code-style.md), and the quill package guides remain authoritative, except that [`design.md`](./design.md) wins over a quill package guide for UI behind `today-rail-nav` — this file does not repeat them, it points at them. ## Rule 1 — Reuse before you create @@ -34,7 +34,7 @@ If nothing fits, say so and propose extending the existing component before addi The same goes for patterns, not just components: before building a new scene or view, read 2–3 comparable ones and model yours on those that follow these rules. Precedent that violates Rule 5 or `/writing-ui-components` is legacy to route around, not license to repeat — conventions outrank precedent, and compliant precedent outranks invention. -> LemonUI vs quill lives in the root `AGENTS.md` ("Code Style → Frontend (quill vs LemonUI)"). If you're working somewhere quill applies (an MCP app, the desktop app, UI behind `today-rail-nav`), [`design.md`](./design.md) and `packages/quill/packages/primitives/AGENTS.md` have its component-choice and spacing rules, and the two libraries must not be mixed inside one component's internals. +> LemonUI vs quill lives in [`.agents/code-style.md`](../../.agents/code-style.md) ("Frontend"). If you're working somewhere quill applies (an MCP app, the desktop app, UI behind `today-rail-nav`), [`design.md`](./design.md) and `packages/quill/packages/primitives/AGENTS.md` have its component-choice and spacing rules, and the two libraries must not be mixed inside one component's internals. ## Rule 2 — A product's UI goes in `products//frontend/` @@ -62,7 +62,7 @@ When touching `lib/api`, `api.get<`, `api.create<`, or any handwritten API inter ## Rule 4 — Business logic in kea, not React hooks -Covered by the root `AGENTS.md` (Code Style → Frontend). The discovery hint for this tree: if a scene/component has a `*Logic.ts`, that's where actions/reducers/selectors/listeners belong. See `/writing-kea-logics` and `/using-kea-disposables`. +Covered by [`.agents/code-style.md`](../../.agents/code-style.md) ("Frontend"). The discovery hint for this tree: if a scene/component has a `*Logic.ts`, that's where actions/reducers/selectors/listeners belong. See `/writing-kea-logics` and `/using-kea-disposables`. ## Rule 5 — Structure and abstraction @@ -127,7 +127,7 @@ These are slow; run them at the right moment, not after every edit. ## Lint & format -Run `pnpm --filter=@posthog/frontend fix` before finishing. It applies safe Oxlint fixes, never suggestion fixes, and always runs Oxfmt; it fails if either tool fails. `format` runs Oxfmt only, while `lint` and `format:check` only verify. Config: root `.oxlintrc.json`. CSS, spelling, and copy-casing rules live in the root `AGENTS.md` (Code Style). +Run `pnpm --filter=@posthog/frontend fix` before finishing. It applies safe Oxlint fixes, never suggestion fixes, and always runs Oxfmt; it fails if either tool fails. `format` runs Oxfmt only, while `lint` and `format:check` only verify. Config: root `.oxlintrc.json`. CSS and spelling rules live in [`.agents/code-style.md`](../../.agents/code-style.md), and copy-casing rules in `/writing-user-facing-copy`. ## Adding actions to a scene @@ -137,7 +137,7 @@ Adding a button/toggle/action to a scene's `ScenePanel`? It must also go in that ## Deeper references -- Root `AGENTS.md` — full Code Style + architecture rules (authoritative). +- [`.agents/code-style.md`](../../.agents/code-style.md) and [`.agents/architecture.md`](../../.agents/architecture.md) — full code style and architecture rules (authoritative). - `layout/scenes/AGENTS.md` (scene action surfaces: `ScenePanel` + `SceneMenuBar` dual-write rule). - `packages/quill/packages/primitives/AGENTS.md` — component selection matrix. - `/working-with-charts`: chart consumption workflow with links to the relevant examples and package docs. diff --git a/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py b/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py index c0d7aa555822..9118556f97d5 100644 --- a/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py +++ b/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py @@ -16,8 +16,11 @@ # is not loaded by a blocking job, so it cannot back a tag. LOADED_SEMGREP_ROOTS = (".semgrep/rules/security", ".semgrep/rules/devex") -# Sections whose every rule must declare what enforces it. -TAGGED_SECTIONS = (("## Architecture guidelines", "## Code Style"), ("## Code Style", "## User-facing copy")) +# Agent guides whose every rule must declare what enforces it. +TAGGED_GUIDES = (".agents/architecture.md", ".agents/code-style.md") + +# Every agent guide that may carry a [lint: ...] tag. +GUIDES_WITH_TAGS = ("AGENTS.md", ".agents/ci.md", *TAGGED_GUIDES) # Tags naming a command or CI job rather than a rule id. Add to this set only # when no single rule id covers the check. @@ -32,21 +35,17 @@ def _repo_root() -> Path: return Path(__file__).resolve().parents[3] -def _agents_md(repo_root: Path) -> str: - return (repo_root / "AGENTS.md").read_text() +def _read(repo_root: Path, guides: tuple[str, ...]) -> str: + return "\n".join((repo_root / guide).read_text() for guide in guides) -def _tags(agents_md: str) -> list[str]: - tags = (tag.strip() for marker in MARKER.findall(agents_md) for tag in marker.split(",")) +def _tags(guides: str) -> list[str]: + tags = (tag.strip() for marker in MARKER.findall(guides) for tag in marker.split(",")) return [tag for tag in tags if tag != LEGEND_PLACEHOLDER] -def _rules(agents_md: str) -> list[str]: - rules: list[str] = [] - for start, end in TAGGED_SECTIONS: - section = agents_md[agents_md.index(start) : agents_md.index(end)] - rules.extend(line for line in section.split("\n") if line.startswith("- ")) - return rules +def _rules(guides: str) -> list[str]: + return [line for line in guides.split("\n") if line.startswith("- ")] def _semgrep_ids(repo_root: Path) -> set[str]: @@ -98,13 +97,13 @@ def test_agents_md_lint_tags_name_something_real() -> None: repo_root = _repo_root() semgrep_ids = _semgrep_ids(repo_root) - tags = _tags(_agents_md(repo_root)) - assert tags, "AGENTS.md has no [lint: ...] tags — did the marker format change?" + tags = _tags(_read(repo_root, GUIDES_WITH_TAGS)) + assert tags, "The agent guides have no [lint: ...] tags — did the marker format change?" unresolved = [tag for tag in tags if not _resolves(tag, repo_root, semgrep_ids)] assert not unresolved, ( - f"AGENTS.md tags these as machine-enforced, but nothing by that name enforces them: {unresolved}. " + f"The agent guides tag these as machine-enforced, but nothing by that name enforces them: {unresolved}. " "Either the rule was renamed or deleted, or a ruff code moved to `ignore` or a per-file exemption " "list (fix the tag, or retag the " f"rule as [review]), or the tag names a command rather than a rule id (add it to FREE_FORM in {Path(__file__).name})." @@ -113,13 +112,13 @@ def test_agents_md_lint_tags_name_something_real() -> None: def test_every_architecture_and_code_style_rule_is_tagged() -> None: # An untagged rule reads as review-only without saying so, which is the ambiguity the tags remove. - rules = _rules(_agents_md(_repo_root())) - assert rules, "Found no rules to check — did the section headings change?" + rules = _rules(_read(_repo_root(), TAGGED_GUIDES)) + assert rules, "Found no rules to check — did the guides move?" untagged = [rule[:80] for rule in rules if not WELL_FORMED.search(rule)] assert not untagged, ( - f"These AGENTS.md rules carry no well-formed enforcement tag: {untagged}. " + f"These architecture and code style rules carry no well-formed enforcement tag: {untagged}. " "Add `[lint: ]` when a linter or invariant test blocks the violation, or `[review]` when " "nothing catches it. An empty or unclosed marker does not count." ) From 68daefb0809b5deb1c194d200a1cc2de4eb7586d Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:08:44 +0000 Subject: [PATCH 2/6] chore(agents): close review gaps in the agents file split Add the new guides to the frozen stamphog policy copy, repoint the remaining references to moved rules, and make the tag test read every .agents guide and reject rules under the root pointer headings. Widen three pointer lines to cover every trigger of the rules they replace, and promote Mandatory skill invocation to its own section. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/editing-agents-md/SKILL.md | 2 +- .agents/skills/writing-kea-logics/SKILL.md | 4 +-- .../writing-kea-logics/references/testing.md | 2 +- .../skills/writing-user-facing-copy/SKILL.md | 2 +- .depot/workflows/ci-backend.yml | 12 ++++---- AGENTS.md | 8 ++--- .../test_agents_md_enforcement_tags.py | 29 +++++++++++++++++-- products/ai_observability/AGENTS.md | 5 ++-- .../management/commands/reset_review_hog.py | 2 +- products/stamphog/AGENTS.md | 2 +- .../packages/pr-approval-agent/gates.py | 2 +- .../packages/pr-approval-agent/test_policy.py | 1 + 12 files changed, 47 insertions(+), 24 deletions(-) diff --git a/.agents/skills/editing-agents-md/SKILL.md b/.agents/skills/editing-agents-md/SKILL.md index 6df0bfeeb5ce..d6d6f59580fc 100644 --- a/.agents/skills/editing-agents-md/SKILL.md +++ b/.agents/skills/editing-agents-md/SKILL.md @@ -27,7 +27,7 @@ Do not reorder for attention. Reorder only when a heading makes a rule easier fo ## 1. Does this belong in the file at all? -Work down this ladder and stop at the first rung that fits. It is the same ladder `AGENTS.md` states for itself. +Work down this ladder and stop at the first rung that fits. It is the same ladder [`.agents/agent-automation.md`](../../agent-automation.md) states for the repo. | Rung | Mechanism | Use when | | ---- | ------------------------------------------------------------ | ------------------------------------------------------- | diff --git a/.agents/skills/writing-kea-logics/SKILL.md b/.agents/skills/writing-kea-logics/SKILL.md index 06fe7ca4bd29..b887c99618cd 100644 --- a/.agents/skills/writing-kea-logics/SKILL.md +++ b/.agents/skills/writing-kea-logics/SKILL.md @@ -33,8 +33,8 @@ If your work overlaps it, read the companion skill first. ## Core principles 1. **Business logic lives in a logic, not in a component.** - [CLAUDE.md](../../../CLAUDE.md) is explicit: "If there is a kea logic file, write all - business logic there, avoid React hooks at all costs." Hooks are for view concerns. + [`.agents/code-style.md`](../../code-style.md) is explicit: "Business logic goes in the + kea logic file, not a React hook." Hooks are for view concerns. 2. **One concept, one source of truth.** Pick exactly one of: action-driven reducer, derived selector, async loader. Don't mirror the same value into multiple places. diff --git a/.agents/skills/writing-kea-logics/references/testing.md b/.agents/skills/writing-kea-logics/references/testing.md index f5d372783565..4f90d79bc588 100644 --- a/.agents/skills/writing-kea-logics/references/testing.md +++ b/.agents/skills/writing-kea-logics/references/testing.md @@ -106,7 +106,7 @@ rule. Never add logic-path exceptions to the Jest breakpoint wrapper. ## Parameterise -Per repo convention ([CLAUDE.md](../../../../CLAUDE.md)), parameterise variations of the +Per repo convention ([`.agents/code-style.md`](../../../code-style.md)), parameterise variations of the same shape rather than copy-pasting describe bodies: ```ts diff --git a/.agents/skills/writing-user-facing-copy/SKILL.md b/.agents/skills/writing-user-facing-copy/SKILL.md index 9d1d65921a58..1a78381d2aa7 100644 --- a/.agents/skills/writing-user-facing-copy/SKILL.md +++ b/.agents/skills/writing-user-facing-copy/SKILL.md @@ -21,7 +21,7 @@ Does not apply to: code comments, commit messages, log lines, variable/function Write the way a person would. Neutral and humane. - **Sentence case.** Capitalize only the first word and proper nouns. "Save as view", not "Save As View". "Product analytics", not "Product Analytics". -- **Be direct and friendly.** Use short sentences. Say what happened and what to do next. +- **Be direct and friendly.** Use short sentences and keep one tone across surfaces. Say what happened and what to do next. - **Plain language, no jargon.** Use the label the user sees, not the internal name. `surveyPopupDelaySeconds` becomes "Delay the survey popup". - **Don't editorialize.** State what is, not how exciting it is. Cut "powerful", "seamless", "effortless", "simply", "just", "easily", "supercharge", "unlock". - **No sales-y or edgy copy.** No marketing hooks, no clever one-liners, no hype. diff --git a/.depot/workflows/ci-backend.yml b/.depot/workflows/ci-backend.yml index 226ec6ed26ca..8a15613763d6 100644 --- a/.depot/workflows/ci-backend.yml +++ b/.depot/workflows/ci-backend.yml @@ -493,19 +493,17 @@ jobs: tasks_temporal: - 'products/tasks/backend/temporal/**/*' repo_invariants: - # posthog/test/repo_invariants/ reads AGENTS.md, the tagged .agents/ guides, and the semgrep - # rule ids it names. Both are excluded from the backend filter - # (markdown is dropped outright, .semgrep is not listed), so a - # change to either would skip repo-checks and report green while + # posthog/test/repo_invariants/ reads AGENTS.md, the .agents/ + # guides, and the semgrep rule ids they name. All are excluded + # from the backend filter (markdown is dropped outright, + # .semgrep is not listed), so a change to any would skip repo-checks and report green while # leaving a tag pointing at a rule that no longer exists. # The out-of-band domain ratchet reads these trees, so a resolver added there must run it. - 'nodejs/src/**' - 'rust/**' - 'products/desktop/**' - 'AGENTS.md' - - '.agents/architecture.md' - - '.agents/code-style.md' - - '.agents/ci.md' + - '.agents/*.md' - '.semgrep/**' - 'pyproject.toml' # A generated file in a frontend-only PR still needs a known producer. diff --git a/AGENTS.md b/AGENTS.md index 56c1b397939f..c6bfe658da46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,7 +134,7 @@ Examples: ## CI / GitHub Actions -Read [.agents/ci.md](.agents/ci.md) before you change a workflow, `.nvmrc`, a publish job, or tests that read event properties. +Read [.agents/ci.md](.agents/ci.md) before you change a workflow, `.nvmrc`, a publish job, event ingestion, or tests that read event properties, and when you debug flaky CI. ## Security @@ -159,7 +159,7 @@ Read [.agents/security.md](.agents/security.md#content-security-policy) for the ## Architecture guidelines -Read [.agents/architecture.md](.agents/architecture.md) before you add or change application code: entrypoints, `team_id` scoping, personhog, transactions, API schemas, Celery and Temporal, outbound calls, storage, settings, Django admin, and the `services/llm-gateway` freeze. +Read [.agents/architecture.md](.agents/architecture.md) before you add or change application or dev-stack code: entrypoints, `team_id` scoping, personhog, transactions, API schemas, Celery and Temporal, outbound calls, storage, settings, Django admin, and the `services/llm-gateway` freeze. ## Code Style @@ -175,9 +175,9 @@ Read [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md) befo ## Agent automation -Read [.agents/agent-automation.md](.agents/agent-automation.md) before you automate a convention, or add a Claude Code hook, a lint-staged task, or a pre-commit check. +Read [.agents/agent-automation.md](.agents/agent-automation.md) before you automate a convention, or change a Claude Code hook, `.claude/settings.json`, a lint-staged task, or a pre-commit check. -### Mandatory skill invocation +## Mandatory skill invocation Each entry below is a trigger, not a task list: it fires on what the diff contains, whether you are writing that code or reading someone else's. ALWAYS invoke the matching skill **first** — do not skip it, and do not attempt the work or the review without loading it. diff --git a/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py b/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py index 9118556f97d5..e9305675907a 100644 --- a/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py +++ b/posthog/test/repo_invariants/test_agents_md_enforcement_tags.py @@ -19,8 +19,8 @@ # Agent guides whose every rule must declare what enforces it. TAGGED_GUIDES = (".agents/architecture.md", ".agents/code-style.md") -# Every agent guide that may carry a [lint: ...] tag. -GUIDES_WITH_TAGS = ("AGENTS.md", ".agents/ci.md", *TAGGED_GUIDES) +# Root sections that only point at a tagged guide, so a rule added there would skip the tag check. +POINTER_SECTIONS = ("## Architecture guidelines", "## Code Style") # Tags naming a command or CI job rather than a rule id. Add to this set only # when no single rule id covers the check. @@ -39,6 +39,17 @@ def _read(repo_root: Path, guides: tuple[str, ...]) -> str: return "\n".join((repo_root / guide).read_text() for guide in guides) +def _guides_with_tags(repo_root: Path) -> tuple[str, ...]: + agents_dir = repo_root / ".agents" + return ("AGENTS.md", *sorted(str(path.relative_to(repo_root)) for path in agents_dir.glob("*.md"))) + + +def _section(markdown: str, heading: str) -> str: + start = markdown.index(heading) + len(heading) + end = markdown.find("\n## ", start) + return markdown[start:] if end == -1 else markdown[start:end] + + def _tags(guides: str) -> list[str]: tags = (tag.strip() for marker in MARKER.findall(guides) for tag in marker.split(",")) return [tag for tag in tags if tag != LEGEND_PLACEHOLDER] @@ -97,7 +108,7 @@ def test_agents_md_lint_tags_name_something_real() -> None: repo_root = _repo_root() semgrep_ids = _semgrep_ids(repo_root) - tags = _tags(_read(repo_root, GUIDES_WITH_TAGS)) + tags = _tags(_read(repo_root, _guides_with_tags(repo_root))) assert tags, "The agent guides have no [lint: ...] tags — did the marker format change?" unresolved = [tag for tag in tags if not _resolves(tag, repo_root, semgrep_ids)] @@ -122,3 +133,15 @@ def test_every_architecture_and_code_style_rule_is_tagged() -> None: "Add `[lint: ]` when a linter or invariant test blocks the violation, or `[review]` when " "nothing catches it. An empty or unclosed marker does not count." ) + + +def test_root_pointer_sections_hold_no_rules() -> None: + # A rule written under a pointer heading in the root file would escape the tag check above. + agents_md = (_repo_root() / "AGENTS.md").read_text() + + misplaced = [heading for heading in POINTER_SECTIONS if _rules(_section(agents_md, heading))] + + assert not misplaced, ( + f"These AGENTS.md sections only point at a guide under .agents/, but hold rules: {misplaced}. " + "Move each rule into the guide the section points at." + ) diff --git a/products/ai_observability/AGENTS.md b/products/ai_observability/AGENTS.md index 1ae3bb5abc5c..4d22a534f5e5 100644 --- a/products/ai_observability/AGENTS.md +++ b/products/ai_observability/AGENTS.md @@ -1,7 +1,8 @@ # AI observability agent guide -Repo-wide conventions live in [`../../AGENTS.md`](../../AGENTS.md) — kea over hooks, generated -API over `api.get`, `ph_scoped_capture` in Celery, etc. Read that first. +Repo-wide conventions live in [`../../AGENTS.md`](../../AGENTS.md) and the guides it links, +[`code-style.md`](../../.agents/code-style.md) and [`architecture.md`](../../.agents/architecture.md) — kea over hooks, generated +API over `api.get`, `ph_scoped_capture` in Celery, etc. Read those first. This file is only for rules that are specific to `products/ai_observability/` (and to the query runners in `posthog/hogql_queries/ai/`). diff --git a/products/review_hog/backend/management/commands/reset_review_hog.py b/products/review_hog/backend/management/commands/reset_review_hog.py index b14ce18a0dd8..efbf62156eda 100644 --- a/products/review_hog/backend/management/commands/reset_review_hog.py +++ b/products/review_hog/backend/management/commands/reset_review_hog.py @@ -46,7 +46,7 @@ def handle(self, *args: Any, **options: Any) -> None: skip_confirm: bool = options["yes"] # A full local wipe is genuinely cross-team, so go through `unscoped()` — the fail-closed - # managers would otherwise raise without a team context (CLAUDE.md cross-team escape hatch). + # managers would otherwise raise without a team context (.agents/architecture.md cross-team escape hatch). artefacts = ReviewReportArtefact.objects.unscoped() reports = ReviewReport.objects.unscoped() configs = ReviewSkillConfig.objects.unscoped() diff --git a/products/stamphog/AGENTS.md b/products/stamphog/AGENTS.md index 7257fa8af1e3..9b42ad53070c 100644 --- a/products/stamphog/AGENTS.md +++ b/products/stamphog/AGENTS.md @@ -64,7 +64,7 @@ A team left with none of those is not a digest audience at all. Subtracting rather than only dropping the all-generated case came from the same channel getting swept again: one regenerated file next to one real one read as a two-file stake, which is above the graze rule's threshold, so the sweep looked like ownership. Three things keep it from being the harmless-file rule above. It decides who hears about a merge, never whether code is safe to approve. -The root `CLAUDE.md` forbids hand-editing those files, so the hand-edited-and-still-ships case does not apply to this path. +[`.agents/architecture.md`](../../.agents/architecture.md) forbids hand-editing those files, so the hand-edited-and-still-ships case does not apply to this path. And it names one exact directory shape rather than any `generated/` directory, which is the match that would catch hand-editable code elsewhere. Both sides are read with `compare_diff`, from the base and head shas the run and the payload already fixed. diff --git a/products/stamphog/packages/pr-approval-agent/gates.py b/products/stamphog/packages/pr-approval-agent/gates.py index 345335d595cb..582a36fb45d9 100644 --- a/products/stamphog/packages/pr-approval-agent/gates.py +++ b/products/stamphog/packages/pr-approval-agent/gates.py @@ -262,7 +262,7 @@ def build_ownership(repo_root: Path, sources: tuple[OwnershipSource, ...]) -> li # # Deliberately narrow. This is a "these files carry no news" rule, which AGENTS.md says needs an # argument, and the argument only holds for this one directory shape: the repo forbids hand-editing -# it (root CLAUDE.md, "don't edit them manually, change serializers and rerun"), every product's +# it (.agents/architecture.md, "never hand-edit, change the serializer and regenerate"), every product's # generated directory has this exact path, and its only consumer decides who hears about a merge # rather than whether code is safe to approve. A bare `generated/` match would also catch # hand-editable directories elsewhere in the tree, which is the case AGENTS.md warns about. diff --git a/products/stamphog/packages/pr-approval-agent/test_policy.py b/products/stamphog/packages/pr-approval-agent/test_policy.py index e54a1e93d217..91a6f54b0cab 100644 --- a/products/stamphog/packages/pr-approval-agent/test_policy.py +++ b/products/stamphog/packages/pr-approval-agent/test_policy.py @@ -152,6 +152,7 @@ "paths": [ "^agents\\.md$", "^claude\\.md$", + "^\\.agents/(agent-automation|architecture|ci|code-style|feature-usage-tracking)\\.md$", "^\\.github/pull_request_template\\.md$", "^\\.agents/skills/writing-pr-descriptions/", "^\\.claude/settings\\.json$", From 94438760c70f5060c6a9aa337425ebe30a89d80b Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:10:51 +0000 Subject: [PATCH 3/6] chore(agents): drop a paraphrased quote in a stamphog comment Co-Authored-By: Claude Opus 5.5 --- products/stamphog/packages/pr-approval-agent/gates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/products/stamphog/packages/pr-approval-agent/gates.py b/products/stamphog/packages/pr-approval-agent/gates.py index 582a36fb45d9..ea10ded0e1e3 100644 --- a/products/stamphog/packages/pr-approval-agent/gates.py +++ b/products/stamphog/packages/pr-approval-agent/gates.py @@ -262,7 +262,7 @@ def build_ownership(repo_root: Path, sources: tuple[OwnershipSource, ...]) -> li # # Deliberately narrow. This is a "these files carry no news" rule, which AGENTS.md says needs an # argument, and the argument only holds for this one directory shape: the repo forbids hand-editing -# it (.agents/architecture.md, "never hand-edit, change the serializer and regenerate"), every product's +# it (.agents/architecture.md: change the serializer and regenerate instead), every product's # generated directory has this exact path, and its only consumer decides who hears about a merge # rather than whether code is safe to approve. A bare `generated/` match would also catch # hand-editable directories elsewhere in the tree, which is the case AGENTS.md warns about. From ad0af945074626a06d865946b09ff4ebee9daf12 Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:30:06 +0000 Subject: [PATCH 4/6] chore(agents): route each task to its topic guide from one table Haiku and Sonnet skipped the per-section pointers when a task only showed a diff to review, and missed guide-only rules there. A table that maps each area to its guide, and says a review counts, made every measured run open the right guide. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index c6bfe658da46..34b5df81070b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,6 +132,21 @@ Examples: - ❌ A PR description that quotes verbatim what a coworker said in a Slack thread - ❌ A test fixture adapted from a real support conversation with the names and domains replaced +## Read the guide for your area first + +Most topic rules live in guides under `.agents/`, not in this file. +Before your first edit, plan, or review finding, open every guide whose area the task touches. +Reviewing a diff counts: the diff touches the areas it changes. + +| The task touches | Read first | +| ------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------- | +| Python or Django backend: models, views, serializers, Celery, Temporal, outbound calls, settings | [.agents/architecture.md](.agents/architecture.md) and [.agents/code-style.md](.agents/code-style.md) | +| Frontend: components, kea logic, buttons, forms | [.agents/code-style.md](.agents/code-style.md) and [frontend/src/AGENTS.md](frontend/src/AGENTS.md) | +| A new or changed button, menu item, form, or scene | [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md) | +| GitHub workflows, `.nvmrc`, publish jobs | [.agents/ci.md](.agents/ci.md) | +| Claude Code hooks, lint-staged, pre-commit, `.claude/settings.json` | [.agents/agent-automation.md](.agents/agent-automation.md) | +| Auth, secrets, raw SQL, HogQL string building | [.agents/security.md](.agents/security.md) | + ## CI / GitHub Actions Read [.agents/ci.md](.agents/ci.md) before you change a workflow, `.nvmrc`, a publish job, event ingestion, or tests that read event properties, and when you debug flaky CI. From d2bec2cd96878984fce6c8aee4fd9c1018bb442a Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:10:07 +0000 Subject: [PATCH 5/6] chore(agents): make the routing table the only trigger for each guide The table and the old pointer lines named different triggers for the same guide, so they would drift. Each guide's row now holds its full trigger, and the section under each heading only names the guide. The editing-agents-md skill says where a new trigger goes. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/editing-agents-md/SKILL.md | 2 +- AGENTS.md | 29 ++++++++++++----------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/.agents/skills/editing-agents-md/SKILL.md b/.agents/skills/editing-agents-md/SKILL.md index d6d6f59580fc..b08235da408b 100644 --- a/.agents/skills/editing-agents-md/SKILL.md +++ b/.agents/skills/editing-agents-md/SKILL.md @@ -93,7 +93,7 @@ All three get the same smells check. The budget differs. **Root `AGENTS.md`** — loads every session, in every tool, for every task. Strict. A rule earns its place only if it applies across the repo, or if getting it wrong is expensive and nothing catches it (the public-repo rules are the model). Keep the enforcement tags current. -**Topic guides** (`.agents/.md`: architecture, code style, CI, feature usage tracking, agent automation) — load when the root file's one-line pointer sends an agent there. A repo-wide rule that applies only to one kind of work, such as application code or CI workflows, belongs in a guide. Each guide keeps its heading and pointer line in the root file, and the pointer says when to read it. +**Topic guides** (`.agents/.md`: architecture, code style, CI, feature usage tracking, agent automation, security) load when the routing table in the root file's "Read the guide for your area first" section sends an agent there. A repo-wide rule that applies only to one kind of work, such as application code or CI workflows, belongs in a guide. Each guide keeps its heading in the root file, and its table row is the only place that says when to read it. Put a new trigger in the row, never in the section under the heading, because two triggers drift apart. **Nested `AGENTS.md`** (about 27 of them: `posthog/temporal/`, `rust/`, `frontend/src/`, most of `products/`) — loads when an agent works in that subtree, so a local convention is cheap here and expensive in the root. Move a directory-specific rule down rather than up. No tag convention required, though it does no harm. diff --git a/AGENTS.md b/AGENTS.md index 34b5df81070b..ba3d9bfc4a62 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,19 +137,20 @@ Examples: Most topic rules live in guides under `.agents/`, not in this file. Before your first edit, plan, or review finding, open every guide whose area the task touches. Reviewing a diff counts: the diff touches the areas it changes. +Each guide's row below holds its full trigger. A section may still link one part of a guide for a rule it states. -| The task touches | Read first | -| ------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------- | -| Python or Django backend: models, views, serializers, Celery, Temporal, outbound calls, settings | [.agents/architecture.md](.agents/architecture.md) and [.agents/code-style.md](.agents/code-style.md) | -| Frontend: components, kea logic, buttons, forms | [.agents/code-style.md](.agents/code-style.md) and [frontend/src/AGENTS.md](frontend/src/AGENTS.md) | -| A new or changed button, menu item, form, or scene | [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md) | -| GitHub workflows, `.nvmrc`, publish jobs | [.agents/ci.md](.agents/ci.md) | -| Claude Code hooks, lint-staged, pre-commit, `.claude/settings.json` | [.agents/agent-automation.md](.agents/agent-automation.md) | -| Auth, secrets, raw SQL, HogQL string building | [.agents/security.md](.agents/security.md) | +| Guide | Read it before you write or review | +| ---------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [.agents/architecture.md](.agents/architecture.md) | Application or dev-stack code in any language, such as a model, view, serializer, task, or React component: entrypoints, `team_id` scoping, personhog, transactions, API schemas, Celery and Temporal, outbound calls, storage, Django settings, Django admin, and the `services/llm-gateway` freeze | +| [.agents/code-style.md](.agents/code-style.md) | Python; frontend code such as a component, kea logic, a button, or a form; comments; tests; Markdown; or a choice between quill and LemonUI | +| [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md) | A feature a user interacts with, such as a new action, menu item, or scene, a change to what a user can do there, or the removal of a feature for low usage | +| [.agents/ci.md](.agents/ci.md) | A workflow, `.nvmrc`, a publish job, event ingestion, tests that read event properties, or a flaky CI run | +| [.agents/agent-automation.md](.agents/agent-automation.md) | The automation of a convention, a Claude Code hook, `.claude/settings.json`, a lint-staged task, or a pre-commit check | +| [.agents/security.md](.agents/security.md) | Auth, secrets, service-to-service calls, raw SQL, HogQL string building, a new external origin, an inline script under the Content Security Policy, or a PR for a security fix | ## CI / GitHub Actions -Read [.agents/ci.md](.agents/ci.md) before you change a workflow, `.nvmrc`, a publish job, event ingestion, or tests that read event properties, and when you debug flaky CI. +The rules for CI and GitHub Actions live in [.agents/ci.md](.agents/ci.md). Its row in [Read the guide for your area first](#read-the-guide-for-your-area-first) says when to read it. ## Security @@ -162,7 +163,7 @@ The private mirror carries this file, and its `origin` points at the private rep Before the first push of a branch, run `git remote get-url origin`. If it names `posthog-private` and the change is not an embargoed fix, open the PR from a `PostHog/posthog` checkout. Do not add new `INTERNAL_API_SECRET` callers. -Read [.agents/security.md](.agents/security.md) before touching auth, secrets, service-to-service calls, raw SQL, or HogQL string building — it covers least privilege, the injection rules, and how to respond when semgrep flags your code. +[.agents/security.md](.agents/security.md) covers least privilege, the injection rules, and how to respond when semgrep flags your code. `.semgrep/rules/security/` is the enforced set; run `semgrep --config .semgrep/rules/security/ .` to check a change locally. **Treat the Content Security Policy in `CSPMiddleware` as enforced.** @@ -174,11 +175,11 @@ Read [.agents/security.md](.agents/security.md#content-security-policy) for the ## Architecture guidelines -Read [.agents/architecture.md](.agents/architecture.md) before you add or change application or dev-stack code: entrypoints, `team_id` scoping, personhog, transactions, API schemas, Celery and Temporal, outbound calls, storage, settings, Django admin, and the `services/llm-gateway` freeze. +The rules for architecture live in [.agents/architecture.md](.agents/architecture.md). Its row in [Read the guide for your area first](#read-the-guide-for-your-area-first) says when to read it. ## Code Style -Read [.agents/code-style.md](.agents/code-style.md) before you write Python, frontend code, comments, tests, or Markdown, or choose between quill and LemonUI. +The rules for code style live in [.agents/code-style.md](.agents/code-style.md). Its row in [Read the guide for your area first](#read-the-guide-for-your-area-first) says when to read it. ## User-facing copy @@ -186,11 +187,11 @@ Invoke `/writing-user-facing-copy` before you write or edit text a person reads. ## Feature usage tracking -Read [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md) before you add or change a feature a user interacts with, or remove one for low usage. +The rules for feature usage tracking live in [.agents/feature-usage-tracking.md](.agents/feature-usage-tracking.md). Its row in [Read the guide for your area first](#read-the-guide-for-your-area-first) says when to read it. ## Agent automation -Read [.agents/agent-automation.md](.agents/agent-automation.md) before you automate a convention, or change a Claude Code hook, `.claude/settings.json`, a lint-staged task, or a pre-commit check. +The rules for agent automation live in [.agents/agent-automation.md](.agents/agent-automation.md). Its row in [Read the guide for your area first](#read-the-guide-for-your-area-first) says when to read it. ## Mandatory skill invocation From d2ddf4e8c375f18e4985b355930e0b67ec6bb8ea Mon Sep 17 00:00:00 2001 From: "Reichenbach, Michael" <755327+Silthus@users.noreply.github.com> Date: Mon, 26 Oct 2026 19:21:05 +0000 Subject: [PATCH 6/6] chore(agents): tighten the copy skill's tone rule Co-Authored-By: Claude Opus 5.5 --- .agents/skills/writing-user-facing-copy/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/writing-user-facing-copy/SKILL.md b/.agents/skills/writing-user-facing-copy/SKILL.md index 1a78381d2aa7..8ae22a6cee59 100644 --- a/.agents/skills/writing-user-facing-copy/SKILL.md +++ b/.agents/skills/writing-user-facing-copy/SKILL.md @@ -21,7 +21,7 @@ Does not apply to: code comments, commit messages, log lines, variable/function Write the way a person would. Neutral and humane. - **Sentence case.** Capitalize only the first word and proper nouns. "Save as view", not "Save As View". "Product analytics", not "Product Analytics". -- **Be direct and friendly.** Use short sentences and keep one tone across surfaces. Say what happened and what to do next. +- **Be direct and friendly.** Keep one tone across surfaces, say only what happened and what to do next. - **Plain language, no jargon.** Use the label the user sees, not the internal name. `surveyPopupDelaySeconds` becomes "Delay the survey popup". - **Don't editorialize.** State what is, not how exciting it is. Cut "powerful", "seamless", "effortless", "simply", "just", "easily", "supercharge", "unlock". - **No sales-y or edgy copy.** No marketing hooks, no clever one-liners, no hype.