diff --git a/crates/wright-analyzer/src/declarative.rs b/crates/wright-analyzer/src/declarative.rs index 2eafebef..4d6afe5d 100644 --- a/crates/wright-analyzer/src/declarative.rs +++ b/crates/wright-analyzer/src/declarative.rs @@ -78,6 +78,18 @@ impl DeclarativeRule { return Err(RuleError::FirstPartyMetadataField { field: "severity" }); } } + // Error-severity audit (#1010): an error-severity finding yields + // `ok: false` and `exit: 1`, so `error` is reserved for findings + // confirmed by runtime evaluation. A first-party rule that declares + // `severity: error` without `evidence: runtime-validated` is rejected + // rather than silently downgraded — the load error is the structured + // `lint-rule-error` diagnostic, same as any invalid definition. + if identity == RuleIdentity::FirstParty + && metadata.severity == Some(Severity::Error) + && metadata.evidence != Some(EvidenceClass::RuntimeValidated) + { + return Err(RuleError::UnjustifiedErrorSeverity); + } let locale = Locale::new(&locale_name); if !catalog.supports(&locale) { return Err(RuleError::UnsupportedLocale(locale_name)); diff --git a/crates/wright-analyzer/src/declarative/error.rs b/crates/wright-analyzer/src/declarative/error.rs index 7f773677..97b0ba0d 100644 --- a/crates/wright-analyzer/src/declarative/error.rs +++ b/crates/wright-analyzer/src/declarative/error.rs @@ -34,6 +34,12 @@ pub enum RuleError { FirstPartyMetadataField { field: &'static str, }, + /// A first-party rule may declare `severity: error` only when its + /// evidence class is `runtime-validated` — the audit contract (#1010, + /// `docs/agent-contract.md`) reserves `error` for findings confirmed by + /// runtime evaluation, because any error-severity finding yields + /// `ok: false` and `exit: 1`. + UnjustifiedErrorSeverity, } impl fmt::Display for RuleError { @@ -94,6 +100,12 @@ impl fmt::Display for RuleError { "metadata field '{field}' is only allowed in first-party rule packages" ) } + RuleError::UnjustifiedErrorSeverity => { + write!( + f, + "`severity: error` requires `evidence: runtime-validated`" + ) + } } } } diff --git a/crates/wright-analyzer/tests/error_severity_audit.rs b/crates/wright-analyzer/tests/error_severity_audit.rs new file mode 100644 index 00000000..7f6515c8 --- /dev/null +++ b/crates/wright-analyzer/tests/error_severity_audit.rs @@ -0,0 +1,45 @@ +//! Error-severity audit enforcement (#1010): any `error`-severity lint +//! finding yields `ok: false`/`exit: 1`, and lint correctness is not +//! guaranteed, so `error` is reserved for findings confirmed by runtime +//! evaluation — only a `runtime-validated` rule may emit it. + +use wright_analyzer::registry::LintRegistry; + +fn first_party_rule(evidence: Option<&str>) -> String { + let evidence = evidence + .map(|evidence| format!(" evidence: {evidence}\n")) + .unwrap_or_default(); + format!( + "id: wright/audited\nmetadata:\n summary: s\n rationale: r\n documentation: d\n known-limits: k\n tags: []\n{evidence} severity: error\nmatcher: {{}}\n" + ) +} + +#[test] +fn first_party_error_severity_requires_runtime_validated_evidence() { + let mut registry = LintRegistry::default(); + for (evidence, accepted) in [ + (Some("runtime-validated"), true), + (Some("exact"), false), + (Some("static-indicator"), false), + (Some("heuristic"), false), + (None, false), + ] { + let result = registry.load_first_party_yaml_str(&first_party_rule(evidence)); + assert_eq!( + result.is_ok(), + accepted, + "{evidence:?} with severity error: {result:?}" + ); + if result.is_ok() { + // Re-load under a fresh registry for the next rejected case. + registry = LintRegistry::default(); + } + } + // Non-error severities remain unrestricted. + let mut registry = LintRegistry::default(); + registry + .load_first_party_yaml_str( + "id: wright/audited-warn\nmetadata:\n summary: s\n rationale: r\n documentation: d\n known-limits: k\n tags: []\n evidence: heuristic\n severity: warning\nmatcher: {}\n", + ) + .unwrap(); +} diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 8cfc3381..5fa852ba 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -54,7 +54,7 @@ error (exit 2), never a silently remapped default: A configured session example: `wright serve --lint-config policy.yaml --disable-rule repeated-value project.ws` serves `lint`/`lintRules` results under `policy.yaml` plus the disabled rule, reporting the same -effective policy the equivalent `wright lint` invocation prints. +effective policy `wright lint` prints. Stdio returns one `ToolResponse` per request. A successful response has a `result` member; an application-level refusal has an `error` member containing @@ -248,16 +248,15 @@ in the next `symbols`, `references`, `check`, `lint`, or other program-reading request without restarting the session. An unchanged input is never reloaded. -A load or reload that fails, whether from a missing or unreadable entry or -a source that does not parse, refuses the request with the loader's structured +A load or reload that fails — a missing or unreadable entry, or a source +that does not parse — refuses the request with the loader's structured diagnostic code (`input-io`, `parse-error`, `input-kind-ambiguous`, a provider diagnostic). The service keeps refusing until the project loads again, retrying on every program-reading request; the previously loaded -program is never served silently, and the service never substitutes an -empty or placeholder program. The failed attempt invalidates the served -snapshot: even restoring the input to byte-identical content loads a fresh -program rather than resurrecting the dropped one. Repairing the input -recovers the same running session, with no restart and no explicit reload +program is never served silently and is never replaced by an empty or +placeholder program. The failed attempt invalidates the served snapshot: +even restoring byte-identical input loads a fresh program rather than +resurrecting the dropped one; repair needs no restart or explicit reload request. `check` and `compile` are workflows rather than snapshot queries (#591): @@ -295,15 +294,15 @@ generation they belong to in a `snapshot` member: `symbols`, `rules`, all carry it (on the `wright-result/v1` envelope for `inspect`/`analyze`, in the result object elsewhere). A request that addresses a symbol or rule numerically must echo that value: `references` and `usage` require -`snapshot` when `symbol` is an integer, `cfg` and `rules` when `rule` is an -integer, and `references` requires it when its `rule` filter is an integer -too — every numeric address a request carries is checked (#592). A numeric -address without `snapshot`, or with a generation other than the served -one, is refused `stale-id`; the client re-issues the id from a current -response's `snapshot` and id members, or addresses by declared name. -Listing `symbols` or `rules` has no side effect on id validity — a -`symbols {max: 0}` response still reports `snapshot`, and it is the value, -not the act of listing, that scopes the ids it reports. +`snapshot` when `symbol` is an integer, `cfg` and `rules` when `rule` is +an integer, and `references` when its `rule` filter is an integer too — +every numeric address is checked (#592). A numeric address without +`snapshot`, or with another generation, is refused `stale-id`; the +client re-issues the id from a current response's `snapshot` and id +members, or addresses by declared name. Listing `symbols` or `rules` +has no side effect on id validity — a `symbols {max: 0}` response still +reports `snapshot`, and it is the value, not the act of listing, that +scopes the ids it reports. `snapshot` is ignored when every address in the request is a declared name: name addressing resolves against the current program in both states @@ -322,13 +321,11 @@ The responses that issue numeric ids or indexes into the loaded program are `references`, `findings`, `lint`, `persistentObjects`, `inspect`, `analyze`, `project`, `costEstimate`, and the `ambiguous-*` refusal candidates, plus `skipped[].rule` in `lint` and `lintRules`. `cfg` block ids are local to -that one response and are not -program addresses; `callGraph`, `targetMetadata`, +that response, not program addresses; `callGraph`, `targetMetadata`, `capabilities`, the `compile`/`check` envelopes, and the edit/provider -payloads issue no program ids. Positional fields such as `rule`, `action`, -`value`, and `span.file` inside findings and references are coordinates into -the current program rather than reusable addresses, but they are always -computed from the program that was live at request time. +payloads issue none. Positional fields such as `rule`, `action`, `value`, +and `span.file` inside findings and references are coordinates into the +current program, always computed from the program live at request time. ### Name and address grammar (#429, ADR-0035) @@ -383,6 +380,13 @@ and `costEstimate` add a `selection` member to their existing result objects. capped result still reports every code's real count (#791). Requests without selection fields receive the previous shapes unchanged. +### Error-severity lint policy (#1010) + +Any `error`-severity finding yields `ok: false` and `exit: 1`. Lint +correctness is not guaranteed, so `error` is reserved for confirmed +defects — only `runtime-validated` rules may emit it. The audit (none) +and enforcement are in [cli/lint.md](cli/lint.md). + ### Rule content (ADR-0024) `rules` with `"content": true` adds the owner's @@ -433,10 +437,13 @@ that matches nothing in the loaded program, is a structured `ambiguous-rule`, never merged), `callee` (a declared subroutine name), `max`. -The `file` field resolves like finding selection: any spelling that resolves -to the same source file selects it. A capped list reports a `truncation` member with its total, shown and withheld counts, and an `expand` hint. Unselected arrays become `{items, truncation}` only when capped. Explicit selection wrappers retain their own totals. The CLI options -`--only`, `--rule`, `--file`, `--caller`, `--callee`, and `--max` on `inspect` -subcommands drive the same `wright-driver` selection. +The `file` field resolves like finding selection: any spelling that +resolves to the same source file selects it. A capped list reports a +`truncation` member with its total, shown and withheld counts, and an +`expand` hint. Unselected arrays become `{items, truncation}` only when +capped. Explicit selection wrappers retain their own totals. The CLI +options `--only`, `--rule`, `--file`, `--caller`, `--callee`, and `--max` +on `inspect` subcommands drive the same `wright-driver` selection. ### Essential default results @@ -465,12 +472,17 @@ owning rule's location without a second lookup. Full fact dumps are available only through the in-process `CompilerSession` API; external callers use the narrower semantic operations. -General lists default to at most 20 items; lint also bounds its compact result to 5500 bytes before the envelope so long finding messages cannot consume the default token budget; the ranked analyze report keeps its top-five hotspots, risks, and variables and top-three condition trees. Capped lists report their total, truncation, and how to -see the rest. `all: true` lifts the default cap (CLI `--all`); filters and an -explicit `max` continue to apply. Verdicts and exit codes are computed on the -complete set before truncation. Static catalogs are not capped. `brief` and -`--brief` are removed. [ADR-0025](adr/0025-essential-default-output.md) records -this breaking pre-1.0 shape change without a contract-id bump. +General lists default to at most 20 items; lint also bounds its compact +result to 5500 bytes before the envelope so long finding messages cannot +consume the default token budget; the ranked analyze report keeps its +top-five hotspots, risks, and variables and top-three condition trees. +Capped lists report their total, truncation, and how to see the rest. +`all: true` lifts the default cap (CLI `--all`); filters and an explicit +`max` continue to apply. Verdicts and exit codes are computed on the +complete set before truncation. Static catalogs are not capped. `brief` +and `--brief` are removed. +[ADR-0025](adr/0025-essential-default-output.md) records this breaking +pre-1.0 shape change without a contract-id bump. Edit transactions use source identities and half-open, 1-based line/column ranges. Provider positions use 0-based line/character coordinates. Wright @@ -589,8 +601,8 @@ diagnostics or mutation refusals, not guessed source locations or textual fallbacks. `capabilities` is the version negotiation step. Clients should check -`agent_contract` and the advertised operation list before sending requests. -Clients should ignore unknown response fields and must not assume an operation +`agent_contract` and the advertised operation list before sending +requests, ignore unknown response fields, and not assume an operation exists unless it is advertised. ## Versioning @@ -605,7 +617,7 @@ evolution policy is defined in [`docs/cli/machine-contract.md`](cli/machine-cont [ADR-0025](adr/0025-essential-default-output.md) records the essential-default shape change across both contracts, including the removal of `brief` and full -external fact dumps. The complete typed Rust API remains available. +external fact dumps. One recorded exception applies to the same rule in both contracts, decided before the 1.0 contract freeze (#134): the `lint` result's `rules` member @@ -620,6 +632,6 @@ provider operation (#548): `documents` and `sources` are optional on `providerValidateEdit`, defaulting to the session's loaded project and each document's text. -The optional guide installed by `wright agent install` (and distributed by -`wrightkit/skills`) teaches clients to discover and use these capabilities. It -is not required to expose, execute, or validate any semantic operation. +The optional guide installed by `wright agent install` (distributed by +`wrightkit/skills`) teaches clients these capabilities; it is not required +to expose, execute, or validate any semantic operation. diff --git a/docs/cli/lint.md b/docs/cli/lint.md index 90545e09..2276e79b 100644 --- a/docs/cli/lint.md +++ b/docs/cli/lint.md @@ -204,6 +204,52 @@ finding: Non-`while-without-wait` findings carry `"boundedness": null`. +## Error severity (#1010) + +Any `error`-severity finding yields `ok: false` and `exit: 1` on every +verdict surface. Lint correctness is not guaranteed — rules can produce +false positives and the analyzer itself can have implementation errors — +so `error` is reserved for findings whose defect is confirmed, not +indicated: only a rule whose evidence class is `runtime-validated` may +emit `error`. + +### Audit: rules that can emit `error` severity + +| Rule | Kind | Evidence class | Verdict | +| --- | --- | --- | --- | +| *(none)* | — | — | — | + +All seven built-in native rules ship `warning` or `info` defaults: +`min-wait-loop` (`static-indicator`), `duplicate-condition` (`exact`), +`repeated-value` (`exact`), `while-without-wait` (`static-indicator`), +`nested-binder-lambda` (`static-indicator`) → `warning`; +`expensive-loop-check` (`heuristic`), `ongoing-condition-hot-path` +(`heuristic`) → `info`. None is `runtime-validated`, so none justifies +`error` and none needed a downgrade. Real-project runs over +`tests/fixtures/workshop/real-world/*.ws` (overpy-cake, overpy-pixelart, +overpy-client-to-server, reproducer-bastion-prophet) and the metrics +corpus emit only `warning`/`info` findings. + +The boundaries that keep this true are enforced, not advisory: + +* External (`--rule-file`, community package) declarative rules cannot + declare `severity` at all — the field is rejected at load time as a + `lint-rule-error`, so they can never emit `error` by default. +* A first-party (`wrightkit/lint-rules`) declarative rule may declare + `severity: error` only together with + `evidence: runtime-validated`; any other combination is a load-time + `lint-rule-error`, never a silent downgrade. The first-party package + currently ships no rules. +* `severity: error` in a project lint config or + `--rule-severity :error` is explicit project policy, not a rule + default, and the verdict contract intentionally honors it. + +The `workshop.unknown-action`, `workshop.unknown-value`, and +`workshop.opaque-action` completeness residuals are `error`-severity +envelope *diagnostics* owned by `workshop-rs` catalog resolution — exact +facts that appear in `diagnostics`, never in lint `findings`, and outside +this audit's rule scope. + ## Validated automated fixes (#556) An `exact`-evidence finding whose correction is mechanically unambiguous diff --git a/docs/specs/SPEC-309-extensible-lint-rules.md b/docs/specs/SPEC-309-extensible-lint-rules.md index 6b28bd7b..c803f83e 100644 --- a/docs/specs/SPEC-309-extensible-lint-rules.md +++ b/docs/specs/SPEC-309-extensible-lint-rules.md @@ -133,7 +133,11 @@ covers instead of colliding. The first-party package may also declare `evidence` and `severity` in rule metadata (#616 addendum, #970): the former sets the finding's evidence class, the latter the rule's default severity. Both fields are rejected with a `lint-rule-error` diagnostic in -community packages and `--rule-file` rules. +community packages and `--rule-file` rules. `severity: error` is further +restricted to `evidence: runtime-validated` rules — the error-severity +audit policy (#1010, `docs/agent-contract.md`) reserves `error` for +findings confirmed by runtime evaluation, since any error-severity +finding yields `ok: false` and `exit: 1`. `enabled` defaults to `true`; `severity` may be `off`, `warn`, or `error`; the only shared rule options are the bounded `min-matches` and `max-matches`