# Rejected Audit Findings
Issues flagged during code audits (rounds 7-12) that were assessed and deliberately rejected. Documented here to prevent re-flagging in future audits.
## Duration u128-to-u64 overflow in JSON output
**File:** `src/json_output.rs` (lines 130, 151)
**Flagged in:** rounds 9, 10, 11, 12
`Duration::as_millis()` returns `u128`, cast to `u64` without bounds checking. Overflows after ~584 million years. No real build will ever hit this. Not fixing.
## Pre-1970 mtime cache collision
**File:** `src/object_store/checksums.rs` (lines 25-27)
**Flagged in:** rounds 9, 10, 11, 12
Files with mtime before Unix epoch (1970) get `unwrap_or_default()` mapping to `(0, 0)`. Two such files could share a cached mtime entry. Pre-1970 timestamps don't occur on real build inputs. The mtime cache is only an optimization — the actual input checksum comparison catches real changes. Not fixing.
## Dependency unchanged logic — no-dep products
**File:** `src/executor/execution.rs` (line 587)
**Flagged in:** round 9
Agent claimed `!deps.is_empty() && deps.iter().all(...)` should be `deps.is_empty() || deps.iter().all(...)`. Wrong — products with no dependencies should NOT reuse cached checksums. The optimization is specifically for products whose upstream deps produced identical output, meaning transitive inputs are unchanged. No-dep products have no such guarantee.
## Batch handle_success return value ignored
**File:** `src/executor/execution.rs` (line 339)
**Flagged in:** round 10
`handle_success()` return value is not checked in batch processing. This is correct — `handle_success` already calls `record_failure` internally when caching fails, properly marking the product as failed. In non-batch, the return value triggers a `break` from the retry loop, but batch has no retry loop. Stats are correct either way.
## record_failure ignores mark_processor_failed in keep-going mode
**File:** `src/executor/handlers.rs` (lines 20-39)
**Flagged in:** rounds 11, 12
In keep-going mode, `mark_processor_failed` parameter is ignored. This is by design — `failed_processors` is only checked in non-keep-going mode to skip subsequent products from the same processor. In keep-going mode, all products run regardless, so tracking failed processors is unnecessary.
## Arc reference leak — failed_processors not unwrapped
**File:** `src/executor/execution.rs` (collect_build_stats)
**Flagged in:** round 11
Agent claimed not unwrapping `failed_processors` Arc prevents other `Arc::try_unwrap` calls from succeeding. Wrong — each Arc has its own independent reference count. Not unwrapping one has zero effect on others.
## Tera output paths lose directory structure
**File:** `src/processors/generators/tera.rs` (lines 100-106)
**Flagged in:** round 10
Templates in subdirectories produce output at project root (e.g., `tera.templates/sub/README.md.tera` → `README.md`). This is intentional — the comment on line 105 explicitly says "Output is at project root with the .tera extension stripped." By design.
## Lua stub_path uses suffix as directory name
**File:** `src/processors/lua_processor.rs` (line 126)
**Flagged in:** round 10
`rsconstruct.stub_path(source, suffix)` uses `suffix` to construct the output directory (`out/{suffix}`). This is the designed Lua API — plugins control their own output directory naming via the suffix parameter.
## Lua clean count masking with saturating_sub
**File:** `src/processors/lua_processor.rs` (lines 450-468)
**Flagged in:** rounds 10, 11
Custom Lua clean functions report removal count via `existed_before.saturating_sub(exist_after)`. If the Lua function doesn't remove files, that's the plugin's responsibility. The count accurately reflects what was actually removed. Not a bug.
## file_index src_exclude_dirs substring matching
**File:** `src/file_index.rs` (lines 76-80)
**Flagged in:** rounds 9, 10
`src_exclude_dirs` uses `path_str.contains(dir)` for filtering. The documented convention uses slash-delimited patterns like `"/kernel/"`, which prevents false positives on path substrings. This is the configured behavior.
## Object store trim path reconstruction
**File:** `src/object_store/management.rs` (lines 86-103)
**Flagged in:** round 9
Reconstructing checksums from filesystem paths (`prefix + rest`) during cache trim. The path structure is fixed (`objects/[2-char]/[rest]`), set by `store_object()`. Unexpected files in the objects directory are silently ignored during trim, which is the correct behavior.
## Partial output caching (before the fix)
**File:** `src/object_store/operations.rs` (lines 144-147)
**Flagged in:** round 9
Originally flagged as a design choice. User overruled — missing outputs are now an error (`anyhow::ensure!`). This was **accepted and fixed** in a later commit, not rejected.
## Zspell read-modify-write race
**File:** `src/processors/checkers/zspell.rs` (lines 192-229)
**Flagged in:** round 11
Agent claimed file read-modify-write isn't protected. Wrong — `self.words_to_add.lock()` on line 193 acquires the mutex, which is held for the entire function (not dropped until return). The lock prevents concurrent threads from interleaving. Cross-process races are not a concern for RSConstruct.
## Duplicate dependency edges in resolve_dependencies
**File:** `src/graph.rs` (lines 227-230)
**Flagged in:** round 12
Agent claimed duplicate edges cause incorrect topological sort. The scenario requires a product to list the same input file twice, which doesn't happen — `FileIndex.scan()` returns unique paths. Even if it did, duplicate edges would increment and decrement `in_degree` the same number of times, netting out correctly.
## Python string injection in load_python_config
**File:** `src/processors/generators/tera.rs` (lines 205-208)
**Flagged in:** round 12
Agent claimed newlines in file paths could inject Python code. File paths come from `FileIndex` (filesystem scan) or Tera templates written by the project author — both are trusted input. Linux file paths from filesystem scans don't contain newlines.
## Batch assert_eq should be error return
**File:** `src/executor/execution.rs` (lines 323-325)
**Flagged in:** round 12
Agent suggested replacing `assert_eq!` with `anyhow::bail!` for batch result count validation. The assert is deliberate — a processor returning the wrong number of results is a contract violation (programming error), not a recoverable runtime condition. Assertions are appropriate for invariant violations.
## Platform portability (Windows, macOS)
**Flagged in:** rounds 9, 10, 11, 12
Multiple agents flagged `std::os::unix` usage without `#[cfg(unix)]` guards, and missing `#[cfg(windows)]`/`#[cfg(target_os = "macos")]` blocks. RSConstruct is Linux-only. No platform compatibility code will be added.
## DB recovery — file might not exist
**File:** `src/db.rs`
**Flagged in:** round 12
Agent re-flagged db.rs recovery, claiming `fs::remove_file` could fail if the file doesn't exist. This was already fixed in round 8 — `let _ = fs::remove_file()` was changed to `fs::remove_file()?` which properly propagates errors.