docs(skill): add code review skill

.agents/skills/code-review/SKILL.md

@@ -0,0 +1,80 @@
+---
+name: code-review
+description: |
+  Review a PR or code change against dd-trace-js codebase standards. Use this skill whenever
+  someone asks to "review" code or a PR, types "/review", asks "can you check this before I
+  submit", wants to identify issues in a change, asks for a code review, says "PTAL", or
+  mentions a PR number and wants quality feedback. Also use proactively when someone is about
+  to open a PR and wants a pre-submission check. The input can be a PR number, a file path,
+  a diff pasted inline, or a description of the change.
+allowed-tools: Read, Grep, Glob, Bash, WebFetch
+---
+
+# Code Review
+
+You are performing a thorough code review of a dd-trace-js change. Work through
+[references/review-checklist.md](references/review-checklist.md) category by category, flag
+every issue you find, and always pair a finding with a concrete fix suggestion.
+
+## Getting the change
+
+`$ARGUMENTS` — a PR number, file path, diff, or description of the change to review.
+
+**If a PR number is given**, prefer the GitHub CLI — it handles auth and gives the full diff:
+
+```bash
+gh pr view <PR_NUMBER> --repo DataDog/dd-trace-js      # title, description, labels
+gh pr diff <PR_NUMBER> --repo DataDog/dd-trace-js      # full unified diff
+```
+
+If `gh` is unavailable, fall back to WebFetch:
+- `https://github.com/DataDog/dd-trace-js/pull/<PR_NUMBER>/files`
+
+**If a file path is given**, read the file and read the surrounding context (callers, tests,
+similar patterns in the codebase) before commenting.
+
+## How to review
+
+1. Read [references/review-checklist.md](references/review-checklist.md) before starting.
+2. Work through each applicable category. For each finding, ask: what is the actual risk, and
+   is there an existing pattern in the codebase the author should follow?
+3. For reviewer-specific preferences (what rochdev, BridgeAR, simon-id, etc. tend to flag),
+   see `.cursor/skills/reviewer-profiles.md`.
+
+## Output format
+
+Group findings by category. Omit any category with no findings.
+
+```
+## Architecture & Design
+
+### [BLOCKER | CONCERN | NIT] Short title
+
+**File:** path/to/file.js:line
+
+**Comment:** Specific feedback explaining why this matters.
+
+**Suggested fix:** Concrete code or approach.
+
+---
+```
+
+Categories:
+- Architecture & Design
+- Performance & Memory
+- Configuration System
+- Async & Correctness
+- Test Quality
+- Code Style & Readability
+- Observability & Logging
+- Documentation & PR Hygiene
+
+Severity:
+- **BLOCKER** — must be fixed before merge
+- **CONCERN** — worth discussing; approval may be conditional on author's response
+- **NIT** — non-blocking style preference
+
+End with a `## Summary` section giving an overall verdict:
+- `LGTM` — no significant issues
+- `LGTM with caveats` — concerns worth tracking, not blocking
+- `CHANGES_REQUESTED` — one or more BLOCKERs present

.agents/skills/code-review/references/review-checklist.md

@@ -0,0 +1,133 @@
+# dd-trace-js Code Review Checklist
+
+Aggregated from analysis of review patterns across the primary maintainers of this repository.
+
+---
+
+## Architecture & Design
+
+**BLOCKER when a shortcut bypasses the intended system design.**
+
+- Route through designed interfaces — don't call internal APIs directly when a public interface
+  exists (Diagnostics Channel, `runStores`/`bindStore`, propagator interfaces).
+- Don't mix cross-cutting concerns. Systems that are intentionally separate (e.g., baggage and
+  span context) must stay separate.
+- Avoid `WeakRef` for cleanup — it has caused memory leaks in this codebase. Use proper store
+  lifecycle (`runStores`/`bindStore`) instead.
+- Don't add helpers that only call another function with no transformation. Inline them.
+- Don't re-export symbols without modification — it adds confusion.
+- Functions called in only one place should be inlined for clarity.
+- Plugin hook setup that is repetitive should be made implicit via `addHooks`.
+
+---
+
+## Performance & Memory
+
+**BLOCKER or CONCERN in hot paths; CONCERN elsewhere.**
+
+- dd-trace runs in application hot paths — every allocation and lookup counts.
+- Flag unbounded collections: streams, maps, or listeners that can grow without limit.
+- Flag unnecessary allocations or object creation in hot paths.
+- A Map keyed by user-operation objects (requests, contexts) must have a guaranteed cleanup
+  path, or use `WeakMap` instead. Timer-based cleanup is risky — it can cause sawtooth memory
+  growth.
+- Prefer lazy initialisation: don't create expensive objects or run expensive computations
+  until they are needed.
+- If a PR claims a performance improvement, a benchmark is required to demonstrate it.
+
+---
+
+## Configuration System
+
+**BLOCKER when registration is missing.**
+
+- New environment variables must be defined in `packages/dd-trace/src/config/index.js` inside
+  `#applyEnvironment()`.
+- Every new env var must have an entry in `supported-configurations.json` with a
+  `configurationNames` field.
+- Every new env var must be registered for telemetry.
+- `supported-configurations.json` must not be corrupted (watch for rebase artifacts).
+- Don't add a new boolean toggle that mirrors an existing option. If a feature is enabled by a
+  parent config, it should not need its own sub-toggle.
+
+---
+
+## Async & Correctness
+
+**BLOCKER for unhandled rejections; CONCERN for logic issues.**
+
+- Sequential `await` calls where rejection of the first would leave the second as an unhandled
+  rejection must be rewritten as `Promise.all([...])`.
+- Don't rely on NaN being falsy — be explicit. TypeScript rejects it and it obscures intent.
+- Don't introduce a fallback that silently changes existing behaviour (e.g., `undefined` → `null`).
+- Check whether a Node.js built-in API already solves the problem before writing a custom
+  workaround.
+
+---
+
+## Test Quality
+
+**BLOCKER for missing bug-fix tests or broken test isolation; CONCERN for flakiness risks.**
+
+- Every bug fix must have a test that would have caught the bug.
+- Plugin tests are integration tests — don't mock tracer internals. Reconfigure via the public
+  API instead.
+- Cleanup/reset logic must be in `afterEach`, not after an assertion. If the assertion throws,
+  the reset never runs.
+- Don't spy on implementation details — test behaviour only.
+- Each `it` block must be independently runnable with `it.only`. If it depends on side effects
+  from a previous `it`, move the shared setup into `beforeEach` and teardown into `afterEach`.
+- Don't add an existence check (`assert(x.foo)`) immediately before asserting the value
+  (`assert.strictEqual(x.foo, 'bar')`). Assert the value directly.
+- Combine multiple `assert.strictEqual` calls on the same object into a single
+  `assert.deepStrictEqual` or `assertObjectContains` call.
+- Check for copy-paste mistakes: duplicate assertions on the same property.
+
+---
+
+## Code Style & Readability
+
+**NIT unless it affects correctness.**
+
+- Use `#private` class fields instead of `_underscore` prefixes for class-internal state.
+- Pre-construct objects with all possible fields (some `undefined`) to keep V8 object shapes
+  stable. Don't conditionally add properties after construction.
+- Use static class properties, not static getters (static getters are a holdover from before
+  static properties were supported).
+- Non-class symbols must not have a leading capital letter.
+- `require()` calls must be at the top of the file. Only the instantiation (not the require)
+  should be inside try/catch if the require itself cannot fail.
+- Keep try/catch blocks narrow — wrap only the code that can actually throw.
+- Avoid bitwise or type-coercion tricks that require knowing operator precedence or implicit
+  coercion rules. Write explicit, readable code.
+- Remove commented-out code before merging.
+
+---
+
+## Observability & Logging
+
+**CONCERN when silent failures are possible; NIT for improvements.**
+
+- Don't remove existing log or debug output without a replacement. Diagnostic information helps
+  production debugging.
+- Any code path where a feature could silently fail should have a log statement.
+- `catch` blocks that swallow errors should log them.
+- Log at the point of failure, not only at the call site.
+
+---
+
+## Documentation & PR Hygiene
+
+**CONCERN or NIT.**
+
+- Unrelated changes belong in a separate PR.
+- A new CI workflow must be justified — prefer adding to an existing workflow.
+- Retry logic must be scoped: only commands that can be flaky (e.g., registry installs) should
+  be retried.
+- New public API surface area requires a semver-minor label and may require a corresponding PR
+  in dd-trace-api-js.
+- AGENTS.md: prefer inline code over fenced code blocks (token efficiency). Avoid vague
+  language; use concrete examples.
+- AGENTS.md must not contain inaccurate or contradictory claims, and should not include
+  contributing-guide content that belongs in CONTRIBUTING.md.
+- Moving a file to a new directory must not change a user-facing `require()` path.

.agents/skills/code-review/review-checklist.md

@@ -0,0 +1,108 @@
+# dd-trace-js Code Review Checklist
+
+Aggregated from analysis of review patterns across the primary maintainers of this repository.
+
+---
+
+## Architecture & Design
+
+**BLOCKER when a shortcut bypasses the intended system design.**
+
+- Route through designed interfaces — don't call internal APIs directly when a public interface exists (Diagnostics Channel, `runStores`/`bindStore`, propagator interfaces).
+- Don't mix cross-cutting concerns. Systems that are intentionally separate (e.g., baggage and span context) must stay separate.
+- Avoid `WeakRef` for cleanup — it has caused memory leaks in this codebase. Use proper store lifecycle (`runStores`/`bindStore`) instead.
+- Don't add helpers that only call another function with no transformation. Inline them.
+- Don't re-export symbols without modification — it adds confusion.
+- Functions called in only one place should be inlined for clarity.
+- Plugin hook setup that is repetitive should be made implicit via `addHooks`.
+
+---
+
+## Performance & Memory
+
+**BLOCKER or CONCERN in hot paths; CONCERN elsewhere.**
+
+- dd-trace runs in application hot paths — every allocation and lookup counts.
+- Flag unbounded collections: streams, maps, or listeners that can grow without limit.
+- Flag unnecessary allocations or object creation in hot paths.
+- A Map keyed by user-operation objects (requests, contexts) must have a guaranteed cleanup path, or use `WeakMap` instead. Timer-based cleanup is risky — it can cause sawtooth memory growth.
+- Prefer lazy initialisation: don't create expensive objects or run expensive computations until they are needed.
+- If a PR claims a performance improvement, a benchmark is required to demonstrate it.
+
+---
+
+## Configuration System
+
+**BLOCKER when registration is missing.**
+
+- New environment variables must be defined in `packages/dd-trace/src/config/index.js` inside `#applyEnvironment()`.
+- Every new env var must have an entry in `supported-configurations.json` with a `configurationNames` field.
+- Every new env var must be registered for telemetry.
+- `supported-configurations.json` must not be corrupted (watch for rebase artifacts).
+- Don't add a new boolean toggle that mirrors an existing option. If a feature is enabled by a parent config, it should not need its own sub-toggle.
+
+---
+
+## Async & Correctness
+
+**BLOCKER for unhandled rejections; CONCERN for logic issues.**
+
+- Sequential `await` calls where rejection of the first would leave the second as an unhandled rejection must be rewritten as `Promise.all([...])`.
+- Don't rely on NaN being falsy — be explicit. TypeScript rejects it and it obscures intent.
+- Don't introduce a fallback that silently changes existing behaviour (e.g., `undefined` → `null`).
+- Check whether a Node.js built-in API already solves the problem before writing a custom workaround.
+
+---
+
+## Test Quality
+
+**BLOCKER for missing bug-fix tests or broken test isolation; CONCERN for flakiness risks.**
+
+- Every bug fix must have a test that would have caught the bug.
+- Plugin tests are integration tests — don't mock tracer internals. Reconfigure via the public API instead.
+- Cleanup/reset logic must be in `afterEach`, not after an assertion. If the assertion throws, the reset never runs.
+- Don't spy on implementation details — test behaviour only.
+- Each `it` block must be independently runnable with `it.only`. If it depends on side effects from a previous `it`, move the shared setup into `beforeEach` and teardown into `afterEach`.
+- Don't add an existence check (`assert(x.foo)`) immediately before asserting the value (`assert.strictEqual(x.foo, 'bar')`). Assert the value directly.
+- Combine multiple `assert.strictEqual` calls on the same object into a single `assert.deepStrictEqual` or `assertObjectContains` call.
+- Check for copy-paste mistakes: duplicate assertions on the same property.
+
+---
+
+## Code Style & Readability
+
+**NIT unless it affects correctness.**
+
+- Use `#private` class fields instead of `_underscore` prefixes for class-internal state.
+- Pre-construct objects with all possible fields (some `undefined`) to keep V8 object shapes stable. Don't conditionally add properties after construction.
+- Use static class properties, not static getters (static getters are a holdover from before static properties were supported).
+- Non-class symbols must not have a leading capital letter.
+- `require()` calls must be at the top of the file. Only the instantiation (not the require) should be inside try/catch if the require itself cannot fail.
+- Keep try/catch blocks narrow — wrap only the code that can actually throw.
+- Avoid bitwise or type-coercion tricks that require knowing operator precedence or implicit coercion rules. Write explicit, readable code.
+- Remove commented-out code before merging.
+
+---
+
+## Observability & Logging
+
+**CONCERN when silent failures are possible; NIT for improvements.**
+
+- Don't remove existing log or debug output without a replacement. Diagnostic information helps production debugging.
+- Any code path where a feature could silently fail should have a log statement.
+- `catch` blocks that swallow errors should log them.
+- Log at the point of failure, not only at the call site.
+
+---
+
+## Documentation & PR Hygiene
+
+**CONCERN or NIT.**
+
+- Unrelated changes belong in a separate PR.
+- A new CI workflow must be justified — prefer adding to an existing workflow.
+- Retry logic must be scoped: only commands that can be flaky (e.g., registry installs) should be retried.
+- New public API surface area requires a semver-minor label and may require a corresponding PR in dd-trace-api-js.
+- AGENTS.md: prefer inline code over fenced code blocks (token efficiency). Avoid vague language; use concrete examples.
+- AGENTS.md must not contain inaccurate or contradictory claims, and should not include contributing-guide content that belongs in CONTRIBUTING.md.
+- Moving a file to a new directory must not change a user-facing `require()` path.