#198 engine: escalate bulk-delete of gitignored, uncommitted directories

[Anonymous]

Originally created by: fu351
Originally owned by: harshitagrawal2O

What

DestructiveCommandRule's bulk-delete check (_segment_verdict, src/doberman/engine/rules/commands.py:568) steps up to AUTH only when _count_delete_operands(tokens) >= bulk_threshold (default 25, DEFAULT_BULK_THRESHOLD, line 84).

Rescoped 2026-08-20. AN-1 (shipped in 0.18.0) already added a lexical, filename-only gate for unrecoverable gitignored data: _any_operand_unrecoverable / _rm_targets_unrecoverable_data (commands.py:323-340) match each operand's basename against _UNRECOVERABLE_DELETE_GLOBS (local DB / secret / key filenames), with no filesystem or git access. A directory operand (rm -rf data/) cannot be classified lexically and is deliberately outside that gate (the ponytail: note at line 336 defers it). That directory-level gap is what this issue is now about: the rule still has no awareness of whether a directory target is gitignored-but-uncommitted — deleting build/ or node_modules/ is cheap to regenerate, but deleting an uncommitted .env.local-adjacent working directory or a gitignored data/ folder holding un-backed-up state can be genuinely irrecoverable, and today it gets exactly the same treatment as any other bulk delete. Add a new, additive escalation: when a rm/bulk-delete segment's target(s) are inside a directory git check-ignore reports as ignored and the repo has no committed copy, lower the effective bulk threshold (or force AUTH) for that segment.

Difficulty: level-7

Additive engine change (new rule condition), raise-only by construction — it can only turn an existing PASS into AUTH for a specific new case, never the reverse. The judgment is in scoping the git integration cleanly (shell out to git check-ignore, Doberman's established pattern for "ask git rather than reimplement .gitignore parsing" — see how role_boundary.py treats repo boundaries) and handling "not a git repo" / "git not installed" gracefully.

Safety constraints (do not weaken these)

• Raise-only: this must only ever make an already-recoverable-but-risky bulk delete step up to (or stay at) AUTH — it must never lower the verdict for any command the existing bulk_threshold check already escalates, and it must never turn an existing BLOCK (e.g. _rm_is_catastrophic, line 295) into anything weaker, and it must leave the AN-1 lexical gate (_rm_targets_unrecoverable_data) exactly as it is — build beside it, not over it.
• Fail toward the existing behavior, not toward PASS: if git check-ignore fails, times out, or the target isn't inside a git repo, the segment must fall back to exactly today's behavior (the plain bulk_threshold check) — never silently skip escalation because gitignore status couldn't be determined.
• No raw path in the explanation: as with every other rule in this file, the explanation names the category ("bulk delete of gitignored, uncommitted data"), never the actual path.

Starting files

• src/doberman/engine/rules/commands.py — _segment_verdict (line 568), _count_delete_operands (line 307), the AN-1 lexical gate to build beside: _any_operand_unrecoverable / _rm_targets_unrecoverable_data (lines 323–340), DestructiveCommandRule (line 756 onward).
• New helper, e.g. _targets_gitignored_uncommitted(paths, root) — shells out to git check-ignore -q <path> (and git ls-files --error-unmatch to rule out "ignored but already tracked" false positives), following the "ask git, don't reimplement gitignore parsing" pattern.
• tests/unit/test_rule_commands.py — existing bulk-threshold tests to extend.

Acceptance criteria

• [ ] A bulk rm targeting paths inside a git-ignored, never-committed directory is escalated to AUTH at a lower operand count than DEFAULT_BULK_THRESHOLD (or always, contributor's documented choice — call it out in the PR description).
• [ ] The exact same command against a tracked (committed) directory is unaffected — same behavior as today.
• [ ] If git is unavailable or the target isn't inside a git repo, behavior falls back to today's plain bulk_threshold check (no crash, no silent PASS).
• [ ] No raw path or filename appears in the resulting explanation.
• [ ] Existing behaviour unchanged for every case not touched here (no test weakened, no output format broken)
• [ ] ruff check . && ruff format --check . and lint-imports pass
• [ ] pytest passes

How to validate

pytest tests/unit/test_rule_commands.py -v

Out of scope

• Do not implement .gitignore pattern matching yourself (no new parsing dependency) — shell out to git check-ignore.
• Do not change _rm_is_catastrophic's existing BLOCK conditions or the protected-branch force-push check.
• Do not add a new dependency (e.g. pathspec) — this issue is scoped to reusing the git binary already required for git_op actions.

Notes for contributors

New here? Start with CONTRIBUTING.md. Issues are labelled level-1 (docs only) through level-10 (new subsystem) — pick one at your level and climb. Comment to claim an issue before starting.

[Anonymous]

Ticket changed by: fu351

• status: open --> closed

[Anonymous]

Ticket changed by: fu351

• status: closed --> open

[Anonymous]

Originally posted by: harshitagrawal2O

Picking this one up.

[Anonymous]

• assigned_to: harshitagrawal2O

[Anonymous]

Originally posted by: harshitagrawal2O

Thanks for the assignment — starting on this, but I want to flag a real tension before I pick an approach.

Since this issue was filed, src/doberman/engine/rules/commands.py picked up _rm_targets_unrecoverable_data (AN-1, the lexical unrecoverable-file gate), and its neighbor _any_operand_unrecoverable carries this note:

# ponytail: lexical glob on operands only — no filesystem or git access in the
# decision path. Catches file targets; a directory operand (rm -rf data/) cannot
# be classified lexically and is deliberately out of scope (deferred — see ADR).

That's exactly this issue's scenario (a directory operand), and it reads as a deliberate call to keep the objective decision path free of filesystem/git subprocess calls. It also means the issue's own suggested precedent doesn't hold up as written — I checked role_boundary.py's repo-boundary handling, and it's pure in-memory path-prefix matching (classify(role, raw_path, root=root)), never a git subprocess; there's no existing "ask git" pattern anywhere in engine/rules/ today (confirmed: no import subprocess in that package).

So before I implement git check-ignore-based detection in the hot path, I'd rather confirm which way you want this resolved:

1. The ADR's constraint applies here too — directory-level gitignore detection needs a different strategy that keeps subprocess/filesystem calls out of the synchronous decision path (e.g., a cached/precomputed gitignore-directory set built off-path, or a narrower heuristic that doesn't need git at all), or
2. A bounded, timeout-guarded git check-ignore call is acceptable specifically here despite that note — e.g. because AN-1's comment was scoped to justify why that fix stayed lexical-only, not a blanket ban — in which case I'll implement it exactly as the issue describes (shell to git check-ignore -q, fail toward today's plain bulk_threshold behavior on any git/subprocess error).

Whichever it is, happy to proceed the same day you confirm.

[Anonymous]

Originally posted by: fu351

Assigned! This one goes deep into the engine's classification path, so take whatever design room you need and ask here before locking in an approach, I'd rather talk it through early than re-review late.