diff --git a/.markdownlint-cli2.yaml b/.markdownlint-cli2.yaml index 4a34fdd..32fef20 100644 --- a/.markdownlint-cli2.yaml +++ b/.markdownlint-cli2.yaml @@ -1,7 +1,16 @@ +ignores: + - ".claude/**" + - "node_modules/**" + config: - MD013: false # Line length — many existing docs exceed 200 chars; fix incrementally - MD024: false # Duplicate headings — AGENTS.md has intentional repeated section headers - MD032: false # Blanks around lists — many existing docs use compact list formatting - MD033: false # Inline HTML — used in standards docs for tables/details - MD034: false # Bare URLs — AGENTS.md uses bare URLs for references - MD041: true # First line must be a top-level heading + MD013: + line_length: 200 + tables: false # Tables often have long rows — exclude + code_blocks: false # Code blocks need exact formatting — exclude + MD024: + siblings_only: true # Allow duplicate headings in different sections + MD032: true # Blanks around lists + MD033: false # Inline HTML — used in standards docs + MD034: true # No bare URLs + MD041: true # First line must be a top-level heading + MD060: false # Table column style — too strict for compact tables diff --git a/AGENTS.md b/AGENTS.md index 8285c23..2f4760b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,7 @@ # AGENTS.md — Petry Projects Organization Standards -This file defines cross-cutting development standards for all repositories in the **petry-projects** organization. It follows the [AGENTS.md convention](https://agents.md/) and is intended to be imported by each repository's own AGENTS.md or CLAUDE.md. +This file defines cross-cutting development standards for all repositories in the **petry-projects** organization. +It follows the [AGENTS.md convention](https://agents.md/) and is intended to be imported by each repository's own AGENTS.md or CLAUDE.md. > Individual repositories extend these standards with project-specific guidance. If a repo-level rule conflicts with this file, the repo-level rule takes precedence. @@ -8,7 +9,8 @@ This file defines cross-cutting development standards for all repositories in th ## Project Context -- **Assume brownfield.** When exploring a repo, check for existing source code before assuming it is greenfield. Look at all directories, worktrees, and non-main branches before concluding that code does not exist. +- **Assume brownfield.** When exploring a repo, check for existing source code before assuming it is greenfield. +Look at all directories, worktrees, and non-main branches before concluding that code does not exist. --- @@ -25,7 +27,8 @@ This file defines cross-cutting development standards for all repositories in th ### Branch Switching -- **Before switching branches, always commit or stash current work.** Git usually prevents checkouts that would overwrite local changes, but forced operations (e.g., `git checkout -f` or `git reset --hard`) or resolving conflicts incorrectly can cause you to lose or misplace work. +- **Before switching branches, always commit or stash current work.** Git usually prevents checkouts that would overwrite local changes, +but forced operations (e.g., `git checkout -f` or `git reset --hard`) or resolving conflicts incorrectly can cause you to lose or misplace work. - After switching, verify you are on the correct branch with `git branch --show-current` before making any changes. --- @@ -48,7 +51,8 @@ If a dependency cannot be resolved, report the specific blocker and a workaround - **TDD is mandatory.** Write tests before implementing features or bug fixes. Include tests in the same PR as the implementation. - **Achieve and maintain excellent test coverage.** Verify locally before pushing. PRs that reduce coverage below repo-defined thresholds will be rejected. - **NEVER use `.skip()` to avoid failing tests.** If tests fail, fix them. If functionality cannot be directly tested, extract testable logic and test the extraction. -- **NEVER add coverage-ignore comments** (e.g., `/* istanbul ignore next */`, `/* c8 ignore next */`, `// v8 ignore next`) to artificially boost coverage. If code is difficult to test, improve mocking strategies or adjust thresholds instead. +- **NEVER add coverage-ignore comments** (e.g., `/* istanbul ignore next */`, `/* c8 ignore next */`, `// v8 ignore next`) to artificially boost coverage. + If code is difficult to test, improve mocking strategies or adjust thresholds instead. - Unit tests MUST be fast, deterministic, and not access external networks. - Integration tests are allowed but MUST be clearly marked. They may be skipped locally during rapid iteration, but CI MUST always run them (for example, in a separate job or scheduled workflow). - Mock external services using project-provided helpers where available. @@ -57,16 +61,20 @@ If a dependency cannot be resolved, report the specific blocker and a workaround ## End-to-End Testing — Validate Real Functional Requirements -E2E tests validate real functional requirements through the full stack. They exist to catch bugs that would affect real users. **A test that does not verify a real business outcome is a test that provides false confidence and must not exist.** +E2E tests validate real functional requirements through the full stack. They exist to catch bugs that would affect real users. +**A test that does not verify a real business outcome is a test that provides false confidence and must not exist.** -> Every E2E test must answer one question: **"What functional requirement would be broken for a real user if this test didn't exist?"** If the test could pass while the requirement is fundamentally unmet, it is worthless and must be rewritten. +> Every E2E test must answer one question: **"What functional requirement would be broken for a real user if this test didn't exist?"** +> If the test could pass while the requirement is fundamentally unmet, it is worthless and must be rewritten. ### What E2E Tests MUST Do 1. **Full round-trip verification.** Action → API call → database mutation → response → frontend reflects new state. Not a subset — the whole chain. 2. **Multi-layer assertions.** The frontend shows correct data AND the database contains the correct record AND side effects occurred (events published, notifications queued, cache invalidated). -3. **Verify at the data layer.** After a form submission, query the database directly to verify the record exists with correct fields. After a delete, verify it's gone. After auth, verify the token's claims and scopes. Do NOT stop at "success toast appeared." -4. **Test error paths.** For every happy-path test, write corresponding tests for: invalid input, unauthorized access, conflict/duplicate states, and not-found resources. +3. **Verify at the data layer.** After a form submission, query the database directly to verify the record exists with correct fields. + After a delete, verify it's gone. After auth, verify the token's claims and scopes. Do NOT stop at "success toast appeared." +4. **Test error paths.** For every happy-path test, write corresponding tests for: + invalid input, unauthorized access, conflict/duplicate states, and not-found resources. 5. **Test authorization boundaries.** Verify user A cannot access user B's resources. Verify regular users cannot hit admin endpoints. Verify expired/revoked tokens are rejected. 6. **Use realistic data.** Factories that produce production-realistic data (unicode names, long strings, special characters, realistic cardinalities) — not `"test"` and `"foo"`. 7. **Deterministic waits.** Wait for specific conditions (element visible, API response received, database row present) using polling with timeouts — never arbitrary sleeps. @@ -101,11 +109,13 @@ Every E2E test follows this structure: ### Test Design Patterns **Page/Screen Object Model (for frontend E2E):** + - Encapsulate page interactions in page/screen objects. Tests read as functional requirements, not DOM/view manipulation. - Page objects expose user-intent methods (`loginAs(user)`, `submitOrder(items)`) — not element-level methods. - Selectors live in exactly one place (the page object). Use stable test-ID attributes exclusively. **Test Data Factories:** + - Every test creates its own data. Never rely on pre-existing seed data. - Factories produce realistic, randomized data: `createUser({role: "admin"})`, `createOrder({status: "pending", items: 3})`. - Factories use the API or database — NOT the frontend. @@ -194,7 +204,8 @@ Pre-commit hooks may not run in agent sessions — apply formatting and checks m ## Coding Standards & Principles -All repositories MUST follow these software engineering principles. They apply to every language, framework, and layer in the stack. Repository-level AGENTS.md or CLAUDE.md files may specify how each principle maps to project-specific patterns — those specifics take precedence over the general guidance here. +All repositories MUST follow these software engineering principles. They apply to every language, framework, and layer in the stack. +Repository-level AGENTS.md or CLAUDE.md files may specify how each principle maps to project-specific patterns — those specifics take precedence over the general guidance here. ### SOLID Principles @@ -217,17 +228,22 @@ All repositories MUST follow these software engineering principles. They apply t ### DRY — Don't Repeat Yourself -- **Eliminate knowledge duplication.** Every piece of business knowledge or logic must have a single, authoritative source. If the same rule exists in two places, extract it. -- **DRY applies to knowledge, not code.** Two blocks of code that look identical but represent different domain concepts are NOT duplication — do not merge them. Two blocks that look different but encode the same business rule ARE duplication — unify them. +- **Eliminate knowledge duplication.** Every piece of business knowledge or logic must have a single, authoritative source. + If the same rule exists in two places, extract it. +- **DRY applies to knowledge, not code.** Two blocks of code that look identical but represent different domain concepts are NOT duplication — do not merge them. + Two blocks that look different but encode the same business rule ARE duplication — unify them. - **Premature abstraction is worse than duplication.** Wait until you have at least three concrete instances before extracting a shared abstraction. Two similar cases do not justify a generic helper. ### DDD — Domain-Driven Design - **Ubiquitous language.** Use the same terminology in code, tests, specs, and conversation. If the domain calls it a "market" or "session", the code uses `Market` or `Session` — not `item` or `context`. -- **Bounded contexts.** Each major subdomain has clear boundaries. Code in one context must not directly depend on the internals of another. Communicate across contexts through well-defined interfaces or events. +- **Bounded contexts.** Each major subdomain has clear boundaries. Code in one context must not directly depend on the internals of another. + Communicate across contexts through well-defined interfaces or events. - **Aggregate roots.** Enforce invariants through aggregate roots. External code accesses an aggregate's children only through the root. -- **Value objects.** Use typed value objects (branded types, newtypes, or equivalent) for identifiers, quantities, and domain-specific data. Avoid passing raw primitives (`string`, `int`) when a domain type adds safety and meaning. -- **Repository pattern.** Persistence is abstracted behind repository interfaces that the domain defines. Infrastructure implements those interfaces. Domain code never imports ORM, SQL, or storage libraries directly. +- **Value objects.** Use typed value objects (branded types, newtypes, or equivalent) for identifiers, quantities, and domain-specific data. + Avoid passing raw primitives (`string`, `int`) when a domain type adds safety and meaning. +- **Repository pattern.** Persistence is abstracted behind repository interfaces that the domain defines. Infrastructure implements those interfaces. + Domain code never imports ORM, SQL, or storage libraries directly. ### KISS — Keep It Simple @@ -255,7 +271,8 @@ All repositories MUST follow these software engineering principles. They apply t ### Breaking Changes — Human Approval Required -Agents MUST NOT introduce breaking changes without explicit human approval. A breaking change is any modification that causes existing consumers, callers, or dependents to fail, produce incorrect results, or require changes on their end. **When in doubt, treat it as breaking.** +Agents MUST NOT introduce breaking changes without explicit human approval. A breaking change is any modification that causes existing consumers, callers, or dependents to fail, +produce incorrect results, or require changes on their end. **When in doubt, treat it as breaking.** **What constitutes a breaking change:** @@ -270,7 +287,8 @@ Agents MUST NOT introduce breaking changes without explicit human approval. A br **Detection — tests are the primary safety net:** - Existing tests encode existing contracts. If your change causes an existing test to fail, you have likely introduced a breaking change. -- **NEVER modify an existing test to make a breaking change pass.** Changing a test to accommodate new behavior is hiding the break, not fixing it. New behavior gets new tests; existing tests protect existing contracts. +- **NEVER modify an existing test to make a breaking change pass.** Changing a test to accommodate new behavior is hiding the break, not fixing it. +New behavior gets new tests; existing tests protect existing contracts. - If a test seems "wrong" or "outdated," flag it for human review. It may be the only documentation of a contract someone depends on. - Before removing or changing anything, search the entire codebase for all references (including string-based references like route strings, config keys, and dynamic imports). Report the consumer count. @@ -288,7 +306,8 @@ Agents MUST NOT introduce breaking changes without explicit human approval. A br 2. **Deprecation + migration period** — Mark the old item as deprecated, add the replacement, document the migration path. Removal happens in a separate future change with human approval. 3. **Feature flags / versioning** — Gate new behavior behind a flag so old behavior remains the default. 4. **Adapter / compatibility layers** — Write a thin adapter mapping the old interface to the new implementation. -5. **Staged database changes** — Add new column → migrate data → update application code → deploy and verify → drop old column in a separate, human-approved change. Never combine add and drop in a single migration. +5. **Staged database changes** — Add new column → migrate data → update application code → deploy and verify → +drop old column in a separate, human-approved change. Never combine add and drop in a single migration. **Agentic directives:** @@ -334,7 +353,8 @@ Agents MUST NOT introduce breaking changes without explicit human approval. A br ## BMAD Method — Spec-Driven Development -All project repositories MUST install and use the [BMAD Method](https://github.com/bmad-code-org/BMAD-METHOD) to enforce **Spec-Driven Development (SDD)**. BMAD provides structured agents, workflows, and planning artifacts that ensure every feature is fully specified before implementation begins. +All project repositories MUST install and use the [BMAD Method](https://github.com/bmad-code-org/BMAD-METHOD) to enforce **Spec-Driven Development (SDD)**. +BMAD provides structured agents, workflows, and planning artifacts that ensure every feature is fully specified before implementation begins. ### Setup @@ -350,13 +370,16 @@ All project repositories MUST install and use the [BMAD Method](https://github.c - Architecture — technical decisions, component design, data model - UX Design Specification — user flows, wireframes, accessibility - Epics & Stories — implementation-ready backlog with acceptance criteria -3. **Always consult planning artifacts before implementing.** They are the source of truth for what to build and how. If artifacts are missing or outdated, update them first — do not implement against stale specs. +3. **Always consult planning artifacts before implementing.** They are the source of truth for what to build and how. +If artifacts are missing or outdated, update them first — do not implement against stale specs. 4. **Use BMAD stories for implementation.** Stories created via `bmad-create-story` contain all context an agent needs. Use `bmad-dev-story` to execute them. 5. **Validate readiness before sprints.** Run `bmad-check-implementation-readiness` to ensure specs are complete before starting implementation work. ### BMAD Agents Available -Use BMAD's specialized agents via their slash commands: `bmad-pm` (Product Manager), `bmad-architect` (Solution Architect), `bmad-ux-designer` (UX Designer), `bmad-dev` (Developer), `bmad-tea` (Test Architect), `bmad-qa` (QA), `bmad-sm` (Scrum Master), and others. Run `bmad-help` for guidance on which agent or workflow to use next. +Use BMAD's specialized agents via their slash commands: `bmad-pm` (Product Manager), `bmad-architect` (Solution Architect), +`bmad-ux-designer` (UX Designer), `bmad-dev` (Developer), `bmad-tea` (Test Architect), `bmad-qa` (QA), `bmad-sm` (Scrum Master), and others. +Run `bmad-help` for guidance on which agent or workflow to use next. --- @@ -445,7 +468,9 @@ This org enforces branch protection via **classic branch protection rules** and #### SonarCloud Check Names -SonarCloud check names may not match exactly across repos — expect check name mismatches. If a merge is blocked by a "check expected" error, first identify the exact required check name(s), the actual check name reported, and the `app_id`. Then fix the branch protection configuration. Use `gh pr merge --admin` only with explicit user approval and only after confirming all intended quality gates have passed. +SonarCloud check names may not match exactly across repos — expect check name mismatches. If a merge is blocked by a "check expected" error, +first identify the exact required check name(s), the actual check name reported, and the `app_id`. Then fix the branch protection configuration. +Use `gh pr merge --admin` only with explicit user approval and only after confirming all intended quality gates have passed. #### Thread Resolution Policy @@ -468,7 +493,9 @@ The `dependabot-automerge.yml` workflow handles automatic merging of Dependabot #### Claude Code Workflow on Dependabot PRs -The `claude.yml` workflow skips the Claude Code action step for Dependabot PRs (`github.event.pull_request.user.login != 'dependabot[bot]'`). The job still runs and reports SUCCESS to satisfy required status checks, but the Claude action step is skipped since: +The `claude.yml` workflow skips the Claude Code action step for Dependabot PRs (`github.event.pull_request.user.login != 'dependabot[bot]'`). +The job still runs and reports SUCCESS to satisfy required status checks, but the Claude action step is skipped since: + - `CLAUDE_CODE_OAUTH_TOKEN` is an Actions secret, not a Dependabot secret - AI code review on automated version bumps adds cost without value @@ -476,17 +503,31 @@ The `claude.yml` workflow skips the Claude Code action step for Dependabot PRs ( ## Multi-Agent Isolation — Git Worktrees -When multiple agents work on the same repository concurrently, they MUST use **isolated workspaces** to prevent conflicts. Git worktrees are the industry-standard isolation primitive — used by Claude Code, Cursor, Windsurf, Augment Intent, and dmux. Cloud agents (OpenAI Codex, GitHub Copilot, Devin) use containers or ephemeral environments that provide equivalent isolation. +When multiple agents work on the same repository concurrently, they MUST use **isolated workspaces** to prevent conflicts. +Git worktrees are the industry-standard isolation primitive — used by Claude Code, Cursor, Windsurf, Augment Intent, and dmux. +Cloud agents (OpenAI Codex, GitHub Copilot, Devin) use containers or ephemeral environments that provide equivalent isolation. Never have two agents working in the same working directory simultaneously. ### Rules -1. **One workspace per agent.** Every agent performing code changes MUST operate in its own isolated workspace (git worktree, container, or ephemeral environment). This applies to Claude Code (`isolation: "worktree"` or `--worktree`), Cursor parallel agents, GitHub Copilot coding agent, OpenAI Codex, and any other AI agent tool. +1. **One workspace per agent.** Every agent performing code changes MUST operate in its own isolated workspace + (git worktree, container, or ephemeral environment). This applies to Claude Code (`isolation: "worktree"` or `--worktree`), + Cursor parallel agents, GitHub Copilot coding agent, OpenAI Codex, and any other AI agent tool. 2. **One agent per story/task.** Each workspace maps to exactly one BMAD story, feature, or bug fix. Do not assign the same story to multiple agents. -3. **No overlapping file ownership.** Two agents MUST NOT modify the same file concurrently. If stories touch shared files (e.g., a shared type definition, config, or lockfile), serialize those stories — do not run them in parallel. This is the single most important rule for multi-agent work. -4. **Branch from the default branch** — unless using a stacked PR workflow (see [Stacked PRs for Epic/Feature Development](#stacked-prs-for-epicfeature-development)). Outside a stacked-Epic/Feature workflow, workspaces MUST branch from the repository's configured default branch (for example, `origin/main`). You MAY use `origin/HEAD` as a shortcut when it is correctly configured, but MUST NOT rely on it being present. Never branch from another agent's branch **except** when (a) Epics/Features are part of a declared stack and the child Epic/Feature branches from its parent Epic/Feature's branch, or (b) story worktrees/branches are created from the Epic/Feature integration branch as defined in the stacked-PR workflow. -5. **One PR per workspace.** Each workspace produces exactly one pull request. Do not combine unrelated changes. (In a stacked-Epic/Feature workflow, story worktrees may optionally produce short-lived PRs targeting the Epic/Feature branch for review — these are internal integration PRs, not standalone feature PRs.) +3. **No overlapping file ownership.** Two agents MUST NOT modify the same file concurrently. If stories touch shared files + (e.g., a shared type definition, config, or lockfile), serialize those stories — do not run them in parallel. + This is the single most important rule for multi-agent work. +4. **Branch from the default branch** — unless using a stacked PR workflow +(see [Stacked PRs for Epic/Feature Development](#stacked-prs-for-epicfeature-development)). +Outside a stacked-Epic/Feature workflow, workspaces MUST branch from the repository's configured default branch (for example, `origin/main`). +You MAY use `origin/HEAD` as a shortcut when it is correctly configured, but MUST NOT rely on it being present. +Never branch from another agent's branch **except** when (a) Epics/Features are part of a declared stack and the child Epic/Feature branches +from its parent Epic/Feature's branch, or (b) story worktrees/branches are created from the Epic/Feature integration branch +as defined in the stacked-PR workflow. +5. **One PR per workspace.** Each workspace produces exactly one pull request. Do not combine unrelated changes. +(In a stacked-Epic/Feature workflow, story worktrees may optionally produce short-lived PRs targeting the Epic/Feature branch +for review — these are internal integration PRs, not standalone feature PRs.) 6. **3–5 parallel agents max.** Coordination overhead increases non-linearly. Limit concurrent agents to 3–5 per repository. ### Detecting File Overlap @@ -590,6 +631,7 @@ Do NOT share branches or state between agents operating on different repos. ### Coordination Checklist (for humans orchestrating multiple agents) Before launching parallel agents, verify: + - [ ] Each agent has a distinct story/task assignment - [ ] No two agents will modify the same files - [ ] Shared dependencies (lockfiles, generated types) are up to date on the default branch before agents start @@ -600,7 +642,10 @@ Before launching parallel agents, verify: ## Stacked PRs for Epic/Feature Development -When a project has multiple Epics/Features with **sequential dependencies** — where Epic 2 builds on the foundation laid by Epic 1, Epic 3 extends Epic 2, and so on — the standard "branch from main" model forces each Epic/Feature to wait for the previous one's PR to fully merge before work can begin. Stacked PRs eliminate this bottleneck by letting each Epic/Feature's branch build on the previous one's branch, forming a chain that merges bottom-up. +When a project has multiple Epics/Features with **sequential dependencies** — where Epic 2 builds on the foundation laid by Epic 1, +Epic 3 extends Epic 2, and so on — the standard "branch from main" model forces each Epic/Feature to wait for the previous one's PR +to fully merge before work can begin. Stacked PRs eliminate this bottleneck by letting each Epic/Feature's branch build on the previous +one's branch, forming a chain that merges bottom-up. Each Epic/Feature produces a **single PR** containing all of its stories. The stack is a chain of Epic/Feature-level PRs: @@ -610,7 +655,9 @@ main ← Epic-1-PR ← Epic-2-PR ← Epic-3-PR ← Epic-4-PR ### How It Works -Each Epic/Feature gets one long-lived **Epic/Feature branch** (also called its integration branch). Multiple agents work stories concurrently in separate worktrees that branch from the Epic/Feature branch, then merge their completed stories back into it. The Epic/Feature branch accumulates all story work and becomes one PR in the stack. +Each Epic/Feature gets one long-lived **Epic/Feature branch** (also called its integration branch). +Multiple agents work stories concurrently in separate worktrees that branch from the Epic/Feature branch, +then merge their completed stories back into it. The Epic/Feature branch accumulates all story work and becomes one PR in the stack. | PR | Source branch | Target branch | |----|---------------|---------------| @@ -626,10 +673,15 @@ When Epic 1's PR merges into `main`, Epic 2's PR is retargeted to `main`, and so 1. **One PR per Epic/Feature.** Each Epic/Feature produces exactly one PR. All stories within it are merged into its branch. 2. **Stacks are strictly linear.** No branching within a stack (no diamond or tree shapes). One parent, one child. 3. **Maximum stack depth: 4.** Deeper stacks become fragile and painful to rebase. If a project has more than 4 sequential Epics/Features, look for opportunities to merge intermediate ones before continuing. -4. **Parallel agents within an Epic/Feature.** Multiple agents CAN work on stories within the same Epic/Feature concurrently — each in its own worktree branching from the Epic/Feature branch. The standard multi-agent isolation rules apply: no two agents modify the same file. Story worktrees merge back into the Epic/Feature branch when complete. -5. **Sprints within an Epic/Feature may overlap.** If Sprint 2's stories are independent of Sprint 1's stories, agents may work on both sprints concurrently. Only serialize sprints when later stories depend on earlier ones. -6. **Independent stacks CAN run in parallel.** If your project has two separate dependency chains (e.g., A1→A2 and B1→B2), run those stacks concurrently with separate agents. The standard multi-agent isolation rules apply — no overlapping file ownership across stacks. -7. **File ownership within a stack is cumulative.** Files touched by Epic 1 may also be touched by Epic 2 (that's the nature of sequential dependency). Ensure agents in the child Epic/Feature coordinate with the parent's completed state. +4. **Parallel agents within an Epic/Feature.** Multiple agents CAN work on stories within the same Epic/Feature concurrently — +each in its own worktree branching from the Epic/Feature branch. The standard multi-agent isolation rules apply: +no two agents modify the same file. Story worktrees merge back into the Epic/Feature branch when complete. +5. **Sprints within an Epic/Feature may overlap.** If Sprint 2's stories are independent of Sprint 1's stories, +agents may work on both sprints concurrently. Only serialize sprints when later stories depend on earlier ones. +6. **Independent stacks CAN run in parallel.** If your project has two separate dependency chains (e.g., A1→A2 and B1→B2), +run those stacks concurrently with separate agents. The standard multi-agent isolation rules apply — no overlapping file ownership across stacks. +7. **File ownership within a stack is cumulative.** Files touched by Epic 1 may also be touched by Epic 2 +(that's the nature of sequential dependency). Ensure agents in the child Epic/Feature coordinate with the parent's completed state. 8. **Bottom-up merge order is mandatory.** Always merge the bottom PR first, then retarget the next PR to `main`, and so on. Never merge out of order. ### Workflow — Planning the Stack @@ -675,7 +727,8 @@ git worktree add .worktrees/S-1.3-db-schema -b epic-1/S-1.3-db-schema origin/epi Each agent implements its story, runs quality checks, and pushes. -**Step 3: Merge stories back into the Epic/Feature branch.** As stories complete, merge them into the Epic/Feature branch. See [Story and Sprint Organization Within an Epic/Feature](#story-and-sprint-organization-within-an-epicfeature) for merge strategies and commands. +**Step 3: Merge stories back into the Epic/Feature branch.** As stories complete, merge them into the Epic/Feature branch. +See [Story and Sprint Organization Within an Epic/Feature](#story-and-sprint-organization-within-an-epicfeature) for merge strategies and commands. **Step 4: Create the next Epic/Feature branch.** Once all stories that the next Epic/Feature depends on have been merged into the previous branch, create the next one: @@ -732,7 +785,9 @@ If conflicts are extensive, consider collapsing the stack — merge what you can ### Keeping Epic/Feature Branches in Sync with Main -If `main` advances while a stack is in progress (e.g., hotfixes or other PRs merge), periodically rebase the bottom Epic/Feature branch onto `main` and propagate upward through the stack. Do this between Sprints or at natural breakpoints — not while story agents are actively working. A long-diverged Epic/Feature branch will produce painful conflicts at merge time. +If `main` advances while a stack is in progress (e.g., hotfixes or other PRs merge), periodically rebase the bottom Epic/Feature branch +onto `main` and propagate upward through the stack. Do this between Sprints or at natural breakpoints — not while story agents are +actively working. A long-diverged Epic/Feature branch will produce painful conflicts at merge time. ### Story and Sprint Organization Within an Epic/Feature @@ -828,6 +883,7 @@ Once Agents 1–3 merge their stories into `epic-1/foundation`, Agent 6 can begi ### Stack Coordination Checklist Before starting a stacked Epic/Feature workflow, verify: + - [ ] Epics/Features have genuine sequential dependencies (not just conceptual ordering) - [ ] Stack depth is 4 or fewer - [ ] Stack plan is documented with Epic/Feature order, parent relationships, and Sprint breakdown @@ -872,12 +928,14 @@ Before starting a stacked Epic/Feature workflow, verify: ## Structured Logging -All services MUST use structured logging. Structured logs are machine-parseable, correlatable across services, and essential for observability. These rules apply to every language and framework in the stack. +All services MUST use structured logging. Structured logs are machine-parseable, correlatable across services, and essential for observability. +These rules apply to every language and framework in the stack. ### Format & Fields - **Emit all logs as JSON objects in production.** Never use unstructured print statements or string interpolation for application logs. -- **Every log line MUST include these baseline fields.** Configure your logging library at initialization to automatically include `timestamp`, `service`, and `version` in every log entry — do not rely on developers adding these per-call. +- **Every log line MUST include these baseline fields.** Configure your logging library at initialization to automatically include + `timestamp`, `service`, and `version` in every log entry — do not rely on developers adding these per-call. | Field | Format | Example | |-------|--------|---------| @@ -891,8 +949,13 @@ All services MUST use structured logging. Structured logs are machine-parseable, - WRONG: `logger.Info("User 1234 placed order 5678")` - RIGHT: `logger.Info("order placed", "user_id", "1234", "order_id", "5678")` - **Use `snake_case` for all field names.** Check existing logs in the codebase for canonical field names before inventing new ones. -- **Canonical field names:** `user_id`, `request_id`, `correlation_id`, `causation_id`, `trace_id`, `span_id`, `duration_ms`, `http_method`, `http_path`, `http_status`, `error_message`, `error_stack`. -- **Relationship between correlation IDs:** `request_id` is assigned per inbound request. `correlation_id` tracks a logical operation across multiple services/events (often equal to the originating `request_id`). `causation_id` identifies the direct parent event or command that triggered the current action. `trace_id` and `span_id` are OpenTelemetry-specific and bridge logs to distributed traces. When CQRS domain events include `correlation_id`/`causation_id` in metadata, these MUST use the same field names and values as the logging context. +- **Canonical field names:** `user_id`, `request_id`, `correlation_id`, `causation_id`, `trace_id`, `span_id`, `duration_ms`, +`http_method`, `http_path`, `http_status`, `error_message`, `error_stack`. +- **Relationship between correlation IDs:** `request_id` is assigned per inbound request. `correlation_id` tracks a logical operation + across multiple services/events (often equal to the originating `request_id`). `causation_id` identifies the direct parent event + or command that triggered the current action. `trace_id` and `span_id` are OpenTelemetry-specific and bridge logs to distributed traces. + When CQRS domain events include `correlation_id`/`causation_id` in metadata, these MUST use the same field names and values + as the logging context. ### Correlation & Tracing @@ -912,9 +975,12 @@ All services MUST use structured logging. Structured logs are machine-parseable, | **FATAL** | The process cannot continue and will exit. Use extremely sparingly | Failed to bind port, required config missing at startup | **Rules:** + - A 404 for a missing resource is INFO, not ERROR. - Validation failures caused by user input are WARN at most, usually INFO. -- Every ERROR and FATAL log MUST include `error_message` (string) and, where available, `error_stack` (string). If the logging library supports automatic error serialization (e.g., passing an error object that the library expands into `error_message` and `error_stack` fields), use that mechanism — but verify the emitted JSON contains the canonical field names. +- Every ERROR and FATAL log MUST include `error_message` (string) and, where available, `error_stack` (string). +If the logging library supports automatic error serialization (e.g., passing an error object that the library expands +into `error_message` and `error_stack` fields), use that mechanism — but verify the emitted JSON contains the canonical field names. - If an error is caught and handled with a fallback, log at WARN, not ERROR. ### What to Log @@ -943,7 +1009,9 @@ These rules are deterministic — apply them whenever writing or modifying code: 3. When handling an error, log with at minimum: `level=error`, `msg=`, `error_message`, and all context IDs (`request_id`, `user_id`) available in scope. 4. When calling an external service, log before (at DEBUG) and after (ERROR on failure with `duration_ms`, INFO/DEBUG on success with `duration_ms`). 5. When a function receives a `context.Context` (Go) or request object (Node), extract the logger from it. NEVER create a new bare logger inside a handler. -6. NEVER log any variable whose name exactly matches or ends with: `password`, `secret`, `token`, `api_key`, `private_key`, `secret_key`, `access_key`, `authorization`, `cookie`, `ssn`, `credit_card`, `card_number`. When in doubt, omit the field rather than risk leaking sensitive data. +6. NEVER log any variable whose name exactly matches or ends with: `password`, `secret`, `token`, `api_key`, `private_key`, +`secret_key`, `access_key`, `authorization`, `cookie`, `ssn`, `credit_card`, `card_number`. +When in doubt, omit the field rather than risk leaking sensitive data. 7. When adding retry logic, log each retry at WARN with `attempt_number` and `max_retries`. 8. When choosing a log level, ask: "Will someone be paged?" → ERROR. "Degradation?" → WARN. "Normal operation?" → INFO. "Only useful debugging?" → DEBUG. @@ -967,9 +1035,12 @@ These rules are deterministic — apply them whenever writing or modifying code: ## CQRS — Command Query Responsibility Segregation -CQRS separates read and write models to allow independent optimization of each. These rules define when and how to apply CQRS across the organization. CQRS is a **per-use-case decision**, not an architectural mandate — some operations within a service may use CQRS while others use standard CRUD. +CQRS separates read and write models to allow independent optimization of each. These rules define when and how to apply CQRS across the organization. +CQRS is a **per-use-case decision**, not an architectural mandate — some operations within a service may use CQRS while others use standard CRUD. -**CQRS is not Event Sourcing.** CQRS only requires separate read and write models. Event Sourcing (persisting state as a sequence of events rather than current state) is an orthogonal pattern that pairs well with CQRS but is NOT required. Do not conflate them. Apply Event Sourcing only when there is an explicit requirement for full audit trails, temporal queries, or event replay — not as a default. +**CQRS is not Event Sourcing.** CQRS only requires separate read and write models. Event Sourcing (persisting state as a sequence of events +rather than current state) is an orthogonal pattern that pairs well with CQRS but is NOT required. Do not conflate them. +Apply Event Sourcing only when there is an explicit requirement for full audit trails, temporal queries, or event replay — not as a default. ### When to Apply @@ -1002,7 +1073,8 @@ Mixing these conventions (e.g., a command named `OrderPlaced` or an event named ### Command Patterns - A command MUST be a plain data structure (DTO) with no behavior. It carries the intent and the data needed to fulfill it. -- Every command that creates or mutates state MUST be idempotent, or the system MUST detect and reject duplicates. Strategies: idempotency key from client, natural idempotency ("set X" not "append X"), or conditional/versioned updates (`expectedVersion`/`ETag`). +- Every command that creates or mutates state MUST be idempotent, or the system MUST detect and reject duplicates. +Strategies: idempotency key from client, natural idempotency ("set X" not "append X"), or conditional/versioned updates (`expectedVersion`/`ETag`). - Each command MUST have exactly one handler. - A command handler's responsibilities are: (1) validate, (2) load aggregate/entity, (3) invoke domain logic, (4) persist changes, (5) publish domain events. - **One command = one transaction = one aggregate mutation.** Do NOT mutate multiple aggregates in one command handler. Use domain events and sagas for cross-aggregate coordination. @@ -1025,9 +1097,14 @@ Mixing these conventions (e.g., a command named `OrderPlaced` or an event named - Events MUST be immutable. Once published, an event's schema and data MUST NOT change. To evolve, publish new event types and deprecate old ones. - Events SHOULD carry enough data for consumers to process them without querying back to the source ("fat" events over "thin" events). -- Every event MUST include: `event_id` (unique), `event_type`, `aggregate_id`, `aggregate_type`, `timestamp`, `version`/`sequence_number`, `payload`, `metadata` (`correlation_id`, `causation_id`, `user_id`). The `correlation_id` and `causation_id` in event metadata use the same field names and semantics defined in the Structured Logging section — propagate them from the request/command context that triggered the event. +- Every event MUST include: `event_id` (unique), `event_type`, `aggregate_id`, `aggregate_type`, `timestamp`, +`version`/`sequence_number`, `payload`, `metadata` (`correlation_id`, `causation_id`, `user_id`). +The `correlation_id` and `causation_id` in event metadata use the same field names and semantics defined in the Structured Logging section — +propagate them from the request/command context that triggered the event. - Event handlers (projectors, reactors, sagas) MUST be idempotent. Use `sequence_number` or `event_id` to detect and skip already-processed events. -- **Outbox pattern:** When a command handler writes to the database AND publishes events, use the transactional outbox pattern — write events to an outbox table in the same transaction as the aggregate, then publish from the outbox asynchronously. This prevents dual-write problems. +- **Outbox pattern:** When a command handler writes to the database AND publishes events, use the transactional outbox pattern — +write events to an outbox table in the same transaction as the aggregate, then publish from the outbox asynchronously. +This prevents dual-write problems. ### Eventual Consistency @@ -1065,6 +1142,6 @@ Mixing these conventions (e.g., a command named `OrderPlaced` or an event named ## References -- AGENTS.md convention: https://agents.md/ -- BMAD Method: https://github.com/bmad-code-org/BMAD-METHOD -- Organization: https://github.com/petry-projects +- AGENTS.md convention: +- BMAD Method: +- Organization: diff --git a/standards/ci-standards.md b/standards/ci-standards.md index 8b8c28c..67c208e 100644 --- a/standards/ci-standards.md +++ b/standards/ci-standards.md @@ -111,8 +111,15 @@ Each repo needs a `sonar-project.properties` file at root with project key and o ### 4. Claude Code (`claude.yml`) -AI-assisted code review via Claude Code Action on PRs. Also responds to -`@claude` mentions in PR comments. +AI-assisted code review on PRs and issue automation via Claude Code Action. +Claude responds to PR events, `@claude` mentions in comments, and issues +labeled `claude`. The label trigger enables Claude to work issues like a +human contributor — reading the issue, creating a branch, implementing the +fix, and opening a PR. + +**Billing:** This workflow uses Anthropic credits via `CLAUDE_CODE_OAUTH_TOKEN`, +not GitHub Copilot premium requests. This is distinct from the "Assign to Agent" +UI feature which consumes Copilot premium requests. **Standard configuration:** @@ -127,6 +134,8 @@ on: types: [created] pull_request_review_comment: types: [created] + issues: + types: [labeled] permissions: {} @@ -140,28 +149,50 @@ jobs: contains(fromJson('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) || (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude') && - contains(fromJson('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) + contains(fromJson('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)) || + (github.event_name == 'issues' && github.event.action == 'labeled' && + github.event.label.name == 'claude') runs-on: ubuntu-latest timeout-minutes: 60 permissions: - contents: read + contents: write id-token: write pull-requests: write issues: write steps: - name: Run Claude Code if: github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]' - uses: anthropics/claude-code-action@bee87b3258c251f9279e5371b0cc3660f37f3f77 # v1 + uses: anthropics/claude-code-action@6e2bd52842c65e914eba5c8badd17560bd26b5de # v1.0.89 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + label_trigger: "claude" ``` **Required secrets:** `CLAUDE_CODE_OAUTH_TOKEN` +**Required labels:** The `claude` label (color: `7c3aed`) must exist on every +repository. The weekly compliance audit ensures this label is present. It can +also be applied manually to any issue to trigger Claude. + +**How Claude follows org standards:** `claude-code-action` automatically reads +`CLAUDE.md` from the repository root. The org-level `.github/CLAUDE.md` is +inherited by repos without their own. Each repo's `CLAUDE.md` references +`AGENTS.md` for cross-cutting development standards (TDD, SOLID, pre-commit +checks, etc.). No additional `prompt` or `settings` input is needed. + +**Permissions note:** `contents: write` is required for issue-triggered work +where Claude creates branches and pushes commits. PR review mode only needs +`contents: read`, but a single permission set covers both modes. + **Dependabot behavior:** The Claude Code step is skipped for Dependabot PRs (the `if` condition on the step). The job still runs and reports SUCCESS to satisfy required status checks. See [AGENTS.md](../AGENTS.md#claude-code-workflow-on-dependabot-prs). +**Issue trigger security:** The `issues: [labeled]` event fires when any user +with triage or write access applies a label. The label name check in the `if:` +condition ensures only the `claude` label triggers the workflow — other labels +are ignored. Apply the `claude` label manually to any issue to trigger Claude. + ### 5. Dependabot Auto-Merge (`dependabot-automerge.yml`) Automatically approves and squash-merges eligible Dependabot PRs. @@ -250,6 +281,7 @@ steps: ``` **Additional jobs for Electron:** + - Mutation testing (`npm run test:mutate`) — `continue-on-error: true` - E2E tests via Playwright (`npx playwright test`) on macOS — `continue-on-error: true` @@ -321,7 +353,7 @@ For single-job workflows, top-level least-privilege permissions are acceptable |----------|------------| | CI (build/test) | `contents: read` | | SonarCloud | `contents: read`, `pull-requests: read` | -| Claude Code | `contents: read`, `id-token: write`, `pull-requests: write`, `issues: write` | +| Claude Code | `contents: write`, `id-token: write`, `pull-requests: write`, `issues: write` | | CodeQL | `actions: read`, `security-events: write`, `contents: read` | | Dependabot auto-merge | `contents: read`, `pull-requests: read` (+ app token for merge) | @@ -461,4 +493,4 @@ All repos MUST align to the latest version of each action: |--------|---------------|---------------------| | **SonarCloud action** | v7.0.0 | ContentTwin, google-app-scripts (currently v6) | | **CodeQL action** | v4 | markets (currently v3) | -| **Claude Code Action** | Latest SHA | All repos should use the same pinned SHA | +| **Claude Code Action** | v1.0.89 (`6e2bd528`) | All repos should use the same pinned SHA | diff --git a/standards/dependabot-policy.md b/standards/dependabot-policy.md index 702889b..9b282c3 100644 --- a/standards/dependabot-policy.md +++ b/standards/dependabot-policy.md @@ -139,6 +139,7 @@ relevant ecosystem entries into a single `dependabot.yml`. See See [`workflows/dependabot-automerge.yml`](workflows/dependabot-automerge.yml). Behavior: + - Triggers on `pull_request_target` from `dependabot[bot]` - Fetches Dependabot metadata to determine update type - For **patch** and **minor** updates (and indirect dependency updates):