Skip to content

fix: indirect JS/TS call resolution for dispatch tables and callbacks#1985

Open
Tefa-hup wants to merge 1 commit into
Graphify-Labs:v8from
Tefa-hup:fix/js-indirect-call-dispatch
Open

fix: indirect JS/TS call resolution for dispatch tables and callbacks#1985
Tefa-hup wants to merge 1 commit into
Graphify-Labs:v8from
Tefa-hup:fix/js-indirect-call-dispatch

Conversation

@Tefa-hup

Copy link
Copy Markdown

Ready-to-submit GitHub issue (or PR description) for Graphify-Labs/graphify

Repo: https://github.com/Graphify-Labs/graphify (redirects from https://github.com/safishamsi/graphify)
Diff is against main @ 8cee776cee7d5dc3279085797bde2d2dcce21e36 (2026-07-17).

I don't have push/fork access to this repo from my GitHub account, so I could not open
this as an issue or PR directly. Steps to submit it yourself:

  1. Fork https://github.com/Graphify-Labs/graphify on GitHub (web UI button, top right).
  2. Clone your fork locally, or apply the patch file directly to a fresh clone:
    git apply graphify-js-indirect-call-fix.diff (from the repo root — the diff already
    has correct paths for graphify/extractors/engine.py and tests/test_indirect_dispatch.py).
  3. Commit, push to your fork, open a PR against Graphify-Labs/graphify:main using the
    title/body below (or open it as an issue first if you'd rather report it without a PR).

Title

JS/TS AST extractor: two false-positive indirect_call edge sources (for-of destructuring, module.exports = {...})

Body

Found while auditing a graphify run against a real Node.js codebase: several indirect_call edges in the graph did not correspond to any real call in the source. Root-caused to two distinct bugs in graphify/extractors/engine.py, both in the JS/TS indirect-dispatch resolution path.

Bug 1 — for (const [a, b] of x) loop variables aren't recognized as local bindings

_js_local_bound_names's walk() (around L1151) only special-cases variable_declarator nodes to collect bound names. But a for_in_statement's loop variable(s) live directly in its left field — never wrapped in a variable_declarator:

for_in_statement
  kind: const
  left: array_pattern            <- not a variable_declarator!
  operator: of
  right: call_expression
  body: statement_block

So a destructured loop var like count in for (const [file, count] of Object.entries(x)) is never added to the function's local-shadow set. A later bare read of it (if (count >= 3)) then gets treated as a reference to a same-named module-level function, and if a same-named callable exists ANYWHERE in the corpus, the cross-file resolver (extract.py ~L4905-4927) manufactures a phantom indirect_call edge to it — confidence_score 0.8, i.e. presented as fairly confident.

Concretely: a file with for (const [file, count] of editCounts) { if (count >= 3) {...} } and an unrelated JsonFileBackend.count() method defined elsewhere in the repo produced a graph edge claiming the first function calls the second. It doesn't — there's no textual reference between them at all.

Bug 2 — module.exports = { a, b, c } misidentified as a dispatch table

The module-level dispatch-table scanner (_scan_js_module_dispatch, ~L4386) treats any top-level object/array literal as a potential handler-registry, via _js_dispatch_value_idents. The standard CommonJS export idiom module.exports = { init, consolidate, stats }; has exactly that shape (an object of bare/shorthand identifiers), so every plain CommonJS module hits this and gets a spurious indirect_call edge from the file node to each of its own exported functions — misrepresenting an export list as a runtime dispatch table.

Fix

Two changes, both scoped to graphify/extractors/engine.py (see attached diff):

  1. In _js_local_bound_names.walk(), add a branch for for_in_statement that pulls identifiers out of its left field via the existing _js_collect_pattern_idents helper (handles both destructured and bare-identifier loop vars).
  2. In _scan_js_module_dispatch, detect when an assignment_expression's LHS is the whole exports surface (module.exports or bare exports — not module.exports.NAME / exports.NAME, which can legitimately hold a real dispatch table under a named property) and skip dispatch-scanning that object directly, while still walking its children so a genuine nested table under a named property is still found.

Verification

  • Re-ran extract() on the four files that surfaced this in a real repo: indirect_call edges went from 9 (all false positives) to 0, with zero change in node count and only those 9 phantom edges removed from the edge count.
  • Added 5 regression tests to tests/test_indirect_dispatch.py (for-of destructured var, for-in bare var, module.exports = {...} exemption, bare exports = {...} exemption, and a control test confirming module.exports.NAME = {...} / named sub-property assignment still resolves as a real dispatch table — the exemption is intentionally narrow).
  • tests/test_indirect_dispatch.py: 31/31 pass with the patch.
  • Broader JS/TS/dispatch/export-related subset across the whole suite: 374 passed, 4 skipped (pre-existing skips, unrelated).
  • Full suite: 35 pre-existing failures (in test_obsidian_filename_cap, test_ollama_retry_cap, test_skillgen, test_watch) reproduce identically with the patch reverted (confirmed via git stash) — unrelated to this change, likely Windows/environment-specific.

Files in this folder

  • graphify-js-indirect-call-fix.diff — the patch (engine.py fix + new tests), apply with git apply from repo root.
  • engine_patched.py — the full post-patch graphify/extractors/engine.py for reference/diffing.

- Fix indirect function reference resolution in JavaScript/TypeScript for:
  - Callbacks passed by name in arrays/objects (dispatch tables)
  - Assignment/return of function values
  - getAttr reflective calls
- Added regression tests in test_indirect_dispatch.py
- The fix was verified against the full test suite (only unrelated pre-existing failures)

Closes #xxxx (لو هتعمل Issue)
@Tefa-hup
Tefa-hup marked this pull request as draft July 18, 2026 02:10
@Tefa-hup Tefa-hup closed this Jul 18, 2026
@Tefa-hup Tefa-hup reopened this Jul 18, 2026
@Tefa-hup
Tefa-hup marked this pull request as ready for review July 18, 2026 02:12
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.

1 participant