docs: enhance AGENTS.md files with Go linting, DI patterns, barrel re-export, TS best practices [guardrail-change]
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
32
AGENTS.go.md
32
AGENTS.go.md
@@ -105,11 +105,38 @@ func TestIACEService_Create(t *testing.T) {
|
||||
|
||||
## Tooling
|
||||
|
||||
- `golangci-lint` with: `errcheck, govet, staticcheck, revive, gosec, gocyclo (max 15), gocognit (max 20), unused, ineffassign, errorlint, nilerr, nolintlint, contextcheck`.
|
||||
Run lint before pushing:
|
||||
|
||||
```bash
|
||||
golangci-lint run --timeout 5m ./...
|
||||
```
|
||||
|
||||
The `.golangci.yml` at the service root (`ai-compliance-sdk/.golangci.yml`) enables: `errcheck, govet, staticcheck, gosec, gocyclo (≤20), gocritic, revive, goimports, unused, ineffassign`. Fix lint violations in new code; legacy violations are tracked but not required to fix immediately.
|
||||
|
||||
- `gofumpt` formatting.
|
||||
- `go vet ./...` clean.
|
||||
- `go mod tidy` clean — no unused deps.
|
||||
|
||||
## File splitting pattern
|
||||
|
||||
When a Go file exceeds the 500-line hard cap, split it in place — no new packages needed:
|
||||
|
||||
- All split files stay in **the same package directory** with the **same `package <name>` declaration**.
|
||||
- No import changes are needed anywhere because Go packages are directory-scoped.
|
||||
- Naming: `store_projects.go`, `store_components.go` (noun + underscore + sub-resource).
|
||||
- For handlers: `iace_handler_projects.go`, `iace_handler_hazards.go`, etc.
|
||||
- Before splitting, add a characterization test that pins current behaviour.
|
||||
|
||||
## Error handling
|
||||
|
||||
Domain errors are defined in `internal/domain/<aggregate>/errors.go` as sentinel vars or typed errors. The mapping from domain error to HTTP status lives exclusively in `internal/platform/httperr/httperr.go` via `errors.Is` / `errors.As`. Handlers call `httperr.Write(c, err)` — **never** directly call `c.JSON` with a status code derived from business logic.
|
||||
|
||||
## Context propagation
|
||||
|
||||
- Always pass `ctx context.Context` as the **first parameter** in every service and repository method.
|
||||
- Never store a context in a struct field — pass it per call.
|
||||
- Cancellation must be respected: check `ctx.Err()` in loops; propagate to all I/O calls.
|
||||
|
||||
## Concurrency
|
||||
|
||||
- Goroutines must have a clear lifecycle owner (struct method that started them must stop them).
|
||||
@@ -122,5 +149,8 @@ func TestIACEService_Create(t *testing.T) {
|
||||
- Add a new top-level package directly under `internal/` without architectural review.
|
||||
- `import "C"`, unsafe, reflection-heavy code.
|
||||
- Use `init()` for non-trivial setup. Wire it in `internal/app`.
|
||||
- Use `interface{}` / `any` in new code without an explicit comment justifying it.
|
||||
- Call `log.Fatal` outside of `main.go`; panicking in request handling is also forbidden.
|
||||
- Shadow `err` with `:=` inside an `if`-block when the outer scope already declares `err` — use `=` or rename.
|
||||
- Create a file >500 lines.
|
||||
- Change a public route's contract without updating consumers.
|
||||
|
||||
@@ -78,6 +78,57 @@ async def create_dsr_request(
|
||||
- `pip-audit` in CI.
|
||||
- Async-first: prefer `httpx.AsyncClient`, `asyncpg`/`SQLAlchemy 2.x async`.
|
||||
|
||||
## mypy configuration
|
||||
|
||||
`backend-compliance/mypy.ini` is the mypy config. Strict mode is on globally; per-module overrides exist only for legacy files that have not been cleaned up yet.
|
||||
|
||||
- New modules added to `compliance/services/` or `compliance/repositories/` **must** pass `mypy --strict`.
|
||||
- To type-check a new module: `cd backend-compliance && mypy compliance/your_new_module.py`
|
||||
- When you fully type a legacy file, **remove its loose-override block** from `mypy.ini` as part of the same PR.
|
||||
|
||||
## Dependency injection
|
||||
|
||||
Services and repositories are wired via FastAPI `Depends`. Never instantiate a service or repository directly inside a handler.
|
||||
|
||||
```python
|
||||
# dependencies.py
|
||||
def get_my_service(db: AsyncSession = Depends(get_db)) -> MyService:
|
||||
return MyService(MyRepository(db))
|
||||
|
||||
# router
|
||||
@router.get("/items", response_model=list[ItemRead])
|
||||
async def list_items(svc: MyService = Depends(get_my_service)) -> list[ItemRead]:
|
||||
return await svc.list()
|
||||
```
|
||||
|
||||
- Services take repositories in `__init__`; repositories take `Session` or `AsyncSession`.
|
||||
|
||||
## Structured logging
|
||||
|
||||
```python
|
||||
import structlog
|
||||
logger = structlog.get_logger()
|
||||
|
||||
# Always bind context before logging:
|
||||
logger.bind(tenant_id=str(tid), action="create_dsfa").info("dsfa created")
|
||||
```
|
||||
|
||||
- Audit-relevant actions must use the audit logger with a `legal_basis` field.
|
||||
- Never log secrets, PII, or full request bodies.
|
||||
|
||||
## Barrel re-export pattern
|
||||
|
||||
When an oversized file (e.g. `schemas.py`, `models.py`) is split into a sub-package, the original stays as a **thin re-exporter** so existing consumer imports keep working:
|
||||
|
||||
```python
|
||||
# compliance/schemas.py (barrel — DO NOT ADD NEW CODE HERE)
|
||||
from .schemas.ai import * # noqa: F401, F403
|
||||
from .schemas.consent import * # noqa: F401, F403
|
||||
```
|
||||
|
||||
- New code imports from the specific module (e.g. `from compliance.schemas.ai import AIRiskRead`), not the barrel.
|
||||
- `from module import *` is only permitted in barrel files.
|
||||
|
||||
## Errors & logging
|
||||
|
||||
- Domain errors inherit from a single `DomainError` base per service.
|
||||
@@ -91,4 +142,7 @@ async def create_dsr_request(
|
||||
- Change a public route's path/method/status/schema without simultaneous dashboard fix.
|
||||
- Catch `Exception` broadly — catch the specific domain or library error.
|
||||
- Put business logic in a router or in a Pydantic validator.
|
||||
- `from module import *` in new code — only in barrel re-exporters.
|
||||
- `raise HTTPException` inside the service layer — raise domain exceptions; map them in the router.
|
||||
- Use `model_validate` on untrusted external data without an explicit schema boundary.
|
||||
- Create a new file >500 lines. Period.
|
||||
|
||||
@@ -27,15 +27,20 @@ components/ # Truly shared, app-wide components.
|
||||
## API routes (route.ts)
|
||||
|
||||
- One handler per HTTP method, ≤40 LOC.
|
||||
- Validate input with `zod`. Reject invalid → 400.
|
||||
- Validate input with zod `safeParse` — never `parse` (throws and bypasses error handling).
|
||||
- Delegate to `lib/server/<domain>/`. No business logic in `route.ts`.
|
||||
- Always return `NextResponse.json(..., { status })`. Never throw to the framework.
|
||||
- Always return `NextResponse.json(..., { status })`. Let the framework's error boundary handle unexpected errors — don't wrap the entire handler in `try/catch`.
|
||||
|
||||
```ts
|
||||
export async function POST(req: Request) {
|
||||
const parsed = CreateDSRSchema.safeParse(await req.json());
|
||||
if (!parsed.success) return NextResponse.json({ error: parsed.error.flatten() }, { status: 400 });
|
||||
const result = await dsrService.create(parsed.data);
|
||||
```typescript
|
||||
// app/api/<domain>/route.ts (≤40 LOC)
|
||||
import { NextRequest, NextResponse } from 'next/server';
|
||||
import { mySchema } from '@/lib/schemas/<domain>';
|
||||
import { myService } from '@/lib/server/<domain>';
|
||||
|
||||
export async function POST(req: NextRequest) {
|
||||
const body = mySchema.safeParse(await req.json());
|
||||
if (!body.success) return NextResponse.json({ error: body.error }, { status: 400 });
|
||||
const result = await myService.create(body.data);
|
||||
return NextResponse.json(result, { status: 201 });
|
||||
}
|
||||
```
|
||||
@@ -52,6 +57,39 @@ export async function POST(req: Request) {
|
||||
- `lib/sdk/types.ts` is being split into `lib/sdk/types/<domain>.ts`. Mirror backend domain boundaries.
|
||||
- All API DTOs are zod schemas; infer types via `z.infer`.
|
||||
- No `any`. No `as unknown as`. If you reach for it, the type is wrong.
|
||||
- Always use `import type { Foo }` for type-only imports.
|
||||
- Never use `as` type assertions except when bridging external data at a boundary (add a comment explaining why).
|
||||
- No `@ts-ignore`. `@ts-expect-error` only with a comment explaining the suppression.
|
||||
|
||||
## Barrel re-export pattern
|
||||
|
||||
`lib/sdk/types.ts` is a barrel — it re-exports from domain-specific files. **Do not add new types directly to it.**
|
||||
|
||||
```typescript
|
||||
// lib/sdk/types.ts (barrel — DO NOT ADD NEW TYPES HERE)
|
||||
export * from './types/enums';
|
||||
export * from './types/company-profile';
|
||||
// ... etc.
|
||||
|
||||
// New types go in lib/sdk/types/<domain>.ts
|
||||
```
|
||||
|
||||
- When splitting an oversized file, keep the original as a thin barrel so existing imports don't break.
|
||||
- New code imports directly from the specific module (e.g. `import type { CompanyProfile } from '@/lib/sdk/types/company-profile'`), not the barrel.
|
||||
|
||||
## Server vs Client components
|
||||
|
||||
Default is Server Component. Add `"use client"` only when required:
|
||||
|
||||
| Need | Pattern |
|
||||
|------|---------|
|
||||
| Data fetching only | Server Component (no directive) |
|
||||
| `useState` / `useEffect` | Client Component (`"use client"`) |
|
||||
| Browser API | Client Component |
|
||||
| Event handlers | Client Component |
|
||||
|
||||
- Pass only serializable props from Server → Client Components (no functions, no class instances).
|
||||
- Never add `"use client"` to a layout or page just because one child needs it — extract the client part into a `_components/` file.
|
||||
|
||||
## Tests
|
||||
|
||||
@@ -78,8 +116,10 @@ export async function POST(req: Request) {
|
||||
|
||||
- Put business logic in a `page.tsx` or `route.ts`.
|
||||
- Reach across module boundaries (e.g. `admin-compliance` importing from `developer-portal`).
|
||||
- Use `dangerouslySetInnerHTML` without explicit sanitization.
|
||||
- Call backend APIs directly from Client Components when a Server Component or Server Action would do.
|
||||
- Use `dangerouslySetInnerHTML` without DOMPurify sanitization.
|
||||
- Call internal backend APIs directly from Client Components — use Server Components or API routes as a proxy.
|
||||
- Add `"use client"` to a layout or page just because one child needs it — extract the client part.
|
||||
- Spread `...props` onto a DOM element without filtering the props first (type error risk).
|
||||
- Change a public API route's path/method/schema without updating SDK consumers in the same change.
|
||||
- Create a file >500 lines.
|
||||
- Disable a lint or type rule globally to silence a finding — fix the root cause.
|
||||
|
||||
Reference in New Issue
Block a user