Skip to content
This repository was archived by the owner on Jul 30, 2026. It is now read-only.

feat(cursor): preToolUse enforcement — block non-compliant writes - #60

Merged
giancolombi merged 2 commits into
mainfrom
feat/cursor-pretooluse-enforce
Jun 9, 2026
Merged

feat(cursor): preToolUse enforcement — block non-compliant writes#60
giancolombi merged 2 commits into
mainfrom
feat/cursor-pretooluse-enforce

Conversation

@giancolombi

Copy link
Copy Markdown
Contributor

Why

The Cursor integration registered a postToolUse hook. Per Cursor's hook spec, post-write hooks (postToolUse/afterFileEdit) can only inject additional_context — they cannot force a revision. So the agent received the remediation but, having finished its turn, often didn't act on it. "Block non-compliant edits as you code" was really "surface and hope." (Observed live: the hook flagged a hardcoded key correctly, but the file stayed.)

What

prodcycle init --agent cursor now registers a preToolUse hook by default (--cursor-mode enforce). prodcycle hook detects the event and on violations returns:

{ "permission": "deny", "agent_message": "<remediation>", "user_message": "ProdCycle blocked this edit: N compliance violation(s)…" }

Cursor rejects the write before it lands and feeds agent_message to the agent, which revises and retries — the true analog of Claude Code's {"decision":"block","reason":…}.

  • --cursor-mode warn keeps the old non-blocking postToolUse / additional_context behavior.
  • Legacy migration: existing postToolUse / afterFileEdit prodcycle hooks are migrated to the chosen event automatically.
  • Fail-open: missing key, 401/403, and scanner-unavailable (exitCode 2) never emit deny — they allow the edit, so we never block on our own error.
  • Corrected the overstated "self-healing loop" wording on the postToolUse formatter.

Verified

preToolUse + violation → {"permission":"deny", agent_message, user_message}
preToolUse + clean     → "" (allow)
postToolUse + violation → {"additional_context":…}   (unchanged)
init (default)         → preToolUse ;  --cursor-mode warn → postToolUse ;  re-init migrates

node + python in lockstep; full suites green (124 node / 112 python), incl. new formatter/detector tests + updated init/collector tests.

🤖 Generated with Claude Code

The Cursor integration registered a `postToolUse` hook, which (per Cursor's
spec) can only inject `additional_context` AFTER the write — it cannot force a
revision. So "block non-compliant edits" was really "surface and hope."

`init --agent cursor` now registers a **`preToolUse`** hook by default
(`--cursor-mode enforce`): `prodcycle hook` detects the event and returns
`{"permission":"deny","agent_message":<remediation>}`, so Cursor rejects the
non-compliant write before it lands and the agent revises and retries — the true
analog of Claude Code's `{"decision":"block","reason":...}`.

- `--cursor-mode warn` keeps the non-blocking postToolUse/additional_context path.
- Legacy postToolUse/afterFileEdit prodcycle hooks migrate automatically.
- Fail-open: missing key / 401-403 / scanner-unavailable (exitCode 2) never emit
  deny — they allow the edit, so we never block on our own error.
- node + python kept in lockstep; corrected the overstated "self-healing" wording
  on the postToolUse formatter. +tests both languages (suites green 124 / 112).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR upgrades the Cursor integration from a non-blocking postToolUse hook to a blocking preToolUse hook by default (--cursor-mode enforce), so violations return {"permission":"deny","agent_message":...} and Cursor rejects the write before it lands. The --cursor-mode warn flag preserves the old surface-only behavior, and legacy postToolUse/afterFileEdit hooks are migrated automatically on re-init.

  • Fail-open is correctly implemented for exitCode === 2 (scanner unavailable) and 401/403 auth errors on both Node and Python sides — neither path can block an edit on its own error.
  • Migration logic removes the conflicting hook from the other event and afterFileEdit atomically, preventing double-registration.
  • Tests cover the new formatter, detector, and init paths in both languages; all tuple unpackings updated for the new 5-element return from _collect_hook_files.

Confidence Score: 5/5

Safe to merge — the fail-open invariants are enforced at every exit path, and the two implementation nits are both in non-critical code paths.

The core blocking logic and all fail-open paths (scanner unavailable, auth error, clean scan, non-Write tool) are handled correctly and tested in both Node and Python. The only gaps are a missing input-validation error for an unrecognized --cursor-mode value in the Node CLI, and the exported formatCursorPreToolUseOutput function not internally guarding against exitCode === 2 — neither of which can cause incorrect blocking behavior in any current code path.

node/src/cli.ts — --cursor-mode option validation and the formatCursorPreToolUseOutput defensive guard.

Important Files Changed

Filename Overview
node/src/cli.ts Core logic for preToolUse enforcement: new formatter, detector, migration logic, and fail-open paths all look correct. Minor: Node --cursor-mode option lacks input validation that the Python side enforces.
python/src/prodcycle/cli.py Python mirror of preToolUse enforcement. Logic is consistent with Node; fail-open for exitCode=2 handled correctly at call sites. Same latent issue: _format_cursor_pretooluse_output would emit deny for exitCode=2 if called directly.
node/test/cursor-hook-output.test.mjs New tests for isCursorPreToolUseHookPayload and formatCursorPreToolUseOutput; covers clean pass, violation deny, and structural correctness.
node/test/cursor-init.test.mjs Updated to verify preToolUse hook placement and legacy afterFileEdit removal; accurate for the new default.
python/tests/test_cursor_hook_output.py New tests for _is_cursor_pre_tool_use_hook_payload and _format_cursor_pretooluse_output; mirrors Node test coverage.
python/tests/test_cursor_init.py Updated to check preToolUse hook entry instead of postToolUse; straightforward migration of the assertion.
CHANGELOG.md Accurate changelog entry describing the new preToolUse enforcement mode and migration behavior.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
node/src/cli.ts:229-231
**`--cursor-mode` silently ignores invalid values**

The Node option accepts any string and coerces it to `'enforce'` if it isn't `'warn'`. A user who typos `--cursor-mode enforced` or passes an unsupported value gets no error — they silently end up in enforce mode, which happens to be the right default, but the discrepancy from the Python side (which uses `choices=['enforce', 'warn']` and rejects unknown values with an argparse error) is confusing. Consider adding explicit validation, e.g. after parsing: `if (opts.cursorMode && !['enforce','warn'].includes(opts.cursorMode)) program.error(...)`.

### Issue 2 of 2
node/src/cli.ts:687-710
**`formatCursorPreToolUseOutput` emits `deny` when called with `exitCode === 2`**

The function only guards `exitCode === 0` → allow, and falls through to emit `{"permission":"deny",...}` for any other exit code, including `exitCode === 2` (scanner unavailable). All current callers correctly intercept `exitCode === 2` before calling this function, but since it is an exported public symbol, a future caller (or a test) that passes a scanner-unavailable response directly would get a deny instead of an allow — violating the fail-open contract. A defensive guard for `exitCode === 2` inside the function would make the invariant self-enforcing. The Python mirror has the same gap.

Reviews (2): Last reviewed commit: "style(test): add blank line between test..." | Re-trigger Greptile

Comment thread python/tests/test_cursor_hook_output.py
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@giancolombi
giancolombi merged commit 49ab696 into main Jun 9, 2026
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants