fix(runner): sequential fail_fast job count + README rewrite#11
Merged
Conversation
The sequential executor's run_sequential skipped incrementing jobs_run when fail_fast triggered, because break 'outer jumped past the counter. The parallel executor already counted failed jobs correctly, making the two paths inconsistent. Move the increment after the commands loop and break the outer loop only after counting, matching the parallel executor's behavior. Before this fix, a sequential fail_fast run showed '0 run, 0 skipped' even though a job clearly ran and failed.
Rewrite README.md from scratch with a human tone: lead with the problem (worktree hooks breaking under parallel agents), explain what betterhook does in terms of the problems it solves, add a plain-language 'How it works' walkthrough, and move the comparison table to the end as reference material. Remove generic Conductor mentions from the README, introduction, and quickstart pages. The dedicated agents/conductor.mdx integration guide remains for users of that tool.
Release automation: - release-plz workflow opens a release PR on every master push with auto-generated changelog and version bump from conventional commits - release.yml now publishes both crates to crates.io after creating the GitHub release (library first, CLI after a 30s index delay) - release-plz.toml configures per-crate changelog, tag, and publish settings (xtask excluded from publish) - release.toml updated to enable crates.io publish Dependency automation: - Replace Dependabot with Renovate for Cargo and GitHub Actions deps - Weekly grouped PRs with auto-merge on minor/patch updates - renovate.json with semantic commit prefixes Crates.io metadata: - Add readme field to workspace and both crate Cargo.toml files - Add exclude for tests/benches/fuzz in the library crate - Fix CLI crate description (remove double-dash)
…rivilege permissions Security: - Pin every GitHub Action to its full SHA digest with a version comment (checkout, rust-toolchain, rust-cache, upload-artifact, download-artifact, action-gh-release, release-plz/action). Prevents supply-chain attacks via mutable tags. - Add cargo-deny to CI: checks RustSec advisories for known vulns, enforces an allow-list of licenses (MIT, Apache-2.0, BSD, ISC, Unicode), blocks wildcard version requirements, and rejects deps from unknown registries or git sources. - Scope workflow permissions to least-privilege: CI gets read-only contents, release-plz gets per-job pull-requests:write or contents:write, release gets contents:write only. - Add CODEOWNERS file. Release pipeline: - Split release-plz into two jobs: release-pr (opens version-bump PR) and release (tags + publishes to crates.io on merge). Previously only release-pr ran, so tags and crates.io publish never happened. - Tar and gzip release binaries (.tar.gz) instead of uploading raw executables. Checksums cover the archives. - Remove the manual crates.io publish job from release.yml since release-plz now handles it. Renovate: - Enable vulnerability alerts via :enableVulnerabilityAlerts preset. - Enable helpers:pinGitHubActionDigests so Renovate auto-updates action SHA pins going forward. - Add weekly Cargo.lock refresh via lockFileMaintenance rule.
Add an npm publish job to release.yml that runs after the GitHub Release is created. On every tag push: 1. Build job compiles binaries for 4 targets 2. Release job creates the GitHub Release with tar.gz archives 3. npm job syncs the version from the git tag and publishes the betterhook package to the npm registry The npm package is a thin wrapper (packaging/npm/) that downloads the correct platform binary from GitHub Releases on postinstall. Changes to the npm wrapper: - install.js now downloads .tar.gz archives (matching the new release format) and extracts them with tar - Native binary stored as bin/betterhook-native to avoid overwriting the JS shim at bin/betterhook - JS shim checks for the native binary and gives a clear error if it is missing - setup-node action pinned to SHA
Replace the NPM_TOKEN secret with OIDC-based trusted publishing. GitHub Actions proves its identity directly to npm via an id-token, so no long-lived secrets are stored or rotated. Changes: - Add id-token: write permission to the npm job - Add contents: read (least privilege) - Update npm to latest for OIDC support (requires npm >= 11.5.1) - Publish with --provenance flag for supply chain attestation - Remove NODE_AUTH_TOKEN / NPM_TOKEN entirely
Four action SHAs were invalid (not found in their repos): - Swatinem/rust-cache: fixed to e18b497 (v2.9.1) - actions/upload-artifact: fixed to 043fb46 (v7.0.1) - softprops/action-gh-release: fixed to 3bb1273 (v2.2.1) - release-plz/action: fixed to 1528104 (v0.5) All SHAs verified against their respective repos via the GitHub API.
leonardomso
force-pushed
the
leonardomso/accra
branch
from
83cf893 to
6d3962c
Compare
Three CI failures: 1. security-audit: deny.toml used the deprecated 'unlicensed' key (removed in cargo-deny 0.16+). Removed it; modern cargo-deny denies unlicensed crates by default. 2. msrv-check (Linux): installed_wrapper_handles_empty_git_dir_env panicked with ETXTBSY. The wrapper file was written with std::fs::write then immediately exec'd. On Linux under load the kernel can return 'Text file busy' if the write fd is not fully synced. Fixed by using File::create + sync_all + explicit drop before chmod+exec. 3. check-macos-latest: exclude_filter_blocks_events panicked because macOS FSEvents delivered the directory-creation event before the watcher started filtering. Added a settle delay after mkdir and a registration delay after starting the watcher.
- Add CC0-1.0 to the license allow list (used by notify v8.2.0, a permissive public-domain-equivalent license) - Ignore RUSTSEC-2025-0141 (bincode unmaintained). bincode 2.0.1 is feature-complete for our daemon wire protocol. Will evaluate postcard or bitcode when the protocol changes.
Shell completions: - Add clap_complete dependency and a 'betterhook completions <shell>' subcommand that generates completions for bash, zsh, fish, elvish, and powershell - Add a completions job to the release workflow that builds the CLI, generates bash/zsh/fish completions, and includes them in the GitHub Release as betterhook-completions.tar.gz README install instructions: - Add a proper Install section with three methods: cargo install from crates.io, npm install -g, and from source - Add a Shell completions subsection with copy-paste commands for bash, zsh, and fish, plus a note about pre-built completions in GitHub Releases - Add the completions command to the commands table
TTY path: - Replace println!/eprintln! (lock-per-call) with explicit stdout().lock() / stderr().lock() + write!/writeln!. Each event now acquires the stdio lock once instead of once per format arg. - Write colored fields directly via Display into the locked writer instead of .to_string() intermediates. Removes one heap allocation per colored field per event. - Eliminate format!() intermediates for diagnostic locations and severity strings by writing fields sequentially. JSON path: - Replace serde_json::to_string + println! with serde_json::to_writer directly into locked stdout. Removes the intermediate String allocation on every NDJSON event. Under 8 parallel jobs each producing hundreds of lines, this cuts the per-line overhead from ~3 allocations + 2 lock cycles to 0 allocations + 1 lock cycle.
Visual improvements:
- Add a hook header line ('pre-commit | 3 jobs, parallel') so users
know which hook is running and its execution mode
- Move the command to a separate indented line below the job name,
dimmed, so the job name stands out
- Indent output lines with 4 spaces for visual grouping under their
job's start marker
- Hide 'exit 0' on success (noise). Only show exit code on failure
where it's actionable
- Human-readable durations: '42ms', '1.5s', '2m 30s' instead of
raw milliseconds
- Replace '∘' skip marker with '○' (more visible at small sizes)
- Show 'cached (N files)' instead of 'cache hit (N files)' for
brevity
- Cleaner summary: 'PASS 3 run 212ms' on success, 'FAIL 1 run'
on failure. Skip count only shown when nonzero
- Add blank lines around the header and summary for breathing room
New event:
- OutputEvent::HookStarted { hook, jobs, parallel } emitted at the
start of run_hook, before job resolution. Gives the TTY writer
enough context to render the header line.
P1 fixes: - release-plz release job: add pull-requests: read permission so release-plz can inspect PR metadata when tagging - CLI Cargo.toml: add version = "0.0.2" alongside path dep so cargo publish accepts the crate (path-only deps are rejected) - npm install.js: enforce HTTPS-only redirects with a max redirect count, resolve relative Location headers via URL constructor, replace execSync tar with in-process extraction using Node zlib (no external tar binary dependency) P2 fixes: - CI cargo-deny: run all checks (advisories, licenses, bans, sources) not just advisories+licenses. The bans and sources policies in deny.toml were never evaluated. - Renovate: add top-level lockFileMaintenance.enabled block. The packageRule alone does not enable the feature. P3 fixes: - README: remove duplicate 'Your next git commit' sentence in the quickstart section
bitflags (v1 + v2) and unicode-width (v0.1 + v0.2) are pulled in by notify and miette respectively. We cannot eliminate these duplicates without dropping the dependencies. Changed multiple-versions from 'warn' to 'allow' so the bans check passes.
xtask (publish = false) depends on betterhook via path only without a version constraint. cargo-deny flags this as a wildcard. Set allow-wildcard-paths = true so private crates can use versionless path deps without triggering the wildcard lint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes: a bug fix in the sequential runner and a full README rewrite.
Bug fix: sequential fail_fast job count
The sequential executor skipped incrementing
jobs_runwhenfail_fasttriggered abreak 'outer, jumping past the counter. A sequential hook withfail_fast = truethat failed on its first job showed0 run, 0 skippedin the summary -- even though the job clearly ran and failed.The parallel executor already counted failed jobs correctly (incrementing before checking failure), making the two code paths inconsistent.
Fix: Move the increment after the commands loop and break the outer loop only after counting. Now both executors report the same way.
Before:
FAIL -- 0 run, 0 skippedAfter:
FAIL -- 1 run, 0 skippedREADME rewrite
The old README read like AI-generated marketing copy. The new one:
Also cleaned up Conductor references in intro and quickstart docs (the dedicated
agents/conductor.mdxintegration guide is untouched).What changed
apps/betterhook/src/runner/executor.rs-- fix sequential fail_fast counting + update test assertionREADME.md-- full rewriteapps/docs/introduction.mdx-- remove generic Conductor mentionapps/docs/quickstart.mdx-- remove generic Conductor mentionValidation
End-to-end tested all CLI commands against a real git repo (63 manual tests covering init, install, uninstall, run, explain, fix, status, doctor, cache, builtins, import, worktree isolation, DAG scheduling, fail_fast, streaming output, error handling).