Skip to content

feat(cursor): upgrade hooks to Cursor 2.4 postToolUse for Read/Grep/Shell coverage#1467

Merged
magyargergo merged 6 commits into
mainfrom
feat/cursor-2.4-hooks-1466
May 10, 2026
Merged

feat(cursor): upgrade hooks to Cursor 2.4 postToolUse for Read/Grep/Shell coverage#1467
magyargergo merged 6 commits into
mainfrom
feat/cursor-2.4-hooks-1466

Conversation

@abhigyanpatwari

@abhigyanpatwari abhigyanpatwari commented May 9, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #1466.

Cursor 2.4 (released 2026-01-22) shipped generic preToolUse/postToolUse hooks with matchers Shell|Read|Write|Grep|Delete|Task|MCP:<tool>. The current gitnexus-cursor-integration/hooks/hooks.json still uses the 2.3-era beforeShellExecution, so graph augmentation only fires when the agent runs rg/grep via shell — Read and Grep (the agent's far more common tools) get no graph context.

This PR brings Cursor parity (and slight superset) with the Claude Code hook coverage:

  • gitnexus-cursor-integration/hooks/hooks.json — single postToolUse entry matching Shell|Read|Grep, invoking the new Node script.
  • gitnexus-cursor-integration/hooks/gitnexus-hook.cjs (new) — cross-platform Node hook ported from the bash + jq script. Mirrors safety patterns from gitnexus/hooks/claude/gitnexus-hook.cjs:
    • path.isAbsolute(cwd) validation
    • .gitnexus/ discovery with linked-worktree fallback (git rev-parse --git-common-dir)
    • Global-registry exclusion (isGlobalRegistryDir heuristic)
    • npx.cmd on Windows, -- end-of-options marker (fix(hooks): pass augment pattern after end-of-options marker #200), .slice(0, 200) debug truncation
    • Graceful failure (status === 0, no broken pipe to Cursor)
    • Per-tool pattern extraction:
      • GreptoolInput.query (also pattern / regex aliases)
      • Read → file basename stripped to identifier chars (≥ 3 chars)
      • Shell → existing rg/grep arg parser
    • Emits { "additional_context": "..." } (Cursor 2.4 shape), not Claude's nested hookSpecificOutput.
  • gitnexus-cursor-integration/hooks/augment-shell.sh (deleted) — bash + jq pipeline doesn't run on Windows and only covered Shell.
  • gitnexus/test/unit/cursor-hook.test.ts (new) — 33 regression tests. Coverage:
    • File presence + legacy augment-shell.sh removal
    • hooks.json wiring (matcher, command, version, no leftover beforeShellExecution)
    • Source-level invariants: no shell: true, npx.cmd, path.isAbsolute(cwd), .slice(0, 200), additional_context (not hookSpecificOutput), pattern length ≥ 3, end-of-options marker, .gitnexus gating, linked-worktree resolution
    • extractPattern coverage per tool (Grep / Read / Shell)
    • Behavioral early-exit tests: empty stdin, invalid JSON, relative cwd, unindexed cwd, unknown tool name, non-search Shell commands, 2-char query, basename with no identifier chars, missing file path, case-insensitive tool name matching
    • Output parser sanity (well-formed JSON, empty stdout, malformed JSON)
  • README.md / gitnexus/README.md — editor-support table now reads **Cursor** | Yes | Yes | Yes (postToolUse) | **Full** instead of — | MCP + Skills.
  • gitnexus/src/cli/augment.ts + gitnexus/src/core/augmentation/engine.ts — doc-string Cursor beforeShellExecutionCursor postToolUse; engine doc tool list expanded to grep/glob/read/search.

Design notes

  • postToolUse only, not also preToolUse. Augmentation is observational (the agent sees enriched context after the tool ran), not gating, so the second matcher is dead weight.
  • Read pattern = file basename (stripped to identifier chars), not the raw file path. Engine uses BM25 over symbol names, so the basename is a higher-signal token than src/auth/handler.ts.
  • codebase_search left unhandled. Cursor's docs (https://cursor.com/docs/agent/hooks) don't list it as a matchable tool name. Per the issue's open question, semantic-search miss is acceptable — Read/Grep/Shell already cover the dominant paths.

Risk

Low. Change is isolated to gitnexus-cursor-integration/ plus four doc-string / README cells. The augment CLI and core/augmentation/engine.ts API are unchanged — neither cares which harness called them. Cursor users on 2.3 will silently lose hook augmentation; users on 2.4+ gain Read/Grep coverage on top of the existing Shell path.

Test plan

  • cd gitnexus && npx vitest run test/unit/cursor-hook.test.ts — 33 / 33 pass in ~0.6s
  • cd gitnexus && npx vitest run test/unit/hooks.test.ts test/unit/cursor-hook.test.ts — 120 / 120 pass (new tests + existing Claude hook tests)
  • cd gitnexus && npx tsc --noEmit — 0 errors
  • cd gitnexus && npx eslint test/unit/cursor-hook.test.ts ../gitnexus-cursor-integration/hooks/gitnexus-hook.cjs src/cli/augment.ts src/core/augmentation/engine.ts — 0 issues on touched code
  • cd gitnexus && npx prettier --check ../gitnexus-cursor-integration/hooks/{hooks.json,gitnexus-hook.cjs} test/unit/cursor-hook.test.ts src/cli/augment.ts src/core/augmentation/engine.ts — clean
  • Manual smoke: piped synthetic JSON for Grep / Read / Shell rg / Shell ls events through the new hook — exits 0, no spurious output for non-search calls
  • Pre-commit hook (lint-staged + typecheck) passes locally

Out of scope

  • gitnexus-cursor-integration/ README — file doesn't exist; Cursor users currently copy hooks/+skills/ directly. If install docs land later, they should reference gitnexus-hook.cjs not augment-shell.sh.
  • setup.ts — does not currently install Cursor hooks (only MCP config + skills); unchanged here.

🤖 Generated with Claude Code


Note

Low Risk
Low risk: changes are isolated to Cursor integration wiring/docs plus a new hook script and regression tests; main risk is hook behavior differences across Cursor versions or tool-input field variations causing missed augmentation.

Overview
Upgrades the Cursor integration from the legacy beforeShellExecution bash hook to a Cursor 2.4+ postToolUse hook that augments Shell/Read/Grep tool results with GitNexus graph context.

Adds a cross-platform Node hook (gitnexus-hook.cjs) with safer pattern extraction, .gitnexus/worktree detection, and Windows npx.cmd handling, deletes the old augment-shell.sh, and introduces unit tests to lock in the hook/manifest contract.

Updates the top-level docs and gitnexus/README.md to mark Cursor hooks as full support with manual install instructions, and adjusts related docstrings in augment.ts and the augmentation engine.

Reviewed by Cursor Bugbot for commit 204a754. Bugbot is set up for automated code reviews on this repo. Configure here.

…hell coverage

Cursor 2.4 (released 2026-01-22) shipped generic preToolUse/postToolUse hooks
matching `Shell|Read|Write|Grep|Delete|Task|MCP:<tool>`, replacing the
2.3-era beforeShellExecution hook that only fired on shell commands. The
existing integration only intercepted the shell path, so Cursor users got
graph augmentation roughly 10% as often as Claude Code users — only when
the agent dropped to rg/grep instead of using its native Read/Grep tools.

This swaps the integration over to postToolUse and ports the bash+jq
hook script to cross-platform Node:

- gitnexus-cursor-integration/hooks/hooks.json: registers a single
  postToolUse hook matching Shell|Read|Grep that invokes the new
  gitnexus-hook.cjs.
- gitnexus-cursor-integration/hooks/gitnexus-hook.cjs: new Node hook
  mirroring the safety patterns from the Claude hook (absolute-cwd
  validation, .gitnexus discovery with linked-worktree fallback,
  npx.cmd on Windows, end-of-options `--` marker, debug truncation,
  graceful failure). Extracts the search pattern per tool kind:
  Grep -> toolInput.query; Read -> file basename stripped to identifier
  chars; Shell -> existing rg/grep arg parser. Emits Cursor-shape
  `{ "additional_context": "..." }` on stdout — no shell, no jq.
- gitnexus-cursor-integration/hooks/augment-shell.sh: removed (Windows
  incompatible, narrower coverage).
- gitnexus/test/unit/cursor-hook.test.ts: 33 regression tests covering
  manifest wiring, source-level invariants (no shell:true, npx.cmd,
  isAbsolute, additional_context output shape, end-of-options marker),
  extractPattern coverage per tool, and behavioral early-exit paths
  (empty/invalid stdin, relative cwd, no .gitnexus, unknown tool name,
  short patterns, non-search shell commands, case-insensitive matching).
- README.md / gitnexus/README.md: editor-support table now lists Cursor
  as Full / hooks=Yes (postToolUse), matching reality.
- gitnexus/src/cli/augment.ts and gitnexus/src/core/augmentation/engine.ts:
  doc-strings updated from `Cursor beforeShellExecution` to
  `Cursor postToolUse`.

Closes #1466.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitnexus Ready Ready Preview, Comment May 10, 2026 11:47am

Request Review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 00e1edc. Configure here.

Comment thread gitnexus-cursor-integration/hooks/hooks.json Outdated
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
8549 8548 0 1 392s

✅ All 8548 tests passed

1 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 77.91% 25463/32679 N/A% 🟢 ███████████████░░░░░
Branches 66.38% 16077/24219 N/A% 🟢 █████████████░░░░░░░
Functions 83.13% 2553/3071 N/A% 🟢 ████████████████░░░░
Lines 80.95% 23016/28431 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

Repository owner deleted a comment from cursor Bot May 10, 2026
@github-actions

github-actions Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 5m 31s —— View job


PR #1467 Review — Cursor 2.4 postToolUse Hook Migration

Todo List

  • Post initial comment
  • Read repo context files (DoD.md, GUARDRAILS.md, AGENTS.md)
  • Read all changed files
  • Read related hook files (Claude hook for comparison)
  • Check CI status and test results
  • Run hidden Unicode scan on changed files
  • Analyze hooks.json schema correctness (timeout units, matcher, command path)
  • Analyze gitnexus-hook.cjs (payload parsing, runtime safety, worktree discovery, pattern extraction)
  • Analyze test coverage (cursor-hook.test.ts)
  • Assess branch hygiene
  • Assess docs truthfulness
  • Write comprehensive review

Review bar for this PR

  • hooks.json uses the correct Cursor hook schema, phase, matcher, timeout unit, and command path
  • The hook emits { "additional_context": "..." } (not Claude's hookSpecificOutput) and reads from the correct subprocess output stream
  • Pattern extraction fields (tool_name, tool_input, cwd, query) match Cursor's actual postToolUse payload — confirmed by real fixtures, not just assumption
  • The hook does not block Cursor: Cursor-level timeout is in safe seconds-scale units; internal spawnSync timeouts cover subprocess hangs
  • .gitnexus gating runs before any CLI invocation; linked worktrees resolve correctly without crossing repo indexes
  • No shell: true, patterns pass after --, user-controlled input cannot become CLI flags or shell commands
  • CI is green on latest head; tests would fail before this PR

Current PR state

Attribute Value
Head SHA 00e1edc
Status Open
Commits 1
Changed files 8
Net diff +555 / -59
CI 1 test failure (8306/8308 pass)
Typecheck ✅ pass
Cursor Bugbot ❌ unresolved — "timeout": 10000 flagged
Gitleaks ✅ pass
Dependency Review ✅ pass
CodeQL ✅ pass
Hidden bidi Unicode ✅ none found
Non-ASCII ⚠️ em dashes in comments/docs only — not in executable code or JSON

The 1 CI test failure appears pre-existing (Ruby block parameter closure inference, a documented skip category) but the CI workflow status still shows ❌ which blocks merge per DoD.


Branch hygiene assessment

Branch hygiene verdict: clean feature/fix PR

One commit (00e1edc). All 8 changed files are: the new hook script, the updated manifest, the deleted shell script, the new test file, two doc comment updates, and two README one-liners. No lockfile churn, no package.json changes, no workflow changes, no unrelated core changes. This is a focused Cursor hook migration.


Understanding of the change

The legacy beforeShellExecution hook with augment-shell.sh only intercepted shell rg/grep invocations. Cursor 2.4's native Read and Grep tools — which the agent uses far more frequently — routed through no hook at all, so the knowledge graph was never consulted during file reads or symbol searches. This PR moves to a single postToolUse node script that covers all three paths. postToolUse is the right phase for observational augmentation: the tool has already run and returned its result to the agent; the hook adds graph context alongside it without blocking or gating the tool call. Node.js replaces bash+jq for Windows compatibility. The hook no-ops on unindexed repos (no .gitnexus) and on linked worktrees it falls back to git rev-parse --git-common-dir to find the canonical root where .gitnexus lives. codebase_search is excluded because Cursor 2.4 docs do not list it as a matchable tool name. Cursor 2.3 users silently lose augmentation — no backward compatibility is maintained.


Findings


[BLOCKER] "timeout": 10000 is almost certainly in seconds — making it ~2.8 hours

  • Category: Hook timeout / hang-safety
  • Files: gitnexus-cursor-integration/hooks/hooks.json:8
  • Issue: Cursor's hooks.json timeout field is specified in seconds per Cursor's documentation. The value 10000 would therefore configure a Cursor-level timeout of ~2.8 hours, not 10 seconds. The legacy hook used "timeout": 5 (5 seconds). Cursor doc examples use "timeout": 30. The Cursor Bugbot flagged this as medium severity. The internal spawnSync calls use 7000ms and 12000ms, which protect against subprocess hangs — but do not protect against hangs before those calls are reached (e.g., a hung stdin read or an exception between readInput() and the spawnSync call). In that window, Cursor would wait 10000 seconds before killing the hook.
  • Why it matters: A single hung hook invocation would freeze Cursor's agent loop for ~2.8 hours. Even if it never hangs in practice today, the wrong unit is a latent reliability time bomb.
  • Recommended fix: Change "timeout": 10000"timeout": 10 (or whatever seconds-appropriate value is intended). Update the test at cursor-hook.test.ts:128-132 to assert timeout <= 60 so this regression is caught. Fix this →
  • Blocks merge: Yes

[BLOCKER] Timeout test does not catch the unit error

  • Category: Test coverage
  • Files: gitnexus/test/unit/cursor-hook.test.ts:128-132
  • Issue: The test asserts only typeof timeout === 'number' and timeout > 0. This passes with 10000. It would not fail before or after this PR if the value were changed from 5 seconds to 10000 seconds.
  • Why it matters: A regression test that cannot catch the documented regression is not a regression test. The DoD requires tests that "would fail if behavior, wiring, or contracts were broken."
  • Recommended fix: Assert timeout <= 60 (or a tighter bound reflecting the intended value) so that millisecond-scale values are caught. Fix this →
  • Blocks merge: Yes (because without this, the fix for finding 1 is unverified)

[BLOCKER] CI is failing

  • Category: CI gate
  • Files: N/A
  • Issue: The main CI run (25614045027) reports ❌ with 1 test failure. The PR body claims 120/120 pass locally but CI disagrees. The failing test appears to be in buildTypeEnv > known limitations (documented skip tests) (Ruby closure inference) which may be pre-existing and unrelated to this PR. However, per DoD §6 "Not Done Signals": even if the failure is pre-existing, a green CI is required before merge — and the current state is not green.
  • Why it matters: The DoD explicitly requires CI to be green. A failing CI gate hides whether this PR's own tests are green in all CI environments.
  • Recommended fix: Confirm the failure is pre-existing and unrelated to this PR (check if main has the same failure). If it is pre-existing, this should be tracked separately and the CI gate fixed before merging any PR. If it is PR-caused, fix it.
  • Blocks merge: Yes (CI gate)

[SIGNIFICANT] Cursor payload field names are unconfirmed guesses — Grep query field is the dominant risk

  • Category: Cursor event payload contract
  • Files: gitnexus-cursor-integration/hooks/gitnexus-hook.cjs:124-146
  • Issue: The hook assumes Cursor's postToolUse payload uses tool_name, tool_input, cwd at the top level, and that Cursor's Grep tool uses a query field (with pattern and regex as fallbacks). These field names are reasonable assumptions based on Cursor's MCP-style naming conventions, but the PR contains no real Cursor 2.4 payload fixtures. The Claude Code hook (gitnexus/hooks/claude/gitnexus-hook.cjs:109) uses toolInput.pattern for its Grep tool. If Cursor's Grep uses pattern (not query) as the primary field, the hook will extract the right value — but only via fallback. If Cursor uses neither query nor pattern, the dominant Grep path silently no-ops for all augmentation.
  • Why it matters: This is the primary new capability being advertised. If Grep extraction silently fails, users on Cursor 2.4 get no augmentation improvement over the legacy hook for the most common agent operation. The failure is completely invisible — hook exits 0, produces no output, Cursor continues normally.
  • Recommended fix: Capture at least one real Cursor 2.4 postToolUse payload for each of Grep, Read, and Shell by running Cursor with GITNEXUS_DEBUG=1 and logging stdin. Use the captured payloads as test fixtures. This is a prerequisite for confidence in the dominant code paths. As a minimum, add a comment explicitly stating these field names were verified against Cursor docs at a specific version.
  • Blocks merge: Yes if the query field turns out to be wrong; no if verified against Cursor docs. Currently unverified.

[SIGNIFICANT] Hook command path resolution depends on undocumented Cursor working directory behavior

  • Category: Hook runtime contract
  • Files: gitnexus-cursor-integration/hooks/hooks.json:7
  • Issue: The command "node ./hooks/gitnexus-hook.cjs" is a relative path. Cursor must resolve this from somewhere. If Cursor resolves relative paths from the repository root, users must place hooks/ at the repo root. If Cursor resolves from the .cursor/ directory, users must place it at .cursor/hooks/. The PR provides no documentation on where to install these files, and no user-facing install instructions exist (gitnexus-cursor-integration/README.md is noted as non-existent in the PR body). Without knowing Cursor's CWD when executing hook commands, users cannot install this correctly.
  • Why it matters: A hook that cannot be correctly installed by users is not a shipped feature. The "Full" support claim in the README requires this to be discoverable and installable.
  • Recommended fix: Document the install path explicitly (e.g., copy hooks/ to .cursor/hooks/ in the project root). Alternatively, use an absolute path or __dirname-relative path in the command. At minimum, add a comment in hooks.json or a gitnexus-cursor-integration/README.md explaining where to place the files.
  • Blocks merge: Moderate — the hook is technically correct but unusable without install docs.

[MODERATE] README claims "Full" Cursor hook support without a documented install path

  • Category: Docs truthfulness
  • Files: README.md:126, gitnexus/README.md:36
  • Issue: Both READMEs now show Yes (postToolUse) | Full for Cursor hooks. However, gitnexus setup does not install Cursor hooks (PR body confirms this is out of scope), there is no gitnexus-cursor-integration/README.md, and the main README's Cursor section only shows MCP config (~/.cursor/mcp.json) with no mention of hooks. A user reading the table and following the setup docs will get MCP working but will not know how to install the hook. "Full" is only true if hooks are actually installed and working.
  • Why it matters: The Claude Code row says "full support" and users can verify it because gitnexus setup installs everything. The Cursor row makes the same claim but delivers only partial automated support.
  • Recommended fix: Either change the Hooks column to "Yes (manual)" with a link to install instructions, or add a Cursor-specific setup section to the README. Fix this →
  • Blocks merge: No (if project accepts follow-up); yes if truthfulness is required at merge.

[MINOR] Shell parser splits quoted multi-word patterns

  • Category: Pattern quality
  • Files: gitnexus-cursor-integration/hooks/gitnexus-hook.cjs:81-117
  • Issue: cmd.split(/\s+/) cannot parse shell quoting. rg "User Service" src/ returns User (quotes stripped) as the pattern, missing Service. This is a known limitation and is acceptable for a best-effort augmentation tool, but it is not documented in comments and no test covers the behavior of quoted multi-word patterns.
  • Why it matters: Low for augmentation quality; the split token is still signal. But unmasked by the test suite.
  • Recommended fix: Add a test case documenting the behavior (e.g., rg "User Service"User). The existing test for parseRgGrepPattern coverage is source-level only.
  • Blocks merge: No

[MINOR] Read basename extraction may be noisy for common generic filenames

  • Category: Pattern quality
  • Files: gitnexus-cursor-integration/hooks/gitnexus-hook.cjs:131-137
  • Issue: Reading index.ts, utils.ts, types.ts, package.json, README.md produces index, utils, types, package, README as augmentation patterns. These are high-frequency but low-specificity tokens that may return noisy results from BM25 search.
  • Why it matters: Acceptable if the augmentation engine's BM25 ranking degrades gracefully on poor tokens. This is a design choice the PR explicitly accepts. The short-pattern guard (≥ 3 chars) does not filter these.
  • Blocks merge: No

Cursor hook contract assessment

hooks.json schema is structurally correct: version: 1, hooks.postToolUse, array of entries with matcher, command, timeout. The hook phase postToolUse is correct for observational augmentation. The matcher Shell|Read|Grep uses pipe-separated tool names, consistent with what Cursor docs describe as the matcher syntax. beforeShellExecution is completely gone. JSON is valid. No comments in JSON (correct).

Critical defect: "timeout": 10000 is almost certainly in seconds. Cursor docs examples use "timeout": 30. The old hook used "timeout": 5. The internal spawnSync timeouts (7s/12s) protect against spawned subprocess hangs but not against hangs before spawnSync is reached. This must be fixed to 10 (seconds) before merge.

The node ./hooks/gitnexus-hook.cjs command is relative. Cursor's hook working directory is not documented in this PR and there are no install instructions. This is a usability blocker even if technically correct.

Cursor payload / extraction assessment

The hook assumes top-level tool_name, tool_input, and cwd fields, which are plausible for Cursor's postToolUse payload shape based on MCP conventions. extractPattern tries:

  • Grep: toolInput.query || toolInput.pattern || toolInput.regex — reasonable aliases but unverified
  • Read: toolInput.target_file || toolInput.file_path || toolInput.path || toolInput.file — good coverage of plausible field names
  • Shell: toolInput.command — standard

The Grep query field is the highest-risk assumption. The Claude Code hook uses toolInput.pattern for its Grep tool (which is Claude Code's payload). Cursor may use pattern, query, or something else entirely. If the primary field is wrong, all Grep augmentation silently no-ops. No real payload fixtures are provided. This is a meaningful gap for a PR claiming to add Grep coverage.

Case-insensitive tool name matching via .toLowerCase() is intentional and correct — this handles Grep, grep, GREP uniformly.

Unknown tool names return null from extractPattern, causing a graceful no-op. This is correct.

Runtime safety assessment

Excellent across the board:

  • No shell: true anywhere in spawnSync calls. Confirmed by source inspection and test assertion.
  • Local CLI resolution via require.resolve('gitnexus/dist/cli/index.js') followed by graceful fallback to npx / npx.cmd. Correct.
  • Windows: npx.cmd used when process.platform === 'win32'. Confirmed by test assertion.
  • -- marker: Pattern passed as ['augment', '--', pattern]. Prevents any pattern starting with - from being parsed as a CLI flag. Test confirms this.
  • cwd validation: path.isAbsolute(cwd) before use. Test confirms this.
  • Graceful failure: All failure modes exit 0 and emit nothing to stdout. Cursor sees no output and continues normally.
  • stdout discipline: console.log only when result.trim() is non-empty. Debug logging goes to stderr only. Output JSON is always well-formed.
  • Option injection: Impossible — user-controlled pattern is passed as a positional argument after --.

One subtle but correct detail: the augmentation CLI (augment.ts) writes results to stderr, not stdout (LadybugDB captures the stdout fd at OS level). The hook correctly reads child.stderr on success. This matches the Claude hook behavior.

Repo discovery / worktree assessment

The .gitnexus gating logic is directly ported from the Claude hook and is well-designed:

  • Global registry exclusion: isGlobalRegistryDir checks for registry.json or repos/ without a meta.json. This correctly distinguishes the global ~/.gitnexus/ from a repo-local .gitnexus/. Logic is identical to the Claude hook. ✅
  • Parent walk depth = 5: Same bound as the Claude hook. Silently misses .gitnexus > 5 levels up, but this is an accepted convention.
  • Linked worktree fallback: git rev-parse --path-format=absolute --git-common-dirpath.dirname(commonDir) correctly resolves the canonical repo root for linked worktrees. Logic is identical to the Claude hook. ✅
  • Unindexed repo no-op: if (!findGitNexusDir(cwd)) return; before any CLI invocation. ✅
  • Git failure no-op: spawnSync errors are caught and return null. ✅

No cross-index risk identified: walkForGitNexusDir finds the nearest .gitnexus walking upward, which will always be the correct index for the given cwd.

Pattern quality assessment

Grep extraction is reasonable (uncertainty on field name aside). Read basename extraction produces meaningful symbol tokens for most filenames — handler, UserService, AuthController etc. Generic filenames like index, utils, types produce low-signal tokens but BM25 ranking is expected to handle this gracefully. The 3-char minimum guard eliminates the worst noise.

Shell parser is intentionally simple (split(/\s+/)) — handles standard rg Symbol src/ and grep -n Symbol file forms correctly. Known failure: quoted multi-word patterns lose words after the first space. Acceptable for a best-effort augmentation signal.

All extraction paths correctly no-op on empty input, missing fields, and short patterns.

Cross-platform assessment

The Node hook eliminates the bash/jq dependency entirely. npx.cmd is used on Windows, confirmed by test assertion. Path operations use Node's path module throughout. path.isAbsolute handles Windows drive-letter paths (C:\repo). path.basename correctly handles Windows path separators. The hook runs on Windows, macOS, and Linux without modification. This is a genuine improvement over the deleted augment-shell.sh. CI runs unit tests on 3 platforms (per the CI report) and the cursor-hook tests passed 33/33 per the PR author's report (though CI currently shows a failure in an unrelated test).

Docs / setup assessment

The README change is factually accurate for Cursor 2.4+ users who manually install the hook, but overclaims for users who follow the standard setup path:

  • gitnexus setup does not install Cursor hooks (confirmed by PR body)
  • No gitnexus-cursor-integration/README.md exists
  • The main README Cursor section only covers MCP setup, with no mention of hooks
  • The note that gitnexus analyze "registers Claude Code hooks" reinforces that Claude Code gets automated installation, but Cursor does not

The doc comment updates in augment.ts and engine.ts are correct and clean.

augment-shell.sh is completely removed and not referenced anywhere in remaining docs. ✅

Test assessment

Strong source-level coverage. The test suite correctly asserts:

  • File presence and augment-shell.sh removal
  • hooks.json wiring: postToolUse, beforeShellExecution absent, matcher contains all three tools, command points to correct file
  • Source invariants: no shell: true, npx.cmd, path.isAbsolute, .slice(0, 200), additional_context present, hookSpecificOutput absent, -- before pattern
  • .gitnexus gating and isGlobalRegistryDir
  • --git-common-dir linked worktree handling

Behavioral tests execute real hook process (via spawnSync(process.execPath, [CURSOR_HOOK], {...}) and runHook). This is the right approach and tests would fail if the hook exited with non-zero or produced unexpected stdout. ✅

Critical gap: timeout test does not bound the value.

// cursor-hook.test.ts:128-132 (current — too weak)
it('declares a positive timeout', () => {
  const timeout: number = manifest.hooks.postToolUse[0].timeout;
  expect(typeof timeout).toBe('number');
  expect(timeout).toBeGreaterThan(0);     // passes with 10000!
});

This test would pass whether timeout is 5, 10, or 10000. The bug Bugbot flagged would not be caught by any test. The test must assert timeout <= 60 (or similar) to be a useful regression guard.

No tests for real Cursor payload fixtures (e.g., a hardcoded JSON blob matching what Cursor actually sends for a Grep event). All extraction tests are source-level string assertions (expect(source).toContain('toolInput.query')), which verify the code exists but not that the field names are correct for Cursor's actual payload.

Hidden Unicode / hygiene assessment

Bidirectional Unicode controls (U+202A–U+202E, U+2066–U+2069): None found. ✅

Non-ASCII characters: Present in comments and docs. In gitnexus-hook.cjs:12: em dash in a comment (— runs on Windows out of the box). In cursor-hook.test.ts: em dashes in section divider comments. In READMEs: em dashes, arrows, emoji throughout (pre-existing style). These are visible punctuation in comments and docs, not in executable code, JSON keys, regexes, command strings, or output shape. Per the review criteria, these do not block merge.

Elegance / maintainability assessment

The hook is a clean, focused port of the Claude hook's core logic adapted for Cursor's contracts. The shared walkForGitNexusDir / findCanonicalRepoRoot / isGlobalRegistryDir pattern is directly reused and is well-proven. The extractPattern multi-tool dispatch is readable. Error handling is consistent: all paths exit 0, nothing crashes Cursor.

The decision to use postToolUse only (not also preToolUse) is well-reasoned — augmentation is observational, not gating.

The main maintainability concern is the unverified payload field names: future Cursor version changes to field names would silently break augmentation without test coverage to detect it.


Final verdict

❌ Not production-ready

Two hard blockers remain before merge:

  1. "timeout": 10000 must be "timeout": 10 (or correct seconds value). The Cursor Bugbot's analysis is almost certainly correct — Cursor's hooks.json timeout is in seconds, making the current value ~2.8 hours. The internal spawnSync timeouts protect against subprocess hangs but not pre-spawn hangs, leaving a real window where Cursor can be frozen for hours. This is a one-line fix.

  2. The timeout regression test must be tightened to catch this unit error. expect(timeout).toBeGreaterThan(0) passes with any positive number and provides no regression protection.

One significant unresolved uncertainty (not currently a hard block but recommended before merge): The Grep query field name — and all payload field names — are unverified assumptions. A single real Cursor 2.4 postToolUse payload sample would confirm or invalidate the primary new capability. Without this, the PR ships a Grep augmentation path that may silently no-op for all users.

The rest of the implementation is solid: cross-platform Node hook, correct output shape, strong graceful failure semantics, good worktree handling, focused branch, and behavioral tests that execute real hook processes. The timeout fix and a tighter test assertion are the only changes needed to clear the hard blockers.

@magyargergo

Copy link
Copy Markdown
Collaborator

@abhigyanpatwari other than this flaky ci test failure, please look into these findings 🙏

Cursor's `timeout` field in hooks.json is in seconds (per
https://cursor.com/docs/agent/hooks and the original integration's
`"timeout": 5`). I'd written `10000` after blindly copying the issue
body's example — that resolves to ~2.8 hours, not 10 seconds. If the
script ever hangs before reaching its inner spawnSync timeouts (e.g.
during stdin read), Cursor would have waited that long before killing
it.

Drop to `10` (seconds), matching the Claude plugin's hooks.json and
giving plenty of headroom over the inner 7s augment-CLI timeout.

Add a regression-guard assertion in cursor-hook.test.ts so a future
ms/s mixup fails fast.

Reported by Cursor Bugbot on PR #1467.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… install docs

Resolves three findings from Claude reviewer on PR #1467:

1. Cursor payload field-name uncertainty (SIGNIFICANT)
   Claude flagged that the Grep `query` field is an unverified assumption
   per Cursor 2.4 docs (https://cursor.com/docs/agent/hooks). Mitigated:
   - Expanded Grep aliases: query | pattern | regex | q | search | searchQuery
   - Added pickLongestStringValue() last-resort fallback so the hook
     extracts *something* even if Cursor renames every documented field
   - Added GITNEXUS_DEBUG=1 stderr logging of the raw stdin payload so
     users can capture Cursor's actual contract when diagnosing silent
     no-ops, and report it back if aliases drift
   - Added Read alias `filePath` (camelCase variant alongside `file_path`)
   - Inline comment block citing the docs URL and the uncertainty

2. Hook command path resolution + install docs (SIGNIFICANT)
   Claude flagged `node ./hooks/gitnexus-hook.cjs` as relative without
   documented install path. Added gitnexus-cursor-integration/README.md
   with explicit install steps:
   - .cursor/hooks.json + hooks/gitnexus-hook.cjs at project root
   - Confirms Cursor's project-root CWD convention with doc link
   - Verify steps including GITNEXUS_DEBUG capture
   - Pattern-extraction contract table per tool
   - Troubleshooting: not-firing, npx fallback, wrong-pattern diagnosis

3. README "Full" overclaim for Cursor (MODERATE)
   Both README rows now read `Yes (postToolUse, manual install)` linking
   to the new install README, accurately signaling that hooks aren't
   automated by `gitnexus setup` like they are for Claude Code.

4. Shell quoted-pattern parser limitation (MINOR, documented)
   Added inline comment in gitnexus-hook.cjs documenting the known
   `rg "User Service"` -> `User` truncation, plus regression tests in
   cursor-hook.test.ts pinning the behavior so a future change is
   visible.

Test additions (33 -> 41):
- Wide-alias source coverage for Grep (query / pattern / regex / q /
  search / searchQuery) plus pickLongestStringValue fallback
- Read alias coverage including camelCase filePath
- GITNEXUS_DEBUG behavioral test: stderr quiet by default, payload
  echoed when env var set, stdout output contract preserved either way
- Shell quoted-pattern documented behavior tests
- Install README presence + content (.cursor/hooks.json, hooks/, debug
  diagnostics)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@abhigyanpatwari

Copy link
Copy Markdown
Owner Author

@claude review it and see that the above issues are fixed and it has complete test coverage

@github-actions

github-actions Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @abhigyanpatwari's task in 1m 32s —— View job


PR #1467 Review — All Previous Issues Resolved ✅

Todo List

  • Post initial comment
  • Read hooks.json — check timeout fix
  • Read gitnexus-hook.cjs — check payload fields, runtime safety
  • Read cursor-hook.test.ts — check timeout test and coverage
  • Read gitnexus-cursor-integration/README.md — check install docs
  • Read README.md / gitnexus/README.md — check "Full" claim
  • Check CI status
  • Analyze all findings and post review

Review Summary

All three hard blockers and the significant findings from the previous review have been addressed. Here's the full status:


[BLOCKER 1] "timeout": 10000 unit error — ✅ FIXED

hooks.json:8 now reads "timeout": 10 (seconds). Correct.


[BLOCKER 2] Timeout test too weak — ✅ FIXED

cursor-hook.test.ts:135 now asserts:

expect(timeout).toBeLessThan(120);

This correctly rejects millisecond-scale values. Any value ≥ 120 is an almost certain unit error (that'd be a 2+ minute timeout). The guard is meaningful and would have caught the original 10000 value. ✅


[BLOCKER 3] CI failing — ✅ RESOLVED

The CI report now shows ✅ All checks passed — 8548/8549 tests pass, 1 skipped (the documented Ruby block parameter closure inference test, a pre-existing known limitation). CI gate is clear.


[SIGNIFICANT] Cursor payload field names unconfirmed — ✅ WELL ADDRESSED

The hook now implements a defensive pickLongestStringValue fallback (lines 128–137 of gitnexus-hook.cjs) for the Grep path, plus an expanded alias set: query, pattern, regex, q, search, searchQuery. The code comment at line 119–127 explicitly acknowledges the unverified contract and directs users to GITNEXUS_DEBUG=1 for diagnostics. This is the right approach for a best-effort observational feature.

The test at cursor-hook.test.ts:202–210 asserts all 6 aliases are present in source, and asserts pickLongestStringValue exists as a fallback — providing a regression guard if any alias is accidentally removed.


[SIGNIFICANT] Hook command path undocumented — ✅ FIXED

gitnexus-cursor-integration/README.md now exists and fully documents:

  • Cursor reads .cursor/hooks.json from project root (line 17)
  • The script goes to <project>/hooks/gitnexus-hook.cjs (line 26)
  • Shell copy commands provided (lines 31–35)
  • Merge instructions for existing hooks.json users (line 37)
  • Verify steps (lines 39–44)

The directory layout in the README matches the command path "node ./hooks/gitnexus-hook.cjs" — consistent. ✅

Tests lock in this contract at cursor-hook.test.ts:413–438 (install README exists, contains .cursor/hooks.json, hooks/gitnexus-hook.cjs, Hook install, and GITNEXUS_DEBUG). ✅


[MODERATE] README "Full" claim without install path — ✅ ADDRESSED

Both README.md:123 and gitnexus/README.md:36 now read:

Yes (postToolUse, [manual install](gitnexus-cursor-integration/README.md#hook-install))

The "Full" label is kept but the manual install qualifier and link are present — accurate and discoverable. ✅


[MINOR] Shell parser quoted multi-word patterns — ✅ DOCUMENTED

gitnexus-hook.cjs:175–180 now has an explicit NOTE: comment documenting the limitation. Two tests at cursor-hook.test.ts:391–409 pin the behavior — one for the multi-word case (rg "User Service" → first word only), one for the single-quoted token case (rg "validateUser" → works correctly). The limitation is intentional and documented. ✅


Additional Coverage Added (Beyond Previous Review's Requests)

  • Debug logging test (cursor-hook.test.ts:341–376): Verifies payload echoes to stderr only with GITNEXUS_DEBUG=1 and never to stdout — preserving the hook output contract.
  • pickLongestStringValue fallback: New defensive function covering the case where Cursor's Grep field name matches none of the aliases — silent failures are minimized.
  • Install README is test-gated: The test suite now enforces the install README exists and contains required content, preventing silent doc rot.

Verdict

✅ Ready to merge

All three previous hard blockers are resolved. Both significant findings are addressed with appropriate defensive patterns and test coverage. Minor findings are documented. CI is green. The implementation is solid — cross-platform Node hook, correct output shape, graceful failure semantics, comprehensive worktree handling, and behavioral tests that execute the real hook process.
· Branch

@github-actions

github-actions Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 5m 12s —— View job


PR #1467 Review — Cursor 2.4 postToolUse Hook Migration

Todo List

  • Post initial comment
  • Read repo context files (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md)
  • Read all changed files (hooks.json, gitnexus-hook.cjs, cursor-hook.test.ts, READMEs, augment.ts, engine.ts)
  • Read related reference files (Claude hook, Claude hooks.json, hooks.test.ts, augment.ts impl)
  • Check CI status on latest commit
  • Run hidden Unicode / hygiene scans
  • Lane A: Cursor hook manifest and install contract
  • Lane B: Runtime hook implementation analysis
  • Lane C: Tests and CI analysis
  • Lane D: Docs/user-facing claims
  • Lane E: Branch hygiene assessment
  • Write and post comprehensive review

Review bar for this pass

The previous Claude review identified three hard blockers (timeout unit, weak timeout test, CI red) and two significant findings (payload field uncertainty, missing install docs). This pass verifies each is resolved and performs a fresh independent deep-dive on the final diff.


Current PR state

Attribute Value
Head SHA 204a754
CI 9 / 9 checks passing (including CI, CodeQL, Gitleaks, Dependency Review, Docker)
Test results 8548 pass / 0 fail / 1 skip (Ruby closure inference — documented pre-existing)
Typecheck ✅ pass
Cursor Bugbot ✅ resolved — timeout: 10000 was the flagged issue; fixed at c5fb3dc
Hidden bidi Unicode ✅ none found in any changed file
Non-ASCII ⚠️ em dashes in hook comments only (lines 12, 177, 218) — not in executable code, JSON, or regex

Branch hygiene assessment

Classification: Category 2 — merge-from-main commits present but harmless and merge-safe

Three merge-from-main commits (e4262a3, d698059, a0e6cd1). Inspected each: the churn they introduce is gitnexus-web/package-lock.json and gitnexus-web/package.json dependency version bumps from main. No unrelated core changes, no shared contract drift, no lockfile for the CLI package, no workflow changes. The feature's own commits are three focused commits (00e1edc, c5fb3dc, 204a754) covering the hook, the timeout fix, and the Claude-review follow-ups. A squash merge to main will not import the web churn into main's linear history. Rebase is not required.


Lane A — Cursor hook manifest and install contract ✅

gitnexus-cursor-integration/hooks/hooks.json after fixes:

{ "version": 1, "hooks": { "postToolUse": [{ "matcher": "Shell|Read|Grep", "command": "node ./hooks/gitnexus-hook.cjs", "timeout": 10 }] } }
  • "version": 1 — correct per Cursor 2.4 docs schema ✅
  • "postToolUse" — correct phase for observational (non-blocking) augmentation ✅
  • "beforeShellExecution" completely absent ✅
  • "matcher": "Shell|Read|Grep" — pipe-separated tool names match Cursor's documented syntax ✅
  • "timeout": 10 — fixed from the previously-flagged 10000. Cursor timeout is in seconds; 10 seconds is a safe value that matches prior 5 + gives headroom without risking hour-scale hangs ✅
  • Timeout regression test now asserts toBeLessThan(120) — catches any future ms/s confusion ✅
  • "command": "node ./hooks/gitnexus-hook.cjs" — relative path. The new gitnexus-cursor-integration/README.md (line 17) explicitly documents that Cursor runs hook commands from the project root and the install layout puts the script at <project>/hooks/gitnexus-hook.cjs, making this path resolve correctly ✅
  • gitnexus setup is clearly documented as NOT installing hooks (README table at line 52 of new README) ✅

Lane B — Runtime hook implementation ✅ (with one pre-existing minor issue)

Stdin / output contract:

readInput() uses fs.readFileSync(0, 'utf-8') — correct for synchronous stdin consumption. JSON.parse failure returns {} → graceful no-op without touching stdout ✅

child.stderr capture — verified correct:

The hook reads child.stderr from the gitnexus augment subprocess. This is intentionally correct. gitnexus/src/cli/augment.ts:30 documents explicitly:

// IMPORTANT: Write to stderr, NOT stdout.
// LadybugDB's native module captures stdout fd at OS level during init,
// which makes stdout permanently broken in subprocess contexts.
process.stderr.write(result + '\n');

So child.stderr is the right capture stream. ✅

resolveCliPath() design:

The Cursor hook drops the __dirname-relative fallback the Claude hook has. This is correct — when the hook is installed at <project>/hooks/gitnexus-hook.cjs, there is no predictable __dirname-relative path to a dist/cli/index.js. The hook correctly tries require.resolve('gitnexus/dist/cli/index.js') (works when gitnexus is a project dependency) then falls back to npx -y gitnexus. The README documents this at line 88 with a tip to install globally to skip npx cold-start latency. ✅

require.resolve('gitnexus/dist/cli/index.js') validity:

The gitnexus package.json has no exports field, so deep imports are unrestricted. The hook runs as .cjs (CommonJS), so require.resolve is available. Resolution only computes the path; the file is then executed via spawnSync, so the target being an ESM module doesn't matter. ✅

Security — injection safety:

  • No shell: true anywhere (verified by source scan and test assertion) ✅
  • Pattern always passed after -- as ['augment', '--', pattern]
  • cwd validated via path.isAbsolute() before any use ✅
  • All spawnSync calls use array arguments ✅

Global registry exclusion:

isGlobalRegistryDir correctly identifies ~/.gitnexus/ (has registry.json or repos/ without meta.json) vs a project .gitnexus/ (has meta.json). Logic is identical to the Claude hook and battle-tested ✅

Worktree discovery:

findCanonicalRepoRoot via git rev-parse --path-format=absolute --git-common-dir correctly resolves linked worktrees. Walk depth 5 matches Claude hook convention ✅

pickLongestStringValue fallback for Grep:

Scoped to tool_input (the tool's argument object, not the entire event), so it won't accidentally pick output blobs or large unrelated strings. Minimum length ≥ 3 applies. Acceptable for best-effort augmentation ✅

-e flag in parseRgGrepPattern — pre-existing minor bug:

-e is in flagsWithValues (meaning its argument is skipped). For rg -e foo src/, the parser skips foo and returns src/ instead of the intended pattern foo. This is incorrect for the -e form but is a pre-existing limitation in the identical Claude hook logic — not introduced by this PR. The PR author documents the whitespace-split limitation but not this specific case. Augmentation for rg -e PATTERN calls will silently extract a path glob instead of the pattern.

Not a blocker (pre-existing, not introduced here, affects a minority of rg invocations). Recommend filing a follow-up issue. Track this →


Lane C — Tests and CI ✅ (with one acknowledged gap)

CI: All 9/9 checks pass on the latest head (204a754). 8548 unit tests pass, 1 pre-existing skip (Ruby block param — documented known limitation). Typecheck clean. ✅

Behavioral tests run the real hook binary:

Tests use spawnSync(process.execPath, [CURSOR_HOOK], {...}) — executing the actual .cjs file, not reading source strings. No-op paths (empty stdin, invalid JSON, relative cwd, unindexed cwd, unknown tool, 2-char query, basename with no identifier chars) all execute the real process and verify exit 0 + empty stdout ✅

Timeout regression test (cursor-hook.test.ts:135):

expect(timeout).toBeLessThan(120);

This correctly catches millisecond-scale values. 10000 would fail; 10 passes ✅

Debug logging test:

Verifies payload echoes to stderr only with GITNEXUS_DEBUG=1 and that stdout remains empty (preserving the hook output contract). Correct and meaningful ✅

Gap: no positive-path behavioral test:

No test exercises the full augmentation path (hook emits { "additional_context": "..." } after a successful gitnexus augment invocation). The test file comment acknowledges this explicitly: "Cursor hooks reach the augment CLI only when cwd is inside an indexed repo, so behavior tests stick to early-exit paths to avoid spawning npx gitnexus."

This is a real gap — a test that verifies the hook correctly reads child.stderr (not child.stdout) from the subprocess would catch a stream-capture regression. However, it requires a test fixture with a real .gitnexus dir or a mocked CLI binary. This is the same coverage gap that existed pre-PR; this PR doesn't worsen it.

Not a blocker — the gap is acknowledged, the stream contract is verified by reading augment.ts directly (it writes to stderr), and the early-exit behavioral tests provide meaningful regression coverage for the common paths.

Platform coverage:

CI report confirms tests run on 3 platforms (ubuntu, windows, macOS). ✅


Lane D — Docs and user-facing claims ✅

gitnexus-cursor-integration/README.md (new file):

  • Clearly states hooks require Cursor 2.4+ and older versions silently no-op (line 5) ✅
  • Install layout matches the command path in hooks.json (.cursor/hooks.json + hooks/gitnexus-hook.cjs at project root) ✅
  • Copy commands provided ✅
  • Merge instructions for users with existing hooks.json
  • Verify steps with GITNEXUS_DEBUG=1 for payload diagnostics ✅
  • Explicit table distinguishing what gitnexus setup installs vs what is manual ✅
  • Troubleshooting section covering the three main failure modes ✅
  • codebase_search not mentioned (correctly omitted per Cursor 2.4 docs) ✅

Top-level README.md / gitnexus/README.md:

  • Both now say Yes (postToolUse, [manual install](...)) with a link — "manual" qualifier is accurate ✅
  • No stale references to augment-shell.sh anywhere in docs ✅
  • augment-shell.sh deletion test at cursor-hook.test.ts:86-97 catches any accidental re-add ✅

Minor pre-existing inaccuracy (not introduced by this PR):
gitnexus/README.md:35 shows Claude Code hooks as Yes (PreToolUse) — missing PostToolUse. The root README correctly says Yes (PreToolUse + PostToolUse). This inconsistency pre-dates this PR and is out of scope here.


Cursor contract verification

The hook payload assumptions (tool_name, tool_input, cwd at top level) are consistent with Cursor's documented postToolUse event shape per the hook docs URL cited in the file. The Grep field name uncertainty is addressed by the 6-alias probe plus pickLongestStringValue fallback, and GITNEXUS_DEBUG=1 is documented as the path to capture real payloads if aliases drift. This is the correct defensive approach for an observational augmentation feature where silent fallback is acceptable. ✅


Summary of findings

Finding Severity Status
"timeout": 10000 (hours not seconds) BLOCKER ✅ Fixed → "timeout": 10
Timeout test too weak BLOCKER ✅ Fixed → toBeLessThan(120)
CI red BLOCKER ✅ Resolved — 9/9 pass
Grep payload field unverified SIGNIFICANT ✅ Mitigated — 6 aliases + pickLongestStringValue + GITNEXUS_DEBUG
Hook command path undocumented SIGNIFICANT ✅ Fixed — new integration README
README "Full" claim without install path MODERATE ✅ Fixed — "manual install" qualifier + link
Shell parser quoted multi-word patterns MINOR ✅ Documented in code + tests
-e flag skips actual pattern (pre-existing) MINOR Pre-existing in Claude hook — not introduced here
No positive-path behavioral test MINOR Acknowledged gap — not introduced here
Em dashes in hook comments INFO Comments only — no runtime impact
Branch hygiene (3 merge commits) INFO Category 2 — harmless, squash-safe

✅ Production-ready — approved to merge

All three previous hard blockers are resolved. Both significant findings are addressed with appropriate defensive patterns and test-gated contracts. The implementation is correct on the critical paths:

  • Cursor gets valid { "additional_context": "..." } JSON on stdout when augmentation fires
  • The hook reads from the right subprocess stream (child.stderr) — verified against augment.ts's documented stderr-only output contract
  • path.isAbsolute(cwd) validation, .gitnexus gating, and graceful failure on every error path are intact
  • No shell injection surface; pattern always passed after --
  • Install docs, command path, and README claims are consistent and accurate

The two remaining minor items (-e flag extraction bug and no positive-path test) are pre-existing limitations not introduced by this PR. A follow-up issue to track the -e flag bug across both hooks would be a clean housekeeping item.
· Branch

@magyargergo magyargergo merged commit 2620b70 into main May 10, 2026
34 checks passed
@magyargergo magyargergo deleted the feat/cursor-2.4-hooks-1466 branch May 10, 2026 12:29
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.

feat(cursor): upgrade Cursor hook integration to 2.4 preToolUse/postToolUse for Read/Grep coverage

2 participants