Rules: never crash — AGENTS.md §0, clippy.toml for tests, forbid unsafe
- AGENTS.md: a new §0 "Never crash" at the top of the rules (why, outranks feature work, the bans, Result/? patterns, hostile input numbers, bounded recursion, lock poisoning, the last-resort guard, a regression test per crash fix), linking craftrules/standards/never-crash.md. §3.1 now points to it. - CLAUDE.md non-negotiables, CONTRIBUTING.md and docs/contributing.md: short Never crash notes pointing to AGENTS.md §0. - clippy.toml: allow unwrap/expect/panic/indexing in tests, so crates can deny the panicking lints in production code without touching their tests. - [workspace.lints.rust] unsafe_code = "forbid" (no unsafe or allow(unsafe_code) in the workspace). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c1224d0ede
commit
e10449de2c
6 files changed
+60
-36
No files matched your search
@@ -3,6 +3,44 @@
|
|||||||
These rules apply to every human and AI contributor. `CLAUDE.md` holds the working instructions; this
|
These rules apply to every human and AI contributor. `CLAUDE.md` holds the working instructions; this
|
||||||
file holds the rules that must never be broken. When the two disagree, this file wins.
|
file holds the rules that must never be broken. When the two disagree, this file wins.
|
||||||
|
|
||||||
|
## 0. Never crash
|
||||||
|
|
||||||
|
People trust FilmCraft with hours of work. A malformed file, a bad command or MCP parameter, a corrupt
|
||||||
|
preset or project, an odd keystroke or a full disk must give an error the user (or agent) can act on,
|
||||||
|
never a crash and never lost work. **This rule outranks feature work:** don't ship a feature by adding a
|
||||||
|
panic path, and fix a crash before building on top of it. The cross-app standard is
|
||||||
|
[`craftrules/standards/never-crash.md`](https://github.com/storytold/craftrules/blob/main/standards/never-crash.md).
|
||||||
|
|
||||||
|
1. **Fail with `Result<T, E>`.** Return the crate's error type and propagate with `?`; add context
|
||||||
|
(`map_err`, an error variant) instead of discarding it. In the UI, report the error (status bar, error
|
||||||
|
dialog) and carry on.
|
||||||
|
2. **No panicking shortcuts outside tests:** no `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!`
|
||||||
|
or `unimplemented!`. Use `?`, `ok_or(..)?`, `let Some(x) = .. else { return Err(..) }`, `if let`, or a
|
||||||
|
fallback that is truly correct (never one that silently corrupts a project). An unfinished feature
|
||||||
|
returns an "unsupported" error. The only exception is a call that is provably infallible from local
|
||||||
|
code alone (a literal that always parses): a single-item `#[allow(clippy::expect_used)]` with the reason.
|
||||||
|
3. **No `unsafe`** (`unsafe_code = "forbid"` for the workspace).
|
||||||
|
4. **Every input-derived number is hostile.** Media files, project and interchange files, presets, fonts,
|
||||||
|
CLI / MCP / control-channel parameters and UI state are untrusted. Index and slice with `get()` (or
|
||||||
|
validate bounds once, up front, for a hot loop); slice strings only at char boundaries; use
|
||||||
|
`checked_*` / `saturating_*` for lengths, offsets and counts; never divide by a value that can be zero
|
||||||
|
(frame rates, timescales, sample rates, sizes); don't cast negative or NaN values to integers; don't
|
||||||
|
call `clamp` with bounds that can cross; cap allocations sized by input.
|
||||||
|
5. **Bound recursion and loops.** Projects can be cyclic (nested sequences) or deeply nested: walk them
|
||||||
|
with seen-sets or depth limits.
|
||||||
|
6. **Don't cascade.** Lock poisoning is not fatal: `lock().unwrap_or_else(PoisonError::into_inner)`.
|
||||||
|
Every thread or job (export, previews, proxies, decoding workers…) runs under `catch_unwind` and
|
||||||
|
reports a failure as an error: a dead worker leaves monitors blank or jobs "running" forever.
|
||||||
|
7. **Last-resort guard.** The panic hook (`crates/ui-egui/src/crash.rs`) logs every panic, and the UI pass
|
||||||
|
runs under `catch_unwind`, so an escaped panic becomes an error window and the session (and unsaved
|
||||||
|
project) survives. It is a safety net, not a substitute for rules 1–6. Keep `panic = "unwind"`.
|
||||||
|
8. **Prove it.** Every crash fix comes with a small synthetic regression test that panicked before the
|
||||||
|
fix. Parsers and decoders get mutation-fuzz tests (truncation, bit flips, corrupt sizes) run under
|
||||||
|
`catch_unwind`; new commands get hostile-parameter tests. Fuzzer findings become tests.
|
||||||
|
9. **Enforced:** clean crates carry `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic,
|
||||||
|
clippy::unimplemented, clippy::todo, clippy::unreachable)]` (moving to `[workspace.lints.clippy]` once
|
||||||
|
every crate is clean). `clippy.toml` allows them in tests: a failing test should fail loudly.
|
||||||
|
|
||||||
## 1. Assets: no Adobe artwork, every asset licensed and attributed
|
## 1. Assets: no Adobe artwork, every asset licensed and attributed
|
||||||
|
|
||||||
This rule is absolute. Breaking it is the most serious mistake a contributor can make on this project.
|
This rule is absolute. Breaking it is the most serious mistake a contributor can make on this project.
|
||||||
@@ -68,32 +106,9 @@ 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`
|
### 3.1 Never crash
|
||||||
|
|
||||||
FilmCraft must not crash. A user losing unsaved work, or an agent's session dying, because of a bad file
|
See [§0](#0-never-crash). It outranks every other engineering rule.
|
||||||
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
|
### 3.2 The rest
|
||||||
|
|
||||||
|
|||||||
@@ -9,11 +9,11 @@ FilmCraft is a clean-room, open-source, pure-Rust non-linear video editor target
|
|||||||
3. Follow the autonomous operation protocol (`plan/execution-plan.md` §7; public summary: [`docs/agents.md`](docs/agents.md) §5–7). Don't stop to ask unless it lists the decision as the user's.
|
3. Follow the autonomous operation protocol (`plan/execution-plan.md` §7; public summary: [`docs/agents.md`](docs/agents.md) §5–7). Don't stop to ask unless it lists the decision as the user's.
|
||||||
|
|
||||||
## Non-negotiables
|
## Non-negotiables
|
||||||
**Read [`AGENTS.md`](AGENTS.md) first; its rules override everything here.** In particular (§1): never use Adobe iconography, images or any other Adobe asset. Every asset must be openly licensed (OSS / public domain / Creative Commons, or original work by a contributor) and must have a `<file>.attribution` sidecar plus an entry in `ATTRIBUTION.md`. `cargo xtask assets` enforces this.
|
**Read [`AGENTS.md`](AGENTS.md) first; its rules override everything here.**
|
||||||
|
- **Never crash** ([`AGENTS.md`](AGENTS.md) §0, standard: [`craftrules/standards/never-crash.md`](https://github.com/storytold/craftrules/blob/main/standards/never-crash.md)). People depend on FilmCraft; a crash loses their work, so this outranks feature work. No `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!`/`unimplemented!` or `unsafe` outside tests: return `Result<T, E>` and propagate with `?` (`ok_or`, `let … else`, `if let`). Treat every number from media, project files, presets or CLI/MCP/control params as hostile: `get()` instead of `[i]`, checked/saturating math, no division by possibly-zero values, capped allocations, bounded recursion. `lock().unwrap_or_else(PoisonError::into_inner)`; background jobs run under `catch_unwind`. The panic hook + UI `catch_unwind` (`crates/ui-egui/src/crash.rs`) are the last-resort guard, not a licence. Every crash fix gets a regression test that panicked before. In particular (§1): never use Adobe iconography, images or any other Adobe asset. Every asset must be openly licensed (OSS / public domain / Creative Commons, or original work by a contributor) and must have a `<file>.attribution` sidecar plus an entry in `ATTRIBUTION.md`. `cargo xtask assets` enforces this.
|
||||||
- **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.
|
||||||
|
|||||||
+4
-1
@@ -5,7 +5,10 @@ Thanks for helping. Before you start:
|
|||||||
1. Read **[AGENTS.md](AGENTS.md)**. Its rules are absolute. In particular: no Adobe assets of any
|
1. Read **[AGENTS.md](AGENTS.md)**. Its rules are absolute. In particular: no Adobe assets of any
|
||||||
kind, every asset openly licensed with an attribution sidecar, no GPL/LGPL code, and ffmpeg only
|
kind, every asset openly licensed with an attribution sidecar, no GPL/LGPL code, and ffmpeg only
|
||||||
as an external test oracle.
|
as an external test oracle.
|
||||||
2. Follow **[docs/contributing.md](docs/contributing.md)**: setup per OS, building and running,
|
2. **Never crash.** A crash loses someone's work, so this rule outranks feature work: no `unwrap()`,
|
||||||
|
`expect()`, `panic!`, `unreachable!`, `todo!`, `unimplemented!` or `unsafe` in production code;
|
||||||
|
return `Result` and treat every input as hostile. See [AGENTS.md](AGENTS.md) §0.
|
||||||
|
3. Follow **[docs/contributing.md](docs/contributing.md)**: setup per OS, building and running,
|
||||||
the quality gates (`cargo xtask ci`), commit conventions, and how to add commands, effects,
|
the quality gates (`cargo xtask ci`), commit conventions, and how to add commands, effects,
|
||||||
transitions, codecs, panels and assets.
|
transitions, codecs, panels and assets.
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -69,7 +69,7 @@ egui_kittest = "0.36"
|
|||||||
rfd = "0.17"
|
rfd = "0.17"
|
||||||
|
|
||||||
[workspace.lints.rust]
|
[workspace.lints.rust]
|
||||||
unsafe_code = "deny"
|
unsafe_code = "forbid"
|
||||||
|
|
||||||
[workspace.lints.clippy]
|
[workspace.lints.clippy]
|
||||||
dbg_macro = "warn"
|
dbg_macro = "warn"
|
||||||
|
|||||||
@@ -0,0 +1,4 @@
|
|||||||
|
allow-unwrap-in-tests = true
|
||||||
|
allow-expect-in-tests = true
|
||||||
|
allow-panic-in-tests = true
|
||||||
|
allow-indexing-slicing-in-tests = true
|
||||||
@@ -58,14 +58,16 @@ 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
|
### Never crash
|
||||||
|
|
||||||
Production code must never panic (rules in [AGENTS.md](../AGENTS.md) §3.1). In short: return
|
Production code must never crash: a crash loses someone's work, so this outranks feature work. The rules
|
||||||
`Result<T, E>` instead of `unwrap()` / `expect()` / `panic!` / `unreachable!`; use `get()`,
|
are in [AGENTS.md](../AGENTS.md) §0. In short: return `Result<T, E>` instead of `unwrap()` / `expect()` /
|
||||||
`checked_*` / `saturating_*` and validated bounds on anything that came from a file, a command parameter
|
`panic!` / `unreachable!` / `todo!` / `unimplemented!`; no `unsafe`; use `get()`, `checked_*` /
|
||||||
or the UI; catch panics in background threads and report them as errors. Clippy denies `unwrap_used`,
|
`saturating_*` and validated bounds on anything that came from a file, a command parameter or the UI;
|
||||||
`expect_used`, `panic` and `unreachable` outside tests. Parsers, decoders and commands get fuzz /
|
bound recursion; tolerate lock poisoning; catch panics in background threads and report them as errors.
|
||||||
hostile-input tests run under `catch_unwind`.
|
Clean crates deny the panicking lints outside tests (`clippy.toml` allows them in tests). Every crash fix
|
||||||
|
comes with a regression test; parsers, decoders and commands get fuzz / hostile-input tests run under
|
||||||
|
`catch_unwind`.
|
||||||
|
|
||||||
## 4. Commits
|
## 4. Commits
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user