In file crates/tirith-core/src/checkpoint.rs around lines 383-390:

The second loop skips files already in `seen_paths`, so backup integrity isn't validated for deleted/modified files. Consider checking backup existence for all files, or at minimum documenting that backup corruption is only detected for unchanged files.

Evidence trail:
crates/tirith-core/src/checkpoint.rs lines 350-379 (first loop adds deleted/modified files to seen_paths), lines 383-398 (second loop skips seen_paths when checking backup integrity), lines 388-389 (explicit skip: `if seen_paths.contains(&entry.original_path) { continue; }`), lines 310-318 (restore function only prints to stderr and continues if backup missing).

In file crates/tirith-core/src/rules/configfile.rs around line 500:

Suggestion: `extract_host_from_url` mis-parses when userinfo or bracketed IPv6 is present, and keeping `[]` causes `Ipv6Addr::parse` to fail. Consider skipping past userinfo (`@`), treating `[`…`]` as a single host and stripping the brackets, then bounding the host by `/`, `:`, or `?`.

Evidence trail:
crates/tirith-core/src/rules/configfile.rs lines 520-526 (extract_host_from_url function), lines 500-501 (Ipv6Addr::parse call) at REVIEWED_COMMIT. The function uses `find(['/', ':', '?'])` which incorrectly matches colons inside IPv6 brackets and colons in userinfo passwords. No `@` handling for userinfo. Rust std::net::Ipv6Addr does not parse bracketed addresses.

In file crates/tirith-core/src/mcp/dispatcher.rs around line 40:

When draining oversized lines, `read_until` has no size limit, so an attacker can exhaust memory with a newline-free stream. Consider using a bounded read loop (e.g., read fixed chunks until newline or limit reached) instead of unbounded `read_until`.

Evidence trail:
crates/tirith-core/src/mcp/dispatcher.rs lines 22-44 at REVIEWED_COMMIT: Line 40-42 shows `let mut discard = Vec::new(); ... let _ = input.read_until(b'\n', &mut discard);` - this `read_until` reads directly from `input` with no size bound, while the main read path at lines 29-31 uses `.take(MAX_LINE_BYTES as u64 + 1)` to limit reads.

In file crates/tirith-core/src/rules/terminal.rs around line 408:

The `ENTITIES` regex doesn't match hex entities like `A`. Consider adding `&#x[a-fA-F0-9]+;` to the pattern to avoid inflated `visible_len` calculations.

Evidence trail:
crates/tirith-core/src/rules/terminal.rs:408 (REVIEWED_COMMIT) - The ENTITIES regex is `r"&[a-zA-Z]+;|&#\d+;"` which matches named entities and decimal numeric entities but not hexadecimal entities (`&#x[0-9a-fA-F]+;`). HTML spec supports hex numeric character references (e.g., https://www.w3.org/TR/html52/syntax.html#character-references). The function doc comment states it extracts "approximate visible text content" and ENTITIES.replace_all is used to strip entities for length calculation.

In file crates/tirith-core/src/checkpoint.rs around lines 378-380:

Race condition: file could be deleted between `exists()` check and `sha256_file()`. Consider catching `NotFound` errors from `sha256_file` and treating them as `Deleted` rather than failing the entire operation.

Evidence trail:
crates/tirith-core/src/checkpoint.rs lines 367-378 (REVIEWED_COMMIT): shows the exists() check on line 367 followed by sha256_file() call on line 377-378 with error propagation via ?. crates/tirith-core/src/checkpoint.rs lines 593-605 (REVIEWED_COMMIT): sha256_file() implementation shows it uses fs::File::open(path) which returns NotFound error if file doesn't exist.

In file crates/tirith-core/src/scan.rs around line 111:

`scanned_count` only counts files where `scan_single_file` returned `Some`. Files that fail or return `None` are neither in `scanned_count` nor `skipped_count`. Consider setting `scanned_count` to `files.len()` (attempted) or tracking failed files separately.

Evidence trail:
crates/tirith-core/src/scan.rs lines 103-115 (REVIEWED_COMMIT): loop pushes to `file_results` only on `Some`, `scanned_count: file_results.len()`. Lines 93-99: `skipped_count` only set for truncation. Lines 118-128: `scan_single_file` returns `None` on metadata/read failures via `.ok()?`.

In file crates/tirith-core/src/mcp/tools.rs around line 172:

The `shell` match only handles `"powershell"` explicitly—`"fish"` and `"pwsh"` fall through to `Posix`. Consider using `ShellType::from_str` which already handles all variants.

Evidence trail:
crates/tirith-core/src/mcp/tools.rs lines 173-178 (REVIEWED_COMMIT) - shows manual match only handling "powershell" explicitly with catch-all to Posix
crates/tirith-core/src/tokenize.rs lines 6-21 (REVIEWED_COMMIT) - shows ShellType enum with Fish variant and FromStr implementation that handles "fish" and "pwsh" correctly

In file crates/tirith-core/src/mcp/dispatcher.rs around lines 183-186:

Serialization errors are being swallowed, causing invalid or missing JSON‑RPC responses. Suggest handling serde failures explicitly: log them and surface an `-32603` error instead of returning `{}` or nothing.

Evidence trail:
crates/tirith-core/src/mcp/dispatcher.rs lines 183-186 (REVIEWED_COMMIT): The `tools/call` handler contains `JsonRpcResponse::ok(id, serde_json::to_value(result).unwrap_or(json!({})))` which uses `.unwrap_or(json!({}))` to silently swallow serialization errors and return an empty object as a success response.

In file crates/tirith-core/src/engine.rs around line 761:

The incomplete bracket trimming logic (lines 761-764) appears to be dead code. Since escape sequences like `[LBRACK]` are only appended when they fit completely (line 752-755), and verbatim characters never include `[`, there's no scenario where `out` ends with an unmatched `[`. Consider removing this block or documenting why it's kept as defensive code.

Evidence trail:
crates/tirith-core/src/engine.rs lines 726, 741-748, 752-755, 761-764 at REVIEWED_COMMIT. Line 726 shows `[` is explicitly matched and escaped to `[LBRACK]`. Lines 752-755 show escape sequences are only added if they fit completely (`if out.len() + escaped.len() > content_cap { break; }`). Lines 741-748 show the verbatim branch excludes `[` since it's handled earlier. Lines 761-764 show the dead code that trims incomplete brackets - a condition that can never be true given the implementation.

In file crates/tirith-core/src/rules/terminal.rs around line 517:

The expected output `""` seems incorrect. `&` and `<` are HTML entities, not tags. Consider updating the assertion to expect `"& <"` (if decoding) or `"& <"` (if preserving), or rename the function if it intentionally strips entities too.

Evidence trail:
crates/tirith-core/src/rules/terminal.rs:517 - test assertion `assert_eq!(strip_html_tags("&amp; &lt;"), "");`
crates/tirith-core/src/rules/terminal.rs:422-438 - function implementation with doc comment "Strip HTML tags to extract approximate visible text content" but line 430 defines ENTITIES regex and line 435 strips entities
crates/tirith-core/src/rules/terminal.rs:392-395 - usage context showing function is used for length comparison in clipboard hidden content detection

In file crates/tirith-core/src/license.rs around lines 204-206:

Test name suggests case insensitivity is verified, but only uppercase `"PRO"` is tested. Consider adding assertions for `"pro"` or `"Pro"` to actually validate case-insensitive behavior.

Evidence trail:
crates/tirith-core/src/license.rs lines 204-207 (test `test_decode_case_insensitive` at REVIEWED_COMMIT) - test only uses `make_key("PRO", ...)` with uppercase.
crates/tirith-core/src/license.rs lines 113-119 (decode_tier implementation at REVIEWED_COMMIT) - shows `.to_lowercase()` is used, confirming case-insensitivity exists but test doesn't verify multiple case variants.

In file crates/tirith-core/src/rules/rendered.rs around line 531:

The `cm` operator concatenates matrices, so scales should multiply with the existing CTM rather than replace it. Consider changing line 531 to `current_scale *= scale;` to properly accumulate transformations.

Evidence trail:
crates/tirith-core/src/rules/rendered.rs lines 525-535 (REVIEWED_COMMIT) - shows `cm` handler setting `current_scale = scale;` instead of multiplying. PDF specification via pdfjs-docs (https://github.com/MeiKatz/pdfjs-docs): 'Modify the current transformation matrix (CTM) by concatenating the specified matrix' confirms cm concatenates (multiplies) matrices. libharu issue #169 (https://github.com/libharu/libharu/issues/169): 'cm operator is an operator that multiplies LEFT to the current matrix' further confirms matrix multiplication semantics.

In file crates/tirith-core/src/mcp/dispatcher.rs around line 306:

The error code `-32603` is incorrect for unknown resources. The validation above only checks that `uri` exists and is a string—not that it refers to a valid resource. When `read_content` returns "Unknown resource", that's still invalid params (`-32602`), not an internal error.

Evidence trail:
crates/tirith-core/src/mcp/dispatcher.rs lines 280-310 (REVIEWED_COMMIT): shows validation only checks uri exists/is a string (lines 281-297), then uses -32603 for read_content errors (line 306) with misleading comment. crates/tirith-core/src/mcp/resources.rs line 75: shows read_content returns Err("Unknown resource: {uri}") for unrecognized URIs. JSON-RPC 2.0 specification defines -32602 as 'Invalid params' for invalid method parameters and -32603 as 'Internal error' for server-side errors.

In file crates/tirith-core/src/rules/rendered.rs around line 66:

The `[^;]*` between `position:absolute/fixed` and `left/top:-9999` won't match across semicolons, but valid CSS separates these with `;` (e.g., `position:absolute;left:-9999px`). Consider changing `[^;]*` to `[^}]*` or similar to span multiple declarations.

Evidence trail:
crates/tirith-core/src/rules/rendered.rs line 66: `Regex::new(r#"(?i)position\s*:\s*(?:absolute|fixed)[^;]*(?:left|top)\s*:\s*-9999"#)` - The regex uses `[^;]*` which matches any character except semicolons, but valid CSS like `position:absolute;left:-9999px` uses semicolons between declarations, so the pattern fails to match properly formatted CSS.