The PR #1497 adversarial review confirmed via grammar inspection that
dynamic PHP call shapes ($obj->$method(), $obj->{$method}(),
Class::$method(), $className::method(), call_user_func with variable
/ string / array callables, dynamic property reads) currently produce
zero captures and zero CALLS edges - because the member_call_expression
and scoped_call_expression query patterns constrain `name:` to
`(name)` rather than `(_)`, deliberately excluding variable_name
nodes.
That safety invariant was not regression-tested. A future query.ts
edit relaxing `name:` to `(_)` would silently emit false-positive
edges. Add a php-dynamic-calls fixture covering all 11 dynamic shapes
from Findings 1-7 plus a sanity-check static call that DOES emit an
edge - without the sanity check every zero-edge assertion would pass
even if the pipeline emitted no edges at all.
Adds inline SAFETY-INVARIANT comments above the three load-bearing
query patterns (member_call_expression, scoped_call_expression, and
the static-write property pattern) referencing this fixture as the
regression net.
The untyped @declaration.variable catch-all pattern in query.ts (lines
101-103) has no `type:` constraint and tree-sitter therefore also
matches it against typed property declarations - emitting a second
capture for the same property_declaration anchor. Graph-level def-id
collision currently masks the duplicate at the node-emit layer, but
the catch-all capture still flows through scope-binding and name-keyed
registries with a `$`-prefixed name that the typed branch's `$`-strip
never normalizes - a known vector for receiver-binding lookup pollution.
The two tree-sitter patterns produce separate rawMatches entries with
separate `grouped` maps, so the dedup must be cross-match. Pre-scan
rawMatches once to collect anchor node IDs already covered by
@declaration.property, then skip @declaration.variable matches whose
anchor is in that set.
Adds php-typed-property-dedup fixture covering typed property,
constructor-promoted typed parameter, and a mixed-untyped declaration
to verify the catch-all path still fires for untyped properties.
When the class-name receiver pass (Case 2 in receiver-bound-calls.ts)
found a most-derived definition that was arity-incompatible with the
call site, the previous code used `continue` to fall through to the
next ancestor in the MRO chain. If an ancestor happened to be arity-
compatible, the resolver emitted a false CALLS edge to it.
This is incorrect: PHP dispatches to the most-derived override at
runtime and throws `ArgumentCountError` when arities don't match -
it never silently redirects to an ancestor. Replace the inner
`continue` with `break` so the chain walk terminates and no edge
is emitted for the site.
Adds php-mro-arity-mismatch fixture and 5 regression tests covering:
- the bug scenario (Child::method/2 + Parent::method/1, call with 1 arg)
- arity-compatible happy path (Child::compat/1)
- no-parent case (Orphan with arity mismatch)
- happy path most-derived call
- class detection sanity check
Claude Code defaults to prompting for Bash approval. In GitHub Actions there
is no human to approve, so gh pr comment and similar commands fail and the
PR receives no review comment. Pass --dangerously-skip-permissions for the
code-review step only (headless CI; token and checkout are already scoped).
Co-authored-by: Cursor <cursoragent@cursor.com>
The Run Claude Code Review step passed an invalid PR ref
(owner/repo/pull/N) which gh interprets as a branch name, causing
early gh pr view failures. More importantly, the prompt omitted
--comment, so the code-review plugin only displayed findings in
terminal output and never invoked gh pr comment to post to the PR.
Switch to a full PR URL and add --comment so the plugin posts the
review during the session, which also routes around upstream bugs
anthropics/claude-code-action#1061 and #1087 where the action's
post-step capture can silently drop output on issue_comment triggers.
After U3 (FQN-keyed bindingAugmentations) lands, the FQN regression
passes under REGISTRY_PRIMARY_PHP=1 but still fails under =0. The
legacy DAG resolves receiver types via simple-name workspace lookup
and has no namespace-prefixed binding channel, so it cannot
distinguish `\App\Other\User` from a same-simple-name class
reachable via `use`. Per the established convention from commit
af9af4a9 ("parity with the legacy DAG is not a correctness
criterion when the legacy DAG itself has the same defect"), the
test registers in LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.php
rather than blocking the migration.
The other two assertions in the new describe block (class detection
and the saveLocal control) pass in both modes and stay green.
Verified:
- REGISTRY_PRIMARY_PHP=0: 174 passed, 4 skipped (3 existing + 1 new).
- REGISTRY_PRIMARY_PHP=1: 178 passed.
- C# + Python + TypeScript + Go + C resolver matrix: 794 passed.
- pickImplicitThisOverload unit tests: 5 passed.
- resolver-parity-expected-failures unit test: 3 passed.
Plan: docs/plans/2026-05-11-002-fix-php-fqn-and-overload-codex-findings-plan.md (U5)
Codex PR #1497 review, finding 2: pickImplicitThisOverload returned
candidates[0] after narrowOverloadCandidates without checking
uniqueness. When two same-name methods on the same class shared
identical arity and the call site lacked disambiguating argument-type
info, narrowing left both compatible and the resolver emitted a
high-confidence CALLS edge whose target depended on registration
order rather than a defensible resolution.
Tighten the picker:
- candidates.length === 1 -> return candidates[0] (unchanged for the
unambiguous case)
- candidates.length !== 1 (zero or multiple) -> return undefined
(call left unresolved; no edge emitted)
This mirrors pickUniqueGlobalCallable's existing pattern in the same
file.
Export pickImplicitThisOverload so a unit test can exercise it with
synthetic stubs — PHP cannot produce the multi-overload failure shape
(no method overloading in PHP), and C# integration coverage would
entangle the unit's contract with the broader C# resolver. The unit
test pins five cases: sole overload, narrowing-disambiguated, the
ambiguous multi-candidate case (the bug regression), no-match, and
no-enclosing-class.
Verified: 972/972 across PHP + C# + Python + TypeScript + Go + C
resolver suites; no regression in any language. tsc clean.
Plan: docs/plans/2026-05-11-002-fix-php-fqn-and-overload-codex-findings-plan.md (U4)
Extend populatePhpNamespaceSiblings with Step 3b: for every PHP file's
Module scope, inject a binding entry keyed by the fully-qualified
class name (`App\Models\User`) for every class-like def in the
workspace. This routes FQN-receivers like `\App\Other\User` to the
exact namespace-qualified class regardless of which simple-name `User`
the caller's `use` imports shadowed.
Why module-scope bindingAugmentations instead of mutating def.qualifiedName:
the shared QualifiedNameIndex consumes def.qualifiedName at finalize time,
but PHP class defs need to remain keyed by simple name throughout the rest
of the pipeline (MRO, method-dispatch-index, namespace-siblings step 3).
An earlier attempt to rewrite def.qualifiedName to namespace-prefixed form
cascaded into 32 unrelated test failures across receiver-binding, MRO, and
heritage. The bindingAugmentations channel is purpose-built for adding
post-finalize visibility without mutating shared semantic state, and
`findClassBindingInScope`'s scope-chain walk already consumes it via
`lookupBindingsAt` — wiring is zero-touch.
Cost: O(PHP files × class-like defs) augmentation entries. Typical PHP
project: hundreds × hundreds = bounded.
Verified locally:
- Registry-primary: 178/178 PHP tests pass (including the new FQN regression).
- Legacy DAG: 174 passed, 3 existing skips, 1 failure (the FQN test, expected
— legacy DAG has no namespace augmentation channel; U5 registers it in
LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.php).
- Cross-language: 794/794 (C#, Python, TypeScript, Go, C) pass. The PHP-only
augmentation does not touch shared resolution code.
- tsc clean.
Plan: docs/plans/2026-05-11-002-fix-php-fqn-and-overload-codex-findings-plan.md (U3)
Stop collapsing `\App\Models\User` to `User` in normalizePhpType (step 6).
Canonicalize the leading backslash off and preserve the qualified path
on TypeRef.rawName so downstream PHP receiver resolution can distinguish
the FQN target from a same-simple-name class reachable via `use`.
- Step 6 rewritten: `\App\Models\User` → `App\Models\User`,
`App\Models\User` → unchanged, `User` → unchanged.
- Final validation regex relaxed from /^\w+$/ to /^\w+(?:\\w+)*$/ to
accept qualified PHP identifiers while still rejecting empty segments,
trailing backslashes, and non-identifier characters.
- Step 5 (single-arg generic strip) already passes qualified inner types
through via its existing /^\w[\w\]*/ pattern — no change needed.
U3 will wire the qualified-name lookup path; this commit alone is a
no-op for resolution (the lookup chain still keys on simple names).
PHP suite remains green at 177 passed; the only failing test is the
intentional FQN regression from U1.
Plan: docs/plans/2026-05-11-002-fix-php-fqn-and-overload-codex-findings-plan.md (U2)
Fixture php-fqn-cross-namespace declares two `User` classes — `App\Models\User`
and `App\Other\User` — and a Service.php that imports the Models simple-name
via `use App\Models\User` but uses a fully-qualified `\App\Other\User` in a
parameter annotation. Adds three assertions:
- Both User classes are detected as Class nodes in distinct files.
- The FQN-parameter method's `$u->record()` resolves to app/Other/User.php
(currently FAILS — Codex review finding 1).
- The simple-name-parameter method's `$u->record()` resolves to
app/Models/User.php (control; still passes pre-fix).
This is the test-first gate Codex required: "Block merge until the PHP FQN
fixture fails before the fix and passes after it." Verified failing on
HEAD before U2-U3 land the fix.
Also sweeps every `toBeGreaterThanOrEqual` in php.test.ts to exact
`.toBe(N)` per DoD.md §2.7 ("avoid bounds-only assertions that mask
regressions"). Touched 10 assertions across the U2/U3 trait MRO blocks,
the namespace-aware free-call fallback block, and the new FQN block.
All previously-green tests stay green with exact counts — no latent
over-emission bugs were hidden by `>= 1`.
The ambiguous-overload finding (Codex finding 2) cannot be exercised by a
PHP integration fixture — PHP does not support method overloading, so
`model.methods.lookupAllByOwner` returns at most 1 entry per (class, name)
pair. That regression test ships as a unit test against
`pickImplicitThisOverload` in U4 once the function is exported.
Plan: docs/plans/2026-05-11-002-fix-php-fqn-and-overload-codex-findings-plan.md (U1)
* fix(augment): add CONTAINS fallback when FTS indexes unavailable
When the MCP server holds the KuzuDB write lock, the augment CLI opens
the DB read-only. FTS indexes cannot be created in read-only mode, so
searchFTSFromLbug returns ftsAvailable=false and an empty results array.
The existing early-return path silently produced no enrichment.
Add a Cypher name CONTAINS fallback that fires only when ftsAvailable is
false and BM25 produced no symbol matches. This covers the read-only DB
case (concurrent MCP server) and the first-run case (indexes not yet
built). The fallback is wrapped in .catch(() => []) and cannot throw.
When FTS indexes exist, this branch is never reached — behaviour is
unchanged for users without a concurrent MCP server.
* fix(augment): guard against CONTAINS '' and add no-FTS test coverage
Blocker 1 — CONTAINS '' on whitespace-leading patterns:
pattern.split(/\s+/)[0] returns "" when the input has leading whitespace
(e.g. " ".split(/\s+/) → ["", ""]). In Kuzu, CONTAINS '' matches every
node with a name property, injecting arbitrary graph nodes into LLM context.
Fix: trim() before split, then guard on !firstWord || firstWord.length < 2.
No behaviour change for normal non-empty patterns.
Blocker 2 — zero test coverage on the FTS-unavailable code path:
The new CONTAINS fallback block (engine.ts lines 146-166) was exercised by
no existing test — all existing tests run with FTS indexes built. A second
withTestLbugDB fixture is added with no ftsIndexes, forcing searchFTSFromLbug
to return ftsAvailable: false, and asserts:
1. augment('login', ...) returns non-empty enrichment (fallback works)
2. augment(' ', ...) returns '' (CONTAINS '' guard holds)
3. augment('nxyz_notfound', ...) returns '' (no matching nodes)
4. executeQuery throwing returns '' (.catch(() => []) path)
* fix(augment): extend CONTAINS '' guard to FTS happy path and consolidate
The same split(/\s+/)[0] bug existed at line 125 (BM25 symbol filter,
FTS-available path) — a leading-whitespace pattern produced CONTAINS ''
there too, matching every node in BM25-matched files.
Fix: hoist patternFirstWord computation with trim() and the length guard
to the top of augment(), before any DB interaction. Both CONTAINS sites
(BM25 symbol filter and CONTAINS fallback) now use the single pre-validated
value. No behaviour change for normal patterns; the guard fires once for
all callers instead of being duplicated.
Also tighten the whitespace test in the no-FTS suite from 3 spaces to
4 spaces so it unambiguously exercises the patternFirstWord guard rather
than straddling the outer pattern.length < 3 boundary.
* test(augment): negative-safety test for ftsAvailable=true gate
Asserts the CONTAINS fallback does NOT fire when FTS is available but
BM25 returns zero results. Pins the safety property promised by the PR
description: behavior is unchanged for users without the read-only-DB
condition.
If anyone later loosens the gate to `symbolMatches.length === 0` alone,
this test fails.
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
* feat(cli): add --skip-skills and --index-only flags to analyze command
The `installSkills()` call in `generateAIContextFiles()` runs
unconditionally, injecting 6 skill files into `.claude/skills/gitnexus/`
even when `--skip-agents-md` is passed. This is problematic for bulk
indexing operations on read-only mirrors or third-party repos.
Add two new flags:
- `--skip-skills`: suppress standard GitNexus skill file injection
- `--index-only`: pure index mode that suppresses all file injection
(AGENTS.md, CLAUDE.md, and skills), writing only to `.gitnexus/`
This gives users three levels of control:
- `--skip-agents-md` — suppress only root context files
- `--skip-skills` — suppress only skill injection
- `--index-only` — suppress everything (pure indexing)
Discovery context: while bulk-indexing 176 repos with
`--skip-agents-md`, all 144 indexed repos were contaminated with
`.claude/skills/gitnexus/` files requiring manual cleanup.
* fix(cli): address PR #742 review — gate community skills, drop dangling refs, add tests
Bot review (#742) flagged three issues with the original commit:
1. `--index-only --skills` still wrote community-derived skill files
to `.claude/skills/generated/`. The `--skills` branch in analyze.ts
was not gated by `skipAll`, so the "skip all file injection" contract
was violated. Gate `generateSkillFiles()` with `!skipAll` so
`--index-only` truly wins over `--skills`.
2. `--skip-skills` without `--skip-agents-md` produced AGENTS.md /
CLAUDE.md that still referenced `.claude/skills/gitnexus/*/SKILL.md`
files that were never installed — every agent load incurred 6
failed reads. Pass `skipSkills` through to `generateGitNexusContent()`
and omit the standard-skill rows (and the entire `## CLI` heading
when the table is empty). Community skills, when present via
`--skills`, are unaffected.
3. No filesystem tests for `skipSkills` / `indexOnly`. Add three
regression guards to `test/unit/ai-context.test.ts`:
- `.claude/skills/gitnexus/` is NOT created when skipSkills=true
- Nothing is written when both skipAgentsMd and skipSkills are true
(the resolved-flag state from --index-only)
- AGENTS.md/CLAUDE.md routing table omits standard skill references
when skipSkills=true, but preserves the load-bearing imperative
sections (Always Do / Never Do / Resources)
* test(cli): PR 1485 review follow-ups (help text, gate test, --skip-skills docs)
- Assert --skip-skills and --index-only in analyze --help (skip-git-cli.test.ts).
- Export shouldGenerateCommunitySkillFiles; unit-test index-only+skills gate.
- Clarify --skip-skills does not suppress --skills community files; --index-only for full skip.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(cli): warn when --index-only silently overrides --skills
Address review findings on PR 1485 follow-ups:
- analyze.ts emits a one-line note when both --index-only and --skills
are set, so users see why a pipeline re-index ran with no skill files
written.
- index.ts --skills help text now flags the --index-only override.
- shouldGenerateCommunitySkillFiles JSDoc documents the dual role of
the gate (community skills + AGENTS.md/CLAUDE.md re-generation).
- skip-git-cli.test.ts pins the override-warning surface end-to-end.
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
CI scope-parity / php parity was failing under REGISTRY_PRIMARY_PHP=0 on
three assertions added by commit af9af4a9 (U1 arity-narrowing, U3 trait
shadows parent). Per that commit's stance — "parity with the legacy DAG
is not a correctness criterion when the legacy DAG itself has the same
defect" — backporting these fixes to the legacy resolver is out of scope.
Adopt the existing sibling pattern (csharp/typescript/python use the
same helper):
- php.test.ts: switch to `const it = createResolverParityIt('php')` so
expected-failure assertions skip under legacy mode.
- helpers.ts: register three test names in
LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.php with the rationale.
Verified locally: 172 passed + 3 skipped under REGISTRY_PRIMARY_PHP=0,
175 passed under REGISTRY_PRIMARY_PHP=1, tsc clean.
* feat(embeddings): forward GITNEXUS_EMBEDDING_DIMS as dimensions in HTTP request body
When GITNEXUS_EMBEDDING_DIMS is set, include it as the `dimensions` field
in the /v1/embeddings request body. This enables Matryoshka-capable models
(OpenAI text-embedding-3-*, Cohere embed-v3, Voyage) to return truncated
vectors at the requested size.
When the env var is unset, the request body remains `{ input, model }` —
no breaking change for backends that reject unknown fields.
Adds 4 unit tests covering both paths (with/without dimensions) on both
the batch embed and single-query embed code paths.
* fix(embeddings): address review findings — strict parseInt, multi-batch test, comment wording
1. Strict parseInt validation: reject non-numeric strings like '1024abc'
by checking /^\d+$/ before parseInt (Finding 1).
2. Add multi-batch test asserting dimensions is forwarded in every fetch
call when inputs exceed batch size (Finding 2).
3. Soften JSDoc comment: backends may ignore or reject the dimensions
field rather than universally ignoring it (Finding 3).
4. Add test for invalid GITNEXUS_EMBEDDING_DIMS values.
---------
Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
These were stripped by the PHP migration commit (69786b1) as
collateral damage during the rebase, but they have nothing to do
with scope resolution. They feed cluster/community detection and
Laravel route-attribute analysis at the higher analysis layers.
Restored verbatim from 69786b1~1:
- entryPointPatterns: 17 PHP idioms (Controller, handle/execute/
boot/register, REST verbs, Service/Repository, find/save/delete)
- astFrameworkPatterns: Laravel routing block with 3.0x multiplier
for Route::get / Route::post / #[Route(...)] attribute, etc.
PHP 175/175 still green; typecheck clean.
Fixes four PHP-semantic defects identified by the production-readiness
review. The bar is "graph edges must reflect what PHP actually does",
not "matches legacy DAG" — parity with the legacy DAG is not a
correctness criterion when the legacy DAG itself has the same defect.
U1: Variadic requiredParameterCount
arity-metadata.ts:49 now subtracts the variadic slot from required
count: total - optionalCount - (hasVariadic ? 1 : 0). f(int $req,
...$rest) requires 1 arg, not undefined. Adds overload-narrowing and
lookup-core arity-filter changes so resolvers actually drop candidates
that are definitively arity-incompatible (was silently rescuing
empty filter sets even when bounds were known).
U2: True transitive trait MRO
buildPhpMro now uses a BFS worklist (collectTransitiveTraits) to
flatten the trait-of-trait DAG to fixpoint instead of expanding one
level. Adds (trait_declaration body (use_declaration ...)) heritage
query so trait-uses-trait IMPLEMENTS edges are emitted at all. Fixes
3+ level trait chains silently dropping methods.
U3: parent:: bypasses composed traits
ScopeResolver gains optional buildExtendsOnlyMro hook; PHP returns
the unaugmented EXTENDS-only chain via buildPhpExtendsOnlyMro.
MethodDispatchIndex gains optional extendsOnlyMroFor accessor wired
through buildPopulatedMethodDispatch. Super-branch dispatch in
receiver-bound-calls now walks extendsOnlyMroFor when present, so
parent::method() routes to the parent class even when a composed
trait shadows the same name. Other languages leave the hook
undefined and fall back to mroFor unchanged.
U4: Namespace-aware free-call fallback
ScopeResolver gains optional isCallableVisibleFromCaller predicate;
pickUniqueGlobalCallable applies it to filter cross-namespace
candidates the caller can't reach without a use-function import.
PHP impl checks same-namespace OR explicit use-function presence,
using a side-channel namespace cache populated by
populatePhpNamespaceSiblings. Fixes the legacy DAG's namespace-
blind false-positive emissions. Existing php-calls fixture
updated: write_audit now correctly imports its targets via
use-function rather than relying on the false-positive name-only
match.
Tests:
- 175/175 PHP both flag states (REGISTRY_PRIMARY_PHP=0 and =1)
- 794/794 C#/Python/TypeScript/Go/C (no cross-language regression)
- typecheck clean
- 4 new fixtures: php-variadic-arity-minimum, php-transitive-traits,
php-parent-vs-trait, php-namespace-fallback-isolation
Plan: docs/plans/2026-05-11-001-fix-php-resolver-semantic-defects-plan.md
* fix(server): sanitize repo name to prevent argument injection
Sanitizes the extracted repository name to prevent argument injection during git clone operations and ensures compatibility with various file systems.
1. Strips leading dashes to prevent git command-line argument injection.
2. Replaces unsafe directory characters with underscores.
3. Blocks path traversal segments ('.' and '..') and Windows reserved names.
4. Fixes ReDoS vulnerability in parseRepoNameFromUrl regex.
5. Added unit tests for sanitization and path traversal edge cases.
* fix(server): expand Windows reserved name check to include extensions
- Updated sanitizeRepoName to block Windows reserved names (CON, NUL, etc.) even when they have extensions (e.g., CON.txt).
- Corrected regex and added unit tests for these edge cases to resolve CI failures on Windows.
- Ref: https://github.com/abhigyanpatwari/GitNexus/pull/1305#issuecomment-4407200914
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Adds an optional post-resolution pass on the ScopeResolver contract:
when a member-call receiver cannot be typed by the scope chain (no
TypeRef), the language may emit CALLS edges via a workspace-wide
unique-name lookup. Runs after emitReceiverBoundCalls and before
emitFreeCallFallback, gated per-language.
PHP wires the hook to recover member calls on mixed/untyped parameters
(e.g. save(mixed $entity) calling $entity->getId()), restoring parity
with the legacy DAG. Re-enables the previously skipped save → getId
test; PHP suite is now 160/160 with no skips in both flag states.
RFC #909 Ring 3 LANG-php. Third language after Python and C# to be
migrated to the scope-based resolver pipeline.
PHP module (gitnexus/src/core/ingestion/languages/php/):
- scope-resolver.ts: ScopeResolver implementation with PHP-specific
MRO builder (traits via IMPLEMENTS edges), namespace-sibling
population, arity compatibility, and merge-binding strategy
- query.ts: tree-sitter-php .scm patterns for all PHP constructs
- captures.ts: emitPhpScopeCaptures with import decomposition,
receiver-binding synthesis, arity metadata, and PHPDoc extraction
- interpret.ts: interpretPhpImport / interpretPhpTypeBinding
- import-decomposer.ts: 1:N decomposer for use/use function/use const
including grouped use declarations
- import-target.ts: PSR-4 + composer.json resolution adapter
- namespace-siblings.ts: cross-file same-namespace visibility
- arity-metadata.ts, arity.ts: PHP arity computation and compatibility
- merge-bindings.ts: PHP binding precedence (local > import > wildcard)
- receiver-binding.ts: $this / parent type-binding synthesis
- simple-hooks.ts: bindingScopeFor hoisting for return types and
constructor-promoted properties
- cache-stats.ts: tree-sitter parse cache metrics
Registry wiring:
- pipeline/registry.ts: add phpScopeResolver alongside existing Go entry
- registry-primary-flag.ts: add PHP to MIGRATED_LANGUAGES
Shared pipeline improvements (PHP-motivated, safe for all languages):
- graph-bridge/node-lookup.ts: add Trait to isLinkableLabel so PHP
and Rust MRO builders can bridge trait defs to graph node ids
- passes/free-call-fallback.ts: add arity-narrowing to
pickUniqueGlobalCallable; when multiple global candidates exist,
narrow by parameterCount/requiredParameterCount before giving up;
fixes registry-primary languages where the semantic model is empty
Test results:
- 159/160 PHP tests pass with REGISTRY_PRIMARY_PHP=1 (scope-resolver)
- 160/160 PHP tests pass with REGISTRY_PRIMARY_PHP=0 (legacy path)
- 788/788 C#/Python/TypeScript/Go tests green
- Typecheck clean
- 1 test skipped in registry-primary mode: save->getId on mixed-typed
receiver; requires a postResolutionFallback hook in the contract
(follow-up to RFC #909 Ring 3)
* fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV
tree-sitter 0.21.x on Windows crashes with SIGSEGV when parsing source
strings longer than 32 767 chars (signed 16-bit integer overflow in the
native binding). Five call sites passed raw file content without any
length guard:
- captures.ts (C# scope extraction)
- namespace-siblings.ts (extractFileStructure)
- parse-worker.ts (worker thread parse path)
- parsing-processor.ts (sequential parse fallback)
Fix: truncate at the last newline before the limit so the fragment stays
syntactically coherent. Files truncated mid-class produce ERROR roots;
captures.ts returns [] for any ERROR-root tree so the legacy DAG handles
the file silently without orphaned scope errors.
Additional C# scope fixes:
- scope-tree.ts: Module scopes may share the same range as a top-level
namespace_declaration (files with no leading `using` directives). The
rangeStrictlyContains check rejects equal ranges. Added
rangeNonStrictlyContains for Module parents.
- scope-extractor.ts: pass1BuildScopes stack-pop used strict containment;
same Module == Namespace range case caused orphaned scopes. Added
moduleAwareContains helper.
- scope-extractor-bridge.ts: empty captures from ERROR-root files still
called extractScope -> "no Module scope found" warning. Added early
return for empty/non-array captures.
- namespace-siblings.ts: three sites pushed onto binding arrays frozen by
finalize-algorithm. Fixed with spread-copy before mutation.
lbug-adapter.ts: INSTALL VECTOR in loadVectorExtension calls the KuzuDB
native extension installer, which crashes with SIGSEGV on Windows via an
unhandled error path in native code. JS try/catch cannot intercept native
signals. Skip extension loading on win32 — vector/embedding search is
unavailable on Windows but all graph index queries work correctly.
Verified on: Windows 11, Node.js 24, gitnexus 1.6.3, pcf8-game codebase
(61 757 nodes / 111 796 edges / 300 flows after fix).
* fix(windows): skip FTS extension load in pool-adapter on Windows to prevent SIGSEGV
LOAD EXTENSION fts crashes the process with SIGSEGV on Windows when the
FTS extension binary is not installed locally. This is an @ladybugdb/core
native bug — the extension loader hits an unhandled error path that raises
a native signal instead of a JS exception, so try/catch cannot protect here.
Add a process.platform === 'win32' guard in both doInitLbug and
initLbugWithDb. When skipped, bm25-index.js catches the resulting
Kuzu catalog errors (CREATE_FTS_INDEX not defined) and returns empty
BM25 results gracefully. All graph queries (cypher, context, impact)
are unaffected.
This is patch 9 of the Windows fix series for gitnexus on Windows:
patch 8 (same PR) already fixed INSTALL VECTOR SIGSEGV in lbug-adapter.ts.
pool-adapter.ts is the separate MCP-server code path that was not covered.
* fix: address codeql findings on PR #1433
The four `lastIndexOf('\n', ...)` calls were committed with a literal
newline inside the single-quoted string instead of the `\n` escape, so
the files do not parse — `tsc` and CodeQL both flagged them. Replace
the embedded newline with `'\n'`.
Also remove the two helpers that were superseded during review and
became dead code: `rangeNonStrictlyContains` in scope-tree.ts (the
equal-range carve-out is handled by `rangeStrictlyContains` +
`rangesEqual` in `canParentScope`) and `moduleAwareContains` in
scope-extractor.ts (`pass1BuildScopes` calls `canParentScope` directly).
* fix(windows): replace 32767-char truncation with chunked-input parsing
The tree-sitter 0.21.x Node binding crashes (SIGSEGV) on Windows when
parser.parse(string, ...) is handed a JS string longer than 32 767 chars.
The crash is in the bindings V8 string-to-buffer conversion and cannot
be intercepted from JS. Previous mitigation truncated source at the last
newline before that boundary, silently losing the file tail and producing
ERROR-root trees from mid-class cuts.
Switch to the callback (Parser.Input) overload via a new parseSourceSafe
helper. tree-sitter pulls source in 16 KiB chunks via repeated callback
invocations, bypassing the broken conversion path. Files are parsed in
full, no data loss, no platform-specific code path.
Removes the now-unnecessary ERROR-root short-circuit in csharp/captures.ts
and the empty-captures shim in scope-extractor-bridge.ts; both existed only
to swallow truncation-induced parse failures.
* fix(windows): cover all parse sites and correct vector-extension state
Address adversarial review on PR #1433:
1. Extend parseSourceSafe to all remaining parser.parse() call sites that
handle full file content. The first commit only converted the four
sites with active truncation hacks; cache-miss paths in
call-processor (x2), heritage-processor (x2), import-processor, and
the Go/Python/TypeScript captures + Go range-binding still called
parser.parse() directly. On Windows those would still SIGSEGV for
files > 32767 chars.
2. Stop setting vectorExtensionLoaded = true on the win32 short-circuit
in lbug-adapter.ts. The flag means "successfully loaded" and is
checked by an early-return at the top of loadVectorExtension; setting
it on the skip path made the second call return true and let
QUERY_VECTOR_INDEX run against a DB without the extension.
3. Drop the placeholder issues/... URL in the same comment.
4. Add unit tests for parseSourceSafe at boundary values: 16 KiB
(direct/callback boundary), the 32 767 Windows crash boundary,
single-line > chunk size, CRLF near boundary, and large all-Chinese
source. Confirms the callback path is correct for non-ASCII content,
which is also exercised by the existing csharp-captures large-file
test.
Researched the chunking concern: tree-sitter Node binding sets
TSInputEncodingUTF16 and divides byte_index by 2 in ByteCountToJS before
calling the JS callback, so the index argument is a UTF-16 code-unit
offset — matching String.prototype.slice. Splitting tokens across chunks
is safe by API contract; the lexer is chunk-agnostic.
* fix(windows): extend parseSourceSafe to group/embeddings + lint enforcement
Closes the remaining Windows SIGSEGV exposure flagged by the Codex
adversarial review on PR #1433. Six pre-existing parser.parse(content)
call sites bypassed parseSourceSafe and could crash the process on
Windows when a contract IDL, route file, or embedding-target source
exceeded 32 767 chars. Adds a lint rule so the regression vector closes
permanently.
Production code:
- Relocate parseSourceSafe from ingestion/utils/ to core/tree-sitter/
so group/ and embeddings/ can import without crossing into ingestion
internals. core/tree-sitter/ already houses parser-loader.ts and is
the natural shared facade. All 11 existing importers updated; no shim
left behind in the old location.
- Route through parseSourceSafe in 5 group extractors (grpc, thrift,
http-route, include, tree-sitter-scanner) and the embeddings
ensureAndParse helper.
- The seventh direct .parse() call in grpc-patterns/proto.ts:49 is a
module-load grammar smoke test parsing a 36-char literal. Trivially
safe by inspection, intentionally direct, filtered out by the lint
rule via the string-literal-arg skip.
Tests:
- 5 caller-side regression tests with a vi.spyOn assertion on
parseSourceSafe. The spy is what catches a regression: parser.parse
on a 40 000-char input succeeds on Linux/macOS, so a "no throw"
assertion alone would silently pass with the bypass reintroduced.
- The vi.mock boilerplate is centralised in
gitnexus/test/helpers/parse-source-safe-mock.ts, dynamic-imported
inside each mock factory so vitest's hoister does not race the
static import binding.
Lint:
- New custom ESLint rule gitnexus/require-safe-parse, scoped to
gitnexus/src/core/**, fails on direct <parser>.parse(<non-literal>,
...) calls and auto-fixes them to parseSourceSafe(<parser>, ...).
Skips JSON/URL/marked/Number/Math, string-literal first args
(smoke tests), test files, and the helper itself. Auto-fix rewrites
the call site only; the developer adds the import after tsc
surfaces the missing identifier — same tradeoff as
unused-imports/no-unused-imports.
Plan: docs/plans/2026-05-10-001-fix-windows-parse-safety-group-and-embeddings-plan.md
* fix(test): use mkdtempSync in http-route-extractor regression test
Address CodeQL js/insecure-temporary-file warning on the new Windows-
SIGSEGV regression test. The test was using path.join(tmpDir, "large-input")
which, when nested inside a Date.now()-based parent tmpDir, lets CodeQL flag
the directory as a predictable-name temp file with race-condition risk.
Switch to fs.mkdtempSync(path.join(tmpDir, "large-input-")) so the suffix
is a secure unique random string.
---------
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
* feat(cursor): upgrade hooks to Cursor 2.4 postToolUse for Read/Grep/Shell 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>
* fix(cursor): hook timeout is in seconds, not milliseconds
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>
* fix(cursor): address Claude review findings — payload aliases, debug, 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>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
* ci(release): skip rc build on release PRs
Suppress the auto-fired Release Candidate workflow when:
1. The HEAD commit subject matches `chore: release vX.Y.Z` (the canonical
release-PR title), or
2. The squash-merged PR carries the `release` label.
Either match short-circuits the guard to should_run=false. This prevents the
rc cycle from racing publish.yml on the v-tag (as happened on v1.6.4 where
we had to manually cancel the auto-fired RC run after merging PR #1473).
Adds pull-requests: read to the guard job for the label lookup. A failed
gh API call falls through to the existing dedup logic rather than silently
suppressing rc builds.
* ci(release): address PR #1474 review — anchor regex + sanitise log echo
Two minor follow-ups from Claude's review:
1. End-anchor the release-subject regex. The previous shape
^chore: release vX.Y.Z would match noisy variants like
chore: release v1.0.0 (something unrelated). The new shape
requires either the bare title or the canonical squash-merge
(#NNNN) suffix exactly.
2. Sanitise HEAD_SUBJECT before echoing to logs. git %s strips
newlines so LF injection is impossible, but a hypothetical
subject containing ::error:: or ::set-output:: could otherwise
forge GitHub Actions annotation entries. Defence-in-depth.
Both findings flagged minor / does not block merge — applying
anyway since they are trivial.
* test(u8): de-flake regex linearity assertions
The single-trial 2x input + 3x ratio bound was razor-thin: a real macOS
CI run failed at ratio 3.01x with small=7.41ms / large=22.31ms - both
above the 5ms noise floor but close enough that single-shot scheduler
jitter pushed the ratio over.
Replace the methodology with four stacked techniques:
1. Warmup runs before timing (let the JIT tier up)
2. Median of 5 trials per measurement (eliminates GC + jitter)
3. 4x input ratio (was 2x) - linear gives ~4x, O(n^2) gives ~16x
4. 8x ratio bound with a 20ms noise floor on the LARGE measurement
Headroom: linear is expected at ~4x, bound is 8x = 2x safety margin.
A real O(n^2) regression on a 4x input would clock 16x, well outside.
Catastrophic backtracking is still caught by the absolute <500ms cap.
Verified: 10 consecutive local runs all passed.
* test(u8): address PR #1475 review — tighten floor + rename for accuracy
Two follow-ups from Claude's review:
1. Floor semantics: revert to 'skip when BOTH measurements below floor'
(AND, not single-check) and lower threshold from 20ms back to 5ms.
Median-of-5 makes 5ms reliably resolvable above performance.now()'s
~10-100us band, so the higher floor was unnecessary defense.
Closes the gap where an O(n^2) regression on a fast runner could
stay under 500ms AND below 20ms-large to escape both detectors.
2. Rename assertSubLinearRatio -> assertNearLinearScaling. The bound
is SIZE_RATIO * 2 = 8x on a 4x input = sub-quadratic with 2x
headroom over linear, not strict sub-linearity. New name reflects
the actual semantics.
