CTORNDGAIN-1321: Hooks runtime, md-file-advisory hook, plugin sync improvements#75
Conversation
- New hook: md-file-advisory.ts with tests and per-IDE bundles - Refactor build-bundles.mjs to iterate HOOK_SOURCES array instead of hardcoding a single entry - Register md-file-advisory.js in hooks.json configs for Claude, Codex, Copilot, Cursor - Document HOOK_SOURCES registration step in ARCHITECTURE.md - Add .codex to .gitignore Made-with: Cursor
Rosetta Triage ReviewSummary: Implements the Findings:
Suggestions:
Automated triage by Rosetta agent |
isolomatov-gd
left a comment
There was a problem hiding this comment.
hooks.json.tmpl is missing in core-copilot and in core-cursor.
md-file-advisory.js is missing in core-codex.
| rosetta-mcp-server/dist | ||
|
|
||
| #codex | ||
| .codex No newline at end of file |
There was a problem hiding this comment.
This will make all folders regardless where they are to be ignored. Technically this should be committed as well, likely this was used for testing, so I suggest deleting this change in .gitignore and deleting test .codex folder too.
| ]; | ||
|
|
||
| // Hook source files to bundle per plugin. | ||
| const HOOK_SOURCES = ['loose-files.ts', 'md-file-advisory.ts']; |
There was a problem hiding this comment.
I wonder if we list all *.ts files in hooks/src and use it here.
There was a problem hiding this comment.
No, this would be a bad idea, and here's why:
Not all .ts files in hooks/src are hook entry points. Looking at the directory, there are 16 .ts files there, but most are internal modules:
adapter.ts — shared adapter interface
types.ts, lock.ts, debug-log.ts — utility/support modules
entrypoints/adapter-.ts — per-IDE adapter implementations
adapters/.ts — adapter logic
| @@ -13,6 +13,10 @@ | |||
| { | |||
There was a problem hiding this comment.
'.codex' folder contains also 'hooks' folder there should be added the file with the hook
There was a problem hiding this comment.
Good catch, the file was missed from the commit. Will add it now.
| @@ -0,0 +1,230 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
In plugins/core-copilot/.github/plugin folder hooks.json and hooks.json.tmpl should be modified. We don't need to add the hook itself there
There was a problem hiding this comment.
Good point, will remove the hook file from .github/plugin/ and add the md-file-advisory entry to hooks.json and hooks.json.tmpl instead.
| @@ -0,0 +1,231 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
In plugins/core-copilot/hooks folder hooks.json and hooks.json.tmpl should be modified also
| @@ -0,0 +1,231 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
hooks.json in core-copilot folder should be also modified
| @@ -0,0 +1,201 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
In plugins/core-cursor/hooks folder hooks.json and hooks.json.tmpl should be modified as well
There was a problem hiding this comment.
According to the example with loose-files.js, I think hooks/dist/bundles should also contain hook files for every tool (core-claude, core-codex, core-copilot, etc.)
UPD: Turns out hooks/dist/bundles is in .gitignore, so it’s not relevant after all.
Made-with: Cursor
Remove md-file-advisory.js from .github/plugin/ (not needed there) and add the md-file-advisory hook entry to PostToolUse in both hooks.json and hooks.json.tmpl so the hook is properly invoked at runtime. Made-with: Cursor
Add md-file-advisory.js entry to PostToolUse in both hooks.json and hooks.json.tmpl under plugins/core-copilot/hooks/. Made-with: Cursor
Add md-file-advisory.js entry to postToolUse in both hooks.json and hooks.json.tmpl under plugins/core-cursor/hooks/. Made-with: Cursor
| import type { CanonicalOutput } from './types'; | ||
|
|
||
| export const ADVISORY_MESSAGE = | ||
| '[Rosetta Advisory] This Markdown file is outside standard Rosetta documentation locations ' + |
There was a problem hiding this comment.
[Rosetta Advisory] {file-name} is created in non-standard location, think if it is truly needed or you should have updated existing file.
There was a problem hiding this comment.
{file-name} to be replaced with file name without path.
| const normalized = normalize(raw); | ||
| const filePath = | ||
| (normalized.tool_input?.file_path as string) || | ||
| (normalized.tool_input?.path as string) || |
There was a problem hiding this comment.
This normalization belongs to common adapter. Moreover, you should also check for filePath (this is copilot standard)
| import { readStdin, normalize, formatOutput, detectIDE } from './adapter'; | ||
| import { debugLog } from './debug-log'; | ||
| import type { CanonicalOutput } from './types'; | ||
|
|
There was a problem hiding this comment.
We are missing filter by ALLOWED_TOOLS (see example in loose-files). So that we only react to creation of files. Otherwise there will be a lot of noise.
There was a problem hiding this comment.
Added but now we need review and make BIG re-test =)
Gate md-file-advisory on PostToolUse event and file-write tools only (Write, Edit, apply_patch, create_file, etc.) to prevent noise from Read, Bash, Search and other non-mutating tools. Extract getFilePath() helper with file_path/filePath/path priority chain and apply_patch command parsing. Add 29 new tests covering shouldCheck, getFilePath, and integration scenarios for filtered tools. Addresses PR #75 review comment from isolomatov-gd. Made-with: Cursor
Remove stale .codex ignore rule and trailing-newline fix in .gitignore. Add md-file-advisory.js hook entry to core-copilot/hooks.json PostToolUse. Made-with: Cursor
Replace static ADVISORY_MESSAGE constant with advisoryMessage(filePath)
that interpolates the file's basename into the message, per reviewer
suggestion: "[Rosetta Advisory] {name} is created in non-standard
location, think if it is truly needed or you should have updated
existing file."
Addresses PR #75 review comment from isolomatov-gd.
Made-with: Cursor
The reviewer (PR #75 comment #3) noted that filePath normalization logic was duplicated in each hook and belonged in the common adapter. Added extractFilePath() to adapter.ts, enriched normalize() to populate file_path on NormalizedInput, and removed duplicate getFilePath() from md-file-advisory.ts and loose-files.ts. This also fixes loose-files missing the `path` key fallback. Made-with: Cursor
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
Summary
CTORNDGAIN-1321
md-file-advisoryhook — advisory context injected on markdown file editsbuild-bundles.mjsto iterate aHOOK_SOURCESarray instead of hardcoding a single entry pointhooks.jsonconfigs across all IDE plugins (Claude, Codex, Copilot, Cursor)Test plan
node --testpasses for all hooks tests (adapter, loose-files, md-file-advisory, lock)build-bundles.mjsproduces bundles for all 5 IDEs with bothloose-files.jsandmd-file-advisory.jsscripts/pre_commit.pycopies built bundles into all plugin folders correctlyverify_mcp.pypasses for r2pytest ims-mcp-server/tests/)npm run testin rosettify/)