* fix: make min-height work again
* fix: revert to String and add integration tests
* chore: misc warnings
* fix: windows tests
* fix: ci public api fails because of incompatible deps version between HEAD and release
* test: replace tmux e2e harness with cross-platform Zellij harness
Rewrite the end-to-end test harness to drive `sk` through Zellij instead of
tmux, keeping the same capabilities and public surface (ZellijController,
Keys, wait, sk, the sk_test! DSL and the line!/keys!/out! helpers) so the
existing tests port over with only import/type renames.
Zellij has no detached-server model like tmux, so the harness spawns a Zellij
client attached to an in-process pseudo-terminal via portable-pty (openpty on
Unix, ConPTY on Windows). Because Zellij 0.44+ and portable-pty are both
cross-platform, the harness — and the tests that only rely on it — are now
available on Windows too: the interactive tests (formerly unix.rs) are
un-gated. execute.rs, popup.rs and listen.rs stay unix-only for reasons
unrelated to the multiplexer (PermissionsExt, a mock sh/tmux binary, unix
sockets).
Key harness details:
- Session per test via `zellij attach --create` on a fixed 80x24 PTY.
- Keys injected as raw terminal bytes with `zellij action write`; screen read
back with `zellij action dump-screen [--ansi]`, reversed to match the old
bottom-anchored indexing.
- A generated config disables startup tips, pane frames, mouse mode and — the
crucial bit — the kitty keyboard protocol, so injected legacy escape
sequences (arrows, etc.) reach sk.
- All zellij CLI calls are run under a timeout and wait() has a wall-clock
budget, so a wedged server surfaces as a fast retryable error instead of
hanging a test.
popup.rs unsets $ZELLIJ and sets $TMUX so skim selects its tmux popup backend
(the mock) rather than the zellij one while running inside a Zellij pane.
Because each test spins up a full Zellij session, the e2e binaries are put in
a serialized nextest test-group; CI installs Zellij (all three OSes) in place
of tmux, and the obsolete tmux setup-scripts are removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* ci: fix rustfmt and stop Windows from cancelling the other nextest legs
- Run `cargo +nightly fmt` on the new Zellij harness (rustfmt CI was red).
- Set `fail-fast: false` on the nextest matrix so a failing OS leg no longer
cancels the others, giving a clear pass/fail signal per platform.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* ci: install zellij via winget on Windows
taiki-e/install-action has no prebuilt Zellij binary for Windows and falls
back to `cargo install zellij`, which fails building openssl-sys from source
on the runner. Install via winget on Windows instead (taiki-e still handles
Linux/macOS), and expose winget's shim dir on PATH for the test step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test/ci: address review feedback on the Zellij harness
- Pin the Windows winget Zellij install to 0.44.3 to match the Linux/macOS
runners (reproducible CI).
- Drop the unused `&locale` YAML anchor (actionlint flagged it).
- `wait` now surfaces the last predicate error on timeout instead of a
generic one, so a persistent failure keeps its diagnostic cause.
- `output_with_timeout` tears down the child and reader threads on a
`try_wait` error instead of leaking them.
- Add rustdoc to the public harness surface (`sk`, `wait`, `Keys`,
`ZellijController` and its methods).
Deliberately not changed: a non-zero `zellij` exit is still not treated as an
error (some `zellij action` calls exit non-zero in transient states — e.g.
inline `sk` viewport teardown — while returning usable output; propagating it
broke `inline_clear_on_exit`), and `to_lines` keeps trimming to preserve the
tmux-parity bottom-anchored indexing the ported tests rely on.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* ci: put the real zellij.exe dir on PATH for the Windows test step
The winget install succeeds, but its Links shim wasn't reliably visible to the
`cargo nextest` step's processes, so `which("zellij")` failed and every
interactive test panicked at setup. Locate the installed zellij.exe under the
WinGet Packages dir and add its directory to GITHUB_PATH instead, failing the
step loudly if it isn't found.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* ci: reload PATH from registry after the MSI zellij install on Windows
The winget Zellij package is an MSI installer that installs to Program Files
and updates the machine PATH in the registry, not a portable under
WinGet\Packages — so the previous "search Packages" lookup threw. Reload PATH
from the machine/user registry values (with a Program Files fallback), then
export zellij's directory via GITHUB_PATH for the test step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test/ci: gate interactive e2e tests off Windows
Enabling the interactive tests on the Windows runner surfaced a real gap: the
PATH/install issues are fixed (winget install works), but under the Windows
runner's ConPTY the Zellij session never renders — dump-screen stays empty and
wait_ready times out with "pane not rendered yet" for every interactive test.
That's a harness-runtime gap on Windows (and sk's escape-code disambiguation on
Windows would be a further blocker), so gate interactive.rs `#![cfg(not(windows))]`
with a TODO, keeping the harness code cross-platform.
Since no Windows test now uses the harness, drop the winget Zellij install from
the Windows leg; Linux/macOS still install it via taiki-e. Adjust the docs that
claimed the e2e tests run on Windows.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): gate Zellij harness tests to Linux only
The Zellij-backed e2e harness renders reliably under the Linux CI runner,
but on the macOS and Windows runners the pane never comes up under their
PTY (`wait_ready` times out with "pane not rendered yet"). Restrict all
four e2e test files (interactive, execute, popup, listen) to
`#![cfg(target_os = "linux")]`, install Zellij only on the Linux runner,
and update the harness/agent/architecture docs to match.
The harness code stays cross-platform so macOS/Windows e2e can be
re-enabled once their runners render the session.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): make the Zellij harness render on macOS and Windows
The Zellij e2e harness previously only came up reliably on the Linux CI
runner; on macOS and Windows the pane never rendered and every e2e test
timed out with "pane not rendered yet". Root cause (surfaced by capturing
the Zellij client's PTY output): Zellij's client/server startup handshake
is racy — the client occasionally dies with "Received empty unknown from
server" and the session never renders. It's rare on Linux (flaky) but
frequent on the cold macOS/Windows runners.
Harden the harness so it renders everywhere instead of gating tests to
Linux:
- Detect a dead session fast (drain thread flags client PTY EOF) and
respawn a fresh session, up to SESSION_SPAWN_ATTEMPTS times, instead of
waiting out the whole render budget and failing.
- Resolve the pane's shell to an absolute `bash` path via `which`; the
Zellij server's own environment may not have `bash` on PATH on the
macOS/Windows runners, which would leave the pane with no shell to render.
- Nudge the client's terminal size until the server gives the pane a
non-zero geometry to render into (the initial size can be dropped under
ConPTY / a cold runner).
- Give the first render its own longer budget and, on timeout, surface a
tail of the Zellij client output for diagnosing runners we can't
reproduce locally.
Un-gate the tests accordingly: interactive.rs (pure harness) now runs on
Linux, macOS and Windows; execute.rs/popup.rs/listen.rs go back to
#![cfg(unix)] (Linux + macOS) — their Windows-incompatibility is POSIX
mock binaries / a unix socket, unrelated to the multiplexer. CI installs
Zellij on all three OSes (taiki-e on Linux/macOS, winget on Windows) and
the nextest job gets a 45-minute cap so a harness regression fails fast.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): fix macOS session-name rejection via short ZELLIJ_SOCKET_DIR
The macOS runner failed every Zellij e2e test at CLI-parse time:
error: Invalid value "skim_e2e_..." for '--session <SESSION>':
session name must be less than 0 characters
This is not the render race the previous commit addressed. Zellij places
each session's unix socket at `$ZELLIJ_SOCKET_DIR/<protocol>/<session>`,
and a unix socket path is length-capped by the OS (~104 bytes on macOS).
Zellij's default base is `$TMPDIR/zellij-<uid>`; on the macOS runners
`$TMPDIR` is a long `/var/folders/…` path that leaves ~0 bytes for the
session name, so Zellij rejects every name and the client exits before it
attaches (zellij-org/zellij#4211). Linux's short `/run`|`/tmp` base never
hits this, which is why it only failed on macOS.
- Export ZELLIJ_SOCKET_DIR=/tmp/skim-zj (a short base) on every zellij
invocation — the attached client, `action`, and `run` — so they share a
short socket path well under the cap on Linux and macOS alike.
- Shorten session names (`sk_<=10 chars_<6 rand>`): several were derived
from long test names (e.g. execute_interactive_child_keeps_receiving_
keys_fullscreen) and exceeded Zellij's ~36-char limit and ate socket
budget; the random suffix still keeps them unique.
Also fix a stale doc command in AGENTS.md (`cargo nextest --tests` ->
`cargo nextest run --tests`), per PR review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): answer the DSR cursor-position probe so Windows renders
The Windows nextest leg hung on every interactive.rs e2e test:
Error: pane not rendered within 60s. zellij client output tail:
\u{1b}[6n
The captured client output was a single `ESC[6n` — a Device Status
Report requesting the cursor position. Under the Windows ConPTY the
Zellij client probes the terminal size by asking for the cursor position
and blocks until the terminal replies; on Unix the size comes from the
PTY ioctl, so the client never waits (which is why only Windows hung).
The harness owns the master PTY — it *is* the terminal — so the drain
thread now watches for `ESC[6n` and writes back a Cursor Position Report
(`ESC[24;80R`, reporting the 24x80 pane). This unblocks the client so the
pane renders. The reply is harmless on Linux/macOS (all 45 e2e tests
still pass there), keeping interactive.rs on all three platforms.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): address review nits in the Zellij harness
Follow-ups from PR review, none affecting the cross-platform fixes:
- zellij_socket_dir() now returns io::Result and propagates a
create_dir_all failure through run()/action()/spawn_once() instead of
swallowing it, so a socket-dir problem surfaces directly rather than as
a confusing downstream Zellij error.
- Fix a latent typo in the (currently unused) assert_line!/line! macro:
std::io::std::io::Error{,Kind} -> std::io::Error / std::io::ErrorKind,
so the macro compiles if a test ever uses it.
- tempfile() returns an InvalidData error instead of panicking on a
non-UTF-8 temp path.
Skipped the reviewer's suggestion to stop trimming captured output: the
trim is load-bearing. It drops Zellij's blank padding rows so capture()[0]
is the bottom content line that every test indexes against; stripping only
CR/LF would reintroduce ~20 empty rows and shift every index. No test
exercises intentionally-spaced items, so there is no real defect.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): silence unused_assignments warning in wait()
`last_err` was initialised to `None` and always overwritten before it
could be read, so the initial assignment was dead (unused_assignments
warning at the top of every test build). Return the current predicate
error directly on timeout instead of stashing it — same behaviour (the
most recent error is surfaced), no dead variable, no warning.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): wrap assert_line! timeout error to 120 columns
Pure formatting: split the Err/Error::new/format! construction in the
(rustfmt-skipped) assert_line! macro body across lines to satisfy the
repo's 120-column limit. No behaviour change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): guard against [-0] in the negative-index DSL macro
@method_neg_dispatch used `lines.len() >= $idx`, which is always true for
$idx == 0, so `@capture[-0]` would index `lines[lines.len()]` and panic.
Require `$idx > 0` in both the predicate and diagnostic paths so a `[-0]`
index falls through to the graceful "not enough lines" / "<no line>"
handling instead. No current test uses negative indices; this only closes
the latent edge case. Per PR review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): use forward slashes for Windows paths in the bash command
With the DSR fix the Windows pane now renders and runs the command, which
surfaced the next issue: the harness drives a `bash` shell but embedded
native Windows paths (backslashes) into the command string. bash treats
`\` as an escape, so `.\target\release\sk.exe` collapsed to
`.targetreleasesk.exe` ("command not found") and the `C:\Users\...`
redirect/mv targets would mangle the same way.
Convert `\` to `/` for the `sk` binary and the outfile when building the
bash command in sk(); bash on Windows accepts `./target/release/sk.exe`
and `C:/Users/...`. On Unix the paths have no backslashes so it is a
no-op, and the stored outfile the test reads back keeps native separators.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
* test(e2e): reap the Zellij client child in Drop
ZellijController::drop killed the client with child.kill() but never
waited on it, so on Unix each dropped controller left a zombie until the
test binary exited — and many controllers are created per binary. Pair
the kill with child.wait() (matching output_with_timeout) so the process
is reaped immediately. Per PR review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wTqRsi31RYJXEM3ZZQhQU
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(tui): stop input reader and give execute() children their own tty
Interactive/ncurses programs run via an `execute()` action (e.g. `ncdu`)
would freeze after a few keystrokes. Two independent problems caused it:
1. skim's background input reader (the `EventStream` task started in
`Tui::start`) kept reading the terminal while the child ran, so skim and
the child raced for keystrokes on the same tty — roughly half the keys
were stolen from the child.
2. The child inherited skim's stdin (fd 0), which is a pipe whenever items
are piped in (`find | sk`). An interactive child then had no keyboard
source at all.
Fix both:
- Add `Tui::stop_and_join`, which cancels the event-pump task and blocks
until it has dropped its `EventStream`, guaranteeing skim has released the
terminal before the child starts. `run_foreground` calls it before running
the child and `Tui::start` after.
- Give the child its own stdin opened from the controlling terminal
(`/dev/tty`, or `CONIN$` on Windows), falling back to inheriting skim's
stdin if that fails.
Because running a foreground process needs the `Tui` (which `handle_action`
does not have), `Execute` now only expands the command and returns a new
`Event::RunExecute`, which `handle_event` runs via `run_foreground`. This
mirrors the existing `RunPreview` pattern. `execute-silent` is unchanged.
Update ARCHITECTURE.md (event dispatch table, terminal lifecycle, and
cross-reference line numbers) and add tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBQ1chjHyN3JNTWqMMED44
* test(execute): add tmux e2e test; fix reader restart and post-execute repaint
Add a tmux-based integration test (tests/execute.rs, unix-gated so it runs
on the Linux and macOS CI legs) that drives an interactive child through an
`execute` action and asserts it keeps receiving keystrokes, then that skim
is interactive again once the child exits. It covers both the fullscreen and
inline (`--height`) layouts.
Writing the test surfaced two bugs in the execute reader-suspend work that
unit tests could not catch:
1. Reader never resumed. `Tui::stop_and_join` cancels the shared
`CancellationToken`, but `Tui::start` reused that same token — and a
cancelled token stays cancelled — so the respawned reader observed the
cancellation immediately and exited without reading input. `start` now
installs a fresh token on every call (also fixing the latent
restart-while-running path).
2. Post-execute repaint hung when stdout was redirected. The repaint went
through `Event::Redraw` → `tui.clear()`, and ratatui's `Terminal::clear`
queries the cursor position, which crossterm writes to stdout via
`ESC [ 6 n`. skim renders to stderr and its stdout is routinely redirected
(`sk > file`), so the query reached no terminal, got no reply, and stalled
the UI for seconds before erroring out. Replace it with
`Tui::force_full_redraw`, which resets ratatui's diff buffers for a full
repaint with no cursor query and works for both fullscreen and inline
viewports.
Update ARCHITECTURE.md and the cross-reference table accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBQ1chjHyN3JNTWqMMED44
* ci: fix matrix
* fix: pop kitty keyboard flag before entering execute
* chore: remove duplication in backend.rs
* chore: refactor
* fix: kitty maintains a different set of flags in alt screen
---------
Co-authored-by: Claude <noreply@anthropic.com>
* ci: add .deb and .rpm packages to releases via dist
Configure cargo-deb and cargo-generate-rpm to build Linux packages
containing the sk executable, the man pages (sk.1, sk-tmux.1) and the
bash/zsh/fish shell completions.
A new reusable workflow (package.yml) builds both packages and uploads
them under an artifacts-* name. It is wired into the release pipeline as
a dist global-artifacts-job in dist-workspace.toml, and release.yml is
regenerated with `dist generate` (not hand-edited) so dist's host job
attaches the packages to the GitHub Release.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* ci: drop sk-tmux man page from .deb and .rpm packages
Package only the sk.1 man page; the sk-tmux.1 page is no longer shipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* ci: temporarily build release artifacts on PRs
Set dist pr-run-mode = "upload" so the .deb and .rpm (and the other
release artifacts) are built and uploaded on pull requests, allowing the
packages to be downloaded and verified before merging.
This is temporary and should be reverted before merge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* ci: install packaging tools via taiki-e/install-action
Address review feedback on package.yml:
- Install cargo-deb and cargo-generate-rpm with taiki-e/install-action
(prebuilt binaries) plus a Swatinem/rust-cache step, matching the
patterns used in test.yml, instead of compiling them with cargo install.
- Drop the `--output target/debian` flag from `cargo deb`; the default
target/debian/ directory is what the collect step expects.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* ci: build arm64 .deb and .rpm packages too
Turn the package job into a matrix that builds natively for both amd64
(ubuntu-22.04) and arm64 (ubuntu-22.04-arm), producing a .deb and .rpm
per architecture. Artifacts are uploaded under per-arch names so dist's
host job attaches all of them to the release.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* ci: submit releases to winget via winget-releaser
Add a dist publish job that, after the GitHub Release is created, submits
the new version to the Windows Package Manager Community Repository using
vedantmgoyal9/winget-releaser and the Windows .zip artifact dist already
attaches to the release.
Uses the package identifier and installer regex from skim-rs/skim#769:
identifier: skim-rs.skim
installers-regex: '-pc-windows-msvc\.zip$'
Wired in through publish-jobs in dist-workspace.toml; release.yml is
regenerated with `dist generate` (not hand-edited). Prereleases are never
submitted. Requires a WINGET_TOKEN secret (a public_repo-scoped PAT that
owns a microsoft/winget-pkgs fork under skim-rs).
Refs: skim-rs/skim#769
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* ci: validate package version against the release plan
Consume the `plan` input dist passes to the package job: assert the
crate version equals the version dist planned for this release, so the
source-built .deb/.rpm can't silently drift from the release.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* Update .github/workflows/winget.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Update .github/workflows/package.yml
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* docs: document .deb, .rpm, winget and scoop installation
Add the new install methods to the README: winget and Scoop rows plus a
Debian/RPM section with install commands, covering amd64 and arm64.
Also clarify in winget.yml that WINGET_TOKEN must be a classic PAT
(fine-grained tokens can't open the winget-pkgs PR).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmNevtS7EV3vXyYRTVu7Aw
* fix: install cargo-deb manually to avoid libc version mismatch
* ci: use blacksmith runners for long job
* fix: version spec for cargo install
* chore: revert pr action upload
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* 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>
* feat!: feature-gate listen and image to allow opting out
This is breaking since disabling the default features now also disables
those. It is NOT breaking for cli users, only for library ones.
* fix: add warn on listener transfer failure
* wip: stable rust, but no match indices
* feat: use restored indices api
* chore: use crates.io pushed 0.8.0
* chore: generate completions & manpage
* fix: remove nightly-specific coverage annotations
---------
Co-authored-by: Skim bot <skim-bot@skim-rs.github.io>
This PR has grown beyond its initial scope due to me over-optimizing everything, but it leads to:
Paving the way for future actually interactive previews
Consistently better performance than fzf in our bench thanks to thread and concurrency optimizations as well as the use of kanal for the items channels
Given the scope, I'm marking this as breaking because:
setting wrap in the preview window layout disables the pty since we don't want to manipulate the raw buffer to word-wrap it manually
kanal channels work slightly differently and might break library usage, even though switching to them did not require any modifications of the examples so it's unlikely that users will see anything break
* fix: force cwd for preview
* fix: correctly set cwd & kill pty child in the right order
* fix: use std threads & reopen new pty for each preview
* feat: use tui-term for displaying
* feat: scroll in pty
* fix: make nested skim previews work
* fix: clippy mistake
* feat: reactive preview triggering
* chore: generate completions & manpage
* chore: optimizations & thread cleanup
* chore: use kanal for faster channels
* fix: tests
* fix: only send items if the matcher hasn't been killed in the meantime (#947)
* tests: add coverage
* tests: fix bin path with coverage
* tests: upload tests to codecov
* chore: make pty opt-in through preview-window
* chore: generate completions & manpage
---------
Co-authored-by: Skim bot <skim-bot@skim-rs.github.io>
* test: use insta for applicable integration tests, making them cross-platform
* fix: remove @cmd from insta tests
* fix: remove @cmd from insta tests
* fix: use printf instead of echo
