takt: # タスク指示書: 専門知識のknowledgeへの抽出と付与

## 概要
既存のスタンス/インストラクションに埋め込まれているフロントエンド・バックエンド等の専門知識をknowledgeファイルとして抽出し、抽出元に適切に付与する。

---

## タスク

### 1. 専門知識の抽出(優先度: 高)

既存のスタンス・インストラクションファイルをレビューし、以下の専門知識を特定・抽出:
- **フロントエンド知識**(React、CSS、UI/UXなど)
- **バックエンド知識**(API設計、DB、サーバーサイドなど)
- **その他の専門知識**(発見したもの)

抽出した知識をknowledgeファイルとして作成する。

### 2. 抽出元への付与(優先度: 高)

抽出した知識を、元々その知識を使用していたスタンス/インストラクションに付与設定する。
- 抽出元 = 付与先

---

## 確認方法
- 抽出後、元のスタンス/インストラクションから専門知識が分離されていること
- 抽出元にknowledgeが正しく付与設定されていること
This commit is contained in:
nrslib 2026-02-07 13:01:15 +09:00
parent e7d5dbfb33
commit b7c2a4db08
24 changed files with 3050 additions and 2733 deletions

View File

@ -0,0 +1,427 @@
# Architecture Knowledge
## Structure & Design
**File Organization:**
| Criteria | Judgment |
|----------|----------|
| Single file > 200 lines | Consider splitting |
| Single file > 300 lines | REJECT |
| Single file with multiple responsibilities | REJECT |
| Unrelated code coexisting | REJECT |
**Module Structure:**
- High cohesion: Related functionality grouped together
- Low coupling: Minimal inter-module dependencies
- No circular dependencies
- Appropriate directory hierarchy
**Function Design:**
- One responsibility per function
- Consider splitting functions over 30 lines
- Side effects clearly defined
**Layer Design:**
- Dependency direction: Upper layers -> Lower layers (reverse prohibited)
- Controller -> Service -> Repository flow maintained
- 1 interface = 1 responsibility (no giant Service classes)
**Directory Structure:**
Structure pattern selection:
| Pattern | Use Case | Example |
|---------|----------|---------|
| Layered | Small scale, CRUD-centric | `controllers/`, `services/`, `repositories/` |
| Vertical Slice | Medium-large scale, high feature independence | `features/auth/`, `features/order/` |
| Hybrid | Common foundation + feature modules | `core/` + `features/` |
Vertical Slice Architecture (organizing code by feature):
```
src/
├── features/
│ ├── auth/
│ │ ├── LoginCommand.ts
│ │ ├── LoginHandler.ts
│ │ ├── AuthRepository.ts
│ │ └── auth.test.ts
│ └── order/
│ ├── CreateOrderCommand.ts
│ ├── CreateOrderHandler.ts
│ └── ...
└── shared/ # Shared across features
├── database/
└── middleware/
```
Vertical Slice criteria:
| Criteria | Judgment |
|----------|----------|
| Single feature spans 3+ layers | Consider slicing |
| Minimal inter-feature dependencies | Recommend slicing |
| Over 50% shared processing | Keep layered |
| Team organized by features | Slicing required |
Prohibited patterns:
| Pattern | Problem |
|---------|---------|
| Bloated `utils/` | Becomes graveyard of unclear responsibilities |
| Lazy placement in `common/` | Dependencies become unclear |
| Excessive nesting (4+ levels) | Navigation difficulty |
| Mixed features and layers | `features/services/` prohibited |
**Separation of Concerns:**
- Read and write responsibilities separated
- Data fetching at root (View/Controller), passed to children
- Error handling centralized (no try-catch scattered everywhere)
- Business logic not leaking into Controller/View
## Code Quality Detection
**Explanatory Comment (What/How) Detection Criteria:**
Detect comments that simply restate code behavior in natural language.
| Judgment | Criteria |
|----------|----------|
| REJECT | Restates code behavior in natural language |
| REJECT | Repeats what is already obvious from function/variable names |
| REJECT | JSDoc that only paraphrases the function name without adding information |
| OK | Explains why a particular implementation was chosen |
| OK | Explains the reason behind seemingly unusual behavior |
| Best | No comment needed — the code itself communicates intent |
```typescript
// REJECT - Restates code (What)
// If interrupted, abort immediately
if (status === 'interrupted') {
return ABORT_STEP;
}
// REJECT - Restates the loop
// Check transitions in order
for (const transition of step.transitions) {
// REJECT - Repeats the function name
/** Check if status matches transition condition. */
export function matchesCondition(status: Status, condition: TransitionCondition): boolean {
// OK - Design decision (Why)
// User interruption takes priority over piece-defined transitions
if (status === 'interrupted') {
return ABORT_STEP;
}
// OK - Reason behind seemingly odd behavior
// stay can cause loops, but is only used when explicitly specified by the user
return step.name;
```
**Direct State Mutation Detection Criteria:**
Detect direct mutation of arrays or objects.
```typescript
// REJECT - Direct array mutation
const steps: Step[] = getSteps();
steps.push(newStep); // Mutates original array
steps.splice(index, 1); // Mutates original array
steps[0].status = 'done'; // Nested object also mutated directly
// OK - Immutable operations
const withNew = [...steps, newStep];
const without = steps.filter((_, i) => i !== index);
const updated = steps.map((s, i) =>
i === 0 ? { ...s, status: 'done' } : s
);
// REJECT - Direct object mutation
function updateConfig(config: Config) {
config.logLevel = 'debug'; // Mutates argument directly
config.steps.push(newStep); // Nested mutation too
return config;
}
// OK - Returns new object
function updateConfig(config: Config): Config {
return {
...config,
logLevel: 'debug',
steps: [...config.steps, newStep],
};
}
```
## Security (Basic Checks)
- Injection prevention (SQL, Command, XSS)
- User input validation
- Hardcoded sensitive information
## Testability
- Dependency injection enabled
- Mockable design
- Tests are written
## Anti-Pattern Detection
REJECT when these patterns are found:
| Anti-Pattern | Problem |
|--------------|---------|
| God Class/Component | Single class with too many responsibilities |
| Feature Envy | Frequently accessing other modules' data |
| Shotgun Surgery | Single change ripples across multiple files |
| Over-generalization | Variants and extension points not currently needed |
| Hidden Dependencies | Child components implicitly calling APIs etc. |
| Non-idiomatic | Custom implementation ignoring language/FW conventions |
## Abstraction Level Evaluation
**Conditional Branch Proliferation Detection:**
| Pattern | Judgment |
|---------|----------|
| Same if-else pattern in 3+ places | Abstract with polymorphism → REJECT |
| switch/case with 5+ branches | Consider Strategy/Map pattern |
| Flag arguments changing behavior | Split into separate functions → REJECT |
| Type-based branching (instanceof/typeof) | Replace with polymorphism → REJECT |
| Nested conditionals (3+ levels) | Early return or extract → REJECT |
**Abstraction Level Mismatch Detection:**
| Pattern | Problem | Fix |
|---------|---------|-----|
| Low-level details in high-level processing | Hard to read | Extract details to functions |
| Mixed abstraction levels in one function | Cognitive load | Align to same granularity |
| DB operations mixed with business logic | Responsibility violation | Separate to Repository layer |
| Config values mixed with processing logic | Hard to change | Externalize configuration |
**Good Abstraction Examples:**
```typescript
// Proliferating conditionals
function process(type: string) {
if (type === 'A') { /* process A */ }
else if (type === 'B') { /* process B */ }
else if (type === 'C') { /* process C */ }
// ...continues
}
// Abstract with Map pattern
const processors: Record<string, () => void> = {
A: processA,
B: processB,
C: processC,
};
function process(type: string) {
processors[type]?.();
}
```
```typescript
// Mixed abstraction levels
function createUser(data: UserData) {
// High level: business logic
validateUser(data);
// Low level: DB operation details
const conn = await pool.getConnection();
await conn.query('INSERT INTO users...');
conn.release();
}
// Aligned abstraction levels
function createUser(data: UserData) {
validateUser(data);
await userRepository.save(data); // Details hidden
}
```
## Workaround Detection
Don't overlook compromises made to "just make it work."
| Pattern | Example |
|---------|---------|
| Unnecessary package additions | Mystery libraries added just to make things work |
| Test deletion/skipping | `@Disabled`, `.skip()`, commented out |
| Empty implementations/stubs | `return null`, `// TODO: implement`, `pass` |
| Mock data in production | Hardcoded dummy data |
| Swallowed errors | Empty `catch {}`, `rescue nil` |
| Magic numbers | Unexplained `if (status == 3)` |
## Strict TODO Comment Prohibition
"We'll do it later" never gets done. What's not done now is never done.
TODO comments are immediate REJECT.
```kotlin
// REJECT - Future-looking TODO
// TODO: Add authorization check by facility ID
fun deleteCustomHoliday(@PathVariable id: String) {
deleteCustomHolidayInputPort.execute(input)
}
// APPROVE - Implement now
fun deleteCustomHoliday(@PathVariable id: String) {
val currentUserFacilityId = getCurrentUserFacilityId()
val holiday = findHolidayById(id)
require(holiday.facilityId == currentUserFacilityId) {
"Cannot delete holiday from another facility"
}
deleteCustomHolidayInputPort.execute(input)
}
```
Only acceptable TODO cases:
| Condition | Example | Judgment |
|-----------|---------|----------|
| External dependency prevents implementation + Issued | `// TODO(#123): Implement after API key obtained` | Acceptable |
| Technical constraint prevents + Issued | `// TODO(#456): Waiting for library bug fix` | Acceptable |
| "Future implementation", "add later" | `// TODO: Add validation` | REJECT |
| "No time for now" | `// TODO: Refactor` | REJECT |
Correct handling:
- Needed now → Implement now
- Not needed now → Delete the code
- External blocker → Create issue and include ticket number in comment
## DRY Violation Detection
Detect duplicate code.
| Pattern | Judgment |
|---------|----------|
| Same logic in 3+ places | Immediate REJECT - Extract to function/method |
| Same validation in 2+ places | Immediate REJECT - Extract to validator function |
| Similar components 3+ | Immediate REJECT - Create shared component |
| Copy-paste derived code | Immediate REJECT - Parameterize or abstract |
AHA principle (Avoid Hasty Abstractions) balance:
- 2 duplications → Wait and see
- 3 duplications → Extract immediately
- Different domain duplications → Don't abstract (e.g., customer validation vs admin validation are different)
## Spec Compliance Verification
Verify that changes comply with the project's documented specifications.
Verification targets:
| Target | What to Check |
|--------|---------------|
| CLAUDE.md / README.md | Conforms to schema definitions, design principles, constraints |
| Type definitions / Zod schemas | New fields reflected in schemas |
| YAML/JSON config files | Follows documented format |
Specific checks:
1. When config files (YAML, etc.) are modified or added:
- Cross-reference with schema definitions in CLAUDE.md, etc.
- No ignored or invalid fields present
- No required fields missing
2. When type definitions or interfaces are modified:
- Documentation schema descriptions are updated
- Existing config files are compatible with new schema
REJECT when these patterns are found:
| Pattern | Problem |
|---------|---------|
| Fields not in the spec | Ignored or unexpected behavior |
| Invalid values per spec | Runtime error or silently ignored |
| Violation of documented constraints | Against design intent |
## Call Chain Verification
When new parameters/fields are added, verify not just the changed file but also callers.
Verification steps:
1. When finding new optional parameters or interface fields, `Grep` all callers
2. Check if all callers pass the new parameter
3. If fallback value (`?? default`) exists, verify if fallback is used as intended
Danger patterns:
| Pattern | Problem | Detection |
|---------|---------|-----------|
| `options.xxx ?? fallback` where all callers omit `xxx` | Feature implemented but always falls back | grep callers |
| Tests set values directly with mocks | Don't go through actual call chain | Check test construction |
| `executeXxx()` doesn't receive `options` it uses internally | No route to pass value from above | Check function signature |
```typescript
// Missing wiring: No route to receive projectCwd
export async function executePiece(config, cwd, task) {
const engine = new PieceEngine(config, cwd, task); // No options
}
// Wired: Can pass projectCwd
export async function executePiece(config, cwd, task, options?) {
const engine = new PieceEngine(config, cwd, task, options);
}
```
Logically dead code due to caller constraints:
Call chain verification applies not only to "missing wiring" but also to the reverse — unnecessary guards for conditions that callers already guarantee.
| Pattern | Problem | Detection |
|---------|---------|-----------|
| TTY check when all callers require TTY | Unreachable branch remains | grep all callers' preconditions |
| Null guard when callers already check null | Redundant defense | Trace caller constraints |
| Runtime type check when TypeScript types constrain | Not trusting type safety | Check TypeScript type constraints |
Verification steps:
1. When finding defensive branches (TTY check, null guard, etc.), grep all callers
2. If all callers already guarantee the condition, guard is unnecessary → REJECT
3. If some callers don't guarantee it, keep the guard
## Quality Attributes
| Attribute | Review Point |
|-----------|--------------|
| Scalability | Design handles increased load |
| Maintainability | Easy to modify and fix |
| Observability | Logging and monitoring enabled |
## Big Picture
Don't get lost in minor "clean code" nitpicks.
Verify:
- How will this code evolve in the future
- Is scaling considered
- Is technical debt being created
- Does it align with business requirements
- Is naming consistent with the domain
## Change Scope Assessment
Check change scope and include in report (non-blocking).
| Scope Size | Lines Changed | Action |
|------------|---------------|--------|
| Small | ~200 lines | Review as-is |
| Medium | 200-500 lines | Review as-is |
| Large | 500+ lines | Continue review. Suggest splitting if possible |
Note: Some tasks require large changes. Don't REJECT based on line count alone.
Verify:
- Changes are logically cohesive (no unrelated changes mixed in)
- Coder's scope declaration matches actual changes
Include as suggestions (non-blocking):
- If splittable, present splitting proposal

View File

@ -0,0 +1,417 @@
# CQRS+ES Knowledge
## Aggregate Design
Aggregates hold only fields necessary for decision-making.
Command Model (Aggregate) role is to "receive commands, make decisions, and emit events". Query data is handled by Read Model (Projection).
"Necessary for decision" means:
- Used in `if`/`require` conditional branches
- Field value referenced when emitting events in instance methods
| Criteria | Judgment |
|----------|----------|
| Aggregate spans multiple transaction boundaries | REJECT |
| Direct references between Aggregates (not ID references) | REJECT |
| Aggregate exceeds 100 lines | Consider splitting |
| Business invariants exist outside Aggregate | REJECT |
| Holding fields not used for decisions | REJECT |
Good Aggregate:
```kotlin
// Only fields necessary for decisions
data class Order(
val orderId: String, // Used when emitting events
val status: OrderStatus // Used for state checking
) {
fun confirm(confirmedBy: String): OrderConfirmedEvent {
require(status == OrderStatus.PENDING) { "Cannot confirm in this state" }
return OrderConfirmedEvent(
orderId = orderId,
confirmedBy = confirmedBy,
confirmedAt = LocalDateTime.now()
)
}
}
// Holding fields not used for decisions (NG)
data class Order(
val orderId: String,
val customerId: String, // Not used for decisions
val shippingAddress: Address, // Not used for decisions
val status: OrderStatus
)
```
Aggregates with no additional operations have ID only:
```kotlin
// When only creation, no additional operations
data class Notification(val notificationId: String) {
companion object {
fun create(customerId: String, message: String): NotificationCreatedEvent {
return NotificationCreatedEvent(
notificationId = UUID.randomUUID().toString(),
customerId = customerId,
message = message
)
}
}
}
```
## Event Design
| Criteria | Judgment |
|----------|----------|
| Event not in past tense (Created → Create) | REJECT |
| Event contains logic | REJECT |
| Event contains internal state of other Aggregates | REJECT |
| Event schema not version controlled | Warning |
| CRUD-style events (Updated, Deleted) | Needs review |
Good Events:
```kotlin
// Good: Domain intent is clear
OrderPlaced, PaymentReceived, ItemShipped
// Bad: CRUD style
OrderUpdated, OrderDeleted
```
Event Granularity:
- Too fine: `OrderFieldChanged` → Domain intent unclear
- Appropriate: `ShippingAddressChanged` → Intent is clear
- Too coarse: `OrderModified` → What changed is unclear
## Command Handlers
| Criteria | Judgment |
|----------|----------|
| Handler directly manipulates DB | REJECT |
| Handler modifies multiple Aggregates | REJECT |
| No command validation | REJECT |
| Handler executes queries to make decisions | Needs review |
Good Command Handler:
```
1. Receive command
2. Restore Aggregate from event store
3. Apply command to Aggregate
4. Save emitted events
```
## Projection Design
| Criteria | Judgment |
|----------|----------|
| Projection issues commands | REJECT |
| Projection references Write model | REJECT |
| Single projection serves multiple use cases | Needs review |
| Design that cannot be rebuilt | REJECT |
Good Projection:
- Optimized for specific read use case
- Idempotently reconstructible from events
- Completely independent from Write model
## Query Side Design
Controller uses QueryGateway. Does not use Repository directly.
Types between layers:
- `application/query/` - Query result types (e.g., `OrderDetail`)
- `adapter/protocol/` - REST response types (e.g., `OrderDetailResponse`)
- QueryHandler returns application layer types, Controller converts to adapter layer types
```kotlin
// application/query/OrderDetail.kt
data class OrderDetail(
val orderId: String,
val customerName: String,
val totalAmount: Money
)
// adapter/protocol/OrderDetailResponse.kt
data class OrderDetailResponse(...) {
companion object {
fun from(detail: OrderDetail) = OrderDetailResponse(...)
}
}
// QueryHandler - returns application layer type
@QueryHandler
fun handle(query: GetOrderDetailQuery): OrderDetail? {
val entity = repository.findById(query.id) ?: return null
return OrderDetail(...)
}
// Controller - converts to adapter layer type
@GetMapping("/{id}")
fun getById(@PathVariable id: String): ResponseEntity<OrderDetailResponse> {
val detail = queryGateway.query(
GetOrderDetailQuery(id),
OrderDetail::class.java
).join() ?: throw NotFoundException("...")
return ResponseEntity.ok(OrderDetailResponse.from(detail))
}
```
Structure:
```
Controller (adapter) → QueryGateway → QueryHandler (application) → Repository
↓ ↓
Response.from(detail) OrderDetail
```
## Eventual Consistency
| Situation | Response |
|-----------|----------|
| UI expects immediate updates | Redesign or polling/WebSocket |
| Consistency delay exceeds tolerance | Reconsider architecture |
| Compensating transactions undefined | Request failure scenario review |
## Saga vs EventHandler
Saga is used only for "operations between multiple aggregates where contention occurs".
Cases where Saga is needed:
```
When multiple actors compete for the same resource
Example: Inventory reservation (10 people ordering the same product simultaneously)
OrderPlacedEvent
↓ InventoryReservationSaga
ReserveInventoryCommand → Inventory aggregate (serializes concurrent execution)
InventoryReservedEvent → ConfirmOrderCommand
InventoryReservationFailedEvent → CancelOrderCommand
```
Cases where Saga is not needed:
```
Non-competing operations
Example: Inventory release on order cancellation
OrderCancelledEvent
↓ InventoryReleaseHandler (simple EventHandler)
ReleaseInventoryCommand
InventoryReleasedEvent
```
Decision criteria:
| Situation | Saga | EventHandler |
|-----------|------|--------------|
| Resource contention exists | Use | - |
| Compensating transaction needed | Use | - |
| Non-competing simple coordination | - | Use |
| Retry on failure is sufficient | - | Use |
Anti-pattern:
```kotlin
// NG - Using Saga for lifecycle management
@Saga
class OrderLifecycleSaga {
// Tracking all order state transitions in Saga
// PLACED → CONFIRMED → SHIPPED → DELIVERED
}
// OK - Saga only for operations requiring eventual consistency
@Saga
class InventoryReservationSaga {
// Only for inventory reservation concurrency control
}
```
Saga is not a lifecycle management tool. Create it per "operation" that requires eventual consistency.
## Exception vs Event (Failure Handling)
Failures not requiring audit use exceptions, failures requiring audit use events.
Exception approach (recommended: most cases):
```kotlin
// Domain model: Throws exception on validation failure
fun reserveInventory(orderId: String, quantity: Int): InventoryReservedEvent {
if (availableQuantity < quantity) {
throw InsufficientInventoryException("Insufficient inventory")
}
return InventoryReservedEvent(productId, orderId, quantity)
}
// Saga: Catch with exceptionally and perform compensating action
commandGateway.send<Any>(command)
.exceptionally { ex ->
commandGateway.send<Any>(CancelOrderCommand(
orderId = orderId,
reason = ex.cause?.message ?: "Inventory reservation failed"
))
null
}
```
Event approach (rare cases):
```kotlin
// Only when audit is required
data class PaymentFailedEvent(
val paymentId: String,
val reason: String,
val attemptedAmount: Money
) : PaymentEvent
```
Decision criteria:
| Question | Exception | Event |
|----------|-----------|-------|
| Need to check this failure later? | No | Yes |
| Required by regulations/compliance? | No | Yes |
| Only Saga cares about the failure? | Yes | No |
| Is there value in keeping it in Event Store? | No | Yes |
Default is exception approach. Consider events only when audit requirements exist.
## Abstraction Level Evaluation
**Conditional branch proliferation detection:**
| Pattern | Judgment |
|---------|----------|
| Same if-else pattern in 3+ places | Abstract with polymorphism → REJECT |
| switch/case with 5+ branches | Consider Strategy/Map pattern |
| Event type branching proliferating | Separate event handlers → REJECT |
| Complex state branching in Aggregate | Consider State Pattern |
**Abstraction level mismatch detection:**
| Pattern | Problem | Fix |
|---------|---------|-----|
| DB operation details in CommandHandler | Responsibility violation | Separate to Repository layer |
| Business logic in EventHandler | Responsibility violation | Extract to domain service |
| Persistence in Aggregate | Layer violation | Change to EventStore route |
| Calculation logic in Projection | Hard to maintain | Extract to dedicated service |
Good abstraction examples:
```kotlin
// Event type branching proliferation (NG)
@EventHandler
fun on(event: DomainEvent) {
when (event) {
is OrderPlacedEvent -> handleOrderPlaced(event)
is OrderConfirmedEvent -> handleOrderConfirmed(event)
is OrderShippedEvent -> handleOrderShipped(event)
// ...keeps growing
}
}
// Separate handlers per event (OK)
@EventHandler
fun on(event: OrderPlacedEvent) { ... }
@EventHandler
fun on(event: OrderConfirmedEvent) { ... }
@EventHandler
fun on(event: OrderShippedEvent) { ... }
```
```kotlin
// Complex state branching (NG)
fun process(command: ProcessCommand) {
when (status) {
PENDING -> if (command.type == "approve") { ... } else if (command.type == "reject") { ... }
APPROVED -> if (command.type == "ship") { ... }
// ...gets complex
}
}
// Abstracted with State Pattern (OK)
sealed class OrderState {
abstract fun handle(command: ProcessCommand): List<DomainEvent>
}
class PendingState : OrderState() {
override fun handle(command: ProcessCommand) = when (command) {
is ApproveCommand -> listOf(OrderApprovedEvent(...))
is RejectCommand -> listOf(OrderRejectedEvent(...))
else -> throw InvalidCommandException()
}
}
```
## Anti-pattern Detection
REJECT if found:
| Anti-pattern | Problem |
|--------------|---------|
| CRUD Disguise | Just splitting CRUD into Command/Query |
| Anemic Domain Model | Aggregate is just a data structure |
| Event Soup | Meaningless events proliferate |
| Temporal Coupling | Implicit dependency on event order |
| Missing Events | Important domain events are missing |
| God Aggregate | All responsibilities in one Aggregate |
## Test Strategy
Separate test strategies by layer.
Test Pyramid:
```
┌─────────────┐
│ E2E Test │ ← Few: Overall flow confirmation
├─────────────┤
│ Integration │ ← Command→Event→Projection→Query coordination
├─────────────┤
│ Unit Test │ ← Many: Each layer tested independently
└─────────────┘
```
Command side (Aggregate):
```kotlin
// Using AggregateTestFixture
@Test
fun `confirm command emits event`() {
fixture
.given(OrderPlacedEvent(...))
.`when`(ConfirmOrderCommand(orderId, confirmedBy))
.expectSuccessfulHandlerExecution()
.expectEvents(OrderConfirmedEvent(...))
}
```
Query side:
```kotlin
// Direct Read Model setup + QueryGateway
@Test
fun `can get order details`() {
// Given: Setup Read Model directly
orderRepository.save(OrderEntity(...))
// When: Execute query via QueryGateway
val detail = queryGateway.query(GetOrderDetailQuery(orderId), ...).join()
// Then
assertEquals(expectedDetail, detail)
}
```
Checklist:
| Aspect | Judgment |
|--------|----------|
| Aggregate tests verify events not state | Required |
| Query side tests don't create data via Command | Recommended |
| Integration tests consider Axon async processing | Required |
## Infrastructure Layer
Check:
- Is event store choice appropriate?
- Does messaging infrastructure meet requirements?
- Is snapshot strategy defined?
- Is event serialization format appropriate?

View File

@ -0,0 +1,497 @@
# Frontend Knowledge
## Component Design
Do not write everything in one file. Always split components.
Required splits:
- Has its own state → Must split
- JSX over 50 lines → Split
- Reusable → Split
- Multiple responsibilities → Split
- Independent section within page → Split
| Criteria | Judgment |
|----------|----------|
| Component over 200 lines | Consider splitting |
| Component over 300 lines | REJECT |
| Display and logic mixed | Consider separation |
| Props drilling (3+ levels) | Consider state management |
| Component with multiple responsibilities | REJECT |
Good Component:
- Single responsibility: Does one thing well
- Self-contained: Dependencies are clear
- Testable: Side effects are isolated
Component Classification:
| Type | Responsibility | Example |
|------|----------------|---------|
| Container | Data fetching, state management | `UserListContainer` |
| Presentational | Display only | `UserCard` |
| Layout | Arrangement, structure | `PageLayout`, `Grid` |
| Utility | Common functionality | `ErrorBoundary`, `Portal` |
Directory Structure:
```
features/{feature-name}/
├── components/
│ ├── {feature}-view.tsx # Main view (composes children)
│ ├── {sub-component}.tsx # Sub-components
│ └── index.ts
├── hooks/
├── types.ts
└── index.ts
```
## State Management
Child components do not modify their own state. They bubble events to parent, and parent manipulates state.
```tsx
// ❌ Child modifies its own state
const ChildBad = ({ initialValue }: { initialValue: string }) => {
const [value, setValue] = useState(initialValue)
return <input value={value} onChange={e => setValue(e.target.value)} />
}
// ✅ Parent manages state, child notifies via callback
const ChildGood = ({ value, onChange }: { value: string; onChange: (v: string) => void }) => {
return <input value={value} onChange={e => onChange(e.target.value)} />
}
const Parent = () => {
const [value, setValue] = useState('')
return <ChildGood value={value} onChange={setValue} />
}
```
Exception (OK for child to have local state):
- UI-only temporary state (hover, focus, animation)
- Completely local state that doesn't need to be communicated to parent
| Criteria | Judgment |
|----------|----------|
| Unnecessary global state | Consider localizing |
| Same state managed in multiple places | Needs normalization |
| State changes from child to parent (reverse data flow) | REJECT |
| API response stored as-is in state | Consider normalization |
| Inappropriate useEffect dependencies | REJECT |
State Placement Guidelines:
| State Nature | Recommended Placement |
|--------------|----------------------|
| Temporary UI state (modal open/close, etc.) | Local (useState) |
| Form input values | Local or form library |
| Shared across multiple components | Context or state management library |
| Server data cache | Data fetching library (TanStack Query, etc.) |
## Data Fetching
API calls are made in root (View) components and passed to children via props.
```tsx
// ✅ CORRECT - Fetch at root, pass to children
const OrderDetailView = () => {
const { data: order, isLoading, error } = useGetOrder(orderId)
const { data: items } = useListOrderItems(orderId)
if (isLoading) return <Skeleton />
if (error) return <ErrorDisplay error={error} />
return (
<OrderSummary
order={order}
items={items}
onItemSelect={handleItemSelect}
/>
)
}
// ❌ WRONG - Child fetches its own data
const OrderSummary = ({ orderId }) => {
const { data: order } = useGetOrder(orderId)
// ...
}
```
When UI state changes affect parameters (week switching, filters, etc.):
Manage state at View level and pass callbacks to components.
```tsx
// ✅ CORRECT - State managed at View level
const ScheduleView = () => {
const [currentWeek, setCurrentWeek] = useState(startOfWeek(new Date()))
const { data } = useListSchedules({
from: format(currentWeek, 'yyyy-MM-dd'),
to: format(endOfWeek(currentWeek), 'yyyy-MM-dd'),
})
return (
<WeeklyCalendar
schedules={data?.items ?? []}
currentWeek={currentWeek}
onWeekChange={setCurrentWeek}
/>
)
}
// ❌ WRONG - Component manages state + data fetching
const WeeklyCalendar = ({ facilityId }) => {
const [currentWeek, setCurrentWeek] = useState(...)
const { data } = useListSchedules({ facilityId, from, to })
// ...
}
```
Exceptions (component-level fetching allowed):
| Case | Reason |
|------|--------|
| Infinite scroll | Depends on scroll position (internal UI state) |
| Search autocomplete | Real-time search based on input value |
| Independent widget | Notification badge, weather, etc. Completely unrelated to parent data |
| Real-time updates | WebSocket/Polling auto-updates |
| Modal detail fetch | Fetch additional data only when opened |
Decision criteria: "Is there no point in parent managing this / Does it not affect parent?"
| Criteria | Judgment |
|----------|----------|
| Direct fetch in component | Separate to Container layer |
| No error handling | REJECT |
| Loading state not handled | REJECT |
| No cancellation handling | Warning |
| N+1 query-like fetching | REJECT |
## Shared Components and Abstraction
Common UI patterns should be shared components. Copy-paste of inline styles is prohibited.
```tsx
// ❌ WRONG - Copy-pasted inline styles
<button className="p-2 text-[var(--text-secondary)] hover:...">
<X className="w-5 h-5" />
</button>
// ✅ CORRECT - Use shared component
<IconButton onClick={onClose} aria-label="Close">
<X className="w-5 h-5" />
</IconButton>
```
Patterns to make shared components:
- Icon buttons (close, edit, delete, etc.)
- Loading/error displays
- Status badges
- Tab switching
- Label + value display (detail screens)
- Search input
- Color legends
Avoid over-generalization:
```tsx
// ❌ WRONG - Forcing stepper variant into IconButton
export const iconButtonVariants = cva('...', {
variants: {
variant: {
default: '...',
outlined: '...', // ← Stepper-specific, not used elsewhere
},
size: {
medium: 'p-2',
stepper: 'w-8 h-8', // ← Only used with outlined
},
},
})
// ✅ CORRECT - Purpose-specific component
export function StepperButton(props) {
return (
<button className="w-8 h-8 rounded-full border ..." {...props}>
<Plus className="w-4 h-4" />
</button>
)
}
```
Signs to make separate components:
- Implicit constraints like "this variant is always with this size"
- Added variant is clearly different from original component's purpose
- Props specification becomes complex on the usage side
## Abstraction Level Evaluation
**Conditional branch bloat detection:**
| Pattern | Judgment |
|---------|----------|
| Same conditional in 3+ places | Extract to shared component → **REJECT** |
| Props-based branching with 5+ types | Consider component split |
| Nested ternaries in render | Early return or component separation → **REJECT** |
| Type-based render branching | Consider polymorphic components |
**Abstraction level mismatch detection:**
| Pattern | Problem | Fix |
|---------|---------|-----|
| Data fetching logic mixed in JSX | Hard to read | Extract to custom hook |
| Business logic mixed in component | Responsibility violation | Separate to hooks/utils |
| Style calculation logic scattered | Hard to maintain | Extract to utility function |
| Same transformation in multiple places | DRY violation | Extract to common function |
Good abstraction examples:
```tsx
// ❌ Conditional bloat
function UserBadge({ user }) {
if (user.role === 'admin') {
return <span className="bg-red-500">Admin</span>
} else if (user.role === 'moderator') {
return <span className="bg-yellow-500">Moderator</span>
} else if (user.role === 'premium') {
return <span className="bg-purple-500">Premium</span>
} else {
return <span className="bg-gray-500">User</span>
}
}
// ✅ Abstracted with Map
const ROLE_CONFIG = {
admin: { label: 'Admin', className: 'bg-red-500' },
moderator: { label: 'Moderator', className: 'bg-yellow-500' },
premium: { label: 'Premium', className: 'bg-purple-500' },
default: { label: 'User', className: 'bg-gray-500' },
}
function UserBadge({ user }) {
const config = ROLE_CONFIG[user.role] ?? ROLE_CONFIG.default
return <span className={config.className}>{config.label}</span>
}
```
```tsx
// ❌ Mixed abstraction levels
function OrderList() {
const [orders, setOrders] = useState([])
useEffect(() => {
fetch('/api/orders')
.then(res => res.json())
.then(data => setOrders(data))
}, [])
return orders.map(order => (
<div>{order.total.toLocaleString()} USD</div>
))
}
// ✅ Aligned abstraction levels
function OrderList() {
const { data: orders } = useOrders() // Hide data fetching
return orders.map(order => (
<OrderItem key={order.id} order={order} />
))
}
```
## Frontend and Backend Separation of Concerns
### Display Format Responsibility
Backend returns "data", frontend converts to "display format".
```tsx
// ✅ Frontend: Convert to display format
export function formatPrice(amount: number): string {
return `$${amount.toLocaleString()}`
}
export function formatDate(date: Date): string {
return format(date, 'MMM d, yyyy')
}
```
| Criteria | Judgment |
|----------|----------|
| Backend returns display strings | Suggest design review |
| Same format logic copy-pasted | Unify to utility function |
| Inline formatting in component | Extract to function |
### Domain Logic Placement (Smart UI Elimination)
Domain logic (business rules) belongs in the backend. Frontend only displays and edits state.
What is domain logic:
- Aggregate business rules (stock validation, price calculation, status transitions)
- Business constraint validation
- Invariant enforcement
Frontend responsibilities:
- Display state received from server
- Collect user input and send commands to backend
- Manage UI-only temporary state (focus, hover, modal open/close)
- Display format conversion (formatting, sorting, filtering)
| Criteria | Judgment |
|----------|----------|
| Price calculation/stock validation in frontend | Move to backend → **REJECT** |
| Status transition rules in frontend | Move to backend → **REJECT** |
| Business validation in frontend | Move to backend → **REJECT** |
| Recalculating server-computable values in frontend | Redundant → **REJECT** |
Good vs Bad Examples:
```tsx
// ❌ BAD - Business rules in frontend
function OrderForm({ order }: { order: Order }) {
const totalPrice = order.items.reduce((sum, item) =>
sum + item.price * item.quantity, 0
)
const canCheckout = totalPrice >= 100 && order.items.every(i => i.stock > 0)
return <button disabled={!canCheckout}>Checkout</button>
}
// ✅ GOOD - Display state received from server
function OrderForm({ order }: { order: Order }) {
// totalPrice, canCheckout are received from server
return (
<>
<div>{formatPrice(order.totalPrice)}</div>
<button disabled={!order.canCheckout}>Checkout</button>
</>
)
}
```
```tsx
// ❌ BAD - Status transition logic in frontend
function TaskCard({ task }: { task: Task }) {
const canStart = task.status === 'pending' && task.assignee !== null
const canComplete = task.status === 'in_progress' && /* complex conditions... */
return (
<>
<button onClick={startTask} disabled={!canStart}>Start</button>
<button onClick={completeTask} disabled={!canComplete}>Complete</button>
</>
)
}
// ✅ GOOD - Server returns allowed actions
function TaskCard({ task }: { task: Task }) {
// task.allowedActions = ['start', 'cancel'], etc., calculated by server
const canStart = task.allowedActions.includes('start')
const canComplete = task.allowedActions.includes('complete')
return (
<>
<button onClick={startTask} disabled={!canStart}>Start</button>
<button onClick={completeTask} disabled={!canComplete}>Complete</button>
</>
)
}
```
Exceptions (OK to have logic in frontend):
| Case | Reason |
|------|--------|
| UI-only validation | UX feedback like "required field", "max length" (must also validate on server) |
| Client-side filter/sort | Changing display order of lists received from server |
| Display condition branching | UI control like "show details if logged in" |
| Real-time feedback | Preview display during input |
Decision criteria: "Would the business break if this calculation differs from the server?"
- YES → Place in backend (domain logic)
- NO → OK in frontend (display logic)
## Performance
| Criteria | Judgment |
|----------|----------|
| Unnecessary re-renders | Needs optimization |
| Large lists without virtualization | Warning |
| Unoptimized images | Warning |
| Unused code in bundle | Check tree-shaking |
| Excessive memoization | Verify necessity |
Optimization Checklist:
- Are `React.memo` / `useMemo` / `useCallback` appropriate?
- Are large lists using virtual scroll?
- Is Code Splitting appropriate?
- Are images lazy loaded?
Anti-patterns:
```tsx
// ❌ New object every render
<Child style={{ color: 'red' }} />
// ✅ Constant or useMemo
const style = useMemo(() => ({ color: 'red' }), []);
<Child style={style} />
```
## Accessibility
| Criteria | Judgment |
|----------|----------|
| Interactive elements without keyboard support | REJECT |
| Images without alt attribute | REJECT |
| Form elements without labels | REJECT |
| Information conveyed by color only | REJECT |
| Missing focus management (modals, etc.) | REJECT |
Checklist:
- Using semantic HTML?
- Are ARIA attributes appropriate (not excessive)?
- Is keyboard navigation possible?
- Does it make sense with a screen reader?
- Is color contrast sufficient?
## TypeScript/Type Safety
| Criteria | Judgment |
|----------|----------|
| Use of `any` type | REJECT |
| Excessive type assertions (as) | Needs review |
| No Props type definition | REJECT |
| Inappropriate event handler types | Needs fix |
## Frontend Security
| Criteria | Judgment |
|----------|----------|
| dangerouslySetInnerHTML usage | Check XSS risk |
| Unsanitized user input | REJECT |
| Sensitive data stored in frontend | REJECT |
| CSRF token not used | Needs verification |
## Testability
| Criteria | Judgment |
|----------|----------|
| No data-testid, etc. | Warning |
| Structure difficult to test | Consider separation |
| Business logic embedded in UI | REJECT |
## Anti-pattern Detection
REJECT if found:
| Anti-pattern | Problem |
|--------------|---------|
| God Component | All features concentrated in one component |
| Prop Drilling | Deep props bucket brigade |
| Inline Styles abuse | Maintainability degradation |
| useEffect hell | Dependencies too complex |
| Premature Optimization | Unnecessary memoization |
| Magic Strings | Hardcoded strings |
| Hidden Dependencies | Child components with hidden API calls |
| Over-generalization | Components forced to be generic |

View File

@ -0,0 +1,164 @@
# Security Knowledge
## AI-Generated Code Security Issues
AI-generated code has unique vulnerability patterns.
| Pattern | Risk | Example |
|---------|------|---------|
| Plausible but dangerous defaults | High | `cors: { origin: '*' }` looks fine but is dangerous |
| Outdated security practices | Medium | Using deprecated encryption, old auth patterns |
| Incomplete validation | High | Validates format but not business rules |
| Over-trusting inputs | Critical | Assumes internal APIs are always safe |
| Copy-paste vulnerabilities | High | Same dangerous pattern repeated in multiple files |
Require extra scrutiny:
- Auth/authorization logic (AI tends to miss edge cases)
- Input validation (AI may check syntax but miss semantics)
- Error messages (AI may expose internal details)
- Config files (AI may use dangerous defaults from training data)
## Injection Attacks
**SQL Injection:**
- SQL construction via string concatenation → REJECT
- Not using parameterized queries → REJECT
- Unsanitized input in ORM raw queries → REJECT
```typescript
// NG
db.query(`SELECT * FROM users WHERE id = ${userId}`)
// OK
db.query('SELECT * FROM users WHERE id = ?', [userId])
```
**Command Injection:**
- Unvalidated input in `exec()`, `spawn()` → REJECT
- Insufficient escaping in shell command construction → REJECT
```typescript
// NG
exec(`ls ${userInput}`)
// OK
execFile('ls', [sanitizedInput])
```
**XSS (Cross-Site Scripting):**
- Unescaped output to HTML/JS → REJECT
- Improper use of `innerHTML`, `dangerouslySetInnerHTML` → REJECT
- Direct embedding of URL parameters → REJECT
## Authentication & Authorization
**Authentication issues:**
- Hardcoded credentials → Immediate REJECT
- Plaintext password storage → Immediate REJECT
- Weak hash algorithms (MD5, SHA1) → REJECT
- Improper session token management → REJECT
**Authorization issues:**
- Missing permission checks → REJECT
- IDOR (Insecure Direct Object Reference) → REJECT
- Privilege escalation possibility → REJECT
```typescript
// NG - No permission check
app.get('/user/:id', (req, res) => {
return db.getUser(req.params.id)
})
// OK
app.get('/user/:id', authorize('read:user'), (req, res) => {
if (req.user.id !== req.params.id && !req.user.isAdmin) {
return res.status(403).send('Forbidden')
}
return db.getUser(req.params.id)
})
```
## Data Protection
**Sensitive information exposure:**
- Hardcoded API keys, secrets → Immediate REJECT
- Sensitive info in logs → REJECT
- Internal info exposure in error messages → REJECT
- Committed `.env` files → REJECT
**Data validation:**
- Unvalidated input values → REJECT
- Missing type checks → REJECT
- No size limits set → REJECT
## Cryptography
- Use of weak crypto algorithms → REJECT
- Fixed IV/Nonce usage → REJECT
- Hardcoded encryption keys → Immediate REJECT
- No HTTPS (production) → REJECT
## File Operations
**Path Traversal:**
- File paths containing user input → REJECT
- Insufficient `../` sanitization → REJECT
```typescript
// NG
const filePath = path.join(baseDir, userInput)
fs.readFile(filePath)
// OK
const safePath = path.resolve(baseDir, userInput)
if (!safePath.startsWith(path.resolve(baseDir))) {
throw new Error('Invalid path')
}
```
**File Upload:**
- No file type validation → REJECT
- No file size limits → REJECT
- Allowing executable file uploads → REJECT
## Dependencies
- Packages with known vulnerabilities → REJECT
- Unmaintained packages → Warning
- Unnecessary dependencies → Warning
## Error Handling
- Stack trace exposure in production → REJECT
- Detailed error message exposure → REJECT
- Swallowing security events → REJECT
## Rate Limiting & DoS Protection
- No rate limiting (auth endpoints) → Warning
- Resource exhaustion attack possibility → Warning
- Infinite loop possibility → REJECT
## OWASP Top 10 Checklist
| Category | Check Items |
|----------|-------------|
| A01 Broken Access Control | Authorization checks, CORS config |
| A02 Cryptographic Failures | Encryption, sensitive data protection |
| A03 Injection | SQL, Command, XSS |
| A04 Insecure Design | Security design patterns |
| A05 Security Misconfiguration | Default settings, unnecessary features |
| A06 Vulnerable Components | Dependency vulnerabilities |
| A07 Auth Failures | Authentication mechanisms |
| A08 Software Integrity | Code signing, CI/CD |
| A09 Logging Failures | Security logging |
| A10 SSRF | Server-side requests |

View File

@ -40,434 +40,6 @@ Code is read far more often than it is written. Poorly structured code destroys
- Give vague feedback ("clean this up" is prohibited) - Give vague feedback ("clean this up" is prohibited)
- Review AI-specific issues (AI Reviewer's job) - Review AI-specific issues (AI Reviewer's job)
## Review Target Distinction
**Important**: Distinguish between source files and generated files.
| Type | Location | Review Target |
|------|----------|---------------|
| Generated reports | `.takt/reports/` | Not a review target |
| Reports in git diff | `.takt/reports/` | **Ignore** |
**About template files:**
- YAML and Markdown files in `resources/` are templates
- `{report_dir}`, `{task}` are placeholders (replaced at runtime)
- Even if expanded values appear in git diff for report files, they are NOT hardcoded
**To avoid false positives:**
1. Before flagging "hardcoded values", **verify if the file is source or report**
2. Files under `.takt/reports/` are generated during piece execution - not review targets
3. Ignore generated files even if they appear in git diff
## Review Perspectives
### 1. Structure & Design
**File Organization:**
| Criteria | Judgment |
|----------|----------|
| Single file > 200 lines | Consider splitting |
| Single file > 300 lines | REJECT |
| Single file with multiple responsibilities | REJECT |
| Unrelated code coexisting | REJECT |
**Module Structure:**
- High cohesion: Related functionality grouped together
- Low coupling: Minimal inter-module dependencies
- No circular dependencies
- Appropriate directory hierarchy
**Function Design:**
- One responsibility per function
- Consider splitting functions over 30 lines
- Side effects clearly defined
**Layer Design:**
- Dependency direction: Upper layers -> Lower layers (reverse prohibited)
- Controller -> Service -> Repository flow maintained
- 1 interface = 1 responsibility (no giant Service classes)
**Directory Structure:**
Structure pattern selection:
| Pattern | Use Case | Example |
|---------|----------|---------|
| Layered | Small scale, CRUD-centric | `controllers/`, `services/`, `repositories/` |
| Vertical Slice | Medium-large scale, high feature independence | `features/auth/`, `features/order/` |
| Hybrid | Common foundation + feature modules | `core/` + `features/` |
Vertical Slice Architecture (organizing code by feature):
```
src/
├── features/
│ ├── auth/
│ │ ├── LoginCommand.ts
│ │ ├── LoginHandler.ts
│ │ ├── AuthRepository.ts
│ │ └── auth.test.ts
│ └── order/
│ ├── CreateOrderCommand.ts
│ ├── CreateOrderHandler.ts
│ └── ...
└── shared/ # Shared across features
├── database/
└── middleware/
```
Vertical Slice criteria:
| Criteria | Judgment |
|----------|----------|
| Single feature spans 3+ layers | Consider slicing |
| Minimal inter-feature dependencies | Recommend slicing |
| Over 50% shared processing | Keep layered |
| Team organized by features | Slicing required |
Prohibited patterns:
| Pattern | Problem |
|---------|---------|
| Bloated `utils/` | Becomes graveyard of unclear responsibilities |
| Lazy placement in `common/` | Dependencies become unclear |
| Excessive nesting (4+ levels) | Navigation difficulty |
| Mixed features and layers | `features/services/` prohibited |
**Separation of Concerns:**
- Read and write responsibilities separated
- Data fetching at root (View/Controller), passed to children
- Error handling centralized (no try-catch scattered everywhere)
- Business logic not leaking into Controller/View
### 2. Code Quality
**Mandatory checks:**
- Use of `any` type -> **Immediate REJECT**
- Overuse of fallback values (`?? 'unknown'`) -> **REJECT** (see examples below)
- Explanatory comments (What/How comments) -> **REJECT** (see examples below)
- Unused code ("just in case" code) -> **REJECT** (see examples below)
- Direct state mutation (not immutable) -> **REJECT** (see examples below)
**Design principles:**
- Simple > Easy: Readability prioritized
- DRY: No more than 3 duplications
- YAGNI: Only what's needed now
- Fail Fast: Errors detected and reported early
- Idiomatic: Follows language/framework conventions
**Explanatory Comment (What/How) Detection Criteria:**
Comments must only explain design decisions not evident from code (Why), never restate what the code does (What/How). If the code is clear enough, no comment is needed at all.
| Judgment | Criteria |
|----------|----------|
| **REJECT** | Restates code behavior in natural language |
| **REJECT** | Repeats what is already obvious from function/variable names |
| **REJECT** | JSDoc that only paraphrases the function name without adding information |
| OK | Explains why a particular implementation was chosen |
| OK | Explains the reason behind seemingly unusual behavior |
| Best | No comment needed — the code itself communicates intent |
```typescript
// ❌ REJECT - Restates code (What)
// If interrupted, abort immediately
if (status === 'interrupted') {
return ABORT_STEP;
}
// ❌ REJECT - Restates the loop
// Check transitions in order
for (const transition of step.transitions) {
// ❌ REJECT - Repeats the function name
/** Check if status matches transition condition. */
export function matchesCondition(status: Status, condition: TransitionCondition): boolean {
// ✅ OK - Design decision (Why)
// User interruption takes priority over piece-defined transitions
if (status === 'interrupted') {
return ABORT_STEP;
}
// ✅ OK - Reason behind seemingly odd behavior
// stay can cause loops, but is only used when explicitly specified by the user
return step.name;
// ✅ Best - No comment needed. Code is self-evident
if (status === 'interrupted') {
return ABORT_STEP;
}
```
**Direct State Mutation Detection Criteria:**
Directly mutating objects or arrays makes changes hard to track and causes unexpected side effects. Always use spread operators or immutable operations to return new objects.
```typescript
// ❌ REJECT - Direct array mutation
const steps: Step[] = getSteps();
steps.push(newStep); // Mutates original array
steps.splice(index, 1); // Mutates original array
steps[0].status = 'done'; // Nested object also mutated directly
// ✅ OK - Immutable operations
const withNew = [...steps, newStep];
const without = steps.filter((_, i) => i !== index);
const updated = steps.map((s, i) =>
i === 0 ? { ...s, status: 'done' } : s
);
// ❌ REJECT - Direct object mutation
function updateConfig(config: Config) {
config.logLevel = 'debug'; // Mutates argument directly
config.steps.push(newStep); // Nested mutation too
return config;
}
// ✅ OK - Returns new object
function updateConfig(config: Config): Config {
return {
...config,
logLevel: 'debug',
steps: [...config.steps, newStep],
};
}
```
### 3. Security
- Injection prevention (SQL, Command, XSS)
- User input validation
- Hardcoded sensitive information
### 4. Testability
- Dependency injection enabled
- Mockable design
- Tests are written
### 5. Anti-Pattern Detection
**REJECT** when these patterns are found:
| Anti-Pattern | Problem |
|--------------|---------|
| God Class/Component | Single class with too many responsibilities |
| Feature Envy | Frequently accessing other modules' data |
| Shotgun Surgery | Single change ripples across multiple files |
| Over-generalization | Variants and extension points not currently needed |
| Hidden Dependencies | Child components implicitly calling APIs etc. |
| Non-idiomatic | Custom implementation ignoring language/FW conventions |
| Logically dead defensive code | Guards for conditions already guaranteed by all callers |
**Logically dead defensive code:**
Call chain verification applies not only to "missing wiring" but also to the reverse — **unnecessary guards for conditions that callers already guarantee**.
| Pattern | Problem | Detection |
|---------|---------|-----------|
| TTY check when all callers require TTY | Unreachable branch remains | grep all callers' preconditions |
| Null guard when callers already check null | Redundant defense | Trace caller constraints |
| Runtime type check when TypeScript types constrain | Not trusting type safety | Check TypeScript type constraints |
**Verification:**
1. When you find a defensive branch (TTY check, null guard, etc.), grep all callers
2. If all callers already guarantee the condition, the guard is unnecessary → **REJECT**
3. If some callers don't guarantee it, keep the guard
### 6. Abstraction Level Evaluation
**Conditional Branch Proliferation Detection:**
| Pattern | Judgment |
|---------|----------|
| Same if-else pattern in 3+ places | Abstract with polymorphism → **REJECT** |
| switch/case with 5+ branches | Consider Strategy/Map pattern |
| Flag arguments changing behavior | Split into separate functions → **REJECT** |
| Type-based branching (instanceof/typeof) | Replace with polymorphism → **REJECT** |
| Nested conditionals (3+ levels) | Early return or extract → **REJECT** |
**Abstraction Level Mismatch Detection:**
| Pattern | Problem | Fix |
|---------|---------|-----|
| Low-level details in high-level processing | Hard to read | Extract details to functions |
| Mixed abstraction levels in one function | Cognitive load | Align to same granularity |
| DB operations mixed with business logic | Responsibility violation | Separate to Repository layer |
| Config values mixed with processing logic | Hard to change | Externalize configuration |
**Good Abstraction Examples:**
```typescript
// ❌ Proliferating conditionals
function process(type: string) {
if (type === 'A') { /* process A */ }
else if (type === 'B') { /* process B */ }
else if (type === 'C') { /* process C */ }
// ...continues
}
// ✅ Abstract with Map pattern
const processors: Record<string, () => void> = {
A: processA,
B: processB,
C: processC,
};
function process(type: string) {
processors[type]?.();
}
```
```typescript
// ❌ Mixed abstraction levels
function createUser(data: UserData) {
// High level: business logic
validateUser(data);
// Low level: DB operation details
const conn = await pool.getConnection();
await conn.query('INSERT INTO users...');
conn.release();
}
// ✅ Aligned abstraction levels
function createUser(data: UserData) {
validateUser(data);
await userRepository.save(data); // Details hidden
}
```
### 7. Workaround Detection
**Don't overlook compromises made to "just make it work."**
| Pattern | Example |
|---------|---------|
| Unnecessary package additions | Mystery libraries added just to make things work |
| Test deletion/skipping | `@Disabled`, `.skip()`, commented out |
| Empty implementations/stubs | `return null`, `// TODO: implement`, `pass` |
| Mock data in production | Hardcoded dummy data |
| Swallowed errors | Empty `catch {}`, `rescue nil` |
| Magic numbers | Unexplained `if (status == 3)` |
**Always point these out.** Temporary fixes become permanent.
### 8. Spec Compliance Verification
**Verify that changes comply with the project's documented specifications.**
**Verification targets:**
| Target | What to Check |
|--------|---------------|
| CLAUDE.md / README.md | Conforms to schema definitions, design principles, constraints |
| Type definitions / Zod schemas | New fields reflected in schemas |
| YAML/JSON config files | Follows documented format |
**Specific checks:**
1. When config files (YAML, etc.) are modified or added:
- Cross-reference with schema definitions in CLAUDE.md, etc.
- No ignored or invalid fields present
- No required fields missing
2. When type definitions or interfaces are modified:
- Documentation schema descriptions are updated
- Existing config files are compatible with new schema
3. When piece definitions are modified:
- Correct fields used for step type (normal vs. parallel)
- No unnecessary fields remaining (e.g., `next` on parallel sub-steps)
**REJECT when these patterns are found:**
| Pattern | Problem |
|---------|---------|
| Fields not in the spec | Ignored or unexpected behavior |
| Invalid values per spec | Runtime error or silently ignored |
| Violation of documented constraints | Against design intent |
| Step type / field mismatch | Sign of copy-paste error |
### 9. Quality Attributes
| Attribute | Review Point |
|-----------|--------------|
| Scalability | Design handles increased load |
| Maintainability | Easy to modify and fix |
| Observability | Logging and monitoring enabled |
### 10. Big Picture
**Caution**: Don't get lost in minor "clean code" nitpicks.
Verify:
- How will this code evolve in the future
- Is scaling considered
- Is technical debt being created
- Does it align with business requirements
- Is naming consistent with the domain
### 11. Boy Scout Rule
**Leave the code better than you found it.** If changed files have structural issues, flag them for refactoring within the task scope.
**In scope:**
- Existing issues within changed files (dead code, poor naming, broken abstractions)
- Structural issues within changed modules (mixed responsibilities, unnecessary dependencies)
**Out of scope:**
- Issues in unchanged files (record as existing issues only)
- Refactoring that significantly exceeds the task scope (suggest as non-blocking)
**Judgment:**
| Situation | Judgment |
|-----------|----------|
| Clear issues within changed files | **REJECT** — require fix together |
| Structural issues within changed modules | **REJECT** — fix if within scope |
| Issues in unchanged files | Record only (non-blocking) |
**Following poor existing code as justification for leaving problems is not acceptable.** If existing code is bad, improve it rather than match it.
### 12. Change Scope Assessment
**Check change scope and include in report (non-blocking).**
| Scope Size | Lines Changed | Action |
|------------|---------------|--------|
| Small | ~200 lines | Review as-is |
| Medium | 200-500 lines | Review as-is |
| Large | 500+ lines | Continue review. Suggest splitting if possible |
**Note:** Some tasks require large changes. Don't REJECT based on line count alone.
**Verify:**
- Changes are logically cohesive (no unrelated changes mixed in)
- Coder's scope declaration matches actual changes
**Include as suggestions (non-blocking):**
- If splittable, present splitting proposal
### 13. Circular Review Detection
When review count is provided (e.g., "Review count: 3rd"), adjust judgment accordingly.
**From the 3rd review onwards:**
1. Check if the same type of issues are recurring
2. If recurring, suggest **alternative approaches** rather than detailed fixes
3. Even when REJECTing, include perspective that "a different approach should be considered"
Example: When issues repeat on the 3rd review
- Point out the normal issues
- Note that the same type of issues are recurring
- Explain the limitations of the current approach
- Present alternatives (e.g., redesign with a different pattern, introduce new technology)
**Point**: Rather than repeating "fix this again", step back and suggest a different path.
## Important ## Important
**Be specific.** These are prohibited: **Be specific.** These are prohibited:

View File

@ -28,117 +28,6 @@ The truth of a domain is inscribed in events. State is merely a temporary projec
- Snapshot strategies - Snapshot strategies
- Replay and rebuild - Replay and rebuild
## Review Criteria
### 1. Aggregate Design
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Aggregate spans multiple transaction boundaries | REJECT |
| Direct references between Aggregates (not ID references) | REJECT |
| Aggregate exceeds 100 lines | Consider splitting |
| Business invariants exist outside Aggregate | REJECT |
**Good Aggregate:**
- Clear consistency boundary
- References other Aggregates by ID
- Receives commands, emits events
- Protects invariants internally
### 2. Event Design
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Event not in past tense (Created → Create) | REJECT |
| Event contains logic | REJECT |
| Event contains internal state of other Aggregates | REJECT |
| Event schema not version controlled | Warning |
| CRUD-style events (Updated, Deleted) | Needs review |
**Good Events:**
```
// Good: Domain intent is clear
OrderPlaced, PaymentReceived, ItemShipped
// Bad: CRUD style
OrderUpdated, OrderDeleted
```
**Event Granularity:**
- Too fine: `OrderFieldChanged` → Domain intent unclear
- Appropriate: `ShippingAddressChanged` → Intent is clear
- Too coarse: `OrderModified` → What changed is unclear
### 3. Command Handlers
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Handler directly manipulates DB | REJECT |
| Handler modifies multiple Aggregates | REJECT |
| No command validation | REJECT |
| Handler executes queries to make decisions | Needs review |
**Good Command Handler:**
```
1. Receive command
2. Restore Aggregate from event store
3. Apply command to Aggregate
4. Save emitted events
```
### 4. Projection Design
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Projection issues commands | REJECT |
| Projection references Write model | REJECT |
| Single projection serves multiple use cases | Needs review |
| Design that cannot be rebuilt | REJECT |
**Good Projection:**
- Optimized for specific read use case
- Idempotently reconstructible from events
- Completely independent from Write model
### 5. Eventual Consistency
**Required Checks:**
| Situation | Response |
|-----------|----------|
| UI expects immediate updates | Redesign or polling/WebSocket |
| Consistency delay exceeds tolerance | Reconsider architecture |
| Compensating transactions undefined | Request failure scenario review |
### 6. Anti-pattern Detection
**REJECT** if found:
| Anti-pattern | Problem |
|--------------|---------|
| CRUD Disguise | Just splitting CRUD into Command/Query |
| Anemic Domain Model | Aggregate is just a data structure |
| Event Soup | Meaningless events proliferate |
| Temporal Coupling | Implicit dependency on event order |
| Missing Events | Important domain events are missing |
| God Aggregate | All responsibilities in one Aggregate |
### 7. Infrastructure Layer
**Check:**
- Is the event store choice appropriate?
- Does the messaging infrastructure meet requirements?
- Is snapshot strategy defined?
- Is event serialization format appropriate?
## Important ## Important
- **Don't overlook superficial CQRS**: Just splitting CRUD into Command/Query is meaningless - **Don't overlook superficial CQRS**: Just splitting CRUD into Command/Query is meaningless

View File

@ -32,533 +32,6 @@ The user interface is the only point of contact between the system and users. No
- WAI-ARIA compliance - WAI-ARIA compliance
- Responsive design - Responsive design
## Review Criteria
### 1. Component Design
**Principle: Do not write everything in one file. Always split components.**
**Required splits:**
- Has its own state → Must split
- JSX over 50 lines → Split
- Reusable → Split
- Multiple responsibilities → Split
- Independent section within page → Split
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Component over 200 lines | Consider splitting |
| Component over 300 lines | REJECT |
| Display and logic mixed | Consider separation |
| Props drilling (3+ levels) | Consider state management |
| Component with multiple responsibilities | REJECT |
**Good Component:**
- Single responsibility: Does one thing well
- Self-contained: Dependencies are clear
- Testable: Side effects are isolated
**Component Classification:**
| Type | Responsibility | Example |
|------|----------------|---------|
| Container | Data fetching, state management | `UserListContainer` |
| Presentational | Display only | `UserCard` |
| Layout | Arrangement, structure | `PageLayout`, `Grid` |
| Utility | Common functionality | `ErrorBoundary`, `Portal` |
**Directory Structure:**
```
features/{feature-name}/
├── components/
│ ├── {feature}-view.tsx # Main view (composes children)
│ ├── {sub-component}.tsx # Sub-components
│ └── index.ts
├── hooks/
├── types.ts
└── index.ts
```
### 2. State Management
**Principle: Child components do not modify their own state. They bubble events to parent, and parent manipulates state.**
```tsx
// ❌ Child modifies its own state
const ChildBad = ({ initialValue }: { initialValue: string }) => {
const [value, setValue] = useState(initialValue)
return <input value={value} onChange={e => setValue(e.target.value)} />
}
// ✅ Parent manages state, child notifies via callback
const ChildGood = ({ value, onChange }: { value: string; onChange: (v: string) => void }) => {
return <input value={value} onChange={e => onChange(e.target.value)} />
}
const Parent = () => {
const [value, setValue] = useState('')
return <ChildGood value={value} onChange={setValue} />
}
```
**Exception (OK for child to have local state):**
- UI-only temporary state (hover, focus, animation)
- Completely local state that doesn't need to be communicated to parent
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Unnecessary global state | Consider localizing |
| Same state managed in multiple places | Needs normalization |
| State changes from child to parent (reverse data flow) | REJECT |
| API response stored as-is in state | Consider normalization |
| Inappropriate useEffect dependencies | REJECT |
**State Placement Guidelines:**
| State Nature | Recommended Placement |
|--------------|----------------------|
| Temporary UI state (modal open/close, etc.) | Local (useState) |
| Form input values | Local or form library |
| Shared across multiple components | Context or state management library |
| Server data cache | Data fetching library (TanStack Query, etc.) |
### 3. Data Fetching
**Principle: API calls are made in root (View) components and passed to children via props.**
```tsx
// ✅ CORRECT - Fetch at root, pass to children
const OrderDetailView = () => {
const { data: order, isLoading, error } = useGetOrder(orderId)
const { data: items } = useListOrderItems(orderId)
if (isLoading) return <Skeleton />
if (error) return <ErrorDisplay error={error} />
return (
<OrderSummary
order={order}
items={items}
onItemSelect={handleItemSelect}
/>
)
}
// ❌ WRONG - Child fetches its own data
const OrderSummary = ({ orderId }) => {
const { data: order } = useGetOrder(orderId)
// ...
}
```
**Reasons:**
- Data flow is explicit and traceable
- Child components are pure presentation (easier to test)
- No hidden dependencies in child components
**When UI state changes affect parameters (week switching, filters, etc.):**
Manage state at View level and pass callbacks to components.
```tsx
// ✅ CORRECT - State managed at View level
const ScheduleView = () => {
const [currentWeek, setCurrentWeek] = useState(startOfWeek(new Date()))
const { data } = useListSchedules({
from: format(currentWeek, 'yyyy-MM-dd'),
to: format(endOfWeek(currentWeek), 'yyyy-MM-dd'),
})
return (
<WeeklyCalendar
schedules={data?.items ?? []}
currentWeek={currentWeek}
onWeekChange={setCurrentWeek}
/>
)
}
// ❌ WRONG - Component manages state + data fetching
const WeeklyCalendar = ({ facilityId }) => {
const [currentWeek, setCurrentWeek] = useState(...)
const { data } = useListSchedules({ facilityId, from, to })
// ...
}
```
**Exceptions (component-level fetching allowed):**
| Case | Reason |
|------|--------|
| Infinite scroll | Depends on scroll position (internal UI state) |
| Search autocomplete | Real-time search based on input value |
| Independent widget | Notification badge, weather, etc. Completely unrelated to parent data |
| Real-time updates | WebSocket/Polling auto-updates |
| Modal detail fetch | Fetch additional data only when opened |
**Decision criteria: "Is there no point in parent managing this / Does it not affect parent?"**
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Direct fetch in component | Separate to Container layer |
| No error handling | REJECT |
| Loading state not handled | REJECT |
| No cancellation handling | Warning |
| N+1 query-like fetching | REJECT |
### 4. Shared Components and Abstraction
**Principle: Common UI patterns should be shared components. Copy-paste of inline styles is prohibited.**
```tsx
// ❌ WRONG - Copy-pasted inline styles
<button className="p-2 text-[var(--text-secondary)] hover:...">
<X className="w-5 h-5" />
</button>
// ✅ CORRECT - Use shared component
<IconButton onClick={onClose} aria-label="Close">
<X className="w-5 h-5" />
</IconButton>
```
**Patterns to make shared components:**
- Icon buttons (close, edit, delete, etc.)
- Loading/error displays
- Status badges
- Tab switching
- Label + value display (detail screens)
- Search input
- Color legends
**Avoid over-generalization:**
```tsx
// ❌ WRONG - Forcing stepper variant into IconButton
export const iconButtonVariants = cva('...', {
variants: {
variant: {
default: '...',
outlined: '...', // ← Stepper-specific, not used elsewhere
},
size: {
medium: 'p-2',
stepper: 'w-8 h-8', // ← Only used with outlined
},
},
})
// ✅ CORRECT - Purpose-specific component
export function StepperButton(props) {
return (
<button className="w-8 h-8 rounded-full border ..." {...props}>
<Plus className="w-4 h-4" />
</button>
)
}
```
**Signs to make separate components:**
- Implicit constraints like "this variant is always with this size"
- Added variant is clearly different from original component's purpose
- Props specification becomes complex on the usage side
### 5. Abstraction Level Evaluation
**Conditional branch bloat detection:**
| Pattern | Judgment |
|---------|----------|
| Same conditional in 3+ places | Extract to shared component → **REJECT** |
| Props-based branching with 5+ types | Consider component split |
| Nested ternaries in render | Early return or component separation → **REJECT** |
| Type-based render branching | Consider polymorphic components |
**Abstraction level mismatch detection:**
| Pattern | Problem | Fix |
|---------|---------|-----|
| Data fetching logic mixed in JSX | Hard to read | Extract to custom hook |
| Business logic mixed in component | Responsibility violation | Separate to hooks/utils |
| Style calculation logic scattered | Hard to maintain | Extract to utility function |
| Same transformation in multiple places | DRY violation | Extract to common function |
**Good abstraction examples:**
```tsx
// ❌ Conditional bloat
function UserBadge({ user }) {
if (user.role === 'admin') {
return <span className="bg-red-500">Admin</span>
} else if (user.role === 'moderator') {
return <span className="bg-yellow-500">Moderator</span>
} else if (user.role === 'premium') {
return <span className="bg-purple-500">Premium</span>
} else {
return <span className="bg-gray-500">User</span>
}
}
// ✅ Abstracted with Map
const ROLE_CONFIG = {
admin: { label: 'Admin', className: 'bg-red-500' },
moderator: { label: 'Moderator', className: 'bg-yellow-500' },
premium: { label: 'Premium', className: 'bg-purple-500' },
default: { label: 'User', className: 'bg-gray-500' },
}
function UserBadge({ user }) {
const config = ROLE_CONFIG[user.role] ?? ROLE_CONFIG.default
return <span className={config.className}>{config.label}</span>
}
```
```tsx
// ❌ Mixed abstraction levels
function OrderList() {
const [orders, setOrders] = useState([])
useEffect(() => {
fetch('/api/orders')
.then(res => res.json())
.then(data => setOrders(data))
}, [])
return orders.map(order => (
<div>{order.total.toLocaleString()} USD</div>
))
}
// ✅ Aligned abstraction levels
function OrderList() {
const { data: orders } = useOrders() // Hide data fetching
return orders.map(order => (
<OrderItem key={order.id} order={order} />
))
}
```
### 6. Frontend and Backend Separation of Concerns
#### 6.1 Display Format Responsibility
**Principle: Backend returns "data", frontend converts to "display format".**
```tsx
// ✅ Frontend: Convert to display format
export function formatPrice(amount: number): string {
return `$${amount.toLocaleString()}`
}
export function formatDate(date: Date): string {
return format(date, 'MMM d, yyyy')
}
```
**Reasons:**
- Display format is a UI concern, not backend responsibility
- Easy to support internationalization
- Frontend can flexibly change display
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Backend returns display strings | Suggest design review |
| Same format logic copy-pasted | Unify to utility function |
| Inline formatting in component | Extract to function |
#### 6.2 Domain Logic Placement (Smart UI Elimination)
**Principle: Domain logic (business rules) belongs in the backend. Frontend only displays and edits state.**
**What is domain logic:**
- Aggregate business rules (stock validation, price calculation, status transitions)
- Business constraint validation
- Invariant enforcement
**Frontend responsibilities:**
- Display state received from server
- Collect user input and send commands to backend
- Manage UI-only temporary state (focus, hover, modal open/close)
- Display format conversion (formatting, sorting, filtering)
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Price calculation/stock validation in frontend | Move to backend → **REJECT** |
| Status transition rules in frontend | Move to backend → **REJECT** |
| Business validation in frontend | Move to backend → **REJECT** |
| Recalculating server-computable values in frontend | Redundant → **REJECT** |
**Good vs Bad Examples:**
```tsx
// ❌ BAD - Business rules in frontend
function OrderForm({ order }: { order: Order }) {
const totalPrice = order.items.reduce((sum, item) =>
sum + item.price * item.quantity, 0
)
const canCheckout = totalPrice >= 100 && order.items.every(i => i.stock > 0)
return <button disabled={!canCheckout}>Checkout</button>
}
// ✅ GOOD - Display state received from server
function OrderForm({ order }: { order: Order }) {
// totalPrice, canCheckout are received from server
return (
<>
<div>{formatPrice(order.totalPrice)}</div>
<button disabled={!order.canCheckout}>Checkout</button>
</>
)
}
```
```tsx
// ❌ BAD - Status transition logic in frontend
function TaskCard({ task }: { task: Task }) {
const canStart = task.status === 'pending' && task.assignee !== null
const canComplete = task.status === 'in_progress' && /* complex conditions... */
return (
<>
<button onClick={startTask} disabled={!canStart}>Start</button>
<button onClick={completeTask} disabled={!canComplete}>Complete</button>
</>
)
}
// ✅ GOOD - Server returns allowed actions
function TaskCard({ task }: { task: Task }) {
// task.allowedActions = ['start', 'cancel'], etc., calculated by server
const canStart = task.allowedActions.includes('start')
const canComplete = task.allowedActions.includes('complete')
return (
<>
<button onClick={startTask} disabled={!canStart}>Start</button>
<button onClick={completeTask} disabled={!canComplete}>Complete</button>
</>
)
}
```
**Exceptions (OK to have logic in frontend):**
| Case | Reason |
|------|--------|
| UI-only validation | UX feedback like "required field", "max length" (must also validate on server) |
| Client-side filter/sort | Changing display order of lists received from server |
| Display condition branching | UI control like "show details if logged in" |
| Real-time feedback | Preview display during input |
**Decision criteria: "Would the business break if this calculation differs from the server?"**
- YES → Place in backend (domain logic)
- NO → OK in frontend (display logic)
### 7. Performance
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Unnecessary re-renders | Needs optimization |
| Large lists without virtualization | Warning |
| Unoptimized images | Warning |
| Unused code in bundle | Check tree-shaking |
| Excessive memoization | Verify necessity |
**Optimization Checklist:**
- [ ] Are `React.memo` / `useMemo` / `useCallback` appropriate?
- [ ] Are large lists using virtual scroll?
- [ ] Is Code Splitting appropriate?
- [ ] Are images lazy loaded?
**Anti-patterns:**
```tsx
// ❌ New object every render
<Child style={{ color: 'red' }} />
// ✅ Constant or useMemo
const style = useMemo(() => ({ color: 'red' }), []);
<Child style={style} />
```
### 8. Accessibility
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Interactive elements without keyboard support | REJECT |
| Images without alt attribute | REJECT |
| Form elements without labels | REJECT |
| Information conveyed by color only | REJECT |
| Missing focus management (modals, etc.) | REJECT |
**Checklist:**
- [ ] Using semantic HTML?
- [ ] Are ARIA attributes appropriate (not excessive)?
- [ ] Is keyboard navigation possible?
- [ ] Does it make sense with a screen reader?
- [ ] Is color contrast sufficient?
### 9. TypeScript/Type Safety
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| Use of `any` type | REJECT |
| Excessive type assertions (as) | Needs review |
| No Props type definition | REJECT |
| Inappropriate event handler types | Needs fix |
### 10. Frontend Security
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| dangerouslySetInnerHTML usage | Check XSS risk |
| Unsanitized user input | REJECT |
| Sensitive data stored in frontend | REJECT |
| CSRF token not used | Needs verification |
### 11. Testability
**Required Checks:**
| Criteria | Judgment |
|----------|----------|
| No data-testid, etc. | Warning |
| Structure difficult to test | Consider separation |
| Business logic embedded in UI | REJECT |
### 12. Anti-pattern Detection
**REJECT** if found:
| Anti-pattern | Problem |
|--------------|---------|
| God Component | All features concentrated in one component |
| Prop Drilling | Deep props bucket brigade |
| Inline Styles abuse | Maintainability degradation |
| useEffect hell | Dependencies too complex |
| Premature Optimization | Unnecessary memoization |
| Magic Strings | Hardcoded strings |
| Hidden Dependencies | Child components with hidden API calls |
| Over-generalization | Components forced to be generic |
## Important ## Important
- **Prioritize user experience**: UX over technical correctness - **Prioritize user experience**: UX over technical correctness

View File

@ -30,164 +30,6 @@ Security cannot be retrofitted. It must be built in from the design stage; "we'l
- Write code yourself (only provide feedback and fix suggestions) - Write code yourself (only provide feedback and fix suggestions)
- Review design or code quality (that's Architect's role) - Review design or code quality (that's Architect's role)
## AI-Generated Code: Special Attention
AI-generated code has unique vulnerability patterns.
**Common security issues in AI-generated code:**
| Pattern | Risk | Example |
|---------|------|---------|
| Plausible but dangerous defaults | High | `cors: { origin: '*' }` looks fine but is dangerous |
| Outdated security practices | Medium | Using deprecated encryption, old auth patterns |
| Incomplete validation | High | Validates format but not business rules |
| Over-trusting inputs | Critical | Assumes internal APIs are always safe |
| Copy-paste vulnerabilities | High | Same dangerous pattern repeated in multiple files |
**Require extra scrutiny:**
- Auth/authorization logic (AI tends to miss edge cases)
- Input validation (AI may check syntax but miss semantics)
- Error messages (AI may expose internal details)
- Config files (AI may use dangerous defaults from training data)
## Review Perspectives
### 1. Injection Attacks
**SQL Injection:**
- SQL construction via string concatenation → **REJECT**
- Not using parameterized queries → **REJECT**
- Unsanitized input in ORM raw queries → **REJECT**
```typescript
// NG
db.query(`SELECT * FROM users WHERE id = ${userId}`)
// OK
db.query('SELECT * FROM users WHERE id = ?', [userId])
```
**Command Injection:**
- Unvalidated input in `exec()`, `spawn()` → **REJECT**
- Insufficient escaping in shell command construction → **REJECT**
```typescript
// NG
exec(`ls ${userInput}`)
// OK
execFile('ls', [sanitizedInput])
```
**XSS (Cross-Site Scripting):**
- Unescaped output to HTML/JS → **REJECT**
- Improper use of `innerHTML`, `dangerouslySetInnerHTML` → **REJECT**
- Direct embedding of URL parameters → **REJECT**
### 2. Authentication & Authorization
**Authentication issues:**
- Hardcoded credentials → **Immediate REJECT**
- Plaintext password storage → **Immediate REJECT**
- Weak hash algorithms (MD5, SHA1) → **REJECT**
- Improper session token management → **REJECT**
**Authorization issues:**
- Missing permission checks → **REJECT**
- IDOR (Insecure Direct Object Reference) → **REJECT**
- Privilege escalation possibility → **REJECT**
```typescript
// NG - No permission check
app.get('/user/:id', (req, res) => {
return db.getUser(req.params.id)
})
// OK
app.get('/user/:id', authorize('read:user'), (req, res) => {
if (req.user.id !== req.params.id && !req.user.isAdmin) {
return res.status(403).send('Forbidden')
}
return db.getUser(req.params.id)
})
```
### 3. Data Protection
**Sensitive information exposure:**
- Hardcoded API keys, secrets → **Immediate REJECT**
- Sensitive info in logs → **REJECT**
- Internal info exposure in error messages → **REJECT**
- Committed `.env` files → **REJECT**
**Data validation:**
- Unvalidated input values → **REJECT**
- Missing type checks → **REJECT**
- No size limits set → **REJECT**
### 4. Cryptography
- Use of weak crypto algorithms → **REJECT**
- Fixed IV/Nonce usage → **REJECT**
- Hardcoded encryption keys → **Immediate REJECT**
- No HTTPS (production) → **REJECT**
### 5. File Operations
**Path Traversal:**
- File paths containing user input → **REJECT**
- Insufficient `../` sanitization → **REJECT**
```typescript
// NG
const filePath = path.join(baseDir, userInput)
fs.readFile(filePath)
// OK
const safePath = path.resolve(baseDir, userInput)
if (!safePath.startsWith(path.resolve(baseDir))) {
throw new Error('Invalid path')
}
```
**File Upload:**
- No file type validation → **REJECT**
- No file size limits → **REJECT**
- Allowing executable file uploads → **REJECT**
### 6. Dependencies
- Packages with known vulnerabilities → **REJECT**
- Unmaintained packages → Warning
- Unnecessary dependencies → Warning
### 7. Error Handling
- Stack trace exposure in production → **REJECT**
- Detailed error message exposure → **REJECT**
- Swallowing security events → **REJECT**
### 8. Rate Limiting & DoS Protection
- No rate limiting (auth endpoints) → Warning
- Resource exhaustion attack possibility → Warning
- Infinite loop possibility → **REJECT**
### 9. OWASP Top 10 Checklist
| Category | Check Items |
|----------|-------------|
| A01 Broken Access Control | Authorization checks, CORS config |
| A02 Cryptographic Failures | Encryption, sensitive data protection |
| A03 Injection | SQL, Command, XSS |
| A04 Insecure Design | Security design patterns |
| A05 Security Misconfiguration | Default settings, unnecessary features |
| A06 Vulnerable Components | Dependency vulnerabilities |
| A07 Auth Failures | Authentication mechanisms |
| A08 Software Integrity | Code signing, CI/CD |
| A09 Logging Failures | Security logging |
| A10 SSRF | Server-side requests |
## Important ## Important
**Don't miss anything**: Security vulnerabilities get exploited in production. One oversight can lead to a critical incident. **Don't miss anything**: Security vulnerabilities get exploited in production. One oversight can lead to a critical incident.

View File

@ -20,6 +20,9 @@ stances:
review: ../stances/review.md review: ../stances/review.md
testing: ../stances/testing.md testing: ../stances/testing.md
knowledge:
architecture: ../knowledge/architecture.md
personas: personas:
planner: ../personas/planner.md planner: ../personas/planner.md
architect-planner: ../personas/architect-planner.md architect-planner: ../personas/architect-planner.md
@ -224,6 +227,7 @@ movements:
edit: false edit: false
persona: architecture-reviewer persona: architecture-reviewer
stance: review stance: review
knowledge: architecture
report: report:
name: 05-architect-review.md name: 05-architect-review.md
format: architecture-review format: architecture-review

View File

@ -28,6 +28,11 @@ stances:
review: ../stances/review.md review: ../stances/review.md
testing: ../stances/testing.md testing: ../stances/testing.md
knowledge:
frontend: ../knowledge/frontend.md
cqrs-es: ../knowledge/cqrs-es.md
security: ../knowledge/security.md
personas: personas:
planner: ../personas/planner.md planner: ../personas/planner.md
coder: ../personas/coder.md coder: ../personas/coder.md
@ -196,6 +201,7 @@ movements:
edit: false edit: false
persona: cqrs-es-reviewer persona: cqrs-es-reviewer
stance: review stance: review
knowledge: cqrs-es
report: report:
name: 04-cqrs-es-review.md name: 04-cqrs-es-review.md
format: cqrs-es-review format: cqrs-es-review
@ -215,6 +221,7 @@ movements:
edit: false edit: false
persona: frontend-reviewer persona: frontend-reviewer
stance: review stance: review
knowledge: frontend
report: report:
name: 05-frontend-review.md name: 05-frontend-review.md
format: frontend-review format: frontend-review
@ -234,6 +241,7 @@ movements:
edit: false edit: false
persona: security-reviewer persona: security-reviewer
stance: review stance: review
knowledge: security
report: report:
name: 06-security-review.md name: 06-security-review.md
format: security-review format: security-review

View File

@ -28,6 +28,11 @@ stances:
review: ../stances/review.md review: ../stances/review.md
testing: ../stances/testing.md testing: ../stances/testing.md
knowledge:
frontend: ../knowledge/frontend.md
security: ../knowledge/security.md
architecture: ../knowledge/architecture.md
personas: personas:
planner: ../personas/planner.md planner: ../personas/planner.md
coder: ../personas/coder.md coder: ../personas/coder.md
@ -195,6 +200,7 @@ movements:
edit: false edit: false
persona: architecture-reviewer persona: architecture-reviewer
stance: review stance: review
knowledge: architecture
report: report:
name: 04-architect-review.md name: 04-architect-review.md
format: architecture-review format: architecture-review
@ -214,6 +220,7 @@ movements:
edit: false edit: false
persona: frontend-reviewer persona: frontend-reviewer
stance: review stance: review
knowledge: frontend
report: report:
name: 05-frontend-review.md name: 05-frontend-review.md
format: frontend-review format: frontend-review
@ -233,6 +240,7 @@ movements:
edit: false edit: false
persona: security-reviewer persona: security-reviewer
stance: review stance: review
knowledge: security
report: report:
name: 06-security-review.md name: 06-security-review.md
format: security-review format: security-review

View File

@ -0,0 +1,427 @@
# アーキテクチャ知識
## 構造・設計
**ファイル分割**
| 基準 | 判定 |
|--------------|------|
| 1ファイル200行超 | 分割を検討 |
| 1ファイル300行超 | REJECT |
| 1ファイルに複数の責務 | REJECT |
| 関連性の低いコードが同居 | REJECT |
**モジュール構成**
- 高凝集: 関連する機能がまとまっているか
- 低結合: モジュール間の依存が最小限か
- 循環依存がないか
- 適切なディレクトリ階層か
**関数設計**
- 1関数1責務になっているか
- 30行を超える関数は分割を検討
- 副作用が明確か
**レイヤー設計**
- 依存の方向: 上位層 → 下位層(逆方向禁止)
- Controller → Service → Repository の流れが守られているか
- 1インターフェース = 1責務巨大なServiceクラス禁止
**ディレクトリ構造**
構造パターンの選択:
| パターン | 適用場面 | 例 |
|---------|---------|-----|
| レイヤード | 小規模、CRUD中心 | `controllers/`, `services/`, `repositories/` |
| Vertical Slice | 中〜大規模、機能独立性が高い | `features/auth/`, `features/order/` |
| ハイブリッド | 共通基盤 + 機能モジュール | `core/` + `features/` |
Vertical Slice Architecture機能単位でコードをまとめる構造:
```
src/
├── features/
│ ├── auth/
│ │ ├── LoginCommand.ts
│ │ ├── LoginHandler.ts
│ │ ├── AuthRepository.ts
│ │ └── auth.test.ts
│ └── order/
│ ├── CreateOrderCommand.ts
│ ├── CreateOrderHandler.ts
│ └── ...
└── shared/ # 複数featureで共有
├── database/
└── middleware/
```
Vertical Slice の判定基準:
| 基準 | 判定 |
|------|------|
| 1機能が3ファイル以上のレイヤーに跨る | Slice化を検討 |
| 機能間の依存がほぼない | Slice化推奨 |
| 共通処理が50%以上 | レイヤード維持 |
| チームが機能別に分かれている | Slice化必須 |
禁止パターン:
| パターン | 問題 |
|---------|------|
| `utils/` の肥大化 | 責務不明の墓場になる |
| `common/` への安易な配置 | 依存関係が不明確になる |
| 深すぎるネスト4階層超 | ナビゲーション困難 |
| 機能とレイヤーの混在 | `features/services/` は禁止 |
**責務の分離**
- 読み取りと書き込みの責務が分かれているか
- データ取得はルートView/Controllerで行い、子に渡しているか
- エラーハンドリングが一元化されているか各所でtry-catch禁止
- ビジネスロジックがController/Viewに漏れていないか
## コード品質の検出手法
**説明コメントWhat/Howの検出基準**
コードの動作をそのまま言い換えているコメントを検出する。
| 判定 | 基準 |
|------|------|
| REJECT | コードの動作をそのまま自然言語で言い換えている |
| REJECT | 関数名・変数名から明らかなことを繰り返している |
| REJECT | JSDocが関数名の言い換えだけで情報を追加していない |
| OK | なぜその実装を選んだかの設計判断を説明している |
| OK | 一見不自然に見える挙動の理由を説明している |
| 最良 | コメントなしでコード自体が意図を語っている |
```typescript
// REJECT - コードの言い換えWhat
// If interrupted, abort immediately
if (status === 'interrupted') {
return ABORT_STEP;
}
// REJECT - ループの存在を言い換えただけ
// Check transitions in order
for (const transition of step.transitions) {
// REJECT - 関数名の繰り返し
/** Check if status matches transition condition. */
export function matchesCondition(status: Status, condition: TransitionCondition): boolean {
// OK - 設計判断の理由Why
// ユーザー中断はピース定義のトランジションより優先する
if (status === 'interrupted') {
return ABORT_STEP;
}
// OK - 一見不自然な挙動の理由
// stay はループを引き起こす可能性があるが、ユーザーが明示的に指定した場合のみ使われる
return step.name;
```
**状態の直接変更の検出基準**
配列やオブジェクトの直接変更(ミューテーション)を検出する。
```typescript
// REJECT - 配列の直接変更
const steps: Step[] = getSteps();
steps.push(newStep); // 元の配列を破壊
steps.splice(index, 1); // 元の配列を破壊
steps[0].status = 'done'; // ネストされたオブジェクトも直接変更
// OK - イミュータブルな操作
const withNew = [...steps, newStep];
const without = steps.filter((_, i) => i !== index);
const updated = steps.map((s, i) =>
i === 0 ? { ...s, status: 'done' } : s
);
// REJECT - オブジェクトの直接変更
function updateConfig(config: Config) {
config.logLevel = 'debug'; // 引数を直接変更
config.steps.push(newStep); // ネストも直接変更
return config;
}
// OK - 新しいオブジェクトを返す
function updateConfig(config: Config): Config {
return {
...config,
logLevel: 'debug',
steps: [...config.steps, newStep],
};
}
```
## セキュリティ(基本チェック)
- インジェクション対策SQL, コマンド, XSS
- ユーザー入力の検証
- 機密情報のハードコーディング
## テスタビリティ
- 依存性注入が可能な設計か
- モック可能か
- テストが書かれているか
## アンチパターン検出
以下のパターンを見つけたら REJECT:
| アンチパターン | 問題 |
|---------------|------|
| God Class/Component | 1つのクラスが多くの責務を持っている |
| Feature Envy | 他モジュールのデータを頻繁に参照している |
| Shotgun Surgery | 1つの変更が複数ファイルに波及する構造 |
| 過度な汎用化 | 今使わないバリアントや拡張ポイント |
| 隠れた依存 | 子コンポーネントが暗黙的にAPIを呼ぶ等 |
| 非イディオマティック | 言語・FWの作法を無視した独自実装 |
## 抽象化レベルの評価
**条件分岐の肥大化検出**
| パターン | 判定 |
|---------|------|
| 同じif-elseパターンが3箇所以上 | ポリモーフィズムで抽象化 → REJECT |
| switch/caseが5分岐以上 | Strategy/Mapパターンを検討 |
| フラグ引数で挙動を変える | 別関数に分割 → REJECT |
| 型による分岐instanceof/typeof | ポリモーフィズムに置換 → REJECT |
| ネストした条件分岐3段以上 | 早期リターンまたは抽出 → REJECT |
**抽象度の不一致検出**
| パターン | 問題 | 修正案 |
|---------|------|--------|
| 高レベル処理の中に低レベル詳細 | 読みにくい | 詳細を関数に抽出 |
| 1関数内で抽象度が混在 | 認知負荷 | 同じ粒度に揃える |
| ビジネスロジックにDB操作が混在 | 責務違反 | Repository層に分離 |
| 設定値と処理ロジックが混在 | 変更困難 | 設定を外部化 |
**良い抽象化の例**
```typescript
// 条件分岐の肥大化
function process(type: string) {
if (type === 'A') { /* 処理A */ }
else if (type === 'B') { /* 処理B */ }
else if (type === 'C') { /* 処理C */ }
// ...続く
}
// Mapパターンで抽象化
const processors: Record<string, () => void> = {
A: processA,
B: processB,
C: processC,
};
function process(type: string) {
processors[type]?.();
}
```
```typescript
// 抽象度の混在
function createUser(data: UserData) {
// 高レベル: ビジネスロジック
validateUser(data);
// 低レベル: DB操作の詳細
const conn = await pool.getConnection();
await conn.query('INSERT INTO users...');
conn.release();
}
// 抽象度を揃える
function createUser(data: UserData) {
validateUser(data);
await userRepository.save(data); // 詳細は隠蔽
}
```
## その場しのぎの検出
「とりあえず動かす」ための妥協を見逃さない。
| パターン | 例 |
|---------|-----|
| 不要なパッケージ追加 | 動かすためだけに入れた謎のライブラリ |
| テストの削除・スキップ | `@Disabled``.skip()`、コメントアウト |
| 空実装・スタブ放置 | `return null``// TODO: implement``pass` |
| モックデータの本番混入 | ハードコードされたダミーデータ |
| エラー握りつぶし | 空の `catch {}``rescue nil` |
| マジックナンバー | 説明なしの `if (status == 3)` |
## TODOコメントの厳格な禁止
「将来やる」は決してやらない。今やらないことは永遠にやらない。
TODOコメントは即REJECT。
```kotlin
// REJECT - 将来を見越したTODO
// TODO: 施設IDによる認可チェックを追加
fun deleteCustomHoliday(@PathVariable id: String) {
deleteCustomHolidayInputPort.execute(input)
}
// APPROVE - 今実装する
fun deleteCustomHoliday(@PathVariable id: String) {
val currentUserFacilityId = getCurrentUserFacilityId()
val holiday = findHolidayById(id)
require(holiday.facilityId == currentUserFacilityId) {
"Cannot delete holiday from another facility"
}
deleteCustomHolidayInputPort.execute(input)
}
```
TODOが許容される唯一のケース:
| 条件 | 例 | 判定 |
|------|-----|------|
| 外部依存で今は実装不可 + Issue化済み | `// TODO(#123): APIキー取得後に実装` | 許容 |
| 技術的制約で回避不可 + Issue化済み | `// TODO(#456): ライブラリバグ修正待ち` | 許容 |
| 「将来実装」「後で追加」 | `// TODO: バリデーション追加` | REJECT |
| 「時間がないので」 | `// TODO: リファクタリング` | REJECT |
正しい対処:
- 今必要 → 今実装する
- 今不要 → コードを削除する
- 外部要因で不可 → Issue化してチケット番号をコメントに入れる
## DRY違反の検出
重複コードを検出する。
| パターン | 判定 |
|---------|------|
| 同じロジックが3箇所以上 | 即REJECT - 関数/メソッドに抽出 |
| 同じバリデーションが2箇所以上 | 即REJECT - バリデーター関数に抽出 |
| 似たようなコンポーネントが3個以上 | 即REJECT - 共通コンポーネント化 |
| コピペで派生したコード | 即REJECT - パラメータ化または抽象化 |
AHA原則Avoid Hasty Abstractionsとのバランス:
- 2回の重複 → 様子見
- 3回の重複 → 即抽出
- ドメインが異なる重複 → 抽象化しない(例: 顧客用バリデーションと管理者用バリデーションは別物)
## 仕様準拠の検証
変更が、プロジェクトの文書化された仕様に準拠しているか検証する。
検証対象:
| 対象 | 確認内容 |
|------|---------|
| CLAUDE.md / README.md | スキーマ定義、設計原則、制約に従っているか |
| 型定義・Zodスキーマ | 新しいフィールドがスキーマに反映されているか |
| YAML/JSON設定ファイル | 文書化されたフォーマットに従っているか |
具体的なチェック:
1. 設定ファイルYAML等を変更・追加した場合:
- CLAUDE.md等に記載されたスキーマ定義と突合する
- 無視されるフィールドや無効なフィールドが含まれていないか
- 必須フィールドが欠落していないか
2. 型定義やインターフェースを変更した場合:
- ドキュメントのスキーマ説明が更新されているか
- 既存の設定ファイルが新しいスキーマと整合するか
このパターンを見つけたら REJECT:
| パターン | 問題 |
|---------|------|
| 仕様に存在しないフィールドの使用 | 無視されるか予期しない動作 |
| 仕様上無効な値の設定 | 実行時エラーまたは無視される |
| 文書化された制約への違反 | 設計意図に反する |
## 呼び出しチェーン検証
新しいパラメータ・フィールドが追加された場合、変更ファイル内だけでなく呼び出し元も検証する。
検証手順:
1. 新しいオプショナルパラメータや interface フィールドを見つけたら、`Grep` で全呼び出し元を検索
2. 全呼び出し元が新しいパラメータを渡しているか確認
3. フォールバック値(`?? default`)がある場合、フォールバックが使われるケースが意図通りか確認
危険パターン:
| パターン | 問題 | 検出方法 |
|---------|------|---------|
| `options.xxx ?? fallback` で全呼び出し元が `xxx` を省略 | 機能が実装されているのに常にフォールバック | grep で呼び出し元を確認 |
| テストがモックで直接値をセット | 実際の呼び出しチェーンを経由しない | テストの構築方法を確認 |
| `executeXxx()` が内部で使う `options` を引数で受け取らない | 上位から値を渡す口がない | 関数シグネチャを確認 |
```typescript
// 配線漏れ: projectCwd を受け取る口がない
export async function executePiece(config, cwd, task) {
const engine = new PieceEngine(config, cwd, task); // options なし
}
// 配線済み: projectCwd を渡せる
export async function executePiece(config, cwd, task, options?) {
const engine = new PieceEngine(config, cwd, task, options);
}
```
呼び出し元の制約による論理的デッドコード:
呼び出しチェーンの検証は「配線漏れ」だけでなく、逆方向——呼び出し元が既に保証している条件に対する不要な防御コード——にも適用する。
| パターン | 問題 | 検出方法 |
|---------|------|---------|
| 呼び出し元がTTY必須なのに関数内でTTYチェック | 到達しない分岐が残る | grep で全呼び出し元の前提条件を確認 |
| 呼び出し元がnullチェック済みなのに再度nullガード | 冗長な防御 | 呼び出し元の制約を追跡 |
| 呼び出し元が型で制約しているのにランタイムチェック | 型安全を信頼していない | TypeScriptの型制約を確認 |
検証手順:
1. 防御的な条件分岐TTYチェック、nullガード等を見つけたら、grep で全呼び出し元を確認
2. 全呼び出し元がその条件を既に保証しているなら、防御は不要 → REJECT
3. 一部の呼び出し元が保証していない場合は、防御を残す
## 品質特性
| 特性 | 確認観点 |
|------|---------|
| Scalability | 負荷増加に対応できる設計か |
| Maintainability | 変更・修正が容易か |
| Observability | ログ・監視が可能な設計か |
## 大局観
細かい「クリーンコード」の指摘に終始しない。
確認すべきこと:
- このコードは将来どう変化するか
- スケーリングの必要性は考慮されているか
- 技術的負債を生んでいないか
- ビジネス要件と整合しているか
- 命名がドメインと一貫しているか
## 変更スコープの評価
変更スコープを確認し、レポートに記載する(ブロッキングではない)。
| スコープサイズ | 変更行数 | 対応 |
|---------------|---------|------|
| Small | 〜200行 | そのままレビュー |
| Medium | 200-500行 | そのままレビュー |
| Large | 500行以上 | レビューは継続。分割可能か提案を付記 |
大きな変更が必要なタスクもある。行数だけでREJECTしない。
確認すること:
- 変更が論理的にまとまっているか(無関係な変更が混在していないか)
- Coderのスコープ宣言と実際の変更が一致しているか
提案として記載すること(ブロッキングではない):
- 分割可能な場合は分割案を提示

View File

@ -0,0 +1,417 @@
# CQRS+ES知識
## Aggregate設計
Aggregateは判断に必要なフィールドのみ保持する。
Command ModelAggregateの役割は「コマンドを受けて判断し、イベントを発行する」こと。クエリ用データはRead ModelProjectionが担当する。
「判断に必要」とは:
- `if`/`require`の条件分岐に使う
- インスタンスメソッドでイベント発行時にフィールド値を参照する
| 基準 | 判定 |
|------|------|
| Aggregateが複数のトランザクション境界を跨ぐ | REJECT |
| Aggregate間の直接参照ID参照でない | REJECT |
| Aggregateが100行を超える | 分割を検討 |
| ビジネス不変条件がAggregate外にある | REJECT |
| 判断に使わないフィールドを保持 | REJECT |
良いAggregate:
```kotlin
// 判断に必要なフィールドのみ
data class Order(
val orderId: String, // イベント発行時に使用
val status: OrderStatus // 状態チェックに使用
) {
fun confirm(confirmedBy: String): OrderConfirmedEvent {
require(status == OrderStatus.PENDING) { "確定できる状態ではありません" }
return OrderConfirmedEvent(
orderId = orderId,
confirmedBy = confirmedBy,
confirmedAt = LocalDateTime.now()
)
}
}
// 判断に使わないフィールドを保持NG
data class Order(
val orderId: String,
val customerId: String, // 判断に未使用
val shippingAddress: Address, // 判断に未使用
val status: OrderStatus
)
```
追加操作がないAggregateはIDのみ:
```kotlin
// 作成のみで追加操作がない場合
data class Notification(val notificationId: String) {
companion object {
fun create(customerId: String, message: String): NotificationCreatedEvent {
return NotificationCreatedEvent(
notificationId = UUID.randomUUID().toString(),
customerId = customerId,
message = message
)
}
}
}
```
## イベント設計
| 基準 | 判定 |
|------|------|
| イベントが過去形でないCreated → Create | REJECT |
| イベントにロジックが含まれる | REJECT |
| イベントが他Aggregateの内部状態を含む | REJECT |
| イベントのスキーマがバージョン管理されていない | 警告 |
| CRUDスタイルのイベントUpdated, Deleted | 要検討 |
良いイベント:
```kotlin
// Good: ドメインの意図が明確
OrderPlaced, PaymentReceived, ItemShipped
// Bad: CRUDスタイル
OrderUpdated, OrderDeleted
```
イベント粒度:
- 細かすぎ: `OrderFieldChanged` → ドメインの意図が不明
- 適切: `ShippingAddressChanged` → 意図が明確
- 粗すぎ: `OrderModified` → 何が変わったか不明
## コマンドハンドラ
| 基準 | 判定 |
|------|------|
| ハンドラがDBを直接操作 | REJECT |
| ハンドラが複数Aggregateを変更 | REJECT |
| コマンドのバリデーションがない | REJECT |
| ハンドラがクエリを実行して判断 | 要検討 |
良いコマンドハンドラ:
```
1. コマンドを受け取る
2. Aggregateをイベントストアから復元
3. Aggregateにコマンドを適用
4. 発行されたイベントを保存
```
## プロジェクション設計
| 基準 | 判定 |
|------|------|
| プロジェクションがコマンドを発行 | REJECT |
| プロジェクションがWriteモデルを参照 | REJECT |
| 複数のユースケースを1つのプロジェクションで賄う | 要検討 |
| リビルド不可能な設計 | REJECT |
良いプロジェクション:
- 特定の読み取りユースケースに最適化
- イベントから冪等に再構築可能
- Writeモデルから完全に独立
## Query側の設計
ControllerはQueryGatewayを使う。Repositoryを直接使わない。
レイヤー間の型:
- `application/query/` - Query結果の型例: `OrderDetail`
- `adapter/protocol/` - RESTレスポンスの型例: `OrderDetailResponse`
- QueryHandlerはapplication層の型を返し、Controllerがadapter層の型に変換
```kotlin
// application/query/OrderDetail.kt
data class OrderDetail(
val orderId: String,
val customerName: String,
val totalAmount: Money
)
// adapter/protocol/OrderDetailResponse.kt
data class OrderDetailResponse(...) {
companion object {
fun from(detail: OrderDetail) = OrderDetailResponse(...)
}
}
// QueryHandler - application層の型を返す
@QueryHandler
fun handle(query: GetOrderDetailQuery): OrderDetail? {
val entity = repository.findById(query.id) ?: return null
return OrderDetail(...)
}
// Controller - adapter層の型に変換
@GetMapping("/{id}")
fun getById(@PathVariable id: String): ResponseEntity<OrderDetailResponse> {
val detail = queryGateway.query(
GetOrderDetailQuery(id),
OrderDetail::class.java
).join() ?: throw NotFoundException("...")
return ResponseEntity.ok(OrderDetailResponse.from(detail))
}
```
構成:
```
Controller (adapter) → QueryGateway → QueryHandler (application) → Repository
↓ ↓
Response.from(detail) OrderDetail
```
## 結果整合性
| 状況 | 対応 |
|------|------|
| UIが即座に更新を期待している | 設計見直し or ポーリング/WebSocket |
| 整合性遅延が許容範囲を超える | アーキテクチャ再検討 |
| 補償トランザクションが未定義 | 障害シナリオの検討を要求 |
## Saga vs EventHandler
Sagaは「競合が発生する複数アグリゲート間の操作」にのみ使用する。
Sagaが必要なケース:
```
複数のアクターが同じリソースを取り合う場合
例: 在庫確保10人が同時に同じ商品を注文
OrderPlacedEvent
↓ InventoryReservationSaga
ReserveInventoryCommand → Inventory集約同時実行を直列化
InventoryReservedEvent → ConfirmOrderCommand
InventoryReservationFailedEvent → CancelOrderCommand
```
Sagaが不要なケース:
```
競合が発生しない操作
例: 注文キャンセル時の在庫解放
OrderCancelledEvent
↓ InventoryReleaseHandler単純なEventHandler
ReleaseInventoryCommand
InventoryReleasedEvent
```
判断基準:
| 状況 | Saga | EventHandler |
|------|------|--------------|
| リソースの取り合いがある | 使う | - |
| 補償トランザクションが必要 | 使う | - |
| 競合しない単純な連携 | - | 使う |
| 失敗時は再試行で十分 | - | 使う |
アンチパターン:
```kotlin
// NG - ライフサイクル管理のためにSagaを使う
@Saga
class OrderLifecycleSaga {
// 注文の全状態遷移をSagaで追跡
// PLACED → CONFIRMED → SHIPPED → DELIVERED
}
// OK - 結果整合性が必要な操作だけをSagaで処理
@Saga
class InventoryReservationSaga {
// 在庫確保の同時実行制御のみ
}
```
Sagaはライフサイクル管理ツールではない。結果整合性が必要な「操作」単位で作成する。
## 例外 vs イベント(失敗時の選択)
監査不要な失敗は例外、監査が必要な失敗はイベント。
例外アプローチ(推奨: ほとんどのケース):
```kotlin
// ドメインモデル: バリデーション失敗時に例外をスロー
fun reserveInventory(orderId: String, quantity: Int): InventoryReservedEvent {
if (availableQuantity < quantity) {
throw InsufficientInventoryException("在庫が不足しています")
}
return InventoryReservedEvent(productId, orderId, quantity)
}
// Saga: exceptionally でキャッチして補償アクション
commandGateway.send<Any>(command)
.exceptionally { ex ->
commandGateway.send<Any>(CancelOrderCommand(
orderId = orderId,
reason = ex.cause?.message ?: "在庫確保に失敗しました"
))
null
}
```
イベントアプローチ(稀なケース):
```kotlin
// 監査が必要な場合のみ
data class PaymentFailedEvent(
val paymentId: String,
val reason: String,
val attemptedAmount: Money
) : PaymentEvent
```
判断基準:
| 質問 | 例外 | イベント |
|------|------|----------|
| この失敗を後で確認する必要があるか? | No | Yes |
| 規制やコンプライアンスで記録が必要か? | No | Yes |
| Sagaだけが失敗を気にするか? | Yes | No |
| Event Storeに残すと価値があるか? | No | Yes |
デフォルトは例外アプローチ。監査要件がある場合のみイベントを検討する。
## 抽象化レベルの評価
**条件分岐の肥大化検出**
| パターン | 判定 |
|---------|------|
| 同じif-elseパターンが3箇所以上 | ポリモーフィズムで抽象化 → REJECT |
| switch/caseが5分岐以上 | Strategy/Mapパターンを検討 |
| イベント種別による分岐が増殖 | イベントハンドラを分離 → REJECT |
| Aggregate内の状態分岐が複雑 | State Patternを検討 |
**抽象度の不一致検出**
| パターン | 問題 | 修正案 |
|---------|------|--------|
| CommandHandlerにDB操作詳細 | 責務違反 | Repository層に分離 |
| EventHandlerにビジネスロジック | 責務違反 | ドメインサービスに抽出 |
| Aggregateに永続化処理 | レイヤー違反 | EventStore経由に変更 |
| Projectionに計算ロジック | 保守困難 | 専用サービスに抽出 |
良い抽象化の例:
```kotlin
// イベント種別による分岐の増殖NG
@EventHandler
fun on(event: DomainEvent) {
when (event) {
is OrderPlacedEvent -> handleOrderPlaced(event)
is OrderConfirmedEvent -> handleOrderConfirmed(event)
is OrderShippedEvent -> handleOrderShipped(event)
// ...どんどん増える
}
}
// イベントごとにハンドラを分離OK
@EventHandler
fun on(event: OrderPlacedEvent) { ... }
@EventHandler
fun on(event: OrderConfirmedEvent) { ... }
@EventHandler
fun on(event: OrderShippedEvent) { ... }
```
```kotlin
// 状態による分岐が複雑NG
fun process(command: ProcessCommand) {
when (status) {
PENDING -> if (command.type == "approve") { ... } else if (command.type == "reject") { ... }
APPROVED -> if (command.type == "ship") { ... }
// ...複雑化
}
}
// State Patternで抽象化OK
sealed class OrderState {
abstract fun handle(command: ProcessCommand): List<DomainEvent>
}
class PendingState : OrderState() {
override fun handle(command: ProcessCommand) = when (command) {
is ApproveCommand -> listOf(OrderApprovedEvent(...))
is RejectCommand -> listOf(OrderRejectedEvent(...))
else -> throw InvalidCommandException()
}
}
```
## アンチパターン検出
以下を見つけたら REJECT:
| アンチパターン | 問題 |
|---------------|------|
| CRUD偽装 | CQRSの形だけ真似てCRUD実装 |
| Anemic Domain Model | Aggregateが単なるデータ構造 |
| Event Soup | 意味のないイベントが乱発される |
| Temporal Coupling | イベント順序に暗黙の依存 |
| Missing Events | 重要なドメインイベントが欠落 |
| God Aggregate | 1つのAggregateに全責務が集中 |
## テスト戦略
レイヤーごとにテスト方針を分ける。
テストピラミッド:
```
┌─────────────┐
│ E2E Test │ ← 少数: 全体フロー確認
├─────────────┤
│ Integration │ ← Command→Event→Projection→Query の連携確認
├─────────────┤
│ Unit Test │ ← 多数: 各レイヤー独立テスト
└─────────────┘
```
Command側Aggregate:
```kotlin
// AggregateTestFixture使用
@Test
fun `確定コマンドでイベントが発行される`() {
fixture
.given(OrderPlacedEvent(...))
.`when`(ConfirmOrderCommand(orderId, confirmedBy))
.expectSuccessfulHandlerExecution()
.expectEvents(OrderConfirmedEvent(...))
}
```
Query側:
```kotlin
// Read Model直接セットアップ + QueryGateway
@Test
fun `注文詳細が取得できる`() {
// Given: Read Modelを直接セットアップ
orderRepository.save(OrderEntity(...))
// When: QueryGateway経由でクエリ実行
val detail = queryGateway.query(GetOrderDetailQuery(orderId), ...).join()
// Then
assertEquals(expectedDetail, detail)
}
```
チェック項目:
| 観点 | 判定 |
|------|------|
| Aggregateテストが状態ではなくイベントを検証している | 必須 |
| Query側テストがCommand経由でデータを作っていない | 推奨 |
| 統合テストでAxonの非同期処理を考慮している | 必須 |
## インフラ層
確認事項:
- イベントストアの選択は適切か
- メッセージング基盤は要件を満たすか
- スナップショット戦略は定義されているか
- イベントのシリアライズ形式は適切か

View File

@ -0,0 +1,497 @@
# フロントエンド専門知識
## コンポーネント設計
1ファイルにベタ書きしない。必ずコンポーネント分割する。
分離が必須なケース:
- 独自のstateを持つ → 必ず分離
- 50行超のJSX → 分離
- 再利用可能 → 分離
- 責務が複数 → 分離
- ページ内の独立したセクション → 分離
| 基準 | 判定 |
|------|------|
| 1コンポーネント200行超 | 分割を検討 |
| 1コンポーネント300行超 | REJECT |
| 表示とロジックが混在 | 分離を検討 |
| Props drilling3階層以上 | 状態管理の導入を検討 |
| 複数の責務を持つコンポーネント | REJECT |
良いコンポーネント:
- 単一責務: 1つのことをうまくやる
- 自己完結: 必要な依存が明確
- テスト可能: 副作用が分離されている
コンポーネント分類:
| 種類 | 責務 | 例 |
|------|------|-----|
| Container | データ取得・状態管理 | `UserListContainer` |
| Presentational | 表示のみ | `UserCard` |
| Layout | 配置・構造 | `PageLayout`, `Grid` |
| Utility | 共通機能 | `ErrorBoundary`, `Portal` |
ディレクトリ構成:
```
features/{feature-name}/
├── components/
│ ├── {feature}-view.tsx # メインビュー(子を組み合わせる)
│ ├── {sub-component}.tsx # サブコンポーネント
│ └── index.ts
├── hooks/
├── types.ts
└── index.ts
```
## 状態管理
子コンポーネントは自身で状態を変更しない。イベントを親にバブリングし、親が状態を操作する。
```tsx
// 子が自分で状態を変更NG
const ChildBad = ({ initialValue }: { initialValue: string }) => {
const [value, setValue] = useState(initialValue)
return <input value={value} onChange={e => setValue(e.target.value)} />
}
// 親が状態を管理、子はコールバックで通知OK
const ChildGood = ({ value, onChange }: { value: string; onChange: (v: string) => void }) => {
return <input value={value} onChange={e => onChange(e.target.value)} />
}
const Parent = () => {
const [value, setValue] = useState('')
return <ChildGood value={value} onChange={setValue} />
}
```
例外子がローカルstate持ってOK:
- UI専用の一時状態ホバー、フォーカス、アニメーション
- 親に伝える必要がない完全にローカルな状態
| 基準 | 判定 |
|------|------|
| 不要なグローバル状態 | ローカル化を検討 |
| 同じ状態が複数箇所で管理 | 正規化が必要 |
| 子から親への状態変更(逆方向データフロー) | REJECT |
| APIレスポンスをそのまま状態に | 正規化を検討 |
| useEffectの依存配列が不適切 | REJECT |
状態配置の判断基準:
| 状態の性質 | 推奨配置 |
|-----------|---------|
| UIの一時的な状態モーダル開閉等 | ローカルuseState |
| フォームの入力値 | ローカル or フォームライブラリ |
| 複数コンポーネントで共有 | Context or 状態管理ライブラリ |
| サーバーデータのキャッシュ | TanStack Query等のデータフェッチライブラリ |
## データ取得
API呼び出しはルートViewコンポーネントで行い、子コンポーネントにはpropsで渡す。
```tsx
// CORRECT - ルートでデータ取得、子に渡す
const OrderDetailView = () => {
const { data: order, isLoading, error } = useGetOrder(orderId)
const { data: items } = useListOrderItems(orderId)
if (isLoading) return <Skeleton />
if (error) return <ErrorDisplay error={error} />
return (
<OrderSummary
order={order}
items={items}
onItemSelect={handleItemSelect}
/>
)
}
// WRONG - 子コンポーネントが自分でデータ取得
const OrderSummary = ({ orderId }) => {
const { data: order } = useGetOrder(orderId)
// ...
}
```
UIの状態変更でパラメータが変わる場合週切り替え、フィルタ等:
状態もViewレベルで管理し、コンポーネントにはコールバックを渡す。
```tsx
// CORRECT - 状態もViewで管理
const ScheduleView = () => {
const [currentWeek, setCurrentWeek] = useState(startOfWeek(new Date()))
const { data } = useListSchedules({
from: format(currentWeek, 'yyyy-MM-dd'),
to: format(endOfWeek(currentWeek), 'yyyy-MM-dd'),
})
return (
<WeeklyCalendar
schedules={data?.items ?? []}
currentWeek={currentWeek}
onWeekChange={setCurrentWeek}
/>
)
}
// WRONG - コンポーネント内で状態管理+データ取得
const WeeklyCalendar = ({ facilityId }) => {
const [currentWeek, setCurrentWeek] = useState(...)
const { data } = useListSchedules({ facilityId, from, to })
// ...
}
```
例外(コンポーネント内フェッチが許容されるケース):
| ケース | 理由 |
|--------|------|
| 無限スクロール | スクロール位置というUI内部状態に依存 |
| 検索オートコンプリート | 入力値に依存したリアルタイム検索 |
| 独立したウィジェット | 通知バッジ、天気等。親のデータと完全に無関係 |
| リアルタイム更新 | WebSocket/Pollingでの自動更新 |
| モーダル内の詳細取得 | 開いたときだけ追加データを取得 |
判断基準: 「親が管理する意味がない / 親に影響を与えない」ケースのみ許容。
| 基準 | 判定 |
|------|------|
| コンポーネント内で直接fetch | Container層に分離 |
| エラーハンドリングなし | REJECT |
| ローディング状態の未処理 | REJECT |
| キャンセル処理なし | 警告 |
| N+1クエリ的なフェッチ | REJECT |
## 共有コンポーネントと抽象化
同じパターンのUIは共有コンポーネント化する。インラインスタイルのコピペは禁止。
```tsx
// WRONG - インラインスタイルのコピペ
<button className="p-2 text-[var(--text-secondary)] hover:...">
<X className="w-5 h-5" />
</button>
// CORRECT - 共有コンポーネント使用
<IconButton onClick={onClose} aria-label="閉じる">
<X className="w-5 h-5" />
</IconButton>
```
共有コンポーネント化すべきパターン:
- アイコンボタン(閉じる、編集、削除等)
- ローディング/エラー表示
- ステータスバッジ
- タブ切り替え
- ラベル+値の表示(詳細画面)
- 検索入力
- カラー凡例
過度な汎用化を避ける:
```tsx
// WRONG - IconButtonに無理やりステッパー用バリアントを追加
export const iconButtonVariants = cva('...', {
variants: {
variant: {
default: '...',
outlined: '...', // ステッパー専用、他で使わない
},
size: {
medium: 'p-2',
stepper: 'w-8 h-8', // outlinedとセットでしか使わない
},
},
})
// CORRECT - 用途別に専用コンポーネント
export function StepperButton(props) {
return (
<button className="w-8 h-8 rounded-full border ..." {...props}>
<Plus className="w-4 h-4" />
</button>
)
}
```
別コンポーネントにすべきサイン:
- 「このvariantはこのsizeとセット」のような暗黙の制約がある
- 追加したvariantが元のコンポーネントの用途と明らかに違う
- 使う側のprops指定が複雑になる
## 抽象化レベルの評価
### 条件分岐の肥大化検出
| パターン | 判定 |
|---------|------|
| 同じ条件分岐が3箇所以上 | 共通コンポーネントに抽出 → REJECT |
| propsによる分岐が5種類以上 | コンポーネント分割を検討 |
| render内の三項演算子のネスト | 早期リターンまたはコンポーネント分離 → REJECT |
| 型による分岐レンダリング | ポリモーフィックコンポーネントを検討 |
### 抽象度の不一致検出
| パターン | 問題 | 修正案 |
|---------|------|--------|
| データ取得ロジックがJSXに混在 | 読みにくい | カスタムフックに抽出 |
| ビジネスロジックがコンポーネントに混在 | 責務違反 | hooks/utilsに分離 |
| スタイル計算ロジックが散在 | 保守困難 | ユーティリティ関数に抽出 |
| 同じ変換処理が複数箇所に | DRY違反 | 共通関数に抽出 |
良い抽象化の例:
```tsx
// 条件分岐が肥大化
function UserBadge({ user }) {
if (user.role === 'admin') {
return <span className="bg-red-500">管理者</span>
} else if (user.role === 'moderator') {
return <span className="bg-yellow-500">モデレーター</span>
} else if (user.role === 'premium') {
return <span className="bg-purple-500">プレミアム</span>
} else {
return <span className="bg-gray-500">一般</span>
}
}
// Mapで抽象化
const ROLE_CONFIG = {
admin: { label: '管理者', className: 'bg-red-500' },
moderator: { label: 'モデレーター', className: 'bg-yellow-500' },
premium: { label: 'プレミアム', className: 'bg-purple-500' },
default: { label: '一般', className: 'bg-gray-500' },
}
function UserBadge({ user }) {
const config = ROLE_CONFIG[user.role] ?? ROLE_CONFIG.default
return <span className={config.className}>{config.label}</span>
}
```
```tsx
// 抽象度が混在
function OrderList() {
const [orders, setOrders] = useState([])
useEffect(() => {
fetch('/api/orders')
.then(res => res.json())
.then(data => setOrders(data))
}, [])
return orders.map(order => (
<div>{order.total.toLocaleString()}円</div>
))
}
// 抽象度を揃える
function OrderList() {
const { data: orders } = useOrders() // データ取得を隠蔽
return orders.map(order => (
<OrderItem key={order.id} order={order} />
))
}
```
## フロントエンドとバックエンドの責務分離
### 表示形式の責務
バックエンドは「データ」を返し、フロントエンドが「表示形式」に変換する。
```tsx
// フロントエンド: 表示形式に変換
export function formatPrice(amount: number): string {
return `¥${amount.toLocaleString()}`
}
export function formatDate(date: Date): string {
return format(date, 'yyyy年M月d日')
}
```
| 基準 | 判定 |
|------|------|
| バックエンドが表示用文字列を返している | 設計見直しを提案 |
| 同じフォーマット処理が複数箇所にコピペ | ユーティリティ関数に統一 |
| コンポーネント内でインラインフォーマット | 関数に抽出 |
### ドメインロジックの配置SmartUI排除
ドメインロジック(ビジネスルール)はバックエンドに配置。フロントエンドは状態の表示・編集のみ。
ドメインロジックとは:
- 集約のビジネスルール(在庫判定、価格計算、ステータス遷移)
- バリデーション(業務制約の検証)
- 不変条件の保証
フロントエンドの責務:
- サーバーから受け取った状態を表示
- ユーザー入力を収集し、コマンドとしてバックエンドに送信
- UI専用の一時状態管理フォーカス、ホバー、モーダル開閉
- 表示形式の変換(フォーマット、ソート、フィルタ)
| 基準 | 判定 |
|------|------|
| フロントエンドで価格計算・在庫判定 | バックエンドに移動 → REJECT |
| フロントエンドでステータス遷移ルール | バックエンドに移動 → REJECT |
| フロントエンドでビジネスバリデーション | バックエンドに移動 → REJECT |
| サーバー側で計算可能な値をフロントで再計算 | 冗長 → REJECT |
良い例 vs 悪い例:
```tsx
// BAD - フロントエンドでビジネスルール
function OrderForm({ order }: { order: Order }) {
const totalPrice = order.items.reduce((sum, item) =>
sum + item.price * item.quantity, 0
)
const canCheckout = totalPrice >= 1000 && order.items.every(i => i.stock > 0)
return <button disabled={!canCheckout}>注文確定</button>
}
// GOOD - バックエンドから受け取った状態を表示
function OrderForm({ order }: { order: Order }) {
// totalPrice, canCheckout はサーバーから受け取る
return (
<>
<div>{formatPrice(order.totalPrice)}</div>
<button disabled={!order.canCheckout}>注文確定</button>
</>
)
}
```
```tsx
// BAD - フロントエンドでステータス遷移判定
function TaskCard({ task }: { task: Task }) {
const canStart = task.status === 'pending' && task.assignee !== null
const canComplete = task.status === 'in_progress' && /* 複雑な条件... */
return (
<>
<button onClick={startTask} disabled={!canStart}>開始</button>
<button onClick={completeTask} disabled={!canComplete}>完了</button>
</>
)
}
// GOOD - サーバーが許可するアクションを返す
function TaskCard({ task }: { task: Task }) {
// task.allowedActions = ['start', 'cancel'] など、サーバーが計算
const canStart = task.allowedActions.includes('start')
const canComplete = task.allowedActions.includes('complete')
return (
<>
<button onClick={startTask} disabled={!canStart}>開始</button>
<button onClick={completeTask} disabled={!canComplete}>完了</button>
</>
)
}
```
例外フロントエンドにロジックを置いてもOK:
| ケース | 理由 |
|--------|------|
| UI専用バリデーション | 「必須入力」「文字数制限」等のUXフィードバックサーバー側でも検証必須 |
| クライアント側フィルタ/ソート | サーバーから受け取ったリストの表示順序変更 |
| 表示条件の分岐 | 「ログイン済みなら詳細表示」等のUI制御 |
| リアルタイムフィードバック | 入力中のプレビュー表示 |
判断基準: 「この計算結果がサーバーとズレたら業務が壊れるか?」
- YES → バックエンドに配置(ドメインロジック)
- NO → フロントエンドでもOK表示ロジック
## パフォーマンス
| 基準 | 判定 |
|------|------|
| 不要な再レンダリング | 最適化が必要 |
| 大きなリストの仮想化なし | 警告 |
| 画像の最適化なし | 警告 |
| バンドルに未使用コード | tree-shakingを確認 |
| メモ化の過剰使用 | 本当に必要か確認 |
最適化チェックリスト:
- `React.memo` / `useMemo` / `useCallback` は適切か
- 大きなリストは仮想スクロール対応か
- Code Splittingは適切か
- 画像はlazy loadingされているか
アンチパターン:
```tsx
// レンダリングごとに新しいオブジェクト
<Child style={{ color: 'red' }} />
// 定数化 or useMemo
const style = useMemo(() => ({ color: 'red' }), []);
<Child style={style} />
```
## アクセシビリティ
| 基準 | 判定 |
|------|------|
| インタラクティブ要素にキーボード対応なし | REJECT |
| 画像にalt属性なし | REJECT |
| フォーム要素にlabelなし | REJECT |
| 色だけで情報を伝達 | REJECT |
| フォーカス管理の欠如(モーダル等) | REJECT |
チェックリスト:
- セマンティックHTMLを使用しているか
- ARIA属性は適切か過剰でないか
- キーボードナビゲーション可能か
- スクリーンリーダーで意味が通じるか
- カラーコントラストは十分か
## TypeScript/型安全性
| 基準 | 判定 |
|------|------|
| `any` 型の使用 | REJECT |
| 型アサーションasの乱用 | 要検討 |
| Props型定義なし | REJECT |
| イベントハンドラの型が不適切 | 修正が必要 |
## フロントエンドセキュリティ
| 基準 | 判定 |
|------|------|
| dangerouslySetInnerHTML使用 | XSSリスクを確認 |
| ユーザー入力の未サニタイズ | REJECT |
| 機密情報のフロントエンド保存 | REJECT |
| CSRFトークンの未使用 | 要確認 |
## テスタビリティ
| 基準 | 判定 |
|------|------|
| data-testid等の未付与 | 警告 |
| テスト困難な構造 | 分離を検討 |
| ビジネスロジックのUIへの埋め込み | REJECT |
## アンチパターン検出
以下を見つけたら REJECT:
| アンチパターン | 問題 |
|---------------|------|
| God Component | 1コンポーネントに全機能が集中 |
| Prop Drilling | 深いPropsバケツリレー |
| Inline Styles乱用 | 保守性低下 |
| useEffect地獄 | 依存関係が複雑すぎる |
| Premature Optimization | 不要なメモ化 |
| Magic Strings | ハードコードされた文字列 |
| Hidden Dependencies | 子コンポーネントの隠れたAPI呼び出し |
| Over-generalization | 無理やり汎用化したコンポーネント |

View File

@ -0,0 +1,164 @@
# セキュリティ知識
## AI生成コードのセキュリティ問題
AI生成コードには特有の脆弱性パターンがある。
| パターン | リスク | 例 |
|---------|--------|-----|
| もっともらしいが危険なデフォルト | 高 | `cors: { origin: '*' }` は問題なく見えるが危険 |
| 古いセキュリティプラクティス | 中 | 非推奨の暗号化、古い認証パターンの使用 |
| 不完全なバリデーション | 高 | 形式は検証するがビジネスルールを検証しない |
| 入力を過度に信頼 | 重大 | 内部APIは常に安全と仮定 |
| コピペによる脆弱性 | 高 | 同じ危険なパターンが複数ファイルで繰り返される |
特に厳しく審査が必要:
- 認証・認可ロジックAIはエッジケースを見落としがち
- 入力バリデーションAIは構文を検証しても意味を見落とす可能性
- エラーメッセージAIは内部詳細を露出する可能性
- 設定ファイルAIは学習データから危険なデフォルトを使う可能性
## インジェクション攻撃
**SQLインジェクション**
- 文字列連結によるSQL構築 → REJECT
- パラメータ化クエリの不使用 → REJECT
- ORMの raw query での未サニタイズ入力 → REJECT
```typescript
// NG
db.query(`SELECT * FROM users WHERE id = ${userId}`)
// OK
db.query('SELECT * FROM users WHERE id = ?', [userId])
```
**コマンドインジェクション**
- `exec()`, `spawn()` での未検証入力 → REJECT
- シェルコマンド構築時のエスケープ不足 → REJECT
```typescript
// NG
exec(`ls ${userInput}`)
// OK
execFile('ls', [sanitizedInput])
```
**XSS (Cross-Site Scripting)**
- HTML/JSへの未エスケープ出力 → REJECT
- `innerHTML`, `dangerouslySetInnerHTML` の不適切な使用 → REJECT
- URLパラメータの直接埋め込み → REJECT
## 認証・認可
**認証の問題**
- ハードコードされたクレデンシャル → 即REJECT
- 平文パスワードの保存 → 即REJECT
- 弱いハッシュアルゴリズム (MD5, SHA1) → REJECT
- セッショントークンの不適切な管理 → REJECT
**認可の問題**
- 権限チェックの欠如 → REJECT
- IDOR (Insecure Direct Object Reference) → REJECT
- 権限昇格の可能性 → REJECT
```typescript
// NG - 権限チェックなし
app.get('/user/:id', (req, res) => {
return db.getUser(req.params.id)
})
// OK
app.get('/user/:id', authorize('read:user'), (req, res) => {
if (req.user.id !== req.params.id && !req.user.isAdmin) {
return res.status(403).send('Forbidden')
}
return db.getUser(req.params.id)
})
```
## データ保護
**機密情報の露出**
- APIキー、シークレットのハードコーディング → 即REJECT
- ログへの機密情報出力 → REJECT
- エラーメッセージでの内部情報露出 → REJECT
- `.env` ファイルのコミット → REJECT
**データ検証**
- 入力値の未検証 → REJECT
- 型チェックの欠如 → REJECT
- サイズ制限の未設定 → REJECT
## 暗号化
- 弱い暗号アルゴリズムの使用 → REJECT
- 固定IV/Nonceの使用 → REJECT
- 暗号化キーのハードコーディング → 即REJECT
- HTTPSの未使用本番環境 → REJECT
## ファイル操作
**パストラバーサル**
- ユーザー入力を含むファイルパス → REJECT
- `../` のサニタイズ不足 → REJECT
```typescript
// NG
const filePath = path.join(baseDir, userInput)
fs.readFile(filePath)
// OK
const safePath = path.resolve(baseDir, userInput)
if (!safePath.startsWith(path.resolve(baseDir))) {
throw new Error('Invalid path')
}
```
**ファイルアップロード**
- ファイルタイプの未検証 → REJECT
- ファイルサイズ制限なし → REJECT
- 実行可能ファイルのアップロード許可 → REJECT
## 依存関係
- 既知の脆弱性を持つパッケージ → REJECT
- メンテナンスされていないパッケージ → 警告
- 不必要な依存関係 → 警告
## エラーハンドリング
- スタックトレースの本番露出 → REJECT
- 詳細なエラーメッセージの露出 → REJECT
- エラーの握りつぶし(セキュリティイベント) → REJECT
## レート制限・DoS対策
- レート制限の欠如(認証エンドポイント) → 警告
- リソース枯渇攻撃の可能性 → 警告
- 無限ループの可能性 → REJECT
## OWASP Top 10 チェックリスト
| カテゴリ | 確認事項 |
|---------|---------|
| A01 Broken Access Control | 認可チェック、CORS設定 |
| A02 Cryptographic Failures | 暗号化、機密データ保護 |
| A03 Injection | SQL, コマンド, XSS |
| A04 Insecure Design | セキュリティ設計パターン |
| A05 Security Misconfiguration | デフォルト設定、不要な機能 |
| A06 Vulnerable Components | 依存関係の脆弱性 |
| A07 Auth Failures | 認証メカニズム |
| A08 Software Integrity | コード署名、CI/CD |
| A09 Logging Failures | セキュリティログ |
| A10 SSRF | サーバーサイドリクエスト |

View File

@ -24,431 +24,3 @@
- 軽微な問題でも後に持ち越さない。今修正できる問題は今修正させる - 軽微な問題でも後に持ち越さない。今修正できる問題は今修正させる
- 「条件付き承認」はしない。問題があれば差し戻す - 「条件付き承認」はしない。問題があれば差し戻す
- 既存コードの踏襲を理由にした問題の放置は認めない - 既存コードの踏襲を理由にした問題の放置は認めない
## ドメイン知識
### 構造・設計
**ファイル分割**
| 基準 | 判定 |
|--------------|------|
| 1ファイル200行超 | 分割を検討 |
| 1ファイル300行超 | REJECT |
| 1ファイルに複数の責務 | REJECT |
| 関連性の低いコードが同居 | REJECT |
**モジュール構成**
- 高凝集: 関連する機能がまとまっているか
- 低結合: モジュール間の依存が最小限か
- 循環依存がないか
- 適切なディレクトリ階層か
**関数設計**
- 1関数1責務になっているか
- 30行を超える関数は分割を検討
- 副作用が明確か
**レイヤー設計**
- 依存の方向: 上位層 → 下位層(逆方向禁止)
- Controller → Service → Repository の流れが守られているか
- 1インターフェース = 1責務巨大なServiceクラス禁止
**ディレクトリ構造**
構造パターンの選択:
| パターン | 適用場面 | 例 |
|---------|---------|-----|
| レイヤード | 小規模、CRUD中心 | `controllers/`, `services/`, `repositories/` |
| Vertical Slice | 中〜大規模、機能独立性が高い | `features/auth/`, `features/order/` |
| ハイブリッド | 共通基盤 + 機能モジュール | `core/` + `features/` |
Vertical Slice Architecture機能単位でコードをまとめる構造:
```
src/
├── features/
│ ├── auth/
│ │ ├── LoginCommand.ts
│ │ ├── LoginHandler.ts
│ │ ├── AuthRepository.ts
│ │ └── auth.test.ts
│ └── order/
│ ├── CreateOrderCommand.ts
│ ├── CreateOrderHandler.ts
│ └── ...
└── shared/ # 複数featureで共有
├── database/
└── middleware/
```
Vertical Slice の判定基準:
| 基準 | 判定 |
|------|------|
| 1機能が3ファイル以上のレイヤーに跨る | Slice化を検討 |
| 機能間の依存がほぼない | Slice化推奨 |
| 共通処理が50%以上 | レイヤード維持 |
| チームが機能別に分かれている | Slice化必須 |
禁止パターン:
| パターン | 問題 |
|---------|------|
| `utils/` の肥大化 | 責務不明の墓場になる |
| `common/` への安易な配置 | 依存関係が不明確になる |
| 深すぎるネスト4階層超 | ナビゲーション困難 |
| 機能とレイヤーの混在 | `features/services/` は禁止 |
**責務の分離**
- 読み取りと書き込みの責務が分かれているか
- データ取得はルートView/Controllerで行い、子に渡しているか
- エラーハンドリングが一元化されているか各所でtry-catch禁止
- ビジネスロジックがController/Viewに漏れていないか
### コード品質の検出手法
**説明コメントWhat/Howの検出基準**
コードの動作をそのまま言い換えているコメントを検出する。
| 判定 | 基準 |
|------|------|
| REJECT | コードの動作をそのまま自然言語で言い換えている |
| REJECT | 関数名・変数名から明らかなことを繰り返している |
| REJECT | JSDocが関数名の言い換えだけで情報を追加していない |
| OK | なぜその実装を選んだかの設計判断を説明している |
| OK | 一見不自然に見える挙動の理由を説明している |
| 最良 | コメントなしでコード自体が意図を語っている |
```typescript
// REJECT - コードの言い換えWhat
// If interrupted, abort immediately
if (status === 'interrupted') {
return ABORT_STEP;
}
// REJECT - ループの存在を言い換えただけ
// Check transitions in order
for (const transition of step.transitions) {
// REJECT - 関数名の繰り返し
/** Check if status matches transition condition. */
export function matchesCondition(status: Status, condition: TransitionCondition): boolean {
// OK - 設計判断の理由Why
// ユーザー中断はピース定義のトランジションより優先する
if (status === 'interrupted') {
return ABORT_STEP;
}
// OK - 一見不自然な挙動の理由
// stay はループを引き起こす可能性があるが、ユーザーが明示的に指定した場合のみ使われる
return step.name;
```
**状態の直接変更の検出基準**
配列やオブジェクトの直接変更(ミューテーション)を検出する。
```typescript
// REJECT - 配列の直接変更
const steps: Step[] = getSteps();
steps.push(newStep); // 元の配列を破壊
steps.splice(index, 1); // 元の配列を破壊
steps[0].status = 'done'; // ネストされたオブジェクトも直接変更
// OK - イミュータブルな操作
const withNew = [...steps, newStep];
const without = steps.filter((_, i) => i !== index);
const updated = steps.map((s, i) =>
i === 0 ? { ...s, status: 'done' } : s
);
// REJECT - オブジェクトの直接変更
function updateConfig(config: Config) {
config.logLevel = 'debug'; // 引数を直接変更
config.steps.push(newStep); // ネストも直接変更
return config;
}
// OK - 新しいオブジェクトを返す
function updateConfig(config: Config): Config {
return {
...config,
logLevel: 'debug',
steps: [...config.steps, newStep],
};
}
```
### セキュリティ(基本チェック)
- インジェクション対策SQL, コマンド, XSS
- ユーザー入力の検証
- 機密情報のハードコーディング
### テスタビリティ
- 依存性注入が可能な設計か
- モック可能か
- テストが書かれているか
### アンチパターン検出
以下のパターンを見つけたら REJECT:
| アンチパターン | 問題 |
|---------------|------|
| God Class/Component | 1つのクラスが多くの責務を持っている |
| Feature Envy | 他モジュールのデータを頻繁に参照している |
| Shotgun Surgery | 1つの変更が複数ファイルに波及する構造 |
| 過度な汎用化 | 今使わないバリアントや拡張ポイント |
| 隠れた依存 | 子コンポーネントが暗黙的にAPIを呼ぶ等 |
| 非イディオマティック | 言語・FWの作法を無視した独自実装 |
### 抽象化レベルの評価
**条件分岐の肥大化検出**
| パターン | 判定 |
|---------|------|
| 同じif-elseパターンが3箇所以上 | ポリモーフィズムで抽象化 → REJECT |
| switch/caseが5分岐以上 | Strategy/Mapパターンを検討 |
| フラグ引数で挙動を変える | 別関数に分割 → REJECT |
| 型による分岐instanceof/typeof | ポリモーフィズムに置換 → REJECT |
| ネストした条件分岐3段以上 | 早期リターンまたは抽出 → REJECT |
**抽象度の不一致検出**
| パターン | 問題 | 修正案 |
|---------|------|--------|
| 高レベル処理の中に低レベル詳細 | 読みにくい | 詳細を関数に抽出 |
| 1関数内で抽象度が混在 | 認知負荷 | 同じ粒度に揃える |
| ビジネスロジックにDB操作が混在 | 責務違反 | Repository層に分離 |
| 設定値と処理ロジックが混在 | 変更困難 | 設定を外部化 |
**良い抽象化の例**
```typescript
// 条件分岐の肥大化
function process(type: string) {
if (type === 'A') { /* 処理A */ }
else if (type === 'B') { /* 処理B */ }
else if (type === 'C') { /* 処理C */ }
// ...続く
}
// Mapパターンで抽象化
const processors: Record<string, () => void> = {
A: processA,
B: processB,
C: processC,
};
function process(type: string) {
processors[type]?.();
}
```
```typescript
// 抽象度の混在
function createUser(data: UserData) {
// 高レベル: ビジネスロジック
validateUser(data);
// 低レベル: DB操作の詳細
const conn = await pool.getConnection();
await conn.query('INSERT INTO users...');
conn.release();
}
// 抽象度を揃える
function createUser(data: UserData) {
validateUser(data);
await userRepository.save(data); // 詳細は隠蔽
}
```
### その場しのぎの検出
「とりあえず動かす」ための妥協を見逃さない。
| パターン | 例 |
|---------|-----|
| 不要なパッケージ追加 | 動かすためだけに入れた謎のライブラリ |
| テストの削除・スキップ | `@Disabled``.skip()`、コメントアウト |
| 空実装・スタブ放置 | `return null``// TODO: implement``pass` |
| モックデータの本番混入 | ハードコードされたダミーデータ |
| エラー握りつぶし | 空の `catch {}``rescue nil` |
| マジックナンバー | 説明なしの `if (status == 3)` |
### TODOコメントの厳格な禁止
「将来やる」は決してやらない。今やらないことは永遠にやらない。
TODOコメントは即REJECT。
```kotlin
// REJECT - 将来を見越したTODO
// TODO: 施設IDによる認可チェックを追加
fun deleteCustomHoliday(@PathVariable id: String) {
deleteCustomHolidayInputPort.execute(input)
}
// APPROVE - 今実装する
fun deleteCustomHoliday(@PathVariable id: String) {
val currentUserFacilityId = getCurrentUserFacilityId()
val holiday = findHolidayById(id)
require(holiday.facilityId == currentUserFacilityId) {
"Cannot delete holiday from another facility"
}
deleteCustomHolidayInputPort.execute(input)
}
```
TODOが許容される唯一のケース:
| 条件 | 例 | 判定 |
|------|-----|------|
| 外部依存で今は実装不可 + Issue化済み | `// TODO(#123): APIキー取得後に実装` | 許容 |
| 技術的制約で回避不可 + Issue化済み | `// TODO(#456): ライブラリバグ修正待ち` | 許容 |
| 「将来実装」「後で追加」 | `// TODO: バリデーション追加` | REJECT |
| 「時間がないので」 | `// TODO: リファクタリング` | REJECT |
正しい対処:
- 今必要 → 今実装する
- 今不要 → コードを削除する
- 外部要因で不可 → Issue化してチケット番号をコメントに入れる
### DRY違反の検出
重複コードを検出する。
| パターン | 判定 |
|---------|------|
| 同じロジックが3箇所以上 | 即REJECT - 関数/メソッドに抽出 |
| 同じバリデーションが2箇所以上 | 即REJECT - バリデーター関数に抽出 |
| 似たようなコンポーネントが3個以上 | 即REJECT - 共通コンポーネント化 |
| コピペで派生したコード | 即REJECT - パラメータ化または抽象化 |
AHA原則Avoid Hasty Abstractionsとのバランス:
- 2回の重複 → 様子見
- 3回の重複 → 即抽出
- ドメインが異なる重複 → 抽象化しない(例: 顧客用バリデーションと管理者用バリデーションは別物)
### 仕様準拠の検証
変更が、プロジェクトの文書化された仕様に準拠しているか検証する。
検証対象:
| 対象 | 確認内容 |
|------|---------|
| CLAUDE.md / README.md | スキーマ定義、設計原則、制約に従っているか |
| 型定義・Zodスキーマ | 新しいフィールドがスキーマに反映されているか |
| YAML/JSON設定ファイル | 文書化されたフォーマットに従っているか |
具体的なチェック:
1. 設定ファイルYAML等を変更・追加した場合:
- CLAUDE.md等に記載されたスキーマ定義と突合する
- 無視されるフィールドや無効なフィールドが含まれていないか
- 必須フィールドが欠落していないか
2. 型定義やインターフェースを変更した場合:
- ドキュメントのスキーマ説明が更新されているか
- 既存の設定ファイルが新しいスキーマと整合するか
このパターンを見つけたら REJECT:
| パターン | 問題 |
|---------|------|
| 仕様に存在しないフィールドの使用 | 無視されるか予期しない動作 |
| 仕様上無効な値の設定 | 実行時エラーまたは無視される |
| 文書化された制約への違反 | 設計意図に反する |
### 呼び出しチェーン検証
新しいパラメータ・フィールドが追加された場合、変更ファイル内だけでなく呼び出し元も検証する。
検証手順:
1. 新しいオプショナルパラメータや interface フィールドを見つけたら、`Grep` で全呼び出し元を検索
2. 全呼び出し元が新しいパラメータを渡しているか確認
3. フォールバック値(`?? default`)がある場合、フォールバックが使われるケースが意図通りか確認
危険パターン:
| パターン | 問題 | 検出方法 |
|---------|------|---------|
| `options.xxx ?? fallback` で全呼び出し元が `xxx` を省略 | 機能が実装されているのに常にフォールバック | grep で呼び出し元を確認 |
| テストがモックで直接値をセット | 実際の呼び出しチェーンを経由しない | テストの構築方法を確認 |
| `executeXxx()` が内部で使う `options` を引数で受け取らない | 上位から値を渡す口がない | 関数シグネチャを確認 |
```typescript
// 配線漏れ: projectCwd を受け取る口がない
export async function executePiece(config, cwd, task) {
const engine = new PieceEngine(config, cwd, task); // options なし
}
// 配線済み: projectCwd を渡せる
export async function executePiece(config, cwd, task, options?) {
const engine = new PieceEngine(config, cwd, task, options);
}
```
呼び出し元の制約による論理的デッドコード:
呼び出しチェーンの検証は「配線漏れ」だけでなく、逆方向——呼び出し元が既に保証している条件に対する不要な防御コード——にも適用する。
| パターン | 問題 | 検出方法 |
|---------|------|---------|
| 呼び出し元がTTY必須なのに関数内でTTYチェック | 到達しない分岐が残る | grep で全呼び出し元の前提条件を確認 |
| 呼び出し元がnullチェック済みなのに再度nullガード | 冗長な防御 | 呼び出し元の制約を追跡 |
| 呼び出し元が型で制約しているのにランタイムチェック | 型安全を信頼していない | TypeScriptの型制約を確認 |
検証手順:
1. 防御的な条件分岐TTYチェック、nullガード等を見つけたら、grep で全呼び出し元を確認
2. 全呼び出し元がその条件を既に保証しているなら、防御は不要 → REJECT
3. 一部の呼び出し元が保証していない場合は、防御を残す
### 品質特性
| 特性 | 確認観点 |
|------|---------|
| Scalability | 負荷増加に対応できる設計か |
| Maintainability | 変更・修正が容易か |
| Observability | ログ・監視が可能な設計か |
### 大局観
細かい「クリーンコード」の指摘に終始しない。
確認すべきこと:
- このコードは将来どう変化するか
- スケーリングの必要性は考慮されているか
- 技術的負債を生んでいないか
- ビジネス要件と整合しているか
- 命名がドメインと一貫しているか
### 変更スコープの評価
変更スコープを確認し、レポートに記載する(ブロッキングではない)。
| スコープサイズ | 変更行数 | 対応 |
|---------------|---------|------|
| Small | 〜200行 | そのままレビュー |
| Medium | 200-500行 | そのままレビュー |
| Large | 500行以上 | レビューは継続。分割可能か提案を付記 |
大きな変更が必要なタスクもある。行数だけでREJECTしない。
確認すること:
- 変更が論理的にまとまっているか(無関係な変更が混在していないか)
- Coderのスコープ宣言と実際の変更が一致しているか
提案として記載すること(ブロッキングではない):
- 分割可能な場合は分割案を提示

View File

@ -26,421 +26,3 @@
- 読み取りと書き込みは本質的に異なる関心事であり、無理に統合しない - 読み取りと書き込みは本質的に異なる関心事であり、無理に統合しない
- 形だけのCQRSを見逃さない。CRUDをCommand/Queryに分けただけでは意味がない - 形だけのCQRSを見逃さない。CRUDをCommand/Queryに分けただけでは意味がない
- シンプルなCRUDで十分なケースにCQRS+ESを強制しない - シンプルなCRUDで十分なケースにCQRS+ESを強制しない
## ドメイン知識
### Aggregate設計
Aggregateは判断に必要なフィールドのみ保持する。
Command ModelAggregateの役割は「コマンドを受けて判断し、イベントを発行する」こと。クエリ用データはRead ModelProjectionが担当する。
「判断に必要」とは:
- `if`/`require`の条件分岐に使う
- インスタンスメソッドでイベント発行時にフィールド値を参照する
| 基準 | 判定 |
|------|------|
| Aggregateが複数のトランザクション境界を跨ぐ | REJECT |
| Aggregate間の直接参照ID参照でない | REJECT |
| Aggregateが100行を超える | 分割を検討 |
| ビジネス不変条件がAggregate外にある | REJECT |
| 判断に使わないフィールドを保持 | REJECT |
良いAggregate:
```kotlin
// 判断に必要なフィールドのみ
data class Order(
val orderId: String, // イベント発行時に使用
val status: OrderStatus // 状態チェックに使用
) {
fun confirm(confirmedBy: String): OrderConfirmedEvent {
require(status == OrderStatus.PENDING) { "確定できる状態ではありません" }
return OrderConfirmedEvent(
orderId = orderId,
confirmedBy = confirmedBy,
confirmedAt = LocalDateTime.now()
)
}
}
// 判断に使わないフィールドを保持NG
data class Order(
val orderId: String,
val customerId: String, // 判断に未使用
val shippingAddress: Address, // 判断に未使用
val status: OrderStatus
)
```
追加操作がないAggregateはIDのみ:
```kotlin
// 作成のみで追加操作がない場合
data class Notification(val notificationId: String) {
companion object {
fun create(customerId: String, message: String): NotificationCreatedEvent {
return NotificationCreatedEvent(
notificationId = UUID.randomUUID().toString(),
customerId = customerId,
message = message
)
}
}
}
```
### イベント設計
| 基準 | 判定 |
|------|------|
| イベントが過去形でないCreated → Create | REJECT |
| イベントにロジックが含まれる | REJECT |
| イベントが他Aggregateの内部状態を含む | REJECT |
| イベントのスキーマがバージョン管理されていない | 警告 |
| CRUDスタイルのイベントUpdated, Deleted | 要検討 |
良いイベント:
```kotlin
// Good: ドメインの意図が明確
OrderPlaced, PaymentReceived, ItemShipped
// Bad: CRUDスタイル
OrderUpdated, OrderDeleted
```
イベント粒度:
- 細かすぎ: `OrderFieldChanged` → ドメインの意図が不明
- 適切: `ShippingAddressChanged` → 意図が明確
- 粗すぎ: `OrderModified` → 何が変わったか不明
### コマンドハンドラ
| 基準 | 判定 |
|------|------|
| ハンドラがDBを直接操作 | REJECT |
| ハンドラが複数Aggregateを変更 | REJECT |
| コマンドのバリデーションがない | REJECT |
| ハンドラがクエリを実行して判断 | 要検討 |
良いコマンドハンドラ:
```
1. コマンドを受け取る
2. Aggregateをイベントストアから復元
3. Aggregateにコマンドを適用
4. 発行されたイベントを保存
```
### プロジェクション設計
| 基準 | 判定 |
|------|------|
| プロジェクションがコマンドを発行 | REJECT |
| プロジェクションがWriteモデルを参照 | REJECT |
| 複数のユースケースを1つのプロジェクションで賄う | 要検討 |
| リビルド不可能な設計 | REJECT |
良いプロジェクション:
- 特定の読み取りユースケースに最適化
- イベントから冪等に再構築可能
- Writeモデルから完全に独立
### Query側の設計
ControllerはQueryGatewayを使う。Repositoryを直接使わない。
レイヤー間の型:
- `application/query/` - Query結果の型例: `OrderDetail`
- `adapter/protocol/` - RESTレスポンスの型例: `OrderDetailResponse`
- QueryHandlerはapplication層の型を返し、Controllerがadapter層の型に変換
```kotlin
// application/query/OrderDetail.kt
data class OrderDetail(
val orderId: String,
val customerName: String,
val totalAmount: Money
)
// adapter/protocol/OrderDetailResponse.kt
data class OrderDetailResponse(...) {
companion object {
fun from(detail: OrderDetail) = OrderDetailResponse(...)
}
}
// QueryHandler - application層の型を返す
@QueryHandler
fun handle(query: GetOrderDetailQuery): OrderDetail? {
val entity = repository.findById(query.id) ?: return null
return OrderDetail(...)
}
// Controller - adapter層の型に変換
@GetMapping("/{id}")
fun getById(@PathVariable id: String): ResponseEntity<OrderDetailResponse> {
val detail = queryGateway.query(
GetOrderDetailQuery(id),
OrderDetail::class.java
).join() ?: throw NotFoundException("...")
return ResponseEntity.ok(OrderDetailResponse.from(detail))
}
```
構成:
```
Controller (adapter) → QueryGateway → QueryHandler (application) → Repository
↓ ↓
Response.from(detail) OrderDetail
```
### 結果整合性
| 状況 | 対応 |
|------|------|
| UIが即座に更新を期待している | 設計見直し or ポーリング/WebSocket |
| 整合性遅延が許容範囲を超える | アーキテクチャ再検討 |
| 補償トランザクションが未定義 | 障害シナリオの検討を要求 |
### Saga vs EventHandler
Sagaは「競合が発生する複数アグリゲート間の操作」にのみ使用する。
Sagaが必要なケース:
```
複数のアクターが同じリソースを取り合う場合
例: 在庫確保10人が同時に同じ商品を注文
OrderPlacedEvent
↓ InventoryReservationSaga
ReserveInventoryCommand → Inventory集約同時実行を直列化
InventoryReservedEvent → ConfirmOrderCommand
InventoryReservationFailedEvent → CancelOrderCommand
```
Sagaが不要なケース:
```
競合が発生しない操作
例: 注文キャンセル時の在庫解放
OrderCancelledEvent
↓ InventoryReleaseHandler単純なEventHandler
ReleaseInventoryCommand
InventoryReleasedEvent
```
判断基準:
| 状況 | Saga | EventHandler |
|------|------|--------------|
| リソースの取り合いがある | 使う | - |
| 補償トランザクションが必要 | 使う | - |
| 競合しない単純な連携 | - | 使う |
| 失敗時は再試行で十分 | - | 使う |
アンチパターン:
```kotlin
// NG - ライフサイクル管理のためにSagaを使う
@Saga
class OrderLifecycleSaga {
// 注文の全状態遷移をSagaで追跡
// PLACED → CONFIRMED → SHIPPED → DELIVERED
}
// OK - 結果整合性が必要な操作だけをSagaで処理
@Saga
class InventoryReservationSaga {
// 在庫確保の同時実行制御のみ
}
```
Sagaはライフサイクル管理ツールではない。結果整合性が必要な「操作」単位で作成する。
### 例外 vs イベント(失敗時の選択)
監査不要な失敗は例外、監査が必要な失敗はイベント。
例外アプローチ(推奨: ほとんどのケース):
```kotlin
// ドメインモデル: バリデーション失敗時に例外をスロー
fun reserveInventory(orderId: String, quantity: Int): InventoryReservedEvent {
if (availableQuantity < quantity) {
throw InsufficientInventoryException("在庫が不足しています")
}
return InventoryReservedEvent(productId, orderId, quantity)
}
// Saga: exceptionally でキャッチして補償アクション
commandGateway.send<Any>(command)
.exceptionally { ex ->
commandGateway.send<Any>(CancelOrderCommand(
orderId = orderId,
reason = ex.cause?.message ?: "在庫確保に失敗しました"
))
null
}
```
イベントアプローチ(稀なケース):
```kotlin
// 監査が必要な場合のみ
data class PaymentFailedEvent(
val paymentId: String,
val reason: String,
val attemptedAmount: Money
) : PaymentEvent
```
判断基準:
| 質問 | 例外 | イベント |
|------|------|----------|
| この失敗を後で確認する必要があるか? | No | Yes |
| 規制やコンプライアンスで記録が必要か? | No | Yes |
| Sagaだけが失敗を気にするか? | Yes | No |
| Event Storeに残すと価値があるか? | No | Yes |
デフォルトは例外アプローチ。監査要件がある場合のみイベントを検討する。
### 抽象化レベルの評価
**条件分岐の肥大化検出**
| パターン | 判定 |
|---------|------|
| 同じif-elseパターンが3箇所以上 | ポリモーフィズムで抽象化 → REJECT |
| switch/caseが5分岐以上 | Strategy/Mapパターンを検討 |
| イベント種別による分岐が増殖 | イベントハンドラを分離 → REJECT |
| Aggregate内の状態分岐が複雑 | State Patternを検討 |
**抽象度の不一致検出**
| パターン | 問題 | 修正案 |
|---------|------|--------|
| CommandHandlerにDB操作詳細 | 責務違反 | Repository層に分離 |
| EventHandlerにビジネスロジック | 責務違反 | ドメインサービスに抽出 |
| Aggregateに永続化処理 | レイヤー違反 | EventStore経由に変更 |
| Projectionに計算ロジック | 保守困難 | 専用サービスに抽出 |
良い抽象化の例:
```kotlin
// イベント種別による分岐の増殖NG
@EventHandler
fun on(event: DomainEvent) {
when (event) {
is OrderPlacedEvent -> handleOrderPlaced(event)
is OrderConfirmedEvent -> handleOrderConfirmed(event)
is OrderShippedEvent -> handleOrderShipped(event)
// ...どんどん増える
}
}
// イベントごとにハンドラを分離OK
@EventHandler
fun on(event: OrderPlacedEvent) { ... }
@EventHandler
fun on(event: OrderConfirmedEvent) { ... }
@EventHandler
fun on(event: OrderShippedEvent) { ... }
```
```kotlin
// 状態による分岐が複雑NG
fun process(command: ProcessCommand) {
when (status) {
PENDING -> if (command.type == "approve") { ... } else if (command.type == "reject") { ... }
APPROVED -> if (command.type == "ship") { ... }
// ...複雑化
}
}
// State Patternで抽象化OK
sealed class OrderState {
abstract fun handle(command: ProcessCommand): List<DomainEvent>
}
class PendingState : OrderState() {
override fun handle(command: ProcessCommand) = when (command) {
is ApproveCommand -> listOf(OrderApprovedEvent(...))
is RejectCommand -> listOf(OrderRejectedEvent(...))
else -> throw InvalidCommandException()
}
}
```
### アンチパターン検出
以下を見つけたら REJECT:
| アンチパターン | 問題 |
|---------------|------|
| CRUD偽装 | CQRSの形だけ真似てCRUD実装 |
| Anemic Domain Model | Aggregateが単なるデータ構造 |
| Event Soup | 意味のないイベントが乱発される |
| Temporal Coupling | イベント順序に暗黙の依存 |
| Missing Events | 重要なドメインイベントが欠落 |
| God Aggregate | 1つのAggregateに全責務が集中 |
### テスト戦略
レイヤーごとにテスト方針を分ける。
テストピラミッド:
```
┌─────────────┐
│ E2E Test │ ← 少数: 全体フロー確認
├─────────────┤
│ Integration │ ← Command→Event→Projection→Query の連携確認
├─────────────┤
│ Unit Test │ ← 多数: 各レイヤー独立テスト
└─────────────┘
```
Command側Aggregate:
```kotlin
// AggregateTestFixture使用
@Test
fun `確定コマンドでイベントが発行される`() {
fixture
.given(OrderPlacedEvent(...))
.`when`(ConfirmOrderCommand(orderId, confirmedBy))
.expectSuccessfulHandlerExecution()
.expectEvents(OrderConfirmedEvent(...))
}
```
Query側:
```kotlin
// Read Model直接セットアップ + QueryGateway
@Test
fun `注文詳細が取得できる`() {
// Given: Read Modelを直接セットアップ
orderRepository.save(OrderEntity(...))
// When: QueryGateway経由でクエリ実行
val detail = queryGateway.query(GetOrderDetailQuery(orderId), ...).join()
// Then
assertEquals(expectedDetail, detail)
}
```
チェック項目:
| 観点 | 判定 |
|------|------|
| Aggregateテストが状態ではなくイベントを検証している | 必須 |
| Query側テストがCommand経由でデータを作っていない | 推奨 |
| 統合テストでAxonの非同期処理を考慮している | 必須 |
### インフラ層
確認事項:
- イベントストアの選択は適切か
- メッセージング基盤は要件を満たすか
- スナップショット戦略は定義されているか
- イベントのシリアライズ形式は適切か

View File

@ -27,501 +27,3 @@
- アクセシビリティは後付け困難。最初から組み込む - アクセシビリティは後付け困難。最初から組み込む
- 過度な抽象化を警戒。シンプルに保つ - 過度な抽象化を警戒。シンプルに保つ
- フレームワークの作法に従う。独自パターンより標準的なアプローチ - フレームワークの作法に従う。独自パターンより標準的なアプローチ
## ドメイン知識
### コンポーネント設計
1ファイルにベタ書きしない。必ずコンポーネント分割する。
分離が必須なケース:
- 独自のstateを持つ → 必ず分離
- 50行超のJSX → 分離
- 再利用可能 → 分離
- 責務が複数 → 分離
- ページ内の独立したセクション → 分離
| 基準 | 判定 |
|------|------|
| 1コンポーネント200行超 | 分割を検討 |
| 1コンポーネント300行超 | REJECT |
| 表示とロジックが混在 | 分離を検討 |
| Props drilling3階層以上 | 状態管理の導入を検討 |
| 複数の責務を持つコンポーネント | REJECT |
良いコンポーネント:
- 単一責務: 1つのことをうまくやる
- 自己完結: 必要な依存が明確
- テスト可能: 副作用が分離されている
コンポーネント分類:
| 種類 | 責務 | 例 |
|------|------|-----|
| Container | データ取得・状態管理 | `UserListContainer` |
| Presentational | 表示のみ | `UserCard` |
| Layout | 配置・構造 | `PageLayout`, `Grid` |
| Utility | 共通機能 | `ErrorBoundary`, `Portal` |
ディレクトリ構成:
```
features/{feature-name}/
├── components/
│ ├── {feature}-view.tsx # メインビュー(子を組み合わせる)
│ ├── {sub-component}.tsx # サブコンポーネント
│ └── index.ts
├── hooks/
├── types.ts
└── index.ts
```
### 状態管理
子コンポーネントは自身で状態を変更しない。イベントを親にバブリングし、親が状態を操作する。
```tsx
// 子が自分で状態を変更NG
const ChildBad = ({ initialValue }: { initialValue: string }) => {
const [value, setValue] = useState(initialValue)
return <input value={value} onChange={e => setValue(e.target.value)} />
}
// 親が状態を管理、子はコールバックで通知OK
const ChildGood = ({ value, onChange }: { value: string; onChange: (v: string) => void }) => {
return <input value={value} onChange={e => onChange(e.target.value)} />
}
const Parent = () => {
const [value, setValue] = useState('')
return <ChildGood value={value} onChange={setValue} />
}
```
例外子がローカルstate持ってOK:
- UI専用の一時状態ホバー、フォーカス、アニメーション
- 親に伝える必要がない完全にローカルな状態
| 基準 | 判定 |
|------|------|
| 不要なグローバル状態 | ローカル化を検討 |
| 同じ状態が複数箇所で管理 | 正規化が必要 |
| 子から親への状態変更(逆方向データフロー) | REJECT |
| APIレスポンスをそのまま状態に | 正規化を検討 |
| useEffectの依存配列が不適切 | REJECT |
状態配置の判断基準:
| 状態の性質 | 推奨配置 |
|-----------|---------|
| UIの一時的な状態モーダル開閉等 | ローカルuseState |
| フォームの入力値 | ローカル or フォームライブラリ |
| 複数コンポーネントで共有 | Context or 状態管理ライブラリ |
| サーバーデータのキャッシュ | TanStack Query等のデータフェッチライブラリ |
### データ取得
API呼び出しはルートViewコンポーネントで行い、子コンポーネントにはpropsで渡す。
```tsx
// CORRECT - ルートでデータ取得、子に渡す
const OrderDetailView = () => {
const { data: order, isLoading, error } = useGetOrder(orderId)
const { data: items } = useListOrderItems(orderId)
if (isLoading) return <Skeleton />
if (error) return <ErrorDisplay error={error} />
return (
<OrderSummary
order={order}
items={items}
onItemSelect={handleItemSelect}
/>
)
}
// WRONG - 子コンポーネントが自分でデータ取得
const OrderSummary = ({ orderId }) => {
const { data: order } = useGetOrder(orderId)
// ...
}
```
UIの状態変更でパラメータが変わる場合週切り替え、フィルタ等:
状態もViewレベルで管理し、コンポーネントにはコールバックを渡す。
```tsx
// CORRECT - 状態もViewで管理
const ScheduleView = () => {
const [currentWeek, setCurrentWeek] = useState(startOfWeek(new Date()))
const { data } = useListSchedules({
from: format(currentWeek, 'yyyy-MM-dd'),
to: format(endOfWeek(currentWeek), 'yyyy-MM-dd'),
})
return (
<WeeklyCalendar
schedules={data?.items ?? []}
currentWeek={currentWeek}
onWeekChange={setCurrentWeek}
/>
)
}
// WRONG - コンポーネント内で状態管理+データ取得
const WeeklyCalendar = ({ facilityId }) => {
const [currentWeek, setCurrentWeek] = useState(...)
const { data } = useListSchedules({ facilityId, from, to })
// ...
}
```
例外(コンポーネント内フェッチが許容されるケース):
| ケース | 理由 |
|--------|------|
| 無限スクロール | スクロール位置というUI内部状態に依存 |
| 検索オートコンプリート | 入力値に依存したリアルタイム検索 |
| 独立したウィジェット | 通知バッジ、天気等。親のデータと完全に無関係 |
| リアルタイム更新 | WebSocket/Pollingでの自動更新 |
| モーダル内の詳細取得 | 開いたときだけ追加データを取得 |
判断基準: 「親が管理する意味がない / 親に影響を与えない」ケースのみ許容。
| 基準 | 判定 |
|------|------|
| コンポーネント内で直接fetch | Container層に分離 |
| エラーハンドリングなし | REJECT |
| ローディング状態の未処理 | REJECT |
| キャンセル処理なし | 警告 |
| N+1クエリ的なフェッチ | REJECT |
### 共有コンポーネントと抽象化
同じパターンのUIは共有コンポーネント化する。インラインスタイルのコピペは禁止。
```tsx
// WRONG - インラインスタイルのコピペ
<button className="p-2 text-[var(--text-secondary)] hover:...">
<X className="w-5 h-5" />
</button>
// CORRECT - 共有コンポーネント使用
<IconButton onClick={onClose} aria-label="閉じる">
<X className="w-5 h-5" />
</IconButton>
```
共有コンポーネント化すべきパターン:
- アイコンボタン(閉じる、編集、削除等)
- ローディング/エラー表示
- ステータスバッジ
- タブ切り替え
- ラベル+値の表示(詳細画面)
- 検索入力
- カラー凡例
過度な汎用化を避ける:
```tsx
// WRONG - IconButtonに無理やりステッパー用バリアントを追加
export const iconButtonVariants = cva('...', {
variants: {
variant: {
default: '...',
outlined: '...', // ステッパー専用、他で使わない
},
size: {
medium: 'p-2',
stepper: 'w-8 h-8', // outlinedとセットでしか使わない
},
},
})
// CORRECT - 用途別に専用コンポーネント
export function StepperButton(props) {
return (
<button className="w-8 h-8 rounded-full border ..." {...props}>
<Plus className="w-4 h-4" />
</button>
)
}
```
別コンポーネントにすべきサイン:
- 「このvariantはこのsizeとセット」のような暗黙の制約がある
- 追加したvariantが元のコンポーネントの用途と明らかに違う
- 使う側のprops指定が複雑になる
### 抽象化レベルの評価
**条件分岐の肥大化検出**
| パターン | 判定 |
|---------|------|
| 同じ条件分岐が3箇所以上 | 共通コンポーネントに抽出 → REJECT |
| propsによる分岐が5種類以上 | コンポーネント分割を検討 |
| render内の三項演算子のネスト | 早期リターンまたはコンポーネント分離 → REJECT |
| 型による分岐レンダリング | ポリモーフィックコンポーネントを検討 |
**抽象度の不一致検出**
| パターン | 問題 | 修正案 |
|---------|------|--------|
| データ取得ロジックがJSXに混在 | 読みにくい | カスタムフックに抽出 |
| ビジネスロジックがコンポーネントに混在 | 責務違反 | hooks/utilsに分離 |
| スタイル計算ロジックが散在 | 保守困難 | ユーティリティ関数に抽出 |
| 同じ変換処理が複数箇所に | DRY違反 | 共通関数に抽出 |
良い抽象化の例:
```tsx
// 条件分岐が肥大化
function UserBadge({ user }) {
if (user.role === 'admin') {
return <span className="bg-red-500">管理者</span>
} else if (user.role === 'moderator') {
return <span className="bg-yellow-500">モデレーター</span>
} else if (user.role === 'premium') {
return <span className="bg-purple-500">プレミアム</span>
} else {
return <span className="bg-gray-500">一般</span>
}
}
// Mapで抽象化
const ROLE_CONFIG = {
admin: { label: '管理者', className: 'bg-red-500' },
moderator: { label: 'モデレーター', className: 'bg-yellow-500' },
premium: { label: 'プレミアム', className: 'bg-purple-500' },
default: { label: '一般', className: 'bg-gray-500' },
}
function UserBadge({ user }) {
const config = ROLE_CONFIG[user.role] ?? ROLE_CONFIG.default
return <span className={config.className}>{config.label}</span>
}
```
```tsx
// 抽象度が混在
function OrderList() {
const [orders, setOrders] = useState([])
useEffect(() => {
fetch('/api/orders')
.then(res => res.json())
.then(data => setOrders(data))
}, [])
return orders.map(order => (
<div>{order.total.toLocaleString()}円</div>
))
}
// 抽象度を揃える
function OrderList() {
const { data: orders } = useOrders() // データ取得を隠蔽
return orders.map(order => (
<OrderItem key={order.id} order={order} />
))
}
```
### フロントエンドとバックエンドの責務分離
**表示形式の責務**
バックエンドは「データ」を返し、フロントエンドが「表示形式」に変換する。
```tsx
// フロントエンド: 表示形式に変換
export function formatPrice(amount: number): string {
return `¥${amount.toLocaleString()}`
}
export function formatDate(date: Date): string {
return format(date, 'yyyy年M月d日')
}
```
| 基準 | 判定 |
|------|------|
| バックエンドが表示用文字列を返している | 設計見直しを提案 |
| 同じフォーマット処理が複数箇所にコピペ | ユーティリティ関数に統一 |
| コンポーネント内でインラインフォーマット | 関数に抽出 |
**ドメインロジックの配置SmartUI排除**
ドメインロジック(ビジネスルール)はバックエンドに配置。フロントエンドは状態の表示・編集のみ。
ドメインロジックとは:
- 集約のビジネスルール(在庫判定、価格計算、ステータス遷移)
- バリデーション(業務制約の検証)
- 不変条件の保証
フロントエンドの責務:
- サーバーから受け取った状態を表示
- ユーザー入力を収集し、コマンドとしてバックエンドに送信
- UI専用の一時状態管理フォーカス、ホバー、モーダル開閉
- 表示形式の変換(フォーマット、ソート、フィルタ)
| 基準 | 判定 |
|------|------|
| フロントエンドで価格計算・在庫判定 | バックエンドに移動 → REJECT |
| フロントエンドでステータス遷移ルール | バックエンドに移動 → REJECT |
| フロントエンドでビジネスバリデーション | バックエンドに移動 → REJECT |
| サーバー側で計算可能な値をフロントで再計算 | 冗長 → REJECT |
良い例 vs 悪い例:
```tsx
// BAD - フロントエンドでビジネスルール
function OrderForm({ order }: { order: Order }) {
const totalPrice = order.items.reduce((sum, item) =>
sum + item.price * item.quantity, 0
)
const canCheckout = totalPrice >= 1000 && order.items.every(i => i.stock > 0)
return <button disabled={!canCheckout}>注文確定</button>
}
// GOOD - バックエンドから受け取った状態を表示
function OrderForm({ order }: { order: Order }) {
// totalPrice, canCheckout はサーバーから受け取る
return (
<>
<div>{formatPrice(order.totalPrice)}</div>
<button disabled={!order.canCheckout}>注文確定</button>
</>
)
}
```
```tsx
// BAD - フロントエンドでステータス遷移判定
function TaskCard({ task }: { task: Task }) {
const canStart = task.status === 'pending' && task.assignee !== null
const canComplete = task.status === 'in_progress' && /* 複雑な条件... */
return (
<>
<button onClick={startTask} disabled={!canStart}>開始</button>
<button onClick={completeTask} disabled={!canComplete}>完了</button>
</>
)
}
// GOOD - サーバーが許可するアクションを返す
function TaskCard({ task }: { task: Task }) {
// task.allowedActions = ['start', 'cancel'] など、サーバーが計算
const canStart = task.allowedActions.includes('start')
const canComplete = task.allowedActions.includes('complete')
return (
<>
<button onClick={startTask} disabled={!canStart}>開始</button>
<button onClick={completeTask} disabled={!canComplete}>完了</button>
</>
)
}
```
例外フロントエンドにロジックを置いてもOK:
| ケース | 理由 |
|--------|------|
| UI専用バリデーション | 「必須入力」「文字数制限」等のUXフィードバックサーバー側でも検証必須 |
| クライアント側フィルタ/ソート | サーバーから受け取ったリストの表示順序変更 |
| 表示条件の分岐 | 「ログイン済みなら詳細表示」等のUI制御 |
| リアルタイムフィードバック | 入力中のプレビュー表示 |
判断基準: 「この計算結果がサーバーとズレたら業務が壊れるか?」
- YES → バックエンドに配置(ドメインロジック)
- NO → フロントエンドでもOK表示ロジック
### パフォーマンス
| 基準 | 判定 |
|------|------|
| 不要な再レンダリング | 最適化が必要 |
| 大きなリストの仮想化なし | 警告 |
| 画像の最適化なし | 警告 |
| バンドルに未使用コード | tree-shakingを確認 |
| メモ化の過剰使用 | 本当に必要か確認 |
最適化チェックリスト:
- `React.memo` / `useMemo` / `useCallback` は適切か
- 大きなリストは仮想スクロール対応か
- Code Splittingは適切か
- 画像はlazy loadingされているか
アンチパターン:
```tsx
// レンダリングごとに新しいオブジェクト
<Child style={{ color: 'red' }} />
// 定数化 or useMemo
const style = useMemo(() => ({ color: 'red' }), []);
<Child style={style} />
```
### アクセシビリティ
| 基準 | 判定 |
|------|------|
| インタラクティブ要素にキーボード対応なし | REJECT |
| 画像にalt属性なし | REJECT |
| フォーム要素にlabelなし | REJECT |
| 色だけで情報を伝達 | REJECT |
| フォーカス管理の欠如(モーダル等) | REJECT |
チェックリスト:
- セマンティックHTMLを使用しているか
- ARIA属性は適切か過剰でないか
- キーボードナビゲーション可能か
- スクリーンリーダーで意味が通じるか
- カラーコントラストは十分か
### TypeScript/型安全性
| 基準 | 判定 |
|------|------|
| `any` 型の使用 | REJECT |
| 型アサーションasの乱用 | 要検討 |
| Props型定義なし | REJECT |
| イベントハンドラの型が不適切 | 修正が必要 |
### フロントエンドセキュリティ
| 基準 | 判定 |
|------|------|
| dangerouslySetInnerHTML使用 | XSSリスクを確認 |
| ユーザー入力の未サニタイズ | REJECT |
| 機密情報のフロントエンド保存 | REJECT |
| CSRFトークンの未使用 | 要確認 |
### テスタビリティ
| 基準 | 判定 |
|------|------|
| data-testid等の未付与 | 警告 |
| テスト困難な構造 | 分離を検討 |
| ビジネスロジックのUIへの埋め込み | REJECT |
### アンチパターン検出
以下を見つけたら REJECT:
| アンチパターン | 問題 |
|---------------|------|
| God Component | 1コンポーネントに全機能が集中 |
| Prop Drilling | 深いPropsバケツリレー |
| Inline Styles乱用 | 保守性低下 |
| useEffect地獄 | 依存関係が複雑すぎる |
| Premature Optimization | 不要なメモ化 |
| Magic Strings | ハードコードされた文字列 |
| Hidden Dependencies | 子コンポーネントの隠れたAPI呼び出し |
| Over-generalization | 無理やり汎用化したコンポーネント |

View File

@ -24,168 +24,3 @@
- 「信頼しない、検証する」が基本原則 - 「信頼しない、検証する」が基本原則
- 1つの脆弱性がシステム全体を危険にさらす。見逃しは許されない - 1つの脆弱性がシステム全体を危険にさらす。見逃しは許されない
- AI生成コードには特有の脆弱性パターンがある。特に厳しく審査する - AI生成コードには特有の脆弱性パターンがある。特に厳しく審査する
## ドメイン知識
### AI生成コードのセキュリティ問題
AI生成コードには特有の脆弱性パターンがある。
| パターン | リスク | 例 |
|---------|--------|-----|
| もっともらしいが危険なデフォルト | 高 | `cors: { origin: '*' }` は問題なく見えるが危険 |
| 古いセキュリティプラクティス | 中 | 非推奨の暗号化、古い認証パターンの使用 |
| 不完全なバリデーション | 高 | 形式は検証するがビジネスルールを検証しない |
| 入力を過度に信頼 | 重大 | 内部APIは常に安全と仮定 |
| コピペによる脆弱性 | 高 | 同じ危険なパターンが複数ファイルで繰り返される |
特に厳しく審査が必要:
- 認証・認可ロジックAIはエッジケースを見落としがち
- 入力バリデーションAIは構文を検証しても意味を見落とす可能性
- エラーメッセージAIは内部詳細を露出する可能性
- 設定ファイルAIは学習データから危険なデフォルトを使う可能性
### インジェクション攻撃
**SQLインジェクション**
- 文字列連結によるSQL構築 → REJECT
- パラメータ化クエリの不使用 → REJECT
- ORMの raw query での未サニタイズ入力 → REJECT
```typescript
// NG
db.query(`SELECT * FROM users WHERE id = ${userId}`)
// OK
db.query('SELECT * FROM users WHERE id = ?', [userId])
```
**コマンドインジェクション**
- `exec()`, `spawn()` での未検証入力 → REJECT
- シェルコマンド構築時のエスケープ不足 → REJECT
```typescript
// NG
exec(`ls ${userInput}`)
// OK
execFile('ls', [sanitizedInput])
```
**XSS (Cross-Site Scripting)**
- HTML/JSへの未エスケープ出力 → REJECT
- `innerHTML`, `dangerouslySetInnerHTML` の不適切な使用 → REJECT
- URLパラメータの直接埋め込み → REJECT
### 認証・認可
**認証の問題**
- ハードコードされたクレデンシャル → 即REJECT
- 平文パスワードの保存 → 即REJECT
- 弱いハッシュアルゴリズム (MD5, SHA1) → REJECT
- セッショントークンの不適切な管理 → REJECT
**認可の問題**
- 権限チェックの欠如 → REJECT
- IDOR (Insecure Direct Object Reference) → REJECT
- 権限昇格の可能性 → REJECT
```typescript
// NG - 権限チェックなし
app.get('/user/:id', (req, res) => {
return db.getUser(req.params.id)
})
// OK
app.get('/user/:id', authorize('read:user'), (req, res) => {
if (req.user.id !== req.params.id && !req.user.isAdmin) {
return res.status(403).send('Forbidden')
}
return db.getUser(req.params.id)
})
```
### データ保護
**機密情報の露出**
- APIキー、シークレットのハードコーディング → 即REJECT
- ログへの機密情報出力 → REJECT
- エラーメッセージでの内部情報露出 → REJECT
- `.env` ファイルのコミット → REJECT
**データ検証**
- 入力値の未検証 → REJECT
- 型チェックの欠如 → REJECT
- サイズ制限の未設定 → REJECT
### 暗号化
- 弱い暗号アルゴリズムの使用 → REJECT
- 固定IV/Nonceの使用 → REJECT
- 暗号化キーのハードコーディング → 即REJECT
- HTTPSの未使用本番環境 → REJECT
### ファイル操作
**パストラバーサル**
- ユーザー入力を含むファイルパス → REJECT
- `../` のサニタイズ不足 → REJECT
```typescript
// NG
const filePath = path.join(baseDir, userInput)
fs.readFile(filePath)
// OK
const safePath = path.resolve(baseDir, userInput)
if (!safePath.startsWith(path.resolve(baseDir))) {
throw new Error('Invalid path')
}
```
**ファイルアップロード**
- ファイルタイプの未検証 → REJECT
- ファイルサイズ制限なし → REJECT
- 実行可能ファイルのアップロード許可 → REJECT
### 依存関係
- 既知の脆弱性を持つパッケージ → REJECT
- メンテナンスされていないパッケージ → 警告
- 不必要な依存関係 → 警告
### エラーハンドリング
- スタックトレースの本番露出 → REJECT
- 詳細なエラーメッセージの露出 → REJECT
- エラーの握りつぶし(セキュリティイベント) → REJECT
### レート制限・DoS対策
- レート制限の欠如(認証エンドポイント) → 警告
- リソース枯渇攻撃の可能性 → 警告
- 無限ループの可能性 → REJECT
### OWASP Top 10 チェックリスト
| カテゴリ | 確認事項 |
|---------|---------|
| A01 Broken Access Control | 認可チェック、CORS設定 |
| A02 Cryptographic Failures | 暗号化、機密データ保護 |
| A03 Injection | SQL, コマンド, XSS |
| A04 Insecure Design | セキュリティ設計パターン |
| A05 Security Misconfiguration | デフォルト設定、不要な機能 |
| A06 Vulnerable Components | 依存関係の脆弱性 |
| A07 Auth Failures | 認証メカニズム |
| A08 Software Integrity | コード署名、CI/CD |
| A09 Logging Failures | セキュリティログ |
| A10 SSRF | サーバーサイドリクエスト |

View File

@ -20,6 +20,9 @@ stances:
review: ../stances/review.md review: ../stances/review.md
testing: ../stances/testing.md testing: ../stances/testing.md
knowledge:
architecture: ../knowledge/architecture.md
personas: personas:
planner: ../personas/planner.md planner: ../personas/planner.md
architect-planner: ../personas/architect-planner.md architect-planner: ../personas/architect-planner.md
@ -224,6 +227,7 @@ movements:
edit: false edit: false
persona: architecture-reviewer persona: architecture-reviewer
stance: review stance: review
knowledge: architecture
report: report:
name: 05-architect-review.md name: 05-architect-review.md
format: architecture-review format: architecture-review

View File

@ -37,6 +37,11 @@ stances:
review: ../stances/review.md review: ../stances/review.md
testing: ../stances/testing.md testing: ../stances/testing.md
knowledge:
frontend: ../knowledge/frontend.md
cqrs-es: ../knowledge/cqrs-es.md
security: ../knowledge/security.md
personas: personas:
planner: ../personas/planner.md planner: ../personas/planner.md
coder: ../personas/coder.md coder: ../personas/coder.md
@ -205,6 +210,7 @@ movements:
edit: false edit: false
persona: cqrs-es-reviewer persona: cqrs-es-reviewer
stance: review stance: review
knowledge: cqrs-es
report: report:
name: 04-cqrs-es-review.md name: 04-cqrs-es-review.md
format: cqrs-es-review format: cqrs-es-review
@ -224,6 +230,7 @@ movements:
edit: false edit: false
persona: frontend-reviewer persona: frontend-reviewer
stance: review stance: review
knowledge: frontend
report: report:
name: 05-frontend-review.md name: 05-frontend-review.md
format: frontend-review format: frontend-review
@ -243,6 +250,7 @@ movements:
edit: false edit: false
persona: security-reviewer persona: security-reviewer
stance: review stance: review
knowledge: security
report: report:
name: 06-security-review.md name: 06-security-review.md
format: security-review format: security-review

View File

@ -28,6 +28,11 @@ stances:
review: ../stances/review.md review: ../stances/review.md
testing: ../stances/testing.md testing: ../stances/testing.md
knowledge:
frontend: ../knowledge/frontend.md
security: ../knowledge/security.md
architecture: ../knowledge/architecture.md
personas: personas:
planner: ../personas/planner.md planner: ../personas/planner.md
coder: ../personas/coder.md coder: ../personas/coder.md
@ -195,6 +200,7 @@ movements:
edit: false edit: false
persona: architecture-reviewer persona: architecture-reviewer
stance: review stance: review
knowledge: architecture
report: report:
name: 04-architect-review.md name: 04-architect-review.md
format: architecture-review format: architecture-review
@ -214,6 +220,7 @@ movements:
edit: false edit: false
persona: frontend-reviewer persona: frontend-reviewer
stance: review stance: review
knowledge: frontend
report: report:
name: 05-frontend-review.md name: 05-frontend-review.md
format: frontend-review format: frontend-review
@ -233,6 +240,7 @@ movements:
edit: false edit: false
persona: security-reviewer persona: security-reviewer
stance: review stance: review
knowledge: security
report: report:
name: 06-security-review.md name: 06-security-review.md
format: security-review format: security-review