Skip to content

Optional: hooks-deprecation only scans hand-written .amplifier config - #413

Open
Michael J. Jabbour (michaeljabbour) wants to merge 1 commit into
microsoft:mainfrom
michaeljabbour:fix/deprecation-scan-allowlist
Open

Michael J. Jabbour (michaeljabbour) wants to merge 1 commit into
microsoft:mainfrom
michaeljabbour:fix/deprecation-scan-allowlist

Conversation

@michaeljabbour

Copy link
Copy Markdown

Hi Brian,

An optional suggestion. Take it or leave it, and feel free to close it if it isn't the direction you want.

What I noticed

  • Every time a session starts (and every time a helper agent starts), the retired-bundle check reads every YAML file anywhere under ~/.amplifier.
  • On a well-used machine that folder is mostly saved sessions, caches and other bundles' data. On mine it was about 36,000 folders, and the check took about 6–7 seconds each time.
  • It also gave a false alarm. A team knowledge base mentions hooks-redaction in its notes, so the check decided I was still using it and logged a warning in every session (468 of 468 over two days).

What this change does

  • It only looks where people actually write settings: the YAML files at the top of .amplifier/, and the hand-written bundles under .amplifier/bundles/ (up to 4 folders deep, skipping .git and cache).
  • Everything else is ignored.
  • Only find_source_files changed. When to warn, how serious it is, and the message itself all stay the same.

What I saw on my machine (macOS, Python 3.12)

  • The check went from about 6–7 seconds to about 0.002 seconds.
  • Files wrongly flagged as "evidence" went from 15 to 0.

Trade-offs worth knowing

  • A settings file somewhere unusual, like a hand-made .amplifier/profiles/ folder or a bundle more than 4 folders deep, would no longer count as evidence. Since feat(hooks-deprecation): multi-tombstone + on_session_ready firing + observability event #277, a retired bundle that is actually loaded still triggers the warning on its own, so this only affects the backup file check.
  • Smaller changes:
    • Results are de-duplicated and sorted.
    • A symlinked .amplifier/bundles folder is now read.
    • A file that links to itself is skipped safely.
  • Known gaps, not changed here:
    • A copy of a bundle placed under bundles/ can still flag itself.
    • .yml files and bundle.md files still aren't read.
  • The only bundle using this check (hooks-redaction) retires on 2026-10-01. After that, this mostly matters for future retirements.

Tests

  • The 72 existing tests pass unchanged, plus 6 new ones, on Python 3.11, 3.12 and 3.13.
  • The module's tests don't run in CI (ci.yml runs pytest tests/), so here is the local command:
    PYTHONPATH=modules/hooks-deprecation python -m pytest -q modules/hooks-deprecation/tests
  • Side note: on main, a few of those tests read the real home folder, so the suite took 42 seconds on my machine. With this change it takes 0.1 seconds.

Two open questions for you

  1. Is the file check still worth having now that feat(hooks-deprecation): multi-tombstone + on_session_ready firing + observability event #277 can tell whether a bundle is actually loaded, or could it read only settings*.yaml?
  2. Separately: on amplifier-core 2.0.1, the warning text this hook returns at session:start never reached the model in my sessions. That might deserve its own issue.

Thanks,
Michael

🤖 Generated with Claude Code

…ence

find_source_files walked every *.yaml under ~/.amplifier, including tool-managed
state (module cache, session storage, bundle data stores). On a heavily used
machine that took ~6-7 s per session start, blocking the event loop, and matched
bundle names that appear only as data, producing false evidence.

Scan only top-level .amplifier/*.yaml and .amplifier/bundles/** (depth 4,
skipping .git and cache). De-duplicate by real path (os.path.realpath, which
does not raise on symlink loops before Python 3.13).

Generated with Amplifier

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant