Skip to content

fix(doctor): automation snapshot lives in devin-doctor state dir, not data_dir - #112

Merged
Icaro0310 merged 2 commits into
mainfrom
fix/doctor-xdg-snapshot
Oct 25, 2026
Merged

Icaro0310 merged 2 commits into
mainfrom
fix/doctor-xdg-snapshot

Conversation

@Icaro0310

@Icaro0310 Icaro0310 commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • The automation check's removed-task baseline lived at <data-dir>/doctor/automation-snapshot.json — and --data-dir names the install being inspected, which can be any directory. A run pointed at a repo checkout wrote doctor/automation-snapshot.json inside it, and a git add -A swept absolute paths and full cron commands into a public commit (real incident on this box).
  • New paths.default_state_dir() resolves doctor's own state to a dedicated devin-doctor dir: $XDG_DATA_HOME/devin-doctor on Linux, %LOCALAPPDATA%\devin-doctor on Windows (machine-local — a per-host scheduler baseline must not roam), ~/Library/Application Support/devin-doctor on macOS. DEVIN_DOCTOR_STATE_DIR overrides, matching the ecosystem's env escape-hatch convention (DEVIN_DATA_DIR/DEVIN_CONFIG_DIR).
  • Context.state_dir (None → platform default) wired from cli.main; do_check gains a test-only state_dir kwarg (like now_ms) so a unit run never writes the real baseline. Snapshot path is now <state-dir>/automation-snapshot.json.
  • Hermeticity: conftest pins DEVIN_DOCTOR_STATE_DIR under tmp_path, so no entry point (Context direct, cli.main, do_check) can touch the real state dir during tests.
  • Baseline rebuilds on the next run (the check never migrated shipped baselines — the feature is still unreleased); a stale <data-dir>/doctor/ copy can be deleted by hand.

Test plan

  • New regression test test_snapshot_never_written_inside_inspected_dirs: data_dir/cwd pointed at a repo checkout → snapshot lands under state_dir, no doctor/ dir appears in the inspected tree
  • default_state_dir resolution covered for linux (XDG), win32 (LOCALAPPDATA) and the env override across all platforms
  • 454 passed + 2 skipped in packages/doctor; full repo green per package (history 104, search 141, graph 93, pm 150)
  • ruff check clean (repo-pinned 0.16.10)

Generated with Devin


Devin Review

… data_dir

The removed-task baseline lived at <data-dir>/doctor/
automation-snapshot.json — and --data-dir names the install being
inspected, which can be any directory. A run pointed at a repo
checkout wrote doctor/automation-snapshot.json inside it, and
git add -A swept absolute paths and full cron commands into a public
commit.

- paths.default_state_dir() + Context.state_dir: the baseline now
  resolves to a dedicated devin-doctor dir — XDG_DATA_HOME on Linux,
  %LOCALAPPDATA% on Windows (machine-local, the baseline must not
  roam), Application Support on macOS — never derived from the
  inspected root. DEVIN_DOCTOR_STATE_DIR overrides.
- cli.main wires it; do_check gains a test-only state_dir param so a
  unit run never writes the real baseline.
- conftest pins DEVIN_DOCTOR_STATE_DIR under tmp_path: no entry point
  (Context, cli.main, do_check) can touch the real state dir in tests.
- regression test: a run whose data_dir/cwd are a repo checkout leaves
  it pristine — no doctor/ dir for git add -A to eat.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 25, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Devin Review

Comment thread packages/doctor/README.md Outdated
| `disk` | total data-dir size, largest DBs, `acp-messages` accumulation trend |
| `updates` | installed tool versions vs the DevKit manifest (the only check that touches the network — fetches `manifest.json` from GitHub; self-skips when unreachable, or force off with `DEVIN_DOCTOR_OFFLINE=1` / `--offline`) |
| `automation` | scheduled tasks per surface — user crontab, `systemd --user` services/timers (loaded units plus installed unit files, so unloaded timers are still seen), Windows Task Scheduler: referenced paths/binaries that no longer exist (**FAIL**), tasks removed since the last run (**FAIL**, via a snapshot at `<data-dir>/doctor/automation-snapshot.json` — the only file the tool writes, its own state), and targets inside repo checkouts the ecosystem registry marks as absorbed. Surfaces that don't exist on the host report `surface unavailable` — never an error |
| `automation` | scheduled tasks per surface — user crontab, `systemd --user` services/timers (loaded units plus installed unit files, so unloaded timers are still seen), Windows Task Scheduler: referenced paths/binaries that no longer exist (**FAIL**), tasks removed since the last run (**FAIL**, via a snapshot at `<state-dir>/automation-snapshot.json` — the only file the tool writes, its own state; `<state-dir>` is `$XDG_DATA_HOME/devin-doctor` on Linux, `%LOCALAPPDATA%\devin-doctor` on Windows, `~/Library/Application Support/devin-doctor` on macOS, overridable via `DEVIN_DOCTOR_STATE_DIR`), and targets inside repo checkouts the ecosystem registry marks as absorbed. Surfaces that don't exist on the host report `surface unavailable` — never an error |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 State paths bypass platform guides

The new automation entry places Windows and Linux state paths in the shared guide. The platform guides omit those paths, breaking the repository's required documentation split.

Learn more

The repository requires platform-specific paths and commands in the corresponding OS guides. This entry adds both platform-specific snapshot locations to the shared guide, but the Windows and Linux guides still only describe Devin's inspected data locations.

Example: A Linux user consulting the Linux guide after an automation warning finds the Devin data path, but not the new snapshot path under $XDG_DATA_HOME/devin-doctor.

Recommended fix: Keep the shared guide's explanation that the snapshot lives outside --data-dir. Move the Windows and Linux location and override instructions to README.windows.md, README.corporate-windows.md, and README.linux.md as applicable.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0f3670c. The shared README's automation row now keeps only the 'snapshot lives outside --data-dir' explanation plus the DEVIN_DOCTOR_STATE_DIR override name and the macOS path (there is no macOS guide to move it to). The per-OS locations moved into the 'Devin paths' sections: ${XDG_DATA_HOME:-$HOME/.local/share}/devin-doctor/ in README.linux.md, %LOCALAPPDATA%\devin-doctor\ in README.windows.md and README.corporate-windows.md (with the machine-local-not-roaming rationale). A new 'State stays out of git' bullet in the shared guide documents the .gitignore guard added alongside.

Comment on lines +99 to +101
override = env.get(STATE_DIR_ENV_VAR)
if override:
return Path(override).expanduser()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 State override can expose scheduled tasks

When DEVIN_DOCTOR_STATE_DIR points inside a checkout, default_state_dir accepts it without checking the destination. The automation snapshot then stores task commands in a file that git add -A can publish.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0f3670c. _write_snapshot now calls a _gitignore_state_dir guard that drops (or extends) a .gitignore covering automation-snapshot.json inside whatever dir the state resolves to — so a DEVIN_DOCTOR_STATE_DIR pointed inside a checkout still cannot leak the baseline via git add -A. The ignore is scoped to the state dir itself (gitignore semantics only govern that dir and below), a pre-existing .gitignore gets the entry appended rather than clobbered, and the write is best-effort so a guard failure never blocks the baseline. Covered by three new tests (fresh dir, append-without-clobber, already-covered by *).

…uide split

Devin Review on #112 flagged two gaps:

- SEC: DEVIN_DOCTOR_STATE_DIR can point inside a repo checkout and
  default_state_dir accepted it unchecked — git add -A could publish
  the baseline. _write_snapshot now drops/extends a .gitignore in the
  state dir covering automation-snapshot.json (scoped to that dir, so
  nothing outside is hidden; best-effort, never blocks the baseline).
- DOCS: the README automation row inlined per-OS state paths while the
  platform guides documented only the inspected Devin locations.
  Shared README now keeps the 'outside --data-dir' explanation +
  macOS path (no macOS guide exists); Linux/Windows/corporate guides
  document their own state dir under 'Devin paths'.

Changelog entries extended for the gitignore guard; 3 new tests cover
write/append/already-covered .gitignore cases.

Generated with [Devin](https://devin.ai) & QA Engineer Icaro Galvao

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@Icaro0310
Icaro0310 merged commit 43679c3 into main Oct 25, 2026
14 of 16 checks passed
@Icaro0310
Icaro0310 deleted the fix/doctor-xdg-snapshot branch October 25, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant