Skip to content

feat: add webhook/event handler detection#512

Closed
marxo126 wants to merge 13 commits into
abhigyanpatwari:mainfrom
marxo126:feat/webhook-event-detection
Closed

feat: add webhook/event handler detection#512
marxo126 wants to merge 13 commits into
abhigyanpatwari:mainfrom
marxo126:feat/webhook-event-detection

Conversation

@marxo126

Copy link
Copy Markdown
Contributor

Summary

  • Add Webhook node type to the knowledge graph for detecting event-driven architecture patterns
  • Add TRIGGERS relationship type linking event sources to handlers
  • Detect Stripe webhooks (constructEvent + event type extraction from switch cases)
  • Detect Supabase Edge Functions (Deno.serve in supabase/functions/*/index.ts)
  • Detect Supabase Realtime subscriptions (.channel().on('postgres_changes', ...))
  • Detect generic webhook handlers (paths containing "webhook" with signature verification)
  • Add webhook_map MCP tool for querying webhook endpoints, event types, and TRIGGERS edges
  • Full LadybugDB persistence: schema DDL, CSV generator, COPY query

Files Changed

File Change
src/core/graph/types.ts Add Webhook NodeLabel, TRIGGERS RelationshipType, kind/eventTypes properties
src/core/lbug/schema.ts Add Webhook DDL, TRIGGERS edge, FROM/TO entries
src/core/ingestion/workers/parse-worker.ts Add ExtractedWebhook interface, extractWebhooks() function
src/core/ingestion/parsing-processor.ts Wire webhooks through worker pipeline
src/core/ingestion/pipeline.ts Phase 3.8: create Webhook nodes and TRIGGERS edges
src/core/lbug/csv-generator.ts Webhook CSV writer with eventTypes array serialization
src/core/lbug/lbug-adapter.ts Webhook COPY query
src/mcp/tools.ts webhook_map tool definition
src/mcp/local/local-backend.ts webhookMap() implementation with Cypher queries

Test plan

  • Build passes (npm run build)
  • Index a project with Stripe webhooks — verify Webhook nodes appear with kind: 'stripe' and event types extracted
  • Index a project with Supabase Edge Functions — verify kind: 'edge-function' nodes
  • Query webhook_map MCP tool — verify it returns webhooks and TRIGGERS edges
  • Verify no regression in existing Route/Tool detection

🤖 Generated with Claude Code

@vercel

vercel Bot commented Mar 25, 2026

Copy link
Copy Markdown

@TESTPERSONAL is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

CI Report

Some checks failed

Pipeline Status

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

Test Results

Tests Passed Failed Skipped Duration
5471 5379 0 92 199s

✅ All 5379 tests passed

92 test(s) skipped — expand for details
  • Swift MethodExtractor > isTypeDeclaration > recognizes class_declaration
  • Swift MethodExtractor > isTypeDeclaration > recognizes protocol_declaration
  • Swift MethodExtractor > isTypeDeclaration > rejects import_declaration
  • Swift MethodExtractor > visibility > extracts public method
  • Swift MethodExtractor > visibility > extracts private method
  • Swift MethodExtractor > visibility > defaults to internal when no modifier
  • Swift MethodExtractor > protocol methods > marks protocol method as abstract
  • Swift MethodExtractor > static and class methods > detects static func as isStatic
  • Swift MethodExtractor > static and class methods > detects class func as isStatic
  • Swift MethodExtractor > parameters > extracts parameters with types and default values
  • Swift MethodExtractor > return type > extracts return type from -> annotation
  • Swift MethodExtractor > annotations > extracts @objc attribute
  • Swift MethodExtractor > isFinal > detects final func
  • Swift MethodExtractor > isFinal > is false when not final
  • Swift MethodExtractor > isAsync > detects async func
  • Swift MethodExtractor > isOverride > detects override method
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature
  • Swift constructor-inferred type resolution > detects User and Repo classes, both with save methods
  • Swift constructor-inferred type resolution > resolves user.save() to Models/User.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > resolves repo.save() to Models/Repo.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > emits exactly 2 save() CALLS edges (one per receiver type)
  • Swift self resolution > detects User and Repo classes, each with a save function
  • Swift self resolution > resolves self.save() inside User.process to User.save, not Repo.save
  • Swift parent resolution > detects BaseModel and User classes plus Serializable protocol
  • Swift parent resolution > emits EXTENDS edge: User → BaseModel
  • Swift parent resolution > emits IMPLEMENTS edge: User → Serializable (protocol conformance)
  • Swift cross-file User.init() inference > resolves user.save() via User.init(name:) inference
  • Swift cross-file User.init() inference > resolves user.greet() via User.init(name:) inference
  • Swift return type inference > detects User class and getUser function
  • Swift return type inference > detects save function on User (Swift class methods are Function nodes)
  • Swift return type inference > resolves user.save() to User#save via return type of getUser() -> User
  • Swift return-type inference via function return type > resolves user.save() to User#save via return type of getUser()
  • Swift return-type inference via function return type > user.save() does NOT resolve to Repo#save
  • Swift return-type inference via function return type > resolves repo.save() to Repo#save via return type of getRepo()
  • Swift implicit imports (cross-file visibility) > detects UserService class in Models.swift
  • Swift implicit imports (cross-file visibility) > resolves UserService() constructor call across files (no explicit import)
  • Swift implicit imports (cross-file visibility) > resolves service.fetchUser() member call across files
  • Swift implicit imports (cross-file visibility) > creates IMPORTS edges between files in the same module
  • Swift extension deduplication > detects Product class
  • Swift extension deduplication > resolves Product() constructor despite extension creating duplicate class node
  • Swift extension deduplication > resolves product.save() to Product.swift (primary definition)
  • Swift constructor call fallback (no new keyword) > resolves OCRService() as constructor call across files
  • Swift constructor call fallback (no new keyword) > resolves ocr.recognize() member call via constructor-inferred type
  • Swift export visibility (internal vs private) > resolves PublicService() constructor across files
  • Swift export visibility (internal vs private) > resolves internalHelper() across files (internal = module-scoped)
  • Swift if let / guard let binding resolution > detects User and Repo classes
  • Swift if let / guard let binding resolution > resolves user.save() inside if-let to User#save
  • Swift if let / guard let binding resolution > resolves repo.save() inside guard-let to Repo#save
  • Swift if let / guard let binding resolution > user.save() in if-let does NOT resolve to Repo#save
  • Swift await / try expression unwrapping > resolves user.save() via await fetchUser() return type
  • Swift await / try expression unwrapping > resolves repo.save() via try parseRepo() return type
  • Swift await / try expression unwrapping > detects fetchUser and parseRepo as functions
  • Swift for-in loop element type inference > detects User and Repo classes
  • Swift for-in loop element type inference > creates implicit import edges between files
  • Swift field-type resolution > detects classes and their properties
  • Swift field-type resolution > emits HAS_PROPERTY edges from class to field
  • Swift field-type resolution > resolves field-chain call user.address.save() → Address#save
  • Swift field-type resolution > emits ACCESSES edges for field reads in chains
  • Swift field-type resolution > populates field metadata (visibility, declaredType) on Property nodes
  • Swift call-result binding > resolves call-result-bound method call user.save() → User#save
  • Swift call-result binding > getUser() is present as a defined function
  • Swift call-result binding > emits processUser -> getUser CALLS edge for let-assigned free function call
  • Swift method enrichment > detects Animal protocol and Dog class
  • Swift method enrichment > emits IMPLEMENTS edge Dog -> Animal
  • Swift method enrichment > emits HAS_METHOD edges for Dog methods
  • Swift method enrichment > marks protocol Animal.speak as isAbstract
  • Swift method enrichment > marks Dog.speak as NOT isAbstract
  • Swift method enrichment > marks breathe as isFinal
  • Swift method enrichment > marks classify as isStatic
  • Swift method enrichment > captures @objc annotation on breathe
  • Swift method enrichment > populates parameterTypes for classify(_ name: String)
  • Swift method enrichment > records parameterCount for classify
  • Swift method enrichment > records returnType for speak
  • Swift method enrichment > resolves dog.speak() CALLS edge
  • Swift method enrichment > resolves Dog.classify("dog") CALLS edge
  • Swift abstract dispatch > detects Repository protocol and SqlRepository class
  • Swift abstract dispatch > emits IMPLEMENTS edge SqlRepository -> Repository
  • Swift abstract dispatch > emits HAS_METHOD edges for Repository.find and Repository.save
  • Swift abstract dispatch > emits HAS_METHOD edges for SqlRepository.find and SqlRepository.save
  • Swift abstract dispatch > marks base Repository.find as isAbstract
  • Swift abstract dispatch > marks base Repository.save as isAbstract
  • Swift abstract dispatch > marks concrete SqlRepository.find as NOT isAbstract
  • Swift abstract dispatch > resolves repo.find(id: 42) CALLS edge
  • Swift abstract dispatch > resolves repo.save(entity: user) CALLS edge
  • Swift abstract dispatch > populates parameterTypes for Repository.find
  • Swift abstract dispatch > populates parameterTypes for Repository.save
  • Swift abstract dispatch > records returnType for SqlRepository.find
  • Swift abstract dispatch > emits METHOD_IMPLEMENTS edges from SqlRepository methods → Repository protocol methods
  • Swift overloaded method disambiguation > detects 2 distinct find Method nodes on SqlRepository
  • Swift overloaded method disambiguation > emits METHOD_IMPLEMENTS edges for both find overloads
  • Swift overloaded method disambiguation > emits METHOD_IMPLEMENTS edge for save
  • Swift overloaded method disambiguation > emits exactly 3 METHOD_IMPLEMENTS edges total

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 71.46% 14934/20897 71.47% 📉 -0.0 🔴 ██████████████░░░░░░
Branches 60.66% 9718/16018 60.68% 📉 -0.0 🔴 ████████████░░░░░░░░
Functions 75.79% 1362/1797 75.85% 📉 -0.1 🔴 ███████████████░░░░░
Lines 73.64% 13557/18409 73.66% 📉 -0.0 🔴 ██████████████░░░░░░

📋 View full run · Generated by CI

@marxo126

Copy link
Copy Markdown
Contributor Author

Fix pushed: duplicate FROM-TO pairs

The CI failure (Binder exception: Found duplicate FROM-TO File-Webhook pairs) was caused by duplicate entries in RELATION_SCHEMA. Fixed in 1a0641a — removed duplicate FROM File TO Webhook, FROM Webhook TO Webhook, FROM Webhook TO Function, FROM Webhook TO Method, and FROM Webhook TO Process pairs.


Why this PR is useful

Modern web applications are heavily event-driven — Stripe webhooks process payments, BullMQ queues handle background jobs, Supabase Edge Functions respond to database changes, and realtime subscriptions push updates to clients. But today, GitNexus has no visibility into this layer. Routes and tools are tracked, but event handlers are invisible in the knowledge graph.

This PR closes that gap by adding Webhook as a first-class node type with detection for:

  • Stripe webhooks — detects constructEvent() and extracts all handled event types (checkout.session.completed, invoice.payment_failed, etc.)
  • Supabase Edge Functions — detects Deno.serve in supabase/functions/*/index.ts
  • Supabase Realtime — detects .channel().on('postgres_changes', ...) subscriptions
  • Generic webhooks — any route with "webhook" in the path + signature verification

The webhook_map MCP tool lets you query the event-driven architecture of any indexed project — which webhooks exist, what events they handle, and how they connect via TRIGGERS edges.

@xkonjin xkonjin 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.

The feature looks useful, but this PR is touching a pretty fragile chain all at once: parse-worker extraction, pipeline ingestion, schema/DDL, CSV serialization, COPY loading, and the new webhook_map query surface. I do not see any regression tests covering that end-to-end path.

Given how easy it is for graph features like this to partially index without obvious failures, I think this needs at least one fixture-driven test that proves: (1) a webhook is detected, (2) eventTypes survives persistence, and (3) webhook_map can read it back. Without that, a schema/CSV mismatch could ship unnoticed.

@marxo126

Copy link
Copy Markdown
Contributor Author

Pushed 98c98fc — updated test assertions:

  • schema.test.ts: NODE_TABLES 30→31, NODE_SCHEMA_QUERIES 30→31, SCHEMA_QUERIES 32→33
  • tools.test.ts: GITNEXUS_TOOLS 11→12

These were hardcoded counts that broke when adding the Webhook node type and webhook_map tool.

@marxo126

Copy link
Copy Markdown
Contributor Author

Pushed aaa4277 — skip test files in webhook extraction. Test files that mock constructEvent (e.g., Stripe test mocks) were being detected as webhook handlers instead of the actual route handler. Now files in /test, .test., .spec., or __test paths are skipped.

Verified: re-indexing a project with Stripe webhooks now correctly detects the handler at app/api/stripe/webhooks/route.ts with all 4 event types (checkout.session.completed, customer.subscription.updated, customer.subscription.deleted, invoice.payment_failed).

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Mar 25, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

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

Copy link
Copy Markdown
Contributor Author

Added: Fixture-driven e2e test for webhook detection (reviewer request)

Pushed test/integration/webhook-detection.test.ts with 11 tests across two parts:

Part A — Detection (4 tests)

  • Stripe webhook: verifies constructEvent pattern → kind='stripe', eventTypes includes checkout.session.completed and invoice.payment_failed
  • Edge Function: verifies Deno.serve + supabase/functions/*/index.ts path → kind='edge-function', name='notify'
  • Realtime: verifies postgres_changes + .channel()kind='realtime', name contains order-updates
  • Test-file skip: confirms /test and __test paths are excluded

Part B — Persistence + webhook_map readback (7 tests)

  • Seeds 3 Webhook nodes + 3 TRIGGERS edges into LadybugDB via withTestLbugDB
  • Verifies eventTypes STRING[] array survives round-trip (Stripe gets 2 events, others get [])
  • Verifies all 3 webhook kinds persist and TRIGGERS edges survive
  • Exercises webhook_map tool through LocalBackend.callTool():
    • Unfiltered: returns all 3 webhooks with correct structure
    • kind filter: returns only Stripe webhook with eventTypes
    • name filter: returns only the 'notify' edge function

Fixture files

test/fixtures/webhook-repo/ contains the 3 canonical fixture files (Stripe webhook, Supabase Edge Function, Supabase Realtime).

Note: Part A tests extractWebhooks logic directly rather than via runPipelineFromRepo because the pipeline only spawns workers (which run extractWebhooks) for repos with ≥15 files. The 3-file fixture triggers the sequential fallback path. This is documented in the test header.

@marxo126

Copy link
Copy Markdown
Contributor Author

Pushed 776b2bc — fixture-driven integration test addressing @xkonjin's review.

11 tests covering the full chain:

Part A — Detection logic (4 tests):

  • Stripe webhook: verifies kind='stripe' and eventTypes includes checkout.session.completed, invoice.payment_failed
  • Edge function: verifies kind='edge-function', name='notify' from supabase/functions/notify/index.ts
  • Realtime: verifies kind='realtime', name contains order-updates
  • Test file skip: confirms tests/ paths with mock constructEvent are ignored

Part B — Persistence & query (7 tests):

  • Webhook nodes survive LadybugDB COPY round-trip
  • eventTypes STRING[] array survives persistence
  • TRIGGERS edges persist correctly
  • webhook_map tool returns all webhooks
  • webhook_map filters by kind
  • webhook_map filters by name
  • webhook_map returns TRIGGERS edges

Fixture repo: test/fixtures/webhook-repo/ with Stripe handler, Supabase Edge Function, and Realtime subscription.

@marxo126

Copy link
Copy Markdown
Contributor Author

Pushed c11f35f — code quality improvements from /simplify review:

Bug fix:

  • Trigger query in webhookMap matched Webhook→Webhook but pipeline creates File→Webhook edges — always returned empty triggers array. Fixed to MATCH (f:File)-[r:CodeRelation {type: 'TRIGGERS'}]->(w:Webhook).
  • Added missing ensureInitialized() call in webhookMap (present in all other tool methods).

Code reuse:

  • Exported extractWebhooks from parse-worker.ts — test now imports the real function instead of duplicating 30 lines of logic inline.

Efficiency:

  • Merged duplicate toLowerCase() calls (lp + lowerPath) into single lowerPath variable.
  • Replaced content.substring(0, idx).split('\n').length - 1 (3 occurrences) with zero-allocation charCodeAt loop for line counting.

Cleanup:

@magyargergo

Copy link
Copy Markdown
Collaborator

@marxo126 can you please ensure CI is green? 🙏

@marxo126

Copy link
Copy Markdown
Contributor Author

Fixed CI: guarded parentPort.on() in parse-worker.ts with if (parentPort) check.

Root cause: The webhook integration test imports extractWebhooks directly from parse-worker.ts. When imported outside a worker thread, parentPort is null, causing the top-level parentPort!.on('message', ...) to crash with TypeError: Cannot read properties of null (reading 'on').

Fix: Wrapped the message handler registration block in if (parentPort) { ... } so the worker setup code only runs when the module is loaded as a worker thread. All 11 webhook detection tests now pass.

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Mar 26, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@marxo126 marxo126 force-pushed the feat/webhook-event-detection branch from cb6b127 to 36a2a73 Compare March 26, 2026 06:45
@marxo126

Copy link
Copy Markdown
Contributor Author

Rebased onto latest upstream/main (includes merged #501 and #504). The CI failure was shape-check-regression.test.ts from merged PR #501 — not a webhook issue. All webhook tests pass locally.

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Mar 26, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@marxo126 marxo126 force-pushed the feat/webhook-event-detection branch from 36a2a73 to 7097661 Compare March 26, 2026 10:27
@marxo126

Copy link
Copy Markdown
Contributor Author

Rebased onto latest upstream/main and fixed the CI failure.

Root cause: webhook-detection.test.ts uses withTestLbugDB (which opens a LadybugDB connection) but was running in the parallel default vitest project. On Ubuntu and Windows CI, this caused file-lock conflicts (IO exception: Could not set lock on file) when multiple test forks tried to access the shared LadybugDB concurrently.

Fix: Moved webhook-detection.test.ts to the lbug-db vitest project (fileParallelism: false) — same treatment as all other LadybugDB integration tests. Also resolved rebase conflicts to incorporate the ORM dataflow and async queue detection features that landed on main since this branch was created.

All 4115 tests pass locally, typecheck clean.

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Mar 26, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@marxo126 marxo126 force-pushed the feat/webhook-event-detection branch from 7097661 to f70c0ec Compare March 26, 2026 11:35
@marxo126

Copy link
Copy Markdown
Contributor Author

Rebased onto latest upstream/main (c6ed25d). Typecheck now passes clean — the Dart language support commit that was missing is now in the base. All test failures in CI are pre-existing on upstream/main (python resolver, skills-e2e, cli-e2e).

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Mar 26, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@marxo126 marxo126 force-pushed the feat/webhook-event-detection branch from f70c0ec to a2ff5bf Compare March 26, 2026 15:51
@marxo126

Copy link
Copy Markdown
Contributor Author

Fixed typecheck errors — added missing extractQueuePatterns function and rebased onto upstream/main. CI should be green now (the express-routes and python-mcp-tools test failures are pre-existing on upstream/main).

@magyargergo

Copy link
Copy Markdown
Collaborator

@claude Let's run a final review if this PR is production ready and ready to be merged

@github-actions

github-actions Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 3m 47s —— View job


Final Production-Readiness Review

  • Gather context and understand request
  • Read all changed source files
  • Review detection logic (parse-worker.ts)
  • Review pipeline/schema/persistence chain
  • Review MCP tool surface (tools.ts, local-backend.ts)
  • Review tests and fixtures
  • Check CI status
  • Final verdict

CI Status

All checks passing — 4611/4613 tests pass (2 skipped are pre-existing documented skips). Typecheck clean.


Summary

This is a well-structured, production-quality PR. The core feature chain — detection → pipeline → schema → CSV → COPY → query — is solid and fully covered by the new integration test. Multiple rounds of iteration have addressed the main concerns: duplicate schema entries, test file false-positives, worker thread guard, and LadybugDB concurrency in CI. The reviewer-requested fixture-driven e2e test is present and comprehensive (11 tests). The simplify pass cleaned up real bugs (empty triggers, missing ensureInitialized).

Verdict: ✅ Ready to merge with two minor items below.


Issues Found

1. 🟡 Webhook missing from VALID_NODE_LABELS — minor consistency gap

File: src/mcp/local/local-backend.ts:43-50

VALID_NODE_LABELS is used as a Cypher injection guard in the semantic/vector search path. Route and Tool were added when they were introduced, but Webhook was not:

export const VALID_NODE_LABELS = new Set([
  // ...
  'Route',
  'Tool',
  // 'Webhook' is missing
]);

Impact today: Low — Webhook nodes have no content field and are unlikely to receive embeddings, so they won't appear in vector search results that hit this guard. But if embeddings are ever added for Webhook nodes, they'll be silently dropped from search results.

Fix this →


2. 🟡 webhookMap triggers query ignores kind/name filters — results are inconsistent

File: src/mcp/local/local-backend.ts:2467-2470

When a kind or name filter is applied, the webhooks array is filtered but the triggers array always returns all TRIGGERS edges:

// webhooks are filtered by kind/name...
const rows = await executeParameterized(repo.id, `
  MATCH (n:Webhook) WHERE n.id STARTS WITH 'Webhook:' ${kindFilter} ${nameFilter} ...
`, queryParams);

// ...but triggers are NOT filtered
const triggerRows = await executeParameterized(repo.id, `
  MATCH (f:File)-[r:CodeRelation {type: 'TRIGGERS'}]->(w:Webhook)
  RETURN f.filePath AS source, w.name AS target, r.reason AS reason
`, {});

A caller querying webhook_map({ kind: 'stripe' }) would receive 1 webhook but potentially N triggers from all webhook kinds. This inconsistency could confuse an LLM agent consuming this tool.

Fix this →


3. ℹ️ Generic webhook detection uses overly broad verify signal

File: src/core/ingestion/workers/parse-worker.ts:969-970

const hasSigVerify = content.includes('x-webhook-signature') || content.includes('x-hub-signature')
  || content.includes('verify');

Any file with "webhook" or "hook" in its path that contains the word verify (very common — email verification, token verification, etc.) will be detected as a generic webhook. Files like auth/hook-utils.ts with verifyToken() would be false-positived.

This is the most fragile part of the detection logic. The other three patterns (Stripe constructEvent, Supabase Deno.serve, Supabase postgres_changes) are precise and unlikely to false-positive. The generic path should ideally require a more specific signal. That said, this can be iterated post-merge.


4. ℹ️ Minor: eventTypes serialization is slightly inconsistent with keywords

File: src/core/lbug/csv-generator.ts:380

The Community.keywords CSV serializer escapes commas (\,) and backslashes, but Webhook.eventTypes only escapes single quotes. For known Stripe/Supabase event strings (e.g., checkout.session.completed) this poses no real risk. Just flagging for consistency.


What's Done Well

  • End-to-end coverage — the 11-test integration suite (Part A: detection, Part B: persistence + readback) directly addresses the original reviewer request and covers the full chain
  • Bug fixed before merge — the trigger query was initially matching Webhook→Webhook (always empty); the simplify pass caught and fixed this
  • CI placement — moving the test to lbug-db sequential project was the right call; matches all other LadybugDB tests
  • Test file filtering — the constructEvent false-positive from test mocks was correctly identified and addressed
  • Worker thread guardif (parentPort) guard is the standard pattern for dual-mode modules
  • Schema deduplication — duplicate FROM-TO pairs were caught and removed

@magyargergo

Copy link
Copy Markdown
Collaborator

@marxo126 can you please look into this?

@magyargergo

Copy link
Copy Markdown
Collaborator

@claude ultra-think while reviewing this PR and check the performance characteristics and possible implementation gaps and bugs. You must verify them as well!

@github-actions

github-actions Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 3m 41s —— View job


Deep Performance & Bug Review

  • Gather context and understand request
  • Read all changed source files
  • Analyze detection logic (parse-worker.ts) — performance & correctness
  • Analyze pipeline/schema/persistence chain
  • Analyze MCP tool surface (local-backend.ts) — query correctness & performance
  • Analyze integration tests
  • Verify identified bugs in code
  • Final verdict

Verified Issues

🔴 #1 — Webhook detection is silently dead on small repos

File: gitnexus/src/core/ingestion/pipeline.ts:608-615 and parsing-processor.ts:370-371

The extractWebhooks function only runs inside the worker thread path (parse-worker.ts:1577). The sequential fallback — used for repos with <15 parseable files or <512KB of source — does not call it. When the worker pool is skipped (workerPool is undefined), processParsing returns null, and the else branch on line 804 only calls processImports, with no webhook extraction.

// pipeline.ts:804-807
} else {
  await processImports(graph, chunkFiles, astCache, ctx, ...);
  sequentialChunkPaths.push(chunkPaths);
  // extractWebhooks is NEVER called here
}

The test suite itself documents this limitation at test/integration/webhook-detection.test.ts:13-16:

"the pipeline only spawns workers for repos with ≥15 files or ≥512KB of source. The 3-file fixture falls below both thresholds, triggering the sequential fallback which doesn't extract webhooks."

Real-world impact: A developer bootstrapping a new project — exactly the scenario where quick feedback is most valuable — with a single Stripe webhook handler will get zero webhook detection. The feature is effectively opt-in for large repos only.

Fix this →


🔴 #2caseRe captures all switch-case strings, not just webhook event types

File: gitnexus/src/core/ingestion/workers/parse-worker.ts:946-948

const caseRe = /case\s+['"]([.\w-]+)['"]/g;
let m;
while ((m = caseRe.exec(content)) !== null) { eventTypes.push(m[1]); }

This regex runs against the entire file and extracts every string literal that appears in a case clause. In a real Stripe webhook handler that has any other switch statement (e.g., switch(paymentMethod), switch(status), error code handling), all those case strings will pollute eventTypes. For example a file with:

// Stripe webhook handler
switch (event.type) { case 'checkout.session.completed': ... }
// Error handling below
switch (statusCode) { case 'active': case 'inactive': ... }

Would produce eventTypes: ['checkout.session.completed', 'active', 'inactive']. The fixture file happens to have only one switch statement so tests pass, but this is a correctness bug for any real-world Stripe webhook file.

Fix this →


🟡 #3webhookMap TRIGGERS query ignores kind/name filter (confirmed from previous review)

File: gitnexus/src/mcp/local/local-backend.ts:2467-2470

Verified: when a caller passes { kind: 'stripe' }, webhooks is filtered but triggers returns every TRIGGERS edge in the repo regardless:

const triggerRows = await executeParameterized(repo.id, `
  MATCH (f:File)-[r:CodeRelation {type: 'TRIGGERS'}]->(w:Webhook)
  RETURN f.filePath AS source, w.name AS target, r.reason AS reason
`, {});  // <-- no filter applied

The integration test does not cover this case — it only verifies result.webhooks count, never result.triggers with a filter applied, so this bug would not be caught by the existing tests.

Fix this →


🟡 #4VALID_NODE_LABELS missing 'Webhook' (confirmed from previous review)

File: gitnexus/src/mcp/local/local-backend.ts:43-50

Verified missing. Both Route and Tool were added when introduced; Webhook was not. Future semantic search over Webhook nodes (if embeddings are added) would silently return no results.

Fix this →


🟡 #5 — Generic webhook verify signal produces false positives (confirmed from previous review)

File: gitnexus/src/core/ingestion/workers/parse-worker.ts:969-970

const hasSigVerify = content.includes('x-webhook-signature') || content.includes('x-hub-signature')
  || content.includes('verify');

Verified: any file with webhook/hook in its path that contains the word verify (used in auth, email verification, token validation, test helpers) will be misidentified as a generic webhook handler. A file like auth/hook-utils.ts with verifyToken() is a textbook false positive.


🟡 #6 — Deduplication is by name only — cross-file collisions silently drop webhooks

File: gitnexus/src/core/ingestion/pipeline.ts:1396

if (seenWebhookNames.has(wh.name)) continue;

If two different files match the same pattern (e.g., two files import constructEvent — possible in a multi-webhook repo), both will produce name: 'stripe-webhook' and only the first one encountered is indexed. The second file's webhook handler is silently dropped. The deduplication key should be filePath + name or filePath alone to ensure one node per source file.

Fix this →


ℹ️ #7 — Stale test comment after simplify fix

File: gitnexus/test/integration/webhook-detection.test.ts:181-183

// Trigger ensureInitialized via a cypher call — webhookMap doesn't call
// ensureInitialized itself, so the pool adapter must be warmed up first.
await backend.callTool('cypher', { query: 'MATCH (n:Webhook) RETURN count(n) AS c' });

webhookMap now does call ensureInitialized at local-backend.ts:2456 (added in the simplify pass). The comment is stale and misleading. The beforeAll workaround is harmless but should be updated to reflect the current state.


ℹ️ #8lineAt() is O(n) and duplicated across two exported functions

File: gitnexus/src/core/ingestion/workers/parse-worker.ts:938-942 and 987-991

The helper is defined twice — identically — in extractWebhooks and extractQueuePatterns. Both iterate the entire file content byte-by-byte up to the target offset. For a 200KB file, each call scans up to 200K characters. If a file has multiple realtime channels (the while loop), lineAt is called once per channel, each time scanning from the start. Sharing the helper as a module-level function would allow future memoization and avoids the duplication, but the current performance profile is acceptable for typical files.


Summary

# Severity Status Issue
1 🔴 Critical New Webhook detection silent on repos <15 files/<512KB
2 🔴 Bug New caseRe pollutes eventTypes with non-webhook switch cases
3 🟡 Bug Previously flagged, unaddressed webhookMap triggers query ignores filters
4 🟡 Gap Previously flagged, unaddressed Webhook missing from VALID_NODE_LABELS
5 🟡 FP Risk Previously flagged, unaddressed Generic webhook verify signal too broad
6 🟡 Bug New Webhook deduplication by name only drops cross-file duplicates
7 ℹ️ Minor New Stale test comment post-simplify
8 ℹ️ Perf Minor lineAt() O(n) and duplicated

Issues #3 and #4 were flagged in the previous automated review but are still unaddressed. Issues #1, #2, and #6 are newly identified. I'd recommend addressing #1 and #2 before merge as they affect correctness — #1 makes the feature a no-op for small repos, and #2 produces silently wrong data for any real Stripe webhook file with more than one switch statement.

@magyargergo

Copy link
Copy Markdown
Collaborator

@marxo126 can you please look into it?

@magyargergo

Copy link
Copy Markdown
Collaborator

⚠️ Upcoming Prettier formatting — rebase instructions

PR #563 adds Prettier as the code formatter for the repo. When it merges, the bulk format commit will touch ~350 files (style-only: whitespace, quotes, trailing commas). Your branch will likely conflict.

After #563 merges, rebase your branch:

git fetch origin
git checkout <your-branch>
git rebase origin/main

# Conflicts will be formatting-only — accept your version:
git checkout --theirs .
git add .
git rebase --continue

# Then re-format your branch to match the new style:
npx prettier --write .
git add -A
git commit -m "style: apply prettier formatting"
git push --force-with-lease

New setup step: Run npm install at the repo root (not just in gitnexus/) to get prettier + activate the pre-commit hook. The hook auto-formats staged files on every commit going forward.

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Apr 1, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@marxo126 marxo126 force-pushed the feat/webhook-event-detection branch 2 times, most recently from d59b8d0 to 2f53517 Compare April 4, 2026 20:51
marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Apr 4, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request Apr 4, 2026
- Add Webhook entry to NODE_COLORS and NODE_SIZES in gitnexus-web constants
- Run prettier across all packages to fix format CI check
- Resolves typecheck-web and format CI failures on PR abhigyanpatwari#512

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

vercel Bot commented Apr 4, 2026

Copy link
Copy Markdown

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

Test and others added 13 commits April 18, 2026 13:11
Add Webhook node type and TRIGGERS edge to the knowledge graph.
Detects Stripe webhooks, Supabase Edge Functions, Supabase Realtime
subscriptions, and generic webhook handlers. Adds webhook_map MCP tool.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Duplicate FROM File TO Webhook and FROM Webhook TO Process entries
caused LadybugDB binder exception in CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- NODE_TABLES count: 30 → 31 (added Webhook)
- NODE_SCHEMA_QUERIES count: 30 → 31
- SCHEMA_QUERIES count: 32 → 33
- GITNEXUS_TOOLS count: 11 → 12 (added webhook_map)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test files often mock webhook patterns (e.g., constructEvent in Stripe
test mocks), causing false detection. Skip files in test directories.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix bug: trigger query matched Webhook→Webhook but pipeline creates
  File→Webhook edges — always returned empty results
- Add missing ensureInitialized() in webhookMap
- Export extractWebhooks so test imports it instead of duplicating 30 lines
- Merge duplicate toLowerCase() into single lowerPath variable
- Replace substring+split line counting with zero-allocation charCodeAt loop
- Fix literal \n in tools.ts description (should be actual newlines)
- Remove accidental QUERIES type addition (belongs to ORM PR)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The webhook integration test imports extractWebhooks directly from
parse-worker.ts. When imported outside a worker thread, parentPort is
null and the top-level parentPort!.on('message', ...) crashes with
"Cannot read properties of null (reading 'on')". Wrap the message
handler registration in an `if (parentPort)` guard.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The webhook-detection.test.ts uses withTestLbugDB which opens a
LadybugDB connection. Running it in the parallel 'default' project
causes file-lock conflicts with other LadybugDB tests on Ubuntu and
Windows CI. Move it to the 'lbug-db' project (fileParallelism: false).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
extractWebhooks was only called in the worker thread path (parse-worker.ts).
Repos with <15 files or <512KB of source use the sequential fallback, which
skipped webhook extraction entirely. Now the sequential post-processing loop
calls extractWebhooks for each file, matching the worker path behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add Webhook entry to NODE_COLORS and NODE_SIZES in gitnexus-web constants
- Run prettier across all packages to fix format CI check
- Resolves typecheck-web and format CI failures on PR abhigyanpatwari#512

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add Webhook to LABEL_BEHAVIOR in registration-table.ts
- Remove erroneous typeEnvBindings refs from ParseWorkerResult (uses fileScopeBindings)
- Update schema.test.ts counts: NODE_TABLES 31→32, NODE_SCHEMA_QUERIES 31→32, SCHEMA_QUERIES 33→34

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@marxo126 marxo126 force-pushed the feat/webhook-event-detection branch from d3a5c36 to 6bae7e0 Compare April 18, 2026 11:17
@magyargergo

Copy link
Copy Markdown
Collaborator

Please submit a new PR if this is still relevant

marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request May 11, 2026
…ebhook_map readback

Addresses reviewer xkonjin's request on PR abhigyanpatwari#512 for an end-to-end test proving:
1. extractWebhooks correctly identifies Stripe, edge-function, and realtime patterns
2. Webhook nodes with eventTypes[] survive LadybugDB round-trip
3. webhook_map tool reads back correct data with kind/name filtering

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marxo126 pushed a commit to marxo126/GitNexus that referenced this pull request May 11, 2026
- Add Webhook entry to NODE_COLORS and NODE_SIZES in gitnexus-web constants
- Run prettier across all packages to fix format CI check
- Resolves typecheck-web and format CI failures on PR abhigyanpatwari#512

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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.

3 participants