Rules: never panic — Result<T, E> everywhere, no unchecked indexing / zero division on untrusted input, background threads catch panics, fuzz parsers (AGENTS.md §3.1, CLAUDE.md, contributing)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas 2026-10-04 08:13:08 -04:00
parent 0fb2ba99ca
commit f3f603a934
3 changed files with 39 additions and 0 deletions

View File

@ -67,6 +67,35 @@ reproduce an image (for example, point lists traced from someone else's icon).
## 3. Engineering rules ## 3. Engineering rules
### 3.1 Never panic: fail with `Result`
FilmCraft must not crash. A user losing unsaved work, or an agent's session dying, because of a bad file
or an unexpected input is a bug as serious as wrong output.
1. **No panics in production code.** Don't use `unwrap()`, `expect()`, `panic!`, `unreachable!`,
`todo!` or `unimplemented!` outside tests. Return `Result<T, E>` with the crate's error type and
propagate with `?`; in the UI, report the error (status bar, error dialog) and carry on.
2. **Avoid implicit panics too.** Index and slice with `get()` (or validate the bounds once, up front,
where a hot loop needs plain indexing); use `checked_*` / `saturating_*` arithmetic where values come
from outside; never divide by a value that can be zero (frame rates, timescales, sample rates, sizes);
don't call `clamp` with bounds that can cross; bound recursion and loop counts.
3. **All input is untrusted:** media files, project and interchange files, presets, fonts, command
parameters from the CLI / MCP / control channel, and UI state. A malformed or hostile input must give an
error, never a panic, a hang, or an allocation sized by the input without a sane limit.
4. **Background work catches panics.** Every thread or job (export, previews, proxies, decoding workers…)
runs its body under `catch_unwind` and reports a failure as an error: a dead worker leaves monitors
blank or jobs "running" forever.
5. **Lock poisoning is not fatal:** use `lock().unwrap_or_else(|e| e.into_inner())`.
6. **Enforced:** crates deny `clippy::unwrap_used`, `clippy::expect_used`, `clippy::panic` and
`clippy::unreachable` outside tests. Where an invariant truly cannot be violated and restructuring is
unreasonable, a single-item `#[allow(...)]` with a one-line justification comment is acceptable; keep
these rare and reviewable.
7. **Test the failure paths:** parsers and decoders get mutation-fuzz tests (truncation, bit flips,
corrupt sizes) run under `catch_unwind`; new commands get hostile-parameter tests. Fix every panic a
fuzzer finds and keep its reproducer as a test.
### 3.2 The rest
See `CLAUDE.md`: pure Rust, dependency layering (`cargo xtask layers`), exact `Tick` time, everything is See `CLAUDE.md`: pure Rust, dependency layering (`cargo xtask layers`), exact `Tick` time, everything is
a command, everything is agent-drivable, and the quality gates (`cargo xtask ci`) before every commit. a command, everything is agent-drivable, and the quality gates (`cargo xtask ci`) before every commit.

View File

@ -13,6 +13,7 @@ FilmCraft is a clean-room, open-source, pure-Rust non-linear video editor target
- **Clean-room.** Never read/disassemble anything inside Adobe app bundles (names/listings only). Never copy Adobe icons, presets, LUTs, fonts. Never copy GPL/LGPL/AGPL code (FFmpeg, x264, x265, MLT, Kdenlive, Shotcut, Olive, LAME…). ffmpeg/ffprobe run only as external test oracles / fixture generators — never linked or shipped. - **Clean-room.** Never read/disassemble anything inside Adobe app bundles (names/listings only). Never copy Adobe icons, presets, LUTs, fonts. Never copy GPL/LGPL/AGPL code (FFmpeg, x264, x265, MLT, Kdenlive, Shotcut, Olive, LAME…). ffmpeg/ffprobe run only as external test oracles / fixture generators — never linked or shipped.
- **Pure Rust** in the product. OS media APIs only via Rust bindings in `crates/platform` (planned; not created yet), optional, behind traits. - **Pure Rust** in the product. OS media APIs only via Rust bindings in `crates/platform` (planned; not created yet), optional, behind traits.
- **Layering** ([`docs/architecture.md`](docs/architecture.md) §1, enforced by `cargo xtask layers`): nothing below L5 depends on egui/eframe/winit/rfd/cpal. L0 codec/container crates depend only on `filmcraft-bitstream`. - **Layering** ([`docs/architecture.md`](docs/architecture.md) §1, enforced by `cargo xtask layers`): nothing below L5 depends on egui/eframe/winit/rfd/cpal. L0 codec/container crates depend only on `filmcraft-bitstream`.
- **Never panic** ([`AGENTS.md`](AGENTS.md) §3.1): no `unwrap`/`expect`/`panic!`/`unreachable!` outside tests; return `Result<T, E>` and propagate with `?`; no unchecked indexing, division by possibly-zero values or input-sized allocations on untrusted data (media, project files, command params); background threads catch panics and report errors. Parsers and decoders get mutation-fuzz tests.
- **Exact time:** all time is `filmcraft_time::Tick` (254 016 000 000/s). Never use f64 seconds for edit math. - **Exact time:** all time is `filmcraft_time::Tick` (254 016 000 000/s). Never use f64 seconds for edit math.
- **Everything is a command** (`crates/engine`): id, label, menu path, shortcut, params, enabled(), run(). UI, CLI, control channel and MCP all dispatch by id. - **Everything is a command** (`crates/engine`): id, label, menu path, shortcut, params, enabled(), run(). UI, CLI, control channel and MCP all dispatch by id.
- **Everything is agent-drivable:** every interactive widget registers an automation id; UI state is serde so the control channel can read/set it. - **Everything is agent-drivable:** every interactive widget registers an automation id; UI state is serde so the control channel can read/set it.

View File

@ -58,6 +58,15 @@ Every commit must pass all of these:
`cargo xtask` is an alias in `.cargo/config.toml` for `cargo run -p xtask --`. `cargo xtask` is an alias in `.cargo/config.toml` for `cargo run -p xtask --`.
### No panics
Production code must never panic (rules in [AGENTS.md](../AGENTS.md) §3.1). In short: return
`Result<T, E>` instead of `unwrap()` / `expect()` / `panic!` / `unreachable!`; use `get()`,
`checked_*` / `saturating_*` and validated bounds on anything that came from a file, a command parameter
or the UI; catch panics in background threads and report them as errors. Clippy denies `unwrap_used`,
`expect_used`, `panic` and `unreachable` outside tests. Parsers, decoders and commands get fuzz /
hostile-input tests run under `catch_unwind`.
## 4. Commits ## 4. Commits
- One task per commit. The subject starts with the task id: `M3.2: trim mode engine — …`, - One task per commit. The subject starts with the task id: `M3.2: trim mode engine — …`,