fix: quoted-path classifier bypass, edit containment, glob skips, budget env vars (0.4.3)

[vedaant00]

Four documented-contract violations found sweeping the codebase. Each is wrong behavior with no error signal, and each has a test that fails on the old code.

1. High — quoting defeated the unsnapshotted-path guard

_hits_unsnapshotted_path split raw command text, so quote characters stayed glued to the token:

command	verdict (before)
rm -rf .git	reversible=False (confirm) ✅
rm -rf ".git"	reversible=True ❌
rm -rf 'node_modules'	reversible=True ❌

reversible=True means the confirm prompt is skipped entirely, the command runs, and before_action(reversible=True) writes a ledger entry claiming it's undoable — but .git is in the snapshot skip set, so nothing was captured and opendot undo can't restore it. That's precisely the "silent un-undoable surprise" the module docstring forbids.

This was a same-file divergence: the sibling _mentions_outside_path already tokenizes with [^\s'"|&;<>]+ (quote-safe), and _first_word uses shlex. Only this helper was quote-naive.

2. High — edit had no workspace containment

edit used a bare Path.write_text, while write_file routes in-workspace writes through _safe_write_within_workspace (documented: "if any path component is a symlink … the write does not follow it"). Identical input, opposite behavior:

	edit (before)	write_file
in-workspace symlink	followed → wrote to target	neutralized → real file
link afterwards	still a symlink	replaced
reported path	real.txt (the target)	link.txt (as asked)

edit is registered as an in-workspace mutating tool and documented "surgical, undoable", so it now mirrors write_file and reports via _rel_no_resolve (the helper that exists for exactly this, and which move already uses).

3. Medium — documented glob skips were silent no-ops

OPENDOT.md advertises skip: data, *.log, secrets/, but IgnoreRules.skipped was pure set membership, so *.log only matched a file named *.log. A user's .log files were snapshotted anyway — and, per restore_snapshot, overwritten/deleted on restore. The existing test only asserted "*.log" landed in extra_skip, never that it matched, so it passed while the feature didn't work. Adds an fnmatch pass for glob-looking user entries; force_include still wins and literal defaults are untouched.

4. Medium — OPENDOT_MAX_USD / OPENDOT_MAX_TOKENS had no effect

AgentConfig reads them via default_factory, but the CLI always passes argparse's value — None when the flag is absent — and an explicitly-passed None overrides default_factory, so the factory never runs:

OPENDOT_MAX_USD=5 OPENDOT_MAX_TOKENS=1000
no-arg (factory) : 5.0 1000
cli path (None)  : None None

So OPENDOT_MAX_USD=0.50 opendot -p "…" ran with no spend cap and no warning, while README:124 states the env vars supply the defaults. Fixed at the single chokepoint (_build_agent); the CLI flag still wins.

Tests

8 new tests. All five behavioral ones fail on the pre-fix source (verified by stashing the source changes and re-running) and pass with the fix. 390 passed, ruff check + format clean. Bumped to 0.4.3.

Deliberately not included

Three further findings I left out rather than change behavior unilaterally, as they're design calls rather than clear contract violations:

• write_file takes a confirm-then-plain-write path when a symlink resolves outside the workspace, skipping the containment tiers. It is confirm-gated and honestly logged irreversible, so it's a broken-guarantee/false-test issue (the containment tests call the private helpers directly, bypassing the public gate) rather than a silent escape.
• grep --context re-emits overlapping windows, duplicating lines on nearby matches; and max_matches=0 returns one match labelled "capped at 0".
• --sandbox accepts --deny/--usd/--tokens/--api-base but forwards none of them into the container.

Happy to file these as issues.

[Copilot]

🟡 Changes recommended

The unsnapshotted-path detection in the irreversibility classifier still has an operator-adjacent bypass (e.g., redirection without whitespace), which can incorrectly skip confirmation for destructive deletes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes several “silent contract violation” cases where opendot could incorrectly treat actions as undoable (or misapply documented defaults), and adds regression tests to prevent reintroduction.

Changes:

• Harden irreversibility classification for deletes of unsnapshotted paths when quoted.
• Make edit use the same in-workspace write containment as write_file, and report the user-specified path (not a symlink target).
• Make documented skip: glob patterns actually apply, and ensure CLI budget defaults honor OPENDOT_MAX_USD / OPENDOT_MAX_TOKENS when flags are absent; bump version to 0.4.3.

File summaries

File	Description
tests/test_tools.py	Adds regression tests for edit containment + reporting when editing a symlink path.
tests/test_rules.py	Adds tests proving skip: *.log matches filenames and snapshot: still wins.
tests/test_config.py	Adds tests verifying CLI budget defaults fall back to env vars and flags override them.
tests/test_classifier.py	Adds tests ensuring quoted unsnapshotted paths remain irreversible and quoted normal paths remain reversible.
src/opendot/tools/local.py	Routes in-workspace edit writes through _safe_write_within_workspace and reports via _rel_no_resolve.
src/opendot/reversibility/snapshots.py	Implements glob-aware user skip matching via fnmatch while preserving literal defaults and force_include.
src/opendot/reversibility/classifier.py	Updates _hits_unsnapshotted_path to avoid quote-based bypasses (needs one more hardening fix—see comment).
src/opendot/cli.py	Ensures CLI passes env-derived defaults for budget caps when flags are omitted (None).
src/opendot/init.py	Bumps __version__ to 0.4.3.
pyproject.toml	Bumps package version to 0.4.3.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The fixes are narrowly scoped, align behavior with documented guarantees, and are backed by targeted tests that cover the previously silent failure modes.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Lite