From 395bc2fcf6e87e831bf9c4a9d6058b193054ef8d Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:37:22 -0300 Subject: [PATCH 1/2] Finish whole-repository checks: group levels leave opt-in rules off, a status line, budget and key errors said once A level for maintainability turned on hardcoded values, which left the default rules in 0.26: on a 950-file project it doubled a whole check, 1,846 requests where the default rules need 943, and a jevgate.toml ceiling of 1,000 stopped it halfway. A group now turns on the rules it runs by default, or every rule of an opt-in group. A check draws a status line on a terminal, says before its first request when its request budget cannot cover it, names where the budget is set (the old message advised a flag that cannot raise jevgate.toml's ceiling), ends in one line without a key, and reads the macOS Keychain without a terminal, as Git and agent hooks run. --- CHANGELOG.md | 12 +++ Cargo.lock | 1 + Cargo.toml | 4 + site/src/configuration.md | 10 +- site/src/git-hooks.md | 2 +- site/src/output.md | 4 +- site/src/troubleshooting.md | 4 +- src/auth/store.rs | 50 ++++++++- src/catalog.rs | 40 +++++++- src/check.rs | 13 ++- src/config.rs | 98 +++++++++++++++--- src/evaluate.rs | 33 ++++-- src/hook/outage.rs | 4 + src/init.rs | 55 ++++++---- src/main.rs | 3 + src/options/mod.rs | 9 +- src/output/agent.rs | 47 +++++++-- src/progress.rs | 197 ++++++++++++++++++++++++++++++++++++ src/requests.rs | 64 +++++++++++- src/tests/mod.rs | 30 ++++++ src/transport/mod.rs | 11 ++ tests/cli/auth.rs | 12 +++ 22 files changed, 628 insertions(+), 75 deletions(-) create mode 100644 src/progress.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 07386e6..75e29cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ Notable changes to JevGate. Versions follow [Semantic Versioning](https://semver ## [Unreleased] +A whole-repository check now finishes where it stopped halfway, says what it is doing while it runs, and fails at once, in one line, without a key. Measured on a 950-file TypeScript and Python project whose jevgate.toml, written for 0.8, set `maintainability = "consider"`, `max_requests = 1000` and `concurrency = 4`: 0.31.0 planned 1,126 first-pass requests, spent its 1,000 on them in 70 silent seconds and left 377 files unchecked, advising a larger `--max-requests`, which could not raise the ceiling jevgate.toml set; this version asks 930 requests in 73 seconds from an empty cache, drawing a status line all along, and finishes ($0.05). A rerun from the cache takes 2 seconds. + +### Checks of the whole repository + +- A group's level no longer turns on its opt-in rules. `maintainability = "consider"` in `[rules]`, `rules = ["maintainability"]` and `--rule maintainability` turn on file organization, function simplification and shared logic, not hardcoded values, which left the default rules in 0.26 for being right 6 times in 37 on projects JevGate was never tuned on; it runs when named (`"maintainability/hardcoded-values" = "consider"`, `--rule hardcoded-values`) or with `all`. A group with no rule on by default, such as `security` or `documentation`, still turns on every rule of it, and skipping a group still skips all of it. On the project above, the group's level asked 1,846 requests for a whole check where its three default rules need 943: hardcoded values asks about every module constant and every function with a literal, and rechecks and locates many of them. `jevgate init` writes hardcoded values on a line of its own, and a jevgate.toml written by `init` before 0.26 is no longer said to judge it. +- A check says what it is doing on a terminal: one line on stderr, drawn again every eighth of a second, with the stage (reading files, planning, first pass, rechecking undecided units, locating findings…), the requests of that stage answered so far and the time, such as `JevGate · first pass · 312/768 answered · 23s`. Every other line JevGate prints erases it first, and it is gone before the findings. It is not drawn in CI, with `--watch` or `--format jsonl`, when stderr is not a terminal, or for a check that ends within 0.4 seconds. +- A check whose request budget cannot cover it says so on stderr before its first request, with the number of requests it needs at least, and each request left unsent names the budget and where it is set: `Request budget reached (max_requests = 1000 in jevgate.toml); rerun to continue from the cached answers, or raise the budget`. The old message advised a larger `--max-requests`, which can only lower the ceiling jevgate.toml sets. +- Without a key, a check ends at once with one line, `jevgate: No API key configured. Run jevgate auth login, …`, and exit 2, where it reported 660 of the project's 950 files failed, 38 of them for a request budget no request had been sent against. The report still records each file's error, a hook still lets the change through, saying why, and a watcher keeps looking for a key. +- On macOS, the key `jevgate auth login` saved in the Keychain is read without a terminal: Git hooks and coding agents' hooks, whose stdin is not a terminal, did not find it and let every change through unchecked. Without a terminal the Keychain's dialog is turned off, so a read macOS would ask about (the first by a newly upgraded binary) fails at once, saying to run `jevgate auth status` in a terminal once and choose Always Allow, instead of waiting on a dialog nobody may see. +- The agent text marks a finding of a run whose gate was not evaluated, such as one left incomplete, `(would fail the gate)`, not `(fails the gate)`. +- The configuration example and `jevgate init` suggest `max_cost` to bound a check's spend, rather than a `max_requests` sized for pull requests, which stops a check of the whole repository. + ## [0.31.0] - 2026-09-28 0.31.0 brings the gate to the roadmap's third moment, the commit: Git hooks judge what a push sends or a commit records, read from Git, and a check that cannot finish lets the change through and says so. Releases now stage the npm package for the maintainer's approval. diff --git a/Cargo.lock b/Cargo.lock index e0e4057..8c92491 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -979,6 +979,7 @@ dependencies = [ "rpassword", "schemars", "secret-service", + "security-framework", "serde", "serde_json", "sha2", diff --git a/Cargo.toml b/Cargo.toml index 99b48b6..72159b5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,10 @@ schemars = "=1.2.2" libc = "=0.2.189" signal-hook = { version = "=0.4.4", default-features = false } +# Keychain reads without a terminal turn macOS's dialog off (`auth::store`). +[target.'cfg(target_os = "macos")'.dependencies] +security-framework = { version = "=3.7.0", default-features = false } + [target.'cfg(all(unix, not(any(target_os = "macos", target_os = "ios", target_os = "android"))))'.dependencies] secret-service = { version = "=5.2.0", features = ["rt-async-io-crypto-rust"] } zbus = "=5.19.0" diff --git a/site/src/configuration.md b/site/src/configuration.md index 178dcfe..095b9a3 100644 --- a/site/src/configuration.md +++ b/site/src/configuration.md @@ -6,13 +6,13 @@ upload_allow = ["src/**", "tests/**"] # only these paths may be uploaded upload_deny = ["**/.env*", "**/*.pem", "**/*.key"] include_tests = true -max_requests = 300 +max_cost = 1.00 # dollars a check may spend; a whole-repository check costs a few cents [rules] # a level per group or rule -maintainability = "review" # judge every rule of the group, and fail on its reviews +maintainability = "review" # judge the group's default rules, and fail on their reviews tests = "consider" security = "mature" # opt-in group, enabled by naming it; fails only on levels measured mature -"maintainability/hardcoded-values" = "report" # judge but never fail; "off" skips it +"maintainability/hardcoded-values" = "report" # an opt-in rule runs only when named: judge but never fail; "off" skips it [[scope]] # levels for the files these paths match paths = ["scripts/**", "tools/**"] @@ -27,13 +27,13 @@ rules = { security = "consider" } # except these | `generated` | built-in names | Globs of generated files, which are skipped | | `tests` | built-in conventions | Globs of additional test files | | `context` | none | Files always sent as related evidence, like `--context` | -| `rules` | the `default` group | A list selects rules. A table gives each group or rule a level: `review`, `consider`, `mature`, `uncertain`, `report` (judge, never fail) or `off`; a level for a group judges every rule of it, opt-in ones included | +| `rules` | the `default` group | A list selects rules. A table gives each group or rule a level: `review`, `consider`, `mature`, `uncertain`, `report` (judge, never fail) or `off`; a level for a group judges the rules it runs by default (`maintainability` leaves out the opt-in hardcoded values, which runs only when named), and every rule of a group with none on by default, such as `security` | | `[[scope]]` | none | `paths` (globs), with `fail_on` for every rule and `rules` for rules or groups, as above; `off` is not accepted (use `upload_deny`). The last scope that matches a file and addresses a rule wins; flags win over scopes | | `fail_on` | `["mature"]` | The level for rules without their own, like `--fail-on` | | `include_tests` | `false` | Judge tests, like `--include-tests` | | `model` | the key's provider's | The model, as the key's provider names it: `jev-1.13.0` for TypeSafe, `typesafe/jev-1.13` for OpenRouter, `typesafe-ai/jev` for Vercel AI Gateway. A pinned version keeps results repeatable; a repository that sets it for one provider needs `--model` with another provider's key | | `cache_ttl_secs` | `3600` | Cache lifetime for an alias: a model name without an `x.y.z` version, such as `jev-latest` or `jev-1.13`. Pinned versions such as `jev-1.13.0` never expire | -| `max_requests` | unlimited | Ceiling on API attempts per invocation | +| `max_requests` | unlimited | Ceiling on API attempts per invocation; `--max-requests` can only lower it. A whole-repository check asks about one request per file, and more with opt-in rules, so a ceiling sized for pull requests stops it: a check that will not fit says so before its first request. Prefer `max_cost` to bound spend | | `max_seconds` | `60` with `--staged` and `--pre-push`, else unlimited | Ceiling on the seconds a check asks for: no request starts, and no retry waits, past it, and what is left unasked leaves the run incomplete; `--max-seconds` can only lower it | | `max_cost` | unlimited | Ceiling on a check's estimated spend in dollars: each request is priced from its size before it is sent, stderr says when 75% and 90% are spent, and what would pass it is left unasked, leaving the run incomplete; `--max-cost` can only lower it | | `on_incomplete` | `"pass"` with `--staged` and `--pre-push`, else `"fail"` | What a run that cannot finish exits with, like `--on-incomplete`: `"fail"` exits 2; `"pass"` exits 0 and says on stderr that the change was not checked, and why ([Git hooks](git-hooks.md#when-the-check-cannot-finish)) | diff --git a/site/src/git-hooks.md b/site/src/git-hooks.md index 19a63c0..6fc0347 100644 --- a/site/src/git-hooks.md +++ b/site/src/git-hooks.md @@ -38,7 +38,7 @@ jevgate: it goes ahead unchecked, as on_incomplete is "pass"; set on_incomplete ## Recipes -The hooks need JevGate 0.31.0 or later on the PATH (or built by pre-commit), and a key: `jevgate auth login` saves one, or set `TYPESAFE_API_KEY`, `OPENROUTER_API_KEY` or `AI_GATEWAY_API_KEY`. +The hooks need JevGate 0.31.0 or later on the PATH (or built by pre-commit), and a key: `jevgate auth login` saves one, or set `TYPESAFE_API_KEY`, `OPENROUTER_API_KEY` or `AI_GATEWAY_API_KEY`. On macOS a hook reads the saved key from the Keychain since 0.32.0 (0.31.0 needed a terminal, which a hook lacks). After an upgrade macOS asks once whether the new `jevgate` may read it: a hook cannot answer, so run `jevgate auth status` in a terminal and choose Always Allow. ### pre-commit and prek diff --git a/site/src/output.md b/site/src/output.md index 895c40f..69a161e 100644 --- a/site/src/output.md +++ b/site/src/output.md @@ -23,11 +23,13 @@ Findings are `review` (act on it), `consider` (worth a look) or `note` (optional `jevgate hook` is the exception: it exits 0 whatever happens, because agents read exit 2 as "block", and its JSON reply says what happened ([Coding agents](coding-agents.md)). +While a check runs on a terminal, one line on stderr says what it is doing, how many of that stage's requests are answered and for how long it has run: `JevGate · first pass · 312/768 answered · 23s`. It is erased before anything else is printed, and it is not drawn in CI, with `--watch` or `--format jsonl`, or when stderr is not a terminal. + After the findings, the agent text gives each reason files failed or were skipped, with how many files give it, such as `Failed 3: TypeSafe HTTP 402 (credits exhausted; …)`, so an incomplete run says why without `--verbose`, and so does the MCP server's `jevgate_check`, which returns this text. `--fail-on review|consider|mature|uncertain|none` sets what fails the gate; `--fail-on security=consider` sets it for one group or rule. The default, `mature`, fails only on the rules and levels measured right at least 80% of the time on projects JevGate was never tuned on, never on a finding in a [preview language](languages.md#support-levels), and on each [custom question](custom-questions.md)'s own level; `jevgate rules` lists them, and [configuration](configuration.md#what-fails-the-check-by-default) explains it. Baselined findings, findings allowed by a comment, and notes never fail the gate. -The agent text marks each finding that fails the gate with `(fails the gate)`, and says when reviews did not fail it because their rules are still being measured or their files' languages are in preview. The JSON report records how the gate counted each new finding in its `gate` field: `fails`, `measuring` (reported without failing: the level is `mature` and its rule and level are still being measured, or its file's language is in preview) or `advisory` (the level in force does not count it, as `review` does not count a consider). `fail_on_mature` says what `mature` stands for among the selected rules. +The agent text marks each finding that fails the gate with `(fails the gate)`, or `(would fail the gate)` in a run whose gate was not evaluated, such as one left incomplete, and says when reviews did not fail it because their rules are still being measured or their files' languages are in preview. The JSON report records how the gate counted each new finding in its `gate` field: `fails`, `measuring` (reported without failing: the level is `mature` and its rule and level are still being measured, or its file's language is in preview) or `advisory` (the level in force does not count it, as `review` does not count a consider). `fail_on_mature` says what `mature` stands for among the selected rules. A single finding can also be accepted where it is, with a comment on its line or directly above it (doc comments and attributes may sit in between). The comment names a rule ID (`security/injection`), its name (`injection`), its key or a group (a [custom question](custom-questions.md) by its ID, `custom/no-body-logs`, or `custom`), and needs a reason; without one it is ignored and the finding says so: diff --git a/site/src/troubleshooting.md b/site/src/troubleshooting.md index 8842058..c883de9 100644 --- a/site/src/troubleshooting.md +++ b/site/src/troubleshooting.md @@ -36,8 +36,8 @@ A provider error ends with the provider's request id when it sent one (`; reques **`jevgate: this commit was not checked: …` or `this push was not checked: …`** : A [Git hook](git-hooks.md)'s check could not finish, for the reason given, and let the change through, as `on_incomplete = "pass"` does by default for `--staged` and `--pre-push`. Fix the cause (a key, credits, a configuration that loads) and check again with `jevgate check --base `, or set `on_incomplete = "fail"` for a hook that stops the change instead. `it used its 60 seconds (max_seconds)` means the provider did not answer in time: the answers received are cached, so the next run asks only for the rest; raise `max_seconds` for larger pushes. -**`Session API request budget exhausted; restart with an explicit larger --max-requests`** -: `max_requests` or `--max-requests` capped the run. Raise it, or check fewer files with `--base` or paths; `--dry-run` estimates what a run will ask. +**`Request budget reached (max_requests = N in jevgate.toml); rerun to continue from the cached answers, or raise the budget`** +: `max_requests` in jevgate.toml, or `--max-requests`, capped the run; the message names which. A flag can only lower the ceiling jevgate.toml sets, so raise `max_requests` there. The answers the run got are cached, so a rerun asks only for the rest. A check that cannot fit its budget says so on stderr before its first request, with the number it needs at least. A whole-repository check asks about one request per file, and more with opt-in rules: `--dry-run` counts the first pass, and `max_cost` bounds spend without stopping large checks. **`Another JevGate session owns latest.json`** : Another `check` or `--watch` is running in the same repository. Stop it first. diff --git a/src/auth/store.rs b/src/auth/store.rs index b91d55e..696698c 100644 --- a/src/auth/store.rs +++ b/src/auth/store.rs @@ -33,17 +33,54 @@ pub trait Backend { } pub struct NativeBackend; + +/// Whether a person answers at this terminal: stdin and stderr are both one. +fn interactive() -> bool { + std::io::stdin().is_terminal() && std::io::stderr().is_terminal() +} + impl NativeBackend { fn entry(&self) -> Result { // Credential providers may open system dialogs. Only Windows' Credential // Manager is used through this interface without an interactive terminal. ensure!( - cfg!(windows) || (std::io::stdin().is_terminal() && std::io::stderr().is_terminal()), + cfg!(windows) || interactive(), "System credential operation requires a terminal; use --storage file or TYPESAFE_API_KEY for automation" ); + Self::unchecked_entry() + } + + fn unchecked_entry() -> Result { keyring::Entry::new("jevgate", "typesafe-api-key") .map_err(|_| anyhow::anyhow!("System credential store is unavailable or locked")) } + + /// The saved key from the macOS Keychain. Git hooks and coding agents' + /// hooks run without a terminal (Git passes a pre-push hook its refs on + /// stdin, and an agent its event), and a read that required one never + /// found the key `jevgate auth login` saved. Without a terminal the + /// Keychain's own dialog is turned off, so a read macOS would ask about, + /// such as the first by a newly upgraded binary, fails at once instead + /// of waiting on a dialog nobody may be watching. + #[cfg(target_os = "macos")] + fn keychain_get() -> Result>> { + let asking = interactive(); + let _quiet = if asking { + None + } else { + security_framework::os::macos::keychain::SecKeychain::disable_user_interaction().ok() + }; + match Self::unchecked_entry()?.get_password() { + Ok(value) => Ok(Some(Zeroizing::new(value))), + Err(keyring::Error::NoEntry) => Ok(None), + Err(_) if asking => bail!( + "Cannot read the system credential store; unlock it or run jevgate auth login" + ), + Err(_) => bail!( + "macOS asks before this jevgate reads the key saved by jevgate auth login, which it cannot without a terminal; run jevgate auth status in a terminal once and choose Always Allow, or set TYPESAFE_API_KEY" + ), + } + } } impl Backend for NativeBackend { fn get(&self) -> Result>> { @@ -52,9 +89,14 @@ impl Backend for NativeBackend { not(any(target_os = "macos", target_os = "ios", target_os = "android")) ))] return super::native_unix::get(); - #[cfg(not(all( - unix, - not(any(target_os = "macos", target_os = "ios", target_os = "android")) + #[cfg(target_os = "macos")] + return Self::keychain_get(); + #[cfg(not(any( + target_os = "macos", + all( + unix, + not(any(target_os = "macos", target_os = "ios", target_os = "android")) + ) )))] match self.entry()?.get_password() { Ok(value) => Ok(Some(Zeroizing::new(value))), diff --git a/src/catalog.rs b/src/catalog.rs index 7c2b414..c4df49a 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -339,6 +339,44 @@ pub fn select_in(rules: &[Rule], name: &str) -> Option> { (!selected.is_empty()).then_some(selected) } +/// The keys of the rules among `rules` that naming `name` turns on: as +/// [`select_in`], but a group turns on only the rules it runs by default +/// when it has any, so an opt-in rule of a default group (hardcoded values) +/// runs only when its own name or `all` asks for it. A group with none, such +/// as `security`, turns on every rule of it. Before 0.32 a level for +/// `maintainability` turned hardcoded values on: a configuration written for +/// 0.8 that way asked 1,846 requests of a 950-file project where its three +/// default rules needed 943, for a rule right 17% of the time. +pub fn enable_in(rules: &[Rule], name: &str) -> Option> { + let selected = select_in(rules, name)?; + Some( + selected + .into_iter() + .filter(|key| { + rules + .iter() + .find(|r| r.key == *key) + .is_none_or(|rule| enabled_by(name, rule, rules)) + }) + .collect(), + ) +} + +/// Whether naming `name`, which addresses `rule`, turns it on: always, +/// except for an opt-in rule named by a group that runs rules by default. +fn enabled_by(name: &str, rule: &Rule, rules: &[Rule]) -> bool { + rule.default_enabled + || name != rule.group + || !rules + .iter() + .any(|other| other.group == rule.group && other.default_enabled) +} + +/// Whether a level set for `name` turns `rule` on, as [`enable_in`] does. +pub fn enables(name: &str, rule: &Rule, rules: &[Rule]) -> bool { + specificity(name, rule) > 0 && enabled_by(name, rule, rules) +} + /// The built-in rules, then the custom questions in the order they are /// defined. pub fn with_custom(questions: &'static [crate::custom::Question]) -> Vec { @@ -466,7 +504,7 @@ pub fn table(questions: &'static [crate::custom::Question]) -> String { crate::maturity::MIN_LABELS )); lines.push(format!( - "Groups: {}, {DEFAULT_GROUP} (every rule marked yes or tests), {ALL_GROUP}.", + "Groups: {}, {DEFAULT_GROUP} (every rule marked yes or tests), {ALL_GROUP}. A group turns on its rules marked yes or tests, or every rule of it when it has none (security, documentation); an opt-in rule of another group runs when named, or with {ALL_GROUP}.", groups.join(", ") )); lines.push("Select with --rule and --skip-rule, or [rules] in jevgate.toml; `tests` rules need --include-tests. --fail-on and [rules] levels replace the default gate.".into()); diff --git a/src/check.rs b/src/check.rs index 2a73192..70bc6aa 100644 --- a/src/check.rs +++ b/src/check.rs @@ -7,7 +7,7 @@ use crate::{ hook::outage, html_report, inventory, options::{CheckArgs, Format}, - output, + output, progress, revision::{self, Now, Recorded, push}, schema, storage, token_budget, transport, watch, }; @@ -124,6 +124,8 @@ pub fn session<'a>( observed: (0, 0), answered: Default::default(), spend: args.max_cost.map(crate::requests::Spend::new), + halted: None, + budget_noted: false, } } @@ -253,6 +255,8 @@ pub fn run(args: &CheckArgs, context: &ConfigContext) -> Result { validate(args)?; cancellation::install()?; unasked(args); + let progress = + progress::start(progress::wanted() && !args.watch && args.output_format() != Format::Jsonl); let scope = inventory::scope(args, context)?; let inputs = inventory::collect(args, context, &scope)?; let store = if args.dry_run { @@ -263,6 +267,7 @@ pub fn run(args: &CheckArgs, context: &ConfigContext) -> Result { let (previous, mut report) = first_snapshot(args, context, &inputs); if args.dry_run { evaluate::preview_guards(&mut report, args, context, &scope); + drop(progress); output::emit(&report, args)?; return Ok(0); } @@ -287,6 +292,12 @@ pub fn run(args: &CheckArgs, context: &ConfigContext) -> Result { }; let mut session = session(args, context, &store, &mut evaluator); judge(&mut session, &inputs, previous.as_ref(), &mut report)?; + drop(progress); + if let Some(error) = session.halted.take().filter(|_| !args.watch) { + // No request could be sent, as without a key: say why once, not + // once per file. The report keeps each file's error. + return Err(error); + } if args.moment().is_some() && waiting.is_none() { remember_outage(&context.root, &report, &watch); } diff --git a/src/config.rs b/src/config.rs index 145326d..601b0b4 100644 --- a/src/config.rs +++ b/src/config.rs @@ -258,7 +258,7 @@ impl ConfigContext { let mut enabled = if from_file { configured_rules(&rules, &self.config.rules)? } else { - expand(&rules, &args.rules)?.into_iter().collect() + expand_enabled(&rules, &args.rules)?.into_iter().collect() }; let skipped = expand(&rules, &args.skip_rules)?; for rule in &skipped { @@ -365,6 +365,7 @@ impl ConfigContext { /// Configuration is a ceiling; CLI flags may narrow but cannot bypass upload budgets. fn configure_budgets(&self, args: &mut CheckArgs) -> Result<()> { if let Some(n) = self.config.max_requests { + args.max_requests_in_config = args.max_requests.is_none_or(|limit| limit >= n); args.max_requests = Some(args.max_requests.map_or(n, |limit| limit.min(n))); } if let Some(n) = self.config.concurrency { @@ -481,6 +482,17 @@ fn expand(rules: &[catalog::Rule], names: &[String]) -> Result Ok(keys) } +/// The keys of the rules that naming `names` turns on: a group's opt-in +/// rules only when the group runs none by default ([`catalog::enable_in`]). +fn expand_enabled(rules: &[catalog::Rule], names: &[String]) -> Result> { + let mut keys = Vec::new(); + for name in names { + let selected = catalog::enable_in(rules, name).ok_or_else(|| unknown(rules, name))?; + keys.extend(selected); + } + Ok(keys) +} + /// Keys of `rules` that `[rules]` enables: those its list names, else the /// `default` group with each rule turned on or off by its most specific /// level. @@ -491,12 +503,12 @@ fn configured_rules(rules: &[catalog::Rule], configured: &Rules) -> Result (&default[..], None), Rules::Levels(levels) => (&default[..], Some(levels)), }; - let mut enabled: BTreeSet<_> = expand(rules, names)?.into_iter().collect(); + let mut enabled: BTreeSet<_> = expand_enabled(rules, names)?.into_iter().collect(); for rule in rules { - match levels.and_then(|levels| most_specific(levels, rule)) { - Some(level) if level.off() => enabled.remove(rule.key), - Some(_) => enabled.insert(rule.key), - None => false, + match levels.and_then(|levels| most_specific_entry(levels, rule)) { + Some((_, level)) if level.off() => enabled.remove(rule.key), + Some((name, _)) if catalog::enables(name, rule, rules) => enabled.insert(rule.key), + _ => false, }; } Ok(enabled) @@ -537,11 +549,19 @@ fn unknown(rules: &[catalog::Rule], name: &str) -> anyhow::Error { /// The entry that addresses `rule` most specifically: its ID or key, its /// group, then `default` or `all`. fn most_specific<'a, T>(entries: &'a BTreeMap, rule: &catalog::Rule) -> Option<&'a T> { + most_specific_entry(entries, rule).map(|(_, value)| value) +} + +/// [`most_specific`] with the name it is set for. +fn most_specific_entry<'a, T>( + entries: &'a BTreeMap, + rule: &catalog::Rule, +) -> Option<(&'a str, &'a T)> { entries .iter() .filter(|(name, _)| catalog::specificity(name, rule) > 0) .max_by_key(|(name, _)| catalog::specificity(name, rule)) - .map(|(_, value)| value) + .map(|(name, value)| (name.as_str(), value)) } /// The groups `jevgate init` gave a `review` level before 0.26: every group @@ -551,8 +571,9 @@ const INIT_REVIEW_GROUPS: [&str; 2] = ["maintainability", "tests"]; /// What to tell a user whose jevgate.toml still holds the `[rules]` lines /// `jevgate init` wrote before 0.26, such as `maintainability = "review" # /// file-organization, …`. They keep every review of those groups failing -/// the check and judge hardcoded values, which 0.26's measured default gate -/// and rules leave out, and most configurations were written that way. The +/// the check, which 0.26's measured default gate leaves out, and most +/// configurations were written that way. (They judged hardcoded values +/// too, until 0.32: a group's level no longer turns its opt-in rules on.) The /// comment listing the group's rules tells them from a level set by hand, /// so deleting it keeps the level without the notice. fn written_before_mature(text: &str) -> Option { @@ -575,13 +596,8 @@ fn written_before_mature(text: &str) -> Option { .iter() .map(|group| format!("`{group} = \"review\"`")) .collect(); - let hardcoded = if groups.contains(&"maintainability") { - ", and hardcoded values is judged" - } else { - "" - }; Some(format!( - "jevgate.toml keeps {} as `jevgate init` wrote {them} before 0.26: every {} review fails the check{hardcoded}. Delete {them} for the default rules and gate, which fails only on rule levels measured right at least 80% of the time; to keep {them}, delete {their} and this notice stops.", + "jevgate.toml keeps {} as `jevgate init` wrote {them} before 0.26: every {} review fails the check. Delete {them} for the default rules and gate, which fails only on rule levels measured right at least 80% of the time; to keep {them}, delete {their} and this notice stops.", lines.join(" and "), groups.join(" and "), )) @@ -700,7 +716,7 @@ mod tests { let notice = written_before_mature(before).unwrap(); assert!( notice.starts_with( - "jevgate.toml keeps `maintainability = \"review\"` and `tests = \"review\"` as `jevgate init` wrote them before 0.26: every maintainability and tests review fails the check, and hardcoded values is judged. Delete them" + "jevgate.toml keeps `maintainability = \"review\"` and `tests = \"review\"` as `jevgate init` wrote them before 0.26: every maintainability and tests review fails the check. Delete them" ), "{notice}" ); @@ -754,6 +770,56 @@ mod tests { ); } + #[test] + fn a_group_turns_on_the_rules_it_runs_by_default_and_an_opt_in_group_every_rule() { + let on = |file: &str, rules: &[&str]| configured(file, rules, &[]).unwrap().rules; + let maintainability = [ + catalog::FILE_ORGANIZATION, + catalog::FUNCTION_SIMPLIFICATION, + catalog::SHARED_LOGIC, + ]; + assert_eq!(on("[rules]\nmaintainability = \"consider\"\n", &[]), { + let mut expected = maintainability.to_vec(); + expected.extend([catalog::TEST_VALUE, catalog::TEST_REDUNDANCY, catalog::LAWS]); + expected + }); + assert_eq!(on("rules = [\"maintainability\"]\n", &[]), maintainability); + assert_eq!(on("", &["maintainability"]), maintainability); + for named in [ + on( + "[rules]\nmaintainability = \"consider\"\n\"maintainability/hardcoded-values\" = \"report\"\n", + &[], + ), + on("[rules]\nall = \"report\"\n", &[]), + on("", &["maintainability", "hardcoded-values"]), + on("", &["all"]), + ] { + assert!( + named.iter().any(|r| r == catalog::HARDCODED_VALUES), + "{named:?}" + ); + } + let security = on("[rules]\nsecurity = \"mature\"\n", &[]); + for rule in catalog::SECURITY + .into_iter() + .chain([catalog::ACCESS_CONTROL, catalog::WORKFLOWS]) + { + assert!(security.iter().any(|r| r == rule), "{rule}: {security:?}"); + } + // Skipping a group still skips every rule of it. + let mut args = crate::tests::args(); + args.rules = vec!["all".into()]; + args.skip_rules = vec!["maintainability".into()]; + context("").unwrap().configure(&mut args).unwrap(); + assert!( + args.rules + .iter() + .all(|r| !maintainability.contains(&r.as_str()) && r != catalog::HARDCODED_VALUES), + "{:?}", + args.rules + ); + } + #[test] fn the_command_line_wins_over_the_file_and_targets_win_over_every_rule() { let file = "[rules]\nmaintainability = \"review\"\ntests = \"report\"\n"; diff --git a/src/evaluate.rs b/src/evaluate.rs index d3aae3f..190482d 100644 --- a/src/evaluate.rs +++ b/src/evaluate.rs @@ -26,8 +26,16 @@ pub struct Session<'a> { pub answered: crate::requests::Answered, /// With `--max-cost`, what the requests sent are estimated to cost. pub spend: Option, + /// Why nothing could be sent, such as a missing key: every request + /// after it fails with it unsent, and `check` ends with it alone. + pub halted: Option, + /// Whether stderr has said the request budget will stop this check. + pub budget_noted: bool, } +/// A round of follow-up requests, planned from the answers so far. +type FollowUps = fn(&crate::units::Plan, &[FileResult]) -> Vec; + pub struct SnapshotContext<'a> { pub root: &'a std::path::Path, pub generation: u64, @@ -288,17 +296,22 @@ fn cached_purpose( impl Session<'_> { pub fn evaluate(&mut self, inputs: &[Input], report: &mut Report) -> Result<()> { self.evaluator.begin_review(); + // A watcher's next snapshot looks for a key again. + self.halted = None; self.publish(report)?; let (purpose, mut views) = self.schedule_files(inputs, report); if !purpose.is_empty() { + crate::progress::phase("asking what test files hold"); self.resolve_purposes(inputs, report, purpose, &mut views)?; } + crate::progress::phase("planning"); let mut plan = crate::units::plan(inputs, &views, self.args, &self.budget, &self.context.root); record_plan(&plan, report); for &owner in plan.files.keys() { report.files[owner].cached = true; } + crate::progress::phase("first pass"); let first: Vec<_> = plan.requests.iter().map(Task::unit).collect(); let oversized = self.dispatch(report, first, |file, asked, body| { crate::units::record(file, &asked, body) @@ -311,27 +324,29 @@ impl Session<'_> { // locate follow-ups then point split findings at a block, and a // located value is asked what it is. Each depends on the answers // before it. - for follow_up in [ - crate::units::doc_checks, - crate::units::traces, - crate::units::rechecks, - crate::units::settles, - crate::units::kinds, - crate::units::parts, - crate::units::locates, - crate::units::value_kinds, + for (phase, follow_up) in [ + ("checking documents", crate::units::doc_checks as FollowUps), + ("tracing values", crate::units::traces), + ("rechecking undecided units", crate::units::rechecks), + ("settling security checks", crate::units::settles), + ("asking what outlines are", crate::units::kinds), + ("asking about file parts", crate::units::parts), + ("locating findings", crate::units::locates), + ("asking what values are", crate::units::value_kinds), ] { let tasks: Vec<_> = follow_up(&plan, &report.files) .iter() .map(Task::unit) .collect(); if !tasks.is_empty() { + crate::progress::phase(phase); let oversized = self.dispatch(report, tasks, |file, asked, body| { crate::units::record(file, &asked, body) })?; unsent_units(&mut plan, report, oversized); } } + crate::progress::phase("composing findings"); compose_files(&plan, report); self.guard(&plan, report); self.calibrate()?; diff --git a/src/hook/outage.rs b/src/hook/outage.rs index 5c32166..befda5d 100644 --- a/src/hook/outage.rs +++ b/src/hook/outage.rs @@ -74,6 +74,10 @@ impl Evaluator for Watched<'_> { self.inner.begin_review(); } + fn unavailable(&mut self) -> Option { + self.inner.unavailable() + } + fn evaluate(&mut self, request: &Value) -> Result { self.watch.sent.fetch_add(1, Ordering::Relaxed); let result = self.inner.evaluate(request); diff --git a/src/init.rs b/src/init.rs index d400bd5..c83de0d 100644 --- a/src/init.rs +++ b/src/init.rs @@ -112,15 +112,19 @@ upload_deny = ["**/.env*", "**/*.pem", "**/*.key"] # OpenRouter, typesafe-ai/jev for Vercel AI Gateway. # model = "{model}" -# Budgets for one invocation; flags can only lower them. -# max_requests = 200 -# concurrency = 4 +# Budgets for one invocation; flags can only lower them. A check of the +# whole repository asks about one request per file, so a request ceiling +# sized for pull requests stops it; max_cost bounds the spend instead (a +# whole check of a 1,000-file project costs a few cents). +# max_cost = 1.00 +# max_seconds = 300 # Unset, the default rules run and only rule levels measured right at least # 80% of the time on projects JevGate was never tuned on fail the check # ("mature"; `jevgate rules` shows them), never in a preview language; other # findings are reported without failing it. A group or rule ID set to a -# level is judged, every rule of a group included, and fails the check at +# level is judged, a group's opt-in rules only when named on their own +# (security and documentation are opt-in whole), and fails the check at # exactly that level: "review", "consider" (also fails on review), "mature", # "uncertain", "report" (judge, never fail) or "off". A rule's own entry wins # over its group's. Test rules also need include_tests or --include-tests. @@ -155,31 +159,38 @@ upload_deny = ["**/.env*", "**/*.pem", "**/*.key"] ) } -/// A commented `[rules]` line for one group: a level to set, and its rules, -/// with the ones that do not run by default marked opt-in. +/// A commented `[rules]` line for one group: a level to set, and the rules +/// it turns on; then a line of its own for each opt-in rule of a group that +/// runs rules by default, which the group's level leaves out. fn group_example(group: &str) -> String { let members: Vec<_> = catalog::rules() .into_iter() .filter(|r| r.group == group) .collect(); let opt_in = members.iter().all(|r| !r.default_enabled); + let name = |r: &catalog::Rule| { + r.id.trim_start_matches(&format!("{group}/")[..]) + .to_string() + }; let names: Vec = members .iter() - .map(|r| { - let name = r.id.trim_start_matches(&format!("{group}/")[..]); - if r.default_enabled || opt_in { - name.to_string() - } else { - format!("{name} (opt-in)") - } - }) + .filter(|r| r.default_enabled || opt_in) + .map(name) .collect(); let (level, suffix) = if opt_in { ("consider", " (opt-in)") } else { ("review", "") }; - format!("# {group} = \"{level}\" # {}{suffix}\n", names.join(", ")) + let mut lines = format!("# {group} = \"{level}\" # {}{suffix}\n", names.join(", ")); + for rule in members.iter().filter(|r| !r.default_enabled && !opt_in) { + lines.push_str(&format!( + "# \"{}\" = \"consider\" # {} (opt-in, only by name)\n", + rule.id, + name(rule) + )); + } + lines } #[cfg(test)] @@ -225,7 +236,11 @@ mod tests { Rules::List(_) => panic!("rules is a table of levels"), }; assert!(levels(config).is_empty(), "the default rules and gate"); - assert!(text.contains("hardcoded-values (opt-in)"), "{text}"); + assert!( + text.contains("# \"maintainability/hardcoded-values\" = \"consider\" # hardcoded-values (opt-in, only by name)\n"), + "{text}" + ); + assert!(!text.contains("shared-logic, hardcoded-values"), "{text}"); assert!( text.contains("shows them), never in a preview language;"), "{text}" @@ -233,14 +248,14 @@ mod tests { let uncommented: Vec<&str> = text .lines() .map(|line| { - let example = catalog::groups() - .into_iter() - .any(|g| line.starts_with(&format!("# {g} = "))); + let example = catalog::groups().into_iter().any(|g| { + line.starts_with(&format!("# {g} = ")) || line.starts_with(&format!("# \"{g}/")) + }); if example { &line[2..] } else { line } }) .collect(); let config: Config = toml::from_str(&uncommented.join("\n")).unwrap(); - assert_eq!(levels(config).len(), catalog::groups().len()); + assert_eq!(levels(config).len(), catalog::groups().len() + 1); let example: String = text .lines() .skip_while(|line| *line != "# [[question]]") diff --git a/src/main.rs b/src/main.rs index 8faf224..af10956 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,7 @@ macro_rules! say { ($($arg:tt)*) => {{ use std::io::Write as _; + let _paused = crate::progress::pause(); let _ = writeln!(std::io::stdout(), $($arg)*); }}; } @@ -11,6 +12,7 @@ macro_rules! say { macro_rules! note { ($($arg:tt)*) => {{ use std::io::Write as _; + let _paused = crate::progress::pause(); let _ = writeln!(std::io::stderr(), $($arg)*); }}; } @@ -55,6 +57,7 @@ mod options; mod output; mod packages; mod policy; +mod progress; mod provider; mod provider_error; mod requests; diff --git a/src/options/mod.rs b/src/options/mod.rs index bbfd10e..e2a1f1f 100644 --- a/src/options/mod.rs +++ b/src/options/mod.rs @@ -244,7 +244,10 @@ pub struct CheckArgs { /// /// Groups: maintainability, tests, security, documentation, custom (the /// custom questions; one is `custom/`), default (every rule on by - /// default) and all. Naming any rule replaces the configured + /// default) and all. A group selects the rules it runs by default, or + /// every rule of it when it runs none by default (security, + /// documentation), so hardcoded values runs only when named or with + /// `all`. Naming any rule replaces the configured /// selection, so add `--rule default` to keep the defaults. Test rules also /// need --include-tests. `jevgate rules` lists every rule. #[arg(long = "rule", value_name = "RULE", help_heading = RULES)] @@ -342,6 +345,10 @@ pub struct CheckArgs { /// ceiling this flag can only lower. #[arg(long, value_name = "N", value_parser = clap::value_parser!(u32).range(1..=1000000), help_heading = BUDGETS)] pub max_requests: Option, + /// Whether `max_requests` is the ceiling jevgate.toml sets, which a flag + /// cannot raise, rather than `--max-requests`. + #[arg(skip)] + pub max_requests_in_config: bool, /// Stop asking after this many seconds; what is left unasked leaves the run incomplete [default: 60 with --staged and --pre-push] /// /// No request starts, and no retry or pause runs, past it, and an attempt diff --git a/src/output/agent.rs b/src/output/agent.rs index 1b60304..7223624 100644 --- a/src/output/agent.rs +++ b/src/output/agent.rs @@ -77,16 +77,17 @@ fn emit_findings(out: &mut impl Write, report: &Report, verbose: bool, style: St let (custom, notes): (Vec<_>, Vec<_>) = of(Strength::Note) .into_iter() .partition(|(_, f)| crate::catalog::custom(&f.rule)); + let gate = Gate(report.gate.is_some()); if !review.is_empty() { let heading = format!("Review ({}):", review.len()); - emit_section(out, &heading, BOLD_RED, &review, style)?; + emit_section(out, (&heading, BOLD_RED), &review, gate, style)?; } if !consider.is_empty() { - emit_considers(out, &consider, verbose, style)?; + emit_considers(out, &consider, verbose, gate, style)?; } if !custom.is_empty() { let heading = format!("Notes from custom questions ({}):", custom.len()); - emit_section(out, &heading, BOLD, &custom, style)?; + emit_section(out, (&heading, BOLD), &custom, gate, style)?; } if notes.is_empty() { return Ok(()); @@ -100,15 +101,21 @@ fn emit_findings(out: &mut impl Write, report: &Report, verbose: bool, style: St return Ok(()); } let heading = format!("Notes ({}, optional):", notes.len()); - emit_section(out, &heading, BOLD, ¬es, style) + emit_section(out, (&heading, BOLD), ¬es, gate, style) } +/// Whether the run evaluated the gate: a finding of a run that did not, as +/// one left incomplete, would fail it rather than failing it. +#[derive(Clone, Copy)] +struct Gate(bool); + /// The top considers (all with `verbose`), under a heading that says how /// many there are and which are shown. fn emit_considers( out: &mut impl Write, consider: &[(&Path, &Finding)], verbose: bool, + gate: Gate, style: Style, ) -> Result<()> { let shown = if verbose { @@ -127,7 +134,13 @@ fn emit_considers( String::new() }; let heading = format!("Consider ({}{more}):", consider.len()); - emit_section(out, &heading, BOLD_YELLOW, &consider[..shown], style) + emit_section( + out, + (&heading, BOLD_YELLOW), + &consider[..shown], + gate, + style, + ) } /// What the change does to the checks around the code, one line each: the @@ -155,17 +168,17 @@ fn emit_guards(out: &mut impl Write, report: &Report, verbose: bool, style: Styl Ok(()) } -/// A blank line, a heading, then its findings. +/// A blank line, a heading in its color, then its findings. fn emit_section( out: &mut impl Write, - heading: &str, - code: &str, + (heading, code): (&str, &str), findings: &[(&Path, &Finding)], + gate: Gate, style: Style, ) -> Result<()> { writeln!(out, "\n{}", style.paint(code, heading))?; for (path, finding) in findings { - emit_finding(out, path, finding, style)?; + emit_finding(out, path, finding, gate, style)?; } Ok(()) } @@ -349,7 +362,13 @@ fn emit_context_load(out: &mut impl Write, load: &crate::docs::load::ContextLoad /// `path:line [rule] message` and how often such findings were right, then /// the next step; the location is bold, the rule and the share right dim, /// and a finding that fails the gate says so in red. -fn emit_finding(out: &mut impl Write, path: &Path, finding: &Finding, style: Style) -> Result<()> { +fn emit_finding( + out: &mut impl Write, + path: &Path, + finding: &Finding, + Gate(evaluated): Gate, + style: Style, +) -> Result<()> { let location = format!("{}:{}", path.display(), finding.line); let accepted = match (&finding.suppressed, finding.baselined) { (_, true) => " (baselined)".to_string(), @@ -358,7 +377,12 @@ fn emit_finding(out: &mut impl Write, path: &Path, finding: &Finding, style: Sty }; let rule = format!("[{}]{accepted}", finding.rule); let fails = if finding.fails_gate() { - format!("{} ", style.paint(RED, "(fails the gate)")) + let label = if evaluated { + "(fails the gate)" + } else { + "(would fail the gate)" + }; + format!("{} ", style.paint(RED, label)) } else { String::new() }; @@ -419,6 +443,7 @@ mod tests { &mut out, Path::new("src/a.rs"), &finding(Strength::Review), + Gate(true), style, ) .unwrap(); diff --git a/src/progress.rs b/src/progress.rs new file mode 100644 index 0000000..84f2bd3 --- /dev/null +++ b/src/progress.rs @@ -0,0 +1,197 @@ +//! A status line on stderr while a check runs, on a terminal only: what it +//! is doing, how many of the requests it has sent so far are answered, and +//! how long it has run. A whole-repository check of a 950-file project asks +//! Jev about 1,000 times, most of a minute at the 1,200 requests a minute +//! TypeSafe allows, and printed nothing until it ended. +//! +//! Every other line JevGate prints ([`note!`], [`say!`]) erases the status +//! line first, and the line is drawn again on the next tick. +use std::{ + io::{IsTerminal, Write}, + sync::{ + Arc, Mutex, MutexGuard, + atomic::{AtomicBool, Ordering}, + }, + thread::JoinHandle, + time::{Duration, Instant}, +}; + +/// How often the line is drawn again. +const TICK: Duration = Duration::from_millis(125); +/// A check that ends sooner never shows the line, so a cached rerun does not flicker. +const QUIET: Duration = Duration::from_millis(400); + +struct State { + started: Instant, + /// What the check is doing, such as `first pass`. + phase: &'static str, + /// Requests of this phase sent to the provider, and those answered. + sent: usize, + answered: usize, + /// Whether the line is on the terminal now. + shown: bool, +} + +static STATE: Mutex> = Mutex::new(None); + +/// Draws the line until dropped, then erases it. +pub struct Progress { + stop: Arc, + thread: Option>, +} + +/// Whether a check's status line would be read as it is drawn: stderr is a +/// terminal that understands erasing a line, and no CI log records it. +pub fn wanted() -> bool { + std::io::stderr().is_terminal() + && std::env::var_os("TERM").is_none_or(|term| term != "dumb") + && std::env::var_os("CI").is_none_or(|ci| ci.is_empty()) +} + +/// Start drawing the line when `enabled`. +pub fn start(enabled: bool) -> Option { + if !enabled { + return None; + } + *lock() = Some(State { + started: Instant::now(), + phase: "reading files", + sent: 0, + answered: 0, + shown: false, + }); + let stop = Arc::new(AtomicBool::new(false)); + let stopped = Arc::clone(&stop); + let thread = std::thread::spawn(move || { + while !stopped.load(Ordering::Acquire) { + std::thread::sleep(TICK); + draw(); + } + }); + Some(Progress { + stop, + thread: Some(thread), + }) +} + +impl Drop for Progress { + fn drop(&mut self) { + self.stop.store(true, Ordering::Release); + if let Some(thread) = self.thread.take() { + let _ = thread.join(); + } + let mut state = lock(); + erase(&mut state); + *state = None; + } +} + +/// The check now does `phase`; its request counts start again. +pub fn phase(phase: &'static str) { + if let Some(state) = lock().as_mut() { + state.phase = phase; + state.sent = 0; + state.answered = 0; + } +} + +/// `n` more requests of this phase go to the provider. +pub fn sending(n: usize) { + if let Some(state) = lock().as_mut() { + state.sent += n; + } +} + +/// One request of this phase came back. +pub fn answered() { + if let Some(state) = lock().as_mut() { + state.answered += 1; + } +} + +/// Erases the line while held, so another line can be printed; the next +/// tick draws it again. +pub struct Paused { + _held: MutexGuard<'static, Option>, +} + +/// Erase the line until the guard is dropped. +pub fn pause() -> Paused { + let mut state = lock(); + erase(&mut state); + Paused { _held: state } +} + +fn lock() -> MutexGuard<'static, Option> { + STATE + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +fn erase(state: &mut Option) { + if let Some(state) = state.as_mut().filter(|state| state.shown) { + let _ = write!(std::io::stderr(), "\r\x1b[2K"); + state.shown = false; + } +} + +fn draw() { + let mut state = lock(); + let Some(state) = state.as_mut() else { + return; + }; + let elapsed = state.started.elapsed(); + if elapsed < QUIET { + return; + } + let line = line(state.phase, state.answered, state.sent, elapsed); + let _ = write!(std::io::stderr(), "\r\x1b[2K{line}"); + let _ = std::io::stderr().flush(); + state.shown = true; +} + +/// `JevGate · first pass · 312/1,126 answered · 23s`, short enough not to +/// wrap on an 80-column terminal, where erasing it would leave a line. +fn line(phase: &str, answered: usize, sent: usize, elapsed: Duration) -> String { + let requests = if sent == 0 { + String::new() + } else { + format!(" · {answered}/{sent} answered") + }; + format!("JevGate · {phase}{requests} · {}", clock(elapsed.as_secs())) +} + +fn clock(seconds: u64) -> String { + match seconds { + 0..60 => format!("{seconds}s"), + _ => format!("{}m {:02}s", seconds / 60, seconds % 60), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_line_says_the_phase_the_requests_answered_and_the_time() { + assert_eq!( + line("first pass", 312, 1126, Duration::from_secs(23)), + "JevGate · first pass · 312/1126 answered · 23s" + ); + assert_eq!( + line("planning", 0, 0, Duration::from_secs(83)), + "JevGate · planning · 1m 23s" + ); + assert!( + line( + "rechecking undecided units", + 9999, + 9999, + Duration::from_secs(3599) + ) + .chars() + .count() + < 80 + ); + } +} diff --git a/src/requests.rs b/src/requests.rs index 167fdbb..4456cf6 100644 --- a/src/requests.rs +++ b/src/requests.rs @@ -97,17 +97,39 @@ impl Session<'_> { }) .collect(); let mut pending = self.answer_from_cache(requests, &mut receipts); + if !pending.is_empty() && self.halted.is_none() { + // Asked once: a store that refused, or a dialog declined, is not + // asked again for every batch. + self.halted = self.evaluator.unavailable(); + } + if let Some(error) = self.halted.as_ref().filter(|_| !pending.is_empty()) { + // Without a key nothing can be sent: each file fails with the + // reason, and `check` ends with it once. + for unsent in &pending { + receipts[unsent.index].result = Err(anyhow::anyhow!("{error:#}")); + } + return receipts; + } let allowed = pending.len().min( self.args .max_requests .map_or(pending.len(), |n| n.saturating_sub(self.requests) as usize), ); + // The agent hook answers on its own terms; a check says it on stderr. + if allowed < pending.len() + && !crate::hook::invoked() + && !std::mem::replace(&mut self.budget_noted, true) + { + note!( + "jevgate: {}", + budget_short(self.args, self.requests as usize + pending.len()) + ); + } for unsent in pending.drain(allowed..) { - receipts[unsent.index].result = Err(anyhow::anyhow!( - "Session API request budget exhausted; restart with an explicit larger --max-requests" - )); + receipts[unsent.index].result = Err(anyhow::anyhow!(budget_reached(self.args))); } if !pending.is_empty() { + crate::progress::sending(pending.len()); self.send(pending, &mut receipts); } receipts @@ -213,6 +235,7 @@ impl Session<'_> { self.args.concurrency() as usize, &before, &mut |at, outcome| { + crate::progress::answered(); let (index, lookup) = &mut lookups[at]; *requests_count += u32::from(outcome.attempted); let receipt = &mut receipts[*index]; @@ -233,6 +256,41 @@ impl Session<'_> { } } +/// The request budget and where it is set: `--max-requests` only lowers +/// the ceiling jevgate.toml sets, so raising the flag cannot help then. +fn budget_limit(args: &CheckArgs) -> String { + let limit = args.max_requests.unwrap_or_default(); + if args.max_requests_in_config { + format!("max_requests = {limit} in jevgate.toml") + } else { + format!("--max-requests {limit}") + } +} + +/// Why a request was not sent: the run reached its request budget. The +/// answers it got are cached, so a rerun asks only for the rest. +fn budget_reached(args: &CheckArgs) -> String { + format!( + "Request budget reached ({}); rerun to continue from the cached answers, or raise the budget", + budget_limit(args) + ) +} + +/// Said once, before the first request the budget holds back is dropped: +/// that the check will end incomplete, with at least `needed` requests +/// planned, and what finishes it. +fn budget_short(args: &CheckArgs, needed: usize) -> String { + let raise = if args.max_requests_in_config { + "raise max_requests in jevgate.toml" + } else { + "pass a larger --max-requests" + }; + format!( + "this check needs at least {needed} requests, more than {} allows, so it will end incomplete. The answers it gets are cached and a rerun continues from them; {raise} to finish in one run.", + budget_limit(args) + ) +} + /// Record one outcome of sending `sent`, the unanswered questions of `lookup`'s /// request: timing, token usage, and the answers [`receive`] takes from it. /// Returns what an answered request was billed, even when its answers failed diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 1d6ec13..7703fd7 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -213,6 +213,8 @@ pub(super) fn session<'a>( observed: (0, 0), answered: Default::default(), spend: options.max_cost.map(crate::requests::Spend::new), + halted: None, + budget_noted: false, } } @@ -427,6 +429,34 @@ fn malformed_response_and_exhausted_budget_never_pass() { ); } +#[test] +fn a_file_the_budget_left_unasked_names_the_budget_where_it_is_set() { + let project = two_files(); + for (in_config, named) in [ + (true, "(max_requests = 1 in jevgate.toml)"), + (false, "(--max-requests 1)"), + ] { + let mut options = args(); + options.max_requests = Some(1); + options.max_requests_in_config = in_config; + options.refresh = true; + let report = run(&project, &options, &mut Mock::default()); + let unasked: Vec<&str> = report + .files + .iter() + .filter_map(|f| f.error.as_deref()) + .collect(); + assert_eq!(unasked.len(), 1, "{unasked:?}"); + assert!( + unasked[0].starts_with(&format!( + "Request budget reached {named}; rerun to continue" + )), + "{}", + unasked[0] + ); + } +} + #[test] fn edit_during_request_is_reported_stale() { let project = Project::new(); diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 1ba6cee..10b3d0a 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -50,6 +50,13 @@ pub trait Evaluator { fn evaluate(&mut self, request: &Value) -> Result; + /// Why no request can be sent at all, such as a missing key; asked + /// before a batch that needs the provider, so a check stops with it + /// once instead of failing every file with it. + fn unavailable(&mut self) -> Option { + None + } + fn evaluate_batch(&mut self, requests: &[&Value]) -> Vec> { requests .iter() @@ -217,6 +224,10 @@ impl Client { } impl Evaluator for Client { + fn unavailable(&mut self) -> Option { + self.credential().err() + } + fn begin_review(&mut self) { if self.access.reset() { // A rejected credential may have been replaced between snapshots. diff --git a/tests/cli/auth.rs b/tests/cli/auth.rs index 4b6c728..6fd28c6 100644 --- a/tests/cli/auth.rs +++ b/tests/cli/auth.rs @@ -11,6 +11,18 @@ fn absent_credentials_produce_operational_failure_with_atomic_report() { .output() .unwrap(); assert_eq!(output.status.code(), Some(2)); + // One line says why, not one per file, and nothing else is printed. + assert!(output.stdout.is_empty()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + stderr.matches("No API key configured").count(), + 1, + "{stderr}" + ); + assert!( + stderr.starts_with("jevgate: No API key configured."), + "{stderr}" + ); let report = project.snapshot().unwrap(); assert_eq!(report["complete"], false); assert_eq!(report["files"][0]["status"], "error"); From 8014b9d47880f56e0eddf96c613a2703cce95913 Mon Sep 17 00:00:00 2001 From: Tauan BF <11513929+tauanbinato@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:38:43 -0300 Subject: [PATCH 2/2] Name the key and budget steps of a batch, and share rule-name expansion --- src/config.rs | 18 ++++++++++-------- src/requests.rs | 48 ++++++++++++++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/config.rs b/src/config.rs index 601b0b4..8cf3de6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -474,21 +474,23 @@ impl Levels { /// Keys of `rules` named by rule IDs, names, keys or groups; an unknown /// name is an error. fn expand(rules: &[catalog::Rule], names: &[String]) -> Result> { - let mut keys = Vec::new(); - for name in names { - let selected = catalog::select_in(rules, name).ok_or_else(|| unknown(rules, name))?; - keys.extend(selected); - } - Ok(keys) + expand_with(rules, names, catalog::select_in) } /// The keys of the rules that naming `names` turns on: a group's opt-in /// rules only when the group runs none by default ([`catalog::enable_in`]). fn expand_enabled(rules: &[catalog::Rule], names: &[String]) -> Result> { + expand_with(rules, names, catalog::enable_in) +} + +fn expand_with( + rules: &[catalog::Rule], + names: &[String], + select: fn(&[catalog::Rule], &str) -> Option>, +) -> Result> { let mut keys = Vec::new(); for name in names { - let selected = catalog::enable_in(rules, name).ok_or_else(|| unknown(rules, name))?; - keys.extend(selected); + keys.extend(select(rules, name).ok_or_else(|| unknown(rules, name))?); } Ok(keys) } diff --git a/src/requests.rs b/src/requests.rs index 4456cf6..baa0f5a 100644 --- a/src/requests.rs +++ b/src/requests.rs @@ -97,25 +97,46 @@ impl Session<'_> { }) .collect(); let mut pending = self.answer_from_cache(requests, &mut receipts); - if !pending.is_empty() && self.halted.is_none() { - // Asked once: a store that refused, or a dialog declined, is not - // asked again for every batch. + if self.cannot_send(&pending, &mut receipts) { + return receipts; + } + self.hold_to_budget(&mut pending, &mut receipts); + if !pending.is_empty() { + crate::progress::sending(pending.len()); + self.send(pending, &mut receipts); + } + receipts + } + + /// Whether nothing can be sent, as without a key: each of `pending` + /// then fails with the reason, and `check` ends with it once. The + /// evaluator is asked once, so a store that refused, or a dialog + /// declined, is not asked again for every batch. + fn cannot_send(&mut self, pending: &[Pending<'_>], receipts: &mut [Receipt]) -> bool { + if pending.is_empty() { + return false; + } + if self.halted.is_none() { self.halted = self.evaluator.unavailable(); } - if let Some(error) = self.halted.as_ref().filter(|_| !pending.is_empty()) { - // Without a key nothing can be sent: each file fails with the - // reason, and `check` ends with it once. - for unsent in &pending { - receipts[unsent.index].result = Err(anyhow::anyhow!("{error:#}")); - } - return receipts; + let Some(error) = &self.halted else { + return false; + }; + for unsent in pending { + receipts[unsent.index].result = Err(anyhow::anyhow!("{error:#}")); } + true + } + + /// Keep what the request budget allows of `pending`, failing the rest, + /// and say once on stderr that the check will end incomplete. The agent + /// hook answers on its own terms. + fn hold_to_budget(&mut self, pending: &mut Vec>, receipts: &mut [Receipt]) { let allowed = pending.len().min( self.args .max_requests .map_or(pending.len(), |n| n.saturating_sub(self.requests) as usize), ); - // The agent hook answers on its own terms; a check says it on stderr. if allowed < pending.len() && !crate::hook::invoked() && !std::mem::replace(&mut self.budget_noted, true) @@ -128,11 +149,6 @@ impl Session<'_> { for unsent in pending.drain(allowed..) { receipts[unsent.index].result = Err(anyhow::anyhow!(budget_reached(self.args))); } - if !pending.is_empty() { - crate::progress::sending(pending.len()); - self.send(pending, &mut receipts); - } - receipts } /// Fill receipts from cached answers; return the requests still to send,