Code Review

<!--
This file is generated by tools/skills.
Do not edit this file directly.
Edit .ai/skills/code-review.md and run `make generate-ai`.
-->

Skill: Code Review

Purpose

Perform senior-level code review on changes in this repository: judge architecture and design first, then correctness, security, testability, and simplicity. The goal is to protect the project's boundaries and quality, not to nitpick syntax.

When to Use This Skill

• Reviewing a pull request, diff, or another agent's proposed change.
• Self-reviewing your own change before declaring a task complete.
• Evaluating whether a change is safe to merge or must be sent back.

Files to Read First

• AGENTS.md (especially §3 non-negotiable architecture rules).
• docs/ARCHITECTURE.md and the relevant layer docs (RUNTIME_LIFECYCLE.md, YAML_SPEC.md, PROMPT_RENDERING.md, SIDECAR_CONTEXT_AUTH.md, MCP_AND_TOOLS.md).
• docs/adr/ for any contract the change touches.
• .ai/skills/architecture-review.md if the change affects architecture.
• The task file in tasks/ the change claims to implement.

Core Principles

• Review architecture before syntax. A clean diff in the wrong layer is still wrong.
• Boundaries are non-negotiable. Validation, compilation, runtime, prompt rendering, plugin context/access, and tools stay separated.
• Security is enforced outside prompts. Prompt wording is never a boundary.
• Simplicity wins. Reject unnecessary abstraction; prefer the smallest design that works.
• Behavior must be tested, and tests must assert behavior, not internals.

Process

1. Understand intent. Read the task/PR description and confirm the change is in scope for that task. Out-of-scope churn is a finding.
2. Architecture & separation of concerns. Confirm code lives in the correct src/agentplatform/<layer> package and does not cross boundaries (e.g. the runtime reading raw YAML, or client/business logic leaking into the runtime).
3. Lifecycle/state. Verify runtime/startup state is separated from per-request state: nothing request-scoped on RuntimeEngine or the compiled graph; per-request data lives on ExecutionContext.
4. Public interface. Check that public functions/classes have clear, typed, minimal signatures and stable contracts; private details are not leaked.
5. Errors. Confirm errors are typed/actionable and name what went wrong (e.g. which YAML key, which missing prompt variable), not bare exceptions.
6. Security. Confirm permission/tool-policy enforcement happens at the tool/data layer; injected params can't be overridden; no secrets hardcoded; secrets redacted in traces/logs.
7. Testability & tests. Confirm the code is testable (DI, no hidden globals) and that tests cover behavior and edge/negative cases, not private implementation details.
8. Simplicity & abstractions. Flag premature abstractions, clever code, and layers collapsed "for convenience".
9. Backward compatibility. If a public contract (YAML schema, plugin contract, API shape) changed, require an ADR and check for breakage.
10. Write the structured report (below).