Skip to content

Commit 44a96c3

Browse files
Copilotsofthack007coderabbitai[bot]
authored
Revise copilot-instructions.md: consolidate, shorten, extract and extend language-specific rules (#353)
Revise copilot-instructions.md: consolidate redundant sections, extract language-specific rules * Added comprehensive internal documentation files establishing coding standards and development workflows for C++, Web UI, CI/CD, and ESP-IDF components. * Implemented CodeRabbit configuration to enforce repository-specific code review guidance and quality standards across pull requests. * Documented development workflow procedures and contribution expectations to streamline the development process and improve code consistency. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: softhack007 <91616163+softhack007@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 5735fd3 commit 44a96c3

7 files changed

Lines changed: 1690 additions & 174 deletions

.coderabbit.yaml

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
2+
#
3+
# CodeRabbit configuration — references existing guideline files to avoid
4+
# duplicating conventions. See:
5+
# .github/copilot-instructions.md — project overview & general rules
6+
# .github/cpp.instructions.md — C++ coding conventions
7+
# .github/web.instructions.md — Web UI coding conventions
8+
# .github/cicd.instructions.md — GitHub Actions / CI-CD conventions
9+
# .github/esp-idf.instructions.md — ESP-IDF / chip-specific coding guidelines
10+
# (apply when code directly uses ESP-IDF APIs:
11+
# esp_idf_*, I2S, RMT, ADC, GPIO, heap_caps, etc.)
12+
#
13+
# NOTE: This file must be committed (tracked by git) for CodeRabbit to read
14+
# it from the repository. If it is listed in .gitignore, CodeRabbit will
15+
# not see it and these settings will have no effect.
16+
17+
language: en-US
18+
19+
reviews:
20+
path_instructions:
21+
- path: "**/*.{cpp,h,hpp,ino}"
22+
instructions: >
23+
Follow the C++ coding conventions documented in .github/cpp.instructions.md
24+
and the general project guidelines in .github/copilot-instructions.md.
25+
If the code under review directly uses ESP-IDF APIs (e.g. heap_caps_malloc,
26+
I2S, RMT, ADC, GPIO, esp_timer, or any esp_idf_* / soc_* symbols), also
27+
apply the guidelines in .github/esp-idf.instructions.md.
28+
29+
Key rules: 2-space indentation (no tabs), camelCase functions/variables,
30+
PascalCase classes, UPPER_CASE macros. Mark WLED-MM-specific changes with
31+
`// WLEDMM` comments. No C++ exceptions — use return codes and debug macros.
32+
33+
Hot-path optimization guidelines (attributes, uint_fast types, caching,
34+
unsigned range checks) apply from pixel set/get operations downward —
35+
NOT to effect functions in FX.cpp, which have diverse contributor styles.
36+
37+
- path: "wled00/data/**"
38+
instructions: >
39+
Follow the web UI conventions documented in .github/web.instructions.md.
40+
41+
Key rules: indent HTML and JavaScript with tabs, CSS with tabs or spaces.
42+
Files here are built into wled00/html_*.h by tools/cdata.js — never
43+
edit those generated headers directly.
44+
45+
- path: "wled00/html_*.h"
46+
instructions: >
47+
These files are auto-generated from wled00/data/ by tools/cdata.js.
48+
They must never be manually edited or committed. Flag any PR that
49+
includes changes to these files.
50+
51+
- path: "usermods/**"
52+
instructions: >
53+
Usermods are community add-ons following the upstream WLED 0.15.x style.
54+
Each usermod lives in its own directory under usermods/ and is implemented
55+
as a .h file that is pulled in by wled00/usermods_list.cpp (guarded by
56+
#ifdef). Usermods do not use library.json. Follow the same C++ conventions
57+
as the core firmware (.github/cpp.instructions.md).
58+
59+
- path: ".github/workflows/*.{yml,yaml}"
60+
instructions: >
61+
Follow the CI/CD conventions documented in .github/cicd.instructions.md.
62+
63+
Key rules: 2-space indentation, descriptive name: on every workflow/job/step.
64+
Third-party actions must be pinned to a specific version tag — branch pins
65+
such as @main or @master are not allowed. Declare explicit permissions: blocks
66+
scoped to least privilege. Never interpolate github.event.* values directly
67+
into run: steps — pass them through an env: variable to prevent script
68+
injection. Do not use pull_request_target unless fully justified.
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
# Agent-Mode Build & Test Instructions
2+
3+
Detailed build workflow, timeouts, and troubleshooting for making code changes in agent mode. Always reference these instructions first when running builds or validating changes.
4+
5+
## Build Timing and Timeouts
6+
7+
Use these timeout values when running builds:
8+
9+
| Command | Typical Time | Minimum Timeout | Notes |
10+
|---|---|---|---|
11+
| `npm run build` | ~3 s | 30 s | Web UI → `wled00/html_*.h` headers |
12+
| `npm test` | ~40 s | 2 min | Validates build system |
13+
| `npm run dev` | continuous || Watch mode, auto-rebuilds on changes |
14+
| `pio run -e <env>` | 15–20 min | 30 min | First build downloads toolchains; subsequent builds are faster |
15+
16+
**NEVER cancel long-running builds.** PlatformIO downloads and compilation require patience.
17+
18+
## Development Workflow
19+
20+
### Web UI Changes
21+
22+
1. Edit files in `wled00/data/`
23+
2. Run `npm run build` to regenerate `wled00/html_*.h` headers
24+
3. Test with local HTTP server (see Manual Testing below)
25+
4. Run `npm test` to validate
26+
27+
### Firmware Changes
28+
29+
1. Edit files in `wled00/` (but **never** `html_*.h` files)
30+
2. Ensure web UI is built first: `npm run build`
31+
3. Build firmware: `pio run -e esp32_4MB_V4_M` (set timeout ≥ 30 min)
32+
4. Flash to device: `pio run -e [target] --target upload`
33+
34+
### Combined Web + Firmware Changes
35+
36+
1. Always build web UI first
37+
2. Test web interface manually
38+
3. Then build and test firmware
39+
40+
41+
## Before Finishing Work
42+
43+
**You MUST complete ALL of these before marking work as done:**
44+
45+
1. **Run tests**: `npm test` — must pass
46+
2. **Build firmware**: `pio run -e esp32_4MB_V4_M` — must succeed
47+
- Set timeout to 30+ minutes, **never cancel**
48+
- Choose `esp32_4MB_V4_M` as a common, representative environment
49+
- If the build fails, fix the issue before proceeding
50+
3. **For web UI changes**: manually test the interface (see below)
51+
52+
If any step fails, fix the issue. **Do NOT mark work complete with failing builds or tests.**
53+
54+
## Manual Web UI Testing
55+
56+
Start a local server:
57+
58+
```sh
59+
cd wled00/data && python3 -m http.server 8080
60+
# Open http://localhost:8080/index.htm
61+
```
62+
63+
Test these scenarios after every web UI change:
64+
65+
- **Load**: `index.htm` loads without JavaScript errors (check browser console)
66+
- **Navigation**: switching between main page and settings pages works
67+
- **Color controls**: color picker and brightness controls function correctly
68+
- **Effects**: effect selection and parameter changes work
69+
- **Settings**: form submission and validation work
70+
71+
## Troubleshooting
72+
73+
### Common Build Issues
74+
75+
| Problem | Solution |
76+
|---|---|
77+
| Missing `html_*.h` | Run `npm ci; npm run build` |
78+
| Web UI looks broken | Check browser console for JS errors |
79+
| PlatformIO network errors | Retry — downloads can be flaky |
80+
| Node.js version mismatch | Ensure Node.js 20+ (check `.nvmrc`) |
81+
82+
### Recovery Steps
83+
84+
- **Force web UI rebuild**: `npm run build -- -f`
85+
- **Clear generated files**: `rm -f wled00/html_*.h` then `npm run build`
86+
- **Clean PlatformIO cache**: `pio run --target clean`
87+
- **Reinstall Node deps**: `rm -rf node_modules && npm ci`
88+
89+
## CI/CD Validation
90+
91+
The GitHub Actions CI workflow will:
92+
1. Install Node.js and Python dependencies
93+
2. Run `npm test`
94+
3. Build web UI (automatic via PlatformIO)
95+
4. Compile firmware for **all** `default_envs` targets
96+
97+
**To ensure CI success, always validate locally:**
98+
- Run `npm test` and ensure it passes
99+
- Run `pio run -e esp32_4MB_V4_M` (or another common firmware environment, see next section) and ensure it completes successfully
100+
- If either fails locally, it WILL fail in CI
101+
102+
Match this workflow in local development to catch failures before pushing.
103+
104+
## Important Reminders
105+
106+
- **Never edit or commit** `wled00/html_*.h` — auto-generated from `wled00/data/`
107+
- Web UI rebuild is part of the PlatformIO firmware compilation pipeline
108+
- Common firmware environments: `esp32_4MB_V4_M`, `esp32_16MB_V4_S_HUB75`, `esp32S3_8MB_PSRAM_M_qspi`, `esp32_16MB_V4_M_eth`, `esp8266_4MB_S` (deprecated), `esp32dev_compat`
109+
- List all PlatformIO targets: `pio run --list-targets`

.github/cicd.instructions.md

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
---
2+
applyTo: ".github/workflows/*.yml,.github/workflows/*.yaml"
3+
---
4+
# CI/CD Conventions — GitHub Actions Workflows
5+
6+
## YAML Style
7+
8+
- Indent with **2 spaces** (no tabs)
9+
- Every workflow, job, and step must have a `name:` field that clearly describes its purpose
10+
- Group related steps logically; separate unrelated groups with a blank line
11+
- Comments (`#`) are encouraged for non-obvious decisions (e.g., why `fail-fast: false` is set, what a cron expression means)
12+
13+
## Workflow Structure
14+
15+
### Triggers
16+
17+
- Declare `on:` triggers explicitly; avoid bare `on: push` without branch filters on long-running or expensive jobs
18+
- Prefer `workflow_call` for shared build logic (see `build.yml`) to avoid duplicating steps across workflows
19+
- Document scheduled triggers (`cron:`) with a human-readable comment:
20+
21+
```yaml
22+
schedule:
23+
- cron: '0 2 * * *' # run at 2 AM UTC daily
24+
```
25+
26+
### Jobs
27+
28+
- Express all inter-job dependencies with `needs:` — never rely on implicit ordering
29+
- Use job `outputs:` + step `id:` to pass structured data between jobs (see `get_default_envs` in `build.yml`)
30+
- Set `fail-fast: false` on matrix builds so that a single failing environment does not cancel others
31+
32+
### Runners
33+
34+
- Pin to a specific Ubuntu version (`ubuntu-22.04`, `ubuntu-24.04`) rather than `ubuntu-latest` for reproducible builds
35+
- Only use `ubuntu-latest` in jobs where exact environment reproducibility is not required (e.g., trivial download/publish steps)
36+
37+
### Tool and Language Versions
38+
39+
- Pin tool versions explicitly:
40+
```yaml
41+
python-version: '3.12'
42+
```
43+
- Do not rely on the runner's pre-installed tool versions — always install via a versioned setup action
44+
45+
### Caching
46+
47+
- Always cache package managers and build tool directories when the job installs dependencies:
48+
```yaml
49+
- uses: actions/cache@v4
50+
with:
51+
path: ~/.cache/pip
52+
key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }}
53+
restore-keys: |
54+
${{ runner.os }}-pip-
55+
```
56+
- Include the environment name or a relevant identifier in cache keys when building multiple targets
57+
58+
### Artifacts
59+
60+
- Name artifacts with enough context to be unambiguous (e.g., `firmware-${{ matrix.environment }}`)
61+
- Avoid uploading artifacts that will never be consumed downstream
62+
63+
---
64+
65+
## Security
66+
67+
### Permissions — Least Privilege
68+
69+
Declare explicit `permissions:` blocks. The default token permissions are broad; scope them to the minimum required:
70+
71+
```yaml
72+
permissions:
73+
contents: read # for checkout
74+
```
75+
76+
For jobs that publish releases or write to the repository:
77+
78+
```yaml
79+
permissions:
80+
contents: write # create/update releases
81+
```
82+
83+
A common safe baseline for build-only jobs:
84+
85+
```yaml
86+
permissions:
87+
contents: read
88+
```
89+
90+
### Supply Chain — Action Pinning
91+
92+
**Third-party actions** (anything outside the `actions/` and `github/` namespaces) should be pinned to a specific release tag. Branch pins (`@main`, `@master`) are **not allowed** — they can be updated by the action author at any time without notice:
93+
94+
```yaml
95+
# ✅ Acceptable — specific version tag. SHA pinning recommended for more security, as @v2 is still a mutable tag.
96+
uses: softprops/action-gh-release@v2
97+
98+
# ❌ Not acceptable — mutable branch reference
99+
uses: andelf/nightly-release@main
100+
```
101+
102+
SHA pinning (e.g., `uses: someorg/some-action@abc1234`) is the most secure option for third-party actions; it is recommended when auditing supply-chain risk is a priority. At minimum, always use a specific version tag.
103+
104+
**First-party actions** (`actions/checkout`, `actions/cache`, `actions/upload-artifact`, etc.) pinned to a major version tag (e.g., `@v4`) are acceptable because GitHub maintains and audits these.
105+
106+
When adding a new third-party action:
107+
1. Check that the action's repository is actively maintained
108+
2. Review the action's source before adding it
109+
3. Prefer well-known, widely-used actions over obscure ones
110+
111+
### Credentials and Secrets
112+
113+
- Use `${{ secrets.GITHUB_TOKEN }}` for operations within the same repository — it is automatically scoped and rotated
114+
- Never commit secrets, tokens, or passwords into workflow files or any tracked file
115+
- Never print secrets in `run:` steps, even with `echo` — GitHub masks known secrets but derived values are not automatically masked
116+
- Scope secrets to the narrowest step that needs them using `env:` at the step level, not at the workflow level:
117+
118+
```yaml
119+
# ✅ Scoped to the step that needs it
120+
- name: Create release
121+
uses: softprops/action-gh-release@v2
122+
with:
123+
token: ${{ secrets.GITHUB_TOKEN }}
124+
125+
# ❌ Unnecessarily broad
126+
env:
127+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
128+
```
129+
130+
- Personal Access Tokens (PATs, stored as repository secrets) should have the minimum required scopes and should be rotated periodically
131+
132+
### Script Injection
133+
134+
`${{ }}` expressions are evaluated before the shell script runs. If an expression comes from untrusted input (PR titles, issue bodies, branch names from forks), it can inject arbitrary shell commands.
135+
136+
**Never** interpolate `github.event.*` values directly into a `run:` step:
137+
138+
```yaml
139+
# ❌ Injection risk — PR title is attacker-controlled
140+
- run: echo "${{ github.event.pull_request.title }}"
141+
142+
# ✅ Safe — value passed through an environment variable
143+
- env:
144+
PR_TITLE: ${{ github.event.pull_request.title }}
145+
run: echo "$PR_TITLE"
146+
```
147+
148+
This rule applies to any value that originates outside the repository (issue bodies, labels, comments, commit messages from forks).
149+
150+
### Pull Request Workflows
151+
152+
- Workflows triggered by `pull_request` from a fork run with **read-only** token permissions and no access to repository secrets — this is intentional and correct
153+
- Do not use `pull_request_target` unless you fully understand the security implications; it runs in the context of the base branch and *does* have secret access, making it a common attack surface

0 commit comments

Comments
 (0)