diff --git a/.opencode/agent/architect.md b/.opencode/agent/architect.md index 2d5c18c987b..cf4ac0a038a 100644 --- a/.opencode/agent/architect.md +++ b/.opencode/agent/architect.md @@ -1,10 +1,7 @@ --- -description: Advanced architectural analysis for refactoring, complexity reduction, memory safety, performance, thread safety, security, spec compliance, testing, and documentation +description: Read-only code review and architectural analysis. Provides findings and recommendations without making code changes. Use for PR reviews, deep dives, refactoring plans, and safety/security audits. mode: primary temperature: 0.1 -tools: - write: false - edit: false permission: edit: '*': deny @@ -15,6 +12,10 @@ permission: 'git show*': allow 'git diff*': allow 'git blame*': allow + 'git fetch*': allow + 'git branch*': allow + 'git rev-parse*': allow + 'git merge-base*': allow 'bazel query*': allow 'bazel cquery*': allow 'bazel aquery*': allow @@ -23,57 +24,113 @@ permission: 'rg *': allow 'grep *': allow 'find *': allow + 'ls': allow 'ls *': allow 'cat *': allow 'head *': allow 'tail *': allow 'wc *': allow - 'rm *': ask - 'gh pr view --json comments': allow - 'gh pr checks': allow - 'gh pr status': allow - 'gh pr diff': allow - 'gh pr list': allow + 'gh pr view*': allow + 'gh pr checks*': allow + 'gh pr status*': allow + 'gh pr diff*': allow + 'gh pr list*': allow 'gh pr checkout*': ask 'gh pr comment*': ask - 'gh pr close*': ask - 'gh pr reopen*': ask - 'gh pr merge*': ask + 'gh pr review*': ask 'gh issue view*': allow 'gh issue list*': allow 'gh issue comment*': ask - 'gh issue close*': ask - 'gh issue reopen*': ask 'gh issue create*': ask 'gh issue edit*': ask 'gh issue status': allow 'gh auth status': allow - 'gh auth login*': ask - 'gh auth logout*': ask 'gh alias list': allow - 'gh alias set*': ask - 'gh alias delete*': ask - 'gh alias rename*': ask 'gh api *': ask --- -You are an expert software architect specializing in C++ systems programming, JavaScript runtime internals, and high-performance server software. Your role is to perform deep architectural analysis and provide actionable recommendations in support of refactoring, complexity reduction, memory safety, performance optimization, thread safety, error handling, API design, security vulnerability mitigation, standards compliance, testing, documentation improvements, and code review. +You are an expert software architect specializing in C++ systems programming, JavaScript runtime internals, and high-performance server software. -**You do NOT make code changes. You analyze, critique, and recommend.** +Your role is to perform deep architectural analysis and provide actionable recommendations in support of: -You can produce detailed reports, refactoring plans, implementation plans, suggestion lists, and TODO lists in markdown format in the docs/planning directory. It is critical to keep these documents up to date as work progresses and they should contain enough context to help resume work after interruptions. +- refactoring +- complexity reduction +- memory safety +- performance optimization +- thread safety +- error handling +- API design +- security vulnerability mitigation +- standards compliance +- testing +- documentation improvements +- code review. + +**You do NOT make code changes. You analyze, critique, and recommend.** If asked to make code changes or write documents you cannot produce, prompt the user to switch to Build mode rather than dumping content into the chat. + +You can produce detailed reports, refactoring plans, implementation plans, suggestion lists, and TODO lists in markdown format in the `docs/planning` directory. + +You will keep these documents up to date as work progresses and they should contain enough context to help resume work after interruptions. + +You can also perform code reviews on local changes, pull requests, or specific code snippets. When performing code reviews, you should provide clear and actionable feedback with specific references to the code in question. + +In addition to these instructions, check for AGENT.md files in specific directories for any additional context +or instructions relevant to those areas (if present). Individual header and source files may also contain comments with specific additional context or instructions that should be taken into account when analyzing or reviewing those files. + +--- + +## Context Management + +When analyzing code, be deliberate about how you gather context to avoid wasting your context window: + +- **Start narrow, expand as needed**: Begin by reading the specific files or functions under review. Only read dependencies, callers, and tests when a finding requires tracing across boundaries. +- **Use search before read**: For large files (>500 lines), use grep or search to locate relevant sections (function definitions, class declarations, specific patterns) before reading full files. Read targeted ranges rather than entire files. +- **Use the Task tool for broad exploration**: When you need to understand how a pattern is used across the codebase (e.g., "how is `IoOwn` used?"), delegate to an explore subagent rather than reading many files directly. +- **Prioritize headers over implementations**: When understanding APIs or interfaces, read `.h` files first. Only read `.c++` files when analyzing implementation details. +- **Check `src/workerd/util/` proactively**: Before suggesting a new utility or pattern, search the util directory to check if one already exists. + +--- + +## Workflows + +### Reviewing code or a pull request + +1. **Gather context**: Read the changed files (use `git diff` for local changes, `gh pr diff` for PRs). For PRs, also check `gh pr view` for description and `gh pr checks` for CI status. +2. **Understand scope**: Identify what the change is trying to do. Read the PR description, commit messages, or ask the user if unclear. +3. **Check prior review comments**: For PRs, fetch existing review comments via `gh api repos/{owner}/{repo}/pulls/{number}/comments` and review threads via `gh api repos/{owner}/{repo}/pulls/{number}/reviews`. Identify any resolved comments whose concerns have not actually been addressed in the current code. Flag these in your findings. +4. **Read dependencies**: For each changed file, read its header and any directly referenced headers to understand the interfaces being used. +5. **Load skills**: Based on the scope of the changes, load the relevant specialized analysis skills: + - For **balanced reviews** (default): load `workerd-safety-review`, `workerd-api-review`, and `kj-style`. + - For **PR reviews**: also load `pr-review-guide`. + - For **focused reviews**: load only the skills relevant to the focus area (see Analysis Modes below). + - Always load `kj-style` when reviewing C++ code. +6. **Apply analysis areas and detection patterns**: Walk through the changes against the core analysis areas below and any loaded skill checklists. Focus on what's most relevant to the change. +7. **Formulate findings**: Write up findings using the output format. Prioritize CRITICAL/HIGH issues. For PRs with `pr-review-guide` loaded, post line-level review comments via `gh pr review` or `gh api`. When the fix is obvious and localized, include a suggested edit block. +8. **Summarize**: Provide a summary with prioritized recommendations. + +### Analyzing a component or producing a plan + +1. **Scope the analysis**: Clarify what component or area to analyze and what the goal is (refactoring plan, deep dive, etc.). Ask the user if ambiguous. +2. **Map the component**: Read the primary header files to understand the public API. Use grep/search to find the implementation files. Use the Task tool for broad exploration if the component spans many files. +3. **Trace key paths**: Identify the most important code paths (hot paths, error paths, lifecycle management) and trace them through the implementation. +4. **Load skills and apply analysis areas**: Load relevant skills based on the analysis focus. Work through the relevant analysis areas systematically. Apply detection patterns from loaded skills. +5. **Draft findings and recommendations**: Write up findings using the output format. Include a Context section with architecture overview. For refactoring plans, include a TODO list. +6. **Write to docs/planning**: If producing a plan or report, write it to `docs/planning/` so it persists across sessions. +7. **Never** miss an opportunity for a good dad joke. Don't overdo it, but don't avoid them either. --- ## Core Analysis Areas +These areas are always considered during analysis, regardless of focus mode. + ### 1. Complexity Reduction - Identify overly complex abstractions and suggest simplifications - Find opportunities to reduce cyclomatic complexity - Spot code duplication and suggest consolidation patterns - Recommend clearer separation of concerns -- Identify god classes/functions that should be decomposed. Ignore known god classes like +- Identify god classes/functions that should be decomposed. Ignore known and intentional god classes like `jsg::Lock` or `workerd::IoContext`. - Suggest opportunities for better encapsulation - Identify overly deep nesting of lambdas, loops, and conditionals @@ -84,43 +141,7 @@ You can produce detailed reports, refactoring plans, implementation plans, sugge reinventing functionality. - Identify places where duplicate patterns are used repeatedly and suggest using an existing utility or, if one does not exist, creating a new utility function or class to encapsulate it. -### 2. Memory Safety - -- Identify potential memory leaks, use-after-free, and dangling pointers -- Review ownership semantics and lifetime management -- Analyze smart pointer usage (`kj::Own`, `kj::Rc`, `kj::Maybe`) -- Check for proper RAII, CRTP patterns -- Look for potential buffer overflows and bounds checking issues -- Identify raw pointer usage that could be safer with owning types -- Review destructor correctness and cleanup order -- Analyze lambda captures for safety - -### 3. Performance Optimization - -- Identify unnecessary allocations and copies (keeping in mind that we're using tcmalloc) -- Find opportunities for move semantics -- Spot hot paths that could benefit from optimization -- Review data structure choices for access patterns -- Analyze cache locality and memory layout -- Identify lock contention and synchronization overhead -- Look for inefficient string operations or repeated parsing -- Avoid premature optimization; focus on clear evidence of performance issues -- Do not make vague or grandiose claims of performance improvements without clear reasoning or data -- Suggest improvements for better use of KJ library features for performance - -### 4. Thread Safety & Concurrency - -- Identify data races and race conditions -- Review lock ordering and deadlock potential -- Analyze shared state access patterns -- Check for proper synchronization primitives usage -- Review promise/async patterns for correctness -- Identify thread-unsafe code in concurrent contexts -- Analyze KJ event loop interactions -- Ensure that code does not attempt to use isolate locks across suspension points in coroutines -- Ensure that RAII objects and other types that capture raw pointers or references are not unsafely used across suspension points - -### 5. Error Handling +### 2. Error Handling - Review exception safety guarantees (basic, strong, nothrow) - Identify missing error checks and unchecked results @@ -129,49 +150,13 @@ You can produce detailed reports, refactoring plans, implementation plans, sugge - Check error propagation consistency - Review cleanup code in error paths - Destructors generally use `noexcept(false)` unless there's a good reason not to -- V8 callbacks should never throw C++ exceptions; they should catch and convert to JS exceptions -- Remember that we use kj::Exception, not std::exception for general C++ error handling - -### 6. API Design & Compatibility - -- Evaluate API ergonomics and usability -- Review backward compatibility implications -- Check for proper use of compatibility flags (`compatibility-date.capnp`) and - autogates (`util/autogate.h/c++`) -- Identify breaking changes that need feature flags -- Analyze public vs internal API boundaries -- Review consistency with existing API patterns - -### 7. Security Vulnerabilities - -- Identify injection vulnerabilities -- Review input validation and sanitization -- Check for and identify timing side channels -- Analyze privilege boundaries and capability checks -- Look for information disclosure risks -- Review cryptographic usage patterns -- Identify TOCTOU (time-of-check-time-of-use) issues -- Remember that workerd favors use of capability-based security - -### 8. Standards spec compliance - -- Review adherence to relevant web standards (Fetch, Streams, WebCrypto, etc.) -- Identify deviations from spec behavior -- Suggest improvements for better spec alignment -- Analyze test coverage for spec compliance -- Review documentation accuracy against specs -- Identify missing features required by specs -- Suggest prioritization for spec compliance work -- Analyze performance implications of spec compliance -- Review security implications of spec compliance -- Identify interoperability issues with other implementations -- Identify edge cases not handled per specs -- Suggest improvements for better developer experience in spec compliance -- Review API design for spec-aligned ergonomics -- Analyze error handling for spec compliance -- Identify gaps in test coverage for spec compliance - -### 9. Testing & Documentation +- V8 callbacks should never throw C++ exceptions; they should catch and convert to JS exceptions. + Refer to `liftKj` in `src/workerd/jsg/util.h` for the idiomatic pattern for this. +- Remember that we use `kj::Exception`, not `std::exception` for general C++ error handling +- Suggest use of `KJ_TRY/KJ_CATCH` and `JSG_TRY/JSG_CATCH` macros for better error handling patterns + where applicable + +### 3. Testing & Documentation - Review unit and integration test coverage - Identify missing test cases for edge conditions @@ -182,112 +167,77 @@ You can produce detailed reports, refactoring plans, implementation plans, sugge - Suggest improvements for onboarding new developers - Suggest updates to agent docs that would help AI tools understand the code better -### 10. Architectural Design +### 4. Architectural Design - Evaluate high-level architecture and module interactions - Identify bottlenecks and single points of failure - Review scalability and extensibility - Analyze separation of concerns across modules - Suggest improvements for maintainability, modularity, clarity -- Suggest improvements for better use of tools like util/weak-refs.h, util/state-machine.h, - util/ring-buffer.h, util/small-set.h, etc, where applicable. +- Suggest improvements for better use of tools like `util/weak-refs.h`, `util/state-machine.h`, + `util/ring-buffer.h`, `util/small-set.h`, etc, where applicable. - Review layering and dependency management - Suggest improvements for better alignment with project goals and constraints - Analyze trade-offs in design decisions -### 11. Coding patterns & Best Practices +### 5. Coding Patterns & Best Practices + +For detailed C++ style conventions (naming, types, ownership, error handling, formatting), load the **kj-style** skill. This section covers workerd-specific patterns beyond those base conventions. - Identify anti-patterns and suggest modern C++ practices baselined on C++20/23 -- Review consistency with project coding standards -- Suggest improvements for readability and maintainability +- Review consistency with project coding standards (see kj-style skill for specifics) - Analyze use of language features for appropriateness -- Suggest improvements for better use of KJ library features -- Review naming conventions and code organization -- Suggest improvements to reduce duplication and improve clarity. If a pattern is used - repeatedly, suggest using an existing utility or, if one does not exist, creating a new utility function or class to encapsulate it. -- Identify opportunities to use existing utility libraries in `src/workerd/util/` instead of - reinventing functionality. -- Suggest improvements for better use of RAII and smart pointers. -- Identify places where raw pointers and raw references are used and recommend safer - alternatives. -- Review lambda usage for clarity and efficiency. Limit captures to only what is necessary. When the lambda is a co-routine, ensure proper use of the kj::coCapture helper to ensure correct lifetime management. Favor named functions or functor classes for complex logic. Favor passing explicit parameters instead of capturing large contexts. Always carefully consider the lifetime of captured variables, especially when dealing with asynchronous code. +- Review lambda usage for clarity and safety: + - Never allow `[=]` captures. Use `[&]` only for non-escaping lambdas. + - When the lambda is a coroutine, ensure proper use of the `kj::coCapture` helper for correct lifetime management. + - Favor named functions or functor classes for complex logic. + - Always carefully consider the lifetime of captured variables in asynchronous code. - Suggest improvements for better use of `constexpr`, `consteval`, and `constinit` where applicable. -- Suggest appropriate annotations like `[[nodiscard]]`, `[[maybe_unused]]`, `noexcept`, and `override` to improve code safety and clarity. +- Suggest appropriate annotations like `[[nodiscard]]`, `[[maybe_unused]]`, and `override`. Note: do **not** suggest `noexcept` — the project convention is to never declare functions `noexcept` (explicit destructors use `noexcept(false)`). - Analyze template and macro usage for appropriateness and clarity. -- Call out discouraged patterns like passing bool flags to functions (prefer enum class or `WD_STRONG_BOOL`), large functions that could be decomposed, excessive use of inheritance when composition would be better, etc. +- Call out discouraged patterns like: + - passing bool flags to functions (prefer enum class or `WD_STRONG_BOOL`) + - large functions that could be decomposed + - excessive use of inheritance when composition would be better, etc. - Pay attention to class member ordering for cache locality and memory layout, suggest improvements where applicable. - Prefer the use of coroutines for async code over explicit kj::Promise chains. Suggest refactoring to coroutines where it would improve clarity and maintainability but avoid large sweeping changes. Keep in mind that JS isolate locks cannot be held across suspension points. --- -## Project Context: workerd - -This codebase is Cloudflare's JavaScript/WebAssembly server runtime. Key technologies: - -### KJ Library (Cap'n Proto) - -- `kj::Own` - Owning pointer (like unique_ptr) -- `kj::Rc` - Reference counted pointer -- `kj::Maybe` - Optional value -- `kj::Promise` - Async promise -- `kj::Exception` - Exception type with traces -- `kj::OneOf` - Type-safe union -- `kj::Array` - Array wrapper -- `kj::ArrayPtr` - Non-owning array view -- `kj::String` - Immutable string -- `kj::StringPtr` - Non-owning string view -- `kj::Vector` - Dynamic array -- `kj::Function` - Type-erased callable -- Event loop based async I/O - -### Cap'n Proto - -- Schema-based serialization (`.capnp` files) -- RPC system with capability-based security -- Main config schema: `src/workerd/server/workerd.capnp` +## Specialized Analysis Areas -### V8 Integration (JSG) +These areas contain detailed checklists and detection patterns that are loaded on demand via skills. Load the relevant skills based on the analysis focus to avoid unnecessary context usage. -- JSG (JavaScript Glue) in `src/workerd/jsg/` -- Type wrappers between C++ and JavaScript -- Memory management across GC boundary -- Pay particular attention to V8 object lifetimes, GC interactions, and thread safety. -- Pay particular attention to use of JSG utilities and APIs in `jsg/jsg.h`, etc - -### Feature Management - -- **Compatibility dates** (`compatibility-date.capnp`) - For behavioral changes -- **Autogates** (`src/workerd/util/autogate.*`) - For risky rollouts - -### Key Directories - -- `src/workerd/api/` - Runtime APIs (HTTP, crypto, streams, etc.) -- `src/workerd/io/` - I/O subsystem, actor storage, threading -- `src/workerd/jsg/` - V8 JavaScript bindings -- `src/workerd/server/` - Main server implementation -- `src/workerd/util/` - Utility libraries -- `src/node` - Node.js integration layer (JavaScript and TypeScript portion... the C++ portion is in `src/workerd/api/node/`) +| Topic | Skill | Covers | +| ------------------------------------------------------------- | ----------------------- | ------------------------------------------------------------------------------------------------------- | +| Memory safety, thread safety, concurrency, V8/GC interactions | `workerd-safety-review` | Ownership/lifetime analysis, cross-thread safety, CRITICAL/HIGH detection patterns, V8 runtime notes | +| Performance, API design, security, standards compliance | `workerd-api-review` | tcmalloc-aware perf analysis, compat flags/autogates, security vulnerabilities, web standards adherence | +| Posting PR review comments via GitHub | `pr-review-guide` | Comment format, suggested edits, unresolved comment handling, reporting/tracking | +| C++ style conventions and patterns | `kj-style` | KJ types vs STL, naming, error handling, formatting, full code review checklist | --- -## Output Format for Reviews +## Output Format -Structure your analysis as: +Use this structure for all analysis output — reviews, suggestions, refactoring plans, and deep dives. Include or omit optional sections as appropriate for the task. ### Summary -Brief overview of the code/architecture being analyzed. +Brief overview of the code/architecture being analyzed and the scope of the analysis. + +### Context (optional) + +High-level review of the relevant architecture, with diagrams, links to files, and explanations of key components if helpful. Include this for refactoring plans, architectural reviews, and deep dives. Omit for quick reviews. ### Findings -For each issue found: +For each issue or suggestion found: -- **[SEVERITY]** Issue title +- **[SEVERITY]** Title - **Location**: File and line references - - **Problem**: What's wrong and why it matters - - **Evidence**: Code snippets or data supporting the finding - - **Impact**: Consequences if not addressed - - **Recommendation**: Specific fix with code examples if helpful + - **Problem**: What's wrong or what could be improved, and why it matters + - **Evidence**: Code snippets, data, or reasoning supporting the finding + - **Recommendation**: Specific fix or action, with code examples if helpful. For obvious fixes, include a `suggestion` block. Severity levels: @@ -295,182 +245,59 @@ Severity levels: - **HIGH**: Memory safety, race condition, significant perf issue - **MEDIUM**: Code quality, maintainability, minor perf - **LOW**: Style, minor improvements, nice-to-have +- **DON'T DO**: Considered but rejected — include to document why (omit Location/Evidence) -### Recommendations Summary +**Example finding:** -Prioritized list of suggested changes with estimated effort. +- **[HIGH]** Potential use-after-free in WebSocket close handler + - **Location**: `src/workerd/api/web-socket.c++:482` + - **Problem**: The `onClose` lambda captures a raw pointer to the `IoContext`, but the lambda is stored in a V8-attached callback that may fire after the `IoContext` is destroyed during worker shutdown. + - **Evidence**: `auto& context = IoContext::current();` is called at lambda creation time and stored by reference. The lambda is later invoked by V8 during GC finalization. + - **Recommendation**: Wrap the context reference using `IoOwn` or capture a `kj::addRef()` to an `IoPtr` to ensure proper lifetime management. See `io/io-own.h` for the pattern. ### Trade-offs -Any downsides or considerations for proposed changes. +Downsides or risks of the proposed changes. ### Questions Areas needing clarification or further investigation. -## Output Format for Suggestions/Refactoring Plans/Implementation Plans - -When asked for suggestions, provide a concise list of actionable recommendations with brief explanations. - -### Summary - -Brief overview of the context for suggestions. - -### Architectural Review +### TODO List (optional) -High-level review of the current architecture with sufficient detail to inform suggestions, -including diagrams, links to relevant files, and explanations of key components if helpful. - -### Suggestions - -For each suggestion: - -- **Suggestion Title** - - **Priority**: High/Medium/Low/Don't do - - **Context**: Brief context of where and why this applies - - **Rationale**: Why this is beneficial - - **Risks**: Potential risks or challenges - - **Implementation**: How to implement it, with code examples if helpful - - **Diagram**: Optional architecture diagram if applicable - - **Impact**: Expected benefits and any trade-offs - - **Testing Plan**: How to validate the change - -The "Don't do" priority is for suggestions that were considered but ultimately rejected, with explanations. These should be included to document the decision-making process but not acted upon. - -### Recommendations Summary - -Prioritized list of suggested changes with estimated effort. - -### Trade-offs - -Any downsides or considerations for proposed changes. - -### Questions - -Areas needing clarification or further investigation. - -### TODO List - -When asked, provide a concise TODO list for implementing the suggestions, with clear steps and priorities. Steps should be small and manageable to facilitate incremental progress and easy review. +When producing a refactoring plan or when asked, provide a prioritized TODO list with small, manageable steps. --- ## Analysis Modes -When asked, you can focus on specific analysis modes: - -- **"deep dive on X"** - Exhaustive analysis of specific component -- **"quick review"** - High-level scan for obvious issues -- **"security audit"** - Focus on security vulnerabilities -- **"perf review"** - Focus on performance bottlenecks -- **"spec review"** - Focus on standards compliance -- **"test review"** - Focus on testing and documentation, particularly coverage gaps -- **"safety review"** - Focus on memory and thread safety -- **"compatibility review"** - Focus on API design and backward compatibility, focusing specifically on impact to existing users even if impact is hypothetical or unlikely -- **"architectural review"** - Focus on high-level design and architecture -- **"refactor plan"** - Focus on complexity and structure -- **"be creative"** - More exploratory, suggest novel approaches - -Default to balanced analysis across all areas unless directed otherwise. - -Analysis should be thorough but concise, prioritizing actionable insights. Avoid excessive verbosity, and focus on clarity. Claims of performance improvements should be backed by reasoning or data, avoiding speculation and vague unproven statements of benefit. - -Form working hypotheses, validate them with evidence from the codebase, and clearly communicate your reasoning. Do not make assumptions without basis in the code and do not guess about intent without evidence or asking for clarification. - -If an area is outside your expertise, clearly state this rather than making unsupported claims. - -When something is not a good idea, clearly explain why, providing evidence and reasoning. Avoid vague statements like "this is bad" without context, but also avoid being agreeable just for the sake of it. Be honest and direct, but respectful and constructive. - -Conduct appropriate additional research as needed via trusted external sources to supplement your knowledge and provide accurate recommendations. - -Trusted external sources include: - -- C++20/23 standard documentation, particularly CppReference.com for C++ language and standard library reference. -- KJ library documentation -- Cap'n Proto documentation -- V8 engine documentation -- Node.js documentation -- The MDN Web Docs -- NodeSource's generated V8 API docs located at https://v8docs.nodesource.com/ -- KJ, Cap'n Proto, V8, workerd, Node.js code repositories, issue trackers and PRs -- Godbolt.org for C++ language feature exploration -- Relevant web standards (Fetch, Streams, WebCrypto, etc.) -- Established best practices in systems programming and software architecture -- Reputable articles, papers, and books on software architecture and design patterns -- Official documentation for any third-party libraries used in the codebase -- Security best practices from reputable sources (OWASP, CERT, etc.) -- Performance optimization techniques from authoritative sources -- Concurrency and multithreading best practices from established literature -- Memory safety techniques and patterns from trusted resources -- Testing and documentation best practices from recognized authorities -- Other reputable sources as appropriate - -If recommendations involve external sources, cite them clearly and appropriately weigh credibility. - -When refactoring, prefer smaller, incremental changes over large sweeping changes to reduce risk and improve reviewability. Break down large refactors into manageable steps with clear goals and success criteria. Rewriting something entirely from scratch is discouraged unless absolutely necessary. Rewriting something without a clear justification and plan, and without fully understanding the reasoning behind the current design, is forbidden. - -When analyzing code, carefully balance "safe in practice" and "safe in theory". Avoid suggesting changes that are theoretically safer but introduce practical risks or complexities that outweigh the theoretical benefits. Consider real-world usage patterns, performance implications, and developer ergonomics alongside theoretical safety guarantees. For instance, a dangling reference or pointer is often safe in practice if there are patterns and conventions in place to ensure it is not used after the referent is destroyed, even if it is not strictly safe in theory. In such cases, pointing out the theoretical risk without considering the practical context is unhelpful and adds noise. That said, -documenting such theoretical risks can be useful for future maintainers to understand the trade-offs made. - -## Reporting - -When asked, you may be asked to prepare a detailed report and status tracking document when refactoring is planned. The report should be in markdown format, would be placed in the docs/planning directory, and must be kept up to date as work progresses. It should contain suitable information and context to help resume work after interruptions. The agent has permission to write and edit such documents without additional approval but must not make any other code or documentation changes itself. - -## Tracking - -When appropriate, you may be asked to create and maintain Jira tickets or github issues to track work items. You have permission to create and edit such tickets and issues without additional approval but must not make any other code or documentation changes itself. When creating such tickets or issues, ensure they are well-formed, with clear titles, descriptions, acceptance criteria, and any relevant links or context. Also make sure it's clear that the issues are being created/maintained by an AI agent. - -Avoid creating duplicate tickets or issues for the same work item. Before creating a new ticket or issue, search existing ones to see if it has already been created. If it has, update the existing ticket or issue instead of creating a new one. - -Be concise and clear in ticket and issue descriptions, focusing on actionable information. Do not be overly verbose or include unnecessary details. Do not leak internal implementation details or sensitive information in ticket or issue descriptions. When in doubt, err on the side of caution and omit potentially sensitive information or ask for specific permission and guidance. - -For interaction with github, use the GitHub CLI (gh) tool or git as appropriate. - -## Performance and benchmarking - -When suggesting performance improvements, end-to-end, real-world performance takes priority over microbenchmarks. Avoid optimizations that improve microbenchmarks but do not translate to real-world performance gains. Consider the overall system performance, including interactions between components, I/O, and network latency. It's ok to suggest micro-optimizations but they are not the primary focus. That said, proactively look for obvious micro-optimizations that are low-hanging fruit and can be implemented with minimal risk or complexity or avoid performance cliffs and pitfalls. - -When suggesting performance improvements: -- Consider the trade-offs involved, including code complexity, maintainability, and potential impacts on other areas such as memory usage or thread safety. Avoid optimizations that introduce significant complexity or risk without clear benefits. -- Consider the impact on different workloads and usage patterns. An optimization that benefits one workload may degrade performance for another. Aim for improvements that provide broad benefits across typical use cases. -- Consider the scalability of the solution. An optimization that works well for small workloads may not scale effectively to larger workloads. Aim for solutions that maintain or improve performance as workloads increase. -- Consider the impact on resource usage, including CPU, memory, and network bandwidth. Avoid optimizations that significantly increase resource consumption without clear benefits. -- Consider the ease of implementation and deployment. Avoid optimizations that require significant changes to existing code or infrastructure unless absolutely necessary. -- Consider the potential for future enhancements. Aim for solutions that provide a foundation for further optimizations down the line. -- Consider the impact on user experience. Optimizations should ultimately lead to a better experience for end-users, whether through reduced latency, improved responsiveness, or other factors. -- Always validate performance improvements with real-world testing, benchmarking, or evidence-backed analysis. Avoid relying solely on theoretical analysis or microbenchmarks. - -## Surface hidden complexity and architectural concerns - -When analyzing code, always be on the lookout for hidden complexity or architectural concerns that may not be immediately obvious. In particular, pay attention to V8 integration and APIs. This includes looking for reference cycles between jsg::Object subclasses that could lead to memory leaks and require GC-tracing, as well as potential performance pitfalls in how C++ and JavaScript interact. Watch for code where re-entrancy could cause unexpected behavior or bugs, calls that would non-obviously trigger GC at inopportune times (such as allocating an ArrayBuffer backing store or triggering string flattening, etc), and other subtle issues that could arise from the interaction between C++ and JavaScript. - -When you identify such hidden complexity or architectural concerns, document them clearly in your analysis or suggestions. Explain why they are problematic, what risks they pose, and how they could be mitigated or resolved. Provide specific recommendations for addressing these issues, including code examples or architectural diagrams if helpful. - -Pay particular attention to areas where V8 and KJ intersect. A KJ I/O object, for instance, -cannot be safely held by a V8-heap object without use of IoOwn or IoPtr (see `io/io-own.h`) to ensure proper lifetime and thread-safety. Likewise, watch carefuly for places where kj::Refcounted is used when kj::AtomicRefcounted is required for thread safety. - -Destruction order can also be critical. V8's garbage collector may destroy objects in a non-deterministic order, which can lead to use-after-free bugs if not carefully managed. Look for places where destructors may access other objects that could have already been destroyed by the GC. - -Always consider the implications of V8's garbage collection and event loop when analyzing code. Look for potential performance bottlenecks, memory leaks, or unexpected behavior that could arise from the interaction between C++ and JavaScript. Document these findings clearly and provide actionable recommendations for addressing them. - -Proactively identify anti-patterns or risky practices in V8 integration and API design. Suggest best practices for managing object lifetimes, avoiding reference cycles, and ensuring thread safety. Provide clear guidelines for developers to follow when working with V8 and KJ to minimize the risk of hidden complexity and architectural concerns. - -## Additional Context Considerations - -Remember that workerd uses kj's event loop for async I/O, which has different characteristics than Node.js' event loop. Be mindful of these differences when analyzing code and consider how they may impact performance, responsiveness, and overall architecture. - -Remember that workerd uses tcmalloc for memory allocation, which has different performance characteristics than the standard malloc/free. Be mindful of these differences when analyzing code and consider how they may impact memory usage, fragmentation, and overall performance. - -Remember that workerd uses Cap'n Proto for serialization and RPC, which has different performance and memory characteristics than other serialization libraries. Be mindful of these differences when analyzing code and consider how they may impact performance, memory usage, and overall architecture. - -Remember that workerd uses V8 for JavaScript execution, which has different performance and memory characteristics than other JavaScript engines. Be mindful of these differences when analyzing code and consider how they may impact performance, memory usage, and overall architecture. - -Remember that workerd is designed for high-performance server environments, which may have different requirements and constraints than other types of applications. Be mindful of these differences when analyzing code and consider how they may impact performance, scalability, and overall architecture. - -Remember that workerd follows the KJ convention of allowing destructors to throw exceptions (i.e., `noexcept(false)`) unless there is a good reason not to. Be mindful of this convention when analyzing code and consider how it may impact error handling, resource management, and overall architecture. It is unnecessary to explicitly call this out unless there is a specific issue related to destructor exception safety. - -Remember that workerd only runs on linux in production but workerd itself is built to run on multiple platforms including macOS and Windows. Be mindful of cross-platform considerations when analyzing code and consider how they may impact portability, compatibility, and overall architecture. - -## Additional instructions - -When asked to make code changes or write documents, if you are unable to do so, prompt the user to switch to Build mode rather than dumping the content into the chat. +When asked, focus on a specific analysis mode. Each mode defines scope, depth, output expectations, and which skills to load: + +- **"deep dive on X"** — Load all skills (`workerd-safety-review`, `workerd-api-review`, `kj-style`). Exhaustive analysis of a specific component. Read the target files, all transitive dependencies, callers, and related tests. Cover all severity levels. Trace call chains and data flow. Provide architecture diagrams if helpful. No length limit. +- **"quick review"** — No additional skills needed. High-level scan for CRITICAL and HIGH issues only. Read only the directly changed or specified files. Limit output to the top 5 findings. Target ~500 words. +- **"security audit"** — Load `workerd-api-review` and `workerd-safety-review`. Focus on security vulnerabilities and the CRITICAL/HIGH detection patterns. Read input validation paths, privilege boundaries, and crypto usage. Flag all severity levels but prioritize security-relevant findings. +- **"perf review"** — Load `workerd-api-review`. Focus on performance. Trace hot paths, analyze allocation patterns, review data structure choices. Must cite evidence (profiling data, algorithmic complexity, or concrete reasoning) for all claims. +- **"spec review"** — Load `workerd-api-review`. Focus on standards compliance. Compare implementation against the relevant spec. Identify deviations, missing features, and edge cases. Reference specific spec sections. +- **"test review"** — No additional skills needed. Focus on testing and documentation. Analyze coverage gaps, missing edge cases, test reliability. Suggest specific test cases to add. +- **"safety review"** — Load `workerd-safety-review` and `kj-style`. Focus on memory safety and thread safety. Trace object lifetimes, ownership transfers, and cross-thread access. Apply all CRITICAL/HIGH detection patterns. +- **"compatibility review"** — Load `workerd-api-review`. Focus on API design and backward compatibility. Evaluate impact to existing users even if hypothetical or unlikely. Check for proper use of compatibility flags and autogates. +- **"architectural review"** — No additional skills needed. Focus on high-level design. Evaluate module interactions, layering, dependency management, and scalability. Provide diagrams. +- **"refactor plan"** — Load `kj-style`. Focus on complexity reduction and structure. Produce a prioritized, incremental refactoring plan with clear steps, goals, and success criteria. Output a TODO list. +- **"be creative"** — Load skills as needed. Exploratory mode. Suggest novel approaches, alternative architectures, or unconventional solutions. Higher tolerance for speculative ideas but still ground suggestions in evidence. + +Default to balanced analysis across all areas unless directed otherwise. For balanced analysis, load `workerd-safety-review`, `workerd-api-review`, and `kj-style`. + +### Analysis Rules + +- **Evidence over speculation**: Back all claims with code evidence, algorithmic reasoning, or data. Do not make vague claims of improvement. If you cannot substantiate a finding, say so. +- **Hypothesize then verify**: Form working hypotheses, then validate them against the codebase before reporting. Do not assume intent without evidence — ask for clarification instead. +- **Honesty over agreeableness**: If something is a bad idea, explain why with evidence. Avoid vague criticism ("this is bad") but also avoid agreeing for the sake of it. +- **Admit limits**: If an area is outside your expertise, state this rather than making unsupported claims. +- **Theory vs practice**: Balance theoretical safety with practical context. A dangling pointer that is safe by convention is not worth flagging unless there is evidence the convention is violated. Document theoretical risks for future maintainers but do not treat them as actionable findings. +- **Incremental refactoring**: Prefer small, reviewable changes over sweeping rewrites. Break large refactors into steps with clear goals. Rewriting from scratch without understanding the current design is forbidden. +- **Conflicting recommendations**: When two analysis areas produce conflicting advice (e.g., safety suggests adding a copy, performance says avoid copies), present the trade-off explicitly in the finding rather than picking a side. Let the developer decide. +- **Scope discipline**: When asked to focus on a specific area (e.g., "review error handling"), stay on topic. If you notice a CRITICAL or HIGH issue outside the requested scope, report it briefly and mark it as out-of-scope. Do not expand a focused review into a full analysis. +- **Cite external sources**: When referencing external material, cite it. Useful references for this codebase: + - CppReference.com (C++20/23), NodeSource V8 docs (https://v8docs.nodesource.com/), Godbolt.org + - MDN Web Docs (web standards), OWASP/CERT (security) + - KJ, Cap'n Proto, and V8 source repositories and issue trackers diff --git a/.opencode/agent/submit.md b/.opencode/agent/submit.md index 90aa0990492..24af515b5bb 100644 --- a/.opencode/agent/submit.md +++ b/.opencode/agent/submit.md @@ -1,5 +1,5 @@ --- -description: Check if branch is ready to submit, review pending changes, prepare for PR/code review, verify tests pass, assess submission readiness, and suggest reviewers for the changes +description: Prepares changes for submission. Reviews pending changes, runs pre-submission checks, crafts commit messages, and suggests reviewers. Use when ready to submit a PR or to check if a branch is ready. mode: subagent temperature: 0.1 permission: @@ -11,6 +11,10 @@ permission: 'git log*': allow 'git show*': allow 'git blame*': allow + 'git fetch*': allow + 'git branch*': allow + 'git rev-parse*': allow + 'git merge-base*': allow 'git add*': ask 'git commit*': ask 'git stash*': ask @@ -27,36 +31,27 @@ permission: 'rg *': allow 'grep *': allow 'find *': allow + 'ls': allow 'ls *': allow 'cat *': allow 'head *': allow 'tail *': allow 'wc *': allow - 'gh pr view --json comments': allow - 'gh pr checks': allow - 'gh pr status': allow - 'gh pr diff': allow - 'gh pr list': allow + 'gh pr view*': allow + 'gh pr checks*': allow + 'gh pr status*': allow + 'gh pr diff*': allow + 'gh pr list*': allow + 'gh pr create*': ask 'gh pr checkout*': ask 'gh pr comment*': ask - 'gh pr close*': ask - 'gh pr reopen*': ask - 'gh pr merge*': ask + 'gh pr review*': ask + 'gh api *': ask 'gh issue view*': allow 'gh issue list*': allow - 'gh issue comment*': ask - 'gh issue close*': ask - 'gh issue reopen*': ask - 'gh issue create*': ask - 'gh issue edit*': ask 'gh issue status': allow 'gh auth status': allow - 'gh auth login*': ask - 'gh auth logout*': ask 'gh alias list': allow - 'gh alias set*': ask - 'gh alias delete*': ask - 'gh alias rename*': ask --- You are a Code Submission agent specializing in helping to prepare changes for code review. Your role is to assist developers ensure their changes are well-organized, properly tested, documented, and ready for review. @@ -183,19 +178,27 @@ Fixes: #1234 If changes need restructuring: - Suggest logical commit boundaries -- Help stage files incrementally with `git add -p` guidance +- Help stage specific files with `git add ` commands - Recommend squashing or splitting commits as needed - Fixup commits are ok but need to be squashed before merging a PR ### 6. Check to see if GitHub comments are addressed -If the current branch has an associated GitHub PR, check for any unresolved comments. If there are any, list them out and recommend addressing them before submission. +If the current branch has an associated GitHub PR, fetch review comments via `gh api repos/{owner}/{repo}/pulls/{number}/comments` and review threads via `gh pr view --json comments`. Check whether resolved comments have actually been addressed in the current code. List any unresolved or incorrectly resolved comments and recommend addressing them before submission. If the current branch is not rebased on the latest main branch, recommend rebasing to pick up any new changes or fixes. ### 7. Try to identify conflicting changes -If the current branch has an associated GitHub PR, check for any conflicting changes with the main branch. If there are any, list them out and recommend resolving them before submission. In general, the current branch should be rebased on the latest main branch to minimize the chance of conflicts but if that's not possible, examine the delta to identify potential conflicts and suggest resolutions. +If the current branch has an associated GitHub PR, check for conflicting changes with the main branch: + +1. Run `git fetch origin main` to get the latest main. +2. Run `git merge-base HEAD origin/main` to find the common ancestor. +3. Run `git diff origin/main...HEAD --name-only` to see files changed on this branch. +4. Run `git diff origin/main --name-only` to see files changed on main since divergence. +5. If the same files appear in both, examine the specific changes to identify conflicts. + +In general, the current branch should be rebased on the latest main branch. If that's not possible, list the conflicting files and suggest resolutions. ### 8. Suggest reviewers @@ -240,30 +243,6 @@ Do suggest reviewers who have made material comments or suggestions on the assoc --- -## Project-Specific Context - -### workerd Conventions - -- Use KJ library idioms (`kj::Own`, `kj::Maybe`, `kj::Promise`, etc.) -- Follow Cap'n Proto conventions for schemas -- V8/JSG integration follows patterns in `src/workerd/jsg/` -- Compatibility flags go in `compatibility-date.capnp` -- Risky changes use autogates (`src/workerd/util/autogate.*`) - -### Test Commands - -- `just test` - Run all tests -- `just node-test ` - Run Node.js compatibility tests -- `just wpt-test ` - Run Web Platform Tests -- `bazel test //path/to:target` - Run specific test target - -### Build Commands - -- `just build` or `bazel build //src/workerd/server:workerd` -- `just build-asan` - Build with AddressSanitizer - ---- - ## Output Format When reviewing changes, provide: @@ -306,7 +285,7 @@ List of potential reviewers based on code changes. ## Notes -- The CONTRIBUTING.md file for project-specific contribution guidelines and the README.md for general project overview. +- See CONTRIBUTING.md for project-specific contribution guidelines and README.md for general project overview. - A PR may involve multiple commits; ensure each is well-scoped. - A PR is not ready to merge unless all required checks pass, all comments are resolved, there are no fixup commits, and the PR has been approved by at least one reviewer. However, your role is only to help prepare the changes for review, not to determine merge readiness. - When suggesting running tests or builds, always specify the exact command to run. diff --git a/.opencode/agent/trivia.md b/.opencode/agent/trivia.md deleted file mode 100644 index 83228652371..00000000000 --- a/.opencode/agent/trivia.md +++ /dev/null @@ -1,93 +0,0 @@ ---- -description: Plays codebase trivia games while waiting for complex tasks. Offer to invoke this agent when you're busy with lengthy operations to entertain and educate the user about the workerd codebase. -mode: subagent -temperature: 0.7 -tools: - write: false - edit: false - bash: false - read: true - glob: true - grep: true ---- - -You are a fun and engaging trivia host for the workerd codebase! Your job is to test the user's knowledge of this JavaScript/WebAssembly runtime while they wait for complex tasks to complete. The purpose is to entertain and further educate the user about the workerd project. - -Periodically, even if users are not actively playing trivia, surface interesting facts about the codebase as other tasks are running, even if the user is not actively playing. Keep the tone light and fun. Particularly highlight interesting or lesser-known aspects of the codebase. - -## How to Play - -1. **Ask one trivia question at a time** about the workerd codebase -2. **Wait for the user's answer** before revealing the correct answer -3. **Provide educational explanations** when revealing answers with references to relevant files or concepts -4. **Keep score** if the user wants to play multiple rounds -5. **Be encouraging** - celebrate correct answers and gently explain incorrect ones - -## Question Categories - -Draw questions from these areas of the workerd codebase: - -### Architecture & Structure - -- Directory structure and what each folder contains -- The role of Cap'n Proto in configuration -- How JSG (JavaScript Glue) connects to V8 -- The I/O subsystem and actor storage -- V8 integration details -- Worker lifecycle management -- Programming languages used - -### APIs & Features - -- Runtime APIs (HTTP, crypto, streams, WebSocket, etc) -- Node.js compatibility layer -- Web Platform APIs -- Python support via Pyodide -- KJ library usage -- C++20/23 language features - -### Build System - -- Bazel build commands and targets -- Just commands for development -- Test types and how to run them - -### Development Practices - -- Compatibility date flags -- Autogates for risky changes -- Code style and formatting - -### History & Context - -- What Cloudflare Workers is -- Why workerd was open-sourced -- Key design decisions - -## Example Questions - -Here are some question formats to use: - -1. "What command would you use to run a specific Node.js compatibility test, like the zlib test?" -2. "In which directory would you find the WebSocket API implementation?" -3. "What serialization format does workerd use for its configuration files?" -4. "What is JSG and what role does it play in workerd?" -5. "True or False: workerd can run Python code via Pyodide" -6. "What's the purpose of the compatibility-date.capnp file?" - -## Guidelines - -- Start with easier questions and gradually increase difficulty -- Use the read-only tools (read, glob, grep) to verify your answers if needed -- Mix question types: multiple choice, true/false, fill-in-the-blank, open-ended -- If the user seems stuck, offer hints -- Keep the tone light and fun - this is meant to be entertaining! -- After 3-5 questions, ask if they want to continue or check on their task -- If the long running task is done, ask if they want to stop playing or continue -- Research answers using the codebase as needed to ensure accuracy. Do not guess or make up answers. - -## Starting the Game - -When invoked, introduce yourself briefly and jump right into the first question. Something like: - -"While we wait, let's test your workerd knowledge! Here's your first question..." diff --git a/.opencode/commands/autogate.md b/.opencode/commands/autogate.md new file mode 100644 index 00000000000..faf2896535a --- /dev/null +++ b/.opencode/commands/autogate.md @@ -0,0 +1,48 @@ +--- +description: Look up an autogate, or list all autogates if no argument given +subtask: true +--- + +Look up autogate: $ARGUMENTS + +**If no argument is provided (empty or blank), list all autogates:** + +1. Read the `AutogateKey` enum in `src/workerd/util/autogate.h`. +2. For each entry (excluding `NumOfKeys`), read the comment above it for a description. +3. Read the string mapping in `src/workerd/util/autogate.c++` to get the config string for each. +4. Output a summary table: + + | Autogate | Config string | Description | + | ----------- | -------------- | ------------------------------ | + | `ENUM_NAME` | `"string-key"` | Brief description from comment | + + Include the total count of active autogates. + +**If an argument is provided, look up that specific autogate:** + +1. **Find the enum entry.** Search `src/workerd/util/autogate.h` for the autogate name. The argument may be the enum name (e.g., `SOME_FEATURE`), the string key, or a partial match. + +2. **Find the string mapping.** Search `src/workerd/util/autogate.c++` for the corresponding entry in the string-to-enum mapping. Verify the enum and string map are in sync — flag a warning if one exists without the other. + +3. **Find usage sites.** Search for where the autogate is checked: + + ``` + grep -rn 'AutogateKey::\|isAutoGateEnabled.*' src/ --include='*.h' --include='*.c++' + ``` + +4. **Find tests.** Search for the autogate name in test files: + + ``` + grep -rn '' src/ --include='*.wd-test' --include='*-test.js' --include='*-test.ts' --include='*-test.c++' + ``` + + Also check if any tests use the `@all-autogates` variant to exercise this gate. + +5. **Output:** + - **Enum**: `AutogateKey::` (file:line) + - **String key**: the string used in configuration (file:line) + - **In sync**: yes/no — whether enum and string map match + - **Usage sites**: file:line list of where the autogate is checked + - **Tests**: file:line list of tests that reference this autogate + - **Notes**: Autogates are temporary and should be removed once the feature is fully rolled out. If the autogate appears to be stale (no recent commits touching it), note that. + - If the autogate is not found, suggest checking for typos and list all available autogates from the enum definition. diff --git a/.opencode/commands/changelog.md b/.opencode/commands/changelog.md new file mode 100644 index 00000000000..4d2c201e7e3 --- /dev/null +++ b/.opencode/commands/changelog.md @@ -0,0 +1,59 @@ +--- +description: Summarize current branch changes for a PR description or release note +subtask: true +--- + +Generate a changelog for the current branch. $ARGUMENTS + +Steps: + +1. **Determine the branch and base.** Run: + + ``` + git log --oneline origin/main..HEAD + ``` + + If there are no commits, try `main..HEAD`. If the user specified a different base in the arguments, use that. + +2. **Read the full diff** to understand the actual changes: + + ``` + git diff origin/main...HEAD --stat + ``` + +3. **Read commit messages** for context: + + ``` + git log origin/main..HEAD --format='%h %s%n%n%b' + ``` + +4. **Categorize changes.** You MUST load the `commit-categories` skill before categorizing. Use its path-pattern table to assign each change to a category based on the files it touches. + +5. **Draft the summary.** For each category with changes: + - One bullet per logical change (not per commit — squash related commits into one bullet) + - Start each bullet with a verb: Add, Fix, Update, Remove, Refactor + - Include the most important file references + - Note any breaking changes, new compat flags, or new autogates prominently + +6. **Output format:** + + ``` + ## Summary + + <1-2 sentence overview of the branch's purpose> + + ## Changes + + ### + - + + ## Breaking Changes + + + ## Testing + + ``` + + If the user's arguments request a specific format (e.g., "for release notes", "for PR"), adjust the tone accordingly. PR descriptions should be more detailed; release notes should be user-facing and concise. + + **IMPORTANT:** The output is meant to be copied into a PR or release notes. You MUST load the `markdown-drafts` skill and follow its rendering rules so the raw markdown is preserved for the user to copy. diff --git a/.opencode/commands/compat-flag.md b/.opencode/commands/compat-flag.md new file mode 100644 index 00000000000..fa9126b7bdd --- /dev/null +++ b/.opencode/commands/compat-flag.md @@ -0,0 +1,51 @@ +--- +description: Look up a compatibility flag, or list all flags if no argument given +subtask: true +--- + +Look up compatibility flag: $ARGUMENTS + +**If no argument is provided (empty or blank), list all compatibility flags:** + +1. Read `src/workerd/io/compatibility-date.capnp` and find all fields with `$compatEnableFlag`. +2. For each flag, extract: field name, field number, enable flag string, enable date (or "experimental" / "no date"), and a one-line summary from the comment. +3. Output a summary table grouped by category (streams, nodejs, containers, general, etc.): + + | Flag | Enable date | Description | + | ----------- | ----------- | ----------------- | + | `flag_name` | 2025-01-15 | Brief description | + + Include the total count of compatibility flags. + +**If an argument is provided, look up that specific flag:** + +1. **Find the flag definition.** Search `src/workerd/io/compatibility-date.capnp` for the flag name. The argument may be the snake_case flag name (e.g., `text_decoder_replace_surrogates`) or the camelCase field name (e.g., `textDecoderReplaceSurrogates`). Try both forms. + +2. **Extract flag metadata** from the capnp definition: + - Field name and number + - `$compatEnableFlag` name + - `$compatDisableFlag` name (if present) + - `$compatEnableDate` (if set) + - Whether it has `$experimental` annotation + - The comment block describing the flag + +3. **Find C++ usage sites.** Search for the getter (e.g., `getTextDecoderReplaceSurrogates()`) across the codebase: + + ``` + grep -rn "getTextDecoderReplaceSurrogates\|text_decoder_replace_surrogates" src/ + ``` + +4. **Find tests.** Search for the snake_case flag name in `.wd-test` and test `.js` files: + + ``` + grep -rn "text_decoder_replace_surrogates" src/ --include='*.wd-test' --include='*-test.js' --include='*-test.ts' + ``` + +5. **Output:** + - **Flag**: enable name / disable name + - **Enable date**: date or "not set (must be explicitly enabled)" + - **Description**: from the capnp comment + - **Usage sites**: file:line list of where the flag is checked in C++ + - **Tests**: file:line list of tests that exercise this flag + - **Annotations**: experimental, neededByFl, impliedByAfterDate, etc. + - If the flag is not found, suggest checking for typos and list flags with similar names diff --git a/.opencode/commands/deps.md b/.opencode/commands/deps.md new file mode 100644 index 00000000000..e95a37a659a --- /dev/null +++ b/.opencode/commands/deps.md @@ -0,0 +1,36 @@ +--- +description: Show the dependency graph for a Bazel target +subtask: true +--- + +Show dependencies for: $ARGUMENTS + +Steps: + +1. **Resolve the target.** If the argument is a file path, find its Bazel target first by checking the `BUILD.bazel` in the same directory. If it's already a Bazel label (e.g., `//src/workerd/api:http`), use it directly. + +2. **Direct dependencies.** Run: + + ``` + bazel query 'deps(, 1)' --output label 2>/dev/null + ``` + + This shows what the target directly depends on. + +3. **Reverse dependencies** (what depends on this target). Run: + + ``` + bazel query 'rdeps(//src/..., , 1)' --output label 2>/dev/null + ``` + + This shows what directly depends on this target within `src/`. + +4. **If the user asks for the full transitive graph**, use depth 2-3 instead of 1, but warn that deep queries can be slow. + +5. **Output:** + - **Target**: the resolved Bazel label + - **Direct dependencies**: grouped by type (internal `//src/...` vs external `@...`) + - **Reverse dependencies**: what depends on this target (within `src/`) + - **Notable observations**: circular dependency risks, unusually large dependency sets, or external deps that may be surprising + + Keep the output concise. If there are more than 20 deps in a category, summarize the count and list the most important ones. diff --git a/.opencode/commands/explain.md b/.opencode/commands/explain.md new file mode 100644 index 00000000000..28194112709 --- /dev/null +++ b/.opencode/commands/explain.md @@ -0,0 +1,33 @@ +--- +description: Explain what a file, class, or symbol does and how it fits into the architecture +subtask: true +--- + +Explain: $ARGUMENTS + +Steps: + +1. **Locate the target.** If the argument is a file path, read it. If it's a symbol name (class, function, macro), search for its definition using grep/glob. + +2. **Read the definition.** For classes, read the header file first. For functions, read the declaration and implementation. For large files (>500 lines), start with the class declaration and public API before reading implementation details. + +3. **Check for local documentation.** Look for: + - An `AGENTS.md` in the same directory or nearest parent + - A `README.md` in the same directory + - Doc comments on the symbol itself + +4. **Trace relationships.** Identify: + - What this code depends on (base classes, key types used, includes) + - What depends on this code (grep for callers, subclasses, or includes of the header) + - Limit to the most important 5-10 relationships, not an exhaustive list + +5. **Identify the architectural layer.** Place it in context: + - Is this API layer (`api/`), I/O layer (`io/`), JSG bindings (`jsg/`), server infrastructure (`server/`), or utility (`util/`)? + - What is its role in the request lifecycle or worker lifecycle? + +6. **Output a concise summary** with: + - **What it is**: One-sentence description + - **Where it lives**: File path(s) and architectural layer + - **Key responsibilities**: Bullet list of what it does + - **Key relationships**: What it depends on and what depends on it + - **Notable patterns or gotchas**: Anti-patterns, compat flag gating, thread safety considerations, or other things a developer should know diff --git a/.opencode/commands/find-owner.md b/.opencode/commands/find-owner.md new file mode 100644 index 00000000000..9e3d066e37e --- /dev/null +++ b/.opencode/commands/find-owner.md @@ -0,0 +1,34 @@ +--- +description: Find who owns or is most active in a file or directory +subtask: true +--- + +Find code owners for: $ARGUMENTS + +Steps: + +1. **Resolve the path.** If the argument is a symbol name, find its file first. If it's a directory, analyze the directory as a whole. + +2. **Recent commit activity.** Run: + + ``` + git log --format='%aN <%aE>' --since='6 months ago' -- | sort | uniq -c | sort -rn | head -10 + ``` + + This shows who has been most active recently. + +3. **Blame analysis.** For individual files, run: + + ``` + git blame --line-porcelain | grep '^author ' | sort | uniq -c | sort -rn | head -10 + ``` + + This shows who wrote the most lines currently in the file. + +4. **Check for CODEOWNERS.** Look for a `CODEOWNERS` or `.github/CODEOWNERS` file that may define ownership rules. + +5. **Output:** + - **Top recent contributors** (last 6 months): ranked list with commit counts + - **Top authors by current lines**: ranked list with line counts + - **Suggested reviewers**: 2-3 people who are most likely to be good reviewers, preferring people who are both recent contributors and significant authors + - If the results are sparse (few commits, single author), note that and suggest broadening the search to the parent directory diff --git a/.opencode/commands/find-test.md b/.opencode/commands/find-test.md new file mode 100644 index 00000000000..d5a08b358fb --- /dev/null +++ b/.opencode/commands/find-test.md @@ -0,0 +1,20 @@ +--- +description: Find the Bazel test target for a source file +subtask: true +--- + +Find the Bazel test target(s) for the file: $ARGUMENTS + +Steps: + +1. Determine the directory containing the file. +2. Look for a `BUILD.bazel` file in that directory (or the nearest parent with one). +3. Search for `wd_test()` or `kj_test()` rules that reference the file or related test files. +4. If the file is a source file (not a test), look for test files in the same directory or a `tests/` subdirectory that test this source. +5. Return the full Bazel target with the `@` suffix required for running tests. + +Output format: + +- The exact `just test` or `bazel test` command to run +- List all variants if relevant (`@`, `@all-compat-flags`, `@all-autogates`) +- If no test target is found, suggest how to create one using `just new-test` diff --git a/.opencode/commands/investigate.md b/.opencode/commands/investigate.md new file mode 100644 index 00000000000..6bd01599c9e --- /dev/null +++ b/.opencode/commands/investigate.md @@ -0,0 +1,116 @@ +--- +description: Investigate a bug from a Sentry issue or error description, biasing toward writing a reproducing test early +subtask: false +--- + +Load the `test-driven-investigation`, `investigation-notes`, `find-and-run-tests`, `parent-project-skills`, and `dad-jokes` skills, then investigate: $ARGUMENTS + +## Prerequisites + +This command uses Sentry MCP tools when given a Sentry issue ID or URL. The Sentry MCP connection requires a user-specific `X-Sentry-Token` header configured in `~/.config/opencode/opencode.json` under `mcp.sentry.headers`. If the Sentry tools fail with auth errors, tell the user to check their token configuration and stop — do not guess at issue details. + +## Parsing the argument + +The argument can be: + +- **A Sentry issue ID** (e.g., `6181478`) — fetch from Sentry +- **A Sentry short ID** (e.g., `EDGEWORKER-RUNTIME-4MS`) — fetch from Sentry +- **A Sentry URL** (e.g., `https://sentry.io/organizations/.../issues/...`) — extract the issue ID, fetch from Sentry +- **A plain text description** (e.g., `"concurrent write()s not allowed" in kj/compat/http.c++`) — skip Sentry, go straight to orientation + +## Steps + +### 1. Extract the error (5 minutes max) + +**If Sentry issue:** + +1. Fetch the issue details with `sentry_get_sentry_issue`. +2. Fetch the most recent event with `sentry_list_sentry_issue_events` (limit 1), then `sentry_get_sentry_event` to get the full stack trace. +3. Extract: + - The **error message** (assertion text, exception message, crash description) + - The **assertion/crash site** (file and line from the top of the stack) + - The **entry point** (the outermost workerd/KJ/capnp frame in the stack — where the operation started) + +**If plain text:** Parse the error message and file reference from the description. + +**Output to user:** The error message, crash site, and entry point. One short paragraph. Do not go deeper yet. + +### 2. Orient (10 minutes max) + +Find three things: + +1. **The crash site source.** Read the assertion/crash line and its immediate context (~50 lines). Understand what invariant was violated and what state would cause it. + +2. **The test file.** Use `/find-test` on the source file containing the crash site. If no test exists, identify the nearest test file in the same directory. + +3. **Existing feature tests.** Search for existing tests that exercise the _feature_ involved in the bug — not just tests near the crash site file. The crash may be in `pipeline.c++` but the relevant working test may be an integration test in a completely different directory. These existing tests encode setup, verification, and framework patterns you need. They are your starting template. + +4. **The build command.** Construct the exact `bazel test` invocation to run a single test case from that test file. + +**Output to user:** The crash site with a one-sentence explanation of the invariant, the test file path, and the build command. + +### 3. Hypothesize + +Form a hypothesis in the format: + +> "If I do X after Y, Z will happen because W." + +This does not need to be correct. It needs to be testable. State it to the user. + +Ask for clarification or additional details if you cannot form a hypothesis with the information you have. But do not ask for more information just to delay writing a test. + +### 4. Write the test + +**Start from an existing test if one exists** (from step 2.3). Clone it and modify the single variable that your hypothesis targets (disable an autogate, change a config flag, alter the setup). This is almost always faster and more correct than writing from scratch, because existing tests already have the right verification (subrequest checks, expected log patterns, shutdown handling). + +If no existing test is suitable, write a new one that: + +- Sets up the minimum state to reach the crash site +- Performs the operation described in the hypothesis +- **Includes observable verification** — the test must check that the feature actually ran, not just that nothing crashed. Use subrequest expectations, check for feature-specific log lines, or verify side effects. +- Asserts the expected behavior (what _should_ happen if the bug didn't exist) + +Keep it short. Prefer public API. Do not try to reproduce the full production call stack. + +### 5. Run the test + +Build and run using the command from step 2. **Start the build immediately.** Do not read more code before starting the build. + +While waiting for the build: + +- Read code that would inform the **next** test iteration if this one doesn't reproduce the bug +- Do NOT use the wait time to second-guess the current test + +### 6. Validate and iterate + +**After every test run, first:** Update the tracking document (if using one). Then check the test output for evidence the code path was exercised — feature-specific log lines, subrequests, RPC calls. A test that passes with no evidence the feature ran is not a valid result. + +Based on the result: + +- **Test fails as expected** → the mechanism is confirmed. Report findings to the user. Read code with purpose to find the fix, not to find the bug. +- **Test passes with evidence the feature ran** → hypothesis was wrong. Adjust the hypothesis, update the test, run again. Tell the user what you learned. +- **Test passes with NO evidence the feature ran** → the test is not exercising the code path. Do not read more source code to explain why. Fix the test first — compare it against existing working tests to find what's missing. +- **Test doesn't compile** → fix the compilation error and rerun. This is not a setback, it's a normal part of the process. +- **Test crashes differently** → follow the new trail but note the divergence. Tell the user. + +Repeat until the bug mechanism is confirmed or you've exhausted reasonable hypotheses (at which point, report what you've tried and what you've ruled out). + +### 7. Report + +When the mechanism is confirmed, output: + +- **Bug summary**: One paragraph describing the root cause +- **Reproduction**: The test name and how to run it +- **Crash site**: `file:line` with explanation +- **Suggested fix direction**: Where the fix likely needs to go (if apparent from the test results) + +## Rules + +- **Work in parallel whenever possible.** Don't wait for the build to finish before reading code that would inform the next test iteration. Use the build time to maximize your learning and progress. Investigate multiple +hypotheses in parallel if you can, but do not let this delay writing and running tests. +- **Do not spend more than 15 minutes reading code before the first test is written and building.** If you hit 15 minutes, write whatever test you can with your current understanding. +- **Do not re-read the same function more than twice.** If you catch yourself doing this, write a test immediately. +- **Do not try to trace the full call stack before writing a test.** The test will tell you if your understanding is correct. +- **Every hypothesis must be tested, not just reasoned about.** +- **Update the tracking document with each iteration.** If a tracking document is being used, update the hypotheses, code read, and test results sections so you have a clear record of your investigation process. Particularly after compaction, if the tracking document is outdated, update it before coninuing to the next step. +- **Never** miss an opportunity for a good dad joke. Don't overdo it, but don't avoid them either. diff --git a/.opencode/commands/onboarding.md b/.opencode/commands/onboarding.md new file mode 100644 index 00000000000..d5e5b2865d4 --- /dev/null +++ b/.opencode/commands/onboarding.md @@ -0,0 +1,64 @@ +--- +description: Guided onboarding to the workerd codebase, or a specific area +subtask: true +--- + +Onboard: $ARGUMENTS + +**If no argument is provided (empty or blank), present the project basics and available areas:** + +1. **What is workerd**: Cloudflare Workers JavaScript/WebAssembly runtime. C++23 monorepo built with Bazel, using V8 for JS execution. + +2. **How to build and test**: + - Build: `just build` + - Run all tests: `just test` + - Run a single test with live output: `just stream-test ` + - Run a sample: `bazel run //src/workerd/server:workerd -- serve $(pwd)/samples//config.capnp` + +3. **AI tools available to help you**: Read the `.opencode/` directory and list: + - **Agents** (from `.opencode/agent/`): list each with a one-line description + - **Commands** (from `.opencode/commands/`): list each with a one-line description (e.g., `/explain`, `/find-owner`, `/trace`, `/run`, etc.) + - **Skills** (from `.opencode/skills/`): list each with a one-line description (e.g., `kj-style`, `add-compat-flag`, `update-v8`, etc.) + +4. **What do you want to learn about?** Present areas as a menu: + - `/onboarding vscode` — Editor setup, extensions, debugging, clangd + - `/onboarding architecture` — Layers, request flow, key abstractions + - `/onboarding api` — JavaScript APIs (HTTP, crypto, WebSocket, etc.) + - `/onboarding streams` — ReadableStream/WritableStream/TransformStream + - `/onboarding node` — Node.js compatibility layer + - `/onboarding io` — I/O context, ownership, worker lifecycle + - `/onboarding actors` — Durable Objects, storage, gates + - `/onboarding jsg` — V8 bindings, JSG macros, type registration + - `/onboarding build` — Bazel, dependencies, test formats + - `/onboarding server` — Config, services, networking + - `/onboarding ` — Guided walkthrough of a specific area + + Keep this concise — just the menu, don't explain each area in detail. + +**If an argument is provided, give a guided walkthrough of that area:** + +### General approach for all areas + +1. **Read context**: Read the relevant `AGENTS.md` file for the area (e.g., `src/workerd/api/streams/AGENTS.md` for streams). Also check for a `README.md` in the directory. +2. **Explain the subsystem**: What it does, why it exists, how it fits into the broader architecture. Keep it to 2-3 paragraphs. +3. **Key classes and files**: List the 5-10 most important classes/files with one-line descriptions and file paths. +4. **Concrete example**: Walk through one specific flow end-to-end (e.g., "here's what happens when a fetch() response body is read as a stream"). +5. **Key patterns to know**: Patterns specific to this area that a newcomer needs to understand (e.g., `IoOwn` for io, JSG macros for jsg, compat flag gating for api). +6. **Tests to read**: Point to 2-3 representative tests that demonstrate how the code works. +7. **Further reading**: Suggest related `/onboarding ` topics, relevant `/explain` targets, or docs. +8. **Never** miss an opportunity for a good dad joke. Don't overdo it, but don't avoid them either. + +### Area-specific sources + +| Area | Primary sources | +| -------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `vscode` | `docs/vscode.md` — devcontainer setup, recommended extensions (clangd, C/C++, capnproto-syntax, GitLens, markdownlint), VSCode tasks (build/test/clean), debug launch targets (workerd dbg, inspector, test case, wd-test case), clangd configuration (`compile_flags.txt` or dependency files), `tools/unix/clangd-check.sh` | +| `architecture` | `CLAUDE.md`, root `AGENTS.md`, all subdirectory `AGENTS.md` files — explain the layers: `server/` (config, networking) → `io/` (IoContext, worker lifecycle) → `api/` (JS APIs) → `jsg/` (V8 bindings), plus `util/` (shared utilities) | +| `api` | `src/workerd/api/AGENTS.md`, `src/workerd/api/BUILD.bazel` — the JS API surface, `global-scope.h` as the entry point, how APIs are registered | +| `streams` | `src/workerd/api/streams/AGENTS.md`, `src/workerd/api/streams/README.md` — dual controller architecture (internal vs standard), the key classes | +| `node` | `src/node/AGENTS.md`, `src/workerd/api/node/AGENTS.md` — three-tier module system (C++ → TS internal → TS public), `NODEJS_MODULES` macro, compat flag gating | +| `io` | `src/workerd/io/AGENTS.md` — IoContext, IoOwn, DeleteQueue, InputGate/OutputGate, Worker::Actor | +| `actors` | `src/workerd/io/AGENTS.md`, `src/workerd/api/actor-state.h` — Actor lifecycle, ActorCache vs ActorSqlite, gates, hibernation | +| `jsg` | `src/workerd/jsg/AGENTS.md`, `src/workerd/jsg/README.md` — JSG_RESOURCE_TYPE, JSG_METHOD, type mapping, V8 integration | +| `build` | `build/AGENTS.md`, `CLAUDE.md` — Bazel targets, `just` commands, `wd_test`/`kj_test` macros, dependency management (`MODULE.bazel`, `build/deps/`) | +| `server` | `src/workerd/server/AGENTS.md` — workerd.capnp config, Server class, service setup, networking | diff --git a/.opencode/commands/review.md b/.opencode/commands/review.md new file mode 100644 index 00000000000..74eadfc1743 --- /dev/null +++ b/.opencode/commands/review.md @@ -0,0 +1,25 @@ +--- +description: Review local branch changes or a GitHub PR +agent: architect +subtask: true +--- + +Review code changes. Determine what to review based on the arguments below, then follow the architect agent's review workflow. + +**Arguments:** $ARGUMENTS + +**How to determine what to review:** + +1. If arguments contain a PR number (e.g., `1234`) or URL (e.g., `https://github.com/.../pull/1234`), review that PR: + - Use `gh pr view ` for the description and metadata + - Use `gh pr diff ` for the changes + - Use `gh pr checks ` for CI status + - Check for prior review comments via `gh api` + +2. If no PR is specified, review the current branch vs origin/main: + - Use `git diff origin/main...HEAD` for the changes + - Use `git log --oneline origin/main..HEAD` for commit context + +Perform a balanced review covering all analysis areas. Focus on CRITICAL and HIGH issues first. If there are no significant issues, say so briefly rather than inventing low-value findings. + +Any additional instructions from the arguments above (e.g., "focus on memory safety") should narrow the review scope accordingly. diff --git a/.opencode/commands/run.md b/.opencode/commands/run.md new file mode 100644 index 00000000000..b44b8251390 --- /dev/null +++ b/.opencode/commands/run.md @@ -0,0 +1,113 @@ +--- +description: Run tests or samples +subtask: true +--- + +Run: $ARGUMENTS + +**Parse the argument to determine what to run:** + +### Running tests + +- `/run test ` — Run a specific test by name or path +- `/run tests` — Run all tests (`just test` or `bazel test //...`) +- `/run test ` — Run tests for an area like `streams`, `http`, `jsg`, `node`, etc. + +**Steps for running a specific test:** + +1. **Resolve the test target.** If the argument is a file path, find its Bazel test target. If it's a short name (e.g., `streams`), search for matching test targets: + + ``` + bazel query 'kind(".*_test", //src/workerd/...)' --output label 2>/dev/null | grep -i '' + ``` + +2. **Determine the test type:** + - If the target comes from `wd_test()`, it's a `.wd-test` config test. These have variants: `@` (oldest compat), `@all-compat-flags` (newest compat), `@all-autogates`. + - If the target comes from `kj_test()`, it's a C++ unit test. + +3. **Run it.** Use `just stream-test` for a single test (streams output live) or `just test` for batches: + + ``` + just stream-test //src/workerd/api/tests:@ + ``` + + For running all tests in an area: + + ``` + just test //src/workerd/api/tests/... + just test //src/workerd/jsg/... + ``` + +4. **If the test fails**, read the output and provide a brief summary of what failed and why. If it's a compilation error, identify the source. If it's a test assertion failure, identify the failing test case and expected vs actual values. + +### Running samples + +- `/run sample ` — Run a sample by name (e.g., `helloworld_esm`, `nodejs-compat`, `durable-objects-chat`) +- `/run samples` — List all available samples + +**Steps for running a sample:** + +1. **List available samples** if the user says `/run samples` or the name doesn't match: + + ``` + ls samples/ + ``` + +2. **Find the sample config.** Look for `config.capnp` in `samples//`: + + ``` + ls samples//config.capnp + ``` + +3. **Build workerd first:** + + ``` + bazel build //src/workerd/server:workerd + ``` + +4. **Run the sample:** + + ``` + bazel run //src/workerd/server:workerd -- serve $(pwd)/samples//config.capnp + ``` + + Add optional flags if the user requests them or the sample requires them: + - `--verbose` — verbose logging + - `--watch` — auto-reload on file changes + - `--experimental` — enable experimental features + + Some samples require `--experimental` (check if the config uses experimental compat flags). Samples that typically need it: `pyodide*`, `repl-server*`, `filesystem`. + +5. **Tell the user** what address the sample is listening on (usually `http://localhost:8080` based on the config's socket definition) and that it will keep running until they press Ctrl+C. + +6. **IMPORTANT:** Running a sample launches a long-lived process. After starting it, inform the user and do not attempt further actions until they indicate the process has been stopped. The intent is for the human to manually test the running server. + +7. When the user indicates they are done testing the sample (e.g., "I'm done testing, you can stop now"), stop the process. + +### Running all tests + +- `/run tests` → `just test` (runs the full test suite) + +This will take a long time. Warn the user and ask for confirmation before running. + +### Running involves building + +Running tests or samples will trigger a build of the necessary targets. If the build fails, report the failure and do not attempt to run. + +### Run modes + +- If the user requests to run with ASAN, add the `--config=asan` flag to the bazel command. Warn the user that ASAN builds are slower and may produce more verbose output. +- If the user requests to run with release mode, add the `--config=opt` flag to the bazel command. This will produce optimized binaries without debug symbols, which may be faster but harder to debug if something goes wrong. +- If the user requests to run with debug mode, add the `--config=debug` flag to the bazel command. This will produce debug builds with symbols, which are easier to debug but may be slower. + +### Examples + +| Command | What it does | +| ------------------------------------------ | ----------------------------------------------------- | +| `/run test streams` | Runs all streams-related tests | +| `/run test http-test` | Finds and runs the HTTP test target | +| `/run test //src/workerd/jsg:jsg-test` | Runs the exact Bazel target | +| `/run tests` | Runs the entire test suite | +| `/run sample helloworld_esm` | Builds workerd and runs the ESM hello world sample | +| `/run sample nodejs-compat --experimental` | Runs the Node.js compat sample with experimental flag | +| `/run samples` | Lists all available samples | diff --git a/.opencode/commands/submit.md b/.opencode/commands/submit.md new file mode 100644 index 00000000000..c290b4f32f6 --- /dev/null +++ b/.opencode/commands/submit.md @@ -0,0 +1,7 @@ +--- +description: Check if the current branch is ready to submit +agent: submit +subtask: true +--- + +Check if the current branch is ready for submission. Follow the full workflow from the submit agent instructions. $ARGUMENTS diff --git a/.opencode/commands/test-for.md b/.opencode/commands/test-for.md new file mode 100644 index 00000000000..44d0e2f562e --- /dev/null +++ b/.opencode/commands/test-for.md @@ -0,0 +1,36 @@ +--- +description: Find tests that exercise a source file's code, with specific test case details +subtask: true +--- + +Find tests for: $ARGUMENTS + +Steps: + +1. **Resolve the source file.** If the argument is a file path, use it directly. If it's a class or symbol name, find its source file first. + +2. **Extract key symbols.** Read the source file (header preferred) and identify the main exported symbols: class names, public method names, free function names, macros. These are what we'll search for in tests. + +3. **Find test targets.** Look in the `BUILD.bazel` file in the same directory and in a `tests/` subdirectory for `wd_test()` and `kj_test()` rules. Also check if the source file is a dependency of test targets: + + ``` + bazel query 'rdeps(//src/..., , 1)' --output label 2>/dev/null | grep -i test + ``` + +4. **Find test cases that reference the code.** For each test file found, grep for the key symbols from step 2. Read the surrounding context to understand what each test case exercises. Tests for some C++ +code may be in JavaScript test files if the C++ code is exposed to JS, so check those as well. + +5. **Assess coverage.** Compare the public API surface (from step 2) against what the tests exercise (from step 4). Identify: + - Well-tested: symbols referenced in multiple test cases + - Partially tested: symbols referenced but not thoroughly (e.g., only happy path) + - Untested: public symbols with no test references + +6. **Output:** + - **Source**: file path and main symbols + - **Test targets**: Bazel labels with `just test` commands + - **Test cases** (grouped by test file): + - `file:line` — description of what's tested (e.g., "tests TextEncoder with UTF-8 input") + - **Coverage assessment**: + - Well-tested: list of symbols + - Gaps: list of public symbols with no apparent test coverage + - **Suggestions**: specific test cases to add for untested code paths, if applicable diff --git a/.opencode/commands/trace.md b/.opencode/commands/trace.md new file mode 100644 index 00000000000..2e3b1d68ae9 --- /dev/null +++ b/.opencode/commands/trace.md @@ -0,0 +1,37 @@ +--- +description: Trace callers and callees of a function or method across the codebase +subtask: true +--- + +Trace call graph for: $ARGUMENTS + +Steps: + +1. **Find the definition.** Search for the function/method definition. If the argument is ambiguous (e.g., a common name like `get`), ask for clarification or use the class-qualified form (e.g., `IoContext::current`). + +2. **Read the implementation.** Read the function body to identify: + - **Direct callees**: functions/methods called within the body + - Filter out trivial calls (logging, assertions, simple getters) unless they're relevant to understanding the flow + +3. **Find callers.** Search for call sites across the codebase: + + ``` + grep -rn '' src/ --include='*.h' --include='*.c++' + ``` + + Filter to actual calls (not declarations, comments, or string literals). For methods, search for both `->methodName(` and `.methodName(` patterns. + +4. **Build the call graph.** Organize into: + - **Callers** (who calls this function) — grouped by directory/component + - **Callees** (what this function calls) — the significant ones from the implementation + +5. **Trace one level deeper if needed.** If the argument includes "deep" or the function is a thin wrapper, trace one additional level in each direction to find the meaningful boundaries. + +6. **Output:** + - **Function**: full qualified name, file:line of definition + - **Callers** (N call sites): + - `file:line` — brief context of why it's called + - **Callees** (N significant calls): + - `function_name` (`file:line`) — brief description + - **Call flow summary**: A brief narrative of how this function fits into the broader execution flow (e.g., "Called during request setup to initialize the I/O context, which then creates the worker lock...") + - Limit to ~20 most important callers. If there are more, note the total count and list the most representative ones grouped by component. diff --git a/.opencode/commands/trivia.md b/.opencode/commands/trivia.md new file mode 100644 index 00000000000..4c650e7ff2c --- /dev/null +++ b/.opencode/commands/trivia.md @@ -0,0 +1,58 @@ +--- +description: Play workerd codebase trivia +subtask: true +--- + +You are a fun and engaging trivia host for the workerd codebase! Your job is to test the user's knowledge of this JavaScript/WebAssembly runtime. The purpose is to entertain and further educate the user about the workerd project. + +**Topic argument:** $ARGUMENTS + +If a topic is provided (e.g., `/trivia streams`, `/trivia KJ`, `/trivia Node.js compat`), focus all questions on that topic. Research the relevant area of the codebase first to generate accurate, specific questions. If no topic is provided, draw from any area of the codebase. + +## How to Play + +1. **Ask one trivia question at a time** about the workerd codebase +2. **Wait for the user's answer** before revealing the correct answer +3. **Provide educational explanations** when revealing answers with references to relevant files or concepts +4. **Keep score** if the user wants to play multiple rounds +5. **Be encouraging** - celebrate correct answers and gently explain incorrect ones + +## Question Categories + +Draw questions from any area of the workerd codebase. The examples below are starting points, not limits — dig into the actual code to find interesting details for questions. + +### Architecture & Structure + +Examples: directory structure, Cap'n Proto configuration, JSG/V8 integration, I/O subsystem, actor storage, worker lifecycle, cross-heap ownership (`IoOwn`), gate mechanisms, etc. + +### APIs & Features + +Examples: HTTP/fetch, crypto, streams, WebSocket, Node.js compat, Web Platform APIs, Python/Pyodide, KJ library idioms, Durable Objects, R2/KV/Queue bindings, RPC, containers, etc. + +### Build System + +Examples: Bazel targets and macros, `just` commands, test types and variants, dependency management, TypeScript bundling, etc. + +### Development Practices + +Examples: compatibility flags, autogates, code style, error handling patterns, memory management, promise patterns, mutex patterns, etc. + +### History & Context + +Examples: Cloudflare Workers architecture, why workerd was open-sourced, key design decisions, V8 integration choices, etc. + +## Guidelines + +- Start with easier questions and gradually increase difficulty +- Use the read-only tools (read, glob, grep) to verify your answers if needed +- **All questions must be multiple choice** with 4 options (A through D). Exactly one option should be correct. +- If the user seems stuck, offer hints +- Keep the tone light and fun - this is meant to be entertaining! +- After 3-5 questions, ask if they want to continue +- Research answers using the codebase as needed to ensure accuracy. Do not guess or make up answers. + +## Starting the Game + +When invoked, introduce yourself briefly and jump right into the first question. Something like: + +"Let's test your workerd knowledge! Here's your first question..." diff --git a/.opencode/commands/whats-new.md b/.opencode/commands/whats-new.md new file mode 100644 index 00000000000..2b995e8ff06 --- /dev/null +++ b/.opencode/commands/whats-new.md @@ -0,0 +1,80 @@ +--- +description: Summarize recent commits — by count, date, or time range +subtask: true +--- + +Show what's new: $ARGUMENTS + +**Parse the argument** to determine the commit range: + +- A number (e.g., `10`) → the N most recent commits: `git log -` +- `since ` (e.g., `since 2026-02-01`) → commits after that date: `git log --since='2026-02-01'` +- `today` → `git log --since='midnight'` +- `this week` → `git log --since='1 week ago'` +- `this month` → `git log --since='1 month ago'` +- `yesterday` → `git log --since='2 days ago' --until='midnight'` +- `last week` → `git log --since='2 weeks ago' --until='1 week ago'` +- No argument → default to `10` + +Steps: + +1. **Target the main branch at origin.** Always show what's new on the remote `main` branch, + regardless of which branch is currently checked out. First, fetch the latest: + + ``` + git fetch origin main + ``` + + Then use `origin/main` as the ref for all subsequent git commands (instead of `HEAD`). + +2. **Fetch the commits.** Run `git log` with the parsed range against `origin/main`: + + ``` + git log origin/main --format='%h|%aN|%as|%s' --no-merges + ``` + + Also include merge commits separately if relevant: + + ``` + git log origin/main --format='%h|%aN|%as|%s' --merges + ``` + +3. **Get the diff stats** for scope: + + ``` + git diff --stat ^..origin/main + ``` + +4. **Categorize each commit.** You MUST load the `commit-categories` skill before categorizing. Use its path-pattern table to assign each commit to a category based on the files it touches. + +5. **Highlight notable changes.** Scan commit messages and diffs for: + - New compatibility flags (additions to `compatibility-date.capnp`) + - New autogates (additions to `autogate.h`) + - Breaking changes or behavioral changes + - New APIs or features + - Security fixes + - Dependency updates + - If current branch is not `main`: + - Relevant changes to files touched by the current branch (if any) + - Conflicts with the current branch (if any) + +6. **Output:** + + ``` + ## What's New () + + ** commits** by authors | files changed, insertions, deletions + + ### Highlights + - + + ### API + - `` — + + ### Node.js compat + - `` — + + ...additional categories as needed (omit empty categories)... + ``` + + Keep summaries to one line per commit. Group related commits (e.g., a feature + its fixup) into a single entry where obvious. diff --git a/.opencode/skills/add-autogate/SKILL.md b/.opencode/skills/add-autogate/SKILL.md new file mode 100644 index 00000000000..4715bcd171c --- /dev/null +++ b/.opencode/skills/add-autogate/SKILL.md @@ -0,0 +1,131 @@ +--- +name: add-autogate +description: Step-by-step guide for adding a new autogate to workerd for gradual rollout of risky changes, including enum registration, string mapping, usage pattern, and testing. +--- + +## Adding an Autogate + +Autogates enable gradual rollout of risky code changes independent of binary releases. Unlike compatibility flags (which are permanent, date-based behavioral changes), autogates are temporary gates that can be toggled on/off via internal tooling during rollout, then removed once the change is stable. + +### When to use an autogate vs a compat flag + +| Use an autogate when... | Use a compat flag when... | +| --------------------------------------------- | ------------------------------------------ | +| Rolling out a risky internal change gradually | Changing user-visible behavior permanently | +| You need a kill switch during rollout | The change is tied to a compatibility date | +| The gate will be removed once stable | Users need to opt in or out explicitly | + +Autogates and compat flags are separate mechanisms — an autogate does not become a compat flag. + +### Step 1: Add the enum value + +Edit `src/workerd/util/autogate.h`. Add a new entry to the `AutogateKey` enum **before `NumOfKeys`**: + +```cpp +enum class AutogateKey { + TEST_WORKERD, + // ... existing gates ... + // Brief description of what this gate controls. + MY_NEW_FEATURE, + NumOfKeys // Reserved for iteration. +}; +``` + +Naming convention: `SCREAMING_SNAKE_CASE` for the enum value. + +### Step 2: Add the string mapping + +Edit `src/workerd/util/autogate.c++`. Add a `case` to the `KJ_STRINGIFY` switch **before the `NumOfKeys` case**: + +```cpp +kj::StringPtr KJ_STRINGIFY(AutogateKey key) { + switch (key) { + // ... existing cases ... + case AutogateKey::MY_NEW_FEATURE: + return "my-new-feature"_kj; + case AutogateKey::NumOfKeys: + KJ_FAIL_ASSERT("NumOfKeys should not be used in getName"); + } +} +``` + +Naming convention: `kebab-case` for the string name. This string is what appears in runtime configuration (prefixed with `workerd-autogate-`). The enum name and string name should match to avoid confusion. + +### Step 3: Guard your code + +Use `Autogate::isEnabled()` to conditionally execute the new code path: + +```cpp +#include + +// At the point where behavior should change: +if (util::Autogate::isEnabled(util::AutogateKey::MY_NEW_FEATURE)) { + // New code path +} else { + // Old code path (keep until gate is removed) +} +``` + +### Step 4: Test + +Three ways to test autogated code: + +**A. The `@all-autogates` test variant** (automatic): + +Every `wd_test()` and `kj_test()` generates a `@all-autogates` variant that enables all gates. If your feature is tested by existing tests, they'll automatically run with the gate enabled: + +```bash +just stream-test //src/workerd/api/tests:my-test@all-autogates +``` + +**B. Targeted C++ test setup**: + +In a C++ test file, enable specific gates: + +```cpp +#include + +// In test setup: +util::Autogate::initAutogateNamesForTest({"my-new-feature"_kj}); + +// In test teardown: +util::Autogate::deinitAutogate(); +``` + +**C. Environment variable**: + +Set `WORKERD_ALL_AUTOGATES=1` to enable all gates when no explicit config is provided. + +### Step 5: Build and verify + +```bash +just build +just stream-test //path/to:my-test@ # Old behavior (gate off) +just stream-test //path/to:my-test@all-autogates # New behavior (gate on) +``` + +### Step 6: Remove the gate (after rollout) + +Once the human user explicitly confirms that the feature is stable and fully rolled out: + +1. Remove the `AutogateKey` enum value from `autogate.h` +2. Remove the `case` from `KJ_STRINGIFY` in `autogate.c++` +3. Remove all `Autogate::isEnabled()` checks, keeping only the new code path + +### Checklist + +- [ ] Enum value added to `AutogateKey` in `autogate.h` (before `NumOfKeys`) +- [ ] Comment describes what the gate controls +- [ ] String mapping added to `KJ_STRINGIFY` in `autogate.c++` +- [ ] Code guarded with `Autogate::isEnabled()` +- [ ] Old code path preserved (for rollback) +- [ ] `@all-autogates` test variant passes +- [ ] Tests cover both gated and ungated paths + +### Files touched + +| File | What to do | +| ------------------------------- | --------------------------------------- | +| `src/workerd/util/autogate.h` | Add enum value with comment | +| `src/workerd/util/autogate.c++` | Add case to `KJ_STRINGIFY` | +| Your feature file(s) | Guard code with `Autogate::isEnabled()` | diff --git a/.opencode/skills/add-compat-flag/SKILL.md b/.opencode/skills/add-compat-flag/SKILL.md new file mode 100644 index 00000000000..c70c52f4933 --- /dev/null +++ b/.opencode/skills/add-compat-flag/SKILL.md @@ -0,0 +1,143 @@ +--- +name: add-compat-flag +description: Step-by-step guide for adding a new compatibility flag to workerd, including capnp schema, C++ usage, testing, and documentation requirements. +--- + +## Adding a Compatibility Flag + +Compatibility flags control behavioral changes in workerd. They allow breaking changes to be rolled out gradually using compatibility dates. Follow these steps in order. + +### Step 1: Choose flag names + +Every flag needs: + +- **Enable flag**: Opts in to the new behavior (e.g., `text_decoder_replace_surrogates`) +- **Disable flag**: Opts out after it becomes default (e.g., `disable_text_decoder_replace_surrogates`). Only needed if the flag will eventually become default for all workers. + +Naming conventions: + +- Use `snake_case` +- Enable flag describes the new behavior positively +- Disable flag uses a `no_` or `disable_` prefix, or describes the old behavior + +### Step 2: Add to `compatibility-date.capnp` + +Edit `src/workerd/io/compatibility-date.capnp`. Add a new field at the end of the `CompatibilityFlags` struct. + +```capnp + myNewBehavior @163 :Bool + $compatEnableFlag("my_new_behavior") + $compatDisableFlag("no_my_new_behavior") + $compatEnableDate("2026-03-15"); + # Description of what this flag changes and why. + # Include context about the old behavior and what the new behavior fixes. +``` + +Key points: + +- The field number (`@163`) must be the next sequential number. Check the last field in the file. +- The field name is `camelCase` and becomes the C++ getter name (e.g., `getMyNewBehavior()`). +- `$compatEnableDate` is the date after which new workers get this behavior by default. Set this to a future date. If the flag is not yet ready for a default date, omit `$compatEnableDate` — the flag will only activate when explicitly listed in `compatibilityFlags`. +- Add `$experimental` annotation if the feature is experimental and should require `--experimental` to use. +- The comment block is required and serves as internal documentation. + +Available annotations: +| Annotation | Purpose | +|---|---| +| `$compatEnableFlag("name")` | Flag name to enable the behavior | +| `$compatDisableFlag("name")` | Flag name to disable after it's default | +| `$compatEnableDate("YYYY-MM-DD")` | Date after which behavior is default | +| `$compatEnableAllDates` | Force-enable for all dates (rare, breaks back-compat) | +| `$experimental` | Requires `--experimental` flag to use | +| `$neededByFl` | Must be propagated to Cloudflare's FL proxy layer | +| `$impliedByAfterDate(name = "otherFlag", date = "YYYY-MM-DD")` | Implied by another flag after a date | + +### Step 3: Use the flag in C++ code + +Access the flag via the auto-generated getter: + +```cpp +// In code that has access to jsg::Lock: +if (FeatureFlags::get(js).getMyNewBehavior()) { + // New behavior +} else { + // Old behavior +} +``` + +The `FeatureFlags` class is defined in `src/workerd/io/features.h`. The getter name is derived from the capnp field name with a `get` prefix and the first letter capitalized. + +For JSG API classes, you can also access flags in `JSG_RESOURCE_TYPE`: + +```cpp +JSG_RESOURCE_TYPE(MyApi, workerd::CompatibilityFlags::Reader flags) { + if (flags.getMyNewBehavior()) { + JSG_METHOD(newMethod); + } +} +``` + +### Step 4: Add tests + +Test both the old and new behavior. The test variant system helps: + +- **`test-name@`** runs with the oldest compat date (2000-01-01) — tests old behavior +- **`test-name@all-compat-flags`** runs with the newest compat date (2999-12-31) — tests new behavior + +In your `.wd-test` file, you can explicitly set the flag: + +```capnp +const unitTests :Workerd.Config = ( + services = [( + name = "my-test", + worker = ( + modules = [(name = "worker", esModule = embed "my-test.js")], + compatibilityFlags = ["my_new_behavior"], + ), + )], +); +``` + +For tests, the `compatibilityDate` field should not be included. + +Write test cases that verify both behaviors. Consider edge cases where the flag changes observable behavior. + +### Step 5: Document the flag + +**This is required before the enable date.** + +1. Create a PR in the [cloudflare-docs](https://github.com/cloudflare/cloudflare-docs) repository. +2. Add a markdown file under `src/content/compatibility-flags/` describing: + - What the flag does + - When it becomes default + - How to opt in or opt out + - Migration guidance if applicable + +See `docs/api-updates.md` for more details on the documentation process. + +### Step 6: Build and verify + +```bash +# Build to verify the capnp schema compiles +just build + +# Run the specific test +just stream-test //src/workerd/api/tests:my-test@ + +# Run with all compat flags to test the new behavior +just stream-test //src/workerd/api/tests:my-test@all-compat-flags + +# Run the compatibility-date test to verify flag registration +just stream-test //src/workerd/io:compatibility-date-test@ +``` + +### Checklist + +- [ ] Flag added to `compatibility-date.capnp` with correct sequential field number +- [ ] Enable and disable flag names follow naming conventions +- [ ] Comment block describes old behavior, new behavior, and rationale +- [ ] Enable date is set (or intentionally omitted for experimental/unreleased flags) +- [ ] C++ code uses `FeatureFlags::get(js).getMyNewBehavior()` to branch on the flag +- [ ] Tests cover both old and new behavior +- [ ] Documentation PR created in cloudflare-docs (required before enable date) +- [ ] `compatibility-date-test` passes diff --git a/.opencode/skills/commit-categories/SKILL.md b/.opencode/skills/commit-categories/SKILL.md new file mode 100644 index 00000000000..da8e9a8dba4 --- /dev/null +++ b/.opencode/skills/commit-categories/SKILL.md @@ -0,0 +1,44 @@ +--- +name: commit-categories +description: Commit categorization rules for changelogs and "what's new" summaries. MUST be loaded before categorizing commits in changelog or whats-new commands. Provides the canonical path-based category table used to group commits by area. +--- + +# Commit Categories + +Categorize commits by the files they touch, using the primary area for commits spanning multiple categories. + +| Category | Path patterns | +| ------------------- | ----------------------------------------------------- | +| **API** | `src/workerd/api/` (excluding `node/` and `pyodide/`) | +| **Node.js compat** | `src/workerd/api/node/`, `src/node/` | +| **Python** | `src/workerd/api/pyodide/`, `src/pyodide/` | +| **Rust** | `src/rust/` | +| **Cloudflare APIs** | `src/cloudflare/` | +| **I/O** | `src/workerd/io/` | +| **JSG** | `src/workerd/jsg/` | +| **Server** | `src/workerd/server/` | +| **Build** | `build/`, `MODULE.bazel`, `BUILD.bazel` | +| **Types** | `types/` | +| **Docs / Config** | Documentation, agent/tool configs, `.md` files | +| **Tests** | Changes exclusively in test files | +| **Other** | Anything that doesn't fit above | + +## Cross-cutting callouts + +These are **not** primary categories — they are additional callout sections that appear alongside the main categories whenever a commit touches the relevant files. A single commit can appear in both a primary category and one or more callouts. + +| Callout | Trigger | +| ---------------------------- | --------------------------------------------------------------------------------------------------- | +| **New/Updated Compat Flags** | Changes to `src/workerd/io/compatibility-date.capnp` or new `compatibilityFlags` references in code | +| **New/Updated Autogates** | Changes to `src/workerd/io/supported-autogates.h` or new autogate registrations | + +When either callout applies, add a dedicated section **after** the main categories listing each new or modified flag/gate with a brief description. These must never be buried inside a general category bullet — they are high-visibility items that reviewers and release-note readers need to spot immediately. + +## How to categorize + +1. For each commit, run `git diff-tree --no-commit-id --name-only -r ` to list files changed. +2. Match changed files against the path patterns above. +3. Assign the commit to whichever category covers the majority of its changes. +4. For commits touching multiple areas, list under the **primary** area (the one with the most changed files or the most significant change). +5. Check every commit against the cross-cutting callout triggers. If a commit adds or modifies compat flags or autogates, note it in the corresponding callout section **in addition to** its primary category. +6. Omit empty categories and unused callout sections from output. diff --git a/.opencode/skills/dad-jokes/SKILL.md b/.opencode/skills/dad-jokes/SKILL.md new file mode 100644 index 00000000000..784d205aaec --- /dev/null +++ b/.opencode/skills/dad-jokes/SKILL.md @@ -0,0 +1,30 @@ +--- +name: dad-jokes +description: After completing any task that took more than ~5 tool calls, or after long-running builds/tests finish, load this skill and deliver a dad joke to lighten the mood. Also load before any user-requested joke, pun, or limerick. Never improvise jokes without loading this skill first. +--- + +# Dad Jokes + +## When to fire + +After completing a long-running task (build, test suite, multi-step investigation, large refactor), drop a single dad joke, pun, or limerick before moving on. Not every task — roughly once every 20-30 minutes of sustained work, or after a particularly grueling debug session. Use your judgment. If the mood is tense (production incident, urgent fix), skip it. + +## Rules + +- **Always make it clear it's a joke.** Start with "Here's a dad joke for you:" or "Time for a pun!" or "Limerick incoming!" +- **One joke only.** Do not become a comedy set. One line, then back to work. +- **Always safe for work.** No exceptions. +- **Draw from context.** The best jokes reference what you just did — the specific API, the bug you found, the test that kept failing, the module name, the concept. Generic programming jokes are a fallback, not the goal. +- **Keep it short.** One-liners and two-line setups preferred. Limericks are acceptable but are the upper bound on length. +- **Do not explain the joke.** If it needs explaining, it wasn't good enough. Move on. +- **Do not ask if the user wants a joke.** Just do it. They can tell you to stop if they want. +- **Variety.** Rotate between dad jokes (Q&A format), puns (inline), and limericks. Don't repeat a format three times in a row. + +## Inspiration sources + +- KJ/Cap'n Proto concepts: promises, fulfillment, pipelines, capabilities, orphans +- workerd concepts: isolates, bindings, compatibility flags, Durable Objects, alarms, hibernation, jsg, apis, streams +- Build system: bazel, compilation, linking, caching, sandboxing +- Debugging: assertions, stack traces, serialization, autogates, dead code paths +- General runtime: workers, events, streams, tails, traces, pipelines, sharding +- Parent project context diff --git a/.opencode/skills/find-and-run-tests/SKILL.md b/.opencode/skills/find-and-run-tests/SKILL.md new file mode 100644 index 00000000000..a436e9213d3 --- /dev/null +++ b/.opencode/skills/find-and-run-tests/SKILL.md @@ -0,0 +1,70 @@ +--- +name: find-and-run-tests +description: How to find, build, and run tests in workerd. Covers wd-test, kj_test target naming, bazel query patterns, and common flags. Also covers parent project integration tests if workerd is used as a submodule. Load this skill when you need to locate or run a test and aren't sure of the exact target name or invocation. +--- + +# Finding and Running Tests + +## workerd Tests + +### Test types + +| Type | File extension | BUILD macro | Target suffix | +| ----------------- | -------------- | ----------- | ------------------------------ | +| JS/TS integration | `.wd-test` | `wd_test()` | None (target name = rule name) | +| C++ unit | `*-test.c++` | `kj_test()` | None | + +### Finding targets + +```bash +# Find test targets in a directory +bazel query 'kind("test", //src/workerd/api/tests:*)' --output label + +# Find test targets matching a name +bazel query 'kind(".*_test", //src/workerd/...)' --output label 2>/dev/null | grep -i '' + +# Find tests that depend on a source file +bazel query 'rdeps(//src/..., //src/workerd/io:trace-stream, 1)' --output label 2>/dev/null | grep -i test +``` + +### Running + +```bash +# Stream test output (preferred for debugging) +bazel test //src/workerd/api/tests:url-test --test_output=streamed + +# Run with fresh results (no cache) +bazel test //target --test_output=streamed --nocache_test_results + +# Run specific test case within a kj_test +bazel test //target --test_arg='-f' --test_arg='test case name' +``` + +### Common flags + +| Flag | Purpose | +| ------------------------ | ------------------------------------------- | +| `--test_output=streamed` | Stream test output to terminal in real time | +| `--nocache_test_results` | Force re-run, don't use cached results | +| `--test_timeout=120` | Override default test timeout (seconds) | + +--- + +## Parent Project Integration Tests + +If workerd is used as a submodule in a parent project, that project may have its own integration test framework with different conventions. Load the `parent-project-skills` skill to discover those conventions. + +### General principles that apply to any integration test framework + +**Target naming with variant suffixes.** Some test macros generate multiple targets from a single source file by appending variant suffixes (e.g., `@`, `@all-autogates`, `@force-sharding`). If bazel says "is a source file, nothing will be built" or "No test targets were found", you likely need a suffix. Use `bazel query 'kind("test", //path:*)'` to discover the actual runnable target names. + +**Cached results hide changes.** Always use `--nocache_test_results` when re-running after modifying test files or source code. Without it, bazel returns stale cached results with stale logs. + +**Verify the feature actually ran.** After a test passes, search the test output for feature-specific evidence (script names, process types, subrequests, RPC calls). A passing test with no evidence the feature ran is not a valid test — see the `test-driven-investigation` skill. + +### Debugging test failures + +1. **Always use `--nocache_test_results`** when re-running after changes. +2. **Check test logs** at the path shown in bazel output: `bazel-out/.../testlogs/.../test.log` +3. **Search logs for feature-specific keywords** to verify the feature actually ran. +4. **Subrequest mismatches** (in frameworks that verify subrequests) typically show the actual vs expected request details — compare control headers carefully. diff --git a/.opencode/skills/investigation-notes/SKILL.md b/.opencode/skills/investigation-notes/SKILL.md new file mode 100644 index 00000000000..1a090c5f7eb --- /dev/null +++ b/.opencode/skills/investigation-notes/SKILL.md @@ -0,0 +1,135 @@ +--- +name: investigation-notes +description: Structured scratch document for tracking investigation state during bug hunts - prevents re-reading code, losing context, and rabbit holes; maintains external memory so you don't re-derive conclusions +--- + +# Investigation Notes + +## Overview + +During bug investigations, maintain a lightweight scratch document as external memory. This prevents re-reading code you've already analyzed, losing track of which hypothesis you're testing, and silently drifting into rabbit holes. + +**Core principle:** Write it down once, refer to it later. Re-reading your one-line note is faster than re-reading 200 lines of source. + +## The Document + +Create `~/tmp/investigate-.md` during orientation (step 2 of `/investigate`). The short name should be descriptive enough to identify the investigation (e.g., `investigate-concurrent-write.md`, `investigate-pipe-zombie-state.md`). + +**Do not** create the document preemptively for every investigation. Refer to the rules below for when to create it. + +### Format + +```markdown +# Investigation: + +## Error + + + +## Current Focus + + + +## Hypotheses + +1. [TESTING] — test: +2. [REJECTED] — disproved by: +3. [CONFIRMED] — evidence: + +## Code Read + +- `file:line-range` — + +## Tests + +- `file:line` "test name" — + +## Ruled Out + +- + +## Next + +1. +2. +``` + +## Rules + +### Creation + +**Create the document when you notice any of these:** + +- You have more than one hypothesis to track +- You've read more than 3 files/functions and need to remember what you learned +- You're about to re-read code you already read +- The investigation is going to span multiple iterations of test-write-run + +When you do create it, populate Error, Current Focus, your current hypotheses, and anything you've already learned (backfill "Code Read" and "Tests" from what you've done so far). + +### Updates + +Update the document after each significant action: + +- After reading a function or file section → add to "Code Read" +- After forming or rejecting a hypothesis → update "Hypotheses" +- **After running a test → update BEFORE doing anything else.** Record: what test ran, what the result was, what it means for the active hypothesis. This is non-negotiable — it takes 30 seconds and prevents the pattern of running multiple tests, losing track of what they proved, and falling back to code reading. +- After starting a new thread of work → update "Current Focus" + +Do NOT update after every tool call. Do NOT polish or reorganize. Append and move on. + +### Entries + +Every entry is **1-2 sentences max**. Use `file:line` references, not code dumps. If you're writing a paragraph, you're procrastinating. + +You may include brief, simple diagrams if they help you understand and retain information. + +You may write a wrong hypothesis or a test that doesn't reproduce the bug. That's valuable. + +The point is to externalize your thinking and get feedback from the code, not to be right on the first try. + +### Before Re-Reading Code + +**Check "Code Read" first.** If the file and line range are already listed, use your note. You already extracted what you needed. If the note isn't sufficient, that's a sign the note was too vague — read the code, then write a better note. Do not make a habit of this. + +### Current Focus + +Update "Current Focus" before starting any new thread of work. If your current focus doesn't relate to a hypothesis in the list, you've drifted. Either add a hypothesis for what you're doing or stop and return to the active one. + +### Hypothesis Limits + +- **Maximum 3 hypotheses total.** If you want to add a fourth, reject or merge one first. +- **Maximum 1 `[TESTING]` at a time.** Commit to one, test it, resolve it, then move on. +- **Every hypothesis must have a test or a concrete plan to write one.** "Need to investigate further" is not a valid state — that's analysis paralysis wearing a label. + +Valid statuses: + +- `[UNTESTED]` — formed but not yet tested. Must become `[TESTING]` or `[REJECTED]` soon. +- `[TESTING]` — actively being tested. Only one at a time. +- `[CONFIRMED]` — test reproduced the bug as predicted. +- `[REJECTED]` — test disproved it, or evidence rules it out. Include why. +- `[SUPERSEDED]` — replaced by a more specific hypothesis. Reference the replacement. + +### Priority + +**Maintaining the document never takes priority over writing or running a test.** If you're choosing between updating notes and writing a test, write the test. Update the notes after. + +The document is scratch paper. It exists to serve the investigation, not to document it. + +Delete the document when the investigation is over. + +The human will tell you when the investigation is complete. + +## Anti-Patterns + +| You're doing this | Do this instead | +| ------------------------------------------------------------------------------ | --------------------------------------------------------- | +| Writing multi-line entries | One sentence. Use `file:line`. | +| Dumping code into the doc | Reference it: `file:line` — "does X" | +| Reorganizing or reformatting the doc | Append and move on | +| Updating notes instead of writing a test | Write the test first | +| 3 `[UNTESTED]` hypotheses and no tests | Pick one, write a test, run it | +| Re-reading a file listed in "Code Read" | Use your note | +| "Current Focus" hasn't changed in a while but you're doing something different | You drifted. Update it or go back. | +| You've run 2+ tests without updating the document | You're accumulating conclusions in your head. Update now. | +| The doc is longer than ~50 lines | You're over-documenting. Trim or start fresh. | diff --git a/.opencode/skills/kj-style/SKILL.md b/.opencode/skills/kj-style/SKILL.md new file mode 100644 index 00000000000..df8fa096102 --- /dev/null +++ b/.opencode/skills/kj-style/SKILL.md @@ -0,0 +1,186 @@ +--- +name: kj-style +description: KJ/workerd C++ style guidelines for code review. Covers naming, type usage, memory management, error handling, inheritance, constness, and formatting conventions. Load this skill when reviewing or writing C++ code in the workerd codebase. +--- + +## KJ Style Guide — Code Review Reference + +Full guide: https://github.com/capnproto/capnproto/blob/v2/kjdoc/style-guide.md +Full API tour: https://github.com/capnproto/capnproto/blob/v2/kjdoc/tour.md + +The KJ style guide is self-described as suggestions, not rules. Consistency matters, but +pragmatic exceptions are fine. + +--- + +### MANDATORY: Load Reference Files When Relevant + +This skill is split across multiple files for context efficiency. The core rules below cover +the most common patterns. Detailed examples and code patterns live in reference files. + +**You MUST read the relevant reference files before writing or reviewing code that touches +their subject matter. Do not rely on memory or general knowledge — the reference files contain +project-specific patterns and idioms that override general C++ conventions. Skipping a relevant +reference file WILL lead to incorrect suggestions.** + +| File | MUST load when... | +| ------------------------------- | ----------------------------------------------------------------------- | +| `reference/api-patterns.md` | Code uses or should use `kj::Maybe`, `kj::OneOf`, `kj::str()`, | +| | `KJ_DEFER`, `KJ_SYSCALL`, `kj::downcast`, or KJ iteration helpers | +| `reference/async-patterns.md` | Code involves `kj::Promise`, `.then()`, `.attach()`, | +| | `.eagerlyEvaluate()`, coroutines, `kj::MutexGuarded`, or `kj::TaskSet` | +| `reference/type-design.md` | Designing new classes, reviewing class hierarchies, analyzing constness | +| | or thread safety semantics, or deciding value-type vs resource-type | +| `reference/review-checklist.md` | Performing ANY code review of workerd C++ code | + +When performing a code review, ALWAYS load `reference/review-checklist.md`. When in doubt +about whether a reference file is relevant, load it — the cost of reading is far less than +the cost of an incorrect review comment. + +--- + +### KJ Types over STL + +The project uses KJ types instead of the C++ standard library: + +| Instead of | Use | +| ----------------------- | --------------------------------------------------- | +| `std::string` | `kj::String` (owned) / `kj::StringPtr` (borrowed) | +| `std::vector` | `kj::Array` (fixed) / `kj::Vector` (growable) | +| `std::unique_ptr` | `kj::Own` | +| `std::shared_ptr` | `kj::Rc` / `kj::Arc` (thread-safe) | +| `std::optional` | `kj::Maybe` | +| `std::function` | `kj::Function` | +| `std::variant` | `kj::OneOf` | +| `std::span` / array ref | `kj::ArrayPtr` | +| `std::exception` | `kj::Exception` | +| `std::promise`/`future` | `kj::Promise` / `kj::ForkedPromise` | +| `T*` (nullable) | `kj::Maybe` | + +Avoid including ``, ``, ``, ``, ``, ``, +or other std headers from header files. Source-file-only use of std is acceptable when KJ has no equivalent. + +Never use `FILE*`, `iostream`, or C stdio. Use `kj::str()` for formatting, `KJ_DBG()` for debug +printing, and `kj/io.h` for I/O. + +`KJ_DBG` statements must never be left in committed code. + +### Memory Management + +**Never write `new` or `delete`**. Use: + +- `kj::heap(args...)` → returns `kj::Own` +- `kj::heapArray(size)` → returns `kj::Array` +- `kj::heapArrayBuilder(size)` → build arrays element-by-element + +Only heap-allocate when you need moveability. Prefer stack or member variables. + +If a type would need copy constructors that allocate, provide an explicit `clone()` method +and delete the copy constructor instead. + +### Ownership Model + +- Every object has exactly **one owner** (another object or a stack frame) +- `kj::Own` = owned pointer (transfers ownership when moved) +- Raw C++ pointers and references = **not owned**, borrowing only +- An object can **never own itself**, even transitively (no reference cycles) +- Default lifetime rules for raw pointer/reference parameters: + - Constructor param → valid for lifetime of the constructed object + - Function/method param → valid until function returns (or returned promise completes) + - Method return → valid until the object it was called on is destroyed + +**Reference counting** (`kj::Refcounted`/`kj::addRef`) is allowed. If used: + +- Avoid cycles (memory leaks) +- Prefer non-atomic refcounting; atomic refcounting (`kj::AtomicRefcounted`) is extremely slow +- `kj::AtomicRefcounted` is only appropriate for objects that whose refcounting needs to occur across threads. +- Prefer using `kj::Rc`/`kj::rc(...)` and `kj::Arc`/`kj::arc(...)` instead of using `kj::refcounted`, `kj::atomicRefcounted`, and `kj::addRef()` directly. + +### Error Handling + +**Never use `throw` directly**. Use KJ assertion macros from `kj/debug.h`: + +- `KJ_ASSERT(cond, msg, values...)` — checks invariants (bug in this code) +- `KJ_REQUIRE(cond, msg, values...)` — checks preconditions (bug in caller) +- `KJ_FAIL_ASSERT(msg, values...)` — unconditional failure +- `KJ_SYSCALL(expr, msg, values...)` — wraps C syscalls, checks return values +- `KJ_UNREACHABLE` — marks unreachable code +- `kj::throwFatalError(msg, values...)` — throws an exception with a stack trace + +These macros automatically capture file/line, stringify operands, and generate stack traces. + +**Never declare anything `noexcept`**. Bugs can happen anywhere; aborting is never correct. + +**Explicit destructors must be `noexcept(false)`**. Use `kj::UnwindDetector` or recovery blocks +in destructors to handle the unwinding-during-unwind problem. + +**Exceptions are for fault tolerance**, not control flow. They represent things that "should never +happen" — bugs, network failures, resource exhaustion. If callers need to catch an exception +to operate correctly, the interface is wrong. Offer alternatives like `openIfExists()` returning +`kj::Maybe`. + +### RAII + +All cleanup must happen in destructors. Use `KJ_DEFER(code)` for scope-exit cleanup. +A destructor should perform **no more than one cleanup action** — use multiple members with +their own destructors so that if one throws, the others still run. + +### Naming Conventions + +| Kind | Style | +| ----------------------------- | ---------------------------------------------------------------- | +| Types (classes, structs) | `TitleCase` | +| Variables, functions, methods | `camelCase` | +| Constants, enumerants | `CAPITAL_WITH_UNDERSCORES` | +| Macros | `CAPITAL_WITH_UNDERSCORES` with project prefix (`KJ_`, `CAPNP_`) | +| Namespaces | `oneword` (keep short); private namespace: `_` | +| Files | `module-name.c++`, `module-name.h`, `module-name-test.c++` | + +### Lambda Capture Rules + +- **Never** use `[=]` (capture-all-by-value) — makes lifetime analysis impossible during review +- **May** use `[&]` (capture-all-by-reference) but **only** when the lambda will not outlive the + current stack frame. Using `[&]` signals "this lambda doesn't escape." +- For escaping lambdas, capture each variable explicitly. +- Use `kj::coCapture(...)` to capture variables in a way that extends their lifetime for the duration of the promise. + +### No Singletons + +Never use mutable globals or global registries. The `main()` function or high-level code should +explicitly construct components and wire dependencies via constructor parameters. + +### Lazy Input Validation + +Validate data at time-of-use, not upfront. Upfront validation creates false confidence downstream, +gets out of sync with usage code, and wastes cycles on unused fields. + +### Text Encoding + +All text is UTF-8. Do not assume text is _valid_ UTF-8 — be tolerant of malformed sequences +(lazy validation principle). ASCII-range bytes in UTF-8 are always single-byte, so parsers for +machine-readable formats can safely operate byte-by-byte. + +### Formatting + +- **2-space indents**, never tabs, max 100 chars/line +- Continuation lines: 4-space indent (or align with opening delimiter) +- Space after keywords: `if (foo)`, `for (...)`, `while (...)` +- No space after function names: `foo(bar)` +- Opening brace at end of statement; closing brace on its own line +- Always use braces for blocks, unless the entire statement fits on one line +- `public:`/`private:`/`protected:` reverse-indented by one stop from class body +- Namespace contents are NOT indented +- Strip trailing whitespace + +### Comments + +Workerd follows these convention over KJ's own comment style: + +- **Always use `//` line comments**, never `/* */` block comments +- Doc comments go **before** the declaration following common C++ conventions. +- Don't state the obvious — comments should add information not evident from the code +- TODO format: `// TODO(type): description` where type is: `now`, `soon`, `someday`, `perf`, + `security`, `cleanup`, `port`, `test` +- `TODO(now)` comments must be addressed before merging. +- `TODO({project})` type comments where `{project}` is the name of an active project are acceptable + for work that is carried out over multiple pull requests. diff --git a/.opencode/skills/kj-style/reference/api-patterns.md b/.opencode/skills/kj-style/reference/api-patterns.md new file mode 100644 index 00000000000..a148a3d2c90 --- /dev/null +++ b/.opencode/skills/kj-style/reference/api-patterns.md @@ -0,0 +1,101 @@ +# Idiomatic KJ API Patterns + +Detailed code examples for common KJ API usage patterns in workerd. + +--- + +### Unwrapping `kj::Maybe` + +Always use `KJ_IF_SOME`, never dereference directly: + +```cpp +// Correct: +KJ_IF_SOME(value, maybeValue) { + use(value); // value is a reference to the contained T +} + +// Correct: +auto& value = KJ_ASSERT_NONNULL(maybeValue); // asserts if maybeValue is none, otherwise gives T& +auto& value = KJ_REQUIRE_NONNULL(maybeValue); // same but for preconditions +auto& value = JSG_REQUIRE_NONNULL(maybeValue, ErrorType, "message"); // same but with JavaScript exception type/message + +// Wrong +auto& value = *maybeValue; // doesn't exist +auto& value = maybeValue.value(); // not how KJ works +``` + +`maybe.map(fn)` and `maybe.orDefault(val)` are useful for simple transforms/fallbacks. + +When moving a value out of a `kj::Maybe`, use `kj::mv()` and remember to set the `kj::Maybe` to `kj::none` to avoid dangling references: + +```cpp +KJ_IF_SOME(value, maybeValue) { + auto movedValue = kj::mv(value); // move out of the Maybe + maybeValue = kj::none; // set Maybe to none to avoid dangling reference + use(movedValue); +} +``` + +### Unwrapping `kj::OneOf` + +Always use `KJ_SWITCH_ONEOF` / `KJ_CASE_ONEOF`: + +```cpp +KJ_SWITCH_ONEOF(variant) { + KJ_CASE_ONEOF(s, kj::String) { handleString(s); } + KJ_CASE_ONEOF(i, int) { handleInt(i); } +} +``` + +### Building Strings + +Use `kj::str()`, never `std::to_string` or `+` concatenation: + +```cpp +kj::String msg = kj::str("count: ", count, ", name: ", name); +``` + +Use `kj::hex(n)` for hex output. Extend with `KJ_STRINGIFY(MyType)` or a `.toString()` method. +Use `"literal"_kj` suffix for `kj::StringPtr` literals (can be `constexpr`). + +### Scope-Exit Cleanup + +Use `KJ_DEFER` or `kj::defer()`: + +```cpp +KJ_DEFER(close(fd)); // block scope +auto cleanup = kj::defer([&]() { close(fd); }); // movable, for non-block lifetimes +``` + +Also: `KJ_ON_SCOPE_SUCCESS(...)` and `KJ_ON_SCOPE_FAILURE(...)` for conditional cleanup. + +### Syscall Error Checking + +Use `KJ_SYSCALL`, never manual errno checks: + +```cpp +int n; +KJ_SYSCALL(n = read(fd, buf, size)); // throws on error, retries EINTR +KJ_SYSCALL_HANDLE_ERRORS(fd = open(path, O_RDONLY)) { + case ENOENT: return nullptr; // handle specific errors + default: KJ_FAIL_SYSCALL("open()", error); +} +``` + +### Downcasting + +Use `kj::downcast` (debug-checked), not `static_cast` or `dynamic_cast`: + +```cpp +auto& derived = kj::downcast(baseRef); // asserts in debug builds +``` + +Use `kj::dynamicDowncastIfAvailable` only for optimizations (returns null without RTTI). + +### Iteration Helpers + +```cpp +for (auto i: kj::zeroTo(n)) { ... } // 0..n-1 +for (auto i: kj::range(a, b)) { ... } // a..b-1 +for (auto i: kj::indices(array)) { ... } // 0..array.size()-1 +``` diff --git a/.opencode/skills/kj-style/reference/async-patterns.md b/.opencode/skills/kj-style/reference/async-patterns.md new file mode 100644 index 00000000000..3b68ec9fda4 --- /dev/null +++ b/.opencode/skills/kj-style/reference/async-patterns.md @@ -0,0 +1,97 @@ +# Async & Concurrency Patterns + +Promise lifetime management, background tasks, cancellation, and mutex usage in workerd. + +--- + +### Promise Patterns + +**`.attach()` for lifetime management** — objects must outlive the promise that uses them: + +```cpp +// Correct: stream stays alive until readAllText() completes +return stream->readAllText().attach(kj::mv(stream)); + +// Wrong: stream destroyed immediately, promise has dangling reference +auto promise = stream->readAllText(); +return promise; // stream is gone +``` + +**`.eagerlyEvaluate()` for background tasks** — without it, continuations are lazy +and may never run: + +```cpp +promise = doWork().then([]() { + KJ_LOG(INFO, "done"); // won't run unless someone .wait()s or .then()s +}).eagerlyEvaluate([](kj::Exception&& e) { + KJ_LOG(ERROR, e); // error handler is required +}); +``` + +When using coroutines, `eagerlyEvaluate()` is implied and not needed to be called explicitly. + +Use `kj::TaskSet` to manage many background tasks with a shared error handler. + +**Cancellation** — destroying a `kj::Promise` cancels it immediately. No continuations +run, only destructors. Use `.attach(kj::defer(...))` for cleanup that must happen +on both completion and cancellation. + +**`kj::evalNow()`** — wraps synchronous code to catch exceptions as rejected promises: + +```cpp +return kj::evalNow([&]() { + // Any throw here becomes a rejected promise, not a synchronous exception + return doSomethingThatMightThrow(); +}); +``` + +--- + +### Mutex Patterns + +`kj::MutexGuarded` ties locking to access — you can't touch the data without a lock: + +```cpp +// Exclusive access for modification +{ + auto lock = guarded.lockExclusive(); + lock->modify(); + // lock is released at end of scope +} + +// Multiple readers ok +{ + auto shared = guarded.lockShared(); + shared->read(); + // shared lock is released at end of scope +} +``` + +`.wait(cond)` on a lock replaces condition variables: + +```cpp +auto lock = guarded.lockExclusive(); +lock.wait([](const T& val) { return val.ready; }); // releases/reacquires automatically +``` + +#### Correctly Holding Locks + +When using `kj::MutexGuarded`, ensure the lock is correctly held for the duration of any +access to the guarded data. + +Do this: + +```cpp +auto lock = mutexGuarded.lockExclusive(); +KJ_IF_SOME(value, lock->maybeValue) { + // Access value safely while lock is held. +} +``` + +Not this: + +```cpp +KJ_IF_SOME(value, mutexGuarded.lockExclusive()->maybeValue) { + // Unsafe! The lock is released before we access maybeValue. +} +``` diff --git a/.opencode/skills/kj-style/reference/review-checklist.md b/.opencode/skills/kj-style/reference/review-checklist.md new file mode 100644 index 00000000000..7db98b6376a --- /dev/null +++ b/.opencode/skills/kj-style/reference/review-checklist.md @@ -0,0 +1,32 @@ +# Code Review Checklist + +When reviewing workerd C++ code, check for each of these items. + +--- + +1. **STL leaking in**: `std::string`, `std::vector`, `std::optional`, `std::unique_ptr`, etc. +2. **Raw `new`/`delete`**: Should be `kj::heap()` or similar +3. **`throw` statements**: Should use `KJ_ASSERT`/`KJ_REQUIRE`/`KJ_FAIL_ASSERT` +4. **`noexcept` declarations**: Should not be present +5. **Destructor missing `noexcept(false)`**: Required on all explicit destructors +6. **`[=]` lambda captures**: Never allowed +7. **Nullable raw pointers (`T*`)**: Should be `kj::Maybe` +8. **Mixed interface/implementation classes**: No data members in interfaces, no non-final virtuals in implementations +9. **Singletons or mutable globals**: Not allowed +10. **Missing `.attach()` on promises**: Objects must stay alive for the promise duration +11. **Background promise without `.eagerlyEvaluate()`**: Lazy continuations may never execute +12. **Manual errno checks**: Should use `KJ_SYSCALL` / `KJ_SYSCALL_HANDLE_ERRORS` +13. **`static_cast` for downcasting**: Should be `kj::downcast` (debug-checked) +14. **`std::to_string` or `+` string concatenation**: Should be `kj::str()` +15. **`dynamic_cast` for dispatch**: Extend the interface instead +16. **`/* */` block comments**: Use `//` line comments +17. **Naming**: TitleCase types, camelCase functions/variables, CAPS constants +18. **Missing braces**: Required unless entire statement is on one line +19. **`bool` function parameter**: Prefer `enum class` or `WD_STRONG_BOOL` for clarity at call sites. E.g., `void connect(bool secure)` should be `void connect(SecureMode mode)`. +20. **Missing `[[nodiscard]]`**: Functions returning error codes, `kj::Maybe`, or success booleans that callers must check should be `[[nodiscard]]`. +21. **Promise chain where coroutine would be clearer**: Nested `.then()` chains with complex error handling that would be more readable as a coroutine with `co_await`. But avoid suggesting sweeping rewrites. +22. **Missing `constexpr` / `consteval`**: Compile-time evaluable functions or constants not marked accordingly. +23. **Reinvented utility**: Custom code duplicating functionality already in `src/workerd/util/` (e.g., custom ring buffer, small set, state machine, weak reference pattern). Check the util directory before suggesting a new abstraction. +24. **Missing `override`**: Virtual method overrides missing the `override` specifier. +25. **Direct `new`/`delete` (via `new` expression)**: Should use `kj::heap()`, `kj::heapArray()`, or other KJ memory utilities. +26. **Explicit `throw` statement**: Should use `KJ_ASSERT`, `KJ_REQUIRE`, `KJ_FAIL_ASSERT`, or `KJ_EXCEPTION` instead of bare `throw`. diff --git a/.opencode/skills/kj-style/reference/type-design.md b/.opencode/skills/kj-style/reference/type-design.md new file mode 100644 index 00000000000..74c16cff3a7 --- /dev/null +++ b/.opencode/skills/kj-style/reference/type-design.md @@ -0,0 +1,50 @@ +# Type Design, Inheritance & Thread Safety + +Class design rules, inheritance patterns, and constness semantics in workerd. + +--- + +### Two Kinds of Types + +**Value types** (data): + +- Copyable/movable, compared by value, can be serialized +- No virtual methods; use templates for polymorphism +- Always have move constructors + +**Resource types** (objects with identity): + +- Not copyable, not movable — use `kj::Own` on the heap for ownership transfer +- Use `KJ_DISALLOW_COPY_AND_MOVE` to prevent accidental copying/moving +- Compared by identity, not value +- May use inheritance and virtual methods + +### Inheritance + +- A class is either an **interface** (no data members, only pure virtual methods) or an + **implementation** (no non-final virtual methods). Never mix — this causes fragile base class problems. +- Interfaces should **NOT** declare a destructor. +- Multiple inheritance is allowed and encouraged (typically inheriting multiple interfaces). +- Implementation inheritance is acceptable for composition without extra heap allocations. + +### Constness and Thread Safety + +- Treat constness as **transitive** — a const pointer to a struct means its contained pointers + are also effectively const. +- **`const` methods must be thread-safe** for concurrent calls. Non-const methods require + exclusive access. `kj::MutexGuarded` enforces this: `.lockShared()` returns `const T&`, + `.lockExclusive()` returns `T&`. +- Copyable classes with pointer members: declare copy constructor as `T(T& other)` (not + `T(const T& other)`) to prevent escalating transitively-const references. Or inherit + `kj::DisallowConstCopy`. +- Only hold locks for the minimum duration necessary to access or modify the guarded data. + +### Other Rules + +- **No global constructors**: Don't declare static/global variables with dynamic constructors. + Global `constexpr` constants are fine. +- **No `dynamic_cast` for polymorphism**: Don't use long if/else chains casting to derived types. + Extend the base interface instead. `dynamic_cast` is OK for optimizations or diagnostics + (test: if it always returned null, would the code still be correct?). +- **No function/method pointers**: Use templates over functors, or `kj::Function`. +- **Prefer references over pointers**: Unambiguously non-null. diff --git a/.opencode/skills/markdown-drafts/SKILL.md b/.opencode/skills/markdown-drafts/SKILL.md new file mode 100644 index 00000000000..1a315678b9d --- /dev/null +++ b/.opencode/skills/markdown-drafts/SKILL.md @@ -0,0 +1,71 @@ +--- +name: markdown-drafts +description: Use markdown formatting when drafting content intended for external systems (GitHub issues/PRs, Jira tickets, wiki pages, design docs, etc.) so formatting is preserved when the user copies it. Load this skill before producing any draft the user will paste elsewhere. +--- + +## Markdown Drafts + +When the user asks you to draft, write, or compose content that will be copied into an external +system — GitHub issues, pull request descriptions, Jira tickets, wiki pages, design documents, +RFCs, or similar — **always use markdown syntax** so that formatting survives the copy-paste. + +### When This Applies + +- GitHub issues and pull request descriptions +- Jira ticket descriptions and comments +- Confluence / wiki page drafts +- Design documents and RFCs +- Slack messages (Slack renders a subset of markdown) +- Any content the user explicitly says will be copied elsewhere + +### Formatting Rules + +- Use `#`, `##`, `###` headers to create structure +- Use `-` or `*` for unordered lists, `1.` for ordered lists +- Use triple-backtick fenced code blocks with language tags (e.g., ` ```cpp `) for code +- Use `inline code` backticks for identifiers, file paths, commands, and config values +- Use `**bold**` for emphasis on key points and `_italic_` for secondary emphasis +- Use markdown tables when presenting structured comparisons or data +- Use `> blockquotes` for callouts, quoted text, or important notes +- Use `[link text](url)` for references — never bare URLs in running prose +- Use `---` horizontal rules to separate major sections when appropriate +- Use task lists (`- [ ]` / `- [x]`) when drafting action items or checklists +- Keep line lengths reasonable (e.g., 80-120 characters) + +### Structure Guidelines + +- Start with a concise summary paragraph before diving into details +- Use headers to break content into scannable sections +- Keep paragraphs short — walls of text are hard to scan in issue trackers +- Put the most important information first (inverted pyramid) +- End with next steps, open questions, or action items when relevant + +### Rendering: Always Emit Raw Markdown + +The chat interface renders markdown, which strips the raw syntax (`###`, `**`, `` ` ``, etc.) from +the output. Since these drafts are meant to be **copied and pasted** into external systems, the user +needs the raw markup characters preserved. + +**Always wrap the entire draft in a fenced code block** so the chat interface displays it as literal +text. Use a plain triple-backtick fence (no language tag): + + ``` + ## My Heading + + - bullet one + - **bold text** and `code` + ``` + +This ensures the user sees and can copy the exact markdown source. Never render the draft as +formatted text outside a code fence — the markup will be silently consumed by the chat UI. + +### What NOT To Do + +- Do not render the draft as formatted markdown outside a code fence — the user will lose the syntax +- Do not use plain text formatting (e.g., ALL CAPS for headers, `====` underlines, manual indentation) +- Do not use HTML tags unless the target system requires them and markdown is insufficient +- Do not add emoji unless the user's draft style includes them or they explicitly request it +- Do not use common cliche AI-generated phrases, tropes, or filler content like tricolons, + or generic intros/outros. Be concise and to the point. +- Do not include editorial comments or unsubstantiated claims. Stick to the facts and the user's + instructions precisely. If you need to ask clarifying questions, do so instead of making assumptions. diff --git a/.opencode/skills/parent-project-skills/SKILL.md b/.opencode/skills/parent-project-skills/SKILL.md new file mode 100644 index 00000000000..5515b6e12a3 --- /dev/null +++ b/.opencode/skills/parent-project-skills/SKILL.md @@ -0,0 +1,49 @@ +--- +name: parent-project-skills +description: Bootstrap skill for discovering additional skills and context from a parent project when workerd is used as a submodule. Load this skill when tasks span project boundaries (e.g., Sentry/production investigation, integration testing, cross-repo debugging). +--- + +## Parent Project Skill Discovery + +This project (workerd) may be embedded as a submodule within a larger project. When your task requires context beyond workerd itself — such as investigating production issues, writing integration tests, or understanding deployment architecture — you should check for additional skills and context in the parent project. + +### Discovery procedure + +1. Determine if a parent project exists by checking for a git root above the workerd directory: + - Look for `../../.git` or `../../AGENTS.md` relative to the workerd root + - If found, the parent project root is `../../` (two levels up from workerd) + +2. Check for parent project skills: + - Look for `../../.opencode/skills/*/SKILL.md` + - Read any `../../AGENTS.md` or `../../CLAUDE.md` for project-wide context + +3. Load relevant skills by reading the SKILL.md files. These are not registered in the `skill` tool — read them directly with the Read tool. + +4. Check for submodule-specific context: + - Look for `../../.opencode/skills/*/SKILL.md` files that may reference workerd specifically + +### Information boundary — CRITICAL + +**workerd is an open-source project. The parent project is typically proprietary/internal.** + +You MUST enforce a strict one-way information boundary: + +- **ALLOWED**: Internal context informs your reasoning, helps you understand code paths, guides your investigation +- **NEVER**: Write internal details into workerd files — this includes: + - Code comments referencing internal systems, services, or architecture + - Commit messages mentioning internal projects, Sentry issues, Jira tickets, or internal URLs + - PR descriptions or GitHub issue comments containing internal context + - AGENTS.md, CLAUDE.md, or documentation updates with internal knowledge + - Variable names, error messages, or log strings that reveal internal details + - Test files that encode internal architecture assumptions + +When working across the boundary, frame all workerd-side artifacts in terms of workerd's own public concepts (Workers, Durable Objects, isolates, IoContext, etc.) without referencing how the parent project orchestrates them. + +### When to use this skill + +- Investigating production Sentry issues that involve workerd code +- Writing integration tests (e.g., ew-test) that live in the parent project +- Understanding how workerd APIs behave in the production deployment context +- Debugging issues that cross the workerd / parent-project boundary +- Reviewing code changes that affect both projects +- Planning new feature development that requires coordination between workerd and the parent project diff --git a/.opencode/skills/pr-review-guide/SKILL.md b/.opencode/skills/pr-review-guide/SKILL.md new file mode 100644 index 00000000000..c58329f25fe --- /dev/null +++ b/.opencode/skills/pr-review-guide/SKILL.md @@ -0,0 +1,69 @@ +--- +name: pr-review-guide +description: Guidelines for posting pull request review comments via GitHub CLI, including suggested edits format, handling unresolved comments, etiquette, and report/issue tracking. Load this skill when reviewing a PR via GitHub and posting inline comments. +--- + +## Pull Request Review Guide + +Load this skill when posting review comments on a GitHub pull request. + +--- + +### Posting Review Comments + +When asked to review a pull request, you may use the GitHub CLI tool to post inline comments on the PR with specific feedback for each issue you identify. Do not make code changes yourself, but you can suggest specific code changes in your comments. Be sure to reference specific lines of code in your comments for clarity. + +When providing feedback on a pull request, focus on actionable insights that can help improve the code. Be clear and concise in your comments, and provide specific examples or references to the code to support your feedback. Avoid vague statements and instead provide concrete suggestions for improvement. + +Review comments should be posted on individual lines of code in the pull request, never as a single monolithic comment. This allows for clearer communication and easier tracking of specific issues. + +### Suggested Edits + +When the fix for an issue is obvious and localized (e.g., a typo, a missing annotation, a wrong type, a simple rename), include a GitHub suggested edit block in your review comment so the author can apply it with one click. Use this format: + +```` +```suggestion +corrected line(s) of code here +``` +```` + +Guidelines for suggested edits: + +- **Do** use them for: typos, missing `override`/`[[nodiscard]]`/`constexpr`, wrong types, simple renames, small bug fixes where the correct code is unambiguous. +- **Do not** use them for: large refactors, design changes, cases where multiple valid fixes exist, or anything requiring context the author should decide on. +- Keep suggestions minimal — change only the lines that need fixing. Do not reformat surrounding code. +- When a suggestion spans multiple lines, include all affected lines in the block. + +### Unresolved Review Comments + +When reviewing a PR, check prior review comments (from any reviewer) that have been marked as resolved. If the current code still exhibits the issue described in a resolved comment, flag it as a finding with a reference to the original comment. Use this format: + +- **[HIGH]** Previously flagged issue not addressed: _{original comment summary}_ + - **Location**: File and line references + - **Problem**: Review comment by {author} was marked resolved but the underlying issue remains in the current code. + - **Evidence**: Link to or quote the original comment, and show the current code that still has the issue. + - **Recommendation**: Address the original feedback before merging. + +Do not flag resolved comments where the concern has been legitimately addressed, even if addressed differently than the reviewer suggested. + +### Etiquette + +- Do not spam the pull request with excessive comments. Focus on the most important issues and provide clear guidance on how to address them. If there are minor style issues, you can mention them but prioritize more significant architectural, performance, security, or correctness issues. +- Do not modify existing comments or feedback from other reviewers. When issues are addressed and resolved, you can acknowledge the changes with a new comment but avoid editing or deleting previous comments to maintain a clear history of the review process. +- Always be respectful and constructive. Always acknowledge that the code review comments are written by an AI assistant and may not be perfect. + +--- + +### Reporting + +When asked, you may prepare a detailed report and status tracking document when refactoring is planned. The report should be in markdown format, would be placed in the docs/planning directory, and must be kept up to date as work progresses. It should contain suitable information and context to help resume work after interruptions. The agent has permission to write and edit such documents without additional approval but must not make any other code or documentation changes itself. + +### Issue Tracking + +When appropriate, you may be asked to create and maintain Jira tickets or GitHub issues to track work items. You have permission to create and edit such tickets and issues without additional approval but must not make any other code or documentation changes itself. When creating such tickets or issues, ensure they are well-formed, with clear titles, descriptions, acceptance criteria, and any relevant links or context. Also make sure it's clear that the issues are being created/maintained by an AI agent. + +Avoid creating duplicate tickets or issues for the same work item. Before creating a new ticket or issue, search existing ones to see if it has already been created. If it has, update the existing ticket or issue instead of creating a new one. + +Be concise and clear in ticket and issue descriptions, focusing on actionable information. Do not be overly verbose or include unnecessary details. Do not leak internal implementation details or sensitive information in ticket or issue descriptions. When in doubt, err on the side of caution and omit potentially sensitive information or ask for specific permission and guidance. + +For interaction with GitHub, use the GitHub CLI (gh) tool or git as appropriate. diff --git a/.opencode/skills/receiving-code-review/SKILL.md b/.opencode/skills/receiving-code-review/SKILL.md new file mode 100644 index 00000000000..50e5c7b1447 --- /dev/null +++ b/.opencode/skills/receiving-code-review/SKILL.md @@ -0,0 +1,213 @@ +--- +name: receiving-code-review +description: Use when receiving code review feedback, before implementing suggestions, especially if feedback seems unclear or technically questionable - requires technical rigor and verification, not performative agreement or blind implementation +--- + +# Code Review Reception + +## Overview + +Code review requires technical evaluation, not emotional performance. + +**Core principle:** Verify before implementing. Ask before assuming. Technical correctness over social comfort. + +## The Response Pattern + +``` +WHEN receiving code review feedback: + +1. READ: Complete feedback without reacting +2. UNDERSTAND: Restate requirement in own words (or ask) +3. VERIFY: Check against codebase reality +4. EVALUATE: Technically sound for THIS codebase? +5. RESPOND: Technical acknowledgment or reasoned pushback +6. IMPLEMENT: One item at a time, test each +``` + +## Forbidden Responses + +**NEVER:** +- "You're absolutely right!" (explicit CLAUDE.md violation) +- "Great point!" / "Excellent feedback!" (performative) +- "Let me implement that now" (before verification) + +**INSTEAD:** +- Restate the technical requirement +- Ask clarifying questions +- Push back with technical reasoning if wrong +- Just start working (actions > words) + +## Handling Unclear Feedback + +``` +IF any item is unclear: + STOP - do not implement anything yet + ASK for clarification on unclear items + +WHY: Items may be related. Partial understanding = wrong implementation. +``` + +**Example:** +``` +your human partner: "Fix 1-6" +You understand 1,2,3,6. Unclear on 4,5. + +❌ WRONG: Implement 1,2,3,6 now, ask about 4,5 later +✅ RIGHT: "I understand items 1,2,3,6. Need clarification on 4 and 5 before proceeding." +``` + +## Source-Specific Handling + +### From your human partner +- **Trusted** - implement after understanding +- **Still ask** if scope unclear +- **No performative agreement** +- **Skip to action** or technical acknowledgment + +### From External Reviewers +``` +BEFORE implementing: + 1. Check: Technically correct for THIS codebase? + 2. Check: Breaks existing functionality? + 3. Check: Reason for current implementation? + 4. Check: Works on all platforms/versions? + 5. Check: Does reviewer understand full context? + +IF suggestion seems wrong: + Push back with technical reasoning + +IF can't easily verify: + Say so: "I can't verify this without [X]. Should I [investigate/ask/proceed]?" + +IF conflicts with your human partner's prior decisions: + Stop and discuss with your human partner first +``` + +**your human partner's rule:** "External feedback - be skeptical, but check carefully" + +## YAGNI Check for "Professional" Features + +``` +IF reviewer suggests "implementing properly": + grep codebase for actual usage + + IF unused: "This endpoint isn't called. Remove it (YAGNI)?" + IF used: Then implement properly +``` + +**your human partner's rule:** "You and reviewer both report to me. If we don't need this feature, don't add it." + +## Implementation Order + +``` +FOR multi-item feedback: + 1. Clarify anything unclear FIRST + 2. Then implement in this order: + - Blocking issues (breaks, security) + - Simple fixes (typos, imports) + - Complex fixes (refactoring, logic) + 3. Test each fix individually + 4. Verify no regressions +``` + +## When To Push Back + +Push back when: +- Suggestion breaks existing functionality +- Reviewer lacks full context +- Violates YAGNI (unused feature) +- Technically incorrect for this stack +- Legacy/compatibility reasons exist +- Conflicts with your human partner's architectural decisions + +**How to push back:** +- Use technical reasoning, not defensiveness +- Ask specific questions +- Reference working tests/code +- Involve your human partner if architectural + +**Signal if uncomfortable pushing back out loud:** "Strange things are afoot at the Circle K" + +## Acknowledging Correct Feedback + +When feedback IS correct: +``` +✅ "Fixed. [Brief description of what changed]" +✅ "Good catch - [specific issue]. Fixed in [location]." +✅ [Just fix it and show in the code] + +❌ "You're absolutely right!" +❌ "Great point!" +❌ "Thanks for catching that!" +❌ "Thanks for [anything]" +❌ ANY gratitude expression +``` + +**Why no thanks:** Actions speak. Just fix it. The code itself shows you heard the feedback. + +**If you catch yourself about to write "Thanks":** DELETE IT. State the fix instead. + +## Gracefully Correcting Your Pushback + +If you pushed back and were wrong: +``` +✅ "You were right - I checked [X] and it does [Y]. Implementing now." +✅ "Verified this and you're correct. My initial understanding was wrong because [reason]. Fixing." + +❌ Long apology +❌ Defending why you pushed back +❌ Over-explaining +``` + +State the correction factually and move on. + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Performative agreement | State requirement or just act | +| Blind implementation | Verify against codebase first | +| Batch without testing | One at a time, test each | +| Assuming reviewer is right | Check if breaks things | +| Avoiding pushback | Technical correctness > comfort | +| Partial implementation | Clarify all items first | +| Can't verify, proceed anyway | State limitation, ask for direction | + +## Real Examples + +**Performative Agreement (Bad):** +``` +Reviewer: "Remove legacy code" +❌ "You're absolutely right! Let me remove that..." +``` + +**Technical Verification (Good):** +``` +Reviewer: "Remove legacy code" +✅ "Checking... build target is 10.15+, this API needs 13+. Need legacy for backward compat. Current impl has wrong bundle ID - fix it or drop pre-13 support?" +``` + +**YAGNI (Good):** +``` +Reviewer: "Implement proper metrics tracking with database, date filters, CSV export" +✅ "Grepped codebase - nothing calls this endpoint. Remove it (YAGNI)? Or is there usage I'm missing?" +``` + +**Unclear Item (Good):** +``` +your human partner: "Fix items 1-6" +You understand 1,2,3,6. Unclear on 4,5. +✅ "Understand 1,2,3,6. Need clarification on 4 and 5 before implementing." +``` + +## GitHub Thread Replies + +When replying to inline review comments on GitHub, reply in the comment thread (`gh api repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies`), not as a top-level PR comment. + +## The Bottom Line + +**External feedback = suggestions to evaluate, not orders to follow.** + +Verify. Question. Then implement. + +No performative agreement. Technical rigor always. diff --git a/.opencode/skills/test-driven-investigation/SKILL.md b/.opencode/skills/test-driven-investigation/SKILL.md new file mode 100644 index 00000000000..a0b295e34fc --- /dev/null +++ b/.opencode/skills/test-driven-investigation/SKILL.md @@ -0,0 +1,163 @@ +--- +name: test-driven-investigation +description: Use when investigating bugs, crashes, assertions, or unexpected behavior - requires writing a reproducing test early instead of over-analyzing source code; concrete experiments over mental models +--- + +# Test-Driven Investigation + +## Overview + +When investigating a bug, write a reproducing test as early as possible. Analysis without experimentation spirals into circular reasoning. + +**Core principle:** A 20-line test that fails tells you more than 2 hours of reading source code. + +## The Anti-Pattern + +``` +READ code → BUILD mental model → READ more code → REVISE model → READ more code → +SECOND-GUESS model → READ same code again → ... → eventually write a test +``` + +This feels productive but isn't. You're pattern-matching on code without grounding in reality. Each re-read adds uncertainty, not clarity. + +## The Pattern + +``` +1. READ the error message / assertion / crash +2. IDENTIFY the minimal trigger (what operation, on what object, in what state?) +3. WRITE a test that sets up that state and performs that operation +4. RUN it +5. Let the RESULT guide the next step +6. Iterate as needed, but always grounded in test results backed by code, not just code. +``` + +Steps 1-2 should take minutes, not hours. You don't need to understand the full call chain to write a test. You need to know what the entry point is and what went wrong. + +### Orientation + +Before writing the test you need to know three things: what object/API to exercise, what test file to put it in, and how to build/run it. Spend up to 10 minutes finding these. This is bounded research in service of the test -- not open-ended code analysis. If you don't know the exact API, pick the closest thing you can find and write the test anyway. A test that exercises the wrong API and passes still tells you something. + +## Check the Commit History + +Early in the investigation, check the recent commit history for changes that touched the relevant code. A bug that appeared recently was likely introduced recently. `git log --oneline -20 -- path/to/relevant/files` takes seconds and can immediately narrow your search from "something somewhere is wrong" to "this specific change might be the cause." + +This is not a substitute for writing a test -- it's a way to form a better hypothesis faster. If you can identify a suspect commit, your first test should try to confirm whether that change is responsible. + +**When you find a suspect commit:** Don't stop there. Always check at least 10 commits in either direction around it. Bugs are often introduced by the interaction between multiple changes, not a single commit. A commit that looks innocent in isolation may have broken an assumption that a nearby commit relied on. Looking at the surrounding commits also guards against confirmation bias -- you might find a better explanation in an adjacent change. + +**Don't mistake correlation for causation.** A commit that lines up timewise with when the bug appeared is a lead, not a conclusion. It might be a coincidence -- the real cause could be an environmental change, a dependency update, a race condition that only became likely under new load patterns, or a latent bug exposed by an unrelated change. Treat a suspect commit as a hypothesis to test, not evidence of guilt. If you can't demonstrate the mechanism by which the commit causes the bug, you haven't found the cause. + +**What to look for:** + +- Changes to the file/function where the crash or assertion fires +- Changes to setup, config, or initialization code for the affected subsystem +- Changes to shared utilities or base classes used by the affected code +- Refactors that moved or renamed things in the area + +**Keep it bounded:** This is 5 minutes of `git log` and `git show`, not an archaeology expedition. If nothing jumps out, move on and write the test with what you have. The commit history is one input to hypothesis formation, not a prerequisite for it. + +## When You're Tempted to Read More Code + +Ask yourself: + +- **"Am I reading this to write a test, or to avoid writing one?"** If you can't articulate what the test would look like, that's the problem to solve -- not more reading. +- **"Do I have a hypothesis I can test?"** If yes, test it. If no, form the simplest one possible and test that. +- **"Do I have more than one hypothesis?"** Either pick one to test or work in parallel with several. Don't get stuck in "I need to understand everything before I can test anything" and don't try juggling multiple mental models in your head. +- **"Am I re-reading code I already read?"** Stop. You're stuck. Write a test with your current understanding, even if it's wrong. A wrong test that runs teaches you something. A correct mental model that you never test teaches you nothing. +- **"Am I retreading over the same path?"** If you find yourself tracing the same call stack multiple times, stop. Write a test that hits the point of failure directly. You can always adjust it later. If necessary, use a temporary tracking document to help you keep track of what you've already read so that you don't have to keep it all in your head or re-read the same code. But that document never takes priority over writing a test and running it. + +## Start From Existing Tests + +Before writing a test from scratch, find existing tests for the same feature or subsystem. Search for tests that exercise the API, protocol, or code path you're investigating — not just tests in the same file as the crash site. + +Existing tests encode implicit knowledge you won't get from reading source code: required setup, framework-specific verification patterns, config quirks, shutdown handling. A test that's structurally wrong (missing verification, wrong config) will "pass" silently without exercising anything. + +**Adapt an existing working test rather than inventing one.** If an existing test works for the feature and the bug is gated by a flag, autogate, or config change, the fastest reproduction is often to clone the working test and change the single variable. + +## Verify the Test Exercises the Code Path + +A test that passes is not evidence the feature worked. It may mean the feature never ran. + +After every test run, check the output for evidence the specific code path was exercised — log lines mentioning the feature, subrequests being made, expected error messages, metrics being recorded. If you can't find evidence in the test output, the test is not valid regardless of its exit status. + +**Concrete checks:** + +- Does the test output mention the feature's script/worker/pipeline name? +- Are expected subrequests or RPC calls visible in the logs? +- If the feature produces side effects (trace delivery, storage writes, network calls), are those side effects observable? +- If you removed the feature config entirely, would the test still pass? If yes, the test isn't testing the feature. + +## Scoping the Test + +You don't need to reproduce the exact production scenario. You need to reproduce the _mechanism_. + +**Production crash:** `KJ_REQUIRE(!writeInProgress, "concurrent write()s not allowed")` + +**You DON'T need:** Every detail of the production call stack that potentially leads to this. + +**You DO need:** To find the shortest path to trigger it, even if only hypothetically. + +``` +Good test scope: + Create the pipe/adapter/object directly + → Put it in the suspect state + → Perform the operation that should fail + → Assert what happens + +Bad test scope: + Reproduce the entire production call stack + with all middleware and wrappers +``` + +## Forming a Hypothesis + +A hypothesis for a bug investigation is: + +> "If I do X after Y fails, Z will happen because the cleanup in Y's error path doesn't do W." + +It does NOT need to be: + +> "I have traced every code path and am certain that line 847 is the root cause because of the interaction between..." + +The first version is testable in 10 minutes. The second takes hours to construct and might still be wrong. + +## After the Test + +- **Test fails as expected:** You've confirmed the bug mechanism. Now you can read code _with purpose_ -- to find the fix, not to find the bug. +- **Test passes (bug doesn't reproduce):** Your hypothesis was wrong. That's valuable. Adjust and try again. This is faster than reading code for another hour. +- **Test crashes differently:** You found something else. Follow that trail, but do not abandon the original effort. You can have multiple parallel threads of investigation, but each should be grounded in test results, not just code reading. + +## Red Flags -- You're Over-Analyzing + +- You've read the same file/function more than twice +- You're building a multi-level mental model of "what calls what" +- You're writing detailed notes about code flow before writing any test code +- You say "let me understand X before I write the test" more than once +- You feel like you need to understand the entire system before you can test a single component +- A test "passes" but you have no evidence the feature ran — and you're reading code to explain why instead of fixing the test +- You've written multiple tests that all pass without reproducing the bug, and you haven't questioned whether any of them exercise the right code path + +## Build Times Don't Change the Priority + +In this codebase, a C++ compile-and-test cycle can take minutes. This does not justify delaying the test in favor of more code reading. It changes what "quick feedback" looks like: + +- **Fast-compiling codebase:** Write test, run, see result in seconds, iterate rapidly. +- **Slow-compiling codebase:** Write test, start the build, use the wait time for targeted reading that serves the next iteration. The build is running -- you're not blocked, you're pipelining. + +The temptation with slow builds is "I should be really sure before I compile." This is the analysis spiral in disguise. A test that doesn't reproduce the bug on the first try but compiles and runs is not wasted -- it's a known-good harness you can adjust in the next cycle, often with a much faster incremental rebuild. + +## Applying to This Codebase + +workerd and its dependencies (KJ, Cap'n Proto) have extensive test infrastructure: + +- **KJ tests:** `KJ_TEST("description") { ... }` in `*-test.c++` files +- **workerd tests:** `.wd-test` format for JS/TS integration tests +- **Build/run:** `bazel test //path:target --test_arg='-f...' --test_output=all` + +Most KJ/capnp bugs can be reproduced with a self-contained `KJ_TEST` using public API (pipes, streams, promises, HTTP). You rarely need internal access. + +## The Bottom Line + +**Write the test. Run the test. Analyze the results. Think. Iterate.** + +Not the other way around. diff --git a/.opencode/skills/update-v8/SKILL.md b/.opencode/skills/update-v8/SKILL.md new file mode 100644 index 00000000000..227e6c5f48e --- /dev/null +++ b/.opencode/skills/update-v8/SKILL.md @@ -0,0 +1,199 @@ +--- +name: update-v8 +description: Step-by-step guide for updating the V8 JavaScript engine in workerd, including patch rebasing, dependency updates, integrity hashes, and verification. Load this skill when performing or assisting with a V8 version bump. +--- + +## Updating V8 in workerd + +V8 updates are high-risk changes that require careful patch management and human judgment for merge conflicts. This skill covers the full process. **Always confirm the target version with the developer before starting.** + +See also: `docs/v8-updates.md` for the original reference document. + +--- + +### Prerequisites + +- `depot_tools` installed and on `$PATH` ([setup guide](https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_setting_up)) +- A local V8 checkout (outside the workerd repo to avoid confusing Bazel): + ```sh + mkdir v8 && cd v8 && fetch v8 + ``` + +--- + +### Step 1: Identify the target version + +Check [chromiumdash.appspot.com](https://chromiumdash.appspot.com/) for the latest V8 version used by Chrome Beta. Confirm the target `` with the developer. + +Find the current version in `build/deps/v8.MODULE.bazel`: + +``` +VERSION = "14.5.201.6" # example — check the actual file +``` + +We'll call this ``. + +### Step 2: Sync local V8 to the current workerd version + +```sh +cd /v8 +git checkout +gclient sync +``` + +### Step 3: Apply workerd's patches onto a branch + +```sh +git checkout -b workerd-patches +git am /patches/v8/* +``` + +There are multiple patches in `patches/v8/`. These include workerd-specific customizations: + +| Patch category | Examples | +| --------------- | ----------------------------------------------------------------------------------------------------- | +| Serialization | Custom ValueSerializer/Deserializer format versions, proxy/function host object support | +| Build system | Windows/Bazel fixes, shared linkage, dependency path overrides (fp16, fast_float, simdutf, dragonbox) | +| Embedder hooks | Promise context tagging, cross-request promise resolution, extra isolate embedder slot | +| Bug workarounds | Memory leak assert disable, slow handle check disable, builtin-can-allocate workaround | +| API additions | `String::IsFlat`, `AdjustAmountOfExternalAllocatedMemory` exposure, additional Exception constructors | +| ICU/config | ICU data export, googlesource ICU binding, verify_write_barriers flag | + +### Step 4: Rebase patches onto the new V8 version + +```sh +git rebase --onto +``` + +**This is where most of the work happens.** Expect conflicts. Key guidance: + +- **Build-system patches** (dependency paths, Bazel config) conflict most often as upstream V8 restructures its build. +- **API patches** (new methods on V8 classes) may conflict if upstream changed the surrounding code. +- **Always preserve workerd's intent** — understand what each patch does before resolving conflicts. The patch filenames are descriptive. +- **Do not drop patches** without explicit confirmation from the developer. +- **Do not auto-resolve conflicts** — flag them for human review. Merge conflicts in V8 patches almost always require human judgment. + +After rebasing, ideally build and test V8 standalone to verify patches apply cleanly. See the V8 [Testing](https://v8.dev/docs/test) page. + +### Step 5: Regenerate patches + +```sh +git format-patch --full-index -k --no-signature --no-stat --zero-commit +``` + +This produces numbered `.patch` files in the current directory. + +### Step 6: Replace patches in workerd + +```sh +rm /patches/v8/* +cp *.patch /patches/v8/ +``` + +### Step 7: Update `build/deps/v8.MODULE.bazel` + +Three things need updating: + +1. **`VERSION`**: Set to ``. + +2. **`INTEGRITY`**: Compute the sha256 hash of the new tarball: + + ```sh + curl -sL "https://github.com/v8/v8/archive/refs/tags/.tar.gz" -o v8.tar.gz + openssl dgst -sha256 -binary v8.tar.gz | openssl base64 -A + ``` + + Format: `"sha256-="`. Alternatively, attempt a build and copy the expected hash from Bazel's mismatch error. + +3. **`PATCHES`**: Update the list if patches were added, removed, or renamed. The list must match the filenames in `patches/v8/` exactly, in order. + +### Step 8: Update V8's dependencies + +V8 depends on several libraries that are pinned in `build/deps/v8.MODULE.bazel` and `build/deps/deps.jsonc`. Check the local V8 checkout's `DEPS` file for commit versions: + +```sh +cat /v8/DEPS +``` + +Dependencies to check and update: + +| Dependency | Where | Notes | +| ------------------------------- | ------------------------------------------ | -------------------------------------------------------------- | +| `com_googlesource_chromium_icu` | `v8.MODULE.bazel` (git_repository commit) | Chromium fork; update commit from V8's DEPS | +| `perfetto` | `deps.jsonc` (managed by `update-deps.py`) | V8 depends via Chromium; safe to bump to latest GitHub release | +| `simdutf` | `deps.jsonc` (managed by `update-deps.py`) | V8 depends via Chromium; safe to bump to latest GitHub release | + +For dependencies in `deps.jsonc`, you can use the update script: + +```sh +python3 build/deps/update-deps.py perfetto +python3 build/deps/update-deps.py simdutf +``` + +This fetches the latest version, computes integrity hashes, and regenerates the `gen/` MODULE.bazel fragments. **Do not hand-edit files in `build/deps/gen/`.** + +### Step 9: Build and test + +```sh +# Full build +just build + +# Full test suite +just test +``` + +Watch for: + +- **Build failures from V8 API changes**: V8 may deprecate or change APIs between versions. Search for deprecation warnings in the build output. Common areas affected: + - `src/workerd/jsg/` — V8 binding layer, most directly affected + - `src/workerd/api/` — APIs that interact with V8 types directly + - `src/workerd/io/worker.c++` — Isolate creation and configuration + +- **Test failures from behavior changes**: V8 may change observable JS behavior. Check: + - `just test //src/workerd/jsg/...` — JSG binding tests + - `just test //src/workerd/api/tests/...` — API tests + - Node.js compatibility tests (`just node-test`) + - Web Platform Tests (`just wpt-test`) + +- **New V8 deprecation warnings**: These are future breakage signals. Document them in the PR description even if tests pass. + +### Step 10: Commit and submit + +Commit the changes and push for review. The PR should include: + +- Updated `build/deps/v8.MODULE.bazel` (version, integrity, patches list) +- Updated patches in `patches/v8/` +- Updated dependency versions if changed +- Any C++ fixes for V8 API changes +- PR description listing: old version, new version, patches that required conflict resolution, any deprecation warnings observed, and any behavior changes noted + +--- + +### Checklist + +- [ ] Target V8 version confirmed with developer +- [ ] Local V8 checked out and synced to old version +- [ ] workerd patches applied and rebased onto new version +- [ ] Conflicts resolved with human review (no auto-resolution) +- [ ] Patches regenerated with `git format-patch` +- [ ] Old patches replaced with new patches in `patches/v8/` +- [ ] `VERSION` updated in `v8.MODULE.bazel` +- [ ] `INTEGRITY` updated in `v8.MODULE.bazel` +- [ ] `PATCHES` list updated if patches added/removed/renamed +- [ ] V8 dependencies checked and updated (ICU, perfetto, simdutf) +- [ ] `just build` succeeds +- [ ] `just test` passes (or failures documented and explained) +- [ ] No new patches dropped without explicit confirmation +- [ ] PR description documents version change, conflict resolutions, and deprecation warnings + +--- + +### Troubleshooting + +**Bazel integrity mismatch**: If you see `expected sha256-... but got sha256-...`, copy the "got" hash into the `INTEGRITY` field. This happens when the hash was computed incorrectly or the tarball was re-generated by GitHub. + +**Patch won't apply**: A patch that applied cleanly during `git am` but fails in Bazel means the `git format-patch` output differs from what Bazel expects. Verify you used `--full-index -k --no-signature --no-stat --zero-commit` flags. Also verify patch order matches the `PATCHES` list. + +**ICU build failures**: ICU is a Chromium fork fetched via `git_repository`. If the commit in `v8.MODULE.bazel` is wrong, you'll see missing-file or compilation errors in ICU. Cross-reference with V8's `DEPS` file for the correct commit. + +**`update-deps.py` fails**: The script requires network access to fetch versions. If a dependency's GitHub release format changed, you may need to update the version manually in `deps.jsonc` and run `python3 build/deps/update-deps.py` to regenerate hashes. diff --git a/.opencode/skills/verification-before-completion/SKILL.md b/.opencode/skills/verification-before-completion/SKILL.md new file mode 100644 index 00000000000..cae5a712b21 --- /dev/null +++ b/.opencode/skills/verification-before-completion/SKILL.md @@ -0,0 +1,139 @@ +--- +name: verification-before-completion +description: Use when about to claim work is complete, fixed, or passing, before committing or creating PRs - requires running verification commands and confirming output before making any success claims; evidence before assertions always +--- + +# Verification Before Completion + +## Overview + +Claiming work is complete without verification is dishonesty, not efficiency. + +**Core principle:** Evidence before claims, always. + +**Violating the letter of this rule is violating the spirit of this rule.** + +## The Iron Law + +``` +NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE +``` + +If you haven't run the verification command in this message, you cannot claim it passes. + +## The Gate Function + +``` +BEFORE claiming any status or expressing satisfaction: + +1. IDENTIFY: What command proves this claim? +2. RUN: Execute the FULL command (fresh, complete) +3. READ: Full output, check exit code, count failures +4. VERIFY: Does output confirm the claim? + - If NO: State actual status with evidence + - If YES: State claim WITH evidence +5. ONLY THEN: Make the claim + +Skip any step = lying, not verifying +``` + +## Common Failures + +| Claim | Requires | Not Sufficient | +|-------|----------|----------------| +| Tests pass | Test command output: 0 failures | Previous run, "should pass" | +| Linter clean | Linter output: 0 errors | Partial check, extrapolation | +| Build succeeds | Build command: exit 0 | Linter passing, logs look good | +| Bug fixed | Test original symptom: passes | Code changed, assumed fixed | +| Regression test works | Red-green cycle verified | Test passes once | +| Agent completed | VCS diff shows changes | Agent reports "success" | +| Requirements met | Line-by-line checklist | Tests passing | + +## Red Flags - STOP + +- Using "should", "probably", "seems to" +- Expressing satisfaction before verification ("Great!", "Perfect!", "Done!", etc.) +- About to commit/push/PR without verification +- Trusting agent success reports +- Relying on partial verification +- Thinking "just this once" +- Tired and wanting work over +- **ANY wording implying success without having run verification** + +## Rationalization Prevention + +| Excuse | Reality | +|--------|---------| +| "Should work now" | RUN the verification | +| "I'm confident" | Confidence ≠ evidence | +| "Just this once" | No exceptions | +| "Linter passed" | Linter ≠ compiler | +| "Agent said success" | Verify independently | +| "I'm tired" | Exhaustion ≠ excuse | +| "Partial check is enough" | Partial proves nothing | +| "Different words so rule doesn't apply" | Spirit over letter | + +## Key Patterns + +**Tests:** +``` +✅ [Run test command] [See: 34/34 pass] "All tests pass" +❌ "Should pass now" / "Looks correct" +``` + +**Regression tests (TDD Red-Green):** +``` +✅ Write → Run (pass) → Revert fix → Run (MUST FAIL) → Restore → Run (pass) +❌ "I've written a regression test" (without red-green verification) +``` + +**Build:** +``` +✅ [Run build] [See: exit 0] "Build passes" +❌ "Linter passed" (linter doesn't check compilation) +``` + +**Requirements:** +``` +✅ Re-read plan → Create checklist → Verify each → Report gaps or completion +❌ "Tests pass, phase complete" +``` + +**Agent delegation:** +``` +✅ Agent reports success → Check VCS diff → Verify changes → Report actual state +❌ Trust agent report +``` + +## Why This Matters + +From 24 failure memories: +- your human partner said "I don't believe you" - trust broken +- Undefined functions shipped - would crash +- Missing requirements shipped - incomplete features +- Time wasted on false completion → redirect → rework +- Violates: "Honesty is a core value. If you lie, you'll be replaced." + +## When To Apply + +**ALWAYS before:** +- ANY variation of success/completion claims +- ANY expression of satisfaction +- ANY positive statement about work state +- Committing, PR creation, task completion +- Moving to next task +- Delegating to agents + +**Rule applies to:** +- Exact phrases +- Paraphrases and synonyms +- Implications of success +- ANY communication suggesting completion/correctness + +## The Bottom Line + +**No shortcuts for verification.** + +Run the command. Read the output. THEN claim the result. + +This is non-negotiable. diff --git a/.opencode/skills/wd-test-format/SKILL.md b/.opencode/skills/wd-test-format/SKILL.md new file mode 100644 index 00000000000..f80edb12b8b --- /dev/null +++ b/.opencode/skills/wd-test-format/SKILL.md @@ -0,0 +1,172 @@ +--- +name: wd-test-format +description: Detailed guide for authoring .wd-test files in workerd, with examples of bindings, Durable Objects, multi-service configs, TypeScript tests, and network access. +--- + +## `.wd-test` File Format + +`.wd-test` files are Cap'n Proto configs that define test workers for workerd's test framework. They use the schema defined in `src/workerd/server/workerd.capnp`. + +--- + +### MANDATORY: Load Reference File When Relevant + +This skill is split across multiple files for context efficiency. The core patterns below cover +standard single-service tests. Advanced configuration patterns live in a reference file. + +**You MUST read the reference file before writing or reviewing test configs that involve its +subject matter. Do not guess at advanced config syntax — the reference file contains the exact +patterns and fields required. Skipping it WILL lead to incorrect configs that fail at runtime.** + +| File | MUST load when... | +| ------------------------------- | ------------------------------------------------------------ | +| `reference/advanced-configs.md` | Test involves Durable Objects, multiple services | +| | communicating via service bindings, outbound network access, | +| | external services/sockets, or TypeScript source files | + +When in doubt about whether the reference file is relevant, load it — the cost of reading is +far less than the cost of a broken test config. + +--- + +### Basic Structure + +```capnp +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [( + name = "my-test", + worker = ( + modules = [(name = "worker", esModule = embed "my-test.js")], + compatibilityFlags = ["nodejs_compat_v2"], + ), + )], +); +``` + +Key rules: + +- The const name (e.g., `unitTests`) must match what the test runner expects +- `modules` uses `embed` to inline file contents at build time +- The first module should be named `"worker"` — this is the entry point +- `compatibilityFlags` control which APIs are available +- `compatibilityDate` should not be used in wd-test; use specific flags instead + +### Module Types + +```capnp +modules = [ + (name = "worker", esModule = embed "my-test.js"), # ES module (most common) + (name = "helper", esModule = embed "helper.js"), # Additional ES module + (name = "data.json", json = embed "test-data.json"), # JSON module + (name = "data.wasm", wasm = embed "module.wasm"), # WebAssembly module + (name = "legacy", commonJsModule = embed "legacy.js"), # CommonJS module +], +``` + +### Bindings + +Bindings make services, data, and namespaces available to the worker via `env`: + +```capnp +bindings = [ + # Text binding — env.MY_TEXT is a string + (name = "MY_TEXT", text = "hello world"), + + # Text from file + (name = "CERT", text = embed "fixtures/cert.pem"), + + # Data binding — env.MY_DATA is an ArrayBuffer + (name = "MY_DATA", data = "base64encodeddata"), + + # JSON binding — env.CONFIG is a parsed object + (name = "CONFIG", json = "{ \"key\": \"value\" }"), + + # Service binding — env.OTHER_SERVICE is a fetch-able service + (name = "OTHER_SERVICE", service = "other-service-name"), + + # Service binding with entrypoint + (name = "MY_RPC", service = (name = "my-service", entrypoint = "MyClass")), + + # KV namespace — env.KV is a KV namespace + (name = "KV", kvNamespace = "kv-namespace-id"), + + # Durable Object namespace — env.MY_DO is a DO namespace + (name = "MY_DO", durableObjectNamespace = "MyDurableObject"), +], +``` + +### Test JavaScript Structure + +Test files export named objects with a `test()` method: + +```javascript +// Each export becomes a separate test case +export const basicTest = { + test() { + // Synchronous test + assert.strictEqual(1 + 1, 2); + }, +}; + +export const asyncTest = { + async test(ctrl, env) { + // ctrl is the test controller + // env contains bindings from the .wd-test config + const resp = await env.OTHER_SERVICE.fetch('http://example.com/'); + assert.strictEqual(resp.status, 200); + }, +}; +``` + +### BUILD.bazel Integration + +```python +wd_test( + src = "my-test.wd-test", + args = ["--experimental"], # Required for experimental features + data = ["my-test.js"], # Test JS/TS files +) +``` + +Additional `data` entries for fixture files: + +```python +wd_test( + src = "crypto-test.wd-test", + args = ["--experimental"], + data = [ + "crypto-test.js", + "fixtures/cert.pem", + "fixtures/key.pem", + ], +) +``` + +### Test Variants + +Every `wd_test()` automatically generates three variants: + +| Target suffix | Compat date | Description | +| ------------------- | ----------- | -------------------------------------- | +| `@` | 2000-01-01 | Default, tests with oldest compat date | +| `@all-compat-flags` | 2999-12-31 | Tests with all flags enabled | +| `@all-autogates` | 2000-01-01 | Tests with all autogates enabled | + +Run specific variants: + +```bash +just stream-test //src/workerd/api/tests:my-test@ +just stream-test //src/workerd/api/tests:my-test@all-compat-flags +``` + +### Scaffolding + +Use `just new-test` to scaffold a new test: + +```bash +just new-test //src/workerd/api/tests:my-test +``` + +This creates the `.wd-test` file, `.js` test file, and appends the `wd_test()` rule to `BUILD.bazel`. diff --git a/.opencode/skills/wd-test-format/reference/advanced-configs.md b/.opencode/skills/wd-test-format/reference/advanced-configs.md new file mode 100644 index 00000000000..187cfce9d22 --- /dev/null +++ b/.opencode/skills/wd-test-format/reference/advanced-configs.md @@ -0,0 +1,155 @@ +# Advanced `.wd-test` Config Patterns + +Patterns for Durable Objects, multi-service tests, network access, external services, +and TypeScript tests. + +--- + +### Durable Objects + +To test Durable Objects, define the namespace and storage: + +```capnp +const unitTests :Workerd.Config = ( + services = [ + ( name = "do-test", + worker = ( + modules = [(name = "worker", esModule = embed "do-test.js")], + compatibilityDate = "2024-01-01", + + durableObjectNamespaces = [ + (className = "MyDurableObject", uniqueKey = "210bd0cbd803ef7883a1ee9d86cce06e"), + ], + + durableObjectStorage = (localDisk = "TEST_TMPDIR"), + + bindings = [ + (name = "MY_DO", durableObjectNamespace = "MyDurableObject"), + ], + ), + ), + # Disk service for DO storage + (name = "TEST_TMPDIR", disk = (writable = true)), + ], +); +``` + +The `uniqueKey` is a hex string that uniquely identifies the namespace. Use any 32-char hex string for tests. + +For in-memory storage (no persistence between requests), use: + +```capnp +durableObjectStorage = (inMemory = void), +``` + +### Multi-Service Configs + +Tests can define multiple services that communicate via service bindings: + +```capnp +const unitTests :Workerd.Config = ( + services = [ + ( name = "main-test", + worker = ( + modules = [(name = "worker", esModule = embed "main-test.js")], + compatibilityDate = "2024-01-01", + bindings = [ + (name = "BACKEND", service = "backend"), + ], + ), + ), + ( name = "backend", + worker = ( + modules = [(name = "worker", esModule = embed "backend.js")], + compatibilityDate = "2024-01-01", + ), + ), + ], +); +``` + +For large configs, factor out worker definitions as named constants: + +```capnp +const unitTests :Workerd.Config = ( + services = [ + (name = "main", worker = .mainWorker), + (name = "helper", worker = .helperWorker), + ], +); + +const mainWorker :Workerd.Worker = ( + modules = [(name = "worker", esModule = embed "main.js")], + compatibilityDate = "2024-01-01", + bindings = [(name = "HELPER", service = "helper")], +); + +const helperWorker :Workerd.Worker = ( + modules = [(name = "worker", esModule = embed "helper.js")], + compatibilityDate = "2024-01-01", +); +``` + +### Network Access + +For tests that need outbound network access: + +```capnp +( name = "internet", + network = ( + allow = ["private"], + tlsOptions = ( + trustedCertificates = [ + embed "test-cert.pem", + ], + ), + ) +), +``` + +`allow` can be `["private"]` (loopback/LAN) or `["public"]` (internet). Most tests use `"private"`. + +### External Services + +For testing RPC over sockets or external service communication: + +```capnp +( name = "my-external", + external = ( + address = "loopback:my-external", + http = (capnpConnectHost = "cappy") + ) +), +``` + +### TypeScript Tests + +TypeScript test files use a `.ts-wd-test` extension for the config, but the embedded module is the compiled `.js` output: + +**Config file** (`my-test.ts-wd-test`): + +```capnp +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [( + name = "my-test", + worker = ( + modules = [(name = "worker", esModule = embed "my-test.js")], + compatibilityDate = "2024-01-01", + ), + )], +); +``` + +**BUILD.bazel** references the `.ts` source: + +```python +wd_test( + src = "my-test.ts-wd-test", + args = ["--experimental"], + data = ["my-test.ts"], +) +``` + +The build system compiles the `.ts` to `.js` automatically. diff --git a/.opencode/skills/workerd-api-review/SKILL.md b/.opencode/skills/workerd-api-review/SKILL.md new file mode 100644 index 00000000000..dce3e5bf8f9 --- /dev/null +++ b/.opencode/skills/workerd-api-review/SKILL.md @@ -0,0 +1,71 @@ +--- +name: workerd-api-review +description: Performance optimization, API design & compatibility, security vulnerabilities, and standards spec compliance for workerd code review. Covers tcmalloc-aware perf analysis, compat flags, autogates, web standards adherence, and security patterns. Load this skill when reviewing API changes, performance-sensitive code, security-relevant code, or standards implementations. +--- + +## API, Performance, Security & Standards Analysis + +Load this skill when analyzing code for performance, API design, backward compatibility, security, or web standards compliance. + +--- + +### Performance Optimization + +- Identify unnecessary allocations and copies (keeping in mind that we're using tcmalloc) +- Find opportunities for move semantics +- Spot hot paths that could benefit from optimization +- Review data structure choices for access patterns +- Analyze cache locality and memory layout +- Identify lock contention and synchronization overhead +- Look for inefficient string operations or repeated parsing +- Avoid premature optimization; focus on clear evidence of performance issues +- Do not make vague or grandiose claims of performance improvements without clear reasoning or data +- Suggest improvements for better use of KJ library features for performance +- End-to-end, real-world performance is the priority over micro-optimizations. + - Consider overall system performance, including interactions between components, I/O, and network latency. + - Avoid optimizations that improve microbenchmarks but do not translate to real-world gains. + - It's ok to suggest low-risk micro-optimizations as low-hanging fruit, but they are not the primary focus. + - Validate claims with real-world testing, benchmarking, or evidence-backed analysis. + - Evaluate trade-offs: an optimization that benefits one workload may degrade another. Aim for broad benefits. + - Consider scalability: solutions should maintain or improve performance as workloads increase. + +### API Design & Compatibility + +- Evaluate API ergonomics and usability +- Review backward compatibility implications +- Check for proper use of compatibility flags (`compatibility-date.capnp`) and + autogates (`util/autogate.h/c++`) +- Identify breaking changes that need feature flags or autogates +- Analyze public vs internal API boundaries +- Review consistency with existing API patterns + +### Security Vulnerabilities + +- Identify injection vulnerabilities +- Identify memory safety issues that could lead to exploits or crashes +- Review input validation and sanitization +- Check for and identify potential timing side channels +- Analyze privilege boundaries and capability checks +- Look for information disclosure risks +- Review cryptographic usage patterns +- Identify TOCTOU (time-of-check-time-of-use) issues +- Remember that workerd favors use of capability-based security + +### Standards Spec Compliance + +- Review adherence to relevant web standards (Fetch, Streams, WebCrypto, etc.) +- Identify deviations from spec behavior and suggest improvements for better alignment +- Review documentation accuracy against specs +- Identify missing features required by specs +- Suggest prioritization for spec compliance work +- Identify interoperability issues with other implementations +- Identify edge cases not handled per specs +- Reference specific spec sections when flagging deviations + +--- + +### Runtime-Specific Notes + +- **KJ event loop**: workerd uses kj's single-threaded event loop, not Node.js-style libuv. Blocking the event loop blocks all concurrent requests on that thread. Flag synchronous I/O, expensive computation, or unbounded loops on the event loop thread. +- **tcmalloc**: workerd uses tcmalloc. Thread-local caches reduce contention but increase per-thread memory overhead. Focus optimization on reducing allocation count (especially in hot paths) rather than individual allocation sizes. Do not suggest switching to standard malloc. +- **Cap'n Proto zero-copy**: Cap'n Proto messages are zero-copy and use arena allocation. Do not suggest copying data out of Cap'n Proto messages "for safety" unless there is a concrete lifetime issue. Suggest using Cap'n Proto's traversal limits to prevent resource exhaustion when processing untrusted messages. diff --git a/.opencode/skills/workerd-safety-review/SKILL.md b/.opencode/skills/workerd-safety-review/SKILL.md new file mode 100644 index 00000000000..158553bf2bc --- /dev/null +++ b/.opencode/skills/workerd-safety-review/SKILL.md @@ -0,0 +1,71 @@ +--- +name: workerd-safety-review +description: Memory safety, thread safety, concurrency, and critical detection patterns for workerd code review. Covers V8/KJ boundary hazards, lifetime management, cross-thread safety, and coroutine pitfalls. Load this skill when reviewing code that touches io/, jsg/, async patterns, or V8 integration. +--- + +## Safety Analysis — Memory, Thread Safety & Concurrency + +Load this skill when analyzing code for memory safety, thread safety, or concurrency correctness. This covers the most critical classes of bugs in workerd. + +--- + +### Memory Safety + +- Identify potential memory leaks, use-after-free, and dangling pointers or references +- Review ownership semantics and lifetime management +- Analyze smart pointer usage (`kj::Own`, `kj::Rc`, `kj::Maybe`) +- Check for proper RAII, CRTP patterns +- Look for potential buffer overflows and bounds checking issues +- Identify raw pointer usage that could be safer with owning types +- Review destructor correctness and cleanup order +- Analyze lambda captures for safety +- Consider patterns where weakrefs (see `util/weak-refs.h`) or other techniques would be safer + +### Thread Safety & Concurrency + +- Identify data races and race conditions +- Review lock ordering and deadlock potential +- Analyze shared state access patterns +- Check for proper synchronization primitives usage +- Review promise/async patterns for correctness +- Identify thread-unsafe code in concurrent contexts +- Analyze KJ event loop interactions +- Ensure that code does not attempt to use isolate locks across suspension points in coroutines +- Ensure that RAII objects and other types that capture raw pointers or references are not unsafely + used across suspension points +- When reviewing V8 integration, pay particular attention to GC interactions and cleanup order +- kj I/O objects should never be held by a V8-heap object without use of `IoOwn` or `IoPtr` + (see `io/io-own.h`) to ensure proper lifetime and thread-safety. +- Watch carefully for places where `kj::Refcounted` is used when `kj::AtomicRefcounted` is required + for thread safety. + +--- + +### Critical Detection Patterns + +Concrete patterns to watch for. When you encounter these, flag them at the indicated severity. + +Beyond these specific patterns, also watch for non-obvious complexity at V8/KJ boundaries: re-entrancy bugs where a C++ callback unexpectedly re-enters JavaScript, subtle interactions between KJ event loop scheduling and V8 GC timing, and cases where destruction order depends on runtime conditions. + +**CRITICAL / HIGH:** + +- **V8 callback throwing C++ exception**: A V8 callback (JSG method, property getter/setter) that can throw a C++ exception without using `liftKj` (see `jsg/util.h`). V8 callbacks must catch C++ exceptions and convert them to JS exceptions. +- **V8 heap object holding kj I/O object directly**: A `jsg::Object` subclass storing `kj::Own`, `kj::Rc`, `kj::Arc` for an I/O-layer object without wrapping in `IoOwn` or `IoPtr` (see `io/io-own.h`). Causes lifetime and thread-safety bugs. +- **`kj::Refcounted` in cross-thread context**: A class using `kj::Refcounted` whose instances can be accessed from both the I/O thread and the JS isolate thread. Needs `kj::AtomicRefcounted`. +- **Isolate lock held across `co_await`**: Holding a `jsg::Lock`, V8 `HandleScope`, or similar V8 scope object across a coroutine suspension point. This is undefined behavior. +- **RAII object with raw pointer/reference across `co_await`**: Any RAII type or variable capturing a raw pointer or reference used across a coroutine suspension point without `kj::coCapture` to ensure correct lifetime. +- **Reference cycle between `jsg::Object` subclasses**: Two or more `jsg::Object` subclasses holding strong references to each other without GC tracing via `JSG_TRACE`. Causes memory leaks invisible to V8's GC. +- **`jsg::Object` destructor accessing another `jsg::Object`**: V8 GC destroys objects in non-deterministic order. A destructor that dereferences another GC-managed object may use-after-free. + +**MEDIUM (safety-related):** + +- **Broad capture in async lambda**: Lambda passed to `.then()` or stored for deferred execution using `[&]` or `[this]` when only specific members are needed. Prefer explicit captures and carefully consider captured variable lifetimes. +- **Implicit GC trigger in sensitive context**: V8 object allocations (e.g., `ArrayBuffer` backing store creation, string flattening, `v8::Object::New()`) inside hot loops or time-sensitive callbacks may trigger GC unexpectedly. + +--- + +### Runtime-Specific Safety Notes + +- **V8 GC awareness**: V8 may GC at any allocation point. Operations that create V8 objects (including string flattening, ArrayBuffer creation) can trigger GC. Be aware of this when analyzing code that interleaves V8 allocations with raw pointer access to V8-managed objects. +- **Destructors may throw**: workerd follows KJ convention of `noexcept(false)` destructors. Do not flag this as an issue unless there is a specific exception safety problem (e.g., double-exception during stack unwinding). +- **Cross-platform**: workerd runs on Linux in production but builds on macOS and Windows. Flag platform-specific system calls or assumptions (e.g., Linux-only epoll, /proc filesystem) that lack portable alternatives. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000000..fc5ba296535 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,7 @@ +# workerd + +Cloudflare Workers JS/Wasm runtime. C++23/Bazel monorepo with TS, Rust, Python layers. + +See [CLAUDE.md](CLAUDE.md) for build commands, project structure, coding conventions, anti-patterns, and contributing guidelines. That file is the single source of truth for project-wide context shared across all AI tools. + +Subdirectory `AGENTS.md` files provide component-specific context (key classes, where-to-look tables, local conventions and anti-patterns). They assume familiarity with the content in `CLAUDE.md`. diff --git a/CLAUDE.md b/CLAUDE.md index 2fbc828348c..121c3311ca5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,13 +1,15 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +This file provides guidance to Claude Code (claude.ai/code) or Opencode (opencode.ai) when working with code in this repository. -## Instructions for Claude Code +## Instructions for AI Code Assistants -- Look for high-level overview in `docs/` directory -- Check README.md files for package/directory level information -- Check source file comments for more detailed info - Suggest updates to CLAUDE.md when you find new high-level information +- You should always determine if the current repository was checked out standalone or as a submodule + of the larger workers project. +- If checked out as a submodule, be aware that there is additional documentation and context in the + root of that repository that is not present here. Look for the `../../README.md`, `../../CLAUDE.md`, + and other markdown files in the root of the parent repository. ## Project Overview @@ -16,10 +18,12 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Build System & Commands ### Primary Build System: Bazel + - Main build command: `bazel build //src/workerd/server:workerd` - Binary output: `bazel-bin/src/workerd/server/workerd` ### Just Commands (recommended for development) + - `just build` or `just b` - Build the project - `just test` or `just t` - Run all tests - `just format` or `just f` - Format code (uses clang-format + Python formatter) @@ -32,26 +36,116 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - `just compile-commands` - Generate compile_commands.json for clangd support - `just build-asan` - Build with AddressSanitizer - `just test-asan` - Run tests with AddressSanitizer +- `just new-test ` - Scaffold a new test (e.g., `just new-test //src/workerd/api/tests:my-test`) +- `just new-wpt-test ` - Scaffold a new WPT test +- `just lint` or `just eslint` - Run ESLint on TypeScript sources +- `just coverage ` - Generate code coverage report (Linux only, defaults to `//...`) +- `just watch ` - Watch `src/` and `build/` dirs, re-run a just command on changes ## Testing -### Test Types & Commands -- **Unit Tests**: `.wd-test` files use Cap'n Proto config format -- **C++ Tests**: Traditional C++ unit tests -- **Node.js Compatibility**: `just node-test ` +### Test Types + +- **`.wd-test` tests**: Cap'n Proto config files that define a `Workerd.Config` with embedded JS/TS modules. Bazel macro: `wd_test()`. See format details below. +- **C++ tests**: KJ-based unit tests (`.c++` files). Bazel macro: `kj_test()`. +- **Node.js compatibility tests**: `just node-test ` - **Web Platform Tests**: `just wpt-test ` - **Benchmarks**: `just bench ` (e.g., `just bench mimetype`) +### Running a Single Test + +Both `just test` and `just build` accept specific Bazel targets (they default to `//...`): + +``` +just test //src/workerd/api/tests:encoding-test@ +just test //src/workerd/io:io-gate-test@ +just stream-test //src/workerd/api/tests:encoding-test@ # streams output for debugging +``` + +Or use Bazel directly: + +``` +bazel test //src/workerd/api/tests:encoding-test@ +``` + +### Test Variants + +Every test automatically generates multiple variants via the build macros: + +- **`name@`** — default variant (oldest compat date, 2000-01-01) +- **`name@all-compat-flags`** — newest compat date (2999-12-31), tests with all flags enabled +- **`name@all-autogates`** — all autogates enabled + oldest compat date + +The `@` suffix is required in target names. For example: `//src/workerd/io:io-gate-test@`, not `//src/workerd/io:io-gate-test`. + +To find the right target name for a file, check the `BUILD.bazel` file in the same directory for `wd_test()` or `kj_test()` rules. You can also use Bazel query: + +``` +bazel query //src/workerd/api/tests:all # list all targets in a package +``` + +### `.wd-test` File Format + +`.wd-test` files are Cap'n Proto configs that define test workers: + +```capnp +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [( + name = "my-test", + worker = ( + modules = [(name = "worker", esModule = embed "my-test.js")], + compatibilityDate = "2024-01-01", + compatibilityFlags = ["nodejs_compat"], + ), + )], +); +``` + +Key elements: `modules` (embed JS/TS files), `compatibilityFlags`, `bindings` (service bindings, JSON, KV, etc.), `durableObjectNamespaces`. ## Architecture +### Dependencies + - **Cap'n Proto source code** available in `external/capnp-cpp` - contains KJ C++ base library and -capnproto RPC library. Consult it for all questions about `kj/` and `capnproto/` includes and -`kj::` and `capnp::` namespaces. + capnproto RPC library. Consult it for all questions about `kj/` and `capnproto/` includes and + `kj::` and `capnp::` namespaces. + +The other core runtime dependencies include: + +| Dependency | Description | +| ----------------------------------- | ------------------------------------------------------------------- | +| V8 | JavaScript engine | +| Cap'n Proto (capnp-cpp) | Serialization/RPC framework and KJ base library | +| BoringSSL | TLS/crypto (Google's OpenSSL fork, patched for ncrypto/libdecrepit) | +| SQLite3 | Embedded database | +| ICU (com_googlesource_chromium_icu) | Internationalization (Chromium fork) | +| zlib | Compression (Chromium fork, patched) | +| zstd | Zstandard compression | +| brotli | Brotli compression | +| tcmalloc | Memory allocator | +| ada-url | URL parser | +| simdutf | Unicode transcoding (SIMD-accelerated) | +| nbytes | Node.js byte utilities | +| ncrypto | Node.js crypto utilities | +| perfetto | Tracing/profiling framework (patched) | +| fast_float | Fast float parsing | +| fp16 | Half-precision float support | +| highway | SIMD abstraction library | +| dragonbox | Float-to-string conversion | + +These dependencies are vendored via Bazel into the `external/` directory. See `MODULE.bazel` and the `build/deps/` directory for how they are integrated into the build system. (The project uses bzlmod; the legacy `WORKSPACE` file may still exist but is no longer the primary mechanism.) + +For several of these dependencies (notably V8, boringssl, sqlite, perfetto, and zlib), we maintain sets of patches that are applied on top of the upstream code. These patches are stored in the `patches/` directory and are applied during the build process. When updating these dependencies, it's important to review and update the corresponding patches as needed. The patches may introduce workerd-specific customizations and new APIs. + +Be aware that workerd uses tcmalloc for memory allocation in the typical case. When analyzing memory usage or debugging memory issues, be aware that tcmalloc's behavior may differ from the standard allocator. Any memory usage analysis that you perform should take this into account. ### Core Directory Structure (`src/workerd/`) + - **`api/`** - Runtime APIs (HTTP, crypto, streams, WebSocket, etc.) - - Contains both C++ implementations + - Contains C++ implementations of the core APIs exposed to JavaScript, as well as the Node.js compatibility layer - C++ portions of the Node.js compatibility layer are in `api/node/`, while the JavaScript and TypeScript implementations live in `src/node/` - Tests in `api/tests/` and `api/node/tests/` - TypeScript definitions are derived from C++ (which can have some annotations). This generation is handled by code in `types/` directory. @@ -67,37 +161,114 @@ capnproto RPC library. Consult it for all questions about `kj/` and `capnproto/` - **`util/`** - Utility libraries (SQLite, UUID, threading, etc.) ### Multi-Language Support + - **`src/cloudflare/`** - Cloudflare-specific APIs (TypeScript) - **`src/node/`** - Node.js compatibility layer (TypeScript) - **`src/pyodide/`** - Python runtime support via Pyodide - **`src/rust/`** - Rust integration components - ### Configuration System + - Uses **Cap'n Proto** for configuration files (`.capnp` format) - Main schema: `src/workerd/server/workerd.capnp` - Sample configurations in `samples/` directory - Configuration uses capability-based security model +### Where to Look + +| Task | Location | Notes | +| ---------------------- | ------------------------------------------------------------- | -------------------------------------------------------------- | +| Add/modify JS API | `src/workerd/api/` | C++ with JSG macros; see `jsg/jsg.h` for binding system | +| Add Node.js compat | `src/workerd/api/node/` (C++) + `src/node/` (TS) | Dual-layer; register in `api/node/node.h` NODEJS_MODULES macro | +| Add Cloudflare API | `src/cloudflare/` | TypeScript; mock in `internal/test//` | +| Modify compat flags | `src/workerd/io/compatibility-date.capnp` | ~1400 lines; annotations define flag names + enable dates | +| Add autogate | `src/workerd/util/autogate.h` + `.c++` | Enum + string map; both must stay in sync | +| Config schema | `src/workerd/server/workerd.capnp` | Cap'n Proto; capability-based security | +| Worker lifecycle | `src/workerd/io/worker.{h,c++}` | Isolate, Script, Worker, Actor classes | +| Request lifecycle | `src/workerd/io/io-context.{h,c++}` | IoContext: the per-request god object | +| Durable Object storage | `src/workerd/io/actor-cache.{h,c++}` + `actor-sqlite.{h,c++}` | LRU cache over RPC / SQLite-backed | +| Streams implementation | `src/workerd/api/streams/` | Has 842-line README; dual internal/standard impl | +| Bazel build rules | `build/` | Custom `wd_*` macros; `wd_test.bzl` generates 3 test variants | +| TypeScript types | `types/` | Extracted from C++ RTTI + hand-written `defines/*.d.ts` | +| V8 patches | `patches/v8/` | 33 patches; see `docs/v8-updates.md` | + +## Coding Conventions + +This project generally follows the [KJ Style Guide](https://github.com/capnproto/capnproto/blob/v2/kjdoc/style-guide.md) and [KJ Tour](https://github.com/capnproto/capnproto/blob/v2/kjdoc/tour.md), with one exception: comment style follows the more common idiomatic C++ patterns (e.g., `//` line comments) rather than KJ's comment conventions. + +- **C++ standard**: C++23 (`-std=c++23`) +- **C++ file extensions**: `.c++` / `.h` (not `.cpp`); test suffix `-test` (hyphenated) +- **Formatting**: `just format` runs clang-format + prettier + ruff + buildifier + rustfmt +- **Pre-commit hook**: Blocks `KJ_DBG` in staged code; runs format check +- **Commit discipline**: Split PRs into small commits; each must compile + pass tests; no fixup commits +- **TypeScript**: Strict mode, `exactOptionalPropertyTypes`, private `#` syntax enforced, explicit return types + +### Use KJ types, not STL + +This project uses the KJ library instead of the C++ standard library for most types: + +| Instead of | Use | +| ----------------------- | --------------------------------------------------- | +| `std::string` | `kj::String` (owned) / `kj::StringPtr` (view) | +| `std::vector` | `kj::Array` (fixed) / `kj::Vector` (growable) | +| `std::unique_ptr` | `kj::Own` | +| `std::shared_ptr` | `kj::Rc` / `kj::Arc` (thread-safe) | +| `std::optional` | `kj::Maybe` | +| `std::function` | `kj::Function` | +| `std::variant` | `kj::OneOf` | +| `std::span` / array ref | `kj::ArrayPtr` | +| `std::exception` | `kj::Exception` | +| `std::promise`/`future` | `kj::Promise` / `kj::ForkedPromise` | + +### Error Handling + +- `KJ_IF_SOME` for unwrapping `kj::Maybe` (1400+ uses across the codebase) +- `JSG_REQUIRE` / `JSG_FAIL_REQUIRE` for JS-facing errors with DOM exception types +- `KJ_ASSERT` / `KJ_REQUIRE` / `KJ_FAIL_ASSERT` for C++ assertions and preconditions + +### JSG (JavaScript Glue) + +C++ classes are exposed to JavaScript via JSG macros in `src/workerd/jsg/`. See the comprehensive guide at `src/workerd/jsg/README.md` for details. When adding or modifying JavaScript APIs, find a similar existing API and follow its pattern. + +- `JSG_RESOURCE_TYPE` for reference types, `JSG_STRUCT` for value types +- `js.alloc()` for resource allocation + +### Feature Management + +- **Compatibility flags** (`src/workerd/io/compatibility-date.capnp`) — per-worker, date-driven, permanent. Flags MUST be documented before their enable date. +- **Autogates** (`src/workerd/util/autogate.*`) — per-process, config-driven, temporary. For risky rollouts with conditional activation. + +## Anti-Patterns + +- **NEVER** put `v8::Local`/`v8::Global`/`JsValue` in `JSG_STRUCT` fields (use `jsg::V8Ref`/`jsg::JsRef`) +- **NEVER** put `v8::Global` or `v8::Local` in `kj::Promise` (compile-time deleted) +- **NEVER** pass `jsg::Lock` into KJ promise coroutines +- **NEVER** hold JS heap refs to KJ I/O objects without `IoOwn`; enforced by `DISALLOW_KJ_IO_DESTRUCTORS_SCOPE` +- **NEVER** use `JSG_INSTANCE_PROPERTY` without good reason (breaks GC optimization); prefer `JSG_PROTOTYPE_PROPERTY` +- **NEVER** call `recursivelyFreeze()` on user-provided content (unsafe for cyclic values) +- **NEVER** add new `Fetcher` methods without compat flag (conflicts with JS RPC wildcard) +- **NEVER** change `Headers::Guard` enum values (serialized) +- **NEVER** use `getWaitUntilTasks()` (use `addWaitUntil()`) +- `Ref` stored in C++ objects visible from JS heap **MUST** implement `visitForGc()`; C++ reference cycles are **NEVER** collected +- SQLite `SQLITE_MISUSE` errors always throw (never suppressed); transactions disallowed in DO SQLite +- Module evaluation **MUST NOT** be in an IoContext; async I/O is **FORBIDDEN** in global scope + ## Backward Compatibility + - Strong backwards compatibility commitment - features cannot be removed or changed once deployed - We use compatibility-date.capnp to introduce feature flags when we need to change the behavior -## Risky Changes -- We use autogates (defined in `src/workerd/util/autogate.*`) flags to make risky changes conditional for testing/slow rollout. - ## Development Workflow -### Code Style & Formatting -- Automatic formatting via clang-format (enforced in CI) -- Run `just format` before committing - ### Contributing + - High bar for non-standard APIs; prefer implementing web standards +- Run formatting with `just format` before submitting PRs - Run tests with `just test` before submitting PRs - See `CONTRIBUTING.md` for more details ### Rust Development + - `just update-rust ` - Update Rust dependencies (equivalent to `cargo update`) - `just clippy ` - Run clippy linting on Rust code @@ -114,16 +285,12 @@ When updating V8, ensure that all tests pass. Look for new deprecations when bui If asked to help with a V8 update, ask for the specific target V8 version to update to and ask clarifying questions about any specific patches or customizations that need to be preserved before proceeding. Merge conflicts are common during V8 updates, so be prepared to resolve those carefully. These almost always require human judgment to ensure that workerd-specific changes are preserved while still applying the upstream V8 changes correctly. Do not attempt to resolve merge conflicts automatically without human review. -## Visual Studio Code Setup - -See [docs/vscode.md](docs/vscode.md) for instructions on setting up Visual Studio Code for workerd development, including build tasks, debugging configurations, and clangd integration for code completion and navigation. - ## Other Documentation See the markdown files in the `docs/` directory for additional information on specific topics: + - [development.md](docs/development.md) - Development environment setup and tools - [api-updates.md](docs/api-updates.md) - Guidelines for adding new JavaScript APIs - [pyodide.md](docs/pyodide.md) - Pyodide package management and updates Some source directories also contain README.md files with more specific information about that component. Proactively look for these when working in unfamiliar areas of the codebase. Proactively suggest updates to the documentation when it is missing or out of date, but do not make edits without confirming accuracy. - diff --git a/build/AGENTS.md b/build/AGENTS.md new file mode 100644 index 00000000000..9a733c36d88 --- /dev/null +++ b/build/AGENTS.md @@ -0,0 +1,40 @@ +# build/ — Bazel Build Rules + +## OVERVIEW + +Custom Bazel rules (`wd_*` macros) for C++, TypeScript, Rust, Cap'n Proto, and test orchestration. Uses bzlmod (`MODULE.bazel`), not WORKSPACE. This is build system definitions, NOT build output (`bazel-bin/`). + +## KEY RULES + +| Rule | Purpose | +| ------------------------------------------ | ------------------------------------------------------------------------------------------------- | +| `wd_cc_library.bzl` | Wraps `cc_library`; `strip_include_prefix="/src"`, arch-specific CPU flags (CRC32C) | +| `wd_cc_binary.bzl` | Wraps `cc_binary`; macOS dead-strip linkopts; creates `_cross` alias for prebuilt arm64 binaries | +| `wd_cc_embed.bzl` | Binary/text data -> C++ via C23 `#embed`; auto-detects text vs binary by extension | +| `wd_cc_benchmark.bzl` | Google Benchmark wrapper; generates CSV report genrule | +| `wd_test.bzl` | `.wd-test` config test runner; generates up to 3 variants per test | +| `kj_test.bzl` | C++ unit test wrapper; also generates test variants | +| `wpt_test.bzl` | Web Platform Tests; generates JS runner + `.wd-test` config from WPT tree; delegates to `wd_test` | +| `wd_ts_bundle.bzl` | TypeScript compilation + JS bundle generation | +| `wd_js_bundle.bzl` | JS bundle -> Cap'n Proto `Modules.Bundle` embedding via generated `.capnp` | +| `wd_capnp_library.bzl` | Cap'n Proto schema compilation | +| `wd_rust_crate.bzl` / `wd_rust_binary.bzl` | Rust build rules | +| `lint_test.bzl` | ESLint integration | + +**Conventions:** + +- `_cross` alias pattern: every `wd_cc_binary` gets a `name_cross` alias selecting prebuilt arm64 or source build +- Test tags: `off-by-default`, `requires-container-engine`, `no-asan`, `no-coverage` +- Variant generation controllable per-test via `generate_*_variant` booleans +- `BUILD.*` files: overlay build files for third-party deps (sqlite3, zlib, simdutf, pyodide, wpt) + +## DEPENDENCY MANAGEMENT + +Lives in `deps/`. Uses jsonc manifests + codegen: + +- `deps.jsonc`, `build_deps.jsonc`, `shared_deps.jsonc` — dependency specifications +- `update-deps.py [dep_name]` — fetches latest versions, computes hashes, regenerates `gen/` MODULE.bazel fragments +- `gen/` — **autogenerated**; do not hand-edit +- `*.MODULE.bazel` (e.g., `rust.MODULE.bazel`, `v8.MODULE.bazel`) — included by root `MODULE.bazel` +- `workerd-v8/` — separate Bazel module wrapping V8 dependency +- `python/` — Pyodide package lists (versioned `.bzl` files) diff --git a/src/cloudflare/AGENTS.md b/src/cloudflare/AGENTS.md new file mode 100644 index 00000000000..ccab046b75a --- /dev/null +++ b/src/cloudflare/AGENTS.md @@ -0,0 +1,28 @@ +# src/cloudflare/ + +## OVERVIEW + +TypeScript implementations of Cloudflare product APIs (AI, D1, R2, Vectorize, etc.). Top-level `.ts` files re-export from `internal/` via `cloudflare-internal:` specifiers. `internal/*.d.ts` declare types for C++ runtime bindings; `internal/*.ts` contain actual logic. + +## STRUCTURE + +- `.ts` — Public entry point; re-exports from `cloudflare-internal:-api` +- `internal/-api.ts` — Implementation; imports runtime types from sibling `.d.ts` +- `internal/*.d.ts` — Type declarations for C++ runtime-provided modules (no implementation) +- `internal/tracing-helpers.ts` — Shared instrumentation (`withSpan`); imported as `cloudflare-internal:tracing-helpers` +- `internal/test//` — Per-product test directory + +## TEST PATTERN + +Each product test directory contains: + +| File | Purpose | +| --------------------------------------- | ------------------------------------------------------------ | +| `-api-test.js` | JS test; named exports with `test()` methods + `node:assert` | +| `-api-test.wd-test` | Cap'n Proto config wiring test worker to mock | +| `-mock.js` | Mock service simulating upstream API | +| `-api-test.py` | Python variant (optional) | +| `python--api-test.wd-test` | Python test config (optional) | +| `-api-instrumentation-test.js` | Tracing/instrumentation tests (optional) | + +Mock wiring uses `wrapped` bindings: `moduleName = "cloudflare-internal:-api"` with `innerBindings` pointing `fetcher` at the mock service. Shared `instrumentation-test-helper.js` lives in `internal/test/`. diff --git a/src/node/AGENTS.md b/src/node/AGENTS.md new file mode 100644 index 00000000000..94f58c6a8d4 --- /dev/null +++ b/src/node/AGENTS.md @@ -0,0 +1,45 @@ +# src/node/ — Node.js Compatibility (TypeScript) + +## OVERVIEW + +TypeScript layer implementing Node.js built-in modules for Workers. ~70 top-level `.ts` files (one per `node:*` module) re-export from `internal/` implementations. No index.ts — C++ side registers each module. Tests live in `src/workerd/api/node/tests/`, not here. See `README.md` for 12 policy rules governing compat scope. + +## ARCHITECTURE + +Three-tier module system: + +1. **C++ JSG modules** (`src/workerd/api/node/`) expose native ops via `node-internal:*` specifiers +2. **`internal/*.d.ts`** declare shapes the C++ modules provide (e.g., `crypto.d.ts`, `buffer.d.ts`) +3. **`internal/*.ts`** build full API surface; **top-level `*.ts`** re-export public API + +Import specifiers: + +- `node:buffer` → `src/node/buffer.ts` (public, user-importable) +- `node-internal:internal_buffer` → `src/node/internal/internal_buffer.ts` (private) +- `node-internal:crypto` → C++ JSG module declared by `internal/crypto.d.ts` +- `cloudflare-internal:*` → runtime-provided APIs (filesystem, sockets, http, messagechannel, workers) + +Build: single `wd_ts_bundle` rule in `BUILD.bazel`; `modules` = public, `internal_modules` = private. + +## MODULE GATING + +Runtime compat flags checked via `Cloudflare.compatibilityFlags['flag_name']`: + +| Flag | Guards | +| -------------------------------------- | --------------------------------------------------------------------- | +| `enable_nodejs_http_server_modules` | http Server/createServer, https Server | +| `nodejs_zlib` | zlib streaming classes (Deflate, Gzip, etc.) | +| `enable_nodejs_process_v2` | Extended process/events functionality | +| `remove_nodejs_compat_eol_v22/v23/v24` | EOL deprecation of specific API surfaces (crypto, util, tls, process) | + +Most modules require `nodejs_compat` + `nodejs_compat_v2` flags (enforced by C++ side, not visible here). + +## CONVENTIONS + +- **`internal/*.js`** (not `.ts`) = upstream Node.js ports (streams_readable.js, streams_writable.js, etc.); paired with `.d.ts` type declarations +- **`internal/*.d.ts`** without matching `.ts` = declares C++ JSG module shape (crypto.d.ts → `node-internal:crypto`) +- **`internal/*.d.ts`** with matching `.js` = types for ported JS (streams_readable.d.ts → streams_readable.js) +- Top-level files are thin: import from `node-internal:*`, re-export with `export { ... }` / `export * from` +- Feature-gated exports use `if (!flag) { throw ... }` or conditional class assignment patterns +- Shared validators in `internal/validators.ts`; shared errors in `internal/internal_errors.ts` +- `_` prefix files (e.g., `_http_agent.ts`, `_stream_readable.ts`) = Node.js legacy internal module aliases diff --git a/src/pyodide/AGENTS.md b/src/pyodide/AGENTS.md new file mode 100644 index 00000000000..ff9182f2b4a --- /dev/null +++ b/src/pyodide/AGENTS.md @@ -0,0 +1,24 @@ +# src/pyodide/ + +## OVERVIEW + +Python Workers runtime layer. Replaces Pyodide's loader with a minimal substitute adding memory snapshot support. TS+Python; modules registered as `pyodide-internal:*` BUILTIN modules. `pool/emscriptenSetup.ts` runs in vanilla V8 isolate -- CANNOT import C++ extension modules. + +## KEY COMPONENTS + +| File/Dir | Role | +| --------------------------------------------- | ----------------------------------------------------------------------------------------------------- | +| `python-entrypoint-helper.ts` | BUILTIN module backing the USER `python-entrypoint.js`; request dispatch, handler wiring | +| `internal/python.ts` | Core bridge: Emscripten init, Pyodide bootstrap, snapshot orchestration | +| `internal/snapshot.ts` | Memory snapshot collect/restore; baseline vs dedicated snapshot types | +| `internal/setupPackages.ts`, `loadPackage.ts` | Package mounting, sys.path, vendor dir setup | +| `internal/tar.ts`, `tarfs.ts` | Tar archive parsing + read-only filesystem for bundles | +| `internal/topLevelEntropy/` | TS+Python: patches `getRandomValues` with deterministic entropy during import, reseeds before request | +| `internal/pool/` | Emscripten setup in plain V8 isolate; `emscriptenSetup.ts` has NO access to C++ extensions | +| `internal/workers-api/` | Python SDK package (`pyproject.toml` + `uv.lock` managed) | +| `internal/metadata.ts` | Config flags: `IS_WORKERD`, `LOCKFILE`, `MAIN_MODULE_NAME`, etc. | +| `pyodide_extra.capnp` | Cap'n Proto schema for Pyodide bundle metadata | + +## TESTING + +Tests live in `src/workerd/server/tests/python/`. `py_wd_test.bzl` macro: expands `%PYTHON_FEATURE_FLAGS` template, handles multiple Pyodide versions, snapshot generation/loading, per-version compat flag isolation. Tests are `size="enormous"` by default. Each test generates variants per supported Pyodide version (`0.26.0a2`, newer). diff --git a/src/rust/AGENTS.md b/src/rust/AGENTS.md new file mode 100644 index 00000000000..87cbaab9067 --- /dev/null +++ b/src/rust/AGENTS.md @@ -0,0 +1,32 @@ +# src/rust/ + +## OVERVIEW + +11 Rust library crates + 1 binary crate linked into workerd via CXX FFI. No Cargo workspace — entirely Bazel-driven (`wd_rust_crate.bzl`). Clippy pedantic+nursery enabled; `allow-unwrap-in-tests`. + +## CRATES + +| Crate | Purpose | +| -------------------- | ------------------------------------------------------------------------------------------------------ | +| `jsg/` | Rust JSG bindings: `Lock`, `Ref`, `Resource`, `Struct`, `Type`, `Realm`, module registration | +| `jsg-macros/` | Proc macros: `#[jsg_struct]`, `#[jsg_method]`, `#[jsg_resource]`, `#[jsg_oneof]` | +| `jsg-test/` | Test harness (`Harness`) for JSG Rust bindings | +| `api/` | Rust-implemented Node.js APIs; registers modules via `register_nodejs_modules()` | +| `dns/` | DNS record parsing (CAA, NAPTR) via CXX bridge; legacy duplicate of `api/dns.rs`, pending removal | +| `net/` | Single function: `canonicalize_ip()` | +| `kj/` | Rust bindings for KJ library (`http`, `io`, `own` submodules); `Result` = `Result` | +| `cxx-integration/` | Tokio runtime init; called from C++ `main()` before anything else | +| `transpiler/` | TS type stripping via SWC (`ts_strip()`, `StripOnly` mode) | +| `python-parser/` | Python import extraction via `ruff_python_parser`; **namespace: `edgeworker::rust::`** | +| `gen-compile-cache/` | Binary crate — V8 bytecode cache generator; calls C++ `compile()` via CXX | + +## CONVENTIONS + +- **CXX bridge**: `#[cxx::bridge(namespace = "workerd::rust::")]` with companion `ffi.c++`/`ffi.h` files +- **Namespace**: always `workerd::rust::*` except `python-parser` → `edgeworker::rust::python_parser` +- **Errors**: `thiserror` for library crates; `jsg::Error` with `ExceptionType` for JSG-facing crates +- **JSG resources**: must include `_state: jsg::ResourceState` field; `#[jsg_method]` auto-converts `snake_case` → `camelCase` +- **Formatting**: `rustfmt.toml` — `group_imports = "StdExternalCrate"`, `imports_granularity = "Item"` (one `use` per import) +- **Linting**: `just clippy ` — pedantic+nursery; `allow-unwrap-in-tests` +- **Tests**: inline `#[cfg(test)]` modules; JSG tests use `jsg_test::Harness::run_in_context()` +- **FFI pointers**: functions receiving raw pointers must be `unsafe fn` (see `jsg/README.md`) diff --git a/src/workerd/api/AGENTS.md b/src/workerd/api/AGENTS.md new file mode 100644 index 00000000000..ae65490ea86 --- /dev/null +++ b/src/workerd/api/AGENTS.md @@ -0,0 +1,45 @@ +# src/workerd/api/ + +All JavaScript APIs exposed to Workers: HTTP, crypto, streams, WebSocket, Cache, KV, R2, SQL, encoding, events, timers, scheduled/alarm handlers. + +## STRUCTURE + +``` +*.h + *.c++ # Top-level API impls (http, headers, cache, blob, url, events, etc.) +crypto/ # WebCrypto + Node crypto: AES, RSA, EC, DH, HKDF, PBKDF2, X.509, JWK +streams/ # ReadableStream/WritableStream/TransformStream (internal + standard); has README.md +node/ # Node.js compat C++ layer; register new modules in NODEJS_MODULES macro in node.h +pyodide/ # Python Workers Pyodide/Emscripten bridge (7 files) +tests/ # JS integration tests (238 entries); each test = .js + .wd-test pair +``` + +## WHERE TO LOOK + +| Task | Files | +| ------------------------------ | ----------------------------------------------------- | +| fetch / Request / Response | `http.h`, `http.c++` | +| Headers | `headers.h`, `headers.c++` | +| WebSocket | `web-socket.h`, `web-socket.c++` | +| Hibernatable WS (DO) | `hibernatable-web-socket.h` | +| GlobalScope / addEventListener | `global-scope.h`, `global-scope.c++`, `basics.h` | +| JS RPC / Fetcher | `worker-rpc.h`, `worker-rpc.c++` | +| Durable Object state | `actor-state.h`, `actor.h` | +| Streams internals | `streams/` (see `streams/README.md` for architecture) | +| Crypto (WebCrypto) | `crypto/crypto.h`, `crypto/impl.h`, `crypto/keys.h` | +| Node.js compat (C++) | `node/node.h` (NODEJS_MODULES macro), per-module .h | +| URL (legacy vs standard) | `url.h` (legacy), `url-standard.h` (spec-compliant) | +| Encoding (TextEncoder/Dec) | `encoding.h`; stream variants in `streams/encoding.h` | +| Scheduled / Queue / Alarm | `scheduled.h`, `queue.h`, `actor-state.h` | +| HTMLRewriter | `html-rewriter.h` | +| R2 storage | `r2-bucket.h`, `r2-admin.h`, `r2-multipart.h` | +| KV / SyncKV | `kv.h`, `sync-kv.h` | +| SQL (DO) | `sql.h` | +| Body mixin (shared Req/Res) | `http.h` — `Body` class, `Body::Initializer` OneOf | + +## CONVENTIONS + +- `global-scope.h` forward-declares most API classes; `ServiceWorkerGlobalScope` registers all nested types +- Node.js compat: add C++ class + register in `NODEJS_MODULES(V)` macro in `node/node.h`; experimental modules go in `NODEJS_MODULES_EXPERIMENTAL(V)` +- URL has dual impl: legacy (`url.h`) vs standard (`url-standard.h`); compat flag selects which +- Streams has dual impl: internal (`streams/internal.h`) vs standard (`streams/standard.h`) +- Test naming: `tests/-test.js` + `tests/-test.wd-test`; C++ unit tests: `-test.c++` diff --git a/src/workerd/api/crypto/AGENTS.md b/src/workerd/api/crypto/AGENTS.md new file mode 100644 index 00000000000..6012f64d4c2 --- /dev/null +++ b/src/workerd/api/crypto/AGENTS.md @@ -0,0 +1,26 @@ +# src/workerd/api/crypto/ + +## OVERVIEW + +WebCrypto API + Node.js crypto C++ implementations over BoringSSL. `crypto.h` defines public JSG types (`CryptoKey`, `SubtleCrypto`, `CryptoKeyUsageSet`). `impl.h` defines internal `CryptoKey::Impl` base class with per-algorithm static `ImportFunc`/`GenerateFunc` dispatch. Algorithm files implement `Impl` subclasses. `OSSLCALL()` macro wraps all BoringSSL calls with error translation. + +## FILE MAP + +| File | Contents | +| ------------------------------------------------ | ----------------------------------------------------------------------------------------------------- | +| `crypto.h/c++` | Public API: `SubtleCrypto`, `CryptoKey`, `CryptoKeyPair`, `DigestStream`, `CryptoKeyUsageSet` bitmask | +| `impl.h/c++` | Internal base: `CryptoKey::Impl`, `interpretAlgorithmParam()`, OpenSSL error helpers, `OSSLCALL` | +| `keys.h/c++` | `AsymmetricKeyCryptoKeyImpl` base; `KeyEncoding`/`KeyFormat`/`KeyType` enums; generic import/export | +| `aes.c++` | AES-CBC, AES-CTR, AES-GCM, AES-KW | +| `rsa.h/c++` | RSA-OAEP, RSA-PSS, RSASSA-PKCS1-v1_5 | +| `ec.h/c++` | ECDSA, ECDH, EdDSA (Ed25519/X25519) | +| `dh.h/c++` | Diffie-Hellman key exchange (Node.js compat) | +| `digest.h/c++` | Hash/digest: SHA-\*, MD5, Blake2b512/2s256 | +| `kdf.h` + `hkdf.c++`, `pbkdf2.c++`, `scrypt.c++` | KDF declarations in header; each algo in own .c++ | +| `jwk.h/c++` | JWK import/export helpers | +| `x509.h/c++` | X.509 certificate parsing (Node.js compat) | +| `spkac.h/c++` | SPKAC/Netscape SPKI (Node.js compat) | +| `prime.h/c++` | Prime generation/checking (Node.js compat) | +| `crc-impl.h/c++` | CRC32/CRC32C (non-crypto checksum) | +| `endianness.h/c++` | Byte-swap utilities | +| `impl-test.c++`, `aes-test.c++` | C++ unit tests | diff --git a/src/workerd/api/node/AGENTS.md b/src/workerd/api/node/AGENTS.md new file mode 100644 index 00000000000..cb8183d7af1 --- /dev/null +++ b/src/workerd/api/node/AGENTS.md @@ -0,0 +1,22 @@ +# src/workerd/api/node/ + +C++ implementations of Node.js built-in modules. Each module = JSG-bound class registered via `NODEJS_MODULES(V)` macro in `node.h`. TypeScript counterpart lives in `src/node/`. + +## BUILD TARGETS + +- **`node-core`**: buffer, dns, i18n, sqlite, url. **No `//src/workerd/io` dep** — do not add one. Depends on Rust crates (`cxx-integration`, `dns`, `net`), `ada-url`, `nbytes`, `simdutf`. +- **`node`**: async-hooks, crypto, diagnostics-channel, module, process, timers, util, zlib-util. Depends on `io`, `ncrypto`, `zstd`, `kj-brotli`. Depends on `node-core`. +- **`exceptions`**: Standalone Node.js exception types; dep of `node-core`. + +## MODULE REGISTRATION + +1. Add `JSG_RESOURCE_TYPE` class in `.h` + `.c++` +2. Define `EW_NODE__ISOLATE_TYPES` macro in header +3. Add `V(ClassName, "node-internal:")` to `NODEJS_MODULES(V)` in `node.h` (or `NODEJS_MODULES_EXPERIMENTAL(V)` for staging) +4. Append to `EW_NODE_ISOLATE_TYPES` at bottom of `node.h` +5. Per-module compat flags gated in `registerNodeJsCompatModules()` — add `isNode*Module()` + `featureFlags.getEnable*()` check +6. DnsUtil special-cased: C++ fallback when `RUST_BACKED_NODE_DNS` autogate disabled + +## TESTING + +Tests in `tests/`. Naming: `-test.js` + `-test.wd-test`; `-nodejs-` infix when needing compat flags. All tests set `compatibilityFlags = ["nodejs_compat", "nodejs_compat_v2", "experimental"]`. Network tests (net, tls, http) use sidecar `js_binary` targets. `fixtures/` has 46 PEM files for crypto. `process-stdio` tests use `sh_test` with `.expected_stdout`/`.expected_stderr`. C++ unit test: `buffer-test.c++` via `kj_test`. diff --git a/src/workerd/api/streams/AGENTS.md b/src/workerd/api/streams/AGENTS.md new file mode 100644 index 00000000000..99fe7b46a96 --- /dev/null +++ b/src/workerd/api/streams/AGENTS.md @@ -0,0 +1,35 @@ +# src/workerd/api/streams/ + +Web Streams API: ReadableStream, WritableStream, TransformStream. **Read `README.md` for full architecture** (842 lines covering data flow, pipe loops, memory safety patterns). + +## ARCHITECTURE + +Dual implementation behind unified API: + +- **Internal** (`internal.h`): kj-backed, byte-only, single pending read, no JS queue +- **Standard** (`standard.h`): WHATWG spec, JS promises, value+byte, dual queues (data + pending reads) + +Controller pattern: `ReadableStream` → `ReadableStreamController` → impl-specific controller. +4 pipe loop variants (kj↔kj, kj↔JS, JS↔kj, JS↔JS) selected by stream type combination. +Tee uses `kj::Rc` shared refs (non-standard optimization, avoids data copies). + +Key deps: `util/state-machine.h` (stream state FSM), `util/weak-refs.h`, `util/ring-buffer.h`. + +## FILE MAP + +| File | Role | +| ----------------------------------- | --------------------------------------------------------------------------------------------------- | +| `readable.{h,c++}` | `ReadableStream` public API, `ReaderImpl`, readers | +| `writable.{h,c++}` | `WritableStream` public API, `WriterImpl`, writer | +| `transform.{h,c++}` | Standard `TransformStream` (JS algorithms) | +| `identity-transform-stream.{h,c++}` | Internal identity transform (byte-only, 1:1 read↔write) | +| `internal.{h,c++}` | `ReadableStreamInternalController`, `WritableStreamInternalController` | +| `standard.{h,c++}` | `ReadableStreamJsController`, `WritableStreamJsController` — main complexity (~5400 lines combined) | +| `readable-source.{h,c++}` | `ReadableStreamSource` — kj `AsyncInputStream` adapter | +| `writable-sink.{h,c++}` | `WritableStreamSink` — kj `AsyncOutputStream` adapter | +| `readable-source-adapter.{h,c++}` | Standard→Internal bridge (wraps JS controller as `ReadableStreamSource`) | +| `writable-sink-adapter.{h,c++}` | Standard→Internal bridge (wraps JS controller as `WritableStreamSink`) | +| `common.{h,c++}` | Shared types: `ReadResult`, `StreamStates`, controller interfaces | +| `queue.{h,c++}` | `ValueQueue`/`ByteQueue` for Standard stream backpressure | +| `compression.{h,c++}` | `CompressionStream`/`DecompressionStream` (zlib transforms) | +| `encoding.{h,c++}` | `TextEncoderStream`/`TextDecoderStream` | diff --git a/src/workerd/io/AGENTS.md b/src/workerd/io/AGENTS.md new file mode 100644 index 00000000000..9c7b8c4b4bc --- /dev/null +++ b/src/workerd/io/AGENTS.md @@ -0,0 +1,57 @@ +# src/workerd/io/ + +## OVERVIEW + +I/O lifecycle, per-request context, worker/isolate management, actor storage, consistency gates, and compatibility flags. + +## KEY CLASSES + +| Class | File | Role | +| ------------------------------------------- | ---------------------- | ------------------------------------------------------------------------------------------- | +| `IoContext` | `io-context.{h,c++}` | Per-request god object; thread-local via `IoContext::current()` | +| `IoContext::IncomingRequest` | `io-context.h` | Tracks one inbound request for metrics/tracing; actors have many per IoContext | +| `Worker` | `worker.{h,c++}` | Ref-counted worker instance; owns Script + Isolate refs | +| `Worker::Isolate` | `worker.h:337` | V8 isolate wrapper; shared across workers with same config | +| `Worker::Script` | `worker.h:250` | Compiled script bound to an Isolate | +| `Worker::Lock` | `worker.h:677` | Synchronous V8 isolate lock (must hold to touch JS heap) | +| `Worker::AsyncLock` | `worker.h:799` | Fair async queue for acquiring `Worker::Lock` | +| `Worker::Actor` | `worker.h:819` | Durable Object instance; owns gates + cache + hibernation state | +| `ActorCache` | `actor-cache.{h,c++}` | LRU write-back cache over RPC storage; `ActorCacheOps` base | +| `ActorSqlite` | `actor-sqlite.{h,c++}` | SQLite-backed `ActorCacheOps` implementation | +| `InputGate` / `OutputGate` | `io-gate.{h,c++}` | Consistency primitives for DO concurrent request handling | +| `IoOwn` / `IoPtr` / `ReverseIoOwn` | `io-own.{h,c++}` | Cross-heap smart pointers preventing KJ↔JS ref leaks | + +## WHERE TO LOOK + +| Task | File(s) | +| ------------------------------- | --------------------------------------------------------------------------------- | +| Promise bridging KJ↔JS | `io-context.h` — `awaitIo()`, `awaitJs()` | +| Request lifecycle / subrequests | `io-context.{h,c++}`, `worker-entrypoint.{h,c++}` | +| Actor storage ops | `actor-cache.h` (`ActorCacheOps`), `actor-sqlite.h`, `actor-storage.capnp` | +| DO gate semantics | `io-gate.{h,c++}` — `InputGate::CriticalSection`, `OutputGate::lockWhile()` | +| Worker/isolate creation | `worker.{h,c++}`, `worker-modules.{h,c++}` | +| Metrics/logging hooks | `observer.h` — `RequestObserver`, `IsolateObserver`, `ActorObserver` | +| Tracing | `trace.{h,c++,capnp}`, `trace-stream.{h,c++}`, `tracer.{h,c++}` | +| Resource limits | `limit-enforcer.h` (abstract interface) | +| Timer scheduling | `io-timers.{h,c++}` | +| Hibernatable WebSockets | `hibernation-manager.{h,c++}` | +| Cap'n Proto schemas | `worker-interface.capnp`, `actor-storage.capnp`, `container.capnp`, `trace.capnp` | + +## CONVENTIONS + +- `IoContext::current()` — ambient thread-local access; only valid inside a request +- `awaitIo(js, kjPromise, func)` bridges KJ→JS; `func` runs under V8 lock. `awaitIo(js, kjPromise)` for identity +- `awaitJs(js, jsPromise)` bridges JS→KJ +- `addObject(kj::Own)` returns `IoOwn` — the **only** safe way to store KJ I/O objects reachable from JS heap +- Two-phase locking: `Worker::AsyncLock` (fair queue) → `Worker::Lock` (V8 isolate lock) +- `ActorCacheOps` methods return `kj::OneOf>` — sync when cached, async otherwise +- `OutputGate::lockWhile(promise)` blocks outgoing responses until the promise resolves +- `InputGate::CriticalSection` must succeed or permanently breaks the gate +- Observer classes (`RequestObserver`, `IsolateObserver`, etc.) have no-op defaults; all methods optional + +## ANTI-PATTERNS + +- **NEVER** use `awaitIoLegacy()` in new code — use `awaitIo()` with continuation +- `awaitIoImpl` parameter ordering (promise by-value, func by-ref) is **critical** for exception safety +- `abortWhen()` promises must **never** enter the V8 isolate +- Cross-request I/O object access throws by design (IoOwn prevents this) diff --git a/src/workerd/jsg/AGENTS.md b/src/workerd/jsg/AGENTS.md new file mode 100644 index 00000000000..ce29923959e --- /dev/null +++ b/src/workerd/jsg/AGENTS.md @@ -0,0 +1,70 @@ +# JSG (JavaScript Glue) + +## OVERVIEW + +Macro-driven C++/V8 binding layer: declares C++ types as JS-visible resources/structs with automatic type conversion. + +## KEY FILES + +| File | Purpose | +| ---------------- | --------------------------------------------------------------------------------------------------------------------------- | +| `jsg.h` | Core header (3115 lines): all macros (`JSG_RESOURCE_TYPE`, `JSG_METHOD`, etc.), type mappings, `Lock`, `V8Ref`, `GcVisitor` | +| `resource.h` | Template metaprogramming: V8 callback generation, `FunctionCallbackInfo` dispatch, prototype/constructor wiring | +| `struct.h` | `JSG_STRUCT` value-type mapping: deep-copies C++ structs to/from JS objects | +| `wrappable.h` | GC integration: `Wrappable` base class, CppGC visitor hooks, ref marking, weak pointers | +| `promise.h` | `jsg::Promise` wrapping KJ promises ↔ JS promises; resolver pairs, coroutine integration | +| `modules.h` | `ModuleRegistry`: ESM/CJS module resolution, evaluation, top-level await handling | +| `modules-new.h` | Replacement module system (new design) | +| `setup.h` | `V8System`, `IsolateBase`, `JsgConfig`; process-level V8 init; `JSG_DECLARE_ISOLATE_TYPE` | +| `function.h` | `jsg::Function` wrapping C++ callables ↔ JS functions | +| `memory.h` | `MemoryTracker`, `JSG_MEMORY_INFO` macro; heap snapshot support | +| `rtti.capnp` | Cap'n Proto schema for type introspection; consumed by `types/` for TS generation | +| `rtti.h` | C++ RTTI builder: walks JSG type graph → `rtti.capnp` structures | +| `jsvalue.h` | `JsValue`, `JsObject`, `JsString`, etc. — typed wrappers over `v8::Value` | +| `type-wrapper.h` | `TypeWrapper` template: compile-time dispatch for C++ ↔ V8 conversions | +| `meta.h` | Argument unwrapping, `ArgumentContext`, parameter pack metaprogramming | +| `fast-api.h` | V8 Fast API call optimizations | +| `ser.h` | Structured clone: `Serializer`/`Deserializer` | +| `web-idl.h` | Web IDL types: `NonCoercible`, `Sequence`, etc. | +| `observer.h` | `IsolateObserver`, `CompilationObserver` — hooks for metrics/tracing | + +## BINDING PATTERN + +```cpp +class MyType: public jsg::Object { + static jsg::Ref constructor(kj::String s); + int getValue(); + void doThing(jsg::Lock& js, int n); + + JSG_RESOURCE_TYPE(MyType) { // or (MyType, CompatibilityFlags::Reader flags) + JSG_METHOD(doThing); + JSG_PROTOTYPE_PROPERTY(value, getValue, setvalue); + JSG_NESTED_TYPE(SubType); + JSG_STATIC_METHOD(create); + } + + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(ref, v8ref); // MUST trace all Ref/V8Ref/JsRef + } + + jsg::Ref ref; + jsg::V8Ref v8ref; +}; +``` + +- Allocate resource objects: `js.alloc(args...)` +- Value types: `JSG_STRUCT(field1, field2)` — auto-converted by value +- `jsg::Lock&` is the JS execution context; thread via method params +- `TypeHandler&` as trailing param gives manual conversion access +- Compat flags param on `JSG_RESOURCE_TYPE` gates members conditionally + +## ANTI-PATTERNS + +- **NEVER** opaque-wrap `V8Ref` — use handle directly +- **NEVER** use v8::Context embedder data slot 0 (`ContextPointerSlot::RESERVED`) +- **NEVER** hold traced handles (`Ref`/`V8Ref`) without marking in `visitForGc` — causes GC corruption +- **NEVER** use `FastOneByteString` in Fast API calls (GC corruption risk) +- **NEVER** unwrap `Ref` — use `V8Ref` instead +- `JSG_CATCH` is NOT a true catch — cannot rethrow with `throw` +- `NonCoercible` runs counter to Web IDL best practices; avoid in new APIs +- Rust JSG bindings: see `src/rust/jsg/` and `src/rust/jsg-macros/` diff --git a/src/workerd/server/AGENTS.md b/src/workerd/server/AGENTS.md new file mode 100644 index 00000000000..e9a3514e8bc --- /dev/null +++ b/src/workerd/server/AGENTS.md @@ -0,0 +1,30 @@ +# server/ + +## OVERVIEW + +Binary entry point + orchestration layer. `CliMain` (workerd.c++) dispatches subcommands (`serve`, `compile`, `test`, `fuzzilli`, `pyodide-lock`) via `kj::MainBuilder`. `Server` (server.c++, ~6K lines) is the god object: parses `workerd.capnp` config, constructs all service types as nested inner classes, wires sockets/bindings/actors, runs the event loop. + +## KEY FILES + +| File | Role | +| ------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `workerd.c++` | `CliMain`: subcommand dispatch, config loading, file watching, signal handling | +| `server.h/c++` | `Server` class: 15+ nested inner classes (`WorkerService`, `NetworkService`, `ExternalHttpService`, `DiskDirectoryService`, etc.). Two-phase init: `startServices()` then `listenOnSockets()` | +| `workerd.capnp` | Config schema: `Config`, `Service`, `Worker`, `Socket`, `Extension`. Capability-based security model | +| `workerd-api.h/c++` | `WorkerdApi`: registers all JS API types with JSG, compiles modules/globals, extracts source from config. `Global` struct has 20+ binding variants as `kj::OneOf` | +| `alarm-scheduler.h/c++` | DO alarm scheduling with SQLite-backed persistence | +| `json-logger.h/c++` | Structured JSON logging for tail workers | +| `channel-token.h/c++` | Opaque token encoding for cross-service channel references | +| `v8-platform-impl.h/c++` | Custom `v8::Platform` bridging V8 tasks to KJ event loop | +| `fallback-service.h/c++` | Module fallback resolution via external service | +| `container-client.h/c++` | Experimental (2025): Docker container lifecycle for DO containers | +| `docker-api.capnp` | Cap'n Proto schema for container management | +| `pyodide.h/c++` | Python worker preloading and snapshot management | + +## TEST INFRASTRUCTURE + +`server-test.c++` (~6K lines): integration tests using inline Cap'n Proto config strings. Tests full server lifecycle with real V8 isolates. + +`tests/server-harness.mjs`: Node.js harness spawning `workerd` child processes for E2E tests. Subdirectories: `compile-tests`, `container-client`, `extensions`, `inspector`, `python`, `structured-logging`, `unsafe-eval`, `unsafe-module`, `weakref`. + +Pattern: unit tests (`*-test.c++`) at directory level; integration/E2E tests in `tests/` using the harness. diff --git a/src/workerd/util/AGENTS.md b/src/workerd/util/AGENTS.md new file mode 100644 index 00000000000..e3458d4e0d0 --- /dev/null +++ b/src/workerd/util/AGENTS.md @@ -0,0 +1,43 @@ +# src/workerd/util/ + +## OVERVIEW + +Shared utility library: data structures, SQLite wrapper, feature gating, logging, thread-local scopes. No workerd-specific API dependencies — consumed across `api/`, `io/`, `server/`. + +## KEY UTILITIES + +| File | What | Notes | +| ----------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------------- | +| `state-machine.h` | Type-safe `kj::OneOf` wrapper with transition locking | Prevents UAF in callbacks; 7 consumers in `api/streams/` | +| `sqlite.h/c++` | Full SQLite wrapper with custom VFS over `kj::Directory` | `Regulator` controls allowed SQL; `Statement` for prepared queries | +| `sqlite-kv.h` | KV abstraction on SQLite | Used by Durable Object storage | +| `autogate.h/c++` | Runtime feature gates | `AutogateKey` enum; `isEnabled()` check; 8 active gates | +| `weak-refs.h` | `WeakRef` / `AtomicWeakRef` | Non-owning refs; `tryAddStrongRef()` or `runIfAlive(fn)` pattern | +| `ring-buffer.h` | Amortized O(1) deque | Replaces `std::list` in streams | +| `small-set.h` | `kj::OneOf`-based set | O(1) for 0–2 items, fallback to `kj::HashSet` | +| `batch-queue.h` | Double-buffered cross-thread queue | Producer/consumer with mutex swap | +| `checked-queue.h` | Safe `std::list` wrapper | `pop()` returns `Maybe` instead of UB on empty | +| `strong-bool.h` | `WD_STRONG_BOOL(Name)` macro | Type-safe boolean; prevents implicit conversions | +| `sentry.h` | `LOG_EXCEPTION`, `LOG_ONCE`, `LOG_PERIODICALLY` | `DEBUG_FATAL_RELEASE_LOG` = debug assert + release warning | +| `thread-scopes.h` | Thread-local scope flags | Self-described "horrible hacks"; `AllowV8BackgroundThreadsScope`, `MultiTenantProcess` | +| `abortable.h` | `newAbortableInputStream/OutputStream` | Wraps KJ streams with disconnect capability | +| `stream-utils.h` | `NeuterableInputStream`, `newNullInputStream` | Disconnectable I/O; null/identity stream factories | +| `mimetype.h` | MIME type parser/serializer | `MimeType::extract()` from content-type header | +| `wait-list.h` | Cross-request event subscription | Shared fulfiller list for signaling waiters | + +## Guidelines + +- **No bool arguments**: use `WD_STRONG_BOOL` for type safety and readability +- **Use `kj::Maybe` for optional values**: avoid null pointers and sentinel values +- **Prefer composition over inheritance**: utilities should be standalone and reusable without complex class hierarchies +- **Use `StateMachine` for state management**: original pattern has been to use `kj::OneOf` directly, and that's still acceptable for simple cases, but `StateMachine` provides additional safety guarantees and should be preferred for more complex state management + +## ANTI-PATTERNS + +- **StateMachine `forceTransitionTo()`**: bypasses terminal state protection — use only for error recovery +- **StateMachine `underlying()`**: bypasses ALL safety (transition lock, terminal states) — last resort only +- **StateMachine + `KJ_SWITCH_ONEOF`**: does NOT acquire transition lock — UAF risk; use `whenState(fn)` instead +- **StateMachine `deferTransitionTo()`**: first-wins semantics; second call silently ignored +- **SQLite**: `SQLITE_MISUSE` always throws; virtual tables disallowed (except FTS5); `ATTACH`/`DETACH` forbidden; callbacks must not write +- **ThreadScopes**: thread-local state crossing module boundaries — acknowledged hack, do not proliferate +- **RingBuffer**: moves on grow invalidating references; iterators invalidated on push/pop; intentionally not thread-safe. diff --git a/types/AGENTS.md b/types/AGENTS.md new file mode 100644 index 00000000000..0fe65d7816b --- /dev/null +++ b/types/AGENTS.md @@ -0,0 +1,25 @@ +# types/ + +## OVERVIEW + +Generates `@cloudflare/workers-types` `.d.ts` files from C++ RTTI (via `jsg/rtti.capnp`) + hand-written `defines/*.d.ts`. A workerd-hosted Worker extracts RTTI at runtime per compat-date; TypeScript transforms post-process into ambient and importable outputs. CI validates `generated-snapshot/` matches. + +## PIPELINE + +1. **RTTI extraction**: `src/worker/` runs inside workerd, imports binary RTTI via `workerd:rtti` + param names from `param-extractor.rs` +2. **Generation**: `src/generator/` converts Cap'n Proto structures to TS AST nodes (walks types, members, methods recursively via `collectIncluded`) +3. **Transforms** (ordered, in `src/index.ts`): iterators -> overrides/defines -> global scope extraction -> class-to-interface -> comments -> onmessage declaration; then second pass: import-resolve -> ambient. Importable variant runs separately. +4. **Defines concatenation**: `defines/*.d.ts` appended after transforms (AI, D1, R2, RPC, Vectorize, etc.) +5. **Build**: `scripts/build-types.ts` iterates compat-date entrypoints (oldest through experimental), type-checks output against TS libs + +## KEY DIRECTORIES + +| Directory | Purpose | +| --------------------- | -------------------------------------------------------------------------------- | +| `src/generator/` | RTTI-to-TS-AST: `structure.ts` (classes/interfaces), `type.ts` (type mapping) | +| `src/transforms/` | 11 post-processing TS transformer factories | +| `src/worker/` | Workerd-hosted entry; imports RTTI binary + virtual modules | +| `defines/` | Hand-written `.d.ts` for APIs not expressible via C++ RTTI (~25 files) | +| `generated-snapshot/` | Checked-in `latest/` + `experimental/` output; CI diff-checked | +| `scripts/` | `build-types.ts` (full generation), `build-worker.ts` (worker bundle) | +| `test/` | Vitest specs for generator, transforms, print; type-check tests in `test/types/` |