Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 202 additions & 0 deletions .claude/skills/design-rules/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
---
name: design-rules
description: Manage and apply the file-based design rule system in this repo (design-rules/ and per-component DESIGN_RULES.md). Use when the user says "show design rules", "design rules for [component/path/file/PR]", "which rules apply to X", "add design rule", "add ADR", "update design rule", "review [code/PR] against design rules", "list ADRs", "list principles", or "validate design rules". Operations: list, get, add, update, review, validate.
compatibility: Designed for Claude Code. Requires git, gh, find, grep, ripgrep.
allowed-tools: Read Glob Grep Edit Write Bash(git:*) Bash(gh:*) Bash(find:*) Bash(grep:*) Bash(ls:*)
---

# Design Rules Skill

A skill for managing and applying the version-controlled design rule system in this repo. The system has two layers: global rules in `design-rules/` and per-component `DESIGN_RULES.md` files placed next to the code they govern, under `backend/`, `unstract/`, and `workers/` (where present). Coverage is rolled out incrementally — not every component has a file yet. The system is OSS-only — it must never reference cloud / enterprise / pluggable_apps / HITL / subscription / agentic / or any specific PR review tool.

---

## Quick Start

### Basic Commands

| What you want | What to say |
|---------------|-------------|
| List principles | "list principles", "list design principles" |
| List ADRs | "list ADRs", "show all ADRs" |
| List components with rules | "list components with rules", "which components have design rules" |
| Get rules for a file/dir/component | "design rules for backend/connector_v2", "which rules apply to api_v2/views.py" |
| Get rules for a PR or local diff | "design rules for PR #1902", "design rules for my changes" |
| Get all global rules | "show all design rules" |
| Add a new ADR | "add ADR for X" |
| Add a per-component rules file | "add DESIGN_RULES.md for backend/foo" |
| Update an existing rule | "update design rule for X", "rule about Y changed" |
| Review code against the rules | "review my changes against design rules", "check PR #1902 against design rules" |
| Validate the rule system itself | "validate design rules" |

### What you'll get

| Mode | Output |
|------|--------|
| **list** | Compact text list (titles + numbers, no bodies) |
| **get** | Deduped bundle: compatibility statement, relevant principles, AI Review Checklist, referenced ADRs, security standards, and the component's `## Rules` entries (`R1..Rn` with Severity / Why / Refs / Enforced by) |
| **add** | New file written + cross-links updated; refused if content describes an aspiration rather than implemented behaviour |
| **update** | Minimal in-place edit; ADR superseded with a new ADR if the change is a reversal |
| **review** | Per-file findings table: file, line, principle/ADR violated, why, minimal fix |
| **validate** | Pass/fail report from forbidden-word scan, compatibility-statement presence check, and component-directory sanity check |

---

## Related Skills

| Skill | Relationship |
|-------|--------------|
| **code-reviewer** | code-reviewer can call `design-rules review` to add design-rule checks on top of its SOLID/security review |
| **pr-creator** | pr-creator can call `design-rules get` to surface applicable rules in the PR description |
| **jira-manager** | jira-manager can call `design-rules get` when planning implementation of a ticket |
| **architecture-analyst** | architecture-analyst consults `design-rules/principles.md` and ADRs when documenting how a component works |

---

## Command: `list`

Show what rules exist, titles only — fast and cheap.

| Sub-command | Action |
|---|---|
| "list principles" | Read `design-rules/principles.md`, return P1–P9 titles only. |
| "list ADRs" | Read `design-rules/adr/README.md`. If missing, `ls design-rules/adr/ADR-*.md` and return numbers + titles. |
| "list components with rules" | `find backend unstract workers -name DESIGN_RULES.md`. Return grouped by directory. |
| "list global rules" | `ls design-rules/ design-rules/security/ design-rules/adr/`. |

Never dump full file bodies in `list` mode — that's `get`.

---

## Command: `get`

Fetch the rules that actually apply to a target.

**Steps:**
1. **Resolve the target paths.** Accept any of: a single file path, a directory, a component name (e.g. `connector_v2`), a glob, or a list of files (from `gh pr diff <n> --name-only` or `git diff --name-only`).
2. **For each target file**, walk from the file's directory upward to the repo root and collect every `DESIGN_RULES.md` along the way. Always include `design-rules/README.md`, `design-rules/principles.md`, and `design-rules/ai-review-checklist.md`.
3. **Read the collected files** and present them in this order:
- Compatibility statement (once)
- Principles referenced by any collected component file's "Read first"
- AI Review Checklist (always)
- ADRs referenced by any collected component file
- Security standards referenced (e.g. `security/tenant-isolation.md`, `security/standards.md`, the inline SQL Safety Standard under `unstract/connectors/.../databases/`)
- The component-specific `## Rules` entries (`R1..Rn` with Severity / Why / Refs / Enforced by) from each collected `DESIGN_RULES.md`, plus any `## Known Exceptions` entries when present
4. **Deduplicate.** Never show the same principle or ADR twice.
5. If asked for "all" design rules, dump the global files (`design-rules/**`) without per-component noise.

---

## Command: `add`

Create a new rule, ADR, or per-component file.

**Gate before adding anything:** verify the content describes current implemented behaviour, not aspirations. The system explicitly excludes unimplemented designs (no AuditLog table, no SoftDeleteMixin, no proposed key/role architecture, etc.). If the user wants to record an aspiration, push back and suggest the issue tracker or Confluence instead.

### Add a global principle or refinement
- New principle → edit `design-rules/principles.md`, take the next free `P<N>` number, **and** update `design-rules/ai-review-checklist.md` in the same change to add a question for it.
- Refinement of an existing principle → edit it in place; do not duplicate.

### Add an ADR
1. `ls design-rules/adr/ADR-*.md` to find the next free number (the highest existing number plus one — never fill a gap).
2. Create `design-rules/adr/ADR-NNN-<kebab-title>.md`:
```markdown
# ADR-NNN: <title>

## Context
<why this decision is needed; what's currently true>

## Decision
<what was decided>

## Consequences
<what becomes easier / harder / required>
```
There is no `Status` field — the merge IS the acceptance. See `design-rules/adr/README.md` for the full ADR format and the delete-on-supersession flow.
3. Add a one-line entry to the Index table in `design-rules/adr/README.md`.
4. Cross-link from the relevant per-component `DESIGN_RULES.md` files in their `Read first` section.

### Add a per-component `DESIGN_RULES.md`
1. Verify the directory exists on disk and contains real source (not just `__pycache__`).
2. Use the standard template from `design-rules/README.md`. Always include the verbatim compatibility statement.
3. Include "Read first" links to `principles.md`, `ai-review-checklist.md`, and any relevant ADRs / security standards.
4. If the component has no specific rules yet, use the canonical placeholder under `## Rules`: `No component-specific rules yet.` Do not invent Dos/Don'ts — the canonical format is `R1..Rn` entries with the full four-row table.

---

## Command: `update`

Apply new changes to existing rules. Triggered by "update design rule for X", "the rule about Y changed", or when implementation drift is detected during review.

1. Identify the affected files: principle file, ADR(s), per-component file(s).
2. Make minimal edits in place. Never duplicate content between global and component files.
3. If a previously-recorded rule is now wrong because the implementation changed, **update the rule, do not add a contradicting one**. If the change is significant enough to be a decision reversal, add a new ADR that overturns the prior decision and **delete** the old ADR in the same PR — update every reference to the old ADR ID repo-wide (M2). See `design-rules/adr/README.md` for the full supersession flow.
4. If a per-component file is updated, also check whether the global ADR / principle / security standard it references still matches. Fix upstream first if both are wrong.

---

## Command: `review`

Check code or a diff against the design rules. Triggered by "review against design rules", "check this PR against design rules", "does X violate any rule".

1. **Determine the target.** Accept: a list of changed files, a PR number (use `gh pr diff <n> --name-only`), `git diff --name-only` for local changes, or a single file.
2. **Run `get`** on those targets to collect all applicable rules.
3. **For each changed file**, walk through the AI Review Checklist questions (`design-rules/ai-review-checklist.md`) and the component's `## Rules` entries (`R1..Rn`). For every violation, output:
- File and line (when known)
- Principle / ADR / security standard violated
- Why it's a violation
- Minimal fix
4. **Pay special attention to known traps:**

| Trap | What to look for | Reference |
|---|---|---|
| Tenant isolation bypass | New model or queryset bypassing org scoping | `security/tenant-isolation.md` (Three-Layer Defense) |
| SQL injection | String interpolation into SQL inside `unstract/connectors/.../databases/**` | inline SQL Safety Standard S1 in that directory's `DESIGN_RULES.md` |
| Missing auth | New endpoint without authentication | P5 fail-closed |
| Direct org FK on derived model | New model that doesn't inherit org through a parent | P6, ADR-002 |
| Audit/billing CASCADE | New audit/billing record using FK CASCADE to parent | P4, ADR-003 |
| Credential duplication | Storing the same credential twice instead of referencing | P2 |

5. **Do not invent rules.** If a pattern looks suspicious but no rule covers it, say so explicitly and recommend opening a tracker entry instead of pretending a rule exists.

---

## Command: `validate`

Sanity-check the rule system itself. Triggered by "validate design rules", "check design rules consistency".

Run the bundled script from the repo root:

```bash
.claude/skills/design-rules/scripts/validate.sh
```

The script checks: forbidden-word scan, compatibility-statement presence in every per-component file, and that each per-component file's directory contains real source. Exit code 0 = clean; 1 = problems found. Report any hits as fix-required and offer to fix them in place.

---

## What this skill must NOT do

- Never add debt items, vulnerability reports, or open design questions to the repo. Those belong in the issue tracker and the private security channel.
- Never document a proposed / unimplemented design as if it were a rule.
- Never name a specific PR review tool inside any rule file.
- Never duplicate global rule content into per-component files.
- Never create a `DESIGN_RULES.md` for `pluggable_apps/*`, `workers/pluggable_worker`, or `frontend/`.

---

## Path reference

| Path | Purpose |
|---|---|
| `design-rules/README.md` | Entry point: navigation, how-to recipes, and the AI-loading instruction (load `DESIGN_RULES.md` from the dir of any changed file). |
| `design-rules/principles.md` | P1–P9. |
| `design-rules/ai-review-checklist.md` | 9 questions every change must answer "yes" to. |
| `design-rules/lifecycle.md` | Design → assembly → deploy → runtime → monitoring. |
| `design-rules/security/tenant-isolation.md` | Three-Layer Defense. |
| `design-rules/security/standards.md` | SQL Safety S1 + current protection patterns as rules. |
| `design-rules/adr/ADR-*.md` | 8 accepted ADRs. |
| `unstract/connectors/src/unstract/connectors/databases/DESIGN_RULES.md` | SQL Safety Standard S1 inlined for the 8 DB connectors. |
| `backend/**/DESIGN_RULES.md` | Per-Django-app component files (where present). |
| `unstract/**/DESIGN_RULES.md` | Per-shared-lib component files (where present). |
| `workers/**/DESIGN_RULES.md` | Per-worker component files (where present). |
85 changes: 85 additions & 0 deletions .claude/skills/design-rules/scripts/validate.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
#!/usr/bin/env bash
# Validate the design rules system. Run from the repo root.
#
# Checks:
# 1. No forbidden words appear in any design rule file (cloud, enterprise,
# HITL, agentic, vendor names, unimplemented design names, etc.).
# 2. Every per-component DESIGN_RULES.md contains the verbatim compatibility
# statement.
# 3. Every per-component DESIGN_RULES.md lives in a directory that exists
# and contains real source (not just __pycache__).
#
# Exit codes: 0 = clean, 1 = problems found.
#
# Portability: bash 3.2+ (macOS default). Avoids `mapfile`/`readarray` and
# avoids expanding potentially-empty arrays under `set -u`.

set -u
fail=0

ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"
cd "$ROOT" || exit 2

# Populate COMPONENT_FILES portably (no mapfile — bash 3.2 compatible).
COMPONENT_FILES=()
while IFS= read -r -d '' f; do
COMPONENT_FILES+=("$f")
done < <(find backend unstract workers -name DESIGN_RULES.md -print0 2>/dev/null)

echo "==> 1. Forbidden-word scan"
FORBIDDEN='cloud|enterprise|unstract-cloud|hitl|manual.review|subscription|agentic|simple.prompt.studio|public.shares|greptile|AuditLog|SoftDeleteMixin|departure workflow|GDPR|OWNER role|ManagementKey|ExecutionKey|uxm_|uxe_|deletion guard|informed deletion'

check1_fail=0
if [[ ${#COMPONENT_FILES[@]} -gt 0 ]]; then
if grep -rEl -i "$FORBIDDEN" \
design-rules/ "${COMPONENT_FILES[@]}" 2>/dev/null; then
echo " FAIL: forbidden words found in the files listed above"
check1_fail=1
fail=1
fi
else
if grep -rEl -i "$FORBIDDEN" design-rules/ 2>/dev/null; then
echo " FAIL: forbidden words found in the files listed above"
check1_fail=1
fail=1
fi
fi
[[ $check1_fail -eq 0 ]] && echo " OK"

echo
echo "==> 2. Compatibility statement presence"
COMPAT='All changes to this component must remain compatible'
check2_fail=0
for f in "${COMPONENT_FILES[@]+"${COMPONENT_FILES[@]}"}"; do
if ! grep -q "$COMPAT" "$f"; then
echo " MISSING: $f"
check2_fail=1
fail=1
fi
done
[[ $check2_fail -eq 0 ]] && echo " OK"

echo
echo "==> 3. Component directory sanity"
check3_fail=0
for f in "${COMPONENT_FILES[@]+"${COMPONENT_FILES[@]}"}"; do
dir="$(dirname "$f")"
# Real source = at least one file other than DESIGN_RULES.md and __pycache__
real=$(find "$dir" -maxdepth 1 -mindepth 1 \
! -name DESIGN_RULES.md ! -name __pycache__ 2>/dev/null | head -1)
if [[ -z "$real" ]]; then
echo " EMPTY DIR: $f (no real source alongside)"
check3_fail=1
fail=1
fi
done
[[ $check3_fail -eq 0 ]] && echo " OK"

echo
if [[ $fail -eq 0 ]]; then
echo "All design rule checks passed."
exit 0
else
echo "Design rule checks FAILED. Fix the items listed above."
exit 1
fi
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,8 @@ CLAUDE.md
!.claude/skills/
.claude/skills/*
!.claude/skills/worktree/
!.claude/skills/design-rules/
!.claude/skills/design-rules/**
CONTRIBUTION_GUIDE.md
.mcp.json

Expand Down
71 changes: 71 additions & 0 deletions backend/account_v2/DESIGN_RULES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# account_v2 — Design Rules

`account_v2` owns the tenant-graph root — `Organization`, `User`, and `OrganizationMember`. Every other tenant-scoped model in this repo ultimately resolves to an `Organization` defined here.

> **Compatibility:** All changes to this component must remain compatible with extension by an external layer that runs this codebase alongside additional Django applications sharing the same database.

This file follows the [per-component contract](../../design-rules/per-component-contract.md) (rule format, severity vocabulary, Known Exceptions, M1/M2/M3 meta-rules).

---

## Scope

| | |
|---|---|
| **Covers** | `backend/account_v2/**` — `Organization`, `User`, `OrganizationMember`, their managers, serializers, views, signals, and the auth middleware glue |
| **Excludes** | Usage rollups → [`account_usage`](../account_usage/DESIGN_RULES.md). Tenant-scoped data in other apps → each app's own `DESIGN_RULES.md`. |

## Read first

| File | Why it binds here |
|---|---|
| [`principles.md`](../../design-rules/principles.md) | P1 (org scoping), P2 (credentials), P8 (fail-closed) |
| [`ai-review-checklist.md`](../../design-rules/ai-review-checklist.md) | 9 questions every change must answer |
| [`security/tenant-isolation.md`](../../design-rules/security/tenant-isolation.md) | Three-Layer Defense — `account_v2` owns Layer 1 (middleware) and is the root of Layer 3 (filter backend) |
| [`adr/ADR-001`](../../design-rules/adr/ADR-001-org-scoping-query-layer.md) | Org scoping is enforced at the query layer, not via RLS |

---

## Rules

### R1 — `Organization` is the single root of the tenant graph

| | |
|---|---|
| **Severity** | MUST |
| **Why** | Every tenant-scoped model must resolve to exactly one `Organization`, by direct FK or documented FK chain. A second root would split tenant isolation and break `OrganizationFilterBackend`'s BFS walk. |
| **Refs** | `principles.md#P1` · `security/tenant-isolation.md` · `adr/ADR-001` |
| **Enforced by** | `OrganizationFilterBackend` (Three-Layer Defense, Layer 3) + code review |

### R2 — Active-org resolution always goes through `CustomAuthMiddleware`

| | |
|---|---|
| **Severity** | MUST |
| **Why** | The middleware is Layer 1 of the Three-Layer Defense — it validates the user belongs to the org and stores `org_id` in the thread-local `StateStore`. Reading the org from a header, request param, or cookie directly bypasses validation and is a tenant boundary violation. |
| **Refs** | `principles.md#P8` · `security/tenant-isolation.md` |
| **Enforced by** | `CustomAuthMiddleware` + code review |

### R3 — Org membership has exactly one source of truth: `OrganizationMember`

| | |
|---|---|
| **Severity** | MUST |
| **Why** | A second membership table would let an org/user pair exist in one place and not the other, producing inconsistent authorization decisions and orphaned credentials. Membership is the anchor used by every other app to check whether a user can act inside an org. |
| **Refs** | `principles.md#P1` · `principles.md#P2` |
| **Enforced by** | code review only |

### R4 — Credentials on `User` follow P2 (stored once, referenced by ID)

| | |
|---|---|
| **Severity** | MUST |
| **Why** | Duplicating a credential makes rotation impossible and leaves stale copies behind on member departure. `User`-owned credentials must be referenced by ID from anywhere that consumes them, never copied. |
| **Refs** | `principles.md#P2` |
| **Enforced by** | not yet enforced — see project issue tracker (credential lifecycle) |

---

## Checklist

See [Definition of Done](../../design-rules/definition-of-done.md).
Loading