Skip to content

Commit 06161fe

Browse files
test: harden discovery and JSON preservation contracts (#8)
## Summary Addresses the bounded follow-up notes from PRs #5 and #6. - makes the experimental discovery result, observation, marker-kind, and error contracts non-exhaustive, including record-style variants, and documents downstream matching requirements - strengthens discovery coverage for mixed marker kinds, error payloads/formatting/sources, non-UTF-8 Linux filenames, and the known trailing-separator symlink limitation - replaces semantic-only structural deletion checks in the JSON study with exact mutation-envelope assertions - records exact insertion outputs across positions, compact/multiline layouts, LF/CRLF, and unusual indentation - records exact preservation of adversarial neighboring unknown data, numeric lexemes, string escapes, and array contents across removal and insertion - updates ADR 0004 and the study ledger so their evidence claims match the stronger tests ## Verification - `cargo fmt --all --check` - `cargo check --workspace --all-targets --all-features` - `cargo test --workspace --all-targets --all-features` (43 tests passed locally) - `cargo clippy --workspace --all-targets --all-features -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps` - `git diff --check` The invalid-byte filename fixture is Linux-only because the local macOS filesystem rejects that filename at fixture creation time; Ubuntu CI exercises it. Cargo continues to emit the pre-existing rustdoc output-name collision warning between the library and CLI binary, while the documentation command exits successfully. ## Deferred follow-up ledger These items remain intentionally deferred because they require separate design choices, evidence, or production seams rather than being omissions from this hardening PR: - typed views over retained CST nodes and the public raw/typed boundary - stale-snapshot/hash refusal, semantic diffs, and stable resource identity - byte decoding and BOM policy - scalar-replacement contracts and production mutation wrappers - final diagnostic API and Unicode-width policy - representative performance, dependency, and resource-limit budgets - reproducible comparative evidence for alternative JSON representations - broader format fixtures and production integration - filesystem capability/root acquisition and deterministic permission/race fault injection - persistence, transaction, and write-safety design
1 parent ea088ee commit 06161fe

5 files changed

Lines changed: 292 additions & 50 deletions

File tree

‎crates/tilewright/README.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@ This crate is in early development. It currently exposes its package version and
1111

1212
Candidate discovery identifies directories that appear to be RPG Maker MZ projects based on marker files. It does not validate the project contents.
1313

14+
Discovery results, marker observations, marker kinds, and errors are
15+
non-exhaustive while this API is experimental. Downstream matches must retain a
16+
fallback arm and use `..` in record patterns.
17+
1418
```rust
1519
use std::path::Path;
1620
use tilewright::rpg_maker_mz::discovery::{discover_candidate, CandidateDiscovery};
1721

1822
let path = Path::new("path/to/project");
1923
match discover_candidate(path) {
20-
Ok(CandidateDiscovery::Candidate { marker }) => {
24+
Ok(CandidateDiscovery::Candidate { marker, .. }) => {
2125
println!("Found candidate marker at: {}", marker.path.display());
2226
}
2327
Ok(CandidateDiscovery::NoMarker) => {

‎crates/tilewright/src/rpg_maker_mz/discovery.rs‎

Lines changed: 182 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,15 @@ use std::io;
1313
use std::path::{Path, PathBuf};
1414

1515
/// Errors that can occur during candidate discovery.
16+
///
17+
/// This enum and its record-style variants are non-exhaustive so the
18+
/// experimental discovery API can add contextual failure modes and fields
19+
/// without making downstream matches exhaustive.
1620
#[derive(Debug)]
21+
#[non_exhaustive]
1722
pub enum DiscoveryError {
1823
/// Failed to inspect the root directory's metadata.
24+
#[non_exhaustive]
1925
InspectRoot {
2026
/// The root path being inspected.
2127
root: PathBuf,
@@ -24,30 +30,35 @@ pub enum DiscoveryError {
2430
},
2531
/// The supplied root path is a symlink. This is a best-effort check of the
2632
/// final supplied path component at inspection time.
33+
#[non_exhaustive]
2734
RootIsSymlink {
2835
/// The root path that was rejected.
2936
root: PathBuf,
3037
},
3138
/// The supplied root path is not a directory.
39+
#[non_exhaustive]
3240
RootIsNotDirectory {
3341
/// The root path that was rejected.
3442
root: PathBuf,
3543
},
3644
/// Failed to read the contents of the root directory.
45+
#[non_exhaustive]
3746
ReadRoot {
3847
/// The root directory being read.
3948
root: PathBuf,
4049
/// The underlying I/O error.
4150
source: io::Error,
4251
},
4352
/// Failed to read a directory entry.
53+
#[non_exhaustive]
4454
ReadEntry {
4555
/// The root directory being read.
4656
root: PathBuf,
4757
/// The underlying I/O error.
4858
source: io::Error,
4959
},
5060
/// Failed to inspect the metadata of a potential marker entry.
61+
#[non_exhaustive]
5162
InspectMarker {
5263
/// The path of the marker entry being inspected.
5364
path: PathBuf,
@@ -94,7 +105,11 @@ impl Error for DiscoveryError {
94105
}
95106

96107
/// The kind of filesystem entry observed for a marker.
108+
///
109+
/// This enum is non-exhaustive so additional platform entry kinds can be
110+
/// represented without closing the experimental API prematurely.
97111
#[derive(Debug, Clone, PartialEq, Eq)]
112+
#[non_exhaustive]
98113
pub enum MarkerEntryKind {
99114
/// A regular file.
100115
RegularFile,
@@ -107,7 +122,12 @@ pub enum MarkerEntryKind {
107122
}
108123

109124
/// An observation of a potential marker entry.
125+
///
126+
/// This output record is non-exhaustive so later discovery phases can attach
127+
/// additional observation context without preventing callers from reading the
128+
/// currently exposed fields.
110129
#[derive(Debug, Clone, PartialEq, Eq)]
130+
#[non_exhaustive]
111131
pub struct MarkerObservation {
112132
/// The exact path to the observed marker entry.
113133
pub path: PathBuf,
@@ -119,17 +139,23 @@ pub struct MarkerObservation {
119139
///
120140
/// This API identifies candidates based on the presence of a marker file. It does
121141
/// not validate the project, parse its contents, or guarantee compatibility.
142+
/// The enum and its record-style variants are non-exhaustive so callers must
143+
/// retain a fallback for future experimental discovery outcomes and `..` when
144+
/// matching fields.
122145
#[derive(Debug, Clone, PartialEq, Eq)]
146+
#[non_exhaustive]
123147
pub enum CandidateDiscovery {
124148
/// The directory contains exactly one regular file matching the expected
125149
/// lowercase marker name (`game.rmmzproject`).
150+
#[non_exhaustive]
126151
Candidate {
127152
/// The observed marker entry.
128153
marker: MarkerObservation,
129154
},
130155
/// The directory contains exactly one regular file matching the marker name
131156
/// case-insensitively, but not exactly (e.g., `Game.rmmzproject`).
132157
/// This is a platform-limited case variant.
158+
#[non_exhaustive]
133159
CaseVariantCandidate {
134160
/// The observed marker entry.
135161
marker: MarkerObservation,
@@ -138,16 +164,19 @@ pub enum CandidateDiscovery {
138164
NoMarker,
139165
/// The directory contains multiple entries matching the marker name
140166
/// case-insensitively.
167+
#[non_exhaustive]
141168
AmbiguousMarkers {
142169
/// The observed marker entries, deterministically ordered by path.
143170
markers: Vec<MarkerObservation>,
144171
},
145172
/// The marker entry exists but is a symbolic link.
173+
#[non_exhaustive]
146174
SymlinkMarker {
147175
/// The observed marker entry.
148176
marker: MarkerObservation,
149177
},
150178
/// The marker entry exists but is not a regular file or symlink (e.g., a directory).
179+
#[non_exhaustive]
151180
NonRegularMarker {
152181
/// The observed marker entry.
153182
marker: MarkerObservation,
@@ -315,6 +344,27 @@ mod tests {
315344
);
316345
}
317346

347+
#[test]
348+
fn test_classify_mixed_kind_ambiguity() {
349+
let case_variant_directory = MarkerObservation {
350+
path: PathBuf::from("Game.rmmzproject"),
351+
kind: MarkerEntryKind::Directory,
352+
};
353+
let exact_symlink = MarkerObservation {
354+
path: PathBuf::from("game.rmmzproject"),
355+
kind: MarkerEntryKind::Symlink,
356+
};
357+
358+
let result = classify_matches(vec![exact_symlink.clone(), case_variant_directory.clone()]);
359+
360+
assert_eq!(
361+
result,
362+
CandidateDiscovery::AmbiguousMarkers {
363+
markers: vec![case_variant_directory, exact_symlink]
364+
}
365+
);
366+
}
367+
318368
#[test]
319369
fn test_classify_symlink() {
320370
let marker = MarkerObservation {
@@ -465,8 +515,16 @@ mod tests {
465515
let symlink_dir = temp.path().join("symlink_dir");
466516
symlink(&real_dir, &symlink_dir).unwrap();
467517

468-
let result = discover_candidate(&symlink_dir);
469-
assert!(matches!(result, Err(DiscoveryError::RootIsSymlink { .. })));
518+
let error = discover_candidate(&symlink_dir).unwrap_err();
519+
assert!(matches!(
520+
&error,
521+
DiscoveryError::RootIsSymlink { root } if root == &symlink_dir
522+
));
523+
assert_eq!(
524+
error.to_string(),
525+
format!("root path '{}' is a symlink", symlink_dir.display())
526+
);
527+
assert!(std::error::Error::source(&error).is_none());
470528
}
471529

472530
#[cfg(windows)]
@@ -524,8 +582,16 @@ mod tests {
524582
panic!("Failed to create symlink: {e}");
525583
}
526584

527-
let result = discover_candidate(&symlink_path);
528-
assert!(matches!(result, Err(DiscoveryError::RootIsSymlink { .. })));
585+
let error = discover_candidate(&symlink_path).unwrap_err();
586+
assert!(matches!(
587+
&error,
588+
DiscoveryError::RootIsSymlink { root } if root == &symlink_path
589+
));
590+
assert_eq!(
591+
error.to_string(),
592+
format!("root path '{}' is a symlink", symlink_path.display())
593+
);
594+
assert!(std::error::Error::source(&error).is_none());
529595
}
530596

531597
#[test]
@@ -620,20 +686,92 @@ mod tests {
620686
fn test_discover_missing_root() {
621687
let temp = TempDir::new().unwrap();
622688
let missing_path = temp.path().join("missing");
623-
let result = discover_candidate(&missing_path);
624-
assert!(matches!(result, Err(DiscoveryError::InspectRoot { .. })));
689+
let error = discover_candidate(&missing_path).unwrap_err();
690+
match &error {
691+
DiscoveryError::InspectRoot { root, source } => {
692+
assert_eq!(root, &missing_path);
693+
assert_eq!(source.kind(), io::ErrorKind::NotFound);
694+
}
695+
other => panic!("expected InspectRoot, got {other:?}"),
696+
}
697+
assert_eq!(
698+
error.to_string(),
699+
format!("failed to inspect root path '{}'", missing_path.display())
700+
);
701+
assert_eq!(
702+
std::error::Error::source(&error)
703+
.and_then(|source| source.downcast_ref::<io::Error>())
704+
.map(io::Error::kind),
705+
Some(io::ErrorKind::NotFound)
706+
);
625707
}
626708

627709
#[test]
628710
fn test_discover_non_directory_root() {
629711
let temp = TempDir::new().unwrap();
630712
let file_path = temp.path().join("file.txt");
631713
File::create(&file_path).unwrap();
632-
let result = discover_candidate(&file_path);
714+
let error = discover_candidate(&file_path).unwrap_err();
633715
assert!(matches!(
634-
result,
635-
Err(DiscoveryError::RootIsNotDirectory { .. })
716+
&error,
717+
DiscoveryError::RootIsNotDirectory { root } if root == &file_path
636718
));
719+
assert_eq!(
720+
error.to_string(),
721+
format!("root path '{}' is not a directory", file_path.display())
722+
);
723+
assert!(std::error::Error::source(&error).is_none());
724+
}
725+
726+
#[test]
727+
fn test_discovery_read_error_contracts() {
728+
let root = PathBuf::from("project");
729+
let marker = root.join("game.rmmzproject");
730+
let cases = [
731+
(
732+
DiscoveryError::ReadRoot {
733+
root: root.clone(),
734+
source: io::Error::new(io::ErrorKind::PermissionDenied, "fixture read root"),
735+
},
736+
format!("failed to read root directory '{}'", root.display()),
737+
root.clone(),
738+
),
739+
(
740+
DiscoveryError::ReadEntry {
741+
root: root.clone(),
742+
source: io::Error::new(io::ErrorKind::PermissionDenied, "fixture read entry"),
743+
},
744+
format!("failed to read entry in directory '{}'", root.display()),
745+
root.clone(),
746+
),
747+
(
748+
DiscoveryError::InspectMarker {
749+
path: marker.clone(),
750+
source: io::Error::new(io::ErrorKind::PermissionDenied, "fixture marker"),
751+
},
752+
format!("failed to inspect marker entry '{}'", marker.display()),
753+
marker.clone(),
754+
),
755+
];
756+
757+
for (error, expected_display, expected_path) in cases {
758+
match &error {
759+
DiscoveryError::ReadRoot { root, .. } | DiscoveryError::ReadEntry { root, .. } => {
760+
assert_eq!(root, &expected_path)
761+
}
762+
DiscoveryError::InspectMarker { path, .. } => {
763+
assert_eq!(path, &expected_path);
764+
}
765+
other => panic!("unexpected error variant: {other:?}"),
766+
}
767+
assert_eq!(error.to_string(), expected_display);
768+
assert_eq!(
769+
std::error::Error::source(&error)
770+
.and_then(|source| source.downcast_ref::<io::Error>())
771+
.map(io::Error::kind),
772+
Some(io::ErrorKind::PermissionDenied)
773+
);
774+
}
637775
}
638776

639777
#[cfg(unix)]
@@ -657,6 +795,22 @@ mod tests {
657795
);
658796
}
659797

798+
#[cfg(target_os = "linux")]
799+
#[test]
800+
fn test_discover_non_utf8_filename_is_not_a_marker() {
801+
use std::ffi::OsString;
802+
use std::os::unix::ffi::OsStringExt;
803+
804+
let temp = TempDir::new().unwrap();
805+
let invalid_name = OsString::from_vec(b"game.rmmzproject\xff".to_vec());
806+
File::create(temp.path().join(invalid_name)).unwrap();
807+
808+
assert_eq!(
809+
discover_candidate(temp.path()).unwrap(),
810+
CandidateDiscovery::NoMarker
811+
);
812+
}
813+
660814
#[cfg(unix)]
661815
#[test]
662816
fn test_discover_root_symlink_with_dot_is_not_rejected_known_limitation() {
@@ -676,4 +830,23 @@ mod tests {
676830
let result = discover_candidate(&dot_path).unwrap();
677831
assert!(matches!(result, CandidateDiscovery::Candidate { .. }));
678832
}
833+
834+
#[cfg(unix)]
835+
#[test]
836+
fn test_discover_root_symlink_with_trailing_slash_is_not_rejected_known_limitation() {
837+
let temp = TempDir::new().unwrap();
838+
let real_dir = temp.path().join("real_dir");
839+
fs::create_dir(&real_dir).unwrap();
840+
File::create(real_dir.join("game.rmmzproject")).unwrap();
841+
842+
let symlink_dir = temp.path().join("symlink_dir");
843+
symlink(&real_dir, &symlink_dir).unwrap();
844+
845+
// A trailing separator requires directory resolution and causes
846+
// symlink_metadata to observe the target directory rather than the link.
847+
let mut trailing_slash = symlink_dir.as_os_str().to_os_string();
848+
trailing_slash.push("/");
849+
let result = discover_candidate(Path::new(&trailing_slash)).unwrap();
850+
assert!(matches!(result, CandidateDiscovery::Candidate { .. }));
851+
}
679852
}

‎docs/decisions/0004-lossless-json-representation.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,9 @@ If accepted, this proposal would mean:
8080

8181
A tracked prototype exercises the following bounded behavior:
8282
- **Structural edits:** Representative object and array insertions/deletions
83-
remain valid. Exact assertions record selected insertion formatting and
84-
layout invariants; deletion tests currently assert strict semantic validity.
83+
remain valid and have exact output assertions. A combined mutation case also
84+
records exact preservation of tested unknown nested data, numeric lexemes,
85+
string escapes, and array contents outside the measured edit envelope.
8586
- **Strict syntax:** A strict AST pass with every extension disabled runs before
8687
CST construction. A lexical preflight rejects non-JSON whitespace and raw C0
8788
controls that the scanner otherwise accepts. CST construction itself preserves

0 commit comments

Comments
 (0)