* feat: add cargo-fuzz targets for hand-rolled text parsers
Skim's most panic-prone code is the hand-written byte/char-index
bookkeeping over untrusted input: ANSI stripping, --nth/--with-nth field
extraction, the fuzzy matching algorithms, the query->engine->match
pipeline, and the --bind key-map parser. Add five cargo-fuzz (libFuzzer)
targets covering each, asserting real invariants (char-boundary safety,
monotonic index mappings, match indices in bounds) rather than just
catching panics, plus a CI workflow that runs them on every push/PR
touching src/ or fuzz/ and a longer nightly session via cron.
* ci: wire fuzz targets into the existing test matrix
Replace the standalone fuzz.yml workflow with a `fuzz` job in the main
test.yml matrix, running each of the 5 fuzz targets for 60s (5 minutes
total per CI run) alongside nextest/clippy/msrv.
* ci: remove standalone fuzz workflow
Superseded by the fuzz job now in test.yml.
* ci: run fuzz job as a single job on the platform matrix
Reuse the existing linux/macos/windows matrix instead of a separate
per-target matrix; run all 5 fuzz targets sequentially in one step
(60s each, 5 minutes total). cargo-fuzz doesn't support Windows, so
the fuzzing step is skipped there while still installing the
toolchain for consistency with the rest of the matrix.
* ci: reuse existing yaml anchors in the fuzz job
Use *toolchain instead of a bespoke nightly-install step, matching
the coverage job's pattern of letting `cargo +nightly` auto-provision
the toolchain on demand.
* nix: add cargo-fuzz to the tests devShell
Makes cargo-fuzz available via `nix develop` alongside the other test
tooling, matching what CI installs for the fuzz job.
* ci: install cargo-fuzz via taiki-e/install-action
Matches how the other CI-only cargo subcommands (nextest, cargo-msrv,
cargo-llvm-cov) are installed, and is faster than compiling it from
source with cargo install.
* ci: force the native host target for cargo-fuzz
cargo-fuzz was picking a statically-linked musl target on the runner,
which fails since ASan can't link against a static libc. Pass the
actual host triple (from `rustc -vV`) explicitly so the sanitizer
build always targets the dynamically-linked gnu/darwin toolchain.
* ci: skip the whole fuzz job on windows
Rather than skipping just the fuzzing step, exclude the job entirely
for the windows-latest matrix entry via a job-level `if`, since
cargo-fuzz/libFuzzer has no Windows support.
* ci: gate the fuzz job with runner.os instead of matrix.os
Matches the runner.os-based conditionals already used elsewhere in
this workflow (the linux/macos dependency install steps) rather than
comparing matrix.os directly.
* ci: enable the fuzz job on windows without ASan
cargo-fuzz does support Windows, but AddressSanitizer on the MSVC
target needs the separate "C++ AddressSanitizer" VS component plus a
PATH tweak for its DLL, which this runner doesn't have configured.
Rather than skip the job, disable the sanitizer on Windows only
(--sanitizer none) and keep coverage-guided fuzzing there; our
targets assert via plain Rust panics so they don't depend on ASan.
* test: assert exact char_idx correctness in ansi_strip fuzz target
Replace the bounds-only char_idx check with an exact-equality check
against the char position of byte_pos in the original string. This
subsumes (and is stronger than) the monotonicity CodeRabbit flagged,
since strictly-increasing byte positions on char boundaries always
imply strictly-increasing char positions.
* ci: skip windows in fuzz job, scope job permissions
CI showed the Windows fuzz build fails with a real MSVC linker error
(LNK2001: unresolved __start/__stop___sancov_pcs) even with
--sanitizer none: MSVC's linker doesn't synthesize the section
boundary symbols that libFuzzer's coverage instrumentation requires,
so this is unrelated to the earlier ASan/PATH discussion and isn't
fixable by a sanitizer flag. Skip Windows via step-level `if`
(job-level `if` can't reference runner/matrix contexts). Also add an
explicit contents:read permissions block to the job.
* fix(fzy): fix unicode case-folding inconsistency causing overflow panic
The new fuzzy_match fuzz target found a real crash: FzyMatcher panicked
with "attempt to multiply with overflow" on choice="ű\0\0\0\u{1e}ű",
pattern="Űű".
Root cause: fzy_score's case-insensitive comparison used
char::to_ascii_lowercase (a no-op on non-ASCII letters like Ű/ű), while
the shared cheap_matches() prefilter (and the other matchers) use the
Unicode-aware char_equal(). This let cheap_matches accept a pattern
that fzy_score's own DP could then never actually align, since needle
char 'Ű' never matched any haystack position under ASCII-only folding.
The DP's SCORE_MIN sentinel ("impossible") isn't an absorbing element
under plain integer addition, so the broken alignment accumulated to a
value close to, but not exactly, SCORE_MIN, which then overflowed on
the final *SCORE_TO_SKIM conversion since only the exact sentinel was
special-cased.
Fix is_match to use the shared char_equal() so fzy.rs's case folding
matches cheap_matches and the other two matchers (skim.rs, clangd.rs
already do this). Also switch internal_to_skim_score to saturating_mul
as defense in depth, since fzy_score structurally always returns
Some(..) and has no other way to signal "no valid alignment" to the
caller.
* ci: try lld-link to get windows fuzzing working (no ASan)
MSVC ASan is documented broken on GitHub-hosted Windows runners
(actions/runner-images#8891 — ASan binaries crash with
STATUS_DLL_INIT_FAILED even with the runtime DLL on PATH, unresolved
upstream), so it's not viable here regardless of our config. Separately,
the sancov coverage instrumentation cargo-fuzz needs doesn't link with
MSVC's link.exe at all (missing __start/__stop section symbols).
Try switching the Windows leg to rustc's bundled LLD linker
(-C linker-flavor=lld-link -C link-self-contained=+linker) with
--sanitizer none, to at least get coverage-guided fuzzing (no ASan)
working there. Validating live against this PR's CI.
* ci: revert windows fuzzing attempt, exclude it again
The lld-link experiment ruled out the remaining option: LLD's COFF
driver hit the exact same missing __start/__stop___sancov_* symbols as
MSVC's link.exe. This confirms the section-boundary-symbol synthesis
libFuzzer's coverage instrumentation needs simply isn't implemented
for the COFF/Windows target in current LLVM/rustc — an upstream gap,
not a linker choice or CI config problem. Combined with MSVC ASan
being separately documented broken on GH-hosted Windows runners
(actions/runner-images#8891), there's no remaining avenue to try from
the workflow side. Back to excluding Windows from the fuzz job.
* ci: try windows fuzzing with default sanitizer + msvc dev env
Previous Windows attempts both used --sanitizer none, which removes
the ASan runtime that (on Windows) supplies the __start/__stop section
symbol shims libFuzzer's coverage instrumentation needs -- neither
linker synthesizes those on COFF. That's very likely why they failed
to link. Revert to the default sanitizer (address) and add
ilammy/msvc-dev-cmd to put the MSVC ASan DLL directory on PATH, per
the cargo-fuzz Windows setup guide and actions/runner-images#8891.
Testing live whether this builds, and whether the previously-reported
STATUS_DLL_INIT_FAILED runtime crash still reproduces on this runner
image.
* ci: point cargo at the real MSVC linker on windows
msvc-dev-cmd correctly set up Path, but Git Bash prepends its own
usr/bin ahead of it, so cargo picked up Git's coreutils `link`
(hardlink tool) instead of MSVC's link.exe. Set
CARGO_TARGET_X86_64_PC_WINDOWS_MSVC_LINKER explicitly using
VCToolsInstallDir (set by msvc-dev-cmd) to sidestep PATH ordering
entirely.
* fix(event): parse_action returns None instead of panicking on missing args
The keymap_parse fuzz target found a real crash: KeyMap::from("/:if-")
panicked ("no arg specified for event if-") since parse_action's
documented behavior was to panic on if-* actions missing their
argument, even though the function already returns Option<Action> and
every other malformed/unrecognized action already resolves to None
via the surrounding parse_action_chain/KeyMap plumbing.
Fixed that case, and while checking for the same pattern elsewhere in
the function found four more reachable panics of the same kind
(add-char, execute, execute-silent, set-preview-cmd, set-query parsed
without their required argument), confirmed each panics via a small
repro before fixing. All now return None like every other malformed
action, consistent with the function's existing contract, instead of
panicking on user-supplied --bind strings.
* ci: add a single aggregate status check for branch rulesets
Add a ci-success job that depends on every other job in the workflow
and fails if any of them failed or were cancelled (tolerating
deploy-coverage-page's expected skip off master). This gives branch
protection / repository rulesets one stable check name to require,
instead of enumerating every matrix leg (nextest (linux), fuzz
(windows), etc.) individually.
---------
Co-authored-by: Claude <noreply@anthropic.com>
6.2 KiB
Skim Agent Guidelines
Build/Test/Lint Commands
- Build:
cargo build [--release] - Run:
cargo run [--release] - Test (all):
cargo nextest run - Test (single):
cargo nextest test_name - Integration/E2E tests:
cargo nextest --tests(will need tmux under the hood) - Memory leak detection:
cargo nextest run --profile valgrind - Thread leak/race detection:
- Build:
RUSTFLAGS="-Zsanitizer=thread" cargo +nightly build --tests -Zbuild-std --target x86_64-unknown-linux-gnu - Run:
TSAN_OPTIONS="detect_deadlocks=1" cargo +nightly nextest run --profile tsan --target x86_64-unknown-linux-gnu
- Build:
- Lint:
cargo clippy - Format:
cargo +nightly fmt(check only:cargo +nightly fmt --check) - Fuzz (requires nightly +
cargo install cargo-fuzz):cargo +nightly fuzz run <target>— seefuzz/README.mdfor target list
Code Style
- Format with 120 char line width (defined in .rustfmt.toml)
- Use standard Rust naming conventions (snake_case for functions/variables, CamelCase for types)
- Organize imports by standard library, external crates, then internal modules
- Prefer Option/Result types for error handling over panicking
- Use proper error propagation with
?operator - Document public API with rustdoc comments
- Use meaningful type annotations, especially for public functions
- Follow the existing structure for new modules (see src/engine/ or src/model/)
- Implement relevant traits (SkimItem, etc.) for new types when needed
Architecture Documentation
ARCHITECTURE.mddocuments the full architecture: data flow, operating modes, subsystems, threading model, and public API.- Update
ARCHITECTURE.mdwhenever you make structural changes, including:- Adding, removing, or renaming modules, structs, or traits
- Changing the data flow between subsystems (reader → pool → matcher → TUI)
- Adding new operating modes or modifying existing ones
- Changing the threading model or synchronization primitives
- Adding or removing public API surface (
SkimItem,SkimOptions,SkimOutput, etc.) - Changing the event/action system or key binding infrastructure
- Keep call-site line numbers in the cross-reference table up to date when the referenced functions move.
Testing
This application can be tested by :
- creating a new
tmuxsession in the background (tmux new-session -s <session name> -d). Make sure to clear theSKIM_DEFAULT_OPTIONSenv var. - creating a new named tmux window in that session :
tmux new-window -d -P -F '#I' -n <window name> -t <session name>and configuring the pane naming usingtmux set-window-option -t <window name> pane-base-index 0 - sending the command to run and input using
tmux send-keys -t <window name> <keys> - when ready, capturing the window using
tmux capture-pane -b <window name> -t <window name>.0and then saving the capture to a file usingtmux save-buffer -b <window name> <output file>
Insta Snapshot Tests
Most TUI behaviour is covered by insta snapshot tests in tests/. The
infrastructure lives in tests/common/insta.rs and is exposed through two
macros: snap! and insta_test!.
insta_test! — writing tests
Simple variant (single snapshot, no interaction):
insta_test!(my_test, ["item1", "item2"], &["--opt1", "opt2"]);
insta_test!(my_test, @cmd "printf 'a\nb'", &["--ansi"]);
insta_test!(my_test, @interactive, &["-i", "--cmd", "echo {q}"]);
DSL variant (multiple snapshots with interaction between them):
insta_test!(my_test, ["a", "b", "c"], &["--multi"], {
@snap; // take a snapshot (cell text only)
@snap_color; // snapshot cell styling (fg/bg/modifier) instead
@key Up; // send a named key (Enter, Down, Tab, …)
@char 'f'; // send a single character
@type "foo"; // type a string
@ctrl 'w'; // Ctrl+key
@alt 'b'; // Alt+key
@shift Tab; // Shift+key
@action Last; // send an Action variant (no args)
@action Down(1); // send an Action variant (with args)
@snap; // take another snapshot
@assert(|h| condition); // boolean assertion (does not snapshot)
@exited 0; // assert the app exited with this code
});
Snapshot file naming
| Macro form | File pattern | Example |
|---|---|---|
| Simple variant | {file}__{test}.snap |
options__opt_wrap.snap |
DSL variant — Nth @snap |
{file}__{test}@{NNN}.snap |
options__opt_cycle@002.snap |
DSL variant — Nth @snap_color |
{file}__{test}@color{NNN}.snap |
ansi__ansi_flag_enabled@color002.snap |
DSL snapshots use a zero-padded three-digit suffix (@001, @002, …) so that
cargo insta review presents them in the order they were taken.
Snapshot front-matter
Every snapshot includes a description field in its YAML front-matter. For a
DSL test the description shows the input, options, and the DSL commands that
ran since the previous @snap, making it easy to understand what state
each screenshot captures:
description: "input: items [\"a\", \"b\", \"c\"]\noptions: --multi\nafter:\n @key Up\n @shift Tab"
The expression field is intentionally omitted (omit_expression = true) to
keep the files free of internal implementation details.
Snapshot workflow
Generate / update snapshots:
# Generate all missing snapshots and accept them immediately:
INSTA_UPDATE=always cargo nextest run
# Generate missing snapshots as .snap.new files for manual review:
INSTA_UPDATE=new cargo nextest run
cargo insta review # accept / reject interactively
When adding new tests that produce snapshots:
- Write the test with
@snapmarkers. - Run
INSTA_UPDATE=always cargo nextest run --test <file> <test_name>to generate the initial snapshot files. - Inspect the generated
.snapfiles to verify the rendered output is correct. - Commit both the test and its snapshot files.
When changing rendering logic that affects many existing snapshots:
- Delete the affected
.snapfiles:find tests/snapshots -name "prefix__*.snap" -delete - Regenerate:
INSTA_UPDATE=always cargo nextest run - Review the diff with
git diff tests/snapshots/before committing.