Align bonk review with local review process

[jasnell]

"Ok, from the point of view of the Bonk reviewer, tell me what guidelines you would follow for reviewing C++ code..."

Bonk's C++ Review Guidelines

As the Bonk reviewer, when reviewing C++ code in workerd PRs, I follow a layered set of guidelines drawn from the repo's review skills. Here's what I check, organized by priority:

1. Security (highest severity)
- Capability leaks / sandbox escapes — workerd runs untrusted code in production; any capability leak or sandbox escape is critical.
- Input validation gaps — especially at API boundaries where user-provided data enters the runtime.
- Injection vulnerabilities, timing side channels, TOCTOU issues, and privilege boundary violations.
- Memory safety — use-after-free, dangling pointers, buffer overflows, incorrect ownership/lifetime.

2. Memory Safety & Concurrency (critical/high)
Concrete patterns I flag immediately:
- V8 callback throwing C++ exception without liftKj — V8 callbacks must catch C++ exceptions and convert them to JS exceptions.
- jsg::Object holding kj I/O objects directly — must use IoOwn/IoPtr for correct lifetime and thread safety.
- kj::Refcounted in a cross-thread context — needs kj::AtomicRefcounted.
- Isolate lock held across co_await — holding jsg::Lock, v8::HandleScope, etc. across a suspension point is UB.
- RAII objects with raw pointers across co_await without kj::coCapture.
- Reference cycles between jsg::Object subclasses — causes invisible memory leaks.
- Lambda captures: [=] is never allowed; [&] only for non-escaping lambdas; escaping lambdas must capture explicitly.
- Missing KJ_LIFETIMEBOUND on methods returning non-owning views (kj::ArrayPtr, kj::StringPtr, references).
- Missing KJ_WARN_UNUSED_RESULT on functions returning error indicators, kj::Maybe, or expensive results.

3. Backward Compatibility (high severity)
- Any behavioral change must be gated behind a compatibility flag in compatibility-date.capnp. Ungated behavioral changes are flagged as high severity.
- Risky changes should additionally use an autogate (src/workerd/util/autogate.*) for staged rollout.
- Cap'n Proto schema changes: adding fields is fine; removing, renaming, or reordering breaks wire compatibility.

4. KJ Style & Idioms
- KJ types over STL — kj::String not std::string, kj::Own<T> not std::unique_ptr, etc. No STL headers in .h files.
- No raw new/delete — use kj::heap<T>(), kj::rc<T>(), etc.
- No throw — use KJ_ASSERT/KJ_REQUIRE/KJ_FAIL_ASSERT.
- No noexcept — explicit destructors must be noexcept(false).
- No singletons or mutable globals.
- No bool parameters — use enum class or WD_STRONG_BOOL.
- Naming: TitleCase types, camelCase functions/variables, CAPS constants.
- Prefer coroutines over nested .then() chains (but don't suggest sweeping rewrites).
- .attach() on promises must keep objects alive for the promise duration.
- .eagerlyEvaluate() required for background promises (lazy continuations may never run).
- // comments only, never /* */.
- Copyright header with the current year on new files.

5. JSG Bindings
- Correct type conversions between V8 and C++.
- GC safety: Ref<T> stored in C++ objects visible from the JS heap must implement visitForGc().
- Lambdas capturing jsg::Ref<T> should use JSG_VISITABLE_LAMBDA.
- Never put v8::Local/v8::Global in JSG_STRUCT fields or kj::Promise.
- Prefer JSG_PROTOTYPE_PROPERTY over JSG_INSTANCE_PROPERTY.

6. Performance
- Minimize unnecessary allocations/copies (tcmalloc-aware analysis).
- Correct data structure choices (e.g., workerd::RingBuffer for bounded queues).
- Flag lock contention and inefficient string operations.
- No vague performance claims — concrete evidence required.

7. Standards Compliance
- Adherence to web standards (Fetch, Streams, WebCrypto, etc.).
- Deviations from spec must be justified and documented.
- Reference specific spec sections when flagging issues.

What I do NOT flag
- Formatting issues — clang-format handles that.
- Subjective preferences — I'm pragmatic, not nitpicky.
- KJ-style destructors being noexcept(false) — that's intentional in this codebase.

Output behavior
- If no actionable issues: I just say LGTM.
- If there are issues: I rank them by severity, post batch review comments with suggestion blocks for concrete fixes, and lead with "I'm Bonk, and I've done a quick review of your PR."

[ask-bonk]

One small typo -- otherwise LGTM.

[jasnell]

/bigbonk roast this PR

[ask-bonk]

Now I have the actual diff. Time for the roast.

---

Alright, let's talk about this PR.

You wrote an entire essay in the PR body -- complete with a numbered manifesto of Bonk's C++ Review Guidelines -- to justify... checks notes ...replacing one paragraph with a bullet list that says "go read the skills."

The diff is 8 lines added, 5 removed. The PR description is roughly 3,000 characters. That's a description-to-code ratio of about 230:1. Most people pad their PRs to look more impressive; you've somehow managed to write a novella justifying the deletion of two short paragraphs about TypeScript and Rust review.

The actual change: you replaced a concrete sentence about "correct use of KJ patterns (kj::Own, kj::Maybe, kj::Promise, kj::Array)" with "use the kj-style skill" -- which is the code review equivalent of answering a question with "just Google it."

And then Bonk, your own creation, reviewed this PR about how Bonk should review PRs and found a typo. Your reviewer bot reviewed its own instructions and the only thing it caught was a doubled backtick. That's either poetic or a sign that Bonk is being too polite to its creator.

The deleted TypeScript and Rust paragraphs said:

• "Type correctness and consistency with the C++ implementation they describe."
• "Standard Rust review -- safety, error handling, no unnecessary unsafe."

These were perfectly fine one-liners. Now instead of having the guidance right there in the file, Bonk has to go load a skill first. You've added a layer of indirection to a markdown file. You've invented #include for prose.

Net assessment: the change is fine, it's just funny that it took more words to explain why Bonk should read the manual than it took to write the manual reference.

github run