Skip to content

lint: refine or-true diagnostics using corpus examples #50

Description

@x1f9-ricon

Problem

lint:or-true correctly treats || true as a failure-swallowing smell, but the codebase-wide QA pilot found several recurring categories that deserve better diagnostics than a flat hit list.

We should keep catching real silent failures, while making intentional cases easier to classify and justify.

Related:

Evidence from the corpus pilot

Corpus: 22 selected KKL/fold/den repos; included mise.toml, .mise/tasks/**, and GitHub workflows.

Mechanical scan artifact found 122 || true hits across 10 repos. A gpt-5.4-mini analysis drone then categorized high-signal examples.

High-risk / likely real findings

KnickKnackLabs/rudi@d01f16b has rotate-key paths that swallow key-rotation operations:

git -C "$TARGET" crypt lock 2>/dev/null || true
git -C "$TARGET" crypt unlock 2>/dev/null || true
git -C "$TARGET" commit -q -m "Rotate key '$KEY_NAME': re-encrypted with fresh key" 2>/dev/null || true

These look like real risk: key rotation should not silently continue if lock/unlock/commit failed.

ricon-family/den@0e7c312 has agent/welcome probes like:

AGENTS=$(mise run -q agent:list 2>/dev/null) || true
agents=$(cd "$DEN_DIR" && mise run -q agent:list 2>/dev/null || true)
HUMAN_FILE=$(cd "$DEN_DIR" && mise run -q human 2>/dev/null || true)

Some may be acceptable optional probes, but they should be explicit about degraded behavior. Den also does not currently enforce configured codebase lint in CI per #49's scan.

Common intentional/benign categories

Arithmetic increment workaround under set -e:

((modified++)) || true
((changes_new++)) || true

These occurred in KnickKnackLabs/notes@319ab27. They are not the same risk as swallowing a command failure. The lint could suggest safer alternatives such as:

((modified += 1))

Best-effort cleanup/probes:

kill "$PID" 2>/dev/null || true
diff -u "$OUT_FILE" <(echo "$output") || true
count=$(printf '%s\n' "$output" | grep -cE '...' || true)  # grep -c exits 1 on zero matches

Some of these are legitimate, especially with an inline reason. Others should use explicit if ! ...; then handling.

Proposed improvement

Improve lint:or-true guidance/classification rather than just broadening the ban.

Possible behavior:

  1. Continue failing on unannotated || true / || : in executable task/workflow code.

  2. Emit a more specific message for recognized categories:

    • arithmetic increments: suggest ((n += 1)) or equivalent;
    • grep -c / zero-match counts: require or suggest an inline reason;
    • kill cleanup: allow only with explicit ignore reason;
    • destructive/auth/key/commit/validation commands: keep as high-risk findings.
  3. Require an explicit inline ignore for intentional cases, ideally compatible with Structured inline codebase:ignore + lint:ignore-justification meta-rule #20:

    # codebase:ignore or-true — grep -c exits 1 on zero matches
  4. Add fixtures from the corpus categories above so future rule behavior is calibrated.

Acceptance criteria

  • Tests cover risky command suppression, arithmetic increments, best-effort cleanup, count/probe commands, and explicit ignore justifications.
  • Risky examples still fail.
  • Intentional examples either pass with structured ignore or produce actionable category-specific guidance.
  • README/help text explains the distinction: || true is not automatically evil, but unclassified failure swallowing is not acceptable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions