|
| 1 | +# AGENTS.md — odp-embedded-controller |
| 2 | + |
| 3 | +Operational guide for AI coding agents working in this repository. This |
| 4 | +file complements `.github/copilot-instructions.md` (deep |
| 5 | +architecture/conventions) with the *workflow* an agent must follow: |
| 6 | +build, lint, test, commit, and push. |
| 7 | + |
| 8 | +If you are a human contributor, read `README.md` and |
| 9 | +`.github/copilot-instructions.md` first; this file repeats their |
| 10 | +essentials so an autonomous agent has a single entry point. |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## 1. Repository shape |
| 15 | + |
| 16 | +- Language: Rust, `#![no_std]` / `#![no_main]` embedded firmware. |
| 17 | +- **No workspace root.** Each platform under `platform/` is a |
| 18 | + standalone crate built from its own directory. `cargo` commands |
| 19 | + *must* be run inside `platform/<crate>/` so the per-crate |
| 20 | + `.cargo/config.toml` (target triple, linker, runner) is honoured. |
| 21 | +- Crates: |
| 22 | + - `platform/platform-common` — shared `no_std` library (no build |
| 23 | + target; fmt/clippy/check only). |
| 24 | + - `platform/dev-imxrt` — NXP i.MXRT685S, `thumbv8m.main-none-eabihf` |
| 25 | + (requires `flip-link`). |
| 26 | + - `platform/dev-mcxa` — NXP MCXA266, `thumbv8m.main-none-eabihf` |
| 27 | + (requires `flip-link`). |
| 28 | + - `platform/dev-npcx` — Nuvoton NPCX498M, `thumbv7em-none-eabihf` |
| 29 | + (requires `flip-link`). |
| 30 | + - `platform/dev-qemu` — QEMU `virt`, `riscv32imac-unknown-none-elf` |
| 31 | + (no `flip-link`). |
| 32 | +- Toolchain pinned in `rust-toolchain.toml` (stable + all three |
| 33 | + targets + `rust-src`, `rustfmt`, `clippy`, `llvm-tools-preview`). |
| 34 | +- MSRV: `1.83`. |
| 35 | + |
| 36 | +## 2. Setup |
| 37 | + |
| 38 | +One-time host setup (only what isn't auto-installed by `rustup`): |
| 39 | + |
| 40 | +```sh |
| 41 | +cargo install flip-link --locked # required by dev-imxrt / dev-mcxa / dev-npcx |
| 42 | +cargo install --locked cargo-deny # optional but used by check-all.sh |
| 43 | +``` |
| 44 | + |
| 45 | +Targets and components install automatically the first time `cargo` is |
| 46 | +invoked inside the repo. |
| 47 | + |
| 48 | +## 3. Build / lint / test commands |
| 49 | + |
| 50 | +There are **no unit or integration tests in-tree**. Validation is: |
| 51 | +formatting, clippy, build, doc, feature-powerset check, cargo-deny, |
| 52 | +cargo-machete, no_std check, and an out-of-tree QEMU integration test |
| 53 | +driven by `scripts/integration-test.sh`. |
| 54 | + |
| 55 | +### 3.1 Per-crate gates (run from `platform/<crate>/`) |
| 56 | + |
| 57 | +| Gate | Command | Crates | |
| 58 | +|-------------------|----------------------------------------------------------|-----------------------------------------------| |
| 59 | +| Format | `cargo fmt --check` | `platform-common`, all `dev-*` | |
| 60 | +| Build | `cargo build --locked` | all `dev-*` | |
| 61 | +| Clippy | `cargo clippy --locked -- -D warnings` | all `dev-*` (CI uses `cargo clippy --locked`) | |
| 62 | +| Doc | `RUSTDOCFLAGS=--cfg docsrs cargo doc --locked --no-deps --all-features` | all `dev-*` | |
| 63 | +| Feature powerset | `cargo hack --locked --feature-powerset check` | all `dev-*` | |
| 64 | +| Dependency policy | `cargo deny --locked --all-features check` | all `dev-*` | |
| 65 | +| Unused deps | `cargo machete` | all `dev-*` | |
| 66 | +| no_std check | `cargo check --locked --all-features` | all `dev-*` | |
| 67 | +| MSRV check | `cargo +1.83 check --locked` | all `dev-*` | |
| 68 | + |
| 69 | +`cargo clippy --locked` on its own is what CI runs (`check.yml`); the |
| 70 | +local `-- -D warnings` form mirrors `scripts/check-all.sh` and is the |
| 71 | +stricter gate to use locally because the crates set |
| 72 | +`warnings = "deny"` in `[lints.rust]` anyway. |
| 73 | + |
| 74 | +### 3.2 Full local quality gate (recommended before every push) |
| 75 | + |
| 76 | +```sh |
| 77 | +bash scripts/check-all.sh |
| 78 | +``` |
| 79 | + |
| 80 | +Runs `fmt → build → clippy -D warnings → cargo deny` across |
| 81 | +`platform-common` and every `dev-*`, mirroring `.github/workflows/check.yml`. |
| 82 | +Skips `cargo-deny` with a warning if it isn't installed; warns (but |
| 83 | +proceeds) if `flip-link` is missing. |
| 84 | + |
| 85 | +### 3.3 QEMU integration test |
| 86 | + |
| 87 | +```sh |
| 88 | +./scripts/integration-test.sh |
| 89 | +``` |
| 90 | + |
| 91 | +Requires `qemu-system-riscv32` (Debian/Ubuntu: `qemu-system-misc`), |
| 92 | +`libudev-dev`, and the pinned `ec-test-cli` binary |
| 93 | +(`cargo install --git https://github.com/OpenDevicePartnership/odp-platform-common --locked --rev <pin from check.yml> ec-test-cli`). |
| 94 | +Runs `dev-qemu` and exercises every `ec-test-cli` command against it. |
| 95 | + |
| 96 | +### 3.4 Common pitfalls |
| 97 | + |
| 98 | +- Running `cargo` from the repo root with `--manifest-path` bypasses |
| 99 | + the per-platform `.cargo/config.toml` and fails (notably `dev-qemu` |
| 100 | + cannot find its target). Always `cd platform/<crate>/` first. |
| 101 | +- `linker 'flip-link' not found` → install `flip-link`. |
| 102 | +- `can't find crate for 'core'` → toolchain targets weren't installed; |
| 103 | + run `rustup show` from repo root, or `rustup target add <triple>`. |
| 104 | + |
| 105 | +## 4. Code conventions (agent-relevant subset) |
| 106 | + |
| 107 | +See `.github/copilot-instructions.md` for the full version. Key rules |
| 108 | +an agent must respect: |
| 109 | + |
| 110 | +- **Static allocation only.** No heap, no `alloc`. Use `StaticCell<T>` |
| 111 | + for owned init-once values and `OnceLock<T>` for lazily shared |
| 112 | + references. |
| 113 | +- **Embassy async executor.** Entry point is |
| 114 | + `#[embassy_executor::main] async fn main(spawner: Spawner)`. All |
| 115 | + concurrency is cooperative async tasks. |
| 116 | +- **Inter-task comms** go through a typed |
| 117 | + `PubSubChannel<ThreadModeRawMutex, Message, ...>` per platform. |
| 118 | +- **Shared I2C** via `Mutex<ThreadModeRawMutex, I2cMaster<'static, Async>>` |
| 119 | + inside a `StaticCell`, accessed through `I2cDevice::new(...)`. |
| 120 | +- **Logging:** `defmt` only, transported over RTT. Derive |
| 121 | + `defmt::Format` on logged types. |
| 122 | +- **Panic handlers:** `panic_halt` in release, `panic_probe` in debug |
| 123 | + (gated on `debug_assertions`). |
| 124 | +- **Errors:** `.expect("descriptive message")` for init paths that |
| 125 | + must succeed; `Result<_, _>` for recoverable runtime ops; minimal |
| 126 | + custom error enums. |
| 127 | +- **Lints:** `warnings = "deny"` plus |
| 128 | + `correctness/perf/suspicious/style = "deny"` for clippy in every |
| 129 | + crate. Do not weaken these. |
| 130 | +- **rustfmt:** `max_width = 120` (see `rustfmt.toml`). Always |
| 131 | + `cargo fmt` before committing. |
| 132 | +- **Module layout:** subsystems are directory modules with `mod.rs` |
| 133 | + re-exporting children. |
| 134 | + |
| 135 | +When adding a new platform, mirror the matrix entries in |
| 136 | +`.github/workflows/check.yml`, `nostd.yml`, and `scripts/check-all.sh` |
| 137 | +(the workflow comment explicitly calls this out — there is no single |
| 138 | +source of truth for the platform list). |
| 139 | + |
| 140 | +## 5. Commit & PR workflow |
| 141 | + |
| 142 | +### 5.1 Commit messages |
| 143 | + |
| 144 | +- Subject line: capitalised, ≤ 50 chars, imperative mood ("Add foo", |
| 145 | + not "Added foo" / "Adds foo"). |
| 146 | +- Blank line, then body wrapped at 72 chars explaining *what* and |
| 147 | + *why*, not *how*. |
| 148 | +- Reference issues/PRs in the body where relevant. |
| 149 | + |
| 150 | +### 5.2 AI attribution (mandatory for any AI-assisted commit) |
| 151 | + |
| 152 | +Every commit produced with AI assistance **must** carry an |
| 153 | +`Assisted-by` trailer: |
| 154 | + |
| 155 | +``` |
| 156 | +Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2] |
| 157 | +``` |
| 158 | + |
| 159 | +- `AGENT_NAME` — e.g. `GitHub Copilot`. |
| 160 | +- `MODEL_VERSION` — the *actual* model the agent is running as. The |
| 161 | + agent **must verify its own identity** before composing the |
| 162 | + trailer; never hard-code a previous session's model. |
| 163 | +- Optional bracketed tools: specialised analysis tools used |
| 164 | + (`coccinelle`, `clang-tidy`, …). Do **not** list editors, `git`, or |
| 165 | + `cargo`. |
| 166 | +- Agents **must not** add `Signed-off-by`. Only humans can certify |
| 167 | + the DCO. |
| 168 | + |
| 169 | +### 5.3 Author identity (agent runs) |
| 170 | + |
| 171 | +When committing on behalf of a human, set author per-commit; never |
| 172 | +mutate global git config: |
| 173 | + |
| 174 | +```sh |
| 175 | +git -c user.name="Felipe Balbi" \ |
| 176 | + -c user.email="felipe.balbi@microsoft.com" \ |
| 177 | + commit -m "..." -m "Assisted-by: GitHub Copilot:<model>" |
| 178 | +``` |
| 179 | + |
| 180 | +### 5.4 Branch / push policy |
| 181 | + |
| 182 | +- Work on a topic branch off `main` (e.g. |
| 183 | + `improve-agentic-workflow`). Never commit directly to `main`. |
| 184 | +- Push to the contributor's **fork** remote, not `origin`. Never |
| 185 | + force-push to a shared branch. |
| 186 | +- Open PRs against `OpenDevicePartnership/odp-embedded-controller` |
| 187 | + `main`. Code-review ownership is in `CODEOWNERS`. |
| 188 | + |
| 189 | +## 6. Continuous Integration |
| 190 | + |
| 191 | +Workflows in `.github/workflows/`: |
| 192 | + |
| 193 | +- `check.yml` — per-platform fmt, clippy, doc, cargo-hack, cargo-deny, |
| 194 | + cargo-machete, MSRV, build, and the `dev-qemu` integration-test job. |
| 195 | +- `nostd.yml` — `cargo check --locked --all-features` per platform |
| 196 | + to keep everything `no_std`-clean. |
| 197 | +- `benchmark.yml` — binsize tracking (`dev-imxrt` / `dev-npcx`). |
| 198 | +- `rolling.yml` — nightly dependency-update verification. |
| 199 | +- `cargo-vet.yml`, `cargo-vet-pr-comment.yml` — supply-chain audit. |
| 200 | + |
| 201 | +`scripts/check-all.sh` is the local mirror of `check.yml`. Run it (or |
| 202 | +the equivalent per-crate commands above) before pushing. |
| 203 | + |
| 204 | +## 7. Files & directories of interest |
| 205 | + |
| 206 | +- `platform/<crate>/Cargo.toml` — pinned deps, lint config, MSRV. |
| 207 | +- `platform/<crate>/.cargo/config.toml` — target, linker (`flip-link` |
| 208 | + where applicable), runner (`probe-rs` / `qemu-system-riscv32`). |
| 209 | +- `platform/<crate>/src/main.rs` — entry point and task spawn graph. |
| 210 | +- `platform/platform-common/src/` — shared HAL/service abstractions. |
| 211 | +- `deny.toml` — license / source / advisory policy. |
| 212 | +- `rustfmt.toml` — formatter config. |
| 213 | +- `rust-toolchain.toml` — pinned toolchain + targets + components. |
| 214 | +- `CODEOWNERS` — review routing. |
| 215 | +- `.github/copilot-instructions.md` — deep convention reference. |
| 216 | + |
| 217 | +## Model selection & cost discipline |
| 218 | + |
| 219 | +Premium models (Opus, GPT-5 family, "high"/"xhigh" reasoning variants) |
| 220 | +cost an order of magnitude more than standard models (Sonnet, Haiku, |
| 221 | +mini). Most steps in a typical task do not need premium reasoning, |
| 222 | +and over-using premium models wastes credits without improving |
| 223 | +outcomes. The rules below apply to *all* model selection: your own |
| 224 | +session, sub-agents launched via the `task` tool, and parallel work |
| 225 | +launched via `/fleet`. |
| 226 | + |
| 227 | +### Default posture |
| 228 | + |
| 229 | +- **Default to the cheapest model that can do the job.** Reach for a |
| 230 | + premium model only when one of the escalation triggers below is hit. |
| 231 | +- **Plan with premium, execute with cheap.** Spend at most one or two |
| 232 | + premium turns on design / planning, then downshift to a cheaper |
| 233 | + model for mechanical execution of the plan. |
| 234 | +- **Never bump the model "just in case."** If you cannot articulate |
| 235 | + *why* a cheaper model would fail, use the cheaper model. |
| 236 | + |
| 237 | +### Escalation triggers (use a premium model) |
| 238 | + |
| 239 | +Reach for a premium model when *any* of these are true: |
| 240 | + |
| 241 | +- Cross-module refactor, architectural design, or API design from |
| 242 | + scratch. |
| 243 | +- Subtle correctness reasoning: concurrency, lifetimes, `unsafe`, |
| 244 | + FFI ABI, cryptography, safety-critical control paths. |
| 245 | +- Debugging a failure that survived one prior cheap-model attempt. |
| 246 | +- Reviewing code on a safety-, security-, or money-critical path. |
| 247 | +- The diff cannot be predicted in advance — i.e. there is genuine |
| 248 | + creative or design work to do, not just typing. |
| 249 | + |
| 250 | +### De-escalation triggers (use a cheap model) |
| 251 | + |
| 252 | +Use the cheapest available model when *any* of these are true: |
| 253 | + |
| 254 | +- Searching, reading, summarising files or docs. |
| 255 | +- Single-file mechanical edits: rename, format, lint fix, dependency |
| 256 | + bump, boilerplate, scaffolding from a known template. |
| 257 | +- Generating tests for code that already works. |
| 258 | +- Running builds, tests, linters, or other commands where the model |
| 259 | + only needs to report success/failure. |
| 260 | +- Routine commits, PR descriptions, changelog entries. |
| 261 | +- The diff is essentially predictable before generation. |
| 262 | + |
| 263 | +### Sub-agent routing (the `task` tool) |
| 264 | + |
| 265 | +When delegating with the `task` tool, set `model:` explicitly. Do not |
| 266 | +let sub-agents inherit a premium default for cheap work. |
| 267 | + |
| 268 | +| Sub-agent type | Default model | Override to | |
| 269 | +|-------------------|---------------------------|-------------------------------------------------| |
| 270 | +| `explore` | cheap | keep cheap (`claude-haiku-4.5` or `gpt-5-mini`) | |
| 271 | +| `task` (run cmd) | cheap | keep cheap | |
| 272 | +| `research` | cheap for breadth | premium only for the final synthesis | |
| 273 | +| `general-purpose` | match task | cheap for mechanical work; premium for design | |
| 274 | +| `rubber-duck` | premium | keep premium — this is where reasoning pays off | |
| 275 | +| `code-review` | premium on critical paths | cheap on cosmetic / mechanical diffs | |
| 276 | + |
| 277 | +### `/fleet` (parallel sub-agents) rules |
| 278 | + |
| 279 | +- Fleet mode multiplies cost by the fleet width. Apply the rules |
| 280 | + above *per worker*, not in aggregate. |
| 281 | +- Split a fleet job along complexity lines: route the cheap, |
| 282 | + parallelisable workers (file edits, test runs, doc updates) to a |
| 283 | + cheap model; reserve premium models for the small number of |
| 284 | + workers that need real reasoning. |
| 285 | +- If every worker in a fleet would need a premium model, the work is |
| 286 | + probably not a good fit for fleet mode — reconsider the |
| 287 | + decomposition before paying N× premium. |
| 288 | + |
| 289 | +### Session hygiene |
| 290 | + |
| 291 | +- Keep sessions short and focused. Long premium sessions are the |
| 292 | + single largest source of waste because every turn re-processes the |
| 293 | + full history. |
| 294 | +- Use `/compact` when the conversation grows long, and `/new` for |
| 295 | + unrelated work. |
| 296 | +- Prefer `/ask` for one-off side questions so they don't extend the |
| 297 | + main session. |
| 298 | + |
| 299 | +### When in doubt |
| 300 | + |
| 301 | +Ask: *"If a cheaper model produced the wrong answer here, would I |
| 302 | +catch it in seconds (compiler, tests, my own review) or in |
| 303 | +weeks (production incident)?"* If the former, use the cheap model |
| 304 | +and let the feedback loop do its job. |
0 commit comments