From 51a41973a1d2afe4985618abb1e369560fbfcf51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bir=C3=B3=2C=20Csaba=20Attila?= Date: Sat, 14 Feb 2026 15:57:51 +0100 Subject: [PATCH] docs(config): update docs for CI rules and ADS architecture - CLAUDE.md: replace ADS-over-MQTT with direct ADS references, add CI Checks section with gotchas (pull_request_target, header/body limits, npm cache, PR title re-trigger) - CONTRIBUTING.md: add ci scope, document hard CI rules (header 72, body 100, subject-case disabled), add CI Checks section - GIT_WORKFLOW.md: add ci scope, fix body wrap limit (100 not 72), remove lowercase subject rule, add PR title validation note and CLA check step Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 71 +++++++++++++++++++++++++++++++++++++-------- CONTRIBUTING.md | 27 ++++++++++++----- doc/GIT_WORKFLOW.md | 25 +++++++++++----- 3 files changed, 96 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5ea624c..5379fca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,30 +8,30 @@ FlowForge is a visual no-code PLC programming environment inspired by Unreal Eng ## Architecture -Four main components: **Visual Editor** (web frontend) → **.NET Backend API** → **PLC Build Server(s)** + **Monitor Server(s)** → Beckhoff PLC via ADS over MQTT. +Four main components: **Visual Editor** (web frontend) → **.NET Backend API** → **PLC Build Server(s)** + **Monitor Server(s)** → Beckhoff PLC via direct ADS (Beckhoff.TwinCAT.Ads). -- `src/shared/FlowForge.Shared/` — Common DTOs, enums, MQTT topic constants. No external dependencies. +- `src/shared/FlowForge.Shared/` — Common DTOs (Flow, Build, Deploy, Project, Target, Auth, Monitor, Ads), enums, MQTT topic constants. No external dependencies. - `src/frontend/` — Web-based node editor (React + TypeScript + React Flow + Zustand + React Query); feature-based folder structure with auth (Keycloak), layout, and feature modules (editor, projects, build, deploy, targets, monitoring, admin) - `src/backend/` — Clean Architecture Lite (3 projects): - `src/backend/src/FlowForge.Backend.Api/` — ASP.NET Core API (Controllers, Middleware, Auth, SignalR hubs). References Application + Infrastructure + Shared. - `src/backend/src/FlowForge.Backend.Application/` — Business logic, entities, service/repository interfaces. References Shared only. - `src/backend/src/FlowForge.Backend.Infrastructure/` — EF Core, external integrations (Git, MQTT, Docker, Keycloak, AES encryption). References Application + Shared. -- `src/build-server/` — PLC build server (C#/.NET); pipeline architecture with sequential build steps (IBuildStep), code generation (INodeTranslator strategy pattern), TwinCAT COM facades (IVisualStudioInstance, IAutomationInterface). References Shared. -- `src/monitor-server/` — On-demand PLC monitoring container (C#/.NET, SignalR, MQTTnet); typed hub interface, subscription manager, MQTT ADS client. References Shared. -- `doc/` — Architecture docs (`ARCHITECTURE.md`, `BUILD_SERVER_DESIGN.md`, `MODULE_ARCHITECTURE.md`), decision log, tech decisions +- `src/build-server/` — PLC build server (C#/.NET); pipeline architecture with sequential build steps (IBuildStep), code generation (INodeTranslator strategy pattern), TwinCAT COM facades (IVisualStudioInstance, IAutomationInterface), ADS deploy client (IAdsDeployClient). References Shared. Uses `Beckhoff.TwinCAT.Ads` for direct PLC communication. +- `src/monitor-server/` — On-demand PLC monitoring container (C#/.NET, SignalR); typed hub interface, subscription manager, direct ADS client (IAdsClient) via `Beckhoff.TwinCAT.Ads` + `Beckhoff.TwinCAT.Ads.TcpRouter` for Linux/Docker. References Shared. +- `doc/` — Architecture docs (`ARCHITECTURE.md`, `BUILD_SERVER_DESIGN.md`, `MODULE_ARCHITECTURE.md`, `ADS_INTEGRATION.md`), decision log, tech decisions - `samples/` — Example visual programs - `test/` — xUnit + NSubstitute + FluentAssertions test projects: `FlowForge.Shared.Tests`, `FlowForge.Backend.Api.Tests`, `FlowForge.Backend.Application.Tests`, `FlowForge.Backend.Infrastructure.Tests`, `FlowForge.BuildServer.Tests`, `FlowForge.MonitorServer.Tests` - `src/FlowForge.sln` — Root solution including all .NET projects **Key architectural decisions:** +- **Direct ADS communication**: Monitor Server uses `Beckhoff.TwinCAT.Ads` + `TcpRouter` (Linux/Docker); Build Server uses `Beckhoff.TwinCAT.Ads` natively (Windows). MQTT is for FlowForge internal messaging only (build notifications, deploy status), NOT for ADS relay. See `doc/ADS_INTEGRATION.md`. - **Keycloak as auth layer**: all authentication/authorization via Keycloak (local users, LDAP federation, external SSO — all Keycloak config). Backend only validates JWT from Keycloak. User management via FlowForge admin UI (facade over Keycloak Admin REST API). - **Git repo as single source of truth** on GitHub — each project is a repo containing flow JSON (user-edited) and generated PLC solution (build server output); users commit/push with their own identity; service user handles repo creation only - **Git-backed workflow**: backend handles git operations for open (fetch) and save (commit/push); build server clones repo, generates TwinCAT solution, commits/pushes back; PostgreSQL stores only metadata, auth, target registry, audit - **Machine type templates** for new projects (e.g., 3-axis standalone, 3+1 axis standalone, x-division rotary table) -- **Build and Deploy are separate operations with permission hierarchy**: Deploy permission implies Build permission. Deploy button triggers Build + Deploy sequentially; Build button triggers Build only. Build generates PLC solution and commits to repo; Deploy activates on target PLC via ADS over MQTT — both executed by the build server (requires TwinCAT Engineering). Backend orchestrates, build server executes. Deploy lock and 4-eyes principle for production. +- **Build and Deploy are separate operations with permission hierarchy**: Deploy permission implies Build permission. Deploy button triggers Build + Deploy sequentially; Build button triggers Build only. Build generates PLC solution and commits to repo; Deploy activates on target PLC via direct ADS — both executed by the build server (requires TwinCAT Engineering). Backend orchestrates, build server executes. Deploy lock and 4-eyes principle for production. - **Build queue**: PostgreSQL-based queue with MQTT notify; backend inserts jobs, build servers poll via REST with `FOR UPDATE SKIP LOCKED`; MQTT is lightweight wake-up signal only - **Build servers are version-specific** (one per TwinCAT version on separate Windows Servers); target runtime readable via ADS enables automatic build server selection -- **PLC activation via ADS over MQTT** through a central MQTT broker (free, no per-server ADS route config needed) - **Deploy safety**: target labeling/grouping, deploy protection for production targets (4-eyes principle), deploy lock on running PLCs, full audit trail via git history - **Infrastructure**: Single Docker Compose stack (frontend/nginx, backend, Keycloak, PostgreSQL, MQTT broker, Traefik, docker-socket-proxy). Monitor containers created on-demand via docker-socket-proxy, auto-discovered by Traefik via Docker labels. Build server external on Windows Server (TwinCAT Engineering requirement), connects via REST + MQTT. @@ -69,26 +69,73 @@ cd src/frontend && npm run dev ## Commit Conventions -Commits are validated by commitlint via husky git hook. **All commits must follow Conventional Commits format:** +Commits are validated by commitlint via husky git hook **and** by GitHub Actions CI on PRs. Both commit messages AND PR titles must pass validation. + +**All commits must follow Conventional Commits format:** ``` [optional scope]: + +[optional body] + +[optional footer(s)] ``` Types: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert` -Scopes: `frontend`, `backend`, `build-server`, `monitor-server`, `docs`, `config`, `ci`, `deps` +Scopes (warning-level, not required): `frontend`, `backend`, `build-server`, `monitor-server`, `docs`, `config`, `ci`, `deps` + +**Hard rules enforced by CI:** +- `header-max-length`: **72 characters max** (includes type, scope, colon, space, and description) +- `body-max-line-length`: **100 characters max** per line in commit body +- `subject-empty`: subject must not be empty +- `subject-full-stop`: no trailing period +- `body-leading-blank`: blank line between header and body +- `footer-leading-blank`: blank line before footer +- `subject-case`: **disabled** (acronyms like DTO, MQTT, TwinCAT are common) + +**PR titles** are also validated against the same rules. Keep PR titles under 72 characters. -Rules: imperative mood, lowercase subject, no trailing period, max 72 chars. Use `!` after type/scope for breaking changes. +Use `!` after type/scope for breaking changes. Use imperative mood, no trailing period. + +## CI Checks + +Every PR runs two GitHub Actions checks: + +1. **Validate Commit Messages** (`commitlint.yml`): validates all PR commits AND the PR title against `.commitlintrc.json` rules. Triggered on `opened`, `synchronize`, `reopened` events. +2. **CLA Assistant** (`cla.yml`): checks Contributor License Agreement signature. Uses `pull_request_target` (reads workflow from `main` branch). Signatures stored on `cla-signatures` branch. Org members in allowlist skip signing. + +**Important CI notes:** +- CLA workflow reads from `main` branch (`pull_request_target` event) — changes to `cla.yml` must be merged to `main` first (via hotfix) to take effect +- Commitlint workflow reads `.commitlintrc.json` from the PR branch — rule changes take effect after rebase on `develop` +- PR title changes do NOT trigger CI re-runs (no `edited` event) — push a commit to trigger `synchronize` +- `package-lock.json` is in `.gitignore` — do NOT use `cache: 'npm'` in GitHub Actions setup-node steps ## Git Workflow Uses **GitFlow** branching model: -- `main` — production releases (tagged with semver) -- `develop` — integration branch +- `main` — production releases (tagged with semver), protected branch +- `develop` — integration branch, protected branch - `feature/*` — new features (from/to `develop`) - `bugfix/*` — bug fixes (from/to `develop`) - `release/*` — release prep (from `develop`, merge to both `main` and `develop`) - `hotfix/*` — emergency fixes (from `main`, merge to both `main` and `develop`) +- `cla-signatures` — CLA signature storage (unprotected, used by CLA bot) PRs target `develop` for features/bugfixes. Squash-merge for feature branches, merge commit for release/hotfix branches. + +**Release process:** +1. Create `release/x.y.z` from `develop` +2. Update `CHANGELOG.md` (move Unreleased to new version section) +3. Commit, push, create PR to `main` +4. Merge to `main` with merge commit (not squash) +5. Tag `main` with `vx.y.z` +6. Merge `main` back to `develop` +7. Delete release branch (local + remote) + +**Hotfix process** (for CI/workflow fixes that need to be on `main` immediately): +1. Create `hotfix/*` from `main` +2. Fix, commit, push, create PR to `main` +3. Merge to `main` with merge commit +4. Cherry-pick to `develop` (or merge `main` into `develop`) +5. Delete hotfix branch diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a94860b..4ff5061 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -135,6 +135,7 @@ All commit messages **MUST** follow the [Conventional Commits](https://www.conve - `monitor-server` - Monitor server changes - `docs` - Documentation - `config` - Configuration files +- `ci` - CI/CD configuration - `deps` - Dependencies **Examples:** @@ -183,20 +184,32 @@ git commit -m "your message" # ❌ Commit message does not follow Conventional Commits format ``` +**Hard Rules (enforced by CI):** +- **Header max 72 characters** (includes `type(scope): ` prefix — plan accordingly) +- **Body lines max 100 characters** each +- No trailing period in subject +- Blank line between header and body +- Subject must not be empty +- `subject-case` is **disabled** (acronyms like DTO, MQTT, TwinCAT are common) + +**PR titles** are also validated against the same rules — keep them under 72 characters. + **Good Practices:** - Use present tense: "add feature" not "added feature" - Use imperative mood: "fix bug" not "fixes bug" -- Keep first line under 72 characters - Reference issues: `Closes #123`, `Fixes #456`, `Relates to #789` - Write descriptive body for complex changes -#### Coding Standards +#### CI Checks -Once technology stack is finalized, we'll establish: -- Code formatting rules (ESLint, Prettier, etc.) -- Naming conventions -- Documentation requirements -- Testing requirements +Every PR runs two automated checks: + +1. **Validate Commit Messages** — validates all PR commits AND the PR title against `.commitlintrc.json` +2. **CLA Assistant** — checks Contributor License Agreement signature (first-time contributors only; org members are allowlisted) + +Both checks must pass before merge. See [CLAUDE.md](CLAUDE.md#ci-checks) for detailed CI notes. + +#### Coding Standards **Current Standards:** - Use meaningful variable and function names diff --git a/doc/GIT_WORKFLOW.md b/doc/GIT_WORKFLOW.md index 338aa49..6ef9f33 100644 --- a/doc/GIT_WORKFLOW.md +++ b/doc/GIT_WORKFLOW.md @@ -202,17 +202,19 @@ git push origin --delete hotfix/critical-bug-fix - `monitor-server` - Monitor server (src/monitor-server/) - `docs` - Documentation - `config` - Configuration files +- `ci` - CI/CD configuration - `deps` - Dependencies ### Guidelines 1. **Use imperative mood**: "add feature" not "added feature" -2. **Lowercase subject**: `feat: add feature` not `feat: Add feature` -3. **No period at end**: `fix: resolve bug` not `fix: resolve bug.` -4. **Max 72 characters** for subject line +2. **No period at end**: `fix: resolve bug` not `fix: resolve bug.` +3. **Max 72 characters** for header (includes `type(scope): ` prefix) +4. **Max 100 characters** per line in body 5. **Separate subject from body** with blank line -6. **Wrap body at 72 characters** -7. **Reference issues**: `Closes #123`, `Fixes #456` +6. **Reference issues**: `Closes #123`, `Fixes #456` + +> **Note**: `subject-case` is disabled — acronyms (DTO, MQTT, TwinCAT) are common in this project. ### Breaking Changes @@ -254,7 +256,7 @@ See migration guide in docs/MIGRATION.md" ### PR Title Format -PR titles should follow Conventional Commits: +PR titles are validated by CI against the same commitlint rules. **Keep titles under 72 characters.** ``` feat(frontend): add visual timer node component @@ -262,6 +264,8 @@ fix(backend): resolve project save error docs: update git workflow documentation ``` +> **Note**: PR title changes do NOT re-trigger CI. Push a commit to trigger a new check. + ### PR Description Template ```markdown @@ -305,8 +309,13 @@ Relates to #456 - Address all review comments - Keep discussion professional -3. **Merge strategy**: - - **Squash and merge** for small feature branches +3. **CLA check**: + - First-time contributors must sign the CLA via PR comment + - Org members are allowlisted and skip CLA signing + - Signatures stored on `cla-signatures` branch + +4. **Merge strategy**: + - **Squash and merge** for feature/bugfix branches - **Merge commit** for release/hotfix branches - Delete source branch after merge