Add non-failing linter settings advice

[justin808]

Why

The seam doctor currently verifies that a repository exposes a lint command, but it cannot show whether agent-written code is bounded by complexity, size, or parameter limits. This adds actionable linter posture without converting quality guidance into a seam-contract failure.

Fixes #468

What changed

• Added a static, non-executing RuboCop and ESLint settings analyzer with actual configured values and copy-pasteable recommendations for missing or disabled limits.
• Added a top-level advice JSON channel and matching standalone seam-doctor human output. Advice never enters issues and never changes the seam-doctor exit status.
• Passed advice through workflows.seam.details while preserving the stack check's healthy status.
• Added repo-owned suggested thresholds and exact per-recommendation suppressions under optional lint_advice policy.
• Kept v1 intentionally bounded to root .rubocop.yml, eslint.config.*, and .eslintrc* files. It does not evaluate JavaScript, rewrite consumer configs, add broader linter ecosystems, or re-enable this repository's RuboCop metrics.

How to review and verify

1. Review bin/agent_doctor/linter_advice.rb for the parser boundary and bin/agent-workflow-seam-doctor for the non-failing output integration.
2. Review the focused fixtures for disabled/enabled RuboCop metrics, ESLint, no-config output, policy suppression/thresholds, JSON, and stack-doctor health.
3. A real source-checkout run returns PASS, reports all eight disabled Metrics/* cops, and links Refactor oversized Ruby files and re-enable default RuboCop metrics #309.

Test plan

• ruby bin/agent-workflow-seam-doctor-test.rb — 354 runs, 2788 assertions, 0 failures, 0 errors, 0 skips.
• bin/agent-workflows-doctor-test.bash — PASS.
• Ruby syntax checks for the seam doctor and changed components, plus bash -n bin/agent-workflows-doctor-test.bash — PASS.
• Targeted RuboCop, ShellCheck, and markdownlint-cli2 on changed files — PASS.
• bin/agent-workflow-seam-doctor --root . --shared . --json — PASS with 0 issues and all eight expected RuboCop recommendations.
• git diff --check — PASS.
• bin/validate intentionally not run locally per lane instruction; hosted CI is the full-suite source because current main has the known prompt-headroom failure owned by Deduplicate batch-title contracts and restore prompt headroom #662.
• Hosted current-head validate ran the full suite and failed only at the known Deduplicate batch-title contracts and restore prompt headroom #662 baseline: pr-batch Codex goal prompt template has 299 chars of headroom, must keep at least 300.
• Changelog classification: deferred_to_update_changelog; CHANGELOG.md is untouched.

Integration note

This isolated lane does not modify the overlapping PRs. Later integration must reconcile these exact shared paths:

• Add repo-owned generated artifact drift verification gate #677: bin/agent-workflow-seam-doctor, bin/agent-workflow-seam-doctor-test.rb
• Distinguish an unavailable configured-review gate from unsettled reviews #665: bin/agent-workflow-seam-doctor, bin/agent-workflow-seam-doctor-test.rb, docs/seam-design.md
• Fail closed on unsettled configured reviews #469: bin/agent-workflow-seam-doctor, bin/agent-workflow-seam-doctor-test.rb, docs/seam-design.md
• Honor optional CI policy in merge assurance #446: bin/agent-workflow-seam-doctor, bin/agent-workflow-seam-doctor-test.rb, docs/seam-design.md

Risk

The ESLint analyzer intentionally recognizes literal rule settings only. Imported, computed, or spread-provided values are never executed and may be reported as missing; repositories can suppress a recommendation when that static result is intentional. Inherited RuboCop settings are identified but not resolved; recommendations then reflect only the root file. Advice-only parsing failures remain informational and cannot degrade doctor health.

Agent details

Commands and results

• Focused verification commands and outcomes are recorded in the test plan above.
• Starting main: db989c34d6910095a066beef07b67fb6be657c94.
• First published head: 5639dbca.
• Current published head: ff8ae5ec.

Decision log

• Non-blocking: support boundary for v1
  • Decision: inspect RuboCop and ESLint only, using static parsing.
  • Why: these are the acceptance-tested ecosystems; Standard Ruby does not permit reconfiguring its built-in rules in .standard.yml, and broader ecosystems would add unverified scope.
  • Review later: add other ecosystems only with concrete fixtures and valid config snippets.
• Non-blocking: advice transport
  • Decision: add top-level seam advice and copy it into healthy workflows.seam.details.
  • Why: this preserves the existing issues and exit-status contract without a stack schema/status change.
  • Review later: None.

Coordination and reviewer telemetry

• Batch: aw-medium-wave12-20260902
• Claim: codex-m5-issue468-linter-advice
• Instance: 5960ac20-452a-4b6d-bbac-6172cd4fad71
• Merge authority: none; this lane must never merge.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[justin808]

Address-review summary

Scan scope: full PR review history through 2026-09-02T11:41:23Z.

Status: cutoff-safe summary. Detailed review outcomes are collapsed below.

Detailed review outcomes

Mattered

• Seven correctness and static-boundary findings were addressed in ff8ae5ec; regressions cover malformed encodings/shapes, comments, balanced options, inherited-config disclosure, and stable source detection.
• All seven threads were replied to and resolved after current-head verification.

Optional

• Policy loading remains self-contained in the advisory component to preserve its standalone, fault-contained boundary.
• Custom stack-doctor human rendering is deferred; v1 keeps structured advice in existing stack details and friendly formatting in the standalone seam doctor.

Skipped

• CodeRabbit draft-review status contained no actionable feedback.

Deferred-work tracking: PR summary comment only.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

[justin808]

CLEAN (exact-head review, head ff8ae5e):

• Claim holder rechecked: codex-spark-m1-pr696-checker / 6eb85d0f-3d12-4a30-bc9e-7faa5c525a2b
• High-risk lens checks reviewed (linter-advice fail-open/closed, parsing, compatibility, false-positive risk, overlaps with Add repo-owned generated artifact drift verification gate #677/Distinguish an unavailable configured-review gate from unsettled reviews #665/Fail closed on unsettled configured reviews #469/Honor optional CI policy in merge assurance #446).
• Focused read-only test: ruby bin/agent-workflow-seam-doctor-test.rb -n '/AgentWorkflowSeamDoctorLinterAdviceTest/' => 17 runs, 97 assertions, 0 failures, 0 errors.
• Hosted state (pr-ci-readiness exact-head): NOT_READY; only known gate failure is validate on actions run 33624779589. Lint/CodeRabbit/claude-review passed.
• reviewDecision empty; all 9 review threads are resolved (0 unresolved).
• No blocking functional or security findings identified in current diff for this lane.

[justin808]

Reconciled exact origin/main commit f15211f into PR head 98dee35 and pushed it to jg-codex/issue-468-linter-advice. Focused seam-doctor linter-advice tests passed (AgentWorkflowSeamDoctorLinterAdviceTest plus the new numeric-literal regression), bin/lint passed cleanly, and all review threads are resolved. Live hosted state is still queued on GitHub Actions for validate, Lint, and claude-review; CodeRabbit is green but skipped because the PR is draft. Smallest remaining action: wait for GitHub Actions to leave the queue.

[claude]

call wraps the entire advisory computation in a blanket rescue StandardError. Every helper it calls into (rubocop, eslint, etc.) already rescues a narrow, intentional list of exceptions (ArgumentError, EncodingError, Psych::Exception, SystemCallError, TypeError) — this outer rescue StandardError is much broader and will also catch genuine programming bugs (e.g. NoMethodError from a future edit).

Because advice is designed to never affect seam-doctor's pass/fail status, a regression caught here just prints "Linter settings advice is unavailable..." and exits 0 — silently, with no test failure or visible error, potentially for a long time. Consider narrowing this to the same exception set used elsewhere, so real bugs surface instead of being swallowed as "advice unavailable".

[claude]

When a RuboCop cop entry exists but isn't a Hash (e.g. Metrics/AbcSize: 15 instead of Metrics/AbcSize: {Enabled: true, Max: 15}), this falls through to state = "missing" — the same label used when the cop isn't mentioned in the config at all.

setting.is_a?(Hash) is false, department_disabled is false, so the elsif branch is skipped and it lands on "missing". That's a bit misleading: the user does have an entry for this cop, it's just malformed (and RuboCop itself would likely reject/ignore it). Worth considering a distinct state (e.g. "invalid") for "key present but not a Hash", separate from "key absent entirely", so the advisory output doesn't imply nothing was configured when something (broken) actually was.

[claude]

exported_javascript_expression/javascript_export_start only resolve the literal expression written directly after export default / module.exports =. A very common flat-config idiom — building the config in a variable and exporting it by name:

const config = [{ rules: { complexity: ["error", 8], "max-lines": ["error", 240] } }];
export default config;

resolves start to just past export default, and javascript_expression then captures the bare identifier config (no {). javascript_object_bodies("config") finds no {, so javascript_rule_bodies returns [], and every rule in ESLINT_LIMITS is reported "missing" even though everything is fully configured — with no note indicating the file couldn't be resolved (unlike the JSON/YAML path, which surfaces "Could not inspect linter settings" on genuine parse failure).

The PR description's Risk section does call out that "imported, computed, or spread-provided values" may be reported as missing, so this is a known/accepted v1 boundary — but worth double-checking whether this exact const config = ...; export default config shape (arguably the most common flat-config pattern in ESLint's own docs) is the intended scope, since it means the advisory silently reports "unconfigured" for a large share of real-world configs rather than surfacing an inspection-failure note.

[claude]

Minor duplication note: javascript_export_start, javascript_expression, javascript_object_bodies, javascript_array_end, javascript_object_body, and eslint_rule_expression each hand-roll their own char-by-char scanning loop that separately tracks quote/regex/escape state (via the shared javascript_literal_state helper) and depth/exit conditions. That's ~6 near-identical loops (~250 lines) instead of one shared tokenizer/segment-scanner that the callers configure with a stop condition.

Not blocking, but a fix to one edge case (e.g. resolving named exports, per the other comment on this file) would need to be replicated across every one of these scanners individually, and it's easy for them to silently drift out of sync over time.

[claude]

Review summary

Reviewed the non-failing linter advisory feature (RuboCop + ESLint settings analyzer). Overall the design is sound: advice is cleanly separated from the seam-doctor issues/exit-status contract, the RuboCop path is a straightforward YAML analysis, and the test suite is thorough (354 runs / 2788 assertions).

Left 4 inline comments on bin/agent_doctor/linter_advice.rb, roughly in priority order:

1. Broad rescue StandardError in call (line 52) — every inner helper rescues a narrow, deliberate exception list, but the outer call catches everything, including real bugs (e.g. a future NoMethodError), silently downgrading them to "advice unavailable" with no test failure.
2. Malformed RuboCop entries reported as "missing" (line 78) — e.g. Metrics/AbcSize: 15 (not a Hash) falls through to the same "missing" state as a cop that's absent entirely, rather than being flagged as malformed/invalid.
3. export default <identifier> isn't resolved (lines 259–266) — the hand-rolled JS scanner only reads the literal expression directly after export default/module.exports =. A config built as const config = {...}; export default config (a very common flat-config idiom) resolves to a bare identifier with no {, so every rule is reported "missing" even when fully configured, with no inspection-failure note. The PR's Risk section does disclose that "computed" values may be reported as missing, so this may be an accepted v1 boundary — flagging since this specific shape is arguably the most common ESLint flat-config pattern, so worth confirming it's the intended tradeoff.
4. Duplicated hand-rolled scanning loops (line 327, minor/non-blocking) — six functions (javascript_export_start, javascript_expression, javascript_object_bodies, javascript_array_end, javascript_object_body, eslint_rule_expression) each separately track quote/regex/escape state and depth/exit conditions. A shared tokenizer would cut the duplication and reduce the risk of the scanners drifting out of sync as edge cases get fixed.

No security concerns — the analyzer is purely static/non-executing as intended, and file reads are scoped to the repo root's known config filenames.