feat(review): retire bundled linter execution #54

Merged
jwilger merged 3 commits from issue-45-remove-bundled-linters into main 2026-05-03 13:34:06 -07:00
Owner

Summary

  • Remove bundled linter execution, linter-only mode, linter prompt context, and linter review summaries from the normal review pipeline.
  • Retire linter routing CLI/docs/deployment surfaces and remove the linter sandbox image from runtime packaging.
  • Update operator docs, ADRs, threat model, examples, and changelog to make CI responsible for deterministic linters/tests/builds.
  • Approve warning-only or note-only auto_review results while preserving inline advisory comments, so stale request-changes reviews do not keep branch protection blocked.

Verification

  • nix develop -c cargo fmt --all -- --check
  • nix develop -c cargo check -p ar-gateway -p ar-sandbox
  • env -u FORGEJO_BASE_URL -u FORGEJO_TOKEN -u LLM_BASE_URL -u LLM_REASONING_MODEL -u LLM_API_KEY nix develop -c cargo nextest run -p ar-review -p ar-orchestrator -p ar-prompts -p ar-cli -p ar-gateway --no-tests=pass (459 passed)
  • nix develop --command cargo test -p ar-review mapping::tests::warning_only_or_note_only_output_is_approved_but_keeps_inline_comments -- --exact
  • nix develop --command cargo test -p ar-review --test red_team_pipeline t9_review_event_is_derived_from_severity_not_llm_input -- --exact
  • nix develop --command cargo nextest run -p ar-review --no-tests=pass (190 passed)
  • nix develop --command cargo fmt --all -- --check
  • nix develop --command cargo clippy -p ar-review --all-targets -- -D warnings

Closes #45.
Closes #55.

## Summary - Remove bundled linter execution, linter-only mode, linter prompt context, and linter review summaries from the normal review pipeline. - Retire linter routing CLI/docs/deployment surfaces and remove the linter sandbox image from runtime packaging. - Update operator docs, ADRs, threat model, examples, and changelog to make CI responsible for deterministic linters/tests/builds. - Approve warning-only or note-only auto_review results while preserving inline advisory comments, so stale request-changes reviews do not keep branch protection blocked. ## Verification - `nix develop -c cargo fmt --all -- --check` - `nix develop -c cargo check -p ar-gateway -p ar-sandbox` - `env -u FORGEJO_BASE_URL -u FORGEJO_TOKEN -u LLM_BASE_URL -u LLM_REASONING_MODEL -u LLM_API_KEY nix develop -c cargo nextest run -p ar-review -p ar-orchestrator -p ar-prompts -p ar-cli -p ar-gateway --no-tests=pass` (459 passed) - `nix develop --command cargo test -p ar-review mapping::tests::warning_only_or_note_only_output_is_approved_but_keeps_inline_comments -- --exact` - `nix develop --command cargo test -p ar-review --test red_team_pipeline t9_review_event_is_derived_from_severity_not_llm_input -- --exact` - `nix develop --command cargo nextest run -p ar-review --no-tests=pass` (190 passed) - `nix develop --command cargo fmt --all -- --check` - `nix develop --command cargo clippy -p ar-review --all-targets -- -D warnings` Closes #45. Closes #55.
feat(review): retire bundled linter execution
All checks were successful
auto_review auto_review: 10 warnings
CI / Nix flake check (pull_request) Successful in 3m36s
256a39626a
auto-review requested changes 2026-05-03 10:34:04 -07:00
Dismissed
auto-review left a comment

This PR removes the bundled linter execution and related configurations from the auto_review project, shifting the responsibility for deterministic checks to CI. The changes include removing linter-related fields from configuration files, updating documentation, and cleaning up code dependencies. The PR appears safe to merge, but ensure that any external dependencies or scripts relying on the removed functionality are updated accordingly.

Walkthrough

  • Configuration Changes

    • Removed disabled_tools and mode fields from .auto_review.example.yaml.
    • Updated AGENTS.md and README.md to reflect the removal of linter execution from the pipeline.
  • Code Changes

    • Removed ar-sandbox and ar-tools dependencies from Cargo.toml files across multiple crates.
    • Updated dispatcher.rs to remove sandbox-related code and adjust the review job pipeline.
    • Removed CLI commands related to linter routing and listing.
  • Documentation Updates

    • Updated CHANGELOG.md to document the removal of linter execution and the shift to CI for deterministic checks.
    • Updated SECURITY.md to reflect the new responsibility of CI for running deterministic checks.
  • Testing and Validation

    • Ensure that any tests or scripts relying on the removed linter functionality are updated or removed.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — source changed but no test file appears in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution and related configurations from the `auto_review` project, shifting the responsibility for deterministic checks to CI. The changes include removing linter-related fields from configuration files, updating documentation, and cleaning up code dependencies. The PR appears safe to merge, but ensure that any external dependencies or scripts relying on the removed functionality are updated accordingly. ## Walkthrough - **Configuration Changes** - Removed `disabled_tools` and `mode` fields from `.auto_review.example.yaml`. - Updated `AGENTS.md` and `README.md` to reflect the removal of linter execution from the pipeline. - **Code Changes** - Removed `ar-sandbox` and `ar-tools` dependencies from `Cargo.toml` files across multiple crates. - Updated `dispatcher.rs` to remove sandbox-related code and adjust the review job pipeline. - Removed CLI commands related to linter routing and listing. - **Documentation Updates** - Updated `CHANGELOG.md` to document the removal of linter execution and the shift to CI for deterministic checks. - Updated `SECURITY.md` to reflect the new responsibility of CI for running deterministic checks. - **Testing and Validation** - Ensure that any tests or scripts relying on the removed linter functionality are updated or removed. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [ ] Tests touched — source changed but no test file appears in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.
Author
Owner

Addressed: retired config keys now remain rejected by strict validation, and I added to make that behavior visible in a test file. Classification: one-off plus test evidence for the review check.

Addressed: retired config keys now remain rejected by strict validation, and I added to make that behavior visible in a test file. Classification: one-off plus test evidence for the review check.
Author
Owner

Correction: the added test file is crates/ar-review/tests/retired_linter_runtime.rs, covering strict rejection of retired mode and disabled_tools keys.

Correction: the added test file is crates/ar-review/tests/retired_linter_runtime.rs, covering strict rejection of retired mode and disabled_tools keys.
jwilger marked this conversation as resolved
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the Cargo.toml. Ensure that any functionality relying on these crates is updated or removed.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the `Cargo.toml`. Ensure that any functionality relying on these crates is updated or removed.
Author
Owner

Addressed: ar-cli no longer needs ar-sandbox/ar-tools after removing linter routing/listing and sandbox construction from review-once. Cargo check and the focused test pass. Classification: one-off confirmation.

Addressed: ar-cli no longer needs ar-sandbox/ar-tools after removing linter routing/listing and sandbox construction from review-once. Cargo check and the focused test pass. Classification: one-off confirmation.
jwilger marked this conversation as resolved
Owner

🟡 Warning: The linter_findings field has been removed from the Fixture struct. Ensure that any code relying on this field is updated accordingly.

🟡 **Warning:** The `linter_findings` field has been removed from the `Fixture` struct. Ensure that any code relying on this field is updated accordingly.
Author
Owner

Addressed: benchmark fixtures no longer carry linter findings because issue #45 removes linter context from semantic review inputs. The bench README was updated as well. Classification: one-off confirmation.

Addressed: benchmark fixtures no longer carry linter findings because issue #45 removes linter context from semantic review inputs. The bench README was updated as well. Classification: one-off confirmation.
jwilger marked this conversation as resolved
@ -341,35 +327,6 @@ pub struct TestWebhookArgs {
pub timeout_secs: u64,
Owner

🟡 Warning: The ExplainRouting and ListLinters commands have been removed. Ensure that any scripts or documentation referencing these commands are updated.

🟡 **Warning:** The `ExplainRouting` and `ListLinters` commands have been removed. Ensure that any scripts or documentation referencing these commands are updated.
Author
Owner

Addressed: the CLI commands were intentionally removed with the retired bundled-linter runtime, and active docs were updated to stop referencing them. Classification: one-off confirmation.

Addressed: the CLI commands were intentionally removed with the retired bundled-linter runtime, and active docs were updated to stop referencing them. Classification: one-off confirmation.
jwilger marked this conversation as resolved
@ -108,9 +107,6 @@ pub async fn review_once(args: ReviewOnceArgs) -> Result<()> {
// shot debug command, so the no-incremental fall-through is what
Owner

🟡 Warning: The DirectSandbox and PodmanSandbox references have been removed. Ensure that any deployment scripts or configurations relying on these are updated.

🟡 **Warning:** The `DirectSandbox` and `PodmanSandbox` references have been removed. Ensure that any deployment scripts or configurations relying on these are updated.
Author
Owner

Addressed: review-once no longer constructs DirectSandbox/PodmanSandbox because normal review runtime no longer executes bundled linters. Deployment docs/config were updated accordingly. Classification: one-off confirmation.

Addressed: review-once no longer constructs DirectSandbox/PodmanSandbox because normal review runtime no longer executes bundled linters. Deployment docs/config were updated accordingly. Classification: one-off confirmation.
jwilger marked this conversation as resolved
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the Cargo.toml. Ensure that any functionality relying on these crates is updated or removed.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the `Cargo.toml`. Ensure that any functionality relying on these crates is updated or removed.
Author
Owner

Addressed: ar-gateway no longer wires sandbox selection or linter runtime dependencies; now reports . Classification: one-off confirmation.

Addressed: ar-gateway no longer wires sandbox selection or linter runtime dependencies; now reports . Classification: one-off confirmation.
Author
Owner

Correction: ar-gateway no longer wires sandbox selection or linter runtime dependencies; the info endpoint now reports sandbox as not-used.

Correction: ar-gateway no longer wires sandbox selection or linter runtime dependencies; the info endpoint now reports sandbox as not-used.
jwilger marked this conversation as resolved
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the Cargo.toml. Ensure that any functionality relying on these crates is updated or removed.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the `Cargo.toml`. Ensure that any functionality relying on these crates is updated or removed.
Author
Owner

Addressed: ar-orchestrator no longer dispatches linter execution, so ar-sandbox/ar-tools dependencies were intentionally removed and the workspace-prep/review flow was updated. Classification: one-off confirmation.

Addressed: ar-orchestrator no longer dispatches linter execution, so ar-sandbox/ar-tools dependencies were intentionally removed and the workspace-prep/review flow was updated. Classification: one-off confirmation.
jwilger marked this conversation as resolved
@ -138,7 +135,6 @@ pub struct SpawningDispatcher {
/// re-embed unchanged symbols. None ⇒ build_review_context_with_store
Owner

🟡 Warning: The sandbox field has been removed from SpawningDispatcher. Ensure that any code relying on sandbox functionality is updated.

🟡 **Warning:** The `sandbox` field has been removed from `SpawningDispatcher`. Ensure that any code relying on sandbox functionality is updated.
Author
Owner

Addressed: this removal is intentional for issue #45. The runtime no longer owns a sandbox handle, dependent orchestrator/gateway code was updated, and verification passed. Classification: one-off confirmation, no new guardrail needed.

Addressed: this removal is intentional for issue #45. The runtime no longer owns a sandbox handle, dependent orchestrator/gateway code was updated, and verification passed. Classification: one-off confirmation, no new guardrail needed.
jwilger marked this conversation as resolved
Owner

🟡 Warning: The ar-tools dependency has been removed from the Cargo.toml. Ensure that any functionality relying on this crate is updated or removed.

🟡 **Warning:** The `ar-tools` dependency has been removed from the `Cargo.toml`. Ensure that any functionality relying on this crate is updated or removed.
Author
Owner

Addressed: the dependency removal is intentional because semantic prompts no longer accept linter findings. The workspace checks pass without ar-tools here. Classification: one-off confirmation, no new guardrail needed.

Addressed: the dependency removal is intentional because semantic prompts no longer accept linter findings. The workspace checks pass without ar-tools here. Classification: one-off confirmation, no new guardrail needed.
jwilger marked this conversation as resolved
Owner

🟡 Warning: The Finding struct from ar_tools has been removed from the ReviewPromptInputs. Ensure that any code relying on linter findings is updated.

🟡 **Warning:** The `Finding` struct from `ar_tools` has been removed from the `ReviewPromptInputs`. Ensure that any code relying on linter findings is updated.
Author
Owner

Addressed: ReviewPromptInputs intentionally no longer has an ar_tools Finding field. The prompt regression verifies linter context is omitted from semantic review. Classification: one-off confirmation.

Addressed: ReviewPromptInputs intentionally no longer has an ar_tools Finding field. The prompt regression verifies linter context is omitted from semantic review. Classification: one-off confirmation.
jwilger marked this conversation as resolved
test(review): cover retired linter config keys
All checks were successful
CI / Nix flake check (pull_request) Successful in 1m58s
auto_review auto_review: 9 warnings
f73ba3a68f
auto-review left a comment

This PR removes the bundled linter execution and related configurations from the auto_review project, shifting the responsibility for deterministic checks to CI. The changes include removing linter-related code, configurations, and documentation, as well as updating the review pipeline to focus on semantic analysis. Ensure that any existing configurations, scripts, or documentation relying on the removed features are updated accordingly.

Walkthrough

  • Configuration Changes

    • .auto_review.example.yaml: Removed disabled_tools and mode fields. Update configurations to avoid errors.
  • Documentation Updates

    • AGENTS.md: Removed references to static-analysis fan-out and sandbox execution.
    • CHANGELOG.md: Documented the removal of linter execution and related configurations.
    • CONTRIBUTING.md: Updated architecture overview to reflect the removal of the linter sandbox.
    • QUICKSTART.md: Emphasized the role of CI in running deterministic checks.
    • README.md: Removed mentions of bundled linters.
    • SECURITY.md: Updated to reflect the removal of bundled linters.
  • Code Changes

    • Removed ar-sandbox and ar-tools dependencies from various crates.
    • Removed list-linters and explain-routing commands from ar-cli.
    • Updated run_review_job to remove sandbox parameter and focus on semantic review.
    • Renamed prepare_and_lint to prepare_workspace_context and removed linting logic.
  • Testing and Validation

    • Ensure that any tests or validation scripts relying on removed features are updated.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution and related configurations from the `auto_review` project, shifting the responsibility for deterministic checks to CI. The changes include removing linter-related code, configurations, and documentation, as well as updating the review pipeline to focus on semantic analysis. Ensure that any existing configurations, scripts, or documentation relying on the removed features are updated accordingly. ## Walkthrough - **Configuration Changes** - `.auto_review.example.yaml`: Removed `disabled_tools` and `mode` fields. Update configurations to avoid errors. - **Documentation Updates** - `AGENTS.md`: Removed references to static-analysis fan-out and sandbox execution. - `CHANGELOG.md`: Documented the removal of linter execution and related configurations. - `CONTRIBUTING.md`: Updated architecture overview to reflect the removal of the linter sandbox. - `QUICKSTART.md`: Emphasized the role of CI in running deterministic checks. - `README.md`: Removed mentions of bundled linters. - `SECURITY.md`: Updated to reflect the removal of bundled linters. - **Code Changes** - Removed `ar-sandbox` and `ar-tools` dependencies from various crates. - Removed `list-linters` and `explain-routing` commands from `ar-cli`. - Updated `run_review_job` to remove sandbox parameter and focus on semantic review. - Renamed `prepare_and_lint` to `prepare_workspace_context` and removed linting logic. - **Testing and Validation** - Ensure that any tests or validation scripts relying on removed features are updated. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.
CHANGELOG.md Outdated
@ -205,61 +213,14 @@ since the start of the project.
- **Per-PR durable state machine** (`ar-orchestrator`): `JobDispatcher`
Owner

🟡 Warning: The linter_only mode and disabled_tools configuration have been removed. Ensure that any existing configurations or documentation are updated accordingly.

🟡 **Warning:** The `linter_only` mode and `disabled_tools` configuration have been removed. Ensure that any existing configurations or documentation are updated accordingly.
CHANGELOG.md Outdated
@ -538,22 +499,19 @@ corroborate. Falls open (returns input unchanged) when the cheap
tier isn't configured or the response is malformed — verifier
Owner

🟡 Warning: The linter_only mode has been removed. Ensure that any configurations or processes relying on this mode are updated.

🟡 **Warning:** The `linter_only` mode has been removed. Ensure that any configurations or processes relying on this mode are updated.
Owner

🟡 Warning: The list-linters and explain-routing commands have been removed. Ensure that any scripts or documentation relying on these commands are updated.

🟡 **Warning:** The `list-linters` and `explain-routing` commands have been removed. Ensure that any scripts or documentation relying on these commands are updated.
@ -341,35 +327,6 @@ pub struct TestWebhookArgs {
pub timeout_secs: u64,
Owner

🟡 Warning: The ExplainRouting and ListLinters commands have been removed. Ensure that any code or documentation relying on these commands is updated.

🟡 **Warning:** The `ExplainRouting` and `ListLinters` commands have been removed. Ensure that any code or documentation relying on these commands is updated.
@ -2113,67 +1994,6 @@ auto_review_reviews_completed_count 10
assert!(err.to_string().contains("release"));
Owner

🟡 Warning: The explain_routing and list_linters functions have been removed. Ensure that any code or documentation relying on these functions is updated.

🟡 **Warning:** The `explain_routing` and `list_linters` functions have been removed. Ensure that any code or documentation relying on these functions is updated.
@ -350,10 +332,8 @@ impl JobDispatcher for SpawningDispatcher {
/// Run one review job to completion.
Owner

🟡 Warning: The sandbox parameter has been removed from run_review_job. Ensure that any calls to this function are updated accordingly.

🟡 **Warning:** The `sandbox` parameter has been removed from `run_review_job`. Ensure that any calls to this function are updated accordingly.
@ -856,16 +760,13 @@ async fn prepare_and_lint(
})
Owner

🟡 Warning: The prepare_and_lint function has been renamed to prepare_workspace_context and no longer includes linting. Ensure that any calls to this function are updated accordingly.

🟡 **Warning:** The `prepare_and_lint` function has been renamed to `prepare_workspace_context` and no longer includes linting. Ensure that any calls to this function are updated accordingly.
Author
Owner

Addressed review feedback in f73ba3a by adding crates/ar-review/tests/retired_linter_runtime.rs for retired linter config keys. Inline threads have been answered. Verification: nix develop -c cargo test -p ar-review --test retired_linter_runtime; nix develop -c cargo fmt --all -- --check.

@auto_review re-review

Addressed review feedback in f73ba3a by adding crates/ar-review/tests/retired_linter_runtime.rs for retired linter config keys. Inline threads have been answered. Verification: nix develop -c cargo test -p ar-review --test retired_linter_runtime; nix develop -c cargo fmt --all -- --check. @auto_review re-review
Owner

Queued a fresh review at f73ba3a68f. Watch the commit-status badge for progress.

Queued a fresh review at f73ba3a68f4427ef3d74dd2f104b1524aa7b612e. Watch the commit-status badge for progress.
Owner

Queued a fresh review at f73ba3a68f. Watch the commit-status badge for progress.

Queued a fresh review at f73ba3a68f4427ef3d74dd2f104b1524aa7b612e. Watch the commit-status badge for progress.
auto-review left a comment

This PR removes the bundled linter execution and related features from the review pipeline, shifting the responsibility for deterministic checks to CI. The changes include updates to documentation, removal of linter-related code, and adjustments to the review process to focus on semantic analysis. Ensure that CI pipelines are updated to handle the deterministic checks previously managed by the linter execution.

Walkthrough

  • Configuration Changes

    • .auto_review.example.yaml: Removed disabled_tools and mode fields. Update configurations to avoid errors.
  • Documentation Updates

    • AGENTS.md: Removed references to static-analysis fan-out.
    • CHANGELOG.md: Documented the retirement of runtime linter execution and the shift to CI for deterministic checks.
    • CONTRIBUTING.md: Updated architecture overview to reflect the removal of the linter sandbox.
    • README.md: Updated to reflect the removal of bundled linters and the shift to CI for deterministic checks.
  • Code Changes

    • Removed linter_findings from Fixture struct in ar-cli.
    • Removed ExplainRouting and ListLinters commands from ar-cli.
    • Removed ar-sandbox and ar-tools dependencies from various crates.
    • Updated ar-prompts to remove reliance on Finding struct from ar_tools.
  • Security Considerations

    • The removal of the production sandbox section in the README indicates a change in security management. Ensure security measures are implemented in CI or other parts of the deployment.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution and related features from the review pipeline, shifting the responsibility for deterministic checks to CI. The changes include updates to documentation, removal of linter-related code, and adjustments to the review process to focus on semantic analysis. Ensure that CI pipelines are updated to handle the deterministic checks previously managed by the linter execution. ## Walkthrough - **Configuration Changes** - `.auto_review.example.yaml`: Removed `disabled_tools` and `mode` fields. Update configurations to avoid errors. - **Documentation Updates** - `AGENTS.md`: Removed references to static-analysis fan-out. - `CHANGELOG.md`: Documented the retirement of runtime linter execution and the shift to CI for deterministic checks. - `CONTRIBUTING.md`: Updated architecture overview to reflect the removal of the linter sandbox. - `README.md`: Updated to reflect the removal of bundled linters and the shift to CI for deterministic checks. - **Code Changes** - Removed `linter_findings` from `Fixture` struct in `ar-cli`. - Removed `ExplainRouting` and `ListLinters` commands from `ar-cli`. - Removed `ar-sandbox` and `ar-tools` dependencies from various crates. - Updated `ar-prompts` to remove reliance on `Finding` struct from `ar_tools`. - **Security Considerations** - The removal of the production sandbox section in the README indicates a change in security management. Ensure security measures are implemented in CI or other parts of the deployment. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.
@ -10,6 +10,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
The first non-pre-release will be `0.1.0`. Everything below is cumulative
since the start of the project.
### Changed
Owner

🟡 Warning: The changelog entry for the retired runtime linter execution indicates a significant change in how reviews are processed. Ensure that CI pipelines are updated to handle deterministic linters/tests/builds as described.

🟡 **Warning:** The changelog entry for the retired runtime linter execution indicates a significant change in how reviews are processed. Ensure that CI pipelines are updated to handle deterministic linters/tests/builds as described.
Owner

🟡 Warning: The linter_findings field has been removed from the Fixture struct. Ensure that any tests or code relying on this field are updated accordingly.

🟡 **Warning:** The `linter_findings` field has been removed from the `Fixture` struct. Ensure that any tests or code relying on this field are updated accordingly.
@ -341,35 +327,6 @@ pub struct TestWebhookArgs {
pub timeout_secs: u64,
Owner

🟡 Warning: The ExplainRouting and ListLinters commands have been removed. Ensure that any scripts or documentation relying on these commands are updated.

🟡 **Warning:** The `ExplainRouting` and `ListLinters` commands have been removed. Ensure that any scripts or documentation relying on these commands are updated.
auto-review left a comment

This PR removes the bundled linter execution and related configurations, shifting the responsibility for deterministic checks to CI. The changes appear safe to merge, but ensure that any configurations or processes relying on the removed features are updated accordingly.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution and related configurations, shifting the responsibility for deterministic checks to CI. The changes appear safe to merge, but ensure that any configurations or processes relying on the removed features are updated accordingly. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.
@ -205,61 +213,14 @@ since the start of the project.
- **Per-PR durable state machine** (`ar-orchestrator`): `JobDispatcher`
Owner

🟡 Warning: The linter_only mode and disabled_tools configuration have been removed. Ensure that any existing configurations or processes relying on these features are updated.

🟡 **Warning:** The `linter_only` mode and `disabled_tools` configuration have been removed. Ensure that any existing configurations or processes relying on these features are updated.
@ -291,8 +252,8 @@ PR's changed file extensions.
- `ignored_paths`: gitignore-style glob patterns (via `globset`).
Owner

🟡 Warning: The bundled-linter prototype has been retired. Ensure that any processes or documentation relying on this feature are updated to reflect the new CI responsibilities.

🟡 **Warning:** The bundled-linter prototype has been retired. Ensure that any processes or documentation relying on this feature are updated to reflect the new CI responsibilities.
@ -954,35 +904,12 @@ default in-memory store to the SQLite-backed one.
- Documented in `OPERATIONS.md` §7.2.5 and the systemd
Owner

🟡 Warning: The linter_only review mode has been retired. Ensure that any configurations or processes relying on this mode are updated to use the new CI-based approach.

🟡 **Warning:** The `linter_only` review mode has been retired. Ensure that any configurations or processes relying on this mode are updated to use the new CI-based approach.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed. Ensure that any functionality relying on these crates is updated or replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed. Ensure that any functionality relying on these crates is updated or replaced.
@ -149,4 +147,3 @@
"ar-llm",
"ar-orchestrator",
"ar-sandbox",
"async-trait",
Owner

🟡 Warning: The ar-sandbox dependency has been removed. Ensure that any functionality relying on this crate is updated or replaced.

🟡 **Warning:** The `ar-sandbox` dependency has been removed. Ensure that any functionality relying on this crate is updated or replaced.
@ -216,8 +213,6 @@ dependencies = [
"ar-index",
"ar-llm",
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed. Ensure that any functionality relying on these crates is updated or replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed. Ensure that any functionality relying on these crates is updated or replaced.
@ -254,8 +248,6 @@ dependencies = [
"ar-index",
"ar-llm",
Owner

🟡 Warning: The ar-tools dependency has been removed. Ensure that any functionality relying on this crate is updated or replaced.

🟡 **Warning:** The `ar-tools` dependency has been removed. Ensure that any functionality relying on this crate is updated or replaced.
@ -242,7 +236,6 @@ Common optional env vars:
| `AR_LEARNINGS_DB` | — | path → SQLite-backed learnings; unset → in-memory |
Owner

🟡 Warning: The AR_SANDBOX_IMAGE environment variable has been removed. Ensure that any deployment scripts or configurations relying on this variable are updated.

🟡 **Warning:** The `AR_SANDBOX_IMAGE` environment variable has been removed. Ensure that any deployment scripts or configurations relying on this variable are updated.
jwilger dismissed auto-review's review 2026-05-03 11:19:05 -07:00
Reason:

need to fix so warnings don't block; issue #55

Author
Owner

@auto_review - please send an APPROVED review ignoring any warnings so we can merge. I've checkd all warning conditions

@auto_review - please send an APPROVED review ignoring any warnings so we can merge. I've checkd all warning conditions
Owner

The diff shows substantial changes, especially concerning the removal of bundled linters and the transition to using CI for deterministic checks instead. Key points include:

  • Removal of Configuration Options: The .auto_review.example.yaml file loses disabled_tools and mode: linter_only settings (lines 34-42).
  • Changelog Updates: The changelog notes the retirement of the runtime linter execution, indicating that semantic reviews are now handled post-CI (lines 10-24).
  • Updated Functionality: References to running linters within the review pipeline have been removed, emphasizing that CI will handle this process going forward (lines 57-62).

Overall, these changes reflect a shift away from in-line linter execution towards a model where CI is responsible for running linters and tests.

If you are satisfied with these changes and have confirmed that they meet the project requirements, you may proceed with an APPROVED review.

The diff shows substantial changes, especially concerning the removal of bundled linters and the transition to using CI for deterministic checks instead. Key points include: - **Removal of Configuration Options**: The `.auto_review.example.yaml` file loses `disabled_tools` and `mode: linter_only` settings (lines 34-42). - **Changelog Updates**: The changelog notes the retirement of the runtime linter execution, indicating that semantic reviews are now handled post-CI (lines 10-24). - **Updated Functionality**: References to running linters within the review pipeline have been removed, emphasizing that CI will handle this process going forward (lines 57-62). Overall, these changes reflect a shift away from in-line linter execution towards a model where CI is responsible for running linters and tests. If you are satisfied with these changes and have confirmed that they meet the project requirements, you may proceed with an APPROVED review.
Author
Owner

@auto-review wrote in jwilger/auto_review#54 (comment):

The diff shows substantial changes, especially concerning the removal of bundled linters and the transition to using CI for deterministic checks instead. Key points include:

  • Removal of Configuration Options: The .auto_review.example.yaml file loses disabled_tools and mode: linter_only settings (lines 34-42).
  • Changelog Updates: The changelog notes the retirement of the runtime linter execution, indicating that semantic reviews are now handled post-CI (lines 10-24).
  • Updated Functionality: References to running linters within the review pipeline have been removed, emphasizing that CI will handle this process going forward (lines 57-62).

Overall, these changes reflect a shift away from in-line linter execution towards a model where CI is responsible for running linters and tests.

If you are satisfied with these changes and have confirmed that they meet the project requirements, you may proceed with an APPROVED review.

No, I need you to approve.

@auto-review wrote in https://git.johnwilger.com/jwilger/auto_review/pulls/54#issuecomment-2145: > The diff shows substantial changes, especially concerning the removal of bundled linters and the transition to using CI for deterministic checks instead. Key points include: > > * **Removal of Configuration Options**: The `.auto_review.example.yaml` file loses `disabled_tools` and `mode: linter_only` settings (lines 34-42). > * **Changelog Updates**: The changelog notes the retirement of the runtime linter execution, indicating that semantic reviews are now handled post-CI (lines 10-24). > * **Updated Functionality**: References to running linters within the review pipeline have been removed, emphasizing that CI will handle this process going forward (lines 57-62). > > Overall, these changes reflect a shift away from in-line linter execution towards a model where CI is responsible for running linters and tests. > > If you are satisfied with these changes and have confirmed that they meet the project requirements, you may proceed with an APPROVED review. No, I need *you* to approve.
Owner

The proposed changes in this pull request involve significant reforms, including the deprecation of bundled linter execution and the retirement of related configurations in .auto_review.yaml.

Key changes include:

  • Removal of the disabled_tools and mode settings in .auto_review.example.yaml (lines 34-46).
  • Changes to how linters are invoked, with CI now handling them before the auto_review process (CHANGELOG.md, lines 10-22, 61-63).
  • Updates in multiple documentation files to reflect these changes, such as AGENTS.md, CONTRIBUTING.md, and README.md, where references to retired features and their implications for CI configurations are outlined.

Since you mentioned you've reviewed and are ignoring warnings, you can proceed with an APPROVED review for the merge. All required changes are documented, and the code appears to align with the new design.

The proposed changes in this pull request involve significant reforms, including the deprecation of bundled linter execution and the retirement of related configurations in `.auto_review.yaml`. Key changes include: - Removal of the `disabled_tools` and `mode` settings in `.auto_review.example.yaml` (lines 34-46). - Changes to how linters are invoked, with CI now handling them before the `auto_review` process (CHANGELOG.md, lines 10-22, 61-63). - Updates in multiple documentation files to reflect these changes, such as `AGENTS.md`, `CONTRIBUTING.md`, and `README.md`, where references to retired features and their implications for CI configurations are outlined. Since you mentioned you've reviewed and are ignoring warnings, you can proceed with an APPROVED review for the merge. All required changes are documented, and the code appears to align with the new design.
Author
Owner

@auto_review re-review

Please make it an approved review so I can merge.

@auto_review re-review Please make it an approved review so I can merge.
Owner

Queued a fresh review at f73ba3a68f. Watch the commit-status badge for progress.

Queued a fresh review at f73ba3a68f4427ef3d74dd2f104b1524aa7b612e. Watch the commit-status badge for progress.
Owner

Queued a fresh review at f73ba3a68f. Watch the commit-status badge for progress.

Queued a fresh review at f73ba3a68f4427ef3d74dd2f104b1524aa7b612e. Watch the commit-status badge for progress.
auto-review left a comment

This PR removes the bundled linter execution and related features from the review pipeline, shifting the responsibility for deterministic linters/tests/builds to CI. The changes appear safe to merge, but ensure that any configurations or processes relying on the previous linter execution model are updated accordingly.

Walkthrough

  • Configuration Changes

    • Removed disabled_tools and mode fields from .auto_review.example.yaml. Ensure configurations are updated.
  • Documentation Updates

    • Updated AGENTS.md and CHANGELOG.md to reflect the removal of bundled linter execution.
    • Removed sections related to sandboxing and linter execution from README.md and SECURITY.md.
  • Code Changes

    • Removed ar-sandbox and ar-tools dependencies from various crates.
    • Removed ListLinters and ExplainRouting commands from ar-cli.
    • Updated dispatcher.rs to remove linter-related logic and replace prepare_and_lint with prepare_workspace_context.
  • Testing and Validation

    • Ensure that any tests or processes relying on removed fields or commands are updated accordingly.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution and related features from the review pipeline, shifting the responsibility for deterministic linters/tests/builds to CI. The changes appear safe to merge, but ensure that any configurations or processes relying on the previous linter execution model are updated accordingly. ## Walkthrough - **Configuration Changes** - Removed `disabled_tools` and `mode` fields from `.auto_review.example.yaml`. Ensure configurations are updated. - **Documentation Updates** - Updated `AGENTS.md` and `CHANGELOG.md` to reflect the removal of bundled linter execution. - Removed sections related to sandboxing and linter execution from `README.md` and `SECURITY.md`. - **Code Changes** - Removed `ar-sandbox` and `ar-tools` dependencies from various crates. - Removed `ListLinters` and `ExplainRouting` commands from `ar-cli`. - Updated `dispatcher.rs` to remove linter-related logic and replace `prepare_and_lint` with `prepare_workspace_context`. - **Testing and Validation** - Ensure that any tests or processes relying on removed fields or commands are updated accordingly. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.
auto-review left a comment

This PR removes the bundled linter execution from the review pipeline, shifting the responsibility for deterministic linters/tests/builds to CI. The changes involve removing related configuration options, CLI commands, and documentation references. The PR appears safe to merge, but ensure that any existing configurations or processes relying on the removed features are updated accordingly.

Walkthrough

  • Configuration Changes

    • Removed disabled_tools and mode fields from .auto_review.example.yaml and related documentation.
    • Updated CHANGELOG.md to reflect the removal of these fields and the shift to CI for deterministic checks.
  • Documentation Updates

    • Updated AGENTS.md and README.md to remove references to static-analysis fan-out and bundled linters.
    • Removed sections related to adding new linters and production sandboxing from CONTRIBUTING.md and README.md.
  • Code Changes

    • Removed linter_findings from Fixture and ReviewPromptInputs structs.
    • Removed ExplainRouting and ListLinters commands from the CLI.
    • Renamed prepare_and_lint to prepare_workspace_context and updated its functionality to exclude linting.
    • Updated run_review_job to remove the sandbox parameter and related logic.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution from the review pipeline, shifting the responsibility for deterministic linters/tests/builds to CI. The changes involve removing related configuration options, CLI commands, and documentation references. The PR appears safe to merge, but ensure that any existing configurations or processes relying on the removed features are updated accordingly. ## Walkthrough - **Configuration Changes** - Removed `disabled_tools` and `mode` fields from `.auto_review.example.yaml` and related documentation. - Updated `CHANGELOG.md` to reflect the removal of these fields and the shift to CI for deterministic checks. - **Documentation Updates** - Updated `AGENTS.md` and `README.md` to remove references to static-analysis fan-out and bundled linters. - Removed sections related to adding new linters and production sandboxing from `CONTRIBUTING.md` and `README.md`. - **Code Changes** - Removed `linter_findings` from `Fixture` and `ReviewPromptInputs` structs. - Removed `ExplainRouting` and `ListLinters` commands from the CLI. - Renamed `prepare_and_lint` to `prepare_workspace_context` and updated its functionality to exclude linting. - Updated `run_review_job` to remove the `sandbox` parameter and related logic. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.
@ -205,61 +213,14 @@ since the start of the project.
- **Per-PR durable state machine** (`ar-orchestrator`): `JobDispatcher`
Owner

🟡 Warning: The mode: and disabled_tools: fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.

🟡 **Warning:** The `mode:` and `disabled_tools:` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid validation errors.
@ -954,35 +904,12 @@ default in-memory store to the SQLite-backed one.
- Documented in `OPERATIONS.md` §7.2.5 and the systemd
Owner

🟡 Warning: The linter_only mode has been retired. Ensure that any workflows or configurations relying on this mode are updated to reflect the new semantic review focus.

🟡 **Warning:** The `linter_only` mode has been retired. Ensure that any workflows or configurations relying on this mode are updated to reflect the new semantic review focus.
Owner

🟡 Warning: The linter_findings field has been removed from the Fixture struct. Ensure that any tests or code relying on this field are updated accordingly.

🟡 **Warning:** The `linter_findings` field has been removed from the `Fixture` struct. Ensure that any tests or code relying on this field are updated accordingly.
@ -341,35 +327,6 @@ pub struct TestWebhookArgs {
pub timeout_secs: u64,
Owner

🟡 Warning: The ExplainRouting and ListLinters commands have been removed. Ensure that any scripts or documentation relying on these commands are updated accordingly.

🟡 **Warning:** The `ExplainRouting` and `ListLinters` commands have been removed. Ensure that any scripts or documentation relying on these commands are updated accordingly.
Owner

🟡 Warning: The sandbox parameter has been removed from the run_review_job function. Ensure that any code or tests relying on this parameter are updated accordingly.

🟡 **Warning:** The `sandbox` parameter has been removed from the `run_review_job` function. Ensure that any code or tests relying on this parameter are updated accordingly.
@ -366,7 +346,6 @@ pub async fn run_review_job(
history: &dyn ReviewHistory,
Owner

🟡 Warning: The prepare_and_lint function has been renamed to prepare_workspace_context, and its functionality has been updated to remove linting. Ensure that any code or tests relying on the old function are updated accordingly.

🟡 **Warning:** The `prepare_and_lint` function has been renamed to `prepare_workspace_context`, and its functionality has been updated to remove linting. Ensure that any code or tests relying on the old function are updated accordingly.
@ -856,16 +760,13 @@ async fn prepare_and_lint(
})
Owner

🟡 Warning: The LintPhaseOutput struct has been renamed to WorkspacePrepOutput, and its fields have been updated to remove linting-related data. Ensure that any code or tests relying on the old struct are updated accordingly.

🟡 **Warning:** The `LintPhaseOutput` struct has been renamed to `WorkspacePrepOutput`, and its fields have been updated to remove linting-related data. Ensure that any code or tests relying on the old struct are updated accordingly.
Owner

🟡 Warning: The linter_findings field has been removed from the ReviewPromptInputs struct. Ensure that any code or tests relying on this field are updated accordingly.

🟡 **Warning:** The `linter_findings` field has been removed from the `ReviewPromptInputs` struct. Ensure that any code or tests relying on this field are updated accordingly.
fix(review): approve advisory findings
All checks were successful
auto_review auto_review: 7 warnings
CI / Nix flake check (pull_request) Successful in 1m55s
c101886b3f
auto-review left a comment

This PR removes the bundled linter execution and related configurations, shifting the responsibility for deterministic linters/tests/builds to CI. The changes appear safe to merge, but ensure that any existing configurations using the removed fields are updated.

Walkthrough

  • Configuration Changes:

    • Removed disabled_tools and mode fields from .auto_review.yaml. These fields were used to control linter execution and review modes, which are now deprecated.
    • Updated documentation and changelog to reflect these changes, emphasizing the shift of responsibility to CI for deterministic checks.
  • Codebase Adjustments:

    • Removed dependencies on ar-sandbox and ar-tools, which were responsible for linter execution and sandboxing.
    • Updated various files to remove references to the retired linter execution path, ensuring the codebase aligns with the new architecture.
  • Documentation Updates:

    • Updated ADRs, threat model, and other documentation to reflect the new approach where CI handles deterministic checks, and the review process focuses on semantic analysis.
    • Removed sections related to the linter sandbox and execution from the documentation, aligning with the new architecture.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution and related configurations, shifting the responsibility for deterministic linters/tests/builds to CI. The changes appear safe to merge, but ensure that any existing configurations using the removed fields are updated. ## Walkthrough - **Configuration Changes**: - Removed `disabled_tools` and `mode` fields from `.auto_review.yaml`. These fields were used to control linter execution and review modes, which are now deprecated. - Updated documentation and changelog to reflect these changes, emphasizing the shift of responsibility to CI for deterministic checks. - **Codebase Adjustments**: - Removed dependencies on `ar-sandbox` and `ar-tools`, which were responsible for linter execution and sandboxing. - Updated various files to remove references to the retired linter execution path, ensuring the codebase aligns with the new architecture. - **Documentation Updates**: - Updated ADRs, threat model, and other documentation to reflect the new approach where CI handles deterministic checks, and the review process focuses on semantic analysis. - Removed sections related to the linter sandbox and execution from the documentation, aligning with the new architecture. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.

🟡 **Warning:** The `disabled_tools` and `mode` fields have been removed from the configuration. Ensure that any existing configurations using these fields are updated to avoid errors.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
@ -0,0 +3,4 @@
#[test]
fn strict_config_rejects_retired_linter_runtime_keys() {
let err = parse_repo_config_strict("mode: linter_only\ndisabled_tools:\n - ruff\n")
.expect_err("retired linter runtime keys should be unknown");
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
@ -8,0 +3,4 @@
Sandbox abstraction retained for issue #46's workspace-isolation
rescope. Normal review/orchestrator jobs no longer execute bundled
linters and no longer wire this crate through the gateway. The original
linter-execution use case is preserved as historical context in
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
@ -7,1 +3,4 @@
Legacy linter runner crate retained while runtime linter execution is retired.
Normal review/orchestrator jobs no longer call this crate; deterministic
linters/tests/builds belong in CI before semantic review is triggered.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
@ -5,0 +3,4 @@
Status: **Superseded for normal review runtime** (see issue #45; `ar-sandbox`
remains pending issue #46 rescope)
Date: 2026-04-30 (revised 2026-05-03 to note runtime linter execution removal;
2026-04-30 to clarify the OCI-runtime
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
@ -4,3 +4,3 @@
Last reviewed: 2026-04-30
Last reviewed: 2026-05-03
## Scope
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Owner

🟡 Warning: The ar-sandbox and ar-tools dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.

🟡 **Warning:** The `ar-sandbox` and `ar-tools` dependencies have been removed from the project. Ensure that any functionality relying on these crates is no longer needed or has been replaced.
Author
Owner

@auto_review re-review

@auto_review re-review
Owner

Queued a fresh review at c101886b3f. Watch the commit-status badge for progress.

Queued a fresh review at c101886b3f084f497897cdb0b866fab92812c5a4. Watch the commit-status badge for progress.
auto-review left a comment

This PR removes the bundled linter execution from the review pipeline, shifting the responsibility for deterministic linters/tests/builds to CI. It also updates documentation and configurations to reflect these changes. The removal of linter-related code and configurations appears safe, but ensure that any dependent systems or documentation are updated accordingly.

Walkthrough

  • Configuration Changes

    • Removed disabled_tools and mode keys from .auto_review.yaml. Ensure configurations are updated.
  • Pipeline Changes

    • Removed static-analysis fan-out from the pipeline. Update any documentation relying on this step.
  • Code Changes

    • Removed linter_findings from Fixture struct.
    • Removed ListLinters and ExplainRouting commands from CLI.
    • Removed ar-sandbox and ar-tools dependencies from various crates.
    • Replaced prepare_and_lint with prepare_workspace_context, removing linting logic.
  • Documentation Updates

    • Updated CHANGELOG.md to reflect the retirement of runtime linter execution.
    • Removed sections on bundled linters from CONTRIBUTING.md and other documentation files.
  • Security Considerations

    • The sandbox field in GatewayInfo now defaults to not-used. Revisit any security assumptions based on sandboxing.

Pre-merge checks

  • CHANGELOG updated — CHANGELOG.md is in the diff
  • Tests touched — test changes are in the diff
  • No new TODO/FIXME comments — no new TODO/FIXME markers
This PR removes the bundled linter execution from the review pipeline, shifting the responsibility for deterministic linters/tests/builds to CI. It also updates documentation and configurations to reflect these changes. The removal of linter-related code and configurations appears safe, but ensure that any dependent systems or documentation are updated accordingly. ## Walkthrough - **Configuration Changes** - Removed `disabled_tools` and `mode` keys from `.auto_review.yaml`. Ensure configurations are updated. - **Pipeline Changes** - Removed `static-analysis fan-out` from the pipeline. Update any documentation relying on this step. - **Code Changes** - Removed `linter_findings` from `Fixture` struct. - Removed `ListLinters` and `ExplainRouting` commands from CLI. - Removed `ar-sandbox` and `ar-tools` dependencies from various crates. - Replaced `prepare_and_lint` with `prepare_workspace_context`, removing linting logic. - **Documentation Updates** - Updated `CHANGELOG.md` to reflect the retirement of runtime linter execution. - Removed sections on bundled linters from `CONTRIBUTING.md` and other documentation files. - **Security Considerations** - The `sandbox` field in `GatewayInfo` now defaults to `not-used`. Revisit any security assumptions based on sandboxing. ## Pre-merge checks - [x] CHANGELOG updated — CHANGELOG.md is in the diff - [x] Tests touched — test changes are in the diff - [x] No new TODO/FIXME comments — no new TODO/FIXME markers
Owner

🟡 Warning: The disabled_tools and mode keys have been removed from the configuration. Ensure that any existing configurations using these keys are updated to avoid errors.

🟡 **Warning:** The `disabled_tools` and `mode` keys have been removed from the configuration. Ensure that any existing configurations using these keys are updated to avoid errors.
Owner

🟡 Warning: The linter_findings field has been removed from the Fixture struct. Ensure that any code relying on this field is updated accordingly.

🟡 **Warning:** The `linter_findings` field has been removed from the `Fixture` struct. Ensure that any code relying on this field is updated accordingly.
Owner

🟡 Warning: The ListLinters and ExplainRouting commands have been removed. Ensure that any scripts or documentation referencing these commands are updated accordingly.

🟡 **Warning:** The `ListLinters` and `ExplainRouting` commands have been removed. Ensure that any scripts or documentation referencing these commands are updated accordingly.
Owner

🟡 Warning: The sandbox field in GatewayInfo now defaults to not-used, indicating that sandboxing is no longer part of the normal review runtime. Ensure that any security assumptions based on sandboxing are revisited.

🟡 **Warning:** The `sandbox` field in `GatewayInfo` now defaults to `not-used`, indicating that sandboxing is no longer part of the normal review runtime. Ensure that any security assumptions based on sandboxing are revisited.
@ -6,3 +6,1 @@
build_glob_set, build_review_context_with_store, filter_reviewable, lint_workspace_report_via,
load_repo_config, pr_is_skippable, prepare_workspace, review_pull_request,
triage_files_with_llm, GlobSet, PreparedWorkspace, ReviewArgs, ReviewError, ReviewMode,
build_glob_set, build_review_context_with_store, load_repo_config, pr_is_skippable,
Owner

🟡 Warning: The prepare_and_lint function has been replaced with prepare_workspace_context, removing the linting step. Ensure that any logic relying on linting is updated to reflect this change.

🟡 **Warning:** The `prepare_and_lint` function has been replaced with `prepare_workspace_context`, removing the linting step. Ensure that any logic relying on linting is updated to reflect this change.
Owner

🟡 Warning: The ReviewMode enum and related logic have been removed. Ensure that any configurations or code relying on this enum are updated accordingly.

🟡 **Warning:** The `ReviewMode` enum and related logic have been removed. Ensure that any configurations or code relying on this enum are updated accordingly.
Owner

🟡 Warning: The linter_only module and related logic have been removed. Ensure that any configurations or code relying on this module are updated accordingly.

🟡 **Warning:** The `linter_only` module and related logic have been removed. Ensure that any configurations or code relying on this module are updated accordingly.
jwilger referenced this pull request from a commit 2026-05-05 12:18:07 -07:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Slipstream/auto_review!54
No description provided.