Narrow loose-files hook to file-creation events only#73
Conversation
Previously fired on Write|Edit (and Copilot replace_string_in_file, multi_replace_string_in_file, and Codex apply_patch Update File), causing noise and spawn overhead on every in-place edit. Now fires only on actual creation: Write/create_file across all IDEs, and apply_patch Add File/Create File for Codex. Two-layer fix: JSON matchers prevent the process from spawning on edit tools entirely; PATCH_FILE_RE tightened from Update|Add|Create to Add|Create so Codex update patches return early at the code level. Also fixes pre-existing mypy errors in analytics/tracker.py (str cast on header dict values that typed as Any). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rosetta Triage ReviewSummary: This PR narrows the Findings:
Suggestions:
Automated triage by Rosetta agent |
There was a problem hiding this comment.
Pull request overview
This PR narrows the loose-files PostToolUse hook so it only triggers on file-creation-oriented tool events (reducing per-edit overhead/noise), and it also addresses strict mypy typing issues in the analytics tracker.
Changes:
- Tighten per-IDE
hooks.jsonmatchers to avoid spawning theloose-fileshook process for edit-style tools. - Update
loose-fileslogic to treat Codexapply_patchas “creation-only” by matching only*** Add File/*** Create File. - Fix mypy complaints in
ims_mcp/analytics/tracker.pyby ensuringget_client_ip()returnsstr | NonewithoutAnyleakage.
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plugins/core-cursor/hooks/hooks.json.tmpl | Cursor hook matcher narrowed from `Write |
| plugins/core-cursor/hooks/hooks.json | Generated Cursor hooks config updated to match Write only. |
| plugins/core-cursor/.cursor/hooks/loose-files.js | Hook bundle updated: allowed tools reduced; patch regex matches only Add/Create. |
| plugins/core-cursor/.cursor/hooks.json | Runtime Cursor hooks config updated to match Write only. |
| plugins/core-copilot/hooks/loose-files.js | Hook bundle updated: allowed tools reduced; patch regex matches only Add/Create. |
| plugins/core-copilot/hooks/hooks.json.tmpl | Copilot hook matcher narrowed to `Write |
| plugins/core-copilot/hooks/hooks.json | Generated Copilot hooks config updated to `Write |
| plugins/core-copilot/hooks.json | Copilot runtime hooks config updated to `Write |
| plugins/core-copilot/.github/plugin/hooks.json.tmpl | Copilot plugin template matcher narrowed to `Write |
| plugins/core-copilot/.github/plugin/hooks.json | Generated Copilot plugin hooks updated to `Write |
| plugins/core-codex/.codex/hooks/loose-files.js | Hook bundle updated: allowed tools reduced; patch regex matches only Add/Create. |
| plugins/core-codex/.codex/hooks.json | Codex hook matcher narrowed from `Write |
| plugins/core-codex/.codex-plugin/hooks.json.tmpl | Codex plugin template matcher narrowed to exclude Edit. |
| plugins/core-codex/.codex-plugin/hooks.json | Generated Codex plugin hooks updated to exclude Edit. |
| plugins/core-claude/hooks/loose-files.js | Hook bundle updated: allowed tools reduced; patch regex matches only Add/Create. |
| plugins/core-claude/hooks/hooks.json.tmpl | Claude hook matcher narrowed from `Write |
| plugins/core-claude/hooks/hooks.json | Generated Claude hooks config updated to match Write only. |
| ims-mcp-server/ims_mcp/analytics/tracker.py | Ensure get_client_ip() returns str (not Any) to satisfy strict mypy. |
| hooks/tests/loose-files.test.ts | Update tests to assert no trigger on Edit / Update File patches; add Create File coverage. |
| hooks/src/loose-files.ts | Reduce allowed tools + restrict apply_patch parsing to Add/Create file directives. |
| hooks/package-lock.json | Lockfile updated (notably removing libc metadata from some optional native packages). |
| hooks/dist/src/loose-files.js | Built JS output updated to reflect loose-files.ts changes. |
Files not reviewed (1)
- hooks/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "arm64" | ||
| ], | ||
| "dev": true, | ||
| "libc": [ | ||
| "glibc" | ||
| ], | ||
| "license": "MIT", | ||
| "optional": true, | ||
| "os": [ |
There was a problem hiding this comment.
The libc metadata was removed from these Linux optional binary entries. For packages that ship separate *-gnu and *-musl variants, the libc constraint helps npm avoid treating both as installable on the same OS/CPU. Please confirm this lockfile change is intentional (and won’t cause both variants to be installed / extra install overhead on Linux), or regenerate the lockfile with the repo-standard npm so the libc constraints are preserved.
…te tests accordingly
…te IDE detection logic
Plan A.1 (Copilot CLI runtime trace) was not executed by either side, so the team lead's d2a94c6 ("Copilot plugin works too") is treated as authoritative. The shape inversion in 82fb05e/c612baf is reverted: before: out.additionalContext = additionalContext (flat) after: out.hookSpecificOutput = { hookEventName, additionalContext } (nested) Surgical revert — only the shape line in formatOutput and corresponding test assertions. Dual-IDE routing in adapter-copilot.ts, Cursor test expansion, claude-plugin-root smoke test, and file-creation narrowing (49017c5) are all preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…LE_RE into shouldCheck
…ypo in build-bundles.mjs
…event/toolKind (optional until Task 7)
…ove extractFilePath export
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ctivation Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… slim hook bodies Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…olation Each adapter now imports only its IDE's row file instead of the monolithic ide-registry. Fixed esbuild alias filter (./adapter → ../adapter path) so slim per-IDE entrypoints are actually swapped in during bundling. Added bundle isolation regression tests confirming zero foreign-IDE adapter code in each plugin bundle. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…latform dedup now in adapter
… in loose-files and md-file-advisory hooks
| @@ -0,0 +1 @@ | |||
| {"version":"4.1.5","results":[[":hooks/tests/loose-files.test.ts",{"duration":14.917750000000012,"failed":false}],[":hooks/tests/md-file-advisory.test.ts",{"duration":8.606415999999996,"failed":false}],[":hooks/tests/runtime/path-utils.test.ts",{"duration":4.983292000000006,"failed":false}],[":hooks/tests/gitnexus-refresh.test.ts",{"duration":12.985250000000036,"failed":false}],[":rosettify/tests/unit/shared/core-utils.test.ts",{"duration":7.218792000000008,"failed":false}],[":hooks/dist/tests/loose-files.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/md-file-advisory.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/gitnexus-refresh.test.js",{"duration":0,"failed":true}],[":rosettify/tests/e2e/mcp.e2e.test.ts",{"duration":6.509999999999991,"failed":true}],[":rosettify/tests/e2e/cli.e2e.test.ts",{"duration":5.0058329999999955,"failed":true}],[":rosettify/tests/unit/plan/next.test.ts",{"duration":0,"failed":true}],[":hooks/dist/tests/runtime/run-hook.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/adapter.cursor.test.js",{"duration":0,"failed":true}],[":hooks/tests/runtime/run-hook.test.ts",{"duration":6.732875000000007,"failed":false}],[":hooks/tests/adapter.cursor.test.ts",{"duration":5.088583,"failed":false}],[":hooks/dist/tests/adapter.claude-code.test.js",{"duration":0,"failed":true}],[":rosettify/tests/unit/plan/upsert.test.ts",{"duration":0,"failed":true}],[":rosettify/tests/unit/plan/show-status.test.ts",{"duration":0,"failed":true}],[":hooks/dist/tests/adapter.windsurf.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/adapter.copilot.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/adapter.test.js",{"duration":0,"failed":true}],[":hooks/tests/adapter.windsurf.test.ts",{"duration":4.723666000000009,"failed":false}],[":rosettify/tests/unit/plan/create.test.ts",{"duration":0,"failed":true}],[":rosettify/tests/unit/plan/index.test.ts",{"duration":0,"failed":true}],[":hooks/tests/adapter.claude-code.test.ts",{"duration":7.725583999999998,"failed":false}],[":rosettify/tests/unit/shared/dispatch.test.ts",{"duration":0,"failed":true}],[":hooks/tests/adapter.copilot.test.ts",{"duration":3.921166999999997,"failed":false}],[":hooks/dist/tests/claude-plugin-root.test.js",{"duration":0,"failed":true}],[":rosettify/tests/unit/shared/envelope.test.ts",{"duration":4.195624999999993,"failed":false}],[":hooks/dist/tests/adapter.codex.test.js",{"duration":0,"failed":true}],[":rosettify/tests/unit/shared/concurrency.test.ts",{"duration":5.990791999999999,"failed":false}],[":hooks/tests/adapter.test.ts",{"duration":4.10691700000001,"failed":false}],[":hooks/dist/tests/lock.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/runtime/ide-registry.test.js",{"duration":0,"failed":true}],[":hooks/tests/claude-plugin-root.test.ts",{"duration":117.69900000000001,"failed":false}],[":hooks/tests/adapter.codex.test.ts",{"duration":3.417334000000011,"failed":false}],[":rosettify/tests/unit/plan/query.test.ts",{"duration":0,"failed":true}],[":hooks/dist/tests/runtime/path-utils.test.js",{"duration":0,"failed":true}],[":rosettify/tests/unit/plan/update-status.test.ts",{"duration":0,"failed":true}],[":hooks/dist/tests/regression/hooks-registered.test.js",{"duration":0,"failed":true}],[":rosettify/tests/unit/help/help.test.ts",{"duration":0,"failed":true}],[":hooks/tests/runtime/ide-registry.test.ts",{"duration":4.750916000000004,"failed":false}],[":hooks/dist/tests/runtime/throttle.test.js",{"duration":0,"failed":true}],[":hooks/tests/regression/hooks-registered.test.ts",{"duration":3.1682500000000005,"failed":false}],[":rosettify/tests/unit/registry/types.test.ts",{"duration":1.602249999999998,"failed":false}],[":rosettify/tests/unit/shared/registry.test.ts",{"duration":0,"failed":true}],[":hooks/tests/runtime/throttle.test.ts",{"duration":3.049749999999989,"failed":false}],[":hooks/dist/tests/runtime/result-helpers.test.js",{"duration":0,"failed":true}],[":hooks/tests/runtime/result-helpers.test.ts",{"duration":1.6190829999999892,"failed":false}],[":hooks/dist/tests/runtime/ide-rows.test.js",{"duration":0,"failed":true}],[":hooks/dist/tests/regression/bundle-isolation.test.js",{"duration":0,"failed":true}],[":hooks/tests/regression/bundle-isolation.test.ts",{"duration":8.654750000000007,"failed":false}],[":hooks/tests/runtime/ide-rows.test.ts",{"duration":3.311292000000009,"failed":false}]]} No newline at end of file | |||
There was a problem hiding this comment.
I feel like this folder should be ignored or something like that
Brings in hooks runtime adapter, md-file-advisory hook, r3 instruction set, plugin generator improvements, and analytics changes from PR #75. Conflict resolution: - agents/MEMORY.md: both sides' entries preserved - analytics/tracker.py: mypy fixes (PR #73) + analytics changes (PR #75) both kept - plugins/*/hooks.json: regenerated via build-bundles.mjs; both md-file-advisory (PR #75) and loose-files creation-only matchers (PR #73) present
Per PR #73 review feedback. Adds coverage/ to rosettify/.gitignore and removes the previously committed HTML/JSON report from git tracking. Generation is unchanged — npm run test:coverage still works and outputs to the now-ignored rosettify/coverage/ directory.
…heck Replaces timestamp-age comparison (Date.now() - stamp < DEBOUNCE_MS) with token-identity check (current !== token). Each spawn writes a unique token; deferred process exits early if a newer spawn replaced the token, eliminating the race window in the timestamp approach. Syncs debounce strategy with the fix landed in v3 via PR #76.
…y debounce refactor
…n-only Brings in gitnexus-refresh hook (PR #76), token-identity debounce improvement, and matcher fixes from origin/v3. Conflict resolution: - hooks/scripts/build-bundles.mjs: kept ours (auto-discover from src/hooks/) - hooks/src/hooks/gitnexus-refresh.ts: kept ours (defineHook/runHook architecture + token-identity debounce already applied) - hooks/tests/gitnexus-refresh.test.ts: kept ours (runHook-based tests) - plugins/*/hooks/gitnexus-refresh.js: kept ours (bundle artefacts, rebuilt from source) - hooks.json/tmpl per-plugin: auto-merged — loose-files retains creation-only matcher; gitnexus-refresh uses v3 broader matcher
…dex gitnexus matcher - Add node_modules/.vite/ to .gitignore so vitest results cache is never tracked again (addresses Igor's review comment on PR #73) - git rm --cached node_modules/.vite/vitest/.../results.json (the leaked artefact committed in the previous session) - Update plugins/core-codex/.codex/hooks.json gitnexus-refresh matcher from Write|Edit to Write|Edit|apply_patch|functions.apply_patch so gitnexus re-indexes on Codex apply_patch edits too (picked up by plugin sync from the merged .codex-plugin/hooks.json.tmpl) Note: hooks/package-lock.json libc fields are absent — npm on macOS does not emit libc constraints for Linux optional packages; risk is minimal.
Summary
loose-fileshook previously fired on every file-touching tool (Write|Edit, Copilotreplace_string_in_file/multi_replace_string_in_file, Codexapply_patchUpdate), causing context noise and spawn overhead on every edit turnWrite/create_filefor Claude/Cursor/Copilot,apply_patch Add File/Create Filefor CodexPATCH_FILE_REinloose-files.ts(Update|Add|Create→Add|Createso Codex update patches return early in code)analytics/tracker.pythat were blocking the pre-commit hookTest plan
npm testinhooks/)hooks.jsonmatchers verified: Claude=Write, Cursor=Write, Copilot=Write|create_file, Codex=Write|apply_patch|functions.apply_patchapply_patch Update File→ no output; Codexapply_patch Add File→ nudgescripts/pre_commit.pypasses end-to-end (type validation + hooks tests)🤖 Generated with Claude Code