From 6c3b5a586b2fb09ef95c1741f59fdb91005ead43 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 14:38:35 -0800 Subject: [PATCH 01/11] Add CLAUDE.md and consolidate AI agent conventions Establish CLAUDE.md as single source of truth for all AI coding agents. Add slash commands, subagents, issue templates, and PR template to standardize workflows. --- .agents/mcp.json | 8 - .agents/rules/ai-assistant-guidelines.mdc | 68 ---- .agents/rules/git-commit-conventions.mdc | 41 --- .agents/rules/rust-coding-style.mdc | 223 ------------ .agents/rules/rust-documentation.mdc | 388 --------------------- .agents/rules/rust-error-handling.mdc | 74 ---- .agents/rules/rust-testing-strategy.mdc | 95 ----- .agents/rules/rust-tracing-practices.mdc | 32 -- .agents/rules/update-rules.mdc | 6 - .claude/agents/build-validator.md | 43 +++ .claude/agents/code-architect.md | 35 ++ .claude/agents/code-simplifier.md | 32 ++ .claude/agents/issue-creator.md | 90 +++++ .claude/agents/pr-creator.md | 171 +++++++++ .claude/agents/repo-explorer.md | 34 ++ .claude/agents/verify-app.md | 55 +++ .claude/commands/check-ci.md | 8 + .claude/commands/review-changes.md | 11 + .claude/commands/test-all.md | 13 + .claude/commands/test-crate.md | 17 + .claude/commands/verify.md | 10 + .claude/settings.json | 31 ++ .github/ISSUE_TEMPLATE/bug_report.yml | 75 ++++ .github/ISSUE_TEMPLATE/config.yml | 2 + .github/ISSUE_TEMPLATE/story.yml | 58 ++++ .github/ISSUE_TEMPLATE/task.yml | 51 +++ .github/dependabot.yml | 18 +- .github/pull_request_template.md | 40 +++ .github/workflows/codeql.yml | 94 ++--- .gitignore | 7 +- AGENTS.md | 128 +------ CLAUDE.md | 401 +++++++++++++++++++++- README.md | 3 +- docs/guide/onboarding.md | 14 +- docs/guide/testing.md | 4 +- 35 files changed, 1265 insertions(+), 1115 deletions(-) delete mode 100644 .agents/mcp.json delete mode 100644 .agents/rules/ai-assistant-guidelines.mdc delete mode 100644 .agents/rules/git-commit-conventions.mdc delete mode 100644 .agents/rules/rust-coding-style.mdc delete mode 100644 .agents/rules/rust-documentation.mdc delete mode 100644 .agents/rules/rust-error-handling.mdc delete mode 100644 .agents/rules/rust-testing-strategy.mdc delete mode 100644 .agents/rules/rust-tracing-practices.mdc delete mode 100644 .agents/rules/update-rules.mdc create mode 100644 .claude/agents/build-validator.md create mode 100644 .claude/agents/code-architect.md create mode 100644 .claude/agents/code-simplifier.md create mode 100644 .claude/agents/issue-creator.md create mode 100644 .claude/agents/pr-creator.md create mode 100644 .claude/agents/repo-explorer.md create mode 100644 .claude/agents/verify-app.md create mode 100644 .claude/commands/check-ci.md create mode 100644 .claude/commands/review-changes.md create mode 100644 .claude/commands/test-all.md create mode 100644 .claude/commands/test-crate.md create mode 100644 .claude/commands/verify.md create mode 100644 .claude/settings.json create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/story.yml create mode 100644 .github/ISSUE_TEMPLATE/task.yml create mode 100644 .github/pull_request_template.md diff --git a/.agents/mcp.json b/.agents/mcp.json deleted file mode 100644 index bfd5fa23..00000000 --- a/.agents/mcp.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "mcpServers": { - "filesystem": { - "command": "npx", - "args": ["-y", "@modelcontextprotocol/server-filesystem", "."] - } - } -} diff --git a/.agents/rules/ai-assistant-guidelines.mdc b/.agents/rules/ai-assistant-guidelines.mdc deleted file mode 100644 index a9fc41d2..00000000 --- a/.agents/rules/ai-assistant-guidelines.mdc +++ /dev/null @@ -1,68 +0,0 @@ ---- -description: Guidelines for AI Assistants -globs: "**/*" -alwaysApply: true ---- - -# Guidelines for AI Assistants - -This file contains standards for how AI assistants should interact with codebases and follow project guidelines. - -## Core Principles - -1. **Autonomy**: Take responsibility for following all project rules without requiring reminders. -2. **Consistency**: Maintain consistency with the existing codebase and established patterns. -3. **Proactivity**: Anticipate rule requirements before starting work on any task. -4. **Accuracy**: Ensure adherence to all relevant guidelines in every implementation. - -## Things to avoid - -1. Making changes unrelated to the task at hand - if you see other things in a file that could be improved, suggest them to the user! -2. Getting stuck for a long time without asking for help - if you're struggling, just ask. - -## Self-Checking Process - -When working on any project task: - -1. **Discovery**: - - Identify all relevant rule files for the current context - - Scan directories like `.cursor/rules/` for applicable guidelines - - Look for language-specific, style, and organizational conventions - -2. **Preparation**: - - Review all applicable guidelines before beginning work - - Identify patterns in existing code that demonstrate rule application - - Prioritize project-specific conventions over generic best practices - -3. **Validation**: - - Before submitting any changes, verify compliance with ALL project guidelines - - Ensure consistency with surrounding code - - Detect and correct any rule violations proactively - -## Effective Rule Application - -For any development task: - -1. **Context Assessment**: - - Determine which rules apply to the current task - - Consider the specific file type, language, and component - -2. **Implementation Guidance**: - - Apply relevant rules throughout the implementation process - - Use existing code patterns as guidance for ambiguous cases - - Maintain the established style and conventions - -3. **Quality Assurance**: - - Verify rule compliance before finalizing work - - Ensure consistency across similar components - - Validate that all project-specific requirements are met - -## Common Pitfalls to Avoid - -- Requiring reminders about existing project guidelines -- Implementing solutions that contradict established patterns -- Applying inconsistent approaches across similar tasks -- Overlooking project-specific conventions in favor of generic practices -- Introducing deviations from standards without explicit justification - -AI assistants should consistently enforce project guidelines without being prompted. Your role includes ensuring that all contributions adhere to the established rules of the project. diff --git a/.agents/rules/git-commit-conventions.mdc b/.agents/rules/git-commit-conventions.mdc deleted file mode 100644 index 0be93cb5..00000000 --- a/.agents/rules/git-commit-conventions.mdc +++ /dev/null @@ -1,41 +0,0 @@ ---- -description: Git convention -globs: -alwaysApply: true ---- -# Git Commit Conventions - -This project follows specific conventions for git commit messages. - -## Commit Message Format - -Commit messages should: - -1. Be descriptive and concise -2. Use sentence case (capitalize first word) -3. Describe the change in an imperative, present-tense style -4. Not use semantic prefixes (like "fix:", "feat:", "chore:") -5. Not use bracketed tags (like "[Docs]" or "[Fix]") - -## Examples - -### Good commit messages: - -```text -Add feature flags to Real type tests that require `serde` -Update performance documentation guidelines -Fix PostgreSQL integration type reference issues -Improve error handling in token validation -``` - -### Avoid semantic prefixes and tags: - -```text -// Don't use these formats -fix: add feature flags to tests -feat: implement new logging system -chore: update dependencies -docs: update README -[Docs] Update README -[Fix] Handle empty payload -``` diff --git a/.agents/rules/rust-coding-style.mdc b/.agents/rules/rust-coding-style.mdc deleted file mode 100644 index e7621984..00000000 --- a/.agents/rules/rust-coding-style.mdc +++ /dev/null @@ -1,223 +0,0 @@ ---- -description: Rust Coding Style -globs: "**/*.rs" -alwaysApply: false ---- -# Rust Coding Style - -## Project-Specific Patterns - -- Use the 2024 edition of Rust -- Prefer `derive_more` over manual trait implementations -- Feature flags in this codebase use the `#[cfg(feature = "...")]` pattern -- Invoke `cargo clippy` with `--all-features`, `--all-targets`, and `--no-deps` from the root -- Use `cargo doc --no-deps --all-features` for checking documentation -- Use `rustfmt` to format the code -- Use `#[expect(lint, reason = "...")]` over `#[allow(lint)]` - -## Type System - -- Create strong types with newtype patterns for domain entities -- Implement `hash_graph_types` traits for custom domain types -- Consider visibility carefully (avoid unnecessary `pub`) - -```rust -#[derive(Debug, Copy, Clone, Eq, Hash, PartialEq, derive_more::Display)] -pub struct UserId(Uuid); -``` - -## Async Patterns - -- Use `impl Future + Send` in trait definitions: - -```rust -fn get_data( - &self, - id: String, -) -> impl Future>> + Send { - async move { - // Implementation - } -} -``` - -## Function Arguments - -- Functions should **never** take more than 7 arguments. If a function requires more than 7 arguments, encapsulate related parameters in a struct. -- Functions that use data immutably should take a reference to the data, while functions that modify data should take a mutable reference. Never take ownership of data unless the function explicitly consumes it. -- Make functions `const` whenever possible. -- Prefer the following argument types when applicable, but only if this does not reduce performance: - - `impl AsRef` instead of `&str` or `&String` - - `impl AsRef` instead of `&Path` or `&PathBuf` - - `impl IntoIterator` when only iterating over the data - - `&[T]` instead of `&Vec` - - `&mut [T]` instead of `&mut Vec` when the function doesn't need to resize the vector - - `impl Into>` instead of `Cow` - - `impl Into>` instead of `Arc` - - `impl Into>` instead of `Rc` - - `impl Into>` instead of `Box` -- Never use `impl Into>` as from reading the caller site, it's not visible that `None` could potentially be passed - -## `From` and `Into` - -- Generally prefer `From` implementations over `Into` implementations. The Rust compiler will automatically derive `Into` from `From`, but not vice versa. -- When converting between types, prefer using the `from` method over `into` for clarity. The `from` method makes the target type explicit in the code, while `into` requires type inference. -- For wrapper types like `Cow`, `Arc`, `Rc`, `Report`, and `Box`, prefer using explicit constructors (e.g., `Cow::from`, `Arc::new`) instead of `.into()`. This improves readability by clearly indicating the target type. - -## Smart Pointers - -- When cloning smart pointers such as `Arc` and `Rc`, **always** use `Arc::clone(&pointer)` and `Rc::clone(&pointer)` instead of `pointer.clone()`. This explicitly indicates you're cloning the reference, not the underlying data. - -## Instrumentation - -- Annotate functions that perform significant work with `#[tracing::instrument]` -- Use `tracing` macros (e.g., `trace!`, `debug!`, `info!`, `warn!`, `error!`) instead of `println!` or `eprintln!` for logging - -## Allocations - -- Minimize allocations when possible. For example, reuse a `Vec` in a loop instead of creating a new one in each iteration. -- Prefer borrowed data over owned data where appropriate. -- Balance performance and readability—if an allocation makes code significantly more readable or maintainable, the trade-off may be worthwhile. - -## Types - -- Use newtypes when a value should carry specific semantics beyond its underlying type. This improves type safety and code clarity. - -For example: - -```rust -struct UserId(u64); // instead of `type UserId = u64;` or `u64` -``` - -## Naming Conventions - -When suggesting names for variables, functions, or types: - -- Do not prefix test-function names with `test_`, this would otherwise result in `test::test_` names. -- Provide a concise list of naming options with brief explanations of why each fits the context -- Choose names of appropriate length—avoid names that are too long or too short -- Avoid abbreviations unless they are widely recognized in the domain (e.g., `Http` or `Json` is acceptable, but `Ctx` instead of `Context` is not) -- Do not suffix names with their types (e.g., use `users` instead of `usersList`) -- Do not repeat the type name in variable names (e.g., use `user` instead of `userUser`) - -## Crate Preferences - -- Use `similar_asserts` for test assertions -- Use `insta` for snapshot tests -- Use `test_log` for better test output (`#[test_log::test]`) -- Use `tracing` macros, not `log` macros -- Prefer `tracing::instrument` for function instrumentation - -## Import Style - -- Don't use local imports within functions, or blocks -- Avoid wildcard imports like `use super::*;`, or `use crate::module::*;` -- Never use a prelude `use crate::prelude::*` -- Prefer explicit imports to make dependencies clear and improve code readability -- We prefer `core` over `alloc` over `std` for imports to minimize dependencies - - Use `core` for functionality that doesn't require allocation - - Use `alloc` when you need allocation but not OS-specific features - - Only use `std` when necessary for OS interactions or when using `core`/`alloc` would be unnecessarily complex -- Prefer qualified imports (`use foo::Bar; let x = Bar::new()`) over fully qualified paths (`let x = foo::Bar::new()`) for frequently used types -- Use `pub use` re-exports in module roots to create a clean public API -- Avoid importing items with the same name from different modules; use qualified imports -- Import traits using `use module::Trait as _;` when you only need the trait's methods and not the trait name itself - - This pattern brings trait methods into scope without name conflicts - - Use this especially for extension traits or when implementing foreign traits on local types - -```rust -// Good - Importing a trait just for its methods: -use std::io::Read as _; - -// Example with trait methods: -fn read_file(file: &mut File) -> Result { - // Read methods available without importing the Read trait name - let mut content = String::new(); - file.read_to_string(&mut content)?; - Ok(content) -} - -// Bad - Directly importing trait when only methods are needed: -use std::io::Read; - -// Good - Importing trait for implementing it: -use std::io::Write; -impl Write for MyWriter { /* implementation */ } - -// Bad - Wildcard import: -mod tests { - use super::*; // Wildcard import - - #[test] - fn test_something() { - // Test implementation - } -} - -// Good - Explicit imports: -mod tests { - use crate::MyStruct; - use crate::my_function; - - #[test] - fn test_something() { - // Test implementation - } -} - -// Bad - Local import: -fn process_data() { - use std::collections::HashMap; // Local import - let map = HashMap::new(); - // Implementation -} - -// Good - Module-level import: -use std::collections::HashMap; - -fn process_data() { - let map = HashMap::new(); - // Implementation -} - -// Bad - Using std when core would suffice: -use std::fmt::Display; - -// Good - Using core for non-allocating functionality: -use core::fmt::Display; - -// Bad - Using std when alloc would suffice: -use std::collections::BTreeSet; - -// Good - Using alloc for allocation without full std dependency: -use alloc::vec::Vec; - -// Appropriate - Using std when needed: -use std::fs::File; // OS-specific functionality requires std -``` - -## Libraries and Components - -- Abstract integrations with third-party systems behind traits to maintain clean separation of concerns - -## Comments and Assertions - -- Do not add comments after a line of code; place comments on separate lines above the code they describe -- When using assertions, include descriptive messages using the optional description parameter rather than adding a comment -- All `expect()` messages should follow the format "should ..." to clearly indicate the expected behavior - -For example: - -```rust -// Bad: -assert_eq!(result, expected); // This should match the expected value - -// Good: -assert_eq!(result, expected, "Values should match expected output"); - -// Bad: -some_value.expect("The value is not None"); // This should never happen - -// Good: -some_value.expect("should contain a valid value"); -``` diff --git a/.agents/rules/rust-documentation.mdc b/.agents/rules/rust-documentation.mdc deleted file mode 100644 index d3b72cc3..00000000 --- a/.agents/rules/rust-documentation.mdc +++ /dev/null @@ -1,388 +0,0 @@ ---- -description: Rust Documentation Practices -globs: "**/*.rs" -alwaysApply: false ---- - -# Rust Documentation Practices - -## Documentation Structure - -- Each public item must have a doc comment -- Begin doc comments with a single-line summary of the item's purpose -- Include a blank line after the summary line for longer documentation -- Structure complex documentation with Markdown headers -- Always use intra-doc links (`[`Item`]`) whenever referencing other items in the codebase, including standard library types like [`Vec`], [`HashMap`], etc. -- Prefer inline descriptions over section headers for parameters and return values - -```rust -/// Retrieves an entity by its UUID. -/// -/// Loads the entity from the store and verifies access permissions for the -/// requesting account. Returns an [`Entity`] object if found. -``` - -## Intra-Doc Links - -- Use intra-doc links (Rust's reference syntax) liberally to improve user experience -- Link to every type, trait, function, or other item you mention in documentation, including standard library items -- Use the full path for items from other modules: `[`crate::module::Item`]` -- When referring to items in the current module, just use `[`Item`]` -- For standard library items, use their simple name: `[`Vec`]`, `[`HashMap`]`, `[`swap_remove`]` -- Link method names on types where appropriate: `[`Vec::swap_remove`]` -- Add reference link definitions at the bottom of the doc comment if needed - -```rust -/// Updates the [`User`] in the system based on provided changes. -/// -/// This process uses the [`UserUpdateStrategy`] to determine which fields to update -/// and applies validations defined in [`validation::user`]. -/// -/// [`validation::user`]: crate::validation::user -``` - -## Special Documentation Sections - -### Error Documentation - -- Always document errors with an "# Errors" section for fallible functions -- List each error variant with a bullet point -- Link error variants with Rust's reference syntax `[`VariantName`]` for actual error types -- For errors coming from external crates or generated via `Error::custom()`, use a simple description without links - -```rust -/// Creates a new web in the system. -/// -/// This function registers a new web with the given parameters and ensures its -/// uniqueness in the system. -/// -/// # Errors -/// -/// - [`WebAlreadyExists`] if a web with the same ID already exists -/// - [`AuthorizationError`] if the account lacks permission -/// - [`DatabaseError`] if the operation fails at the database level -/// -/// [`WebAlreadyExists`]: WebError::WebAlreadyExists -/// [`AuthorizationError`]: WebError::Authorization -/// [`DatabaseError`]: WebError::Database -pub fn create_web(&mut self) -> Result> { -``` - -For custom errors created at runtime (like validation errors): - -```rust -/// Validates that all input values are unique. -/// -/// # Errors -/// -/// - Returns a validation error if the input contains duplicate values -pub fn validate_unique(values: &[String]) -> Result<(), Report> { -``` - -### Panic Documentation - -- Document potential panics with a "# Panics" section -- Include the exact conditions that could cause a panic - -```rust -/// Converts the entity ID to a UUID. -/// -/// # Panics -/// -/// Panics if the entity ID contains an invalid UUID format. -``` - -### Examples - -- Add "# Examples" sections for public API functions -- Keep examples minimal but complete -- Include error handling in examples -- Use comment-based error handling for brevity when appropriate -- Include assertions to demonstrate expected behavior -- Doc comments should follow high-quality standards like those in crates such as `time`, `jiff`, or `serde` -- Never mention variable types in doc comments when that information is already available in function signatures -- Ensure all code examples compile and function correctly -- Use `#` comments to hide necessary setup code from documentation display - -```rust -/// # Examples -/// -/// ```rust -/// let entities = get_entities_by_type(type_id)?; -/// assert_eq!(entities.len(), 2); -/// for entity in entities { -/// println!("Found entity: {}", entity.title); -/// } -/// # Ok::<(), Box>(()) -/// ``` -``` - -## API Documentation - -### Function Documentation - -- Begin with a clear, single-line summary of what the function does -- Include a detailed description of the function's behavior -- For simple functions (0-2 parameters), describe parameters inline in the main description -- For complex functions (3+ parameters), use an explicit "# Arguments" section with bullet points -- Always describe return values in the main description text, not in a separate "# Returns" section -- Link to relevant types using intra-doc links - -For simple functions: - -```rust -/// Processes the input elements and returns a filtered collection. -/// -/// Takes a collection of `input` elements, applies the `filter_fn` predicate -/// to each, and returns a [`Vec`] containing only the elements that passed -/// the filter condition. -``` - -For complex functions: - -```rust -/// Merges multiple data sources and applies transformation rules. -/// -/// This function combines data from various sources, applies the specified -/// transformation rules, and returns a unified data structure. The process -/// preserves metadata from all sources when possible. -/// -/// # Arguments -/// -/// * `sources` - Collection of data sources to merge -/// * `rules` - Transformation rules to apply during merging -/// * `options` - Configuration options controlling the merge behavior -/// * `callback` - Optional function called for each merged item -/// -/// # Errors -/// -/// - [`SourceError`] if any data source cannot be accessed -/// - [`RuleError`] if transformation rules are invalid -``` - -### Type Documentation - -- Document the purpose of the type and its invariants -- For structs, document each field with field-level doc comments -- For enums, document each variant -- Include examples for non-trivial instantiation -- Describe arguments and return values in the main description rather than in separate sections - -```rust -/// A unique identifier for an entity in the system. -/// -/// This is a combination of the entity UUID and the web it belongs to. -/// Used throughout the API to precisely reference entities. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct EntityId { - /// The UUID of the entity - pub entity_uuid: EntityUuid, - - /// The web this entity belongs to - pub web_id: WebId, -} -``` - -### Trait Documentation - -- Document the purpose and contract of the trait -- Document each method, even if it seems self-explanatory -- Specify implementation requirements in the trait documentation - -```rust -/// Represents a store for entity data. -/// -/// Implementers of this trait provide storage and retrieval capabilities -/// for [`Entity`] instances in the system. All implementations must ensure -/// thread safety and handle concurrent access appropriately. -pub trait EntityStore: Send + Sync { - /// Retrieves an entity by its ID. - /// - /// Looks up the entity with the given `id` in the store and returns - /// it if found. - /// - /// # Errors - /// - /// - [`NotFound`] if the entity does not exist - /// - [`DatabaseError`] if the operation fails - /// - /// [`NotFound`]: StoreError::NotFound - /// [`DatabaseError`]: StoreError::Database - fn get_entity(&self, id: EntityId) -> Result>; - - // Additional methods... -} - -```### Trait Documentation - -- Document the purpose and contract of the trait -- Document each method, even if it seems self-explanatory -- Specify implementation requirements in the trait documentation - -```rust -/// Represents a store for entity data. -/// -/// Implementers of this trait provide storage and retrieval capabilities -/// for [`Entity`] instances in the system. All implementations must ensure -/// thread safety and handle concurrent access appropriately. -pub trait EntityStore: Send + Sync { - /// Retrieves an entity by its ID. - /// - /// Looks up the entity with the given `id` in the store and returns - /// it if found. - /// - /// # Errors - /// - /// - [`NotFound`] if the entity does not exist - /// - [`DatabaseError`] if the operation fails - /// - /// [`NotFound`]: StoreError::NotFound - /// [`DatabaseError`]: StoreError::Database - fn get_entity(&self, id: EntityId) -> Result>; - - // Additional methods... -} -``` - -### Trait Implementation Documentation - -- Don't create documentation for trait implementations, conversions, or trait-derived functions if upstream documentation already exists -- Skip documentation for standard trait implementations like `From`, `Display`, `Debug`, `Serialize`, etc. unless your implementation has unique behavior -- Only document implementations when: - - The implementation has specific behavior beyond the trait's definition - - There are performance considerations users should be aware of - - The implementation has different failure modes or edge cases - - The implementation provides additional functionality beyond the trait contract - -```rust -// Good: No documentation needed for standard traits -impl Debug for MyType { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // implementation... - } -} - -// Good: No documentation needed for standard conversions -impl From for String { - fn from(value: MyType) -> Self { - // implementation... - } -} - -// Good: Documentation needed due to special behavior -/// Custom serialization that handles legacy formats. -/// -/// This implementation supports both the current schema and the deprecated v1 format -/// to maintain backward compatibility with older data. -impl Serialize for ComplexType { - fn serialize(&self, serializer: S) -> Result { - // implementation with special behavior... - } -} -``` - -### Module Documentation - -- Add a doc comment at the top of each module file -- Describe the module's purpose, contents, and how it fits into the overall architecture -- Include links to key types and functions -- Add usage examples for the module as a whole when appropriate - -```rust -//! Entity management functionality. -//! -//! This module provides types and functions for creating, updating, and -//! querying entities in the system. The main types are: -//! -//! - [`Entity`]: The core entity type -//! - [`EntityId`]: Unique identifier for entities -//! - [`EntityStore`]: Trait for storing and retrieving entities -//! -//! # Examples -//! -//! ``` -//! use hash_graph::entity::{Entity, EntityStore}; -//! -//! // Example code showing module usage -//! ``` -``` - -## Code Comments - -### Inline Comments - -- Place comments on separate lines above the code they describe, not on the same line -- Use descriptive comments to explain "why", not just "what" the code does -- For complex algorithms, add comments explaining the approach -- Comments should not simply restate what is obvious from the code itself - -```rust -// Bad: -let result = calculate_sum(values); // Calculate the sum of values - -// Good: -// Calculate the weighted sum to prioritize recent values -let result = calculate_sum(values); -``` - -### Test Documentation - -For guidance on structuring test documentation and writing assertions, refer to the [Rust Testing Strategy](mdc:.cursor/rules/rust-testing-strategy.mdc) file. - -Key points: - -- Document test scenarios with clear comments -- Use descriptive function names that explain the test scenario -- Follow the project's assertion formatting standards - -## Project-Specific Patterns - -### Performance Notes - -- Add "# Performance" sections for performance-critical functions -- Document complexity and potential bottlenecks - -```rust -/// Retrieves all entities matching the filter. -/// -/// # Performance -/// -/// This operation performs pagination internally and has O(n) complexity -/// where n is the number of entities matching the filter. For large result -/// sets, consider using the streaming version instead. -``` - -- Apply performance documentation when: - - **Processing variable-sized inputs** - Document how performance scales with input size - - **In hot paths** - Document components used in performance-critical code paths - - **Complex algorithms** - Document non-obvious time or space complexity - - **Public APIs** - Document performance characteristics of public interfaces - - **Resource-intensive operations** - Document memory usage, I/O, or CPU consumption - - **Operations with tradeoffs** - Document performance vs. correctness/convenience tradeoffs - -- Skip performance documentation when: - - The performance characteristics are obvious (e.g., simple getters/setters) - - The component is an internal implementation detail with predictable characteristics - - The component uses well-documented standard library features with no special usage patterns - - The code is not performance-sensitive and prioritizes other aspects (safety, correctness) - -### Async Documentation - -- Document concurrency considerations for async functions -- Specify if a function spawns tasks or requires specific executor context - -```rust -/// Processes the entity asynchronously. -/// -/// # Concurrency -/// -/// This function spawns multiple tasks to process entity attributes in -/// parallel. It should be called from a multi-threaded runtime context. -``` - -## Tooling Integration - -- Use `#[cfg_attr(doc, doc = "...")]` for conditional documentation when appropriate -- Apply `#[doc(hidden)]` for items that are public for technical reasons but not part of the public API -- Support rustdoc search with descriptive wording in documentation -- Use `cargo doc --no-deps --all-features` to verify documentation diff --git a/.agents/rules/rust-error-handling.mdc b/.agents/rules/rust-error-handling.mdc deleted file mode 100644 index 007ba6e5..00000000 --- a/.agents/rules/rust-error-handling.mdc +++ /dev/null @@ -1,74 +0,0 @@ ---- -description: Rust Error Handling -globs: "**/*.rs" -alwaysApply: false ---- -# Rust Error Handling Practices - -## Error Handling Basics - -- Use error-stack (`Report`) instead of anyhow or eyre for error handling -- Use `Box` only in tests or prototyping code -- Use concrete error types with `Report`, not `Report>` -- Use error-stack macros for early returns: - - `ensure!(condition, MyError::SomeVariant)` (from `error_stack::ensure`) - - `bail!(MyError::SomeVariant)` -- Import `Error` from `core::error::` instead of `std::error::` - -## Error Context Enhancement - -- Use `change_context()` to map error types consistently -- Ensure that errors include sufficient context for debugging -- Add `attach()` or `attach_with()` to include relevant debug information -- When reporting errors to users, provide actionable guidance where possible - -## Error Definition - -- Use `derive_more` for error traits, not thiserror: - - ```rust - #[derive(Debug, derive_more::Display)] - #[display("Operation failed: {_variant}")] - pub enum MyCustomError { - #[display("Resource `{id}` not found")] - NotFound { id: String }, - #[display("Operation timed out after {seconds}s")] - Timeout { seconds: u64 }, - } - - impl Error for MyCustomError {} - ``` - -## Specialized Error Handling - -### Streaming Decoders - -- Return `Ok(None)` for incomplete data rather than an error -- Return errors only for actual error conditions like malformed data -- Document buffer management behavior in error cases - -### Async Operations - -- Propagate errors with proper context across async boundaries -- Consider using structured error types for complex async workflows -- Design error types to preserve causal relationships across async tasks - -## Error Documentation - -For comprehensive guidance on documenting errors, refer to the [Rust Documentation Practices](mdc:.cursor/rules/rust-documentation.mdc) file. - -Key points to remember: - -- Document errors with an "# Errors" section for all fallible functions -- Link error variants with proper reference syntax -- Document potential panics with a "# Panics" section - -## Testing Error Conditions - -For guidance on testing error conditions, refer to the [Rust Testing Strategy](mdc:.cursor/rules/rust-testing-strategy.mdc) file. - -Key points: - -- Write tests for each error case in your code -- Assert on specific error types or message contents -- Use `expect_err()` with clear messages following the "should..." format diff --git a/.agents/rules/rust-testing-strategy.mdc b/.agents/rules/rust-testing-strategy.mdc deleted file mode 100644 index d4c583b3..00000000 --- a/.agents/rules/rust-testing-strategy.mdc +++ /dev/null @@ -1,95 +0,0 @@ ---- -description: Rust Testing strategy -globs: "**/*.rs" -alwaysApply: false ---- -# Rust Testing Strategy - -## Verification Commands - -- When modifying Rust code, run clippy to detect issues: - - ```bash - cargo clippy --all-features --all-targets --workspace --no-deps - ``` - -- If changes affect the `hash-graph-api` crate or any representation influencing the public interface, regenerate the OpenAPI specifications: - - ```bash - cd libs/@local/graph/api - cargo run --bin openapi-spec-generator - ``` - -- Use `cargo doc --no-deps --all-features` to check documentation - -## Test Execution - -- Use `cargo-nextest` for running unit and integration tests -- Use the default test runner for documentation tests -- For comprehensive verification, run the complete test suite -- Database seeding can be performed using yarn commands in the project's package.json - -## Test Design Principles - -- Test both happy paths and error conditions for each function -- For each error case in your code, write a corresponding test -- Test boundary conditions and edge cases explicitly -- Include tests for invalid or malformed inputs -- For streaming encoders/decoders, test partial data handling and buffer management -- Aim for high test coverage but prioritize test quality over quantity -- Structure tests following the Arrange-Act-Assert pattern - -## Assertion Standards - -- Use descriptive assertion messages that explain the expected behavior -- All assertion messages (including `expect()`, `unwrap()` and `assert*()`) should follow the "should..." format: - - ```rust - // Good examples: - value.expect("should contain a valid configuration"); - assert_eq!(result, expected, "Result should match the expected value"); - ``` - -- Use `expect()` or `expect_err()` with clear messages instead of `unwrap()` or `unwrap_err()` -- Prefer `assert_eq!` with custom messages over bare assertions when comparing values -- When testing errors, assert on specific error types or message contents, not just that an error occurred -- Balance assertions to verify functionality without creating brittle tests - -## Test Organization - -- Group related tests into appropriate modules -- Use descriptive test names that explain the test scenario and expected outcome -- Do not prefix test function names with `test_` (avoid `test::test_name` patterns) -- Use helper functions to avoid code duplication in tests -- Consider using parameterized tests for testing similar functionality with different inputs - -## Test Code Quality - -- Follow the same code quality standards in test code as in production code -- Add appropriate assertions for array/slice access to avoid clippy warnings -- Document test scenarios with clear comments explaining: - - The setup (input and environment) - - The action being tested - - The expected outcome - - Why the outcome is expected -- Consider adding custom test utilities to simplify common testing patterns -- Use the `json!` macro from `serde_json` instead of constructing JSON as raw strings - -```rust -// Bad: -let json_str = "{\"name\":\"value\",\"nested\":{\"key\":42}}"; - -// Good: -use serde_json::json; -let json_value = json!({ - "name": "value", - "nested": { - "key": 42 - } -}); -``` - -## Integration With Other Practices - -- For documentation practices in tests, refer to the [Rust Documentation Practices](mdc:.cursor/rules/rust-documentation.mdc) file -- For error handling in tests, refer to the [Rust Error Handling Practices](mdc:.cursor/rules/rust-error-handling.mdc) file diff --git a/.agents/rules/rust-tracing-practices.mdc b/.agents/rules/rust-tracing-practices.mdc deleted file mode 100644 index f59a99a0..00000000 --- a/.agents/rules/rust-tracing-practices.mdc +++ /dev/null @@ -1,32 +0,0 @@ ---- -description: Rust Tracing Practices -globs: "**/*.rs" -alwaysApply: false ---- -# Rust Tracing Practices - -## Event Recording - -- Use level-specific macros for appropriate visibility: `info!`, `debug!`, `trace!`, `warn!`, `error!` -- Provide structured data as named arguments to facilitate automated processing and filtering -- Format log messages consistently, beginning with verbs in present tense -- Include relevant context through key-value pairs: - -```rust -tracing::info!(user_id = user.id, action = "login", "User logged in successfully"); -tracing::error!(error = %e, user_id = user.id, "Failed to authenticate user"); -``` - -## Instrumentation - -- Apply the `#[tracing::instrument]` attribute to functions to automatically create trace spans -- Include the `err` parameter to capture error returns in the span -- Exclude sensitive or large parameters using `skip(password)` or `skip_all` -- Add custom context fields with `fields(request_id = req.id())` - -```rust -#[tracing::instrument(level = "debug", err, skip(password), fields(user_id = user.id))] -async fn authenticate_user(user: &User, password: &[u8]) -> Result { - // Implementation -} -``` diff --git a/.agents/rules/update-rules.mdc b/.agents/rules/update-rules.mdc deleted file mode 100644 index 984b271e..00000000 --- a/.agents/rules/update-rules.mdc +++ /dev/null @@ -1,6 +0,0 @@ ---- -description: Apply new rules -globs: -alwaysApply: true ---- -The user might ask to improve certain aspects on rules, such as a best-practice guideline. These guidelines are defined in `.cursor/rules/*.mdc`. Suggest adding new guidelines to these files to improve the code quality as we go. diff --git a/.claude/agents/build-validator.md b/.claude/agents/build-validator.md new file mode 100644 index 00000000..e559e506 --- /dev/null +++ b/.claude/agents/build-validator.md @@ -0,0 +1,43 @@ +# Build Validator + +You are a build validation specialist for the trusted-server project. + +## Your Job + +Validate that the project builds correctly across all targets. + +## Steps + +1. **Native build** + + ```bash + cargo build --workspace + ``` + +2. **WASM build** (production target) + + ```bash + cargo build --bin trusted-server-fastly --release --target wasm32-wasip1 + ``` + +3. **Clippy** + + ```bash + cargo clippy --all-targets --all-features -- -D warnings + ``` + +4. **Format check** + + ```bash + cargo fmt --all -- --check + ``` + +5. **JS build** + ```bash + cd crates/js/lib && node build-all.mjs + ``` + +## Output + +Report each step's status (pass/fail). For failures, include the first error +message and suggest a fix. Summarize with an overall pass/fail verdict. diff --git a/.claude/agents/code-architect.md b/.claude/agents/code-architect.md new file mode 100644 index 00000000..58c43876 --- /dev/null +++ b/.claude/agents/code-architect.md @@ -0,0 +1,35 @@ +# Code Architect + +You are an architecture analyst for the trusted-server project. + +## Your Job + +Analyze the codebase architecture and suggest improvements when asked. + +## Context + +This is a Rust workspace targeting Fastly Compute (`wasm32-wasip1`) with three +crates: `common` (core logic), `fastly` (entry point), and `js` (TS/JS build). + +Key patterns: + +- **RequestWrapper trait** abstracts HTTP handling for different backends +- **Settings-driven config** via `trusted-server.toml` +- **Integration system** with Rust registration + per-integration JS bundles +- **Runtime JS concatenation** — server assembles core + integration scripts + +## When Analyzing + +1. Read relevant source files before making suggestions. +2. Consider WASM constraints (no filesystem, no threads, no Tokio). +3. Respect existing patterns — suggest improvements that fit the current architecture. +4. Prioritize simplicity and correctness over cleverness. + +## Output + +Provide a structured analysis with: + +- Current state summary +- Identified issues or improvement opportunities +- Concrete suggestions with code examples +- Impact assessment (breaking changes, migration effort) diff --git a/.claude/agents/code-simplifier.md b/.claude/agents/code-simplifier.md new file mode 100644 index 00000000..affaf744 --- /dev/null +++ b/.claude/agents/code-simplifier.md @@ -0,0 +1,32 @@ +# Code Simplifier + +You are a code simplification specialist for the trusted-server project. + +## Your Job + +Find overly complex code and suggest simpler alternatives. + +## What to Look For + +- Functions longer than 50 lines that could be broken up +- Deeply nested control flow (3+ levels) +- Unnecessary abstractions or indirection +- Code that could use standard library features more effectively +- Redundant error handling or validation +- Copy-paste patterns that could be consolidated + +## Rules + +- Read the code before suggesting changes. +- Respect the project's conventions (see `CLAUDE.md`). +- Ensure suggestions maintain correctness — simplification must not break behavior. +- Consider WASM constraints when suggesting alternatives. + +## Output + +For each suggestion: + +1. File and line range +2. What's complex and why +3. Simplified alternative with code +4. Confidence level (high/medium/low) diff --git a/.claude/agents/issue-creator.md b/.claude/agents/issue-creator.md new file mode 100644 index 00000000..ed99dd37 --- /dev/null +++ b/.claude/agents/issue-creator.md @@ -0,0 +1,90 @@ +# Issue Creator + +You are an issue creation agent for the trusted-server project +(`IABTechLab/trusted-server`). + +## Steps + +### 1. Determine issue type + +Choose the appropriate type based on the work: + +| Type | ID | Use for | +| ---------- | --------------------- | --------------------------------------- | +| Task | `IT_kwDOBPEB8s4A35x7` | Technical chores, refactoring, CI, deps | +| Bug | `IT_kwDOBPEB8s4A35x9` | Unexpected behavior or errors | +| Feature | `IT_kwDOBPEB8s4A35yA` | New functionality requests | +| Story | `IT_kwDOBPEB8s4BweiI` | User-facing capability (non-internal) | +| Epic | `IT_kwDOBPEB8s4BweiG` | Large multi-issue initiatives | +| Initiative | `IT_kwDOBPEB8s4BweiH` | High-level product/tech/business goals | + +### 2. Draft issue content + +Follow the structure from `.github/ISSUE_TEMPLATE/` for the chosen type: + +- **Bug**: description, reproduction steps, expected behavior, affected area, version, logs +- **Story**: user story ("As a...I want...so that..."), acceptance criteria, affected area +- **Task**: description, done-when criteria, affected area + +### 3. Create the issue + +Assign the issue to the current user with `--assignee @me`: + +``` +gh issue create --title "" --assignee @me --body "$(cat <<'EOF' + +EOF +)" +``` + +### 4. Set the issue type + +GitHub issue types are set via GraphQL (not labels): + +``` +gh api graphql -f query='mutation { + updateIssue(input: { + id: "", + issueTypeId: "" + }) { issue { id title } } +}' +``` + +Get the issue node ID with: + +``` +gh issue view --json id --jq '.id' +``` + +### 5. Add to project board + +Add the issue to the **Trusted Server Development** project (defaults to +Backlog — no status change needed): + +``` +gh api graphql -f query='mutation { + addProjectV2ItemById(input: { + projectId: "PVT_kwDOBPEB8s4BFKrl" + contentId: "" + }) { item { id } } +}' +``` + +### 6. Report + +Output the issue URL and type. + +## Rules + +- Use issue **types**, not labels, for categorization. +- Every issue should have clear done-when / acceptance criteria. +- Use the affected area dropdown values from the templates: + - Core (synthetic IDs, cookies, GDPR) + - Integrations (prebid, lockr, permutive, etc.) + - HTML processing / JS injection + - Ad serving (Equativ) + - Fastly runtime + - JS build pipeline + - Documentation + - CI / Tooling +- Do not create duplicate issues — search first with `gh issue list`. diff --git a/.claude/agents/pr-creator.md b/.claude/agents/pr-creator.md new file mode 100644 index 00000000..c4f69252 --- /dev/null +++ b/.claude/agents/pr-creator.md @@ -0,0 +1,171 @@ +# PR Creator + +You are a pull request creation agent for the trusted-server project +(`IABTechLab/trusted-server`). + +## Steps + +### 1. Gather context + +``` +git status +git diff main...HEAD --stat +git log main..HEAD --oneline +``` + +Understand what changed: which crates, which files, what the commits describe. + +### 2. Run CI gates + +Before creating the PR, verify the branch is healthy: + +``` +cargo fmt --all -- --check +cargo clippy --all-targets --all-features -- -D warnings +cargo test --workspace +cd crates/js/lib && npx vitest run +``` + +If any gate fails, report the failure and stop — do not create a broken PR. + +### 3. Ensure a linked issue exists + +Every PR should close a ticket. + +1. **Ask the user** if there is an existing issue number for this work. +2. If the user provides an issue number, use it in the `Closes #` line. +3. If no issue exists, create one using the appropriate issue type (see Issue + Types below), then reference it in the PR body with `Closes #`. + +Do **not** skip this step or assume an issue exists without asking. + +### 4. Draft PR content + +Using the `.github/pull_request_template.md` structure, draft: + +- **Summary**: 1-3 bullet points describing what the PR does and why. +- **Changes table**: list each file modified and what changed. +- **Closes**: `Closes #` to auto-close the linked issue. +- **Test plan**: check off which verification steps were run. +- **Checklist**: verify each item applies. + +### 5. Create the PR + +Assign the PR to the current user with `--assignee @me`: + +``` +gh pr create --title "" --assignee @me --body "$(cat <<'EOF' + +EOF +)" +``` + +If a PR already exists for the branch, update it instead: + +``` +gh pr edit --title "" --body "$(cat <<'EOF' +<filled template> +EOF +)" +``` + +### 6. Move linked issue to "In review" + +After creating the PR, move the linked issue on the project board — but only +if it is **not** already in "In review" or "Done". + +1. Get the issue node ID: + ``` + gh issue view <number> --json id --jq '.id' + ``` + +2. Query the project item and current status: + ``` + gh api graphql -f query='query { + node(id: "<issue_node_id>") { + ... on Issue { + projectItems(first: 5) { + nodes { + id + project { title } + fieldValueByName(name: "Status") { + ... on ProjectV2ItemFieldSingleSelectValue { + name + optionId + field { ... on ProjectV2SingleSelectField { id options { id name } } } + } + } + } + } + } + } + }' + ``` + +3. If current status is not "In review" or "Done", update it: + ``` + gh api graphql -f query='mutation { + updateProjectV2ItemFieldValue(input: { + projectId: "<project_id>" + itemId: "<item_id>" + fieldId: "<status_field_id>" + value: { singleSelectOptionId: "<in_review_option_id>" } + }) { projectV2Item { id } } + }' + ``` + +### Project Board Reference + +Project: **Trusted Server Development** + +| Status | Option ID | +| ------ | --------- | +| Backlog | `8b41a45a` | +| Ready | `f75ad846` | +| In progress | `47fc9ee4` | +| In review | `4424127f` | +| Done | `98236657` | +| Won't Fix | `b622b030` | + +Field ID: `PVTSSF_lADOBPEB8s4BFKrlzg2lUrA` +Project ID: `PVT_kwDOBPEB8s4BFKrl` + +### 7. Report + +Output the PR URL and a summary of what was included. + +## Issue Types + +This project uses GitHub issue **types** (not labels) to categorize issues. +Set the type via GraphQL after creating the issue: + +``` +gh api graphql -f query='mutation { + updateIssue(input: { + id: "<issue_node_id>", + issueTypeId: "<type_id>" + }) { issue { id title } } +}' +``` + +| Type | ID | Use for | +| ---------- | --------------------- | --------------------------------------- | +| Task | `IT_kwDOBPEB8s4A35x7` | Technical chores, refactoring, CI, deps | +| Bug | `IT_kwDOBPEB8s4A35x9` | Unexpected behavior or errors | +| Feature | `IT_kwDOBPEB8s4A35yA` | New functionality requests | +| Story | `IT_kwDOBPEB8s4BweiI` | User-facing capability (non-internal) | +| Epic | `IT_kwDOBPEB8s4BweiG` | Large multi-issue initiatives | +| Initiative | `IT_kwDOBPEB8s4BweiH` | High-level product/tech/business goals | + +Do **not** use labels as a substitute for types. + +## Rules + +- Keep the PR title under 70 characters. +- Use sentence case for the title. +- Use imperative mood (e.g., "Add caching to proxy" not "Added caching"). +- The summary should focus on _why_, not just _what_. +- Always base PRs against `main` unless told otherwise. +- Never force-push or rebase without explicit user approval. +- Do **not** include any byline, "Generated with" footer, or `Co-Authored-By` + trailer in PR bodies or commit messages. diff --git a/.claude/agents/repo-explorer.md b/.claude/agents/repo-explorer.md new file mode 100644 index 00000000..e7fa3114 --- /dev/null +++ b/.claude/agents/repo-explorer.md @@ -0,0 +1,34 @@ +# Repo Explorer + +You are a codebase exploration specialist for the trusted-server project. + +## Your Job + +Explore the codebase and answer questions about its structure, patterns, and +implementation details. + +## Context + +This is a Rust workspace with three crates: + +- `crates/common/` — core library (integrations, HTML processing, synthetic IDs, GDPR) +- `crates/fastly/` — Fastly Compute entry point +- `crates/js/` — TypeScript/JS build pipeline (per-integration IIFE bundles) + +Target: `wasm32-wasip1` (Fastly Compute) + +## Approach + +1. Use glob and grep to find relevant files quickly. +2. Read source files to understand implementation details. +3. Follow import chains to understand dependencies. +4. Check tests for usage examples and expected behavior. + +## Output + +Provide clear, structured answers with: + +- File paths and line numbers for relevant code +- Code snippets when helpful +- Diagrams or flow descriptions for complex interactions +- Links to related files the user might want to explore diff --git a/.claude/agents/verify-app.md b/.claude/agents/verify-app.md new file mode 100644 index 00000000..b1b7f883 --- /dev/null +++ b/.claude/agents/verify-app.md @@ -0,0 +1,55 @@ +# Verify App + +You are a full verification pipeline for the trusted-server project. + +## Your Job + +Run the complete verification suite and report results. + +## Pipeline + +Run each step in order. Stop and report if any step fails. + +### 1. Format Check + +```bash +cargo fmt --all -- --check +``` + +### 2. Clippy + +```bash +cargo clippy --all-targets --all-features -- -D warnings +``` + +### 3. Rust Tests + +```bash +cargo test --workspace +``` + +### 4. JS Tests + +```bash +cd crates/js/lib && npx vitest run +``` + +### 5. WASM Build + +```bash +cargo build --bin trusted-server-fastly --release --target wasm32-wasip1 +``` + +## Output + +Report a table of results: + +| Step | Status | Notes | +| ---------- | --------- | ------------------ | +| Format | Pass/Fail | ... | +| Clippy | Pass/Fail | ... | +| Rust Tests | Pass/Fail | X passed, Y failed | +| JS Tests | Pass/Fail | X passed, Y failed | +| WASM Build | Pass/Fail | ... | + +If any step fails, include the error output and suggest a fix. diff --git a/.claude/commands/check-ci.md b/.claude/commands/check-ci.md new file mode 100644 index 00000000..2328d347 --- /dev/null +++ b/.claude/commands/check-ci.md @@ -0,0 +1,8 @@ +Run all CI checks locally, in order. Stop and report if any step fails. + +1. `cargo fmt --all -- --check` +2. `cargo clippy --all-targets --all-features -- -D warnings` +3. `cargo test --workspace` +4. `cd crates/js/lib && npx vitest run` + +Report a summary of all results when done. diff --git a/.claude/commands/review-changes.md b/.claude/commands/review-changes.md new file mode 100644 index 00000000..8c954605 --- /dev/null +++ b/.claude/commands/review-changes.md @@ -0,0 +1,11 @@ +Review all staged and unstaged changes in the working tree. + +1. Run `git diff` and `git diff --cached` to see all changes. +2. Review each changed file for: + - Correctness and logic errors + - Style violations (see CLAUDE.md conventions) + - Missing error handling + - Security concerns (hardcoded secrets, injection risks) + - Missing or incorrect tests +3. Suggest specific improvements with code examples. +4. Rate the overall change quality: Good / Needs Work / Concerns. diff --git a/.claude/commands/test-all.md b/.claude/commands/test-all.md new file mode 100644 index 00000000..51faeb7c --- /dev/null +++ b/.claude/commands/test-all.md @@ -0,0 +1,13 @@ +Run the full test suite for both Rust and JavaScript. + +```bash +cargo test --workspace +``` + +Then run JS tests: + +```bash +cd crates/js/lib && npx vitest run +``` + +Report results for both. If any test fails, investigate and suggest a fix. diff --git a/.claude/commands/test-crate.md b/.claude/commands/test-crate.md new file mode 100644 index 00000000..a531287e --- /dev/null +++ b/.claude/commands/test-crate.md @@ -0,0 +1,17 @@ +Test a specific crate by name. + +Usage: /test-crate $ARGUMENTS + +Run: + +```bash +cargo test -p $ARGUMENTS +``` + +If $ARGUMENTS is "js" or "javascript", run: + +```bash +cd crates/js/lib && npx vitest run +``` + +Report results and investigate any failures. diff --git a/.claude/commands/verify.md b/.claude/commands/verify.md new file mode 100644 index 00000000..fef23ec5 --- /dev/null +++ b/.claude/commands/verify.md @@ -0,0 +1,10 @@ +Full verification: build, test, and lint the entire project. + +1. `cargo build --workspace` +2. `cargo build --bin trusted-server-fastly --release --target wasm32-wasip1` +3. `cargo fmt --all -- --check` +4. `cargo clippy --all-targets --all-features -- -D warnings` +5. `cargo test --workspace` +6. `cd crates/js/lib && npx vitest run` + +Report results for each step. Stop and investigate if any step fails. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..c4261ade --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,31 @@ +{ + "permissions": { + "allow": [ + "Bash(cat:*)", + "Bash(find:*)", + "Bash(head:*)", + "Bash(ls:*)", + "Bash(rg:*)", + "Bash(tail:*)", + "Bash(tree:*)", + "Bash(wc:*)", + "Bash(which:*)", + + "Bash(npm ci:*)", + "Bash(npm run:*)", + "Bash(npm test:*)", + + "Bash(cargo build:*)", + "Bash(cargo check:*)", + "Bash(cargo clippy:*)", + "Bash(cargo fmt:*)", + "Bash(cargo metadata:*)", + "Bash(cargo test:*)", + + "Bash(git branch:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git status:*)" + ] + } +} diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 00000000..48e1d755 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,75 @@ +name: Bug Report +description: Report a bug in Trusted Server +labels: ["bug"] +body: + - type: markdown + attributes: + value: | + Thanks for reporting a bug! Please fill out the sections below so we can reproduce and fix it. + + - type: textarea + id: description + attributes: + label: Description + description: A clear description of the bug. + placeholder: What happened? + validations: + required: true + + - type: textarea + id: reproduction + attributes: + label: Steps to reproduce + description: Minimal steps to trigger the bug. + placeholder: | + 1. Configure trusted-server.toml with ... + 2. Run `fastly compute serve` + 3. Send a request to ... + 4. See error ... + validations: + required: true + + - type: textarea + id: expected + attributes: + label: Expected behavior + description: What should have happened instead? + validations: + required: true + + - type: dropdown + id: area + attributes: + label: Affected area + description: Which part of the project is affected? + multiple: true + options: + - Core (synthetic IDs, cookies, GDPR) + - Integrations (prebid, lockr, permutive, etc.) + - HTML processing / JS injection + - Ad serving (Equativ) + - Fastly runtime + - JS build pipeline + - CI / Tooling + validations: + required: true + + - type: input + id: version + attributes: + label: Version + description: Git SHA or version. + placeholder: "commit abc1234" + + - type: textarea + id: logs + attributes: + label: Relevant log output + description: Paste any error messages or logs. + render: shell + + - type: textarea + id: context + attributes: + label: Additional context + description: Anything else that might help (OS, Rust version, related issues). diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000..8005e322 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,2 @@ +blank_issues_enabled: false +contact_links: [] diff --git a/.github/ISSUE_TEMPLATE/story.yml b/.github/ISSUE_TEMPLATE/story.yml new file mode 100644 index 00000000..a847e2cd --- /dev/null +++ b/.github/ISSUE_TEMPLATE/story.yml @@ -0,0 +1,58 @@ +name: Story +description: A user-facing feature or capability +labels: ["story"] +body: + - type: markdown + attributes: + value: | + Describe a new feature or capability from the user's perspective. + + - type: textarea + id: user-story + attributes: + label: User story + description: "As a [role], I want [goal] so that [benefit]." + placeholder: "As a publisher, I want ... so that ..." + validations: + required: true + + - type: textarea + id: acceptance-criteria + attributes: + label: Acceptance criteria + description: What must be true for this story to be considered done? + placeholder: | + - [ ] Criterion 1 + - [ ] Criterion 2 + validations: + required: true + + - type: dropdown + id: scope + attributes: + label: Affected area + description: Which part of the project would this touch? + multiple: true + options: + - Core (synthetic IDs, cookies, GDPR) + - Integrations (prebid, lockr, permutive, etc.) + - HTML processing / JS injection + - Ad serving (Equativ) + - Fastly runtime + - JS build pipeline + - Documentation + - CI / Tooling + validations: + required: true + + - type: textarea + id: solution + attributes: + label: Proposed approach + description: How should this be implemented? Include API examples if relevant. + + - type: textarea + id: context + attributes: + label: Additional context + description: Links, mockups, related issues, or prior art. diff --git a/.github/ISSUE_TEMPLATE/task.yml b/.github/ISSUE_TEMPLATE/task.yml new file mode 100644 index 00000000..91a7eb99 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/task.yml @@ -0,0 +1,51 @@ +name: Task +description: A technical task, chore, or internal improvement +labels: ["task"] +body: + - type: markdown + attributes: + value: | + Describe a technical task that needs to be done (refactoring, CI changes, dependency updates, etc.). + + - type: textarea + id: description + attributes: + label: Description + description: What needs to be done and why? + validations: + required: true + + - type: textarea + id: done-criteria + attributes: + label: Done when + description: How do we know this task is complete? + placeholder: | + - [ ] Criterion 1 + - [ ] Criterion 2 + validations: + required: true + + - type: dropdown + id: scope + attributes: + label: Affected area + description: Which part of the project would this touch? + multiple: true + options: + - Core (synthetic IDs, cookies, GDPR) + - Integrations (prebid, lockr, permutive, etc.) + - HTML processing / JS injection + - Ad serving (Equativ) + - Fastly runtime + - JS build pipeline + - Documentation + - CI / Tooling + validations: + required: true + + - type: textarea + id: context + attributes: + label: Additional context + description: Related issues, dependencies, or notes. diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d8e4fa5c..dac874da 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -5,17 +5,17 @@ version: 2 updates: - - package-ecosystem: 'cargo' # See documentation for possible values - directory: '/' # Location of package manifests + - package-ecosystem: "cargo" # See documentation for possible values + directory: "/" # Location of package manifests schedule: - interval: 'weekly' + interval: "weekly" - - package-ecosystem: 'npm' # See documentation for possible values - directory: 'crates/js/lib/' # Location of package manifests + - package-ecosystem: "npm" # See documentation for possible values + directory: "crates/js/lib/" # Location of package manifests schedule: - interval: 'weekly' + interval: "weekly" - - package-ecosystem: 'npm' # See documentation for possible values - directory: 'docs/' # Location of package manifests + - package-ecosystem: "npm" # See documentation for possible values + directory: "docs/" # Location of package manifests schedule: - interval: 'weekly' + interval: "weekly" diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..d8eccbeb --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,40 @@ +## Summary + +<!-- 1-3 bullet points describing what this PR does and why --> + +- + +## Changes + +<!-- Which files were modified and what changed in each --> + +| File | Change | +| ---- | ------ | +| | | + +## Closes + +<!-- Link to the issue this PR resolves. Every PR should have a ticket. --> +<!-- Use "Closes #123" syntax to auto-close the issue when merged. --> + +Closes # + +## Test plan + +<!-- How did you verify this works? Check all that apply --> + +- [ ] `cargo test --workspace` +- [ ] `cargo clippy --all-targets --all-features -- -D warnings` +- [ ] `cargo fmt --all -- --check` +- [ ] JS tests: `cd crates/js/lib && npx vitest run` +- [ ] WASM build: `cargo build --bin trusted-server-fastly --release --target wasm32-wasip1` +- [ ] Manual testing via `fastly compute serve` +- [ ] Other: <!-- describe --> + +## Checklist + +- [ ] Changes follow [CLAUDE.md](/CLAUDE.md) conventions +- [ ] No `unwrap()` in production code — use `expect("should ...")` +- [ ] Uses `tracing` macros (not `println!`) +- [ ] New code has tests +- [ ] No secrets or credentials committed diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 06d63777..4b963e01 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -13,11 +13,11 @@ name: "CodeQL Advanced" on: push: - branches: [ "main" ] + branches: ["main"] pull_request: - branches: [ "main" ] + branches: ["main"] schedule: - - cron: '42 6 * * 0' + - cron: "42 6 * * 0" jobs: analyze: @@ -43,12 +43,12 @@ jobs: fail-fast: false matrix: include: - - language: actions - build-mode: none - - language: javascript-typescript - build-mode: none - - language: rust - build-mode: none + - language: actions + build-mode: none + - language: javascript-typescript + build-mode: none + - language: rust + build-mode: none # CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'rust', 'swift' # Use `c-cpp` to analyze code written in C, C++ or both # Use 'java-kotlin' to analyze code written in Java, Kotlin or both @@ -58,46 +58,46 @@ jobs: # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages steps: - - name: Checkout repository - uses: actions/checkout@v4 + - name: Checkout repository + uses: actions/checkout@v4 - # Add any setup steps before running the `github/codeql-action/init` action. - # This includes steps like installing compilers or runtimes (`actions/setup-node` - # or others). This is typically only required for manual builds. - # - name: Setup runtime (example) - # uses: actions/setup-example@v1 + # Add any setup steps before running the `github/codeql-action/init` action. + # This includes steps like installing compilers or runtimes (`actions/setup-node` + # or others). This is typically only required for manual builds. + # - name: Setup runtime (example) + # uses: actions/setup-example@v1 - # Initializes the CodeQL tools for scanning. - - name: Initialize CodeQL - uses: github/codeql-action/init@v4 - with: - languages: ${{ matrix.language }} - build-mode: ${{ matrix.build-mode }} - # If you wish to specify custom queries, you can do so here or in a config file. - # By default, queries listed here will override any specified in a config file. - # Prefix the list here with "+" to use these queries and those in the config file. + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v4 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. - # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs - # queries: security-extended,security-and-quality + # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs + # queries: security-extended,security-and-quality - # If the analyze step fails for one of the languages you are analyzing with - # "We were unable to automatically build your code", modify the matrix above - # to set the build mode to "manual" for that language. Then modify this step - # to build your code. - # ℹ️ Command-line programs to run using the OS shell. - # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun - - name: Run manual build steps - if: matrix.build-mode == 'manual' - shell: bash - run: | - echo 'If you are using a "manual" build mode for one or more of the' \ - 'languages you are analyzing, replace this with the commands to build' \ - 'your code, for example:' - echo ' make bootstrap' - echo ' make release' - exit 1 + # If the analyze step fails for one of the languages you are analyzing with + # "We were unable to automatically build your code", modify the matrix above + # to set the build mode to "manual" for that language. Then modify this step + # to build your code. + # ℹ️ Command-line programs to run using the OS shell. + # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun + - name: Run manual build steps + if: matrix.build-mode == 'manual' + shell: bash + run: | + echo 'If you are using a "manual" build mode for one or more of the' \ + 'languages you are analyzing, replace this with the commands to build' \ + 'your code, for example:' + echo ' make bootstrap' + echo ' make release' + exit 1 - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 - with: - category: "/language:${{matrix.language}}" + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v4 + with: + category: "/language:${{matrix.language}}" diff --git a/.gitignore b/.gitignore index af8ff99b..a408536f 100644 --- a/.gitignore +++ b/.gitignore @@ -15,9 +15,14 @@ src/*.html .DS_Store # IDE -.claude .specstory .vscode +# Claude Code — ignore all, then whitelist shared config +.claude/* +!.claude/settings.json +!.claude/commands/ +!.claude/agents/ + # SSL certificates *.pem diff --git a/AGENTS.md b/AGENTS.md index 7d3a8a93..17b51b88 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,117 +1,25 @@ # AGENTS.md -Centralized guidelines for AI coding agents working in this repository. Review the -common setup steps before executing tasks, then follow any assistant-specific -instructions. +**Before doing anything else, read `CLAUDE.md` in this repository root.** It +contains all project conventions, coding standards, build commands, workflow +rules, and CI requirements. Everything in `CLAUDE.md` applies to you. -## Common Setup +This file exists because Codex looks for `AGENTS.md` by convention. All shared +rules are maintained in `CLAUDE.md` to avoid duplication and drift. If you +cannot access `CLAUDE.md`, the critical rules are summarized below as a +fallback. -- Always read the full coding standards at the start of a session: +--- - ```bash - cat .agents/rules/* - ``` +## Fallback Summary -- Use `cargo fmt`, `cargo clippy`, and `cargo test` where appropriate before - delivering code changes. -- When work touches Fastly behavior or runtime configuration, review - `fastly.toml`, `trusted-server.toml`, and `.env.dev`. +If you cannot read `CLAUDE.md`, follow these rules: -## Claude Code - -This section consolidates the guidance previously stored in `CLAUDE.md` for -Claude Code (claude.ai/code). - -### Project Overview - -Rust-based edge computing application targeting Fastly Compute. Handles -privacy-preserving synthetic ID generation, ad serving with GDPR compliance, and -real-time bidding integration. - -### Key Commands - -#### Build & Development - -```bash -# Standard build -cargo build - -# Production build for Fastly -cargo build --bin trusted-server-fastly --release --target wasm32-wasip1 - -# Run locally with Fastly simulator -fastly compute serve - -# Deploy to Fastly -fastly compute publish -``` - -#### Testing & Quality - -```bash -# Run tests (requires viceroy) -cargo test - -# Install test runtime if needed -cargo install viceroy - -# Format code -cargo fmt - -# Lint with clippy -cargo clippy --all-targets --all-features - -# Check compilation -cargo check -``` - -### Architecture Overview - -#### Workspace Structure - -- **trusted-server-common**: Core library with shared functionality - - Synthetic ID generation (`src/synthetic.rs`) - - Cookie handling (`src/cookies.rs`) - - HTTP abstractions (`src/http_wrapper.rs`) - - GDPR consent management (`src/gdpr.rs`) -- **trusted-server-fastly**: Fastly-specific implementation - - Main application entry point - - Fastly SDK integration - - Request/response handling - -### Key Design Patterns - -1. **RequestWrapper Trait**: Abstracts HTTP request handling to support different - backends. -2. **Settings-Driven Config**: External configuration via `trusted-server.toml`. -3. **Privacy-First**: All tracking requires GDPR consent checks. -4. **HMAC-Based IDs**: Synthetic IDs generated using HMAC with configurable templates. - -### Configuration Files - -- `fastly.toml`: Fastly service configuration and build settings. -- `trusted-server.toml`: Application settings (ad servers, KV stores, ID templates). -- `rust-toolchain.toml`: Pins Rust version to 1.91.1. - -### Key Functionality Areas - -1. Synthetic ID generation using HMAC-based templates. -2. Ad serving integrations (currently Equativ). -3. GDPR consent handling and validation. -4. Geolocation utilities (DMA code extraction). -5. Prebid integration for real-time bidding flows. -6. KV store usage for counters and domain mappings. - -### Testing Approach - -- Unit tests reside alongside source files under `#[cfg(test)]` modules. -- Uses Viceroy for local Fastly Compute simulation. -- GitHub Actions CI runs format and test workflows. - -### Important Notes - -- Target platform is WebAssembly (`wasm32-wasip1`). -- Relies on Fastly KV stores for persistence. -- Uses Handlebars templates for dynamic responses. -- Emits detailed logs for edge debugging. -- Follow the commit message format in `CONTRIBUTING.md`. +1. Present a plan and get approval before coding. +2. Keep changes minimal — do not refactor unrelated code. +3. Run `cargo test --workspace` after every code change. +4. Run `cargo fmt --all -- --check` and `cargo clippy --all-targets --all-features -- -D warnings`. +5. Run JS tests with `cd crates/js/lib && npx vitest run` when touching JS/TS code. +6. Use `error-stack` (`Report<E>`) for error handling — not anyhow, eyre, or thiserror. +7. Use `tracing` macros (not `println!`) and `expect("should ...")` (not `unwrap()`). +8. Target is `wasm32-wasip1` — no Tokio or OS-specific dependencies in core crates. diff --git a/CLAUDE.md b/CLAUDE.md index 2968df42..bdf7de81 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,4 +1,401 @@ # CLAUDE.md -Guidance for Claude Code has moved to `AGENTS.md`. Keep this stub for tooling that -still expects `CLAUDE.md`, but refer to `AGENTS.md` for the canonical instructions. +> Single source of truth for all AI coding agents (Claude Code, Codex, Cursor, +> etc.). If you're reading `AGENTS.md`, it redirects here. + +## Project Overview + +Rust-based edge computing application targeting **Fastly Compute**. Handles +privacy-preserving synthetic ID generation, ad serving with GDPR compliance, +real-time bidding integration, and publisher-side JavaScript injection. + +## Workspace Layout + +``` +crates/ + common/ # Core library — shared logic, integrations, HTML processing + fastly/ # Fastly Compute entry point (wasm32-wasip1 binary) + js/ # TypeScript/JS build — per-integration IIFE bundles + lib/ # TS source, Vitest tests, esbuild pipeline +``` + +Supporting files: `fastly.toml`, `trusted-server.toml`, `.env.dev`, +`rust-toolchain.toml`, `CONTRIBUTING.md`. + +## Toolchain + +| Tool | Version / Target | +|------|-----------------| +| Rust | 1.91.1 (pinned in `rust-toolchain.toml`) | +| WASM target | `wasm32-wasip1` | +| Node | LTS (for JS build) | +| Viceroy | Latest (Fastly local simulator) | + +--- + +## Build & Test Commands + +### Rust + +```bash +# Build +cargo build + +# Production build for Fastly +cargo build --bin trusted-server-fastly --release --target wasm32-wasip1 + +# Run locally with Fastly simulator +fastly compute serve + +# Deploy to Fastly +fastly compute publish +``` + +### Testing & Quality + +```bash +# Run all Rust tests (uses viceroy) +cargo test --workspace + +# Format +cargo fmt --all -- --check + +# Lint +cargo clippy --all-targets --all-features -- -D warnings + +# Check compilation +cargo check + +# JS tests +cd crates/js/lib && npx vitest run + +# JS build +cd crates/js/lib && node build-all.mjs +``` + +### Install prerequisites + +```bash +cargo install viceroy # Fastly local test runtime +``` + +--- + +## Coding Conventions + +### Rust Style + +- Use the **2024 edition** of Rust. +- Prefer `derive_more` over manual trait implementations. +- Use `#[expect(lint, reason = "...")]` over `#[allow(lint)]`. +- Use `rustfmt` to format code. +- Invoke clippy with `--all-targets --all-features -- -D warnings`. +- Use `cargo doc --no-deps --all-features` for checking documentation. + +### Type System + +- Create strong types with **newtype patterns** for domain entities. +- Consider visibility carefully — avoid unnecessary `pub`. + +```rust +#[derive(Debug, Copy, Clone, Eq, Hash, PartialEq, derive_more::Display)] +pub struct UserId(Uuid); +``` + +### Function Arguments + +- Functions should **never** take more than 7 arguments — use a struct instead. +- Take references for immutable access, mutable references for modification. Only take ownership when the function consumes the value. +- Make functions `const` whenever possible. +- Preferred argument types (when it doesn't reduce performance): + - `impl AsRef<str>` instead of `&str` or `&String` + - `impl AsRef<Path>` instead of `&Path` or `&PathBuf` + - `impl IntoIterator<Item = &T>` when only iterating + - `&[T]` instead of `&Vec<T>` +- Never use `impl Into<Option<_>>` — it hides that `None` can be passed. + +### `From` / `Into` + +- Prefer `From` implementations over `Into` — the compiler derives `Into` automatically. +- Prefer `Type::from(value)` over `value.into()` for clarity. +- For wrapper types (`Cow`, `Arc`, `Rc`, `Box`), use explicit constructors. + +### Smart Pointers + +- Always use `Arc::clone(&pointer)` instead of `pointer.clone()` to clearly indicate you're cloning the reference. + +### Async Patterns + +- Use `impl Future<Output = T> + Send` in trait definitions. + +### Allocations + +- Minimize allocations — reuse buffers, prefer borrowed data. +- Balance performance and readability. + +### Naming Conventions + +- Do **not** prefix test names with `test_` (avoids `test::test_name`). +- Avoid abbreviations unless widely recognized (e.g., `Http`, `Json` — not `Ctx`). +- Do not suffix names with their types (`users` not `usersList`). + +### Import Style + +- No local imports within functions or blocks. +- No wildcard imports (`use super::*`, `use crate::module::*`). +- Never use a prelude `use crate::prelude::*`. +- Prefer `core` > `alloc` > `std` to minimize dependencies. +- Use `use Trait as _` when you only need trait methods, not the trait name. +- Prefer qualified imports for frequently used types. + +### Comments and Assertions + +- Place comments on separate lines **above** the code, never inline. +- All `expect()` messages should follow the format `"should ..."`. +- Use descriptive assertion messages: `assert_eq!(result, expected, "should match expected output")`. + +### Crate Preferences + +- `tracing` (not `log`) for instrumentation. +- `similar_asserts` for test assertions. +- `insta` for snapshot tests. + +--- + +## Error Handling + +- Use `error-stack` (`Report<MyError>`) — not anyhow or eyre. +- Use `Box<dyn Error>` only in tests or prototyping. +- Use concrete error types with `Report<E>`. +- Use `ensure!()` / `bail!()` macros for early returns. +- Import `Error` from `core::error::` instead of `std::error::`. +- Use `change_context()` to map error types, `attach()` / `attach_with()` for debug info. +- Define errors with `derive_more::Display` (not thiserror): + +```rust +#[derive(Debug, derive_more::Display)] +pub enum MyError { + #[display("Resource `{id}` not found")] + NotFound { id: String }, + #[display("Operation timed out after {seconds}s")] + Timeout { seconds: u64 }, +} + +impl core::error::Error for MyError {} +``` + +--- + +## Testing Strategy + +- Unit tests live alongside source files under `#[cfg(test)]` modules. +- Uses **Viceroy** for local Fastly Compute simulation. +- GitHub Actions CI runs format and test workflows. +- Structure tests with **Arrange-Act-Assert** pattern. +- Test both happy paths and error conditions. +- Use `expect()` / `expect_err()` with `"should ..."` messages instead of `unwrap()`. +- Use `json!` macro instead of raw JSON strings. +- Do not prefix test function names with `test_`. +- Follow the same code quality standards in test code as production code. +- For JS tests: use `vi.hoisted()` for mock definitions referenced in `vi.mock()` factories. + +--- + +## Documentation Standards + +- Each public item must have a doc comment. +- Begin with a single-line summary, blank line, then details. +- Always use intra-doc links (`[`Item`]`) for referenced types. +- Document errors with `# Errors` section for all fallible functions. +- Document panics with `# Panics` section. +- Add `# Examples` sections for public API functions. +- Add `# Performance` sections for performance-critical functions. +- Skip documentation for standard trait implementations unless behavior is unique. +- Use `cargo doc --no-deps --all-features` to verify. + +--- + +## Tracing Practices + +- Use level-specific macros: `info!`, `debug!`, `trace!`, `warn!`, `error!`. +- Provide structured data as named arguments. +- Format messages with present-tense verbs. +- Apply `#[tracing::instrument]` with `err`, `skip()`, and `fields()`: + +```rust +#[tracing::instrument(level = "debug", err, skip(password), fields(user_id = user.id))] +async fn authenticate_user(user: &User, password: &[u8]) -> Result<AuthToken, AuthError> { + // ... +} +``` + +--- + +## Git Commit Conventions + +- Be descriptive and concise. +- Use sentence case (capitalize first word). +- Imperative, present-tense style. +- No semantic prefixes (`fix:`, `feat:`, `chore:`). +- No bracketed tags (`[Docs]`, `[Fix]`). +- Follow the detailed guidelines in `CONTRIBUTING.md`. + +Good: `"Add feature flags to Real type tests that require serde"` +Bad: `"fix: added feature flags"` + +--- + +## Integration System + +Integrations register in Rust via: +```rust +IntegrationRegistration::builder(ID) + .with_proxy() + .with_attribute_rewriter() + .with_head_injector() + .build() +``` + +- Integration IDs match JS directory names: `prebid`, `lockr`, `permutive`, `datadome`, `didomi`, `testlight`. +- `creative` is JS-only (no Rust registration); `nextjs`, `aps`, `adserver_mock` are Rust-only. +- `IntegrationRegistry::js_module_ids()` maps registered integrations to JS module names. + +## JS Build Pipeline + +- `build-all.mjs` discovers `src/integrations/*/index.ts` and builds each as a separate IIFE. +- Output: `dist/tsjs-core.js`, `dist/tsjs-{integration}.js`. +- `build.rs` auto-generates `tsjs_modules.rs` with `include_str!()` for each discovered file. +- `bundle.rs` provides `concatenate_modules(ids)` and `concatenated_hash(ids)` APIs. +- Runtime: Rust server concatenates core + enabled integration JS files at request time. + +--- + +## Configuration Files + +| File | Purpose | +|------|---------| +| `fastly.toml` | Fastly service configuration and build settings | +| `trusted-server.toml` | Application settings (ad servers, KV stores, ID templates) | +| `rust-toolchain.toml` | Pins Rust version to 1.91.1 | +| `.env.dev` | Local development environment variables | + +--- + +## CI Gates + +Every PR must pass: + +1. `cargo fmt --all -- --check` +2. `cargo clippy --all-targets --all-features -- -D warnings` +3. `cargo test --workspace` +4. JS build and test (`cd crates/js/lib && npx vitest run`) + +--- + +## Standard Workflow + +1. **Read & plan** — understand the request, explore relevant code. +2. **Get approval** — for non-trivial changes, present a plan first. +3. **Implement incrementally** — small, testable changes. Every change should + impact as little code as possible. +4. **Test after every change** — `cargo test --workspace`. +5. **Explain as you go** — describe what you changed and why. +6. **If blocked** — explain what's blocking and why. + +## Verification & Quality + +- **Verify, don't assume**: after implementing a change, prove it works. Run + tests, check clippy, and compare behavior against `main` when relevant. + Don't say "it works" without evidence. +- **Plan review**: for complex tasks, review your own plan as a staff engineer + would before implementing. Ask: is this the simplest approach? Does it touch + too many files? Are there edge cases? +- **Escape hatch**: if an implementation is going sideways after multiple + iterations, step back and reconsider. Scrap the approach and implement the + simpler solution rather than patching a flawed design. +- **Minimal changes**: every change should impact as little code as possible. + Avoid unnecessary refactoring, docstrings on untouched code, or premature + abstractions. + +--- + +## Subagents + +For complex tasks, use specialized subagents (`.claude/agents/`): + +| Agent | When to use | +|-------|-------------| +| `build-validator` | Validate build across native + wasm32-wasip1 targets | +| `code-architect` | Analyze architecture, suggest improvements | +| `code-simplifier` | Find and simplify overly complex code | +| `verify-app` | Full verification pipeline (build + test + lint) | +| `pr-creator` | Create well-structured pull requests | +| `issue-creator` | Create GitHub issues with proper types via GraphQL | +| `repo-explorer` | Explore and answer questions about the codebase | + +### Multi-Phase Workflow (for complex tasks) + +1. **Phase 1 — Investigation** (read-only): launch subagents with + non-overlapping scopes. Each must return concrete findings with file paths. +2. **Phase 2 — Solution proposals** (no edits): propose minimal fix strategies. + Compare tradeoffs before coding. +3. **Phase 3 — Implementation**: pick one plan (smallest safe change) and + implement centrally. +4. **Phase 4 — Verification**: run `verify-app` or `build-validator`. +5. **Phase 5 — Ship**: use `pr-creator` to create the PR. + +**Default trigger**: use this workflow when work touches 2+ crates or includes +both runtime behavior and build/tooling changes. + +### Selection Matrix + +| Situation | Use first | Optional follow-up | Expected output | +|-----------|-----------|-------------------|-----------------| +| Unfamiliar code area | `repo-explorer` | `code-architect` | File map and risk hotspots | +| Multi-crate change | `repo-explorer` | `code-architect`, `build-validator` | Change plan and validation scope | +| CI/build failures | `build-validator` | `repo-explorer` | Failing combos and fault area | +| Design/API proposal | `code-architect` | `repo-explorer` | Architecture concerns and options | +| Cleanup/refactor | `code-simplifier` | `build-validator` | Simplification summary and checks | +| Pre-PR readiness | `build-validator` | `verify-app`, `pr-creator` | Pass/fail report and PR draft | + +--- + +## Slash Commands + +| Command | Purpose | +|---------|---------| +| `/test-all` | Run full test suite (Rust + JS) | +| `/check-ci` | Run all CI checks locally | +| `/verify` | Full verification: build, test, lint | +| `/review-changes` | Review staged/unstaged changes for issues | +| `/test-crate` | Test a specific crate by name | + +--- + +## Key Files + +| File | Purpose | +|------|---------| +| `crates/common/src/integrations/registry.rs` | IntegrationRegistry, `js_module_ids()` | +| `crates/common/src/tsjs.rs` | Script tag generation with module IDs | +| `crates/common/src/html_processor.rs` | Injects `<script>` at `<head>` start | +| `crates/common/src/publisher.rs` | `/static/tsjs=` handler, concatenates modules | +| `crates/common/src/synthetic.rs` | Synthetic ID generation | +| `crates/common/src/cookies.rs` | Cookie handling | +| `crates/common/src/gdpr.rs` | GDPR consent management | +| `crates/common/src/http_wrapper.rs` | HTTP abstractions | +| `crates/js/build.rs` | Discovers dist files, generates `tsjs_modules.rs` | +| `crates/js/src/bundle.rs` | Module map, concatenation, hashing | + +--- + +## What NOT to Do + +- Do not add unnecessary dependencies without justification. +- Do not use `println!` / `eprintln!` — use `tracing` macros. +- Do not use `unwrap()` in production code — use `expect("should ...")`. +- Do not use thiserror — use `derive_more::Display` + `impl Error`. +- Do not use wildcard imports. +- Do not commit `.env` files or secrets. +- Do not make large refactors without approval. +- Always run tests and linting before committing. diff --git a/README.md b/README.md index 9c83e468..d70c0e5a 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,6 @@ Trusted Server is an open-source, cloud based orchestration framework and runtim Trusted Server is the new execution layer for the open-web, returning control of 1st party data, security, and overall user-experience back to publishers. - ## Documentation The guide in `docs/guide/` (published at the link below) is the source of truth for human-readable documentation. This README is a brief overview. @@ -40,7 +39,7 @@ fastly compute serve cargo fmt # Lint -cargo clippy --all-targets --all-features --workspace --no-deps +cargo clippy --all-targets --all-features -- -D warnings # Run tests cargo test diff --git a/docs/guide/onboarding.md b/docs/guide/onboarding.md index 84ffd3e6..a25afe7e 100644 --- a/docs/guide/onboarding.md +++ b/docs/guide/onboarding.md @@ -99,13 +99,13 @@ Ask your manager or onboarding buddy for calendar invites to relevant meetings. ### Documentation -| Resource | Description | -| ----------------------------------------------------------------------------------------- | --------------------------------------------- | -| [README.md](https://github.com/IABTechLab/trusted-server/blob/main/README.md) | Project overview and setup | -| [CONTRIBUTING.md](https://github.com/IABTechLab/trusted-server/blob/main/CONTRIBUTING.md) | Contribution guidelines | -| [AGENTS.md](https://github.com/IABTechLab/trusted-server/blob/main/AGENTS.md) | AI assistant guidance / architecture overview | -| [SEQUENCE.md](https://github.com/IABTechLab/trusted-server/blob/main/SEQUENCE.md) | Request flow diagrams | -| [FAQ_POC.md](https://github.com/IABTechLab/trusted-server/blob/main/FAQ_POC.md) | Frequently asked questions | +| Resource | Description | +| ----------------------------------------------------------------------------------------- | ------------------------------------------- | +| [README.md](https://github.com/IABTechLab/trusted-server/blob/main/README.md) | Project overview and setup | +| [CONTRIBUTING.md](https://github.com/IABTechLab/trusted-server/blob/main/CONTRIBUTING.md) | Contribution guidelines | +| [CLAUDE.md](https://github.com/IABTechLab/trusted-server/blob/main/CLAUDE.md) | AI agent conventions and project guidelines | +| [SEQUENCE.md](https://github.com/IABTechLab/trusted-server/blob/main/SEQUENCE.md) | Request flow diagrams | +| [FAQ_POC.md](https://github.com/IABTechLab/trusted-server/blob/main/FAQ_POC.md) | Frequently asked questions | For docs site development, see `docs/README.md`. diff --git a/docs/guide/testing.md b/docs/guide/testing.md index 689b6d74..96fee697 100644 --- a/docs/guide/testing.md +++ b/docs/guide/testing.md @@ -240,7 +240,7 @@ cargo fmt ```bash # Run clippy with all checks -cargo clippy --all-targets --all-features --workspace --no-deps +cargo clippy --all-targets --all-features -- -D warnings # Fix clippy warnings automatically cargo clippy --fix --allow-dirty @@ -274,7 +274,7 @@ jobs: - name: Run tests run: cargo test - name: Run clippy - run: cargo clippy --all-targets --all-features --workspace --no-deps + run: cargo clippy --all-targets --all-features -- -D warnings ``` ## Debugging Tests From c4e02e32b35046e89b64517d625919b4c9f117b6 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 14:47:02 -0800 Subject: [PATCH 02/11] Fixed formating --- .claude/agents/pr-creator.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/.claude/agents/pr-creator.md b/.claude/agents/pr-creator.md index c4f69252..a75a4182 100644 --- a/.claude/agents/pr-creator.md +++ b/.claude/agents/pr-creator.md @@ -75,11 +75,13 @@ After creating the PR, move the linked issue on the project board — but only if it is **not** already in "In review" or "Done". 1. Get the issue node ID: + ``` gh issue view <number> --json id --jq '.id' ``` 2. Query the project item and current status: + ``` gh api graphql -f query='query { node(id: "<issue_node_id>") { @@ -118,14 +120,14 @@ if it is **not** already in "In review" or "Done". Project: **Trusted Server Development** -| Status | Option ID | -| ------ | --------- | -| Backlog | `8b41a45a` | -| Ready | `f75ad846` | +| Status | Option ID | +| ----------- | ---------- | +| Backlog | `8b41a45a` | +| Ready | `f75ad846` | | In progress | `47fc9ee4` | -| In review | `4424127f` | -| Done | `98236657` | -| Won't Fix | `b622b030` | +| In review | `4424127f` | +| Done | `98236657` | +| Won't Fix | `b622b030` | Field ID: `PVTSSF_lADOBPEB8s4BFKrlzg2lUrA` Project ID: `PVT_kwDOBPEB8s4BFKrl` From c46069da59a1624538f8ba9f2da0dc8a3bb2414b Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:10:55 -0800 Subject: [PATCH 03/11] Align pr-creator with mocktioneer conventions Add project board add-to-project fallback step and use cleaner GraphQL variable syntax for issue lookup. --- .claude/agents/pr-creator.md | 45 +++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/.claude/agents/pr-creator.md b/.claude/agents/pr-creator.md index a75a4182..a61d853d 100644 --- a/.claude/agents/pr-creator.md +++ b/.claude/agents/pr-creator.md @@ -74,48 +74,51 @@ EOF After creating the PR, move the linked issue on the project board — but only if it is **not** already in "In review" or "Done". -1. Get the issue node ID: +1. Get the issue's project item ID and current status: ``` - gh issue view <number> --json id --jq '.id' - ``` - -2. Query the project item and current status: - - ``` - gh api graphql -f query='query { - node(id: "<issue_node_id>") { + gh api graphql -f query='query($issueId: ID!) { + node(id: $issueId) { ... on Issue { - projectItems(first: 5) { + projectItems(first: 10) { nodes { id - project { title } fieldValueByName(name: "Status") { - ... on ProjectV2ItemFieldSingleSelectValue { - name - optionId - field { ... on ProjectV2SingleSelectField { id options { id name } } } - } + ... on ProjectV2ItemFieldSingleSelectValue { name optionId } } } } } } - }' + }' -f issueId="$(gh issue view <number> --json id --jq '.id')" ``` -3. If current status is not "In review" or "Done", update it: +2. If current status is not "In review" or "Done", update it: + ``` gh api graphql -f query='mutation { updateProjectV2ItemFieldValue(input: { - projectId: "<project_id>" + projectId: "PVT_kwDOBPEB8s4BFKrl" itemId: "<item_id>" - fieldId: "<status_field_id>" - value: { singleSelectOptionId: "<in_review_option_id>" } + fieldId: "PVTSSF_lADOBPEB8s4BFKrlzg2lUrA" + value: { singleSelectOptionId: "4424127f" } }) { projectV2Item { id } } }' ``` +3. If the issue is not yet on the project, add it first: + + ``` + gh api graphql -f query='mutation { + addProjectV2ItemById(input: { + projectId: "PVT_kwDOBPEB8s4BFKrl" + contentId: "<issue_node_id>" + }) { item { id } } + }' + ``` + + Then set the status as above. + ### Project Board Reference Project: **Trusted Server Development** From 081a307e593cd87b6d184367df443292514a7409 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:14:16 -0800 Subject: [PATCH 04/11] Fix pr-creator to move issue to In progress, not In review --- .claude/agents/pr-creator.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.claude/agents/pr-creator.md b/.claude/agents/pr-creator.md index a61d853d..04b37868 100644 --- a/.claude/agents/pr-creator.md +++ b/.claude/agents/pr-creator.md @@ -69,7 +69,7 @@ EOF )" ``` -### 6. Move linked issue to "In review" +### 6. Move linked issue to "In progress" After creating the PR, move the linked issue on the project board — but only if it is **not** already in "In review" or "Done". @@ -93,7 +93,7 @@ if it is **not** already in "In review" or "Done". }' -f issueId="$(gh issue view <number> --json id --jq '.id')" ``` -2. If current status is not "In review" or "Done", update it: +2. If current status is not "In review" or "Done", set it to "In progress" (`47fc9ee4`): ``` gh api graphql -f query='mutation { @@ -101,7 +101,7 @@ if it is **not** already in "In review" or "Done". projectId: "PVT_kwDOBPEB8s4BFKrl" itemId: "<item_id>" fieldId: "PVTSSF_lADOBPEB8s4BFKrlzg2lUrA" - value: { singleSelectOptionId: "4424127f" } + value: { singleSelectOptionId: "47fc9ee4" } }) { projectV2Item { id } } }' ``` From f915d4e9b0c5b222cb418faf4cb30408bc93ed95 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:27:00 -0800 Subject: [PATCH 05/11] Add explicit --assignee @me rule to pr-creator --- .claude/agents/pr-creator.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/agents/pr-creator.md b/.claude/agents/pr-creator.md index 04b37868..b8495d81 100644 --- a/.claude/agents/pr-creator.md +++ b/.claude/agents/pr-creator.md @@ -171,6 +171,7 @@ Do **not** use labels as a substitute for types. - Use imperative mood (e.g., "Add caching to proxy" not "Added caching"). - The summary should focus on _why_, not just _what_. - Always base PRs against `main` unless told otherwise. +- Always assign the PR to the current user (`--assignee @me`). - Never force-push or rebase without explicit user approval. - Do **not** include any byline, "Generated with" footer, or `Co-Authored-By` trailer in PR bodies or commit messages. From 961ae8d0dd5fb03cb159a040992b69d11e3650a2 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 17:13:45 -0800 Subject: [PATCH 06/11] Add pr-reviewer subagent for staff-engineer PR reviews Adapted from edgezero PR #190. Performs deep analysis of changed files, classifies findings by severity, and submits formal GitHub reviews with inline comments after user approval. --- .claude/agents/pr-reviewer.md | 258 ++++++++++++++++++++++++++++++++++ CLAUDE.md | 95 +++++++------ 2 files changed, 307 insertions(+), 46 deletions(-) create mode 100644 .claude/agents/pr-reviewer.md diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md new file mode 100644 index 00000000..d4b5f4a5 --- /dev/null +++ b/.claude/agents/pr-reviewer.md @@ -0,0 +1,258 @@ +# PR Reviewer + +You are a staff-engineer-level code review agent for the trusted-server project +(`IABTechLab/trusted-server`). You perform thorough reviews of pull requests and +submit formal GitHub PR reviews with inline comments. + +## Input + +You will receive either: + +- A PR number (e.g., `#165`) +- A branch name to review against `main` +- No input — in which case review the current branch against `main` + +## Steps + +### 1. Gather PR context + +``` +gh pr view <number> --json number,title,body,headRefName,headRefOid,baseRefName,commits +git diff main...HEAD --stat +git log main..HEAD --oneline +``` + +If no PR number is given, find the PR for the current branch: + +``` +gh pr list --head "$(git branch --show-current)" --json number --jq '.[0].number' +``` + +If no PR exists, review the branch diff directly and skip the GitHub review +submission (report findings as text instead). + +### 2. Read all changed files + +Get the full list of changed files and read every one: + +``` +git diff main...HEAD --name-only +``` + +Read each file in its entirety. Do not skip files or skim — a thorough review +requires understanding the full context of every change. + +### 3. Run CI gates + +Verify the branch is healthy before reviewing: + +``` +cargo fmt --all -- --check +cargo clippy --all-targets --all-features -- -D warnings +cargo test --workspace +cd crates/js/lib && npx vitest run +``` + +Note any CI failures in the review but continue with the code review regardless. + +### 4. Deep analysis + +For each changed file, evaluate: + +#### Correctness + +- Logic errors, off-by-one, missing edge cases +- Race conditions (especially in concurrent/async code) +- Error handling: are errors propagated, swallowed, or misclassified? +- Resource leaks (files, connections, transactions) + +#### WASM compatibility + +- Target is `wasm32-wasip1` — no std::net, std::thread, or OS-specific APIs +- No Tokio or runtime-specific deps in `crates/common` +- Fastly-specific APIs only in `crates/fastly` + +#### Convention compliance (from CLAUDE.md) + +- `expect("should ...")` instead of `unwrap()` in production code +- `thiserror` for library errors, `anyhow` for application-level +- `tracing` macros (not `println!`) +- `vi.hoisted()` for mock definitions in JS tests +- Integration IDs match JS directory names +- Colocated tests with `#[cfg(test)]` + +#### Security + +- Input validation: size limits on bodies, key lengths, value sizes +- No unbounded allocations (collect without limits, unbounded Vec growth) +- No secrets or credentials in committed files +- OWASP top 10: XSS, injection, etc. + +#### API design + +- Public API surface: too broad? Too narrow? Breaking changes? +- Consistency with existing patterns in the codebase +- Error types: are they specific enough for callers to handle? + +#### Dependencies + +- New deps justified? WASM compatible (`wasm32-wasip1`)? +- Feature gating: are deps behind the correct feature flags? +- Unconditional deps that should be optional + +#### Test coverage + +- Are new code paths tested? +- Are edge cases covered (empty input, max values, error paths)? +- Rust tests: `cargo test --workspace` +- JS tests: `npx vitest run` in `crates/js/lib/` + +### 5. Classify findings + +Assign each finding a severity: + +| Severity | Criteria | +| ------------ | ------------------------------------------------------------------ | +| P0 — Blocker | Must fix before merge: bugs, data loss, security, CI failures | +| P1 — High | Should fix: race conditions, API design issues, missing validation | +| P2 — Medium | Recommended: inconsistencies, test gaps, dead code | +| P3 — Low | Nice to have: style, minor improvements, documentation | + +### 6. Present findings for user approval + +**Do not submit the review automatically.** Present all findings to the user +organized by severity, with: + +- Severity and title +- File path and line number +- Description and suggested fix +- Whether it would be an inline comment or body-level finding + +Ask the user which findings to include in the PR review. The user may: + +- Approve all findings +- Exclude specific findings +- Adjust severity levels +- Edit descriptions +- Add additional comments + +Wait for explicit confirmation before proceeding to submission. + +### 7. Submit GitHub PR review + +After user approval, submit the selected findings as a formal review. + +#### Determine the review verdict + +- If any P0 findings are included: `CHANGES_REQUESTED` +- If any P1 findings are included: `CHANGES_REQUESTED` +- If only P2 or below: `COMMENT` +- If no findings: `APPROVE` + +#### Build inline comments + +For each finding that can be pinpointed to a specific line, create an inline +comment. Use the file's **current line number** (not diff position) with the +`line` and `side` parameters: + +````json +{ + "path": "crates/common/src/publisher.rs", + "line": 166, + "side": "RIGHT", + "body": "**P1 — Race condition**: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```" +} +```` + +#### Build the review body + +Include findings that cannot be pinpointed to a single line (cross-cutting +concerns, architectural issues, dependency problems) in the review body: + +```markdown +## Staff Engineer Review + +### Summary + +<1-2 sentence overview of the changes and overall assessment> + +### Findings + +#### P0 — Blockers + +- **Title**: description (file:line) + +#### P1 — High + +- **Title**: description (file:line) + +#### P2 — Medium + +- **Title**: description + +#### P3 — Low + +- **Title**: description + +### CI Status + +- fmt: PASS/FAIL +- clippy: PASS/FAIL +- rust tests: PASS/FAIL +- js tests: PASS/FAIL +``` + +#### Submit the review + +Use the GitHub API to submit. Handle these known issues: + +1. **"User can only have one pending review"**: Delete the existing pending + review first: + + ``` + # Find pending review + gh api repos/IABTechLab/trusted-server/pulls/<number>/reviews --jq '.[] | select(.state == "PENDING") | .id' + # Delete it + gh api repos/IABTechLab/trusted-server/pulls/<number>/reviews/<review_id> -X DELETE + ``` + +2. **"Position could not be resolved"**: Use `line` + `side: "RIGHT"` instead + of the `position` field. The `line` value is the line number in the file + (not the diff position). + +3. **Large reviews**: GitHub limits inline comments. If you have more than 30 + comments, consolidate lower-severity findings into the review body. + +Submit the review: + +``` +gh api repos/IABTechLab/trusted-server/pulls/<number>/reviews -X POST \ + -f event="<APPROVE|COMMENT|REQUEST_CHANGES>" \ + -f body="<review body>" \ + --input comments.json +``` + +Where `comments.json` contains the array of inline comment objects. + +### 8. Report + +Output: + +- The review URL +- Total findings by severity (e.g., "2 P0, 3 P1, 5 P2, 2 P3") +- Whether the review requested changes or approved +- Any CI failures encountered + +## Rules + +- Read every changed file completely before forming opinions. +- Be specific: include file paths, line numbers, and code snippets. +- Suggest fixes, not just problems. Show the corrected code when possible. +- Don't nitpick style that `cargo fmt` handles — focus on substance. +- Don't flag things that are correct but unfamiliar — verify before flagging. +- Cross-reference findings: if an issue appears in multiple places, group them. +- Do not include any byline, "Generated with" footer, or `Co-Authored-By` + trailer in review comments. +- If the diff is very large (>50 files), prioritize `crates/common/` changes + and new files over mechanical changes (Cargo.lock, generated code). +- Never submit a review without explicit user approval of the findings. diff --git a/CLAUDE.md b/CLAUDE.md index bdf7de81..e7671b7c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,12 +24,12 @@ Supporting files: `fastly.toml`, `trusted-server.toml`, `.env.dev`, ## Toolchain -| Tool | Version / Target | -|------|-----------------| -| Rust | 1.91.1 (pinned in `rust-toolchain.toml`) | -| WASM target | `wasm32-wasip1` | -| Node | LTS (for JS build) | -| Viceroy | Latest (Fastly local simulator) | +| Tool | Version / Target | +| ----------- | ---------------------------------------- | +| Rust | 1.91.1 (pinned in `rust-toolchain.toml`) | +| WASM target | `wasm32-wasip1` | +| Node | LTS (for JS build) | +| Viceroy | Latest (Fastly local simulator) | --- @@ -248,6 +248,7 @@ Bad: `"fix: added feature flags"` ## Integration System Integrations register in Rust via: + ```rust IntegrationRegistration::builder(ID) .with_proxy() @@ -272,12 +273,12 @@ IntegrationRegistration::builder(ID) ## Configuration Files -| File | Purpose | -|------|---------| -| `fastly.toml` | Fastly service configuration and build settings | +| File | Purpose | +| --------------------- | ---------------------------------------------------------- | +| `fastly.toml` | Fastly service configuration and build settings | | `trusted-server.toml` | Application settings (ad servers, KV stores, ID templates) | -| `rust-toolchain.toml` | Pins Rust version to 1.91.1 | -| `.env.dev` | Local development environment variables | +| `rust-toolchain.toml` | Pins Rust version to 1.91.1 | +| `.env.dev` | Local development environment variables | --- @@ -323,15 +324,16 @@ Every PR must pass: For complex tasks, use specialized subagents (`.claude/agents/`): -| Agent | When to use | -|-------|-------------| -| `build-validator` | Validate build across native + wasm32-wasip1 targets | -| `code-architect` | Analyze architecture, suggest improvements | -| `code-simplifier` | Find and simplify overly complex code | -| `verify-app` | Full verification pipeline (build + test + lint) | -| `pr-creator` | Create well-structured pull requests | -| `issue-creator` | Create GitHub issues with proper types via GraphQL | -| `repo-explorer` | Explore and answer questions about the codebase | +| Agent | When to use | +| ----------------- | -------------------------------------------------------------------- | +| `build-validator` | Validate build across native + wasm32-wasip1 targets | +| `code-architect` | Analyze architecture, suggest improvements | +| `code-simplifier` | Find and simplify overly complex code | +| `verify-app` | Full verification pipeline (build + test + lint) | +| `pr-creator` | Create well-structured pull requests | +| `pr-reviewer` | Staff-engineer PR review with inline GitHub comments (user approves) | +| `issue-creator` | Create GitHub issues with proper types via GraphQL | +| `repo-explorer` | Explore and answer questions about the codebase | ### Multi-Phase Workflow (for complex tasks) @@ -349,43 +351,44 @@ both runtime behavior and build/tooling changes. ### Selection Matrix -| Situation | Use first | Optional follow-up | Expected output | -|-----------|-----------|-------------------|-----------------| -| Unfamiliar code area | `repo-explorer` | `code-architect` | File map and risk hotspots | -| Multi-crate change | `repo-explorer` | `code-architect`, `build-validator` | Change plan and validation scope | -| CI/build failures | `build-validator` | `repo-explorer` | Failing combos and fault area | -| Design/API proposal | `code-architect` | `repo-explorer` | Architecture concerns and options | -| Cleanup/refactor | `code-simplifier` | `build-validator` | Simplification summary and checks | -| Pre-PR readiness | `build-validator` | `verify-app`, `pr-creator` | Pass/fail report and PR draft | +| Situation | Use first | Optional follow-up | Expected output | +| -------------------- | ----------------- | ----------------------------------- | ---------------------------------- | +| Unfamiliar code area | `repo-explorer` | `code-architect` | File map and risk hotspots | +| Multi-crate change | `repo-explorer` | `code-architect`, `build-validator` | Change plan and validation scope | +| CI/build failures | `build-validator` | `repo-explorer` | Failing combos and fault area | +| Design/API proposal | `code-architect` | `repo-explorer` | Architecture concerns and options | +| Cleanup/refactor | `code-simplifier` | `build-validator` | Simplification summary and checks | +| Pre-PR readiness | `build-validator` | `verify-app`, `pr-creator` | Pass/fail report and PR draft | +| PR review | `pr-reviewer` | `code-architect`, `repo-explorer` | Inline GitHub review with findings | --- ## Slash Commands -| Command | Purpose | -|---------|---------| -| `/test-all` | Run full test suite (Rust + JS) | -| `/check-ci` | Run all CI checks locally | -| `/verify` | Full verification: build, test, lint | +| Command | Purpose | +| ----------------- | ----------------------------------------- | +| `/test-all` | Run full test suite (Rust + JS) | +| `/check-ci` | Run all CI checks locally | +| `/verify` | Full verification: build, test, lint | | `/review-changes` | Review staged/unstaged changes for issues | -| `/test-crate` | Test a specific crate by name | +| `/test-crate` | Test a specific crate by name | --- ## Key Files -| File | Purpose | -|------|---------| -| `crates/common/src/integrations/registry.rs` | IntegrationRegistry, `js_module_ids()` | -| `crates/common/src/tsjs.rs` | Script tag generation with module IDs | -| `crates/common/src/html_processor.rs` | Injects `<script>` at `<head>` start | -| `crates/common/src/publisher.rs` | `/static/tsjs=` handler, concatenates modules | -| `crates/common/src/synthetic.rs` | Synthetic ID generation | -| `crates/common/src/cookies.rs` | Cookie handling | -| `crates/common/src/gdpr.rs` | GDPR consent management | -| `crates/common/src/http_wrapper.rs` | HTTP abstractions | -| `crates/js/build.rs` | Discovers dist files, generates `tsjs_modules.rs` | -| `crates/js/src/bundle.rs` | Module map, concatenation, hashing | +| File | Purpose | +| -------------------------------------------- | ------------------------------------------------- | +| `crates/common/src/integrations/registry.rs` | IntegrationRegistry, `js_module_ids()` | +| `crates/common/src/tsjs.rs` | Script tag generation with module IDs | +| `crates/common/src/html_processor.rs` | Injects `<script>` at `<head>` start | +| `crates/common/src/publisher.rs` | `/static/tsjs=` handler, concatenates modules | +| `crates/common/src/synthetic.rs` | Synthetic ID generation | +| `crates/common/src/cookies.rs` | Cookie handling | +| `crates/common/src/gdpr.rs` | GDPR consent management | +| `crates/common/src/http_wrapper.rs` | HTTP abstractions | +| `crates/js/build.rs` | Discovers dist files, generates `tsjs_modules.rs` | +| `crates/js/src/bundle.rs` | Module map, concatenation, hashing | --- From c4db3c2c1a57bf9a08c145cea51845c2553b5b41 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 18:23:07 -0800 Subject: [PATCH 07/11] Fixed logging --- .claude/agents/pr-reviewer.md | 4 ++-- CLAUDE.md | 19 ++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md index d4b5f4a5..c74b0963 100644 --- a/.claude/agents/pr-reviewer.md +++ b/.claude/agents/pr-reviewer.md @@ -75,8 +75,8 @@ For each changed file, evaluate: #### Convention compliance (from CLAUDE.md) - `expect("should ...")` instead of `unwrap()` in production code -- `thiserror` for library errors, `anyhow` for application-level -- `tracing` macros (not `println!`) +- `error-stack` (`Report<E>`) with `derive_more::Display` for errors (not thiserror/anyhow) +- `log` macros (not `println!`) - `vi.hoisted()` for mock definitions in JS tests - Integration IDs match JS directory names - Colocated tests with `#[cfg(test)]` diff --git a/CLAUDE.md b/CLAUDE.md index e7671b7c..fea31160 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,7 +156,7 @@ pub struct UserId(Uuid); ### Crate Preferences -- `tracing` (not `log`) for instrumentation. +- `log` (with `log-fastly`) for instrumentation. - `similar_asserts` for test assertions. - `insta` for snapshot tests. @@ -215,19 +215,12 @@ impl core::error::Error for MyError {} --- -## Tracing Practices +## Logging Practices -- Use level-specific macros: `info!`, `debug!`, `trace!`, `warn!`, `error!`. -- Provide structured data as named arguments. +- Use `log` crate level-specific macros: `log::info!`, `log::debug!`, `log::trace!`, `log::warn!`, `log::error!`. +- Provide context in log messages with format strings. - Format messages with present-tense verbs. -- Apply `#[tracing::instrument]` with `err`, `skip()`, and `fields()`: - -```rust -#[tracing::instrument(level = "debug", err, skip(password), fields(user_id = user.id))] -async fn authenticate_user(user: &User, password: &[u8]) -> Result<AuthToken, AuthError> { - // ... -} -``` +- Use `log-fastly` as the backend for Fastly Compute. --- @@ -395,7 +388,7 @@ both runtime behavior and build/tooling changes. ## What NOT to Do - Do not add unnecessary dependencies without justification. -- Do not use `println!` / `eprintln!` — use `tracing` macros. +- Do not use `println!` / `eprintln!` — use `log` macros. - Do not use `unwrap()` in production code — use `expect("should ...")`. - Do not use thiserror — use `derive_more::Display` + `impl Error`. - Do not use wildcard imports. From 0bea68194e685ad3fb85aa01fe64b7738e82c174 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 18:38:21 -0800 Subject: [PATCH 08/11] Remove aspirational conventions not practiced in the codebase Audit found 9/14 CLAUDE.md conventions were never used. Removed rules for #[expect], const fn, impl AsRef, Arc::clone, core>alloc>std, similar_asserts, insta, and test_ prefix ban. Aligned wildcard import rule with actual test module usage. --- CLAUDE.md | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fea31160..c2236e6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -87,7 +87,7 @@ cargo install viceroy # Fastly local test runtime - Use the **2024 edition** of Rust. - Prefer `derive_more` over manual trait implementations. -- Use `#[expect(lint, reason = "...")]` over `#[allow(lint)]`. +- Use `#[allow(lint)]` to suppress lints when necessary. - Use `rustfmt` to format code. - Invoke clippy with `--all-targets --all-features -- -D warnings`. - Use `cargo doc --no-deps --all-features` for checking documentation. @@ -106,12 +106,7 @@ pub struct UserId(Uuid); - Functions should **never** take more than 7 arguments — use a struct instead. - Take references for immutable access, mutable references for modification. Only take ownership when the function consumes the value. -- Make functions `const` whenever possible. -- Preferred argument types (when it doesn't reduce performance): - - `impl AsRef<str>` instead of `&str` or `&String` - - `impl AsRef<Path>` instead of `&Path` or `&PathBuf` - - `impl IntoIterator<Item = &T>` when only iterating - - `&[T]` instead of `&Vec<T>` +- Prefer `&[T]` instead of `&Vec<T>`. - Never use `impl Into<Option<_>>` — it hides that `None` can be passed. ### `From` / `Into` @@ -120,14 +115,6 @@ pub struct UserId(Uuid); - Prefer `Type::from(value)` over `value.into()` for clarity. - For wrapper types (`Cow`, `Arc`, `Rc`, `Box`), use explicit constructors. -### Smart Pointers - -- Always use `Arc::clone(&pointer)` instead of `pointer.clone()` to clearly indicate you're cloning the reference. - -### Async Patterns - -- Use `impl Future<Output = T> + Send` in trait definitions. - ### Allocations - Minimize allocations — reuse buffers, prefer borrowed data. @@ -135,18 +122,15 @@ pub struct UserId(Uuid); ### Naming Conventions -- Do **not** prefix test names with `test_` (avoids `test::test_name`). - Avoid abbreviations unless widely recognized (e.g., `Http`, `Json` — not `Ctx`). - Do not suffix names with their types (`users` not `usersList`). ### Import Style - No local imports within functions or blocks. -- No wildcard imports (`use super::*`, `use crate::module::*`). +- `use super::*` is acceptable in `#[cfg(test)]` modules only. - Never use a prelude `use crate::prelude::*`. -- Prefer `core` > `alloc` > `std` to minimize dependencies. - Use `use Trait as _` when you only need trait methods, not the trait name. -- Prefer qualified imports for frequently used types. ### Comments and Assertions @@ -157,8 +141,6 @@ pub struct UserId(Uuid); ### Crate Preferences - `log` (with `log-fastly`) for instrumentation. -- `similar_asserts` for test assertions. -- `insta` for snapshot tests. --- @@ -195,7 +177,6 @@ impl core::error::Error for MyError {} - Test both happy paths and error conditions. - Use `expect()` / `expect_err()` with `"should ..."` messages instead of `unwrap()`. - Use `json!` macro instead of raw JSON strings. -- Do not prefix test function names with `test_`. - Follow the same code quality standards in test code as production code. - For JS tests: use `vi.hoisted()` for mock definitions referenced in `vi.mock()` factories. @@ -391,7 +372,7 @@ both runtime behavior and build/tooling changes. - Do not use `println!` / `eprintln!` — use `log` macros. - Do not use `unwrap()` in production code — use `expect("should ...")`. - Do not use thiserror — use `derive_more::Display` + `impl Error`. -- Do not use wildcard imports. +- Do not use wildcard imports (except `use super::*` in test modules). - Do not commit `.env` files or secrets. - Do not make large refactors without approval. - Always run tests and linting before committing. From 8e6716b15391189afb5d76a54796bd8fbe50c841 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 18:44:48 -0800 Subject: [PATCH 09/11] Remove self-referential title from pr-reviewer template --- .claude/agents/pr-reviewer.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md index c74b0963..2ab7fa5a 100644 --- a/.claude/agents/pr-reviewer.md +++ b/.claude/agents/pr-reviewer.md @@ -170,31 +170,29 @@ Include findings that cannot be pinpointed to a single line (cross-cutting concerns, architectural issues, dependency problems) in the review body: ```markdown -## Staff Engineer Review - -### Summary +## Summary <1-2 sentence overview of the changes and overall assessment> -### Findings +## Findings -#### P0 — Blockers +### P0 — Blockers - **Title**: description (file:line) -#### P1 — High +### P1 — High - **Title**: description (file:line) -#### P2 — Medium +### P2 — Medium - **Title**: description -#### P3 — Low +### P3 — Low - **Title**: description -### CI Status +## CI Status - fmt: PASS/FAIL - clippy: PASS/FAIL @@ -251,8 +249,9 @@ Output: - Don't nitpick style that `cargo fmt` handles — focus on substance. - Don't flag things that are correct but unfamiliar — verify before flagging. - Cross-reference findings: if an issue appears in multiple places, group them. -- Do not include any byline, "Generated with" footer, or `Co-Authored-By` - trailer in review comments. +- Do not include any byline, "Generated with" footer, `Co-Authored-By` + trailer, or self-referential titles (e.g., "Staff Engineer Review") in + review comments or the review body. - If the diff is very large (>50 files), prioritize `crates/common/` changes and new files over mechanical changes (Cargo.lock, generated code). - Never submit a review without explicit user approval of the findings. From fa087c2eeb03234ccc8a34dd6618c9c6841a0e8d Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:55:54 -0800 Subject: [PATCH 10/11] Check GitHub CI status instead of running locally in pr-reviewer The pr-reviewer agent now checks `gh pr checks` for existing CI results before falling back to local runs, preventing false "Not run" reports when GitHub Actions have already completed. --- .claude/agents/pr-reviewer.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md index 2ab7fa5a..1b4216d7 100644 --- a/.claude/agents/pr-reviewer.md +++ b/.claude/agents/pr-reviewer.md @@ -42,18 +42,19 @@ git diff main...HEAD --name-only Read each file in its entirety. Do not skip files or skim — a thorough review requires understanding the full context of every change. -### 3. Run CI gates +### 3. Check CI status -Verify the branch is healthy before reviewing: +Check the PR's CI status from GitHub first — do not report "Not run" when +checks have already run: ``` -cargo fmt --all -- --check -cargo clippy --all-targets --all-features -- -D warnings -cargo test --workspace -cd crates/js/lib && npx vitest run +gh pr checks <number> --repo IABTechLab/trusted-server ``` -Note any CI failures in the review but continue with the code review regardless. +If the PR has passing CI checks, report them as PASS in the review. Only run +CI locally if checks haven't run yet or if you need to verify a specific +failure. Note any CI failures in the review but continue with the code review +regardless. ### 4. Deep analysis From 417726d7dabeb5d09d189a19ff567c208cf23519 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:58:37 -0800 Subject: [PATCH 11/11] Use code review emoji guide in pr-reviewer findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopt the emoji conventions from CONTRIBUTING.md: 🔧 for blockers/high (must change), 🤔 for medium (thinking), ⛏ for low (nitpick), 👍 for positive highlights. --- .claude/agents/pr-reviewer.md | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md index 1b4216d7..0ffc4b34 100644 --- a/.claude/agents/pr-reviewer.md +++ b/.claude/agents/pr-reviewer.md @@ -110,14 +110,18 @@ For each changed file, evaluate: ### 5. Classify findings -Assign each finding a severity: +Assign each finding a severity. Use emojis from the project's +[code review emoji guide](https://github.com/erikthedeveloper/code-review-emoji-guide) +(referenced in `CONTRIBUTING.md`): -| Severity | Criteria | -| ------------ | ------------------------------------------------------------------ | -| P0 — Blocker | Must fix before merge: bugs, data loss, security, CI failures | -| P1 — High | Should fix: race conditions, API design issues, missing validation | -| P2 — Medium | Recommended: inconsistencies, test gaps, dead code | -| P3 — Low | Nice to have: style, minor improvements, documentation | +| Severity | Emoji | Criteria | +| ------------ | ----- | ------------------------------------------------------------------ | +| P0 — Blocker | 🔧 | Must fix before merge: bugs, data loss, security, CI failures | +| P1 — High | 🔧 | Should fix: race conditions, API design issues, missing validation | +| P2 — Medium | 🤔 | Recommended: inconsistencies, test gaps, dead code | +| P3 — Low | ⛏ | Nice to have: style, minor improvements, documentation | + +Also use 👍 to highlight particularly good code or design decisions. ### 6. Present findings for user approval @@ -161,7 +165,7 @@ comment. Use the file's **current line number** (not diff position) with the "path": "crates/common/src/publisher.rs", "line": 166, "side": "RIGHT", - "body": "**P1 — Race condition**: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```" + "body": "🔧 **Race condition**: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```" } ```` @@ -177,19 +181,19 @@ concerns, architectural issues, dependency problems) in the review body: ## Findings -### P0 — Blockers +### 🔧 Blockers (P0) - **Title**: description (file:line) -### P1 — High +### 🔧 High (P1) - **Title**: description (file:line) -### P2 — Medium +### 🤔 Medium (P2) - **Title**: description -### P3 — Low +### ⛏ Low (P3) - **Title**: description