[codex] clarify agent guidance ownership (#4574)

Signed-off-by: Peter Turi <peter.turi@openmeter.io>

Author: turip

.agents/skills/charges/SKILL.md

@@ -65,6 +65,10 @@ The generic rule is:
 - type-specific service subpackages may own reusable realization mechanics when the parent service becomes too broad; keep state-machine decisions in the lifecycle files and move only mechanical operations such as rating snapshots, realization persistence, credit allocation/correction, and realization lineage persistence into these helpers
 - invoice-backed charges must not reach the meta `final` state until their payment lifecycle is fully settled; if a charge is waiting on invoice payment authorization or settlement, keep it in an `active.*` detailed status instead of collapsing to `final`
 
+Adapter rules:
+
+- Keep Ent access transaction-aware in `openmeter/billing/charges/.../adapter`, including shared helper functions. Prefer helpers to accept the adapter/repo handle rather than a raw `*entdb.Client`, so `entutils.TransactingRepo(...)` or `entutils.TransactingRepoWithNoValue(...)` can pass the transaction-bound handle from `ctx`. If a helper must accept a raw client, only call it with the swapped handle's client, such as `tx.db` inside the transacting callback.
+
 Important types:
 
 - `charges.AdvanceChargesInput` identifies the customer whose charges should advance

.agents/skills/samber-lo/SKILL.md

@@ -1,23 +1,45 @@
 ---
 name: samber-lo
-description: Use when writing or refactoring Go code in OpenMeter that can use github.com/samber/lo helpers, especially trivial slice-to-map, map keys/values, mapping, filtering, grouping, uniqueness, set-like conversions, and map entry transformations.
+description: Use when writing or refactoring Go collection/pointer helper code in OpenMeter, especially when choosing between standard library slices/maps helpers and github.com/samber/lo for cloning, copying, equality, sorting, pointer literals, slice-to-map transforms, map keys/values, mapping, filtering, grouping, uniqueness, set-like conversions, and map entry transformations.
 user-invocable: true
 argument-hint: "[collection transformation or lo helper question]"
 allowed-tools: Read, Edit, Write, Bash, Grep, Glob
 ---
 
 # samber/lo
 
-Use `github.com/samber/lo` for small, local collection transformations when it makes the intent clearer than a hand-written loop. OpenMeter pins `github.com/samber/lo v1.53.0` in `go.mod`.
+Use standard library collection helpers first when they express the operation directly, and use `github.com/samber/lo` for small, local collection transformations when it makes the intent clearer than a hand-written loop. OpenMeter pins `github.com/samber/lo v1.53.0` in `go.mod`.
 
-## Default Import
+## Imports
 
 ```go
+import "maps"
+import "slices"
 import "github.com/samber/lo"
 ```
 
 Do not use dot imports. Do not reach for `lo/parallel`, `lo/mutable`, or `lo/it` unless the surrounding package already uses that subpackage or the task explicitly needs it.
 
+## Standard Library First
+
+Prefer `slices` for operations the standard library already names clearly:
+
+- `slices.Clone(s)` for defensive slice copies instead of `append([]T(nil), s...)`.
+- `slices.Contains(s, v)` for membership checks on short or already-available slices.
+- `slices.Sort`, `slices.SortFunc`, and `slices.SortStableFunc` before comparing, logging, serializing, or asserting on order.
+- `slices.Equal` and `slices.EqualFunc` for equality checks.
+- `slices.Concat(a, b, c)` for concatenating known slices.
+- `slices.Compact` after sorting when normalizing a slice.
+
+Prefer `maps` for direct map operations:
+
+- `maps.Clone(m)` for defensive copies.
+- `maps.Copy(dst, src)` when merging maps.
+- `maps.Equal` and `maps.EqualFunc` for equality checks.
+- `maps.Keys(m)` and `maps.Values(m)` when the iterator result is acceptable; collect or sort when a slice is required or order matters.
+
+Prefer `lo.ToPtr(...)`, `lo.FromPtr(...)`, and `lo.FromPtrOr(...)` for pointer literals and pointer defaults. Avoid local wrappers such as `ptr`, `loPtr`, `must`, or `loMust`.
+
 ## Common Helpers
 
 For slice transforms:
@@ -49,6 +71,8 @@ For map work:
 
 Use `*Err` variants such as `MapErr`, `KeyByErr`, `GroupByMapErr`, `MapValuesErr`, or `MapToSliceErr` when the callback can fail and the first error should stop the transform.
 
+For slice-wide invariants where the exact offending element is not important, prefer collecting distinct values with `lo.Map` plus `lo.Uniq` and validating cardinality over stateful "first seen value" loops.
+
 ## Correctness Notes
 
 - Duplicate keys in `KeyBy`, `SliceToMap`, `MapKeys`, and `MapEntries` overwrite earlier entries; the last value wins.

.coderabbit.yaml

@@ -58,8 +58,13 @@ reviews:
       instructions: |
         Make sure the tests are comprehensive and cover the changes. Keep a strong focus on unit tests and in-code integration tests.
         When appropriate, recommend e2e tests for critical changes.
-
+        
         Do not request additional tests only because a helper or fixture could also cover an override/base-layer variant. Suggest missing tests only when the PR introduces behavior without meaningful coverage, or when the current tests would pass despite a concrete regression in the changed code.
+    - path: ".agents/skills/**/SKILL.md"
+      instructions: |
+        Review skills for clarity, durable guidance, and domain correctness.
+        Do not request additions for issues that the compiler or normal toolchain will detect, such as Go generic type constraints or missing imports, unless the skill text is actively misleading in a way compilation would not catch.
+        Prefer comments only when the skill would teach the wrong durable practice or omit non-obvious project-specific context.        
     - path: "**/*.md"
       instructions: |
         "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

AGENTS.md

@@ -17,8 +17,11 @@ The committed `.nvmrc` is the GitHub Actions source of truth for Node-based jobs
 ## AGENTS.md maintenance
 
 - Treat this file as long-lived project guidance for all agents and contributors.
+- Treat AGENTS.md as the repo-local source of truth. Do not bypass coding style, workflow, testing, or documentation guidance in this file. If a requested change appears to conflict with AGENTS.md, ask the human developer/reviewer to confirm the exception before proceeding, and make the exception explicit in the handoff.
 - Prefer durable wording over time-based wording (avoid labels like "recent", "latest", "today").
 - Keep entries actionable and specific (what to do, where, and why), not conversational history.
+- Capture universal truths and cross-cutting coding conventions here when they become repeated practice or reviewer expectation. Do not leave them only in chat or pull request comments.
+- Capture subsystem-specific guidance in the closest applicable nested `AGENTS.md` when the guidance should always apply to a subtree, such as `api/spec/AGENTS.md` for TypeSpec and SDK guidance. Use skills for reusable workflows or domain procedures that agents opt into for a task. Skills must stay usable by both Claude and Codex: write them as plain repo guidance, keep `.agents/skills` as the source of truth, and avoid assistant-specific assumptions unless a workflow truly requires them.
 - When adding new guidance, fold it into the most relevant section and remove/merge stale or duplicate notes.
 
 ## Testing
@@ -94,8 +97,6 @@ The TypeSpec JS client emitted from `api/spec/packages/aip` now lands in `api/sp
 
 The emitted `api/spec/packages/aip-client-javascript/src/openMeterClient.ts` exposes aggregated sub-client getters on `OpenMeterClient` (e.g. `meters`, `customers`, `features`, `productCatalog`, `subscriptions`, `billing`, `entitlements`). Access operations through those getters, e.g. `sdk.meters.list()`, `sdk.customers.create(...)`, `sdk.productCatalog.createPlan(...)`. Note that resources are grouped by tag, so some operations live under a grouped client rather than a same-named top-level getter (for example plan operations are `sdk.productCatalog.createPlan`, not `sdk.plans.create`).
 
-When adding query decorators (for example `@query`) to a TypeSpec file that does not already use HTTP decorators, import `@typespec/http` and add `using TypeSpec.Http;` in that file; otherwise compilation fails with `Unknown decorator @query`.
-
 **Workflow for changing Go types/DI:**
 
 1. Edit the source files (ent schema, wire.go, converter interfaces)
@@ -212,33 +213,29 @@ All builds use `GO_BUILD_FLAGS=-tags=dynamic`.
 
 See the `/service` skill for service/adapter patterns, constructors, input types, errors, transactions, hooks, logging, multi-tenancy, and DI wiring. See the `/api` skill for HTTP handler patterns and ValidationIssue. See the `/ent` skill for Ent ORM patterns and Postgres type gotchas. See the `/ledger` skill for ledger package architecture, wiring, and testing. See the `/subscription` skill for subscription domain model, sync algorithm, patch system, workflow layer, and addon sub-system. See the `/notification` skill for notification event pipeline, Kafka consumers, Svix webhook delivery, reconciliation loop, and payload versioning.
 
-Do not extract helper functions only to hide a couple of simple operations or short guard checks. If the helper would only wrap 2-4 lines and its name does not add meaningful domain or business intent, keep the code inline even when there is some duplication. Readers can inspect the function body to see what the code does; prefer function names that explain the domain reason for the call over names that merely restate the implementation steps. When you encounter a leftover pass-through wrapper that only calls another function without adding behavior, remove it and call the underlying function directly, even if it is outside the immediate change area.
-
-For `Validate() error` methods, prefer collecting all validation issues into `var errs []error` and returning `models.NewNillableGenericValidationError(errors.Join(errs...))` instead of returning on the first invalid field. Preserve field context with wrapped errors like `fmt.Errorf("field: %w", err)` and use plain `errors.New(...)` for simple local checks.
-
-In `openmeter/billing/charges/.../adapter`, keep Ent access transaction-aware even in shared helper functions. If a helper accepts a raw `*entdb.Client`, still wrap its body with `entutils.TransactingRepo(...)` / `TransactingRepoWithNoValue(...)` so it rebinds to the transaction already carried in `ctx` instead of depending on the caller to pass a tx-specific client.
-
-Do not introduce `context.Background()` or `context.TODO()` to sidestep missing context propagation in application code. Either propagate the caller's context through the full call path, or remove the unused `context.Context` parameter from the API if the operation is purely local and does not need cancellation, deadlines, or request-scoped values.
-
-Never use `panic` in non-test code paths. If a new failure mode is possible, change the function signature to return an error and propagate it explicitly.
-
-In production constructors and initialization, do not use `slog.Default()` as a fallback dependency. Require a `*slog.Logger` in config/provider inputs and inject it explicitly.
-
-Prefer standard library helpers and existing dependencies over local wrappers: use `slices.Clone(s)` for defensive slice copies instead of `append([]T(nil), s...)`; use `lo.ToPtr(...)` for pointer literals; and use `lo.Must(...)` only for `(value, err)` flows where panic-on-failure is intentional in test setup. Do not add local wrappers such as `ptr`, `loPtr`, `must`, or `loMust` when `github.com/samber/lo` already covers the need.
-
-Keep helper functions honest and narrow. If a production helper is only called once and is just a short guard or a few straightforward lines, inline it unless the name carries meaningful domain semantics. Do not add helpers for trivial single-use struct literals, do not hide aggregate mutation inside construction helpers, and return the domain value a helper actually builds rather than a broader wrapper needed by one caller.
+For TypeSpec-specific coding constraints, update `api/spec/AGENTS.md` instead of adding them here.
 
-Prefer explicit input structs for helpers that combine an aggregate with derived lifecycle values, such as period or timestamp overrides. This makes it clear which fields are construction inputs and which fields are persisted aggregate state.
+### Documentation Constraints
 
-For files and functions that convert between domain, API, and DB representations, use the `/go-types-conversion` skill. In prose, prefer `map` / `mapped` terminology for domain representation translation and avoid `project` / `projected` for that meaning; function names must still follow the skill's `FromAPI...`, `ToAPI...`, `FromDB...`, and `ToDB...` conventions.
+- When adding comments or docstrings, document intent and domain constraints that are only available from human author context, not facts a reader can infer by reading the codebase. Avoid comments that merely translate obvious conditions, such as saying that a branch runs when `servicePeriod > 0`. A good comment should be understandable without the author's chat context and should explain why the code deliberately includes or excludes a case.
+- Add a docstring to domain helpers when the name compresses important business semantics that are easy to misread at call sites. Explain the observable business contract and why excluded cases are excluded, not the implementation mechanics.
+- When refactoring or reverting code, preserve existing explanatory comments by default. Remove or rewrite a comment only when the code change makes it false, stale, or misleading.
 
-Add a docstring to domain helpers when the name compresses important business semantics that are easy to misread at call sites. Explain the observable business contract and why excluded cases are excluded, not the implementation mechanics.
+### Go Style Constraints
 
-When refactoring or reverting code, preserve existing explanatory comments by default. Remove or rewrite a comment only when the code change makes it false, stale, or misleading.
+- For Go string enum constants, name values as `<Type><Value>` so the constant carries its enum type at the use site, for example `InvoiceStatusDraft` instead of `Draft`.
+- Do not extract helper functions only to hide a couple of simple operations or short guard checks. If the helper would only wrap 2-4 lines and its name does not add meaningful domain or business intent, keep the code inline even when there is some duplication. Readers can inspect the function body to see what the code does; prefer function names that explain the domain reason for the call over names that merely restate the implementation steps. When you encounter a leftover pass-through wrapper that only calls another function without adding behavior, remove it and call the underlying function directly, even if it is outside the immediate change area.
+- For `Validate() error` methods, prefer collecting all validation issues into `var errs []error` and returning `models.NewNillableGenericValidationError(errors.Join(errs...))` instead of returning on the first invalid field. Preserve field context with wrapped errors like `fmt.Errorf("field: %w", err)` and use plain `errors.New(...)` for simple local checks.
+- Do not introduce `context.Background()` or `context.TODO()` to sidestep missing context propagation in application code. Either propagate the caller's context through the full call path, or remove the unused `context.Context` parameter from the API if the operation is purely local and does not need cancellation, deadlines, or request-scoped values.
+- Never use `panic` in non-test code paths. If a new failure mode is possible, change the function signature to return an error and propagate it explicitly.
+- In production constructors and initialization, do not use `slog.Default()` as a fallback dependency. Require a `*slog.Logger` in config/provider inputs and inject it explicitly.
+- Prefer standard library `slices` and `maps` helpers for common collection operations, and use `github.com/samber/lo` when it makes pointer literals or collection transformations clearer than local wrappers or hand-written loops. See the `/samber-lo` skill for common OpenMeter use cases and caveats. Do not add local wrappers such as `ptr`, `loPtr`, `must`, or `loMust` when standard helpers or `lo` already cover the need.
+- Keep helper functions honest and narrow. If a production helper is only called once and is just a short guard or a few straightforward lines, inline it unless the name carries meaningful domain semantics. Do not add helpers for trivial single-use struct literals, do not hide aggregate mutation inside construction helpers, and return the domain value a helper actually builds rather than a broader wrapper needed by one caller.
+- For files and functions that convert between domain, API, and DB representations, use the `/go-types-conversion` skill. In prose, prefer `map` / `mapped` terminology for domain representation translation and avoid `project` / `projected` for that meaning; function names must still follow the skill's `FromAPI...`, `ToAPI...`, `FromDB...`, and `ToDB...` conventions.
 
-For slice-wide invariants where the exact offending element is not important, prefer collecting distinct values with `lo.Map` + `lo.Uniq` and validating cardinality over stateful "first seen value" loops.
+### Generation And Dependency Constraints
 
-When `make generate` or `atlas migrate --env local diff ...` adds incidental `go.sum` entries, such as `tablewriter`, drop those `go.sum` changes unless the task explicitly requires a dependency change.
+- When `make generate` or `atlas migrate --env local diff ...` adds incidental `go.sum` entries, such as `tablewriter`, drop those `go.sum` changes unless the task explicitly requires a dependency change.
 
 ## Key Dependencies
 
@@ -313,4 +310,4 @@ Initialize it with `codegraph init -i` before doing code exploration. It indexes
 
 ## Skills
 
-Skills are created inside [.agents/skills](.agents/skills/) by default and then symlinked to [.claude/skills](.claude/skills). Make sure you always treat `.agents/skills` as the source of truth.
+Skills are created inside [.agents/skills](.agents/skills/) by default and then symlinked to [.claude/skills](.claude/skills). Make sure you always treat `.agents/skills` as the source of truth. Keep skill guidance compatible with both Claude and Codex; avoid instructions that assume only one agent runtime unless the skill is explicitly about that runtime.

api/spec/AGENTS.md

@@ -123,6 +123,13 @@ renaming.
   `{ body } & <Base>Query` (body nested so query fields don't leak into the JSON
   body). Path params are ULIDs, typed `string`. See "Request types" below.
 
+### TypeSpec style constraints
+
+- When adding query decorators (for example `@query`) to a TypeSpec file that
+  does not already use HTTP decorators, import `@typespec/http` and add
+  `using TypeSpec.Http;` in that file; otherwise compilation fails with
+  `Unknown decorator @query`.
+
 ## Commands
 
 | Task                          | Command                                                  |