readme#3
Merged
Merged
Conversation
5 tasks
This was referenced Mar 11, 2026
Merged
github-actions Bot
pushed a commit
that referenced
this pull request
Mar 18, 2026
Feat/dnn cobol increase trhe beast
4 tasks
6 tasks
9 tasks
This was referenced Mar 25, 2026
magyargergo
added a commit
that referenced
this pull request
Mar 25, 2026
Fourth review findings fixed:
- New #IV: PERFORM TIMES guard uses perfMatch.index instead of
line.indexOf (prevents wrong match when target appears earlier in line)
- New #V: 88-level condition values now handle single-quoted literals
('Y' no longer stored with embedded quotes)
- New #I: CANCEL edges use two-pass resolution like CALL (no longer
silently dropped when target indexed after source)
- New #3: Multi-line SORT/MERGE accumulation — sortAccum state variable
accumulates lines until period, then extracts USING/GIVING from full
statement (95% of production SORT statements span multiple lines)
- New #II: PROCEDURE DIVISION USING on split lines — pendingProcUsing
flag defers parameter capture to next line if USING not on same line
- New #6 (prior): EXCLUDED_PARA_NAMES exact match for SECTION/DIVISION
Updated fixture: RPTGEN.cbl SORT now uses multi-line format with GIVING
on separate line (period-terminated). New sort-giving integration test.
ACCESSES total: 18 → 19 (new sort-giving edge from multi-line capture).
magyargergo
added a commit
that referenced
this pull request
Mar 26, 2026
…ti-CALL gaps #1: scopedCallerLookup(null) now uses owningModuleId(lineNum) instead of parentId, fixing PERFORM/MOVE/GOTO before first paragraph in nested programs. #2+#3: CALL and CANCEL extraction now uses matchAll (global flag) to capture multiple occurrences on the same line. Dynamic CALL/CANCEL checked independently instead of in else branch. #4: SORT/MERGE ACCESSES edge IDs now use owningModuleId(sort.line) instead of parentId for nested program correctness. #5: preprocessCobolSource free-format detection now uses first 10 lines (consistent with extractCobolSymbolsWithRegex threshold). #6: EXCLUDED_PARA_NAMES expanded with DISPLAY, ACCEPT, WRITE, READ, REWRITE, DELETE, OPEN, CLOSE, RETURN, RELEASE, SORT, MERGE to prevent false-positive paragraph detection on isolated verbs. Also removed unused GraphNode import from cobol-processor.ts. Tests: 145 passing | TypeScript clean
6 tasks
magyargergo
added a commit
that referenced
this pull request
Mar 26, 2026
…g anchors #1+#2: Removed incorrect hasQuotedCall/hasQuotedCancel deduplication guards. RE_CALL_DYNAMIC and RE_CANCEL_DYNAMIC require [A-Z] after CALL/CANCEL, so they CANNOT match quoted targets — the guards were both unnecessary and actively harmful, suppressing dynamic CALL/CANCEL in ON EXCEPTION patterns. #3+#5: Changed RE_CALL_DYNAMIC and RE_CANCEL_DYNAMIC trailing anchor from (?:\s|\.) to (?=\s|\.|$) (lookahead). The consuming anchor failed when the identifier was the last token on a physical line. Tests: 168 passing | TypeScript clean
magyargergo
added a commit
that referenced
this pull request
Mar 26, 2026
…coping #1: CICS LOAD unresolved edge no longer silently deleted in second pass. Changed narrow cics-link/cics-xctl check to catch-all pattern: rel.reason?.startsWith('cics-') && rel.reason.endsWith('-unresolved') #2: flushCallAccum USING extraction now stops before COBOL statement verbs (INSPECT, SEARCH, SORT, MERGE, DISPLAY, ACCEPT, MOVE, PERFORM, GO TO, CALL, IF, EVALUATE). Prevents absorbing adjacent statements as false USING parameters in legacy pre-COBOL-85 code without END-CALL. #3: CICS FILE Record nodes now globally-scoped (<cics-file>:FILENAME) instead of per-file-scoped. Enables cross-program CICS file access analysis, consistent with SQL table scoping (<db>:TABLE). #4: callAccum pre-check regex now has (?<![A-Z0-9-]) lookbehind to prevent false activation on compound identifiers like WS-CALL-FLAG. Tests: 168 passing | TypeScript clean
magyargergo
added a commit
that referenced
this pull request
Mar 26, 2026
#1: flushCallAccum now extracts CANCEL statements from within CALL ON EXCEPTION blocks. Adds RE_CANCEL + RE_CANCEL_DYNAMIC matchAll passes alongside existing CALL extraction. #2: Added \bCANCEL(?=\s|$) to USING lookahead regex to prevent CANCEL keyword being captured as false USING parameter. #3: Multi-line CALL start now returns immediately to prevent the CALL start line from simultaneously feeding sortAccum/inspectAccum. #6: Division transitions now flush all active accumulators (callAccum, sortAccum, inspectAccum) to prevent state leakage across programs. Also added CANCEL to callAccum flush trigger verb list. Tests: 215 passing | TypeScript clean
magyargergo
added a commit
that referenced
this pull request
Mar 26, 2026
Fixes: #1: END PROGRAM flushes callAccum/sortAccum/inspectAccum #2: PROGRAM-ID sibling path flushes all accumulators #3: Added COMPUTE/ADD/SUBTRACT/MULTIPLY/DIVIDE/STRING/UNSTRING to COBOL_STATEMENT_VERBS (now 32 verbs) Tests (15 new): - END PROGRAM flush: single + nested programs (2) - PROGRAM-ID sibling flush (1) - Arithmetic verb flush: COMPUTE/ADD/SUBTRACT/MULTIPLY/DIVIDE (5) - String verb flush: STRING/UNSTRING (2) - Arithmetic not captured as false USING params (1) - SORT flushed at END PROGRAM (1) - INSPECT flushed at END PROGRAM (1) - All with exact toBe assertions (2) Total: 239 tests passing | Zero fuzzy assertions
magyargergo
added a commit
that referenced
this pull request
Mar 26, 2026
* feat: add COBOL language support with regex extraction pipeline Standalone COBOL processor following the markdown-processor.ts pattern: - No LanguageProvider modification — COBOL uses regex, not tree-sitter - No SupportedLanguages enum change — standalone processor pattern New files: - cobol-processor.ts — orchestrator (processCobol, isCobolFile, isJclFile) - cobol/cobol-preprocessor.ts — regex state machine extraction (~888 LOC) - cobol/cobol-copy-expander.ts — COPY statement expansion with circular detection - cobol/jcl-parser.ts — JCL job/step/DD extraction - cobol/jcl-processor.ts — JCL graph node creation Extraction produces: - Module nodes (PROGRAM-ID) - Function nodes (paragraphs) - Namespace nodes (sections) - Property nodes (data items) - CALLS edges (PERFORM intra-file, CALL cross-program) - IMPORTS edges (COPY statements) - CONTAINS edges (section → paragraph hierarchy) Pipeline integration: single processCobol() call in Phase 2.6 54 new tests (33 COBOL + 21 JCL), all 3889 tests pass. * docs: document custom processor pattern in pipeline.ts Add comment block at the custom processor integration point documenting the pattern for future non-tree-sitter language additions. * feat(cobol): enrich graph with EXEC SQL/CICS, ENTRY points, MOVE data flow, PERFORM THRU Maps the remaining 60% of CobolRegexResults to the graph: - EXEC SQL blocks → CodeElement nodes + ACCESSES edges to DB tables - EXEC CICS LINK/XCTL → CodeElement nodes + cross-program CALLS edges - ENTRY points → Constructor nodes (registered for cross-program resolution) - MOVE statements → ACCESSES edges (read/write data flow tracking) - PERFORM THRU → expanded CALLS edges for range targets - File declarations → Record nodes with assignment metadata - Cross-program CALL 2nd pass: resolves unresolved targets after all programs processed * test(cobol): add 26 integration tests with exact assertions + fix CICS resolution bug Integration tests (test/integration/resolvers/cobol.test.ts): - 26 tests covering full COBOL system extraction - ALL assertions use exact toBe(N) — zero fuzzy assertions - Fixtures: CUSTUPDT.cbl, AUDITLOG.cbl, CUSTDAT.cpy, RPTGEN.cbl, RUNJOBS.jcl Bug fix (cobol-processor.ts): - CICS LINK/XCTL cross-program resolution was broken — edges were created with "resolved" reason but pointing to <unresolved> targets - Fix: use cics-link-unresolved / cics-xctl-unresolved suffix pattern matching the existing cobol-call-unresolved pattern - Second-pass resolver now patches both CALL and CICS unresolved edges All 3915 tests pass, 0 failures. * test(cobol): exhaustive 57-test suite with strict exact assertions Complete rewrite of COBOL integration tests using ground-truth approach: dump the full graph, then assert EVERY node and EVERY edge. 57 tests across 9 sections: - Node completeness: Module(3), Function(13), Namespace(2), Property(21), Record(1), CodeElement(8), Constructor(1) — exact sorted arrays - Edge completeness: 22 tests covering every type+reason combination with exact source→target pairs - Cross-program resolution: 6 tests verifying CALL, CICS LINK/XCTL, JCL - COPY expansion: copybook data items in RPTGEN - Section hierarchy: exact paragraph membership per section - Data item ownership: exact per-module breakdown - MOVE data flow: exact read/write pairs - JCL integration: job/step/dataset containment - Grand totals: CALLS(22), CONTAINS(48), IMPORTS(1), ACCESSES(7) Fixture enhancements: - CUSTUPDT.cbl: added INIT-SECTION + PROCESSING-SECTION, PERFORM THRU - AUDITLOG.cbl: added ENTRY "AUDITLOG-BATCH" - RPTGEN.cbl: added EXEC CICS XCTL Zero fuzzy assertions — every expect uses toBe(N) or toEqual([...sorted]). * fix(cobol): add removeRelationship API + single-quote CALL/COPY/ENTRY, PERFORM keyword skip Phase 0A: Add removeRelationship(id) to KnowledgeGraph interface and implementation (trivial Map.delete wrapper). Required for orphan edge cleanup in next commit. Phase 1A (from PR #500 review, modified): - RE_CALL and RE_COPY_QUOTED now match both "double" and 'single' quotes - parseSingleCopyStatement in copy-expander updated for single quotes - PERFORM_KEYWORD_SKIP set prevents UNTIL/VARYING/WITH/TEST/FOREVER from being stored as false-positive perform targets - Sequence number stripping uses /[^0-9 ]/ (preserves numeric seq numbers unlike PR #500's /\S/ which stripped them) - Normalized || to ?? for regex group extraction in copy-expander 5 new graph unit tests, all 57 COBOL integration tests pass. * fix(cobol): RE_ENTRY single-quote + remove orphan unresolved CALLS edges Phase 1B: RE_ENTRY regex now supports both "double" and 'single' quoted ENTRY targets. Uses named intermediates (entryName, usingClause) with ?? operator. USING capture group shifted from [2] to [3]. Phase 1C: Second-pass resolution now collects resolved orphan edge IDs during iteration and removes them after the loop completes, using the new graph.removeRelationship() API. Graph no longer contains phantom <unresolved>: edges alongside their resolved replacements. CALLS count drops from 22 to 18 (4 orphan edges removed). * fix(cobol): Property ID collisions + O(1) Map lookup for MOVE edges Phase 1D+3C (atomic): Property node IDs now use composite key filePath:section:level:name instead of filePath:name. This prevents duplicate data item names in different sections (e.g., STATUS in both WORKING-STORAGE and LINKAGE) from silently colliding. New generatePropertyId() helper ensures both node creation and MOVE edge lookup use the identical key formula. buildDataItemMap() replaces the O(n) findDataItemNode linear scan with O(1) Map lookup, built once per file before MOVE processing. * feat(cobol): MOVE multi-target extraction with OF/IN qualifier filtering MOVE X TO A B C now produces write edges for all targets, not just the first. extractMoveTargets() helper handles OF/IN qualified names (WS-NAME OF WS-RECORD -> target is WS-NAME), subscript stripping (WS-TABLE(I) -> WS-TABLE), and MOVE_SKIP filtering on targets. Data model: CobolRegexResults.moves.to:string -> targets:string[] MOVE CORRESPONDING stays single-target per COBOL standard. Processor MOVE loop now iterates move.targets. * feat(cobol): COPY IN/OF library, pseudotext REPLACING, dynamic CALL, PERFORM TIMES, CICS MAP unquoted Phase 2B: COPY ... IN/OF library-name now captured as metadata in CopyResolution (IN and OF are synonyms per COBOL-85 standard). Phase 2C: COPY REPLACING ==pseudotext== support. Tokenizer handles ==...== delimiters alongside "quoted" strings. Pseudotext forces EXACT type. Two-pass applyReplacing: first pass handles space-containing/ non-identifier pseudotext via global string replace; second pass handles identifier-level LEADING/TRAILING/EXACT. New test file cobol-copy-expander.test.ts with 10 tests. Phase 2E: PERFORM WS-COUNT TIMES no longer produces a false-positive perform target (checks for TIMES keyword after captured identifier). Phase 2F: Dynamic CALL via data item (CALL WS-PROG-NAME without quotes) now emits a CodeElement annotation node with description 'dynamic-call' instead of silently ignoring. Adds isQuoted:boolean to call results. Phase 3A: CICS MAP(WS-MAP-NAME) unquoted identifiers now captured. Phase 3B: Normalized || to ?? in copy-expander (done in Phase 1A). * feat(cobol): nested program support — capture multiple PROGRAM-IDs per file Phase 2D: The state machine now captures all PROGRAM-IDs, not just the first. The primary program name stays in programName; additional nested programs go into nestedPrograms[]. The processor creates separate Module nodes for each nested program, contained by the outer module, and registers them in moduleNodeIds for cross-program CALL resolution. Paragraphs/data items are not yet scoped per-program (attributed to the outer module) — full per-program scoping is a future enhancement that requires END PROGRAM boundary tracking in the state machine. * test(cobol): expand integration tests for all new language features New fixtures: - NESTED.cbl — two PROGRAM-IDs (OUTER-PROG, INNER-PROG) for nested program support testing - COPYLIB.cpy — copybook for pseudotext REPLACING test target Modified fixtures: - CUSTUPDT.cbl — single-quoted ENTRY 'ALTENTRY', multi-target MOVE (WS-AMT TO FIELD-A FIELD-B), dynamic CALL WS-PROG-NAME, COPY COPYLIB with pseudotext REPLACING, LINKAGE SECTION with LS-PARAM - RPTGEN.cbl — PERFORM WS-COUNT TIMES (false-positive guard), unquoted MAP(WS-MAP-NAME), additional data items WS-COUNT WS-MAP-NAME Integration test rewritten with 62 exact assertions covering: - 5 Module, 17 Function, 33 Property, 9 CodeElement, 2 Constructor nodes - Nested program containment (OUTER-PROG -> INNER-PROG) - Dynamic CALL annotation (CodeElement with cobol-dynamic-call) - Multi-target MOVE (UPDATE-BALANCE: 2 reads, 3 writes) - Single-quoted ENTRY (ALTENTRY under CUSTUPDT) - PERFORM TIMES guard (WS-COUNT not in CALLS) - Orphan unresolved edge removal (zero -unresolved edges) - Grand totals: 21 CALLS, 68 CONTAINS, 2 IMPORTS, 10 ACCESSES * fix(cobol): pseudotext REPLACING now applies correctly via isPseudotext flag Root cause: ==PREFIX-== matched /^[A-Z][A-Z0-9-]*$/i (trailing hyphens allowed), routing it to the second-pass EXACT identifier match where PREFIX-RECORD !== PREFIX- failed silently. Fix: Propagate isPseudotext from parseReplacingClause to CopyReplacing interface, then use it in applyReplacing first-pass condition to force global string replacement for all pseudotext entries regardless of whether the content looks like an identifier. Result: COPY COPYLIB REPLACING ==PREFIX-== BY ==WS-==. now correctly transforms PREFIX-RECORD → WS-RECORD, PREFIX-CODE → WS-CODE, etc. * refactor(cobol): per-program scoping via boundary tracking + line-range grouping State machine changes (minimal, ~30 lines): - Add RE_END_PROGRAM regex for END PROGRAM program-name. detection - Replace nestedPrograms[] with programs[] containing startLine/endLine/ nestingDepth metadata for each PROGRAM-ID in the file - Reset division/section/paragraph state on new PROGRAM-ID boundary - EOF finalization flushes remaining stack entries (single-program files) - Programs sorted by startLine (outer before inner) Processor changes: - Uses programs[] with line-range containment to find enclosing parent Module for nested programs (replaces hardcoded nestedParent logic) - programModuleIds Map tracks Module node IDs per program name Fixture: NESTED.cbl now includes END PROGRAM lines for both programs. Integration test: PREFIX-* Property nodes now correctly appear as WS-* after the pseudotext REPLACING fix from the previous commit. * feat(cobol): free-format COBOL support (>>source free) Auto-detects >>SOURCE FREE directive in the first 500 chars and switches to free-format line processing: - No column-position rules (cols 1-6 are program text, not sequence area) - Comments use *> prefix instead of col 7 indicator - No continuation line indicator - Strip inline *> comments - Skip >>SOURCE directive lines preprocessCobolSource() skips col-1-6 stripping for free-format files. Paragraph/section regexes relaxed from fixed 7-space prefix to flexible whitespace with case-insensitivity (/^\s*([A-Z][A-Z0-9-]+)\.\s*$/i). EXCLUDED_PARA_NAMES expanded with COBOL verbs (GOBACK, END-READ, etc.) to prevent false-positive paragraph detection in free-format. Also fixes: entry-point-scoring.ts crash when language is 'cobol' (MERGED_ENTRY_POINT_PATTERNS[language] was undefined → optional chaining). Benchmark on ACAS 3.01 (268 GnuCOBOL free-format programs, 10MB): - Before: 407 nodes, 393 edges (near-empty, only file nodes) - After: 4,297 nodes, 3,612 edges, 542 clusters, 11 flows * fix(cobol): relax data item regexes for free-format (^\s+ to ^\s*) RE_FD, RE_DATA_ITEM, RE_ANONYMOUS_REDEFINES, and RE_88_LEVEL all used ^\s+ which requires at least 1 leading space. In free-format mode, lines are trimmed before processing, so data items like "01 WS-FIELD PIC X." have no leading whitespace after trimming. Changed to ^\s* (zero or more spaces) which works for both fixed-format (indented lines still have spaces) and free-format (trimmed lines). ACAS benchmark (268 GnuCOBOL programs): - Before: 4,297 nodes, 3,612 edges (paragraphs only) - After: 13,832 nodes, 8,615 edges (+ data items, FDs, 88-levels) * feat(cobol): 100% structural feature coverage — GO TO, SCREEN, SD/RD, SORT, SEARCH, CANCEL, Level 66 New extractions: GO TO (CALLS edges), SCREEN SECTION data items, SD/RD alongside FD (Record nodes), SORT/MERGE USING/GIVING (ACCESSES), SEARCH (ACCESSES), CANCEL (CALLS), Level 66 RENAMES (Property), IS EXTERNAL/IS GLOBAL (Property description enrichment). ACAS: 13,951 nodes | 13,193 edges | 685 clusters | 150 flows (+53% edges from new GO TO/SORT/SEARCH/CANCEL extractions) * feat(cobol): enriched CICS extraction — file I/O, dynamic PROGRAM, queues, HANDLE ABEND EXEC CICS blocks now extract: - FILE/DATASET clause: captures VSAM file name (literal or data item ref) for READ/WRITE/REWRITE/DELETE/STARTBR/READNEXT/READPREV → ACCESSES edges - PROGRAM clause: now handles unquoted variable references (dynamic CICS program transfer) → CodeElement annotation with cics-dynamic-program reason - QUEUE clause: captures TS/TD queue names from WRITEQ/READQ → ACCESSES edges - LABEL clause: captures HANDLE ABEND error handler targets → CALLS edges - TRANSID: now handles unquoted variable references CodeElement descriptions enriched with all captured fields (map, program, transid, file, queue, label). CardDemo benchmark: +49 nodes, +33 edges from enriched CICS extraction. * feat(cobol): complete CICS command extraction — all 7 expert recommendations From COBOL expert agent analysis: 1. ENDBR added to isRead file command list 2. LOAD added to PROGRAM edge commands (alongside LINK/XCTL) 3. Two-word commands expanded: WRITEQ/READQ/DELETEQ TS/TD, HANDLE ABEND/AID/CONDITION, START TRANSID 4. Queue reason differentiated: cics-queue-read/-write/-delete 5. RETURN/START TRANSID → CALLS edges to synthetic <transid> target 6. MAP → ACCESSES edges for screen traceability 7. INTO/FROM data fields extracted → ACCESSES edges to data items Also: dataItemMap built before CICS block processing (was declared after), CodeElement descriptions enriched with all captured CICS fields. * test(cobol): strict exhaustive integration tests with exact edgeSet assertions Every edge reason has exact sorted pair assertions via edgeSet(), not just counts. Any change to extraction that adds, removes, or reorders edges will produce a precise, descriptive failure. Updated RPTGEN.cbl fixture with: - GO TO EXIT-PARAGRAPH, SORT USING/GIVING, SEARCH table - EXEC CICS READ FILE INTO, WRITEQ TS QUEUE FROM, SEND MAP FROM - EXEC CICS HANDLE ABEND LABEL, RETURN TRANSID, XCTL PROGRAM(variable) - ABEND-HANDLER and EXIT-PARAGRAPH paragraphs 46 tests covering 24 CALLS + 79 CONTAINS + 18 ACCESSES + 2 IMPORTS edges across 15 distinct edge reason codes, all with exact sorted pair lists. * fix(cobol): address 5 findings from second Claude review (compiler front-end perspective) Finding #2: Numeric sequence numbers now stripped (changed /[^0-9 ]/ to /\S/ in preprocessCobolSource). Lines like "000100 MAIN-PARAGRAPH." now have cols 1-6 blanked so paragraph regex matches correctly. Finding #11: JCL in-stream PROC ordering fixed — pre-register all PROCs into moduleNames before step processing. Steps that EXEC a PROC defined later in the same file now get CALLS edges. Finding #A: PROCEDURE DIVISION USING no longer captures calling-convention keywords (BY, VALUE, REFERENCE, CONTENT, ADDRESS, OF) as parameter names. Finding #C: SORT/MERGE USING/GIVING now captures ALL file references (multi-file), not just the first. Changed from single-match to section extraction with split. Finding #D: Section headers no longer set currentParagraph, preventing PERFORM caller misattribution to Namespace instead of Function nodes. * fix(cobol): address code review findings — ReDoS fix, perf, cleanup P1 CRITICAL — ReDoS in SORT USING/GIVING: Replaced nested-quantifier regex with safe indexOf+substring+split approach. No backtracking possible on crafted input. P2 — readCopy O(M) linear scan: Added copybookByPath reverse Map for O(1) path-to-content lookup. P3 — Dead code removal: Deleted unused RE_SORT_USING and RE_SORT_GIVING constants. P3 — EXCLUDED_PARA_NAMES simplification: Replaced 20 END-* entries with startsWith('END-') prefix check. Auto-covers future END-* verbs. P3 — Misplaced JSDoc on removeRelationship: Fixed comment that described removeNodesByFile instead. Added missing JSDoc to removeNodesByFile. Review agents: architecture-strategist, performance-oracle, security-sentinel, code-simplicity-reviewer. * refactor: add Cobol to SupportedLanguages with parseStrategy: standalone New languages/cobol.ts — standalone regex processor provider with no-op tree-sitter fields. Declares parseStrategy: 'standalone' to distinguish from tree-sitter-based languages. Added parseStrategy: 'tree-sitter' | 'standalone' to LanguageProviderConfig for languages that use their own processor instead of tree-sitter. Removed all 11 'cobol' as any casts — now uses SupportedLanguages.Cobol. Added empty Cobol entries to entry-point-scoring and framework-detection. * fix(cobol): 5 fixes from third Claude review + 3 regression tests Fixes: - Line numbers now 1-indexed in fixed-format (was 0-indexed, off-by-one in jump-to-definition links) - Copybook content preprocessed before COPY expansion (sequence numbers and patch markers in copybooks no longer survive into expanded source) - ENTRY USING filters calling-convention keywords (BY, VALUE, REFERENCE, CONTENT, ADDRESS, OF) — same fix as PROCEDURE DIVISION USING - SORT/MERGE trailing period stripped from USING/GIVING file tokens - Paragraph exclusion uses exact match for SECTION/DIVISION (was substring match that excluded valid names like CROSS-SECTION-ANALYSIS) USING_KEYWORDS moved to module scope for reuse by both PROCEDURE DIVISION USING and ENTRY USING handlers. New unit tests: - ENTRY USING BY VALUE filtering - Paragraph names containing SECTION not excluded - Numeric sequence numbers stripped enabling paragraph detection * fix(cobol): address 6 findings from fourth Claude review + tests Fourth review findings fixed: - New #IV: PERFORM TIMES guard uses perfMatch.index instead of line.indexOf (prevents wrong match when target appears earlier in line) - New #V: 88-level condition values now handle single-quoted literals ('Y' no longer stored with embedded quotes) - New #I: CANCEL edges use two-pass resolution like CALL (no longer silently dropped when target indexed after source) - New #3: Multi-line SORT/MERGE accumulation — sortAccum state variable accumulates lines until period, then extracts USING/GIVING from full statement (95% of production SORT statements span multiple lines) - New #II: PROCEDURE DIVISION USING on split lines — pendingProcUsing flag defers parameter capture to next line if USING not on same line - New #6 (prior): EXCLUDED_PARA_NAMES exact match for SECTION/DIVISION Updated fixture: RPTGEN.cbl SORT now uses multi-line format with GIVING on separate line (period-terminated). New sort-giving integration test. ACCESSES total: 18 → 19 (new sort-giving edge from multi-line capture). * fix(cobol): address 4 findings from fifth Claude review Finding #B (5 reviews old): Section/paragraph node IDs now include enclosing program name to prevent collision when nested programs share section/paragraph names. New findOwningProgramName() helper uses programs[] line ranges to find the innermost enclosing program. Finding #α: pendingProcUsing now reset in the if(procUsingMatch) branch (was only set in else branch, could leak across nested programs). Finding #β: RE_CALL_DYNAMIC uses negative lookbehind (?<![A-Z0-9-]) to prevent false-positive on compound identifiers like WS-CALL OCCURS. Finding #γ: sortAccum flushed at EOF (parallel to flushSelect and pendingFdName EOF cleanup). Prevents silent loss of SORT USING/GIVING relationships in truncated files. * fix(cobol): address findings from reviews 5+6 with full test coverage Review 5 fixes: - #α: pendingProcUsing reset in if(procUsingMatch) branch - #β: RE_CALL_DYNAMIC negative lookbehind prevents WS-CALL false positive - #γ: sortAccum flushed at EOF for truncated files - #B: Section/paragraph IDs include owning program name Review 6 fixes: - #P: sectionNodeIds/paraNodeIds maps use program-scoped keys (PROGNAME:NAME). New scopedParaLookup/scopedCallerLookup helpers. findContainingSection updated with programs parameter. - #Q: RETURNING added to USING_KEYWORDS for COBOL 2002+ - #R: RE_PERFORM matches both THRU and THROUGH via alternation New unit tests (6): - PERFORM THROUGH captures thruTarget - PROCEDURE DIVISION USING RETURNING filters keyword - RE_CALL_DYNAMIC no false-match on WS-CALL compound identifier - Multi-line SORT captures USING/GIVING from continuation lines - PROCEDURE DIVISION USING on split line via pendingProcUsing - Copybook preprocessing strips sequence numbers * fix(cobol): address findings from seventh Claude review + 3 tests Review 7 fixes: - #i: findContainingSection only updates best when lookup succeeds (prevents undefined overwriting valid parent section) - #ii: RE_PROC_SECTION handles segment numbers (SECTION 30.) - #III: procedureUsing now stored per-program on boundary stack entries, propagated to programs[] output. Inner programs no longer overwrite outer program's parameters. - #δ: Dynamic CANCEL (CANCEL variable) now creates CodeElement annotation node, matching dynamic CALL behavior. RE_CANCEL_DYNAMIC with negative lookbehind. cancels[] gains isQuoted field. - #Q: RETURNING added to USING_KEYWORDS (already in prev commit) - #R: PERFORM THROUGH already fixed (THRU|THROUGH alternation) New unit tests: - Nested programs carry per-program procedureUsing - SECTION with segment number detected - Dynamic CANCEL via data item captured with isQuoted=false * feat(cobol): link PROCEDURE DIVISION USING to LINKAGE data items + close 4 findings Finding #10 FIXED: procedureUsing parameters now create ACCESSES edges with reason 'cobol-procedure-using' from Module to matching LINKAGE SECTION Property nodes. This exposes the program's parameter contract in the graph (e.g., AUDITLOG → LS-CUST-ID, AUDITLOG → LS-AMOUNT). Findings closed by expert agent consensus: - #6 COPY IN library: WONTFIX — captured metadata, no universal library-to-directory mapping exists. Field costs nothing and is useful for library queries. - #14 SQL DELETE: WONTFIX — DB2 requires FROM; existing FROM pattern handles it. Bare DELETE would risk false positives. - #E OCCURS DEPENDING ON: WONTFIX — runtime sizing concern, not structural. The static occurs count is sufficient for indexing. All 39 findings from 7 Claude reviews now resolved or closed. * fix(cobol): resolve 48 review findings across 9 review cycles Ninth deep review resolved all remaining COBOL parser gaps identified by 5 specialist agents (COBOL expert, architecture strategist, TypeScript reviewer, security sentinel, code simplicity reviewer). Fixes (P1 — critical): - SELECT OPTIONAL now correctly skips OPTIONAL keyword (C1) - RETURNING params excluded from PROCEDURE DIVISION USING list (C7) - SORT GIVING no longer captures clause keywords as file names (C5) - Extract flushSort() helper eliminating 40-line duplication (S2) - Flush unclosed EXEC blocks at EOF matching SORT/SELECT pattern (S3) - Guard undefined map key in jcl-processor moduleNames (S1) - Add MAX_TOTAL_EXPANSIONS=500 to prevent exponential COPY breadth (S4) Fixes (P2 — important): - Quote-aware stripInlineComment for | and *> in string literals (C2+C3) - Fixed-format literal continuation now handles quoted strings (C6) - PROGRAM-ID detected regardless of division state for siblings (C9) Fixes (P3 — cleanup): - EXEC SQL INTO restricted to INSERT INTO to avoid FETCH false-pos (C8) - Copy expander line numbers fixed from 0-based to 1-based (C11) - Remove dead code: inInStreamProc, fileIsLiteral, expansionDepth (S7-S10) Also fixes 8th-review findings: nested program CONTAINS attribution, multi-PERFORM on same line, INPUT/OUTPUT PROCEDURE IS in SORT, GO TO DEPENDING ON multi-target, MOVE CORR abbreviation, per-program procedureUsing ACCESSES edges. Tests: 145 COBOL tests passing (59 integration + 86 unit) Benchmarks: CardDemo 12,323 nodes/8,893 edges (7.4s) ACAS 14,016 nodes/15,452 edges (9.3s, -9% faster) * docs(cobol): update documentation for ninth review cycle fixes Update all 4 COBOL documentation files to reflect the 16 fixes from the ninth review cycle: - regex-extraction.md: quote-aware comment stripping, SELECT OPTIONAL, RETURNING exclusion, SORT_CLAUSE_NOISE filter, flushSort() helper, GO TO multi-target, PROGRAM-ID division-independent detection - copy-expansion.md: MAX_TOTAL_EXPANSIONS=500 breadth guard, 1-based line numbers, removed expansionDepth/warnedCircular param - deep-indexing.md: GO TO DEPENDING ON, INPUT/OUTPUT PROCEDURE IS, MOVE CORR edge reasons, INSERT INTO restriction, literal continuation - performance.md: updated benchmarks (CardDemo 12,323n/8,893e/7.4s, ACAS 14,016n/15,452e/9.3s), COPY breadth guard * fix(cobol): resolve 10th review findings — nested program edge attribution Fix 6 findings from the 10th review (PR #498 comment #4132201110): #A+#F: All CALL/CANCEL/CICS/ENTRY/SQL/SEARCH/file-declaration edges now use owningModuleId() for nested program attribution instead of the outer program's parentId. Added helper function owningModuleId() to centralize the pattern. #B: Added USING and GIVING to SORT_CLAUSE_NOISE set to prevent MERGE USING + OUTPUT PROCEDURE from capturing clause keywords as file names. #C: INPUT/OUTPUT PROCEDURE regex now captures optional THRU/THROUGH range end paragraph, mirroring RE_PERFORM's THRU support. #D: scopedCallerLookup fallback now uses programModuleIds.get(pgm) instead of parentId, so PERFORM/MOVE/GOTO in nested programs with unresolvable paragraphs fall back to the correct inner module. #E: pendingProcUsing only set when PROCEDURE DIVISION line is NOT period-terminated, preventing false USING expectation. Tests: 145 passing | TypeScript clean * fix(cobol): resolve 10th review findings — nested program edge attribution Fix 6 findings from the 10th review (PR #498 comment #4132201110): #A+#F: All CALL/CANCEL/CICS/ENTRY/SQL/SEARCH/file-declaration edges now use owningModuleId() for nested program attribution instead of the outer program's parentId. Added helper function owningModuleId() to centralize the pattern. #B: Added USING and GIVING to SORT_CLAUSE_NOISE set to prevent MERGE USING + OUTPUT PROCEDURE from capturing clause keywords as file names. #C: INPUT/OUTPUT PROCEDURE regex now captures optional THRU/THROUGH range end paragraph, mirroring RE_PERFORM's THRU support. #D: scopedCallerLookup fallback now uses programModuleIds.get(pgm) instead of parentId, so PERFORM/MOVE/GOTO in nested programs with unresolvable paragraphs fall back to the correct inner module. #E: pendingProcUsing only set when PROCEDURE DIVISION line is NOT period-terminated, preventing false USING expectation. Tests: 145 passing | TypeScript clean * fix(cobol): resolve 11th review findings — final nested program + multi-CALL gaps #1: scopedCallerLookup(null) now uses owningModuleId(lineNum) instead of parentId, fixing PERFORM/MOVE/GOTO before first paragraph in nested programs. #2+#3: CALL and CANCEL extraction now uses matchAll (global flag) to capture multiple occurrences on the same line. Dynamic CALL/CANCEL checked independently instead of in else branch. #4: SORT/MERGE ACCESSES edge IDs now use owningModuleId(sort.line) instead of parentId for nested program correctness. #5: preprocessCobolSource free-format detection now uses first 10 lines (consistent with extractCobolSymbolsWithRegex threshold). #6: EXCLUDED_PARA_NAMES expanded with DISPLAY, ACCEPT, WRITE, READ, REWRITE, DELETE, OPEN, CLOSE, RETURN, RELEASE, SORT, MERGE to prevent false-positive paragraph detection on isolated verbs. Also removed unused GraphNode import from cobol-processor.ts. Tests: 145 passing | TypeScript clean * docs(cobol): deepened full language coverage plan with research findings 3 research agents analyzed Phase 1-2 features and graph value ranking. Key findings: cobol-call-using is #1 edge type (9.2/10); multi-line accumulation is dominant challenge; DECLARATIVES is lowest-risk Phase 2 item; SET TO TRUE covers 80-90% of SET usage. * feat(cobol): implement Phase 1 — high-value data flow edges 4 new extraction features that create new ACCESSES and IMPORTS edges: 1.1: EXEC SQL INCLUDE -> IMPORTS edges with reason 'sql-include' Handles unquoted (SQLCA), quoted ('DBRMLIB.MEMBER'), and underscored (CUST_TBL_DCL) member names. 1.2: CALL USING parameter extraction -> ACCESSES edges Extracts parameters from CALL USING clause, filtering BY/REFERENCE/ CONTENT/VALUE/ADDRESS/OF/LENGTH/OMITTED keywords. Creates 'cobol-call-using' ACCESSES edges (graph value: 9.2/10). 1.4: OCCURS DEPENDING ON -> ACCESSES edges with reason 'cobol-depends-on' Extended OCCURS regex captures DEPENDING ON field with subscript stripping. Creates dependency edge from table to controlling field. 1.5: VALUE clause for standard data items Extracts VALUE from data item clauses: quoted strings with type prefix (X/N/G/B), ALL literals, numerics (incl negative/decimal), and figurative constants. Populates Property node values. Tests: 145 passing (+2 ACCESSES from CALL USING) | TypeScript clean * feat(cobol): implement Phase 2 — DECLARATIVES, SET, INSPECT, EXEC DLI 4 new extraction features for error handling, data flow, and IMS/DB: 2.1: EXEC DLI (IMS/DB) -> CodeElement + ACCESSES edges Accumulates EXEC DLI blocks like EXEC SQL. Parses DLI verbs (GU, GN, ISRT, REPL, DLET, CHKP, SCHD, TERM). Extracts SEGMENT, PCB, INTO/FROM, PSB. Creates dli-{verb} ACCESSES edges to <ims>:segment Record nodes. 2.2: DECLARATIVES / USE AFTER EXCEPTION -> ACCESSES edges Tracks inDeclaratives state. Detects USE AFTER STANDARD EXCEPTION ON file-name. Creates cobol-error-handler ACCESSES edge from handler section to file Record. 2.3: SET statement -> ACCESSES edges Detects SET TO TRUE (80-90% of SET usage) and SET index TO/UP BY/DOWN BY. Creates cobol-set-condition / cobol-set-index write edges + cobol-set-read for identifier values. 2.4: INSPECT -> ACCESSES edges with multi-line accumulator Accumulates INSPECT until period (like SORT). Extracts inspected field + tally counters. Creates cobol-inspect-read/write/tally edges. Form detection: tallying/replacing/converting/combined. Preprocessor: 1398 -> 1597 LOC (+199). Tests: 145 passing. * feat(cobol): implement Phase 3 — completeness fixes 6 partial features fixed to first-class support: 3.1: CALL RETURNING -> ACCESSES write edge (cobol-call-returning) 3.2: SELECT OPTIONAL flag preserved in FileDeclaration + Record node 3.3: ALTERNATE RECORD KEY extraction (matchAll for multiple keys) 3.4: COMMON attribute on nested programs (RE_PROGRAM_ID extended) 3.5: IS EXTERNAL / IS GLOBAL as first-class boolean properties (removed usage string hack) 3.6: AUTHOR / DATE-WRITTEN mapped to Module node description Tests: 145 passing | TypeScript clean * feat(cobol): implement Phase 4 — INITIALIZE + metadata completeness 4.1: INITIALIZE statement -> ACCESSES write edge (cobol-initialize) 4.2: DATE-COMPILED and INSTALLATION paragraphs extracted and mapped to Module node description alongside existing AUTHOR/DATE-WRITTEN All 4 plan phases complete. Coverage: ~95% (up from 71.9%). Tests: 145 passing | TypeScript clean * test(cobol): add 24 unit tests for Phase 1-4 features Coverage for all new extraction features: Phase 1 (8 tests): - EXEC SQL INCLUDE (unquoted, quoted, underscored) - CALL USING (simple, mixed modes, ADDRESS OF, OMITTED) - CALL RETURNING - OCCURS DEPENDING ON - VALUE clause (string, numeric, figurative constant) Phase 2 (10 tests): - EXEC DLI GU/ISRT/SCHD (verb, segment, PCB, INTO, FROM, PSB) - DECLARATIVES USE AFTER EXCEPTION (single + multiple sections) - SET TO TRUE, SET index UP BY - INSPECT TALLYING, INSPECT REPLACING Phase 3-4 (6 tests): - SELECT OPTIONAL flag - ALTERNATE RECORD KEY - PROGRAM-ID IS COMMON - IS EXTERNAL / IS GLOBAL booleans - INITIALIZE extraction - Full programMetadata (AUTHOR, DATE-WRITTEN, DATE-COMPILED, INSTALLATION) Total: 168 tests passing (145 + 24 - 1 removed duplicate) * fix(cobol): use /\r?\n/ split for Windows CRLF compatibility All 4 COBOL source files now split on /\r?\n/ instead of '\n' to handle CRLF line endings on Windows. Previously, trailing \r in lines caused RE_GOTO's $ anchor to fail on multi-line GO TO DEPENDING ON statements, producing only 1 goto edge instead of 4. Files fixed: cobol-preprocessor.ts (2 sites), cobol-processor.ts, jcl-parser.ts, cobol-copy-expander.ts Tests: 168 passing | TypeScript clean * fix(cobol): resolve 12th review — dynamic CALL/CANCEL dedup + trailing anchors #1+#2: Removed incorrect hasQuotedCall/hasQuotedCancel deduplication guards. RE_CALL_DYNAMIC and RE_CANCEL_DYNAMIC require [A-Z] after CALL/CANCEL, so they CANNOT match quoted targets — the guards were both unnecessary and actively harmful, suppressing dynamic CALL/CANCEL in ON EXCEPTION patterns. #3+#5: Changed RE_CALL_DYNAMIC and RE_CANCEL_DYNAMIC trailing anchor from (?:\s|\.) to (?=\s|\.|$) (lookahead). The consuming anchor failed when the identifier was the last token on a physical line. Tests: 168 passing | TypeScript clean * feat(cobol): add CALL accumulator + fix SORT double-statement (#4, #6) Finding #4: Multi-line CALL USING accumulator Added callAccum state variable that accumulates CALL statements spanning multiple physical lines until period or END-CALL is found. Uses flushCallAccum() to re-extract CALL target + USING parameters from the full accumulated statement. This fixes the silent loss of ACCESSES parameter edges when USING appears on lines after CALL. Finding #6: SORT double-statement on same line After flushSort(), the code now falls through to re-check the current line for a new SORT/MERGE start (was previously blocked by the sortAccum === null check evaluating before flushSort ran). Also fixed: used non-global regex for CALL detection test to avoid the classic global regex .test() lastIndex bug. Tests: 168 passing (+1 ACCESSES from multi-line CALL USING) * fix(cobol): resolve 13th review — CICS LOAD, USING extraction, file scoping #1: CICS LOAD unresolved edge no longer silently deleted in second pass. Changed narrow cics-link/cics-xctl check to catch-all pattern: rel.reason?.startsWith('cics-') && rel.reason.endsWith('-unresolved') #2: flushCallAccum USING extraction now stops before COBOL statement verbs (INSPECT, SEARCH, SORT, MERGE, DISPLAY, ACCEPT, MOVE, PERFORM, GO TO, CALL, IF, EVALUATE). Prevents absorbing adjacent statements as false USING parameters in legacy pre-COBOL-85 code without END-CALL. #3: CICS FILE Record nodes now globally-scoped (<cics-file>:FILENAME) instead of per-file-scoped. Enables cross-program CICS file access analysis, consistent with SQL table scoping (<db>:TABLE). #4: callAccum pre-check regex now has (?<![A-Z0-9-]) lookbehind to prevent false activation on compound identifiers like WS-CALL-FLAG. Tests: 168 passing | TypeScript clean * fix(cobol): resolve 14th review — callAccum false paragraph + Area A guard #1: callAccum continuation lines now check for COBOL statement verb starts (GO TO, PERFORM, MOVE, etc.) and paragraph/section headers. If detected, the CALL is flushed as-is and the line processed normally — prevents false paragraph detection and currentParagraph corruption from lines like "WS-ADDR." being treated as paragraphs. #4: callAccum pre-check now guarded by currentDivision === 'procedure' to prevent unnecessary activations in DATA DIVISION. #5: Fixed-format paragraph detection now rejects lines with >7 leading spaces (Area B indentation) as paragraph candidates. Paragraph names in fixed-format must start in Area A (col 8-11, max 7 spaces). Free-format mode is unaffected. Tests: 168 passing | TypeScript clean * fix(cobol): resolve 15th review — callAccum Area A + verb boundary fixes #A: Column-position-aware paragraph detection in callAccum flush. #B: inspectAccum early-flush on paragraph/section/verb headers. #C: Verb boundary \b → (?:\s|$) prevents MOVE-COUNT false flush. * test(cobol): add 17 edge-case regression tests + fix USING verb boundary 17 new tests covering all recurring review patterns: Multi-line CALL USING (7 tests): - Parameters on separate continuation lines (IBM mainframe style) - No absorption of INSPECT/GO TO/paragraphs following CALL - END-CALL scope terminator - Hyphenated identifiers (MOVE-COUNT) not triggering false flush - Dual quoted+dynamic CALL on same line (ON EXCEPTION) Nested program attribution (2 tests): - CALL in inner program within inner line range - PERFORM before first paragraph has null caller CRLF compatibility (1 test): - GO TO DEPENDING ON with \r\n line endings Area A paragraph detection (2 tests): - Area B (>7 spaces) rejected; Area A (7 spaces) accepted SORT/MERGE (1 test): COLLATING SEQUENCE keywords not captured PROCEDURE USING (2 tests): RETURNING excluded, period-terminated Comment stripping (1 test): pipe in quoted string preserved SELECT OPTIONAL (1 test): correct file name, not OPTIONAL keyword Bug fix: USING extraction regex verb terminators changed from \bVERB\b to \bVERB(?=\s|$) in flushCallAccum — prevents truncation on hyphenated identifiers like MOVE-COUNT, PERFORM-LIMIT. Total: 185 tests passing * test(cobol): add 32 comprehensive edge-case regression tests 13 new describe blocks covering all extraction features: - EXEC DLI: no-SEGMENT, multi-line accumulation (2 tests) - SET: multiple targets, DOWN BY, TO numeric (3 tests) - INSPECT: CONVERTING, multiple counters, tallying-replacing, paragraph flush during accumulation (4 tests) - DECLARATIVES: no-STANDARD keyword, I-O mode, post-END paragraphs (3) - COPY REPLACING: pseudotext deletion ==OLD== BY ==== (1 test) - VALUE: hex literal, negative numeric, ALL literal (3 tests) - OCCURS: TO range, fixed-size without DEPENDING ON (2 tests) - Dynamic CALL/CANCEL: end-of-line, multiple CANCELs (3 tests) - EXEC SQL: INCLUDE skips tables, SELECT INTO host vars, host variable extraction (3 tests) - INITIALIZE: target and caller context (1 test) - Nested programs: sibling scoping, PROGRAM-ID without ID DIV (2) - EXEC EOF flush: unclosed EXEC SQL flushed (1 test) - Multi-PERFORM: IF/ELSE dual PERFORM on single line (1 test) - IS EXTERNAL: USAGE not polluted by external flag (1 test) Total: 215 tests passing * fix(cobol): resolve 16th review — CANCEL in CALL block + USING boundary #1: flushCallAccum now extracts CANCEL statements from within CALL ON EXCEPTION blocks. Adds RE_CANCEL + RE_CANCEL_DYNAMIC matchAll passes alongside existing CALL extraction. #2: Added \bCANCEL(?=\s|$) to USING lookahead regex to prevent CANCEL keyword being captured as false USING parameter. #3: Multi-line CALL start now returns immediately to prevent the CALL start line from simultaneously feeding sortAccum/inspectAccum. #6: Division transitions now flush all active accumulators (callAccum, sortAccum, inspectAccum) to prevent state leakage across programs. Also added CANCEL to callAccum flush trigger verb list. Tests: 215 passing | TypeScript clean * refactor(cobol): extract shared verb constants + resolve 17th review Extract COBOL_STATEMENT_VERBS, RE_STATEMENT_VERB_START, and RE_USING_PARAMS as shared constants — eliminates 4 duplicated 25-verb regex patterns. 17th review: #1 flushCallAccum before EXEC entry, #2 inspectAccum verb parity via shared constant. Tests: 215 passing | TypeScript clean * test(cobol): replace all fuzzy assertions with exact toBe checks Replaced 7 toBeGreaterThan/toBeLessThan/toBeGreaterThanOrEqual assertions with exact toBe values: - dataItems.length: >= 3 → toBe(3) - calls.length: >= 1 → toBe(1) - calls[0].line: range check → toBe(10) - programs[].startLine/endLine: comparison → exact values - innerA.endLine/innerB.startLine: comparison → exact values Also added 11 new edge-case tests (accumulator flush on EXEC/division transitions, free-format, CANCEL in CALL block, SORT THRU, verb flush, integration). 226 tests passing — zero fuzzy assertions remain. * fix(cobol): resolve 19th review + 15 accumulator flush tests Fixes: #1: END PROGRAM flushes callAccum/sortAccum/inspectAccum #2: PROGRAM-ID sibling path flushes all accumulators #3: Added COMPUTE/ADD/SUBTRACT/MULTIPLY/DIVIDE/STRING/UNSTRING to COBOL_STATEMENT_VERBS (now 32 verbs) Tests (15 new): - END PROGRAM flush: single + nested programs (2) - PROGRAM-ID sibling flush (1) - Arithmetic verb flush: COMPUTE/ADD/SUBTRACT/MULTIPLY/DIVIDE (5) - String verb flush: STRING/UNSTRING (2) - Arithmetic not captured as false USING params (1) - SORT flushed at END PROGRAM (1) - INSPECT flushed at END PROGRAM (1) - All with exact toBe assertions (2) Total: 239 tests passing | Zero fuzzy assertions * fix(cobol): resolve 20th review — INITIALIZE multi-target + 2 tests Finding 1: INITIALIZE now captures multiple targets with REPLACING clause keyword filtering. Regex changed to lazy match stopping at REPLACING/WITH/period boundary. Targets split on whitespace and filtered against INITIALIZE_CLAUSE_KEYWORDS set. Tests (2 new): - INITIALIZE multi-target: WS-CUSTOMER WS-ORDER WS-LINE-ITEM → 3 - INITIALIZE with REPLACING: only WS-RECORD captured, not keywords Total: 241 tests passing | TypeScript clean
This was referenced Mar 27, 2026
magyargergo
added a commit
that referenced
this pull request
May 14, 2026
* fix(cpp): complete scope-resolution parity * fix(ci): resolve formatting, lint errors for PR #1520 - prettier: format arity-metadata.ts, captures.ts, index.ts - eslint: rename unused HEADER_GLOB to _HEADER_GLOB - eslint: replace unsafe parser.parse() with parseSourceSafe() - eslint: suppress intentional console.warn/log in sync.ts - eslint: remove unused _it import alias in cpp.test.ts * fix(ci): complete formatting, lint, and typecheck fixes - prettier: format call-processor.ts, imported-return-types.ts, include-extractor.test.ts, cpp-captures.test.ts, cpp-imports.test.ts - eslint: suppress intentional console.warn in manifest-extractor.ts - typecheck: restore 'thrift' in ContractType union (was accidentally removed) and add thrift case to exhaustive switch in manifest-extractor * fix(ci): revert unintended group module changes that broke tests Restore types.ts, config-parser.ts, matching.ts, sync.ts, and manifest-extractor.ts to upstream/main versions. The original commit accidentally removed fields (thrift, workspace_deps, exclude_links_paths, exclude_links_param_only_paths) from DetectConfig/MatchingConfig/ContractType which are still referenced by matching.test.ts, config-parser.test.ts, sync.test.ts and other integration tests. This PR's scope is C++ scope-resolution parity only — group module type definitions and logic should remain unchanged. * fix(codeql): address security and quality alerts - arity-metadata.ts, interpret.ts: replace single-pass template strip regex (/<[^>]*>/g) with a while-loop to fully handle nested templates like Map<List<int>> — resolves 'Incomplete multi-character sanitization' - cpp.test.ts: remove unused vitest 'it' import since the file defines its own 'it' via createResolverParityIt — resolves 'Assignment to constant' - include-extractor.test.ts: use fs.mkdtempSync() instead of predictable os.tmpdir()+Date.now() paths — resolves 'Insecure temporary file' - interpret.ts: remove redundant 'name !== undefined' check (already guaranteed by early return) — resolves 'Comparison between inconvertible types' * review: address Claude review findings on PR #1520 - Findings 1-3 (BLOCKERS): restore include-extractor.ts and its test to the main baseline. Block-comment fallback regression, suffix-resolve false-positive suppression, and the four deleted regression tests (#3-#6) are now back. These changes were unrelated to C++ scope parity and should not have been in this PR. - Finding 4 (MAJOR, partial): revert COMPOUND_RECEIVER_MAX_DEPTH 6 to 4. No C++ test exercises depth > 4 (cpp-chain-call uses a 2-hop chain), so the bump risked silent regressions on other migrated languages without justification. The wildcard-origin propagation in imported-return-types.ts is retained — C++ #include and using namespace both emit wildcard-origin bindings (cpp/import-decomposer .ts:40,90), so wildcard propagation is causal to C++ parity. - Finding 6: tighten write-access dedup test with exact per-field counts (nameWrites = 2, addrWrites = 1) instead of total-count + sub string containment, so a regression in one of the two name writes can no longer be masked. - Finding 8: skipped. Box-drawing characters in cpp/query.ts comments match the established convention used in csharp/java/php query files. Finding 5 (int/long normalization tie-breaker) left as documented follow-up — proper fix requires resolver-level tie-breaker logic and risks regressing other arity-matching tests. * fix(cpp): stop #include from leaking class methods and namespace members (U1) The C++ registry-primary resolver was emitting impossible CALLS edges for ordinary headers: an including file's unqualified save() resolved to User::save and unqualified foo() resolved to ns::foo. Two leak paths converged on localDefs: 1. expandCppWildcardNames (file-local-linkage.ts) iterated the flattened localDefs and exported every simple tail, including class-owned methods and namespace-contained symbols. Replaced with a scope-aware filter: build nodeId -> owning Scope from Scope.ownedDefs and skip defs whose owning scope is Namespace or Class. 2. The shared global free-call fallback's pickUniqueGlobalCallable walks the workspace registry by simple name and would still hit class methods / namespace members even with wildcard expansion fixed. Plugged the gap via the existing isFileLocalDef hook — semantically 'logically invisible cross-file' — by tracking per- file non-globally-visible nodeIds (populateCppNonGloballyVisible, called from populateOwners) and adding an ownerId !== undefined fast-path for class-owned defs. Side fix in shared finalize-algorithm.ts: when wildcard expansion resolves to a real target but produces zero propagating names, the edge was dropped, taking the file-level IMPORTS edge with it. Preserve the original wildcard edge so #include dependencies survive even when the header exposes no unqualified bindings. Tests: cpp-include-no-class-leak, cpp-include-no-namespace-leak, and cpp-anon-ns-same-file-visible fixtures. Negative tests mode-gated to REGISTRY_PRIMARY_CPP=1 via the expected-failures registry — legacy DAG has no scope-aware filtering on the global fallback; backporting is out of scope. All 2104 resolver integration tests pass under registry-primary mode. * fix(cpp): suppress receiver-bound CALLS when integer-width overloads collide (U2) C++ arity-metadata normalizes int, long, short, unsigned, size_t to 'int' so single-candidate flows like 'process(42L)' match a 'long'- typed parameter via loose matching. But when both 'process(int)' and 'process(long)' coexist as method overloads, they both end up with parameterTypes=['int'] in the registry, and pickOverload's narrowing returns 2 candidates with no way to disambiguate. The previous code picked candidates[0] arbitrarily, emitting a CALLS edge to the wrong overload roughly half the time. Fix: - Add isOverloadAmbiguousAfterNormalization in overload-narrowing.ts that detects >1 candidate sharing identical parameterTypes sequences. - Have pickOverload return a new OVERLOAD_AMBIGUOUS sentinel when this fires. - In the receiver-bound-calls loop, when pickOverload signals ambiguity, suppress the edge AND add the site to handledSites so the late-stage emitReferencesViaLookup pass does not re-emit the pre-resolved reference. Without the handled-mark, the reference index still carries a toDef and emits the same wrong edge. Graph schema has no ambiguous-target edge model, so emitting two edges (one per candidate) would require a separate schema change. Zero-edge is the only safe outcome. Other languages: the ambiguity check is a precondition gate, not a behavior change for normal narrowing. Languages whose normalizers do not collapse distinct types into a single token (verified by grep over *-arity-metadata.ts) will never produce >1 candidate with identical parameterTypes from genuinely distinct declarations, so the branch is effectively C++-only in practice. Test: cpp-overload-int-long fixture asserts exactly .toBe(0) CALLS edges. Count=1 = arbitrary pick (the bug); count>1 = unsupported ambiguous-edge model. Mode-gated to REGISTRY_PRIMARY_CPP=1 — legacy DAG has no OVERLOAD_AMBIGUOUS wiring; backporting is out of scope. All 2105 resolver integration tests pass under registry-primary; all 139 cpp tests pass under both modes (3 negative tests skipped in legacy as documented). * test(cpp): add integration coverage for anonymous-namespace, using-namespace conflict, and std-shim leakage (U3+U4+U5) Three new end-to-end fixtures exercise the resolver pipeline against scenarios that previously had only unit-level coverage or no coverage at all (Claude review Finding 7): U3 — cpp-anon-ns-cross-file: helper.cpp declares 'namespace { void worker(); }' and calls it internally. caller.cpp declares a separate 'void worker()' and calls it. Asserts (a) the cross-file CALLS edge from caller's run() does not target helper.cpp's anonymous-namespace worker, and (b) the same-file edge from helper_entry() to its own worker still resolves (positive guard against a 'no edges at all' regression making the negative check vacuously pass). Includes a state-isolation guard that re-runs the same fixture and asserts identical results, proving clearFileLocalNames() is called by the pipeline entry. U4 — cpp-using-namespace-conflict: Two headers each declaring 'namespace a { foo() }' and 'namespace b { foo() }' respectively, plus a caller doing 'using namespace a; using namespace b; foo()'. Asserts exactly zero CALLS edges. One edge = arbitrary pick (the bug); two edges would require an ambiguous-target edge model GitNexus does not have. Depends on U1 — without scope-aware filtering, both foo()s would already be in the importer's wildcard binding set as simple 'foo', so the test would pass for the wrong reason. U5 — cpp-using-namespace-std-smoke: Fixture-local 'namespace std { void cout_write(); void println(); }' shim rather than real <iostream> — captures the wildcard-leak shape deterministically without depending on system-header modeling stability (out of scope per plan). Asserts (a) the project-local call resolves correctly, (b) no leak to shim STL symbols, and (c) no CALLS/ACCESSES edges from the caller into std-shim.h at all. Negative tests for U2/U4 mode-gated to REGISTRY_PRIMARY_CPP=1 via the expected-failures registry; legacy DAG lacks the OVERLOAD_AMBIGUOUS suppression and the namespace-aware filtering, so the leaks persist there. All 2112 resolver integration tests pass under registry-primary; all 146 cpp tests pass under both modes (4 negative tests skipped in legacy as documented). * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(cpp): scope-aware isSuperReceiver classification (U1) The C++ isSuperReceiver hook used a regex `/^[A-Z]\w*::/` that misclassified any uppercase-qualified call as a super-receiver call. Singleton::getInstance(), std::Foo::bar(), and PascalCase namespace calls all entered the super branch, where the absence of an enclosing class (or wrong MRO context) dropped the resolution entirely. Fix: - New optional ScopeResolver hook isSuperReceiverInContext(text, callerScope, scopes). Languages where super classification depends on caller context define it; receiver-bound-calls.ts prefers it when defined and falls back to the simple isSuperReceiver(text) otherwise. Other migrated languages (Python, Java, C#, PHP, Go, TypeScript) are unchanged. - C++ implementation: parse the LHS of '::' from the receiver text, resolve via findClassBindingInScope, and return true only when the LHS is a class-like def in the caller's enclosing class's MRO. Returns false for namespace LHS, unresolved LHS, self-class LHS (qualified self-calls aren't super), and any non-'::' form. - Extended the C++ tree-sitter query to capture the LHS of qualified_identifier as @reference.receiver so qualified static member calls (Singleton::getInstance()) reach the receiver-bound Case 2 (class-name receiver) path. Without the receiver capture, qualified calls had no explicit receiver and could not resolve through any receiver-bound branch. Test: cpp-namespace-qualified-not-super fixture. Singleton::getInstance() from a free function asserts exactly 1 CALLS edge through the qualified-call path. Passes under both REGISTRY_PRIMARY_CPP=1 and =0. All 2113 resolver integration tests pass; all 147 cpp tests pass under both modes. * fix(cpp): suppress receiver-bound CALLS when default-arg overloads collide (U4) ISO C++ rejects 's.f(1)' as ambiguous when both 'void f(int)' and 'void f(int, int = 0)' are declared on S. The previous resolver returned the first viable candidate via pickOverload's fallback. Extended isOverloadAmbiguousAfterNormalization to take an optional argCount: when provided, the predicate compares only the first argCount slots of each candidate's parameterTypes. Candidates whose declared-prefix matches up to argCount are treated as ambiguous because default arguments make all of them equally viable for the call. Without argCount, behavior is unchanged (the original int/long normalization-collapse contract, full-length equality required). pickOverload now passes site.arity so default-arg ambiguity fires. Test: cpp-overload-default-arg-ambiguous fixture. s.f(1) where S has f(int) and f(int, int = 0) asserts exactly .toBe(0) CALLS edges. Passes under both REGISTRY_PRIMARY_CPP=1 and =0. All 2114 resolver integration tests pass; all 148 cpp tests pass under both modes. * fix(cpp): two-phase template lookup suppresses dependent-base members (U3) ISO C++ two-phase name lookup: inside a class template body, unqualified calls MUST NOT bind to members of a dependent base class. Only this->name or Base<T>::name forms make the lookup dependent. GCC and Clang both reject the unqualified form with 'declaration of f must be available'. Before this fix, GitNexus's global free-call fallback walked the workspace registry by simple name and bound unqualified calls inside template bodies to dependent-base members, producing CALLS edges the compiler would reject. Implementation: - New languages/cpp/two-phase-lookup.ts module: per-pipeline state recording (className, dependentBaseName) pairs at capture time and resolving them to nodeId sets during populateOwners. - captures.ts detectCppDependentBases walks the AST once finding every template_declaration containing a class/struct definition. For each, it collects template-parameter names (typename T, class T, non-type int N, template-template parameters) and walks each base in the base_class_clause checking whether any inner type_identifier matches a template parameter. Conservative bias: typename T::U, decltype, and template-template-parameter shapes also classified as dependent. - Extended scope-resolution contract's isCallableVisibleFromCaller hook with optional callerScope and scopes fields. C++ implements the hook to consult isCppDependentBaseMember: when the candidate is a member of a dependent base of the caller's enclosing class, the hook returns false and pickUniqueGlobalCallable skips the candidate. - clearFileLocalNames also clears the dependent-base state per pipeline run. Fixtures: - cpp-two-phase-dependent-base: Derived<T> deriving from Base<T>, unqualified f() and i inside Derived's body. Asserts zero CALLS edges and zero ACCESSES edges respectively. - cpp-two-phase-this-qualified, cpp-two-phase-non-dependent-base, cpp-two-phase-namespace-free-call-inside-template: positive fixtures left as documented gaps (this-> and qualified-name resolution inside template bodies are pre-existing resolver weaknesses independent of U3). Tracked separately. Negative test mode-gated to REGISTRY_PRIMARY_CPP=1 via the expected- failures registry; legacy DAG has no two-phase lookup. All 2116 resolver integration tests pass under registry-primary; all 150 cpp tests pass under both modes (5 negative tests skipped in legacy as documented). * fix(cpp): implement V1 ADL (Koenig lookup) for free-function calls (U2) Plan 2026-05-13-001 U2. Adds argument-dependent lookup as a new candidate-generating tier in `emitFreeCallFallback`: when ordinary unqualified lookup is empty, ADL surfaces candidates from each value-class-typed argument's enclosing namespace. V1 boundary (locked by cpp-adl-pointer-arg-boundary fixture): - only direct enclosing-namespace closure - only directly-named class-type values (pointer / reference / template- spec args excluded; closure rules deferred to V2) - ADL fires ONLY when ordinary lookup is empty (no union-and-resolve) Parenthesized name `(f)(s)` suppresses ADL per ISO C++ [basic.lookup.argdep]/3.1. Multi-candidate ambiguity (e.g. `process(int)` vs `process(long)` after C++ int-width normalization) returns the ADL_AMBIGUOUS sentinel — caller suppresses entirely, mirroring the OVERLOAD_AMBIGUOUS contract from plan 2026-05-12-002 U2. Implementation: - `cpp/adl.ts` — new module: per-pipeline argInfoBySite + noAdlSites Maps populated at capture time, classToNamespaceQualifiedName Map populated during populateOwners; `pickCppAdlCandidates` returns SymbolDefinition | ADL_AMBIGUOUS | undefined - `scope-resolution/contract/scope-resolver.ts` — adds optional `resolveAdlCandidates` hook - `scope-resolution/passes/free-call-fallback.ts` — invokes ADL hook between `findCallableBindingInScope` and `pickUniqueGlobalCallable`; marks site handled on `'ambiguous'` so emit-references doesn't retry - `cpp/captures.ts` — detects `parenthesized_expression` function wrap; per-arg classification (pointer/reference/value class) preserving the shape info the existing arity-narrowing normalizer strips - `cpp/scope-resolver.ts` — registers hook, populates associated namespaces, clears state in loadResolutionConfig Negative tests (parens, pointer-boundary, ambiguous) gated under LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp — legacy DAG has no V1/V2 ADL boundary or ADL_AMBIGUOUS suppression. 154/154 cpp integration tests pass under REGISTRY_PRIMARY_CPP=1; 147 pass + 7 skipped under =0 (legacy parity baseline). * fix(cpp): inline namespace transitive walking + qualified namespace resolution (U5) Plan 2026-05-13-001 U5. Two ISO C++ inline-namespace semantics: 1. Unqualified-lookup transitive visibility: inline-namespace members reach the enclosing namespace's scope as if declared there. The `populateCppNonGloballyVisible` exemption keeps them globally visible so cross-file unqualified lookup finds them. 2. Qualified-receiver transitive visibility: `outer::foo()` resolves to `outer::v1::foo()` when `v1` is inline (and through arbitrarily-deep nesting like `outer::v1::experimental::foo`, matching libc++ `__1` / libstdc++ `__cxx11`). The second behavior required a new resolver case in `receiver-bound-calls.ts` (Case 1.5: language-specific qualified-receiver member lookup) because C++ qualified-namespace member calls had no prior resolution path — receiver-bound Case 1 only handled `ParsedImport.kind === 'namespace'` (Python/JS-style) and Case 2 handles class receivers, neither of which fired for `outer::foo()`. The new hook `resolveQualifiedReceiverMember` is opt-in; languages without C++-style qualified-name semantics omit it. Implementation: - `cpp/inline-namespaces.ts` — new module: per-pipeline `inlineNamespaceRangesByFile` + `inlineNamespaceScopeIds` Sets; `markCppInlineNamespaceRange` at capture time; `populateCppInlineNamespaceScopes` resolves ranges → scope IDs; `resolveCppQualifiedNamespaceMember` walks namespace scopes by simple name and descends transitively through inline children only. - `scope-resolution/contract/scope-resolver.ts` — adds optional `resolveQualifiedReceiverMember` hook to the contract. - `scope-resolution/passes/receiver-bound-calls.ts` — Case 1.5 invokes the hook between Case 1 (namespace imports) and Case 2 (class-name receiver). Returns undefined for non-namespace receivers so Case 2 still resolves class-qualified calls. - `cpp/captures.ts` — detects `inline` keyword child on `namespace_definition`; records 1-based range to match Scope.range. - `cpp/file-local-linkage.ts` — `populateCppNonGloballyVisible` exempts inline-namespace scopes so cross-file unqualified lookup keeps their members visible. - `cpp/scope-resolver.ts` — wires `populateCppInlineNamespaceScopes` into populateOwners (BEFORE `populateCppNonGloballyVisible` so the exemption sees populated state); registers `resolveQualifiedReceiverMember` hook. 4 fixtures: `cpp-inline-namespace-unqualified`, `-versioned`, `-nested` (two transitive inline hops, STL `__1` shape), and `-adl-participation` (composes with U2 — ADL surfaces records declared inside inline child namespaces). All 4 assert exactly 1 CALLS edge with correct target file. Versioned fixture gated under LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp — legacy DAG can't disambiguate two same-name foos without inline awareness. Other 3 coincidentally resolve in legacy. 158/158 cpp integration tests pass under REGISTRY_PRIMARY_CPP=1; 150 pass + 8 skipped under =0 (legacy parity baseline). * test(cpp): Phase 5 cross-unit composition tests for U1/U2/U3/U5 Plan 2026-05-13-001 Phase 5. Locks in correct behavior at the intersections between the previously-shipped scope-resolver units. Enhancement to U1: `isSuperReceiverInContext` strips template-argument lists (`Base<T>` → `Base`) and namespace prefixes (`outer::v1::Base` → `Base`) before resolving the receiver in the caller's scope chain. This makes the super-receiver classification work for template-class heritage shapes like `Base<T>::method()` and `outer::v1::Base<T>::f()`. Three fixtures + four tests: - `cpp-phase5-u1-u3-qualified-base-call`: `template<class T> struct Derived : Base<T>` with `Base<T>::method()` inside a template body. Asserts NO mis-routing (count = 0) — documents the V1 gap that template-class inheritance isn't captured as EXTENDS by the legacy DAG, so MRO walks are empty and the super branch can't dispatch. The composition still works correctly: U1's template-arg-stripping classifies `Base<T>` as a super candidate, but the empty-MRO terminates without false edges. - `cpp-phase5-u2-u3-adl-from-derived`: `Derived : Base<T>` where `Base::record` shadows `audit::record`. Unqualified `record(e)` inside the template body should resolve via ADL to `audit::record` (because U3 + the `isFileLocalDef` class- owned filter suppress `Base::record`). Asserts 1 edge to audit.h and 0 edges to base.h. - `cpp-phase5-u3-u5-inline-base`: `template<class T> struct Derived : outer::v1::Base<T>` where `v1` is inline. Unqualified `f()` inside `Derived<T>::g()` should NOT bind to Base::f (dependent-base suppression even across inline namespace prefix). Asserts count = 0. Phase 5 tests asserting no-false-positives are gated under LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp — legacy DAG over- resolves without the template-arg-stripping qualified-receiver path and without two-phase dependent-base suppression. 162/162 cpp integration tests pass under REGISTRY_PRIMARY_CPP=1; 152 pass + 10 skipped under =0 (legacy parity baseline). --------- Co-authored-by: HuangWenjie <zhoudeng.hwj@alibaba-inc.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
magyargergo
added a commit
that referenced
this pull request
May 18, 2026
…lution (#1657) * perf(scope-resolution): use owner-keyed lookup for Step 2 member resolution (#1656) * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(scope-resolution): index Const/Static in FieldRegistry for Step 2 lookup Extend FieldRegistry to hold multiple defs per (owner, name), reconcile Const and Static into the owner-keyed index, and wire lookupAllByOwner through the production hook so Step 2 does not drop field kinds the registry never indexed. Pass explicitReceiver on read/write reference sites and document undefined-vs-empty hook semantics for defs fallback. Co-authored-by: Cursor <cursoragent@cursor.com> * perf(scope-resolution): centralize O(1) owned-member hook and guard hot path Extract lookupOwnedMembersByOwner for the production Step 2 hook so merges stay O(1) per registry with no defs.byId scan. Add a perf-contract unit test that throws if byId.values runs when the hook is wired. Reuse a frozen empty sentinel on double miss to avoid per-probe allocations. Co-authored-by: Cursor <cursoragent@cursor.com> * chore: drop unused buildFieldRegistry import * chore(scope-resolution): apply ce-code-review safe_auto fixes - Drop unreachable return + unused values() capture in perf-contract trap (Finding #7) - Type lookupOwnedMembersByOwner ownerDefId as DefId (Finding #9) - Add Static-kind Step 2 lookup test mirroring the Const case (Finding #11) * docs(field-registry): document lookupFieldByOwner first-wins semantics Audit of all 6 production callers (call-processor.ts:2279, walkers.ts:535, receiver-bound-calls.ts:380+730, type-env.ts:627+631) confirms none depends on last-wins precedence — all treat the return as a generic 'field with this name owned by this class'. Clarify the JSDoc to surface the semantic change introduced when FieldRegistry moved from last-wins to append-order storage (ce-code-review finding #2). * test(scope-resolution): extend Step 2 perf contract to implicit-self, MRO, field paths Adds three sibling tests under the Step 2 perf contract describe block, each asserting defs.byId.values() does NOT execute when ownedMembersByOwner is wired: - implicit-self receiver via typeBindings.self (no explicitReceiver branch) - 2-level MRO chain (Child extends Parent, save resolves on Parent at depth 1) - FieldRegistry read via Step 2 (property lookup, separate registry path) Pins the perf invariant on every distinct entry into walkReceiverTypeBinding so a regression bypassing the hook on any sub-path now fails CI immediately (ce-code-review finding #8). * test(resolve-references): cover arity-overload filtering via resolveReferenceSites Pins the orchestration-layer wiring of providers.arityCompatibility: hook returns [save(arity 1), save(arity 2)], referenceSite.arity = 1, arityCompatibility verdicts 'compatible'/'incompatible' by parameterCount, exactly one reference emitted with toDef = the arity-1 overload. registries.test.ts already covered arity at the buildMethodRegistry level; this adds the missing entry-point check that resolveReferenceSites threads providers correctly through to lookupCore.Step5 (ce-code-review finding #10). * test(resolve-references): add hook-on vs hook-off parity test Runs resolveReferenceSites twice on the same fixture (Parent.save method hit + Child.name field hit, Child extends Parent MRO chain) — once with ownedMembersByOwner wired to a synthetic registry, once with the hook absent so collectOwnedMembers takes the defs.byId fallback. Asserts: - stats are identical (sitesProcessed / referencesEmitted / unresolved) - referenceIndex.bySourceScope entries have equal length - toDef sets are equal - each per-site reference (including evidence and depth) is .toEqual Locks the semantic-parity claim in code while both paths still exist. Will be removed alongside the fallback in finding #1 (ce-code-review #3). * test(typescript): probe Step 2 MRO walk against ambient (declare class) base Adds typescript-ambient-base-class fixture with an export declare class AmbientBase + Derived extends AmbientBase and a call site d.ambientMethod(). Integration assertions: - Both classes are detected - EXTENDS edge Derived → AmbientBase emitted - CALLS edge to ambient.ts:ambientMethod resolved via MRO walk Probes the ce-code-review #6 concern that ambient-only owners (whose bodies are never parsed) might be silently skipped by Step 2 after the owner-keyed lookup change. Result: the call resolves correctly — the method signature inside the declare class body still flows through reconcileOwnership into model.methods, so the hook returns the right ancestor hits. Residual risk is empirically closed. * feat(scope-resolution): route nested types via owner-keyed TypeRegistry Closes the Step 2 contract footgun where 'hook returns [] = authoritative miss' silently dropped any owned def whose NodeLabel was outside the method/field if-chain in reconcileOwnership. - TypeRegistry: add nestedByOwner Map + lookupAllByOwner(owner, simple) + registerByOwner(owner, simple, def). Mirrors MethodRegistry/ FieldRegistry shape; cleared with the rest on cascade clear. - reconcileOwnership: route class-like NodeLabels (Class/Interface/Enum/ Struct/Union/Trait/TypeAlias/Typedef/Record/Delegate/Annotation/ Template/Namespace) via types.registerByOwner. New nestedTypesRegistered stat. Idempotent skip via nodeId match. - validateOwnershipParity: extend the I9 invariant check to nested types. - lookupOwnedMembersByOwner: merge methods + fields + nested-type hits; short-circuit when any one source contributes the full result. Unblocks future receiver-MRO registries that need to resolve 'Outer.Inner' through the receiver's type-binding chain (ce-code-review finding #5a). * refactor(scope-resolution): make ownedMembersByOwner required; delete byId fallback Per ce-code-review finding #1, the optional-hook design encoded a silent O(|defs|) perf cliff into the type system: any RegistryContext built without the hook regressed Step 2 to scanning every def per probe with no warning. Production wires the hook unconditionally; the fallback was exercised only by tests. - RegistryContext.ownedMembersByOwner: required, returns readonly SymbolDefinition[] (no | undefined). Implementations MUST return [] on authoritative miss. - collectOwnedMembers in lookup-core.ts collapses to a one-line forward to the hook; the defs.byId.values() scan and simpleNameOf helper are deleted (simpleNameOf had no other consumers). - ResolveReferencesInput.ownedMembersByOwner: required to match. - Tests: drop three fallback-path tests (registries Const fallback, resolveReferenceSites no-hook fallback, resolveReferenceSites Const- undefined fallback) and the hook-vs-fallback parity test added by finding #3. makeCtx in registries.test.ts now defaults to a real owner-keyed scan over the test fixture defs so tests that don't care about the hook keep working. * perf(free-call-fallback): cache global callables by simple name once per pass pickUniqueGlobalCallable scanned scopes.defs.byId.values() on every free-call fallback site. After PR #1656 fixed Step 2, this scan became the dominant remaining O(|defs|) hot path on large repos (ce-code-review finding #4). - buildGlobalCallableIndex builds a Map<simpleName, SymbolDefinition[]> over scopes.defs once at the top of emitFreeCallFallback. Same filter the per-site scan applied: Function / Method / Constructor, keyed by the last .-segment of qualifiedName. - pickUniqueGlobalCallable consumes the prebuilt index via O(1) Map.get instead of iterating every def. Per-site complexity drops from O(|defs|) to O(|defs with this simple name|). - Cost: O(|defs|) once per pass instead of O(|defs| * |free-call sites|). Subsequent narrowing (arity, conversion-rank) and the model-side fallback (model.symbols.lookupCallableByName + model.methods.lookupMethodByName) are unchanged. * chore(autofix): apply prettier + eslint fixes via /autofix command * ci: trigger build --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com>
hohaivu
pushed a commit
to hohaivu/GitNexus
that referenced
this pull request
May 19, 2026
…ri#938) (abhigyanpatwari#1520) * fix(cpp): complete scope-resolution parity * fix(ci): resolve formatting, lint errors for PR abhigyanpatwari#1520 - prettier: format arity-metadata.ts, captures.ts, index.ts - eslint: rename unused HEADER_GLOB to _HEADER_GLOB - eslint: replace unsafe parser.parse() with parseSourceSafe() - eslint: suppress intentional console.warn/log in sync.ts - eslint: remove unused _it import alias in cpp.test.ts * fix(ci): complete formatting, lint, and typecheck fixes - prettier: format call-processor.ts, imported-return-types.ts, include-extractor.test.ts, cpp-captures.test.ts, cpp-imports.test.ts - eslint: suppress intentional console.warn in manifest-extractor.ts - typecheck: restore 'thrift' in ContractType union (was accidentally removed) and add thrift case to exhaustive switch in manifest-extractor * fix(ci): revert unintended group module changes that broke tests Restore types.ts, config-parser.ts, matching.ts, sync.ts, and manifest-extractor.ts to upstream/main versions. The original commit accidentally removed fields (thrift, workspace_deps, exclude_links_paths, exclude_links_param_only_paths) from DetectConfig/MatchingConfig/ContractType which are still referenced by matching.test.ts, config-parser.test.ts, sync.test.ts and other integration tests. This PR's scope is C++ scope-resolution parity only — group module type definitions and logic should remain unchanged. * fix(codeql): address security and quality alerts - arity-metadata.ts, interpret.ts: replace single-pass template strip regex (/<[^>]*>/g) with a while-loop to fully handle nested templates like Map<List<int>> — resolves 'Incomplete multi-character sanitization' - cpp.test.ts: remove unused vitest 'it' import since the file defines its own 'it' via createResolverParityIt — resolves 'Assignment to constant' - include-extractor.test.ts: use fs.mkdtempSync() instead of predictable os.tmpdir()+Date.now() paths — resolves 'Insecure temporary file' - interpret.ts: remove redundant 'name !== undefined' check (already guaranteed by early return) — resolves 'Comparison between inconvertible types' * review: address Claude review findings on PR abhigyanpatwari#1520 - Findings 1-3 (BLOCKERS): restore include-extractor.ts and its test to the main baseline. Block-comment fallback regression, suffix-resolve false-positive suppression, and the four deleted regression tests (abhigyanpatwari#3-abhigyanpatwari#6) are now back. These changes were unrelated to C++ scope parity and should not have been in this PR. - Finding 4 (MAJOR, partial): revert COMPOUND_RECEIVER_MAX_DEPTH 6 to 4. No C++ test exercises depth > 4 (cpp-chain-call uses a 2-hop chain), so the bump risked silent regressions on other migrated languages without justification. The wildcard-origin propagation in imported-return-types.ts is retained — C++ #include and using namespace both emit wildcard-origin bindings (cpp/import-decomposer .ts:40,90), so wildcard propagation is causal to C++ parity. - Finding 6: tighten write-access dedup test with exact per-field counts (nameWrites = 2, addrWrites = 1) instead of total-count + sub string containment, so a regression in one of the two name writes can no longer be masked. - Finding 8: skipped. Box-drawing characters in cpp/query.ts comments match the established convention used in csharp/java/php query files. Finding 5 (int/long normalization tie-breaker) left as documented follow-up — proper fix requires resolver-level tie-breaker logic and risks regressing other arity-matching tests. * fix(cpp): stop #include from leaking class methods and namespace members (U1) The C++ registry-primary resolver was emitting impossible CALLS edges for ordinary headers: an including file's unqualified save() resolved to User::save and unqualified foo() resolved to ns::foo. Two leak paths converged on localDefs: 1. expandCppWildcardNames (file-local-linkage.ts) iterated the flattened localDefs and exported every simple tail, including class-owned methods and namespace-contained symbols. Replaced with a scope-aware filter: build nodeId -> owning Scope from Scope.ownedDefs and skip defs whose owning scope is Namespace or Class. 2. The shared global free-call fallback's pickUniqueGlobalCallable walks the workspace registry by simple name and would still hit class methods / namespace members even with wildcard expansion fixed. Plugged the gap via the existing isFileLocalDef hook — semantically 'logically invisible cross-file' — by tracking per- file non-globally-visible nodeIds (populateCppNonGloballyVisible, called from populateOwners) and adding an ownerId !== undefined fast-path for class-owned defs. Side fix in shared finalize-algorithm.ts: when wildcard expansion resolves to a real target but produces zero propagating names, the edge was dropped, taking the file-level IMPORTS edge with it. Preserve the original wildcard edge so #include dependencies survive even when the header exposes no unqualified bindings. Tests: cpp-include-no-class-leak, cpp-include-no-namespace-leak, and cpp-anon-ns-same-file-visible fixtures. Negative tests mode-gated to REGISTRY_PRIMARY_CPP=1 via the expected-failures registry — legacy DAG has no scope-aware filtering on the global fallback; backporting is out of scope. All 2104 resolver integration tests pass under registry-primary mode. * fix(cpp): suppress receiver-bound CALLS when integer-width overloads collide (U2) C++ arity-metadata normalizes int, long, short, unsigned, size_t to 'int' so single-candidate flows like 'process(42L)' match a 'long'- typed parameter via loose matching. But when both 'process(int)' and 'process(long)' coexist as method overloads, they both end up with parameterTypes=['int'] in the registry, and pickOverload's narrowing returns 2 candidates with no way to disambiguate. The previous code picked candidates[0] arbitrarily, emitting a CALLS edge to the wrong overload roughly half the time. Fix: - Add isOverloadAmbiguousAfterNormalization in overload-narrowing.ts that detects >1 candidate sharing identical parameterTypes sequences. - Have pickOverload return a new OVERLOAD_AMBIGUOUS sentinel when this fires. - In the receiver-bound-calls loop, when pickOverload signals ambiguity, suppress the edge AND add the site to handledSites so the late-stage emitReferencesViaLookup pass does not re-emit the pre-resolved reference. Without the handled-mark, the reference index still carries a toDef and emits the same wrong edge. Graph schema has no ambiguous-target edge model, so emitting two edges (one per candidate) would require a separate schema change. Zero-edge is the only safe outcome. Other languages: the ambiguity check is a precondition gate, not a behavior change for normal narrowing. Languages whose normalizers do not collapse distinct types into a single token (verified by grep over *-arity-metadata.ts) will never produce >1 candidate with identical parameterTypes from genuinely distinct declarations, so the branch is effectively C++-only in practice. Test: cpp-overload-int-long fixture asserts exactly .toBe(0) CALLS edges. Count=1 = arbitrary pick (the bug); count>1 = unsupported ambiguous-edge model. Mode-gated to REGISTRY_PRIMARY_CPP=1 — legacy DAG has no OVERLOAD_AMBIGUOUS wiring; backporting is out of scope. All 2105 resolver integration tests pass under registry-primary; all 139 cpp tests pass under both modes (3 negative tests skipped in legacy as documented). * test(cpp): add integration coverage for anonymous-namespace, using-namespace conflict, and std-shim leakage (U3+U4+U5) Three new end-to-end fixtures exercise the resolver pipeline against scenarios that previously had only unit-level coverage or no coverage at all (Claude review Finding 7): U3 — cpp-anon-ns-cross-file: helper.cpp declares 'namespace { void worker(); }' and calls it internally. caller.cpp declares a separate 'void worker()' and calls it. Asserts (a) the cross-file CALLS edge from caller's run() does not target helper.cpp's anonymous-namespace worker, and (b) the same-file edge from helper_entry() to its own worker still resolves (positive guard against a 'no edges at all' regression making the negative check vacuously pass). Includes a state-isolation guard that re-runs the same fixture and asserts identical results, proving clearFileLocalNames() is called by the pipeline entry. U4 — cpp-using-namespace-conflict: Two headers each declaring 'namespace a { foo() }' and 'namespace b { foo() }' respectively, plus a caller doing 'using namespace a; using namespace b; foo()'. Asserts exactly zero CALLS edges. One edge = arbitrary pick (the bug); two edges would require an ambiguous-target edge model GitNexus does not have. Depends on U1 — without scope-aware filtering, both foo()s would already be in the importer's wildcard binding set as simple 'foo', so the test would pass for the wrong reason. U5 — cpp-using-namespace-std-smoke: Fixture-local 'namespace std { void cout_write(); void println(); }' shim rather than real <iostream> — captures the wildcard-leak shape deterministically without depending on system-header modeling stability (out of scope per plan). Asserts (a) the project-local call resolves correctly, (b) no leak to shim STL symbols, and (c) no CALLS/ACCESSES edges from the caller into std-shim.h at all. Negative tests for U2/U4 mode-gated to REGISTRY_PRIMARY_CPP=1 via the expected-failures registry; legacy DAG lacks the OVERLOAD_AMBIGUOUS suppression and the namespace-aware filtering, so the leaks persist there. All 2112 resolver integration tests pass under registry-primary; all 146 cpp tests pass under both modes (4 negative tests skipped in legacy as documented). * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(cpp): scope-aware isSuperReceiver classification (U1) The C++ isSuperReceiver hook used a regex `/^[A-Z]\w*::/` that misclassified any uppercase-qualified call as a super-receiver call. Singleton::getInstance(), std::Foo::bar(), and PascalCase namespace calls all entered the super branch, where the absence of an enclosing class (or wrong MRO context) dropped the resolution entirely. Fix: - New optional ScopeResolver hook isSuperReceiverInContext(text, callerScope, scopes). Languages where super classification depends on caller context define it; receiver-bound-calls.ts prefers it when defined and falls back to the simple isSuperReceiver(text) otherwise. Other migrated languages (Python, Java, C#, PHP, Go, TypeScript) are unchanged. - C++ implementation: parse the LHS of '::' from the receiver text, resolve via findClassBindingInScope, and return true only when the LHS is a class-like def in the caller's enclosing class's MRO. Returns false for namespace LHS, unresolved LHS, self-class LHS (qualified self-calls aren't super), and any non-'::' form. - Extended the C++ tree-sitter query to capture the LHS of qualified_identifier as @reference.receiver so qualified static member calls (Singleton::getInstance()) reach the receiver-bound Case 2 (class-name receiver) path. Without the receiver capture, qualified calls had no explicit receiver and could not resolve through any receiver-bound branch. Test: cpp-namespace-qualified-not-super fixture. Singleton::getInstance() from a free function asserts exactly 1 CALLS edge through the qualified-call path. Passes under both REGISTRY_PRIMARY_CPP=1 and =0. All 2113 resolver integration tests pass; all 147 cpp tests pass under both modes. * fix(cpp): suppress receiver-bound CALLS when default-arg overloads collide (U4) ISO C++ rejects 's.f(1)' as ambiguous when both 'void f(int)' and 'void f(int, int = 0)' are declared on S. The previous resolver returned the first viable candidate via pickOverload's fallback. Extended isOverloadAmbiguousAfterNormalization to take an optional argCount: when provided, the predicate compares only the first argCount slots of each candidate's parameterTypes. Candidates whose declared-prefix matches up to argCount are treated as ambiguous because default arguments make all of them equally viable for the call. Without argCount, behavior is unchanged (the original int/long normalization-collapse contract, full-length equality required). pickOverload now passes site.arity so default-arg ambiguity fires. Test: cpp-overload-default-arg-ambiguous fixture. s.f(1) where S has f(int) and f(int, int = 0) asserts exactly .toBe(0) CALLS edges. Passes under both REGISTRY_PRIMARY_CPP=1 and =0. All 2114 resolver integration tests pass; all 148 cpp tests pass under both modes. * fix(cpp): two-phase template lookup suppresses dependent-base members (U3) ISO C++ two-phase name lookup: inside a class template body, unqualified calls MUST NOT bind to members of a dependent base class. Only this->name or Base<T>::name forms make the lookup dependent. GCC and Clang both reject the unqualified form with 'declaration of f must be available'. Before this fix, GitNexus's global free-call fallback walked the workspace registry by simple name and bound unqualified calls inside template bodies to dependent-base members, producing CALLS edges the compiler would reject. Implementation: - New languages/cpp/two-phase-lookup.ts module: per-pipeline state recording (className, dependentBaseName) pairs at capture time and resolving them to nodeId sets during populateOwners. - captures.ts detectCppDependentBases walks the AST once finding every template_declaration containing a class/struct definition. For each, it collects template-parameter names (typename T, class T, non-type int N, template-template parameters) and walks each base in the base_class_clause checking whether any inner type_identifier matches a template parameter. Conservative bias: typename T::U, decltype, and template-template-parameter shapes also classified as dependent. - Extended scope-resolution contract's isCallableVisibleFromCaller hook with optional callerScope and scopes fields. C++ implements the hook to consult isCppDependentBaseMember: when the candidate is a member of a dependent base of the caller's enclosing class, the hook returns false and pickUniqueGlobalCallable skips the candidate. - clearFileLocalNames also clears the dependent-base state per pipeline run. Fixtures: - cpp-two-phase-dependent-base: Derived<T> deriving from Base<T>, unqualified f() and i inside Derived's body. Asserts zero CALLS edges and zero ACCESSES edges respectively. - cpp-two-phase-this-qualified, cpp-two-phase-non-dependent-base, cpp-two-phase-namespace-free-call-inside-template: positive fixtures left as documented gaps (this-> and qualified-name resolution inside template bodies are pre-existing resolver weaknesses independent of U3). Tracked separately. Negative test mode-gated to REGISTRY_PRIMARY_CPP=1 via the expected- failures registry; legacy DAG has no two-phase lookup. All 2116 resolver integration tests pass under registry-primary; all 150 cpp tests pass under both modes (5 negative tests skipped in legacy as documented). * fix(cpp): implement V1 ADL (Koenig lookup) for free-function calls (U2) Plan 2026-05-13-001 U2. Adds argument-dependent lookup as a new candidate-generating tier in `emitFreeCallFallback`: when ordinary unqualified lookup is empty, ADL surfaces candidates from each value-class-typed argument's enclosing namespace. V1 boundary (locked by cpp-adl-pointer-arg-boundary fixture): - only direct enclosing-namespace closure - only directly-named class-type values (pointer / reference / template- spec args excluded; closure rules deferred to V2) - ADL fires ONLY when ordinary lookup is empty (no union-and-resolve) Parenthesized name `(f)(s)` suppresses ADL per ISO C++ [basic.lookup.argdep]/3.1. Multi-candidate ambiguity (e.g. `process(int)` vs `process(long)` after C++ int-width normalization) returns the ADL_AMBIGUOUS sentinel — caller suppresses entirely, mirroring the OVERLOAD_AMBIGUOUS contract from plan 2026-05-12-002 U2. Implementation: - `cpp/adl.ts` — new module: per-pipeline argInfoBySite + noAdlSites Maps populated at capture time, classToNamespaceQualifiedName Map populated during populateOwners; `pickCppAdlCandidates` returns SymbolDefinition | ADL_AMBIGUOUS | undefined - `scope-resolution/contract/scope-resolver.ts` — adds optional `resolveAdlCandidates` hook - `scope-resolution/passes/free-call-fallback.ts` — invokes ADL hook between `findCallableBindingInScope` and `pickUniqueGlobalCallable`; marks site handled on `'ambiguous'` so emit-references doesn't retry - `cpp/captures.ts` — detects `parenthesized_expression` function wrap; per-arg classification (pointer/reference/value class) preserving the shape info the existing arity-narrowing normalizer strips - `cpp/scope-resolver.ts` — registers hook, populates associated namespaces, clears state in loadResolutionConfig Negative tests (parens, pointer-boundary, ambiguous) gated under LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp — legacy DAG has no V1/V2 ADL boundary or ADL_AMBIGUOUS suppression. 154/154 cpp integration tests pass under REGISTRY_PRIMARY_CPP=1; 147 pass + 7 skipped under =0 (legacy parity baseline). * fix(cpp): inline namespace transitive walking + qualified namespace resolution (U5) Plan 2026-05-13-001 U5. Two ISO C++ inline-namespace semantics: 1. Unqualified-lookup transitive visibility: inline-namespace members reach the enclosing namespace's scope as if declared there. The `populateCppNonGloballyVisible` exemption keeps them globally visible so cross-file unqualified lookup finds them. 2. Qualified-receiver transitive visibility: `outer::foo()` resolves to `outer::v1::foo()` when `v1` is inline (and through arbitrarily-deep nesting like `outer::v1::experimental::foo`, matching libc++ `__1` / libstdc++ `__cxx11`). The second behavior required a new resolver case in `receiver-bound-calls.ts` (Case 1.5: language-specific qualified-receiver member lookup) because C++ qualified-namespace member calls had no prior resolution path — receiver-bound Case 1 only handled `ParsedImport.kind === 'namespace'` (Python/JS-style) and Case 2 handles class receivers, neither of which fired for `outer::foo()`. The new hook `resolveQualifiedReceiverMember` is opt-in; languages without C++-style qualified-name semantics omit it. Implementation: - `cpp/inline-namespaces.ts` — new module: per-pipeline `inlineNamespaceRangesByFile` + `inlineNamespaceScopeIds` Sets; `markCppInlineNamespaceRange` at capture time; `populateCppInlineNamespaceScopes` resolves ranges → scope IDs; `resolveCppQualifiedNamespaceMember` walks namespace scopes by simple name and descends transitively through inline children only. - `scope-resolution/contract/scope-resolver.ts` — adds optional `resolveQualifiedReceiverMember` hook to the contract. - `scope-resolution/passes/receiver-bound-calls.ts` — Case 1.5 invokes the hook between Case 1 (namespace imports) and Case 2 (class-name receiver). Returns undefined for non-namespace receivers so Case 2 still resolves class-qualified calls. - `cpp/captures.ts` — detects `inline` keyword child on `namespace_definition`; records 1-based range to match Scope.range. - `cpp/file-local-linkage.ts` — `populateCppNonGloballyVisible` exempts inline-namespace scopes so cross-file unqualified lookup keeps their members visible. - `cpp/scope-resolver.ts` — wires `populateCppInlineNamespaceScopes` into populateOwners (BEFORE `populateCppNonGloballyVisible` so the exemption sees populated state); registers `resolveQualifiedReceiverMember` hook. 4 fixtures: `cpp-inline-namespace-unqualified`, `-versioned`, `-nested` (two transitive inline hops, STL `__1` shape), and `-adl-participation` (composes with U2 — ADL surfaces records declared inside inline child namespaces). All 4 assert exactly 1 CALLS edge with correct target file. Versioned fixture gated under LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp — legacy DAG can't disambiguate two same-name foos without inline awareness. Other 3 coincidentally resolve in legacy. 158/158 cpp integration tests pass under REGISTRY_PRIMARY_CPP=1; 150 pass + 8 skipped under =0 (legacy parity baseline). * test(cpp): Phase 5 cross-unit composition tests for U1/U2/U3/U5 Plan 2026-05-13-001 Phase 5. Locks in correct behavior at the intersections between the previously-shipped scope-resolver units. Enhancement to U1: `isSuperReceiverInContext` strips template-argument lists (`Base<T>` → `Base`) and namespace prefixes (`outer::v1::Base` → `Base`) before resolving the receiver in the caller's scope chain. This makes the super-receiver classification work for template-class heritage shapes like `Base<T>::method()` and `outer::v1::Base<T>::f()`. Three fixtures + four tests: - `cpp-phase5-u1-u3-qualified-base-call`: `template<class T> struct Derived : Base<T>` with `Base<T>::method()` inside a template body. Asserts NO mis-routing (count = 0) — documents the V1 gap that template-class inheritance isn't captured as EXTENDS by the legacy DAG, so MRO walks are empty and the super branch can't dispatch. The composition still works correctly: U1's template-arg-stripping classifies `Base<T>` as a super candidate, but the empty-MRO terminates without false edges. - `cpp-phase5-u2-u3-adl-from-derived`: `Derived : Base<T>` where `Base::record` shadows `audit::record`. Unqualified `record(e)` inside the template body should resolve via ADL to `audit::record` (because U3 + the `isFileLocalDef` class- owned filter suppress `Base::record`). Asserts 1 edge to audit.h and 0 edges to base.h. - `cpp-phase5-u3-u5-inline-base`: `template<class T> struct Derived : outer::v1::Base<T>` where `v1` is inline. Unqualified `f()` inside `Derived<T>::g()` should NOT bind to Base::f (dependent-base suppression even across inline namespace prefix). Asserts count = 0. Phase 5 tests asserting no-false-positives are gated under LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp — legacy DAG over- resolves without the template-arg-stripping qualified-receiver path and without two-phase dependent-base suppression. 162/162 cpp integration tests pass under REGISTRY_PRIMARY_CPP=1; 152 pass + 10 skipped under =0 (legacy parity baseline). --------- Co-authored-by: HuangWenjie <zhoudeng.hwj@alibaba-inc.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
magyargergo
added a commit
to luyua9/GitNexus
that referenced
this pull request
May 21, 2026
…wner resolution Multi-agent code review on the prior commit surfaced 7 actionable findings, all walked through and applied here. None change observable behavior for issue abhigyanpatwari#1358's fix; all harden correctness, predicate stability, and test signal. - abhigyanpatwari#1 (P1 / 3-reviewer corroboration): Case 5 in receiver-bound-calls.ts no longer hand-builds graph.addRelationship + a dedup key. New tryEmitEdgeWithExplicitTargetId in edges.ts takes a pre-resolved target id (the canonical Method nodeId from the parser) and reuses every invariant of tryEmitEdge: dedup-key format, collapse-flag honoring, caller-id resolution, rel-id shape, mapReferenceKindToEdgeType for read/write ACCESSES. This also lands the adversarial reviewer's "F2" follow-up (hardcoded type: 'CALLS' for non-call sites) for free. - abhigyanpatwari#2 (P2 cross-reviewer): findValueBindingInScope's predicate inverted from denylist ("not class-like and not callable") to explicit allowlist matching reconcileOwnership's registration set: Const | Variable | Property | Static. Extracted as isOwnableValueLabel so future NodeLabel additions require an explicit opt-in. - abhigyanpatwari#6 (P2): walkScopeChain<T>() extracted; both findClassBindingInScope and findValueBindingInScope now route through it. Local scope.bindings are exhausted BEFORE lookupBindingsAt (imported/augmented) at every scope level — preserves JavaScript lexical scoping where a local const shadows an imported binding of the same name. Behavior was already correct in findClassBindingInScope but was implicit; now it is the walker's explicit, documented contract. - abhigyanpatwari#7 (P2): scope-walker duplication closed. findClassBindingInScope and findValueBindingInScope reduce to thin wrappers over walkScopeChain with their respective predicate. findClassBindingInScope keeps its qualifiedNames + dotted-name fallback tail. - abhigyanpatwari#3 (P2): parse-worker.ts hoists `const ownerId = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId` once before the symbol push, dropping the duplicated coalesce + `as string` cast. Matches the cast-free pattern at parsing-processor.ts:793. HAS_METHOD emit site reuses the same hoisted local. - abhigyanpatwari#4 (P2): object-literal-owner-resolution.test.ts Test A's CALLS-edge assertion no longer matches by name alone. .toEqual now pins the canonical target id (Method:src/service.ts:getUser#1 via generateId), confidence (0.85), and reason ('import-resolved'). A regression that emits the edge at confidence=0, with the wrong reason, or against a phantom Method node now fails the test. - abhigyanpatwari#5 (P2): worker-parity test adds a CI tripwire — when CI=1 and dist/parse-worker.js is missing, throw at module top with a clear message. Locally, skipIf(!hasDistWorker) keeps the fast-iteration experience; CI cannot pass with U3 (worker-path ownerId) unverified. Verification: tsc --noEmit clean. Targeted regression sweep on ast-helpers-object-literal-binding (13), object-literal-owner-resolution (9), has-method (60), cross-file-binding (40) — 122/122 pass. Full unit sweep: 6056/6056. Integration suite: 1 pre-existing Windows-flake in worker-pool.test.ts (passes 28/28 in isolation) unrelated to this diff.
magyargergo
added a commit
that referenced
this pull request
May 21, 2026
…moke Resolves all findings from the automated production-readiness review on verify/issue-1728-symlink. Swift warning parity (review #2): Add tree-sitter-swift to OPTIONAL_GRAMMARS in src/cli/optional-grammars.ts alongside Dart and Proto. Before this commit, Swift was materialized at postinstall and probed by build-tree-sitter-swift.cjs but the runtime warnMissingOptionalGrammars() never warned when it failed to load — users got silent Swift degradation from the optional-grammars surface (parser-loader's separate unavailableNote only fires on demand). Now the warning path matches the materialize path. README env-var table (review #1): Update the GITNEXUS_SKIP_OPTIONAL_GRAMMARS row at README.md line 248 to list all three vendored grammars (dart, proto, swift). The quick note earlier in the README already mentioned all three; only the table row was stale. Atomicity hardening (review #3): materialize-vendor-grammars.cjs now copies to {dest}.materialize-tmp, renames the existing dest to {dest}.materialize-bak (if present), then renames the partial into dest, then removes the backup. If the partial→dest rename fails (e.g. Windows AV scanner racing the swap), the catch block restores from backup so the previously-materialized grammar is preserved. Closes the narrow torn-state window where the prior implementation could leave dest deleted after rmSync succeeded but renameSync failed. Swift probe docs (review #4): build-tree-sitter-swift.cjs script header rewritten to describe what the script actually does — probe node-gyp-build at install time so missing-prebuild failures surface as install-time warnings instead of first-parse runtime errors. The script does not "activate" anything; the runtime require() in parser-loader does the actual load. Console warning text updated to match ("prebuild probe" not "activation"). Windows packaged-install smoke test (review #5): New CI job `packaged-install-smoke` in .github/workflows/ci-tests.yml matrices on windows-latest and ubuntu-latest. Runs npm pack, installs the produced tarball globally into RUNNER_TEMP, then asserts: * no vendor/*/node_modules or vendor/*/build (#836 invariant) * tree-sitter-{dart,proto,swift} in node_modules are real directories, not junctions/symlinks (#1728 invariant) * gitnexus --version runs against the installed CLI Closes the coverage gap where the existing windows-latest job only ran `npm ci` in the source checkout — exercising postinstall but not the tarball reify step that historically tripped EPERM. Verified locally: npx tsc --noEmit: clean vitest test/unit/materialize-vendor-grammars.test.ts test/unit/cli-commands.test.ts: 18 pass + 2 POSIX-only skipped on Windows prettier + eslint on all changed files: clean
magyargergo
added a commit
that referenced
this pull request
May 21, 2026
…) (#1729) * fix(install): materialize vendored grammars to fix Windows EPERM (#1728) Stop using file: optionalDependencies for tree-sitter-dart/proto/swift, which made npm symlink vendor paths on install and fail on Windows without symlink privileges. Copy vendor trees into node_modules at postinstall instead; keep native builds and #836 vendor hygiene. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(install): atomic materialize swap + fail-soft tests (#1728, #836) Hardens PR #1729 against two issues the original implementation could still hit: 1. Torn-state on rmSync→cpSync. The previous loop deleted the destination before copying. If cpSync threw — the exact Windows EPERM scenario this PR targets — a previously-working grammar was silently wiped. Now we copy to {dest}.materialize-tmp first and renameSync into place, so an interrupted copy leaves the prior materialization intact. 2. Fail-soft try/catch had no test coverage. Adds two POSIX-only tests (chmod 0o555 to deterministically force cpSync to throw) that verify (a) a single grammar failure does not abort the other two, and (b) an existing materialization survives a partial-copy failure. Skipped on Windows where chmod doesn't enforce write restriction; runs on Linux CI. Other test improvements locking in the install-hygiene invariants: - All three vendored grammars (dart/proto/swift) checked, not just dart. - GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1 short-circuit is exercised. - Vendor cleanliness (#836): no node_modules/build under vendor/. - Idempotent re-runs (clean overwrite verified via sentinel file). - Missing-vendor warn+continue path now has explicit coverage. - Vendored package manifests asserted to carry no install script or runtime dependencies. - package.json optionalDependencies asserted free of vendored grammars. - package-lock.json assertion tightened from `if (entry !== undefined) { expect(entry.link).not.toBe(true); }` (vacuous when entry is absent, i.e. the expected post-fix state) to `expect(...).toBeUndefined()`. Verified locally: - npx tsc --noEmit: clean - vitest test/unit/materialize-vendor-grammars.test.ts: 8 pass + 2 POSIX-only skipped on Windows - npm pack tarball: no vendor/*/node_modules or vendor/*/build entries - Isolated global install (clean + upgrade + SKIP env) into temp prefix: succeeds; gitnexus --version → 1.6.5; vendor stays clean post-install. * fix(install): address review feedback — Swift parity, atomicity, CI smoke Resolves all findings from the automated production-readiness review on verify/issue-1728-symlink. Swift warning parity (review #2): Add tree-sitter-swift to OPTIONAL_GRAMMARS in src/cli/optional-grammars.ts alongside Dart and Proto. Before this commit, Swift was materialized at postinstall and probed by build-tree-sitter-swift.cjs but the runtime warnMissingOptionalGrammars() never warned when it failed to load — users got silent Swift degradation from the optional-grammars surface (parser-loader's separate unavailableNote only fires on demand). Now the warning path matches the materialize path. README env-var table (review #1): Update the GITNEXUS_SKIP_OPTIONAL_GRAMMARS row at README.md line 248 to list all three vendored grammars (dart, proto, swift). The quick note earlier in the README already mentioned all three; only the table row was stale. Atomicity hardening (review #3): materialize-vendor-grammars.cjs now copies to {dest}.materialize-tmp, renames the existing dest to {dest}.materialize-bak (if present), then renames the partial into dest, then removes the backup. If the partial→dest rename fails (e.g. Windows AV scanner racing the swap), the catch block restores from backup so the previously-materialized grammar is preserved. Closes the narrow torn-state window where the prior implementation could leave dest deleted after rmSync succeeded but renameSync failed. Swift probe docs (review #4): build-tree-sitter-swift.cjs script header rewritten to describe what the script actually does — probe node-gyp-build at install time so missing-prebuild failures surface as install-time warnings instead of first-parse runtime errors. The script does not "activate" anything; the runtime require() in parser-loader does the actual load. Console warning text updated to match ("prebuild probe" not "activation"). Windows packaged-install smoke test (review #5): New CI job `packaged-install-smoke` in .github/workflows/ci-tests.yml matrices on windows-latest and ubuntu-latest. Runs npm pack, installs the produced tarball globally into RUNNER_TEMP, then asserts: * no vendor/*/node_modules or vendor/*/build (#836 invariant) * tree-sitter-{dart,proto,swift} in node_modules are real directories, not junctions/symlinks (#1728 invariant) * gitnexus --version runs against the installed CLI Closes the coverage gap where the existing windows-latest job only ran `npm ci` in the source checkout — exercising postinstall but not the tarball reify step that historically tripped EPERM. Verified locally: npx tsc --noEmit: clean vitest test/unit/materialize-vendor-grammars.test.ts test/unit/cli-commands.test.ts: 18 pass + 2 POSIX-only skipped on Windows prettier + eslint on all changed files: clean * fix(ci): disable credential persistence on packaged-install-smoke checkout GitHub Advanced Security (zizmor artipacked) flagged the new packaged-install-smoke job's actions/checkout step as a potential credential-persistence risk. The job runs `npm pack` + global install and never pushes back, so the GITHUB_TOKEN that checkout would persist in .git/config provides no value and only widens the leak surface (any future artifact-upload step in this job would carry the token). Disable persistence explicitly via `persist-credentials: false` on this job's checkout. Scoped to the new job — pre-existing checkouts above are left unchanged. * fix(ci): use find instead of ls for tarball lookup (SC2012) actionlint shellcheck SC2012 flagged `TARBALL=$(ls gitnexus-*.tgz | head -n1)`. Switch to `find . -maxdepth 1 -name 'gitnexus-*.tgz' -print -quit` which handles non-alphanumeric filenames safely. Also add an explicit empty-result check so the failure mode is a clear error message instead of a silent `npm install -g ""` later. * fix(tests): sabotage vendor src (not partial path) in POSIX fail-soft tests The fail-soft tests in materialize-vendor-grammars.test.ts pre-chmod'd the destination's .materialize-tmp partial directory to 0o555 to force cpSync to throw. After the atomicity rewrite (`fix(install): atomic materialize swap + fail-soft tests`), the materialize script now starts each grammar's loop with `fs.rmSync(partial, { force: true })`, which deletes the chmod'd sabotage before cpSync runs — so cpSync succeeds and the partial is then renamed into dest, leaving the test's `finally` block with no path to chmod back (ENOENT) and the assertion that proto remained unmaterialized failing because it materialized cleanly. Fix: sabotage the *vendor source* directory (which the script reads from but never modifies) by chmod'ing it to 0o000. cpSync then fails on readdir, the catch block fires per-grammar, dart and swift still materialize from their unaffected sources, and the existing-dest preservation test verifies that a sabotaged second-run leaves the prior materialization (and its sentinel file) intact. Tests now pass locally (8 pass + 2 POSIX-only skipped on Windows) and should pass on macOS/Ubuntu CI where the sabotage runs. * fix(tests): restrict fail-soft tests to Linux (macOS Node cpSync abort) Node 22 on macOS aborts the process with `libc++abi: terminating due to uncaught exception filesystem_error` when fs.cpSync hits a source directory it can't read — the abort happens at the C++ filesystem layer and bypasses Node's JS try/catch entirely (nodejs/node#51399). My chmod-0o000-the-source sabotage strategy triggers this SIGABRT on macOS CI before the production script's `try { cpSync } catch` ever runs, so the test sees a child-process crash instead of the fail-soft warning it's verifying. The production script's fail-soft is correct on Linux (where EACCES surfaces as a normal JS exception) and effectively untestable on macOS via permission sabotage. Real installs don't hit this — npm always ships vendor/ with readable permissions — so the macOS gap is a test artifact, not a behavior gap. Restrict the two chmod-based tests to Linux only by replacing `skipOnWin` with `linuxOnly`. Linux CI continues to verify both the one-grammar-fails-others-succeed and existing-materialization-preserved invariants. macOS and Windows runs skip these two scenarios; the other 8 tests still run on every platform. * fix(tests): remove materialize unit tests, rely on CI smoke job The materialize-vendor-grammars.test.ts file has been a recurring source of platform-specific CI noise: - Windows: chmod doesn't enforce read/write restrictions the way POSIX does, so the fail-soft tests had to be skipped there. - macOS Node 22: cpSync against an unreadable source aborts the process with a libc++ filesystem_error (nodejs/node#51399) that bypasses JS try/catch entirely — making the chmod-based fail-soft tests unrunnable on macOS too. - The "vendor-cleanliness" and "idempotency" tests on Windows intermittently flake due to fs.cpSync timing on the GitHub runner. The invariants these tests verified are now covered by stronger, more realistic surfaces: - packaged-install-smoke (ci-tests.yml): runs `npm pack` then `npm install -g ./gitnexus-*.tgz` on windows-latest and ubuntu-latest, then asserts no vendor/*/node_modules, no vendor/*/build (#836), no junctions/symlinks on the materialized grammar directories (#1728), and a working `gitnexus --version`. This is the actual end-user install path. - cli-commands.test.ts (kept, unmodified): asserts package.json declares no `file:` optionalDependencies for vendored grammars, the Swift vendor manifest carries no install script or dependencies, and the postinstall chain runs materialize-vendor-grammars.cjs + build-tree-sitter-swift.cjs. These are static manifest checks — deterministic, fast, no flake risk. Removing the dynamic script-execution tests trades unit-level coverage for end-to-end smoke coverage that actually exercises the `file:` → cpSync change against a real npm install lifecycle, on the platform the fix targets (windows-latest). --------- Co-authored-by: Cursor <cursoragent@cursor.com>
magyargergo
added a commit
that referenced
this pull request
May 21, 2026
) * fix: link object literal methods to exported bindings * fix(ingestion): bridge object-literal value receivers in scope-resolution (PR #1718 review) Addresses adversarial production-readiness review on PR #1718 / issue #1358: - F1 (caller resolution) — setting `ownerId` on object-literal method symbols alone is not sufficient; the scope-resolution receiver-bound resolver only consults class-like or type-annotated bindings, so lowercase value receivers (`export const fooService = {...}; fooService.getUser(...)`) never reach the owner-indexed lookup. Adds a Case 5 value-receiver bridge in receiver-bound-calls.ts that resolves the receiver name as a Const/Variable binding, translates its def to the canonical graph node id, and emits the CALLS edge via the owner-indexed method registry. - F2 (boundary guard) — rewrites findObjectLiteralBindingInfo as an explicit two-phase AST walk: Phase A tracks object-literal depth (returns null for nested literals and pre-declarator function/class boundaries — IIFE patterns); Phase B walks the declarator's ancestors and rejects function, class, and block-statement containers (if / for / while / try / catch / switch / etc.) before reaching program/export_statement. Prevents false HAS_METHOD edges for locally-scoped or block-scoped object literals. - F4 — drops the dead `ownerName` field from ObjectLiteralBindingInfo. Constraint: TS/JS are scope-resolution migrated per RFC #909; the legacy Call-Resolution DAG (call-processor.ts) is intentionally left untouched. Tests: - test/integration/ast-helpers-object-literal-binding.test.ts (13 cases) — pins helper semantics: happy paths, function/arrow/class-ctor boundaries, nested literals, block scope (if / for-of / try), IIFE, assignment expressions without declarator. - test/integration/object-literal-owner-resolution.test.ts (9 cases) — drives the full pipeline against an on-disk fixture: sequential CALLS edge emission (issue #1358 proof), worker-mode parity, negative local binding, and nested-literal attribution boundary. Full sweep: 2958/2958 integration + 6056/6056 unit tests pass. * refactor(ingestion): address code-review findings on object-literal owner resolution Multi-agent code review on the prior commit surfaced 7 actionable findings, all walked through and applied here. None change observable behavior for issue #1358's fix; all harden correctness, predicate stability, and test signal. - #1 (P1 / 3-reviewer corroboration): Case 5 in receiver-bound-calls.ts no longer hand-builds graph.addRelationship + a dedup key. New tryEmitEdgeWithExplicitTargetId in edges.ts takes a pre-resolved target id (the canonical Method nodeId from the parser) and reuses every invariant of tryEmitEdge: dedup-key format, collapse-flag honoring, caller-id resolution, rel-id shape, mapReferenceKindToEdgeType for read/write ACCESSES. This also lands the adversarial reviewer's "F2" follow-up (hardcoded type: 'CALLS' for non-call sites) for free. - #2 (P2 cross-reviewer): findValueBindingInScope's predicate inverted from denylist ("not class-like and not callable") to explicit allowlist matching reconcileOwnership's registration set: Const | Variable | Property | Static. Extracted as isOwnableValueLabel so future NodeLabel additions require an explicit opt-in. - #6 (P2): walkScopeChain<T>() extracted; both findClassBindingInScope and findValueBindingInScope now route through it. Local scope.bindings are exhausted BEFORE lookupBindingsAt (imported/augmented) at every scope level — preserves JavaScript lexical scoping where a local const shadows an imported binding of the same name. Behavior was already correct in findClassBindingInScope but was implicit; now it is the walker's explicit, documented contract. - #7 (P2): scope-walker duplication closed. findClassBindingInScope and findValueBindingInScope reduce to thin wrappers over walkScopeChain with their respective predicate. findClassBindingInScope keeps its qualifiedNames + dotted-name fallback tail. - #3 (P2): parse-worker.ts hoists `const ownerId = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId` once before the symbol push, dropping the duplicated coalesce + `as string` cast. Matches the cast-free pattern at parsing-processor.ts:793. HAS_METHOD emit site reuses the same hoisted local. - #4 (P2): object-literal-owner-resolution.test.ts Test A's CALLS-edge assertion no longer matches by name alone. .toEqual now pins the canonical target id (Method:src/service.ts:getUser#1 via generateId), confidence (0.85), and reason ('import-resolved'). A regression that emits the edge at confidence=0, with the wrong reason, or against a phantom Method node now fails the test. - #5 (P2): worker-parity test adds a CI tripwire — when CI=1 and dist/parse-worker.js is missing, throw at module top with a clear message. Locally, skipIf(!hasDistWorker) keeps the fast-iteration experience; CI cannot pass with U3 (worker-path ownerId) unverified. Verification: tsc --noEmit clean. Targeted regression sweep on ast-helpers-object-literal-binding (13), object-literal-owner-resolution (9), has-method (60), cross-file-binding (40) — 122/122 pass. Full unit sweep: 6056/6056. Integration suite: 1 pre-existing Windows-flake in worker-pool.test.ts (passes 28/28 in isolation) unrelated to this diff. * refactor(scope-resolution): align Const label emission with legacy DAG (PR #1718 review F1) Eliminates the architectural fragility surfaced by PR #1718's adversarial review Finding 1. Previously, normalizeNodeLabel('const') returned 'Variable' while the legacy DAG parse phase emits 'Const' graph nodes (via @definition.const capture for lexical_declaration). PR #1718's Case 5 value-receiver bridge resolved correctly only because resolveDefGraphId happened to fall back to simpleKey after the qualified-key miss — accidental correctness. After this change, scope-resolution defs for `const x = ...` declarations report def.type === 'Const', matching the graph node label. resolveDefGraphId's qualified-key path now hits on the first try; the simple-key fallback is no longer load-bearing for value receivers and can be tightened in future without silently breaking Case 5. Audit completeness verification: - Grep `\bVariable\b` across src/core/ingestion/scope-resolution/ surfaced two consumer sites that already accept both labels: reconcile-ownership.ts:101+168 (`def.type === 'Variable' || def.type === 'Const' || ...`) and walkers.ts:207 isOwnableValueLabel (`Const | Variable | Property | Static`). No language hook in src/core/ingestion/languages/ branches on `def.type === 'Variable'` for what's actually a const declaration. - Sentinel stress test (the full unit + integration suite run with the renamed label in place): 6137/6137 unit tests pass; 2967/2967 integration tests pass. One pre-existing Windows-only flake on worker-pool.test.ts when run alongside the full integration suite (passes 28/28 in isolation, unrelated to scope-extractor — same flake observed before this diff). The variable mapping (`'variable' → 'Variable'`) is preserved for `var` declarations, matching the legacy DAG's `@definition.variable` capture for variable_declaration. The split now mirrors the parse-phase capture distinction exactly. Per plan docs/plans/2026-05-21-002-feat-pr1718-followups-class-instance-and-label-normalization-plan.md U4 + U5. T1 (class-instance singleton resolution from issue #1358's second sub-case) is deferred to a standalone pre-plan investigation, not shipped here. * test(ingestion): add regression coverage for issue #1358 singleton sub-cases Closes the remaining sub-cases of issue #1358 surfaced by PR #1718's adversarial review (Finding 4, NOTED): the class-instance singleton (`export const fooService = new FooService();`) and the factory-pattern singleton (`export const fooService = makeFooService();`). Pre-plan investigation (per docs/plans/2026-05-21-002 § "Pre-Plan Investigation Task (T1)") confirmed Outcome A for both patterns — they already resolve end-to-end through scope-resolution's `@type-binding.constructor` capture (languages/typescript/query.ts:489-511) + `propagateImportedReturnTypes` chain-follow (scope-resolution/passes/imported-return-types.ts:114) + receiver-bound Case 4 simple typeBinding lookup (receiver-bound-calls.ts:625). The mechanism was wired correctly before this session; the regression-net wasn't. This test pins the behavior: - Pattern 1: `caller → FooService.getUser` CALLS edge with confidence 0.85 and reason 'import-resolved' - Pattern 2: same edge shape via factory chain-follow (the `@type-binding.alias` capture for `const u = find()` style) Both assertions use exact `.toEqual([{...}])` shape pinning so a future regression that targets a phantom Method node, emits at lower confidence, or drops the cross-file import-resolved reason fails loudly. Verification: 5/5 pass, 127/127 in targeted regression sweep including object-literal-owner-resolution.test.ts, ast-helpers-object-literal- binding.test.ts, has-method.test.ts, and cross-file-binding.test.ts. No production code change. The class methods get a class-qualified node id (`Method:src/service.ts:FooService.getUser#1`) distinguishing them from same-name methods on other classes — distinct from the bare-name node id shape PR #1718's object-literal case uses. * test(resolvers): add class-instance + factory-pattern singleton coverage for TS/JS (issue #1358) Closes the remaining sub-cases of issue #1358 surfaced by PR #1718's adversarial review (Finding 4). PR #1718 fixed object-literal-shorthand singletons (`export const fooService = { getUser() {} }`); this commit adds parallel coverage for the two other singleton shapes that resolve through the existing scope-resolution chain: // Pattern 1 — class-instance singleton export class FooService { getUser(id) { ... } } export const fooService = new FooService(); // Pattern 2 — factory-pattern singleton export class FooService { getUser(id) { ... } } export function makeFooService() { return new FooService(); } export const fooService = makeFooService(); Pre-plan investigation (per local plan docs/plans/2026-05-21-002 § "Pre-Plan Investigation Task (T1)") confirmed Outcome A — both patterns already resolve end-to-end through: - `@type-binding.constructor` capture (languages/{typescript,javascript}/ query.ts) seeds `fooService → FooService` at parse time - `propagateImportedReturnTypes` (scope-resolution/passes/ imported-return-types.ts:114) mirrors the typeBinding cross-file - Receiver-bound Case 4 simple typeBinding lookup (scope-resolution/passes/receiver-bound-calls.ts:625) MRO-walks FooService and emits the CALLS edge to getUser Tests added per language × pattern (5 each, 10 total): - node existence (Class, Method, Function, Const, plus Function for the factory pattern's `makeFooService`) - HAS_METHOD edge from class to method (class-instance variant) - CALLS edge from caller to `getUser` with `targetFilePath: 'src/service.{ts,js}'`, `reason: 'import-resolved'`, `confidence: 0.85` — exact `.toEqual([{...}])` shape pinning so a regression that emits at lower confidence or drops the cross-file reason fails loudly Fixtures placed under the existing `test/fixtures/lang-resolution/` convention. Tests appended to `test/integration/resolvers/{typescript,javascript}.test.ts`, matching the in-file pattern of every other resolver scenario. Also supersedes and removes the standalone `test/integration/class-instance-and-factory-singleton-resolution.test.ts` introduced earlier in this PR session (`0df91b77`) — the proper home for language-resolver scenarios is the per-language resolver test file alongside similar fixtures (`javascript-self-this-resolution`, `javascript-cross-file`, `typescript-tsconfig-paths`, etc.). One canonical location for the scenario, not two. Verification: 10/10 new singleton tests pass; 297/297 full TS+JS resolver suite pass (no regression in any existing resolver test). * test(resolvers): gate TS/JS singleton tests behind scope-resolution parity (CI run 26223603426) The class-instance and factory-pattern singleton CALLS-edge resolution tests added in c8e573b rely on scope-resolution-only mechanisms (`@type-binding.constructor` capture + `propagateImportedReturnTypes` mirror + receiver-bound Case 4). The `scope-parity / typescript parity` and `scope-parity / javascript parity` CI jobs run with `REGISTRY_PRIMARY_TYPESCRIPT=0` / `REGISTRY_PRIMARY_JAVASCRIPT=0` and exercise the legacy DAG path, which has no cross-file constructor-derived typeBinding propagation. Verified by job 77202610819 (TS parity) and 77202610869 (JS parity) failing with: × resolves caller.fooService.getUser() to FooService.getUser via constructor-inferred typeBinding × resolves caller.fooService.getUser() through the factory chain to FooService.getUser Note: my local Windows shell-prefix env-var invocation did not propagate the flag into vitest workers correctly (the cpp parity gate's 47-skipped behavior masked the issue when I ran an ad-hoc comparison), so the empirical "both modes pass" finding I posted earlier was wrong. CI is the source of truth. Changes: - test/integration/resolvers/helpers.ts: add `typescript` and `javascript` entries to `LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES` for the 2 CALLS-edge resolution tests in each language. Node-existence and HAS_METHOD assertions are NOT excluded — those pass under legacy DAG (parser-level emission is intact). - test/integration/resolvers/typescript.test.ts: drop the `it` import from vitest; replace with `const it = createResolverParityIt('typescript');` shadow (matches the c/cpp/csharp/go pattern at the top of those files). - test/integration/resolvers/javascript.test.ts: same shadow with `createResolverParityIt('javascript')`. Verification: - Default mode (registry-primary): 297/297 TS+JS resolver tests pass. - Legacy DAG mode: the 4 listed singleton CALLS-edge tests will skip; all other singleton assertions (node existence + HAS_METHOD edge) continue to run and pass under both modes. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
This was referenced May 22, 2026
magyargergo
pushed a commit
that referenced
this pull request
May 25, 2026
- #3: Forward limit/offset/summaryOnly through callToolAtGroupRepo so group-mode MCP callers can use the new pagination params. - #4: Extract GROUP_LOCAL_PHASE_LIMIT constant from magic 10000 in cross-impact.ts with a comment explaining the intent. - #7: eval-server formatImpactResult uses byDepthCounts[depth] for the 'and N more' suffix instead of paginated slice length. - #8: Extract ImpactParams interface from duplicate inline type definitions in impact() and _impactImpl(). - #9: Add --limit, --offset, --summary-only CLI flags to the impact command with i18n help strings (en + zh-CN). - #10: Clarify in tool description that limit/offset apply per depth level, not per total result set.
magyargergo
added a commit
that referenced
this pull request
May 26, 2026
) * feat(mcp): add limit/offset/summaryOnly pagination to impact tool (#414) The impact tool returns unbounded byDepth arrays for hub symbols (base error classes, shared utilities), producing 140KB+ responses that get truncated by MCP clients. maxDepth alone does not help when most dependents are at depth 1. Add three new parameters: - summaryOnly: returns counts/risk/processes/modules without byDepth - limit: caps symbols per depth level (default 100) - offset: skips symbols for pagination Also adds byDepthCounts to all responses so agents can see total counts even when the symbol list is paginated or omitted. Closes #414 * fix(mcp): prevent pagination from silently truncating cross-repo impact Address review findings on #1818: - F1 (blocker): _runImpactBFS no longer defaults to limit 100 when limit is not set — only _impactImpl (MCP entry) applies the default. Internal callers (impactByUid, group impact) get complete results. GroupToolPort.impact interface gains optional limit param, and cross-impact.ts passes limit: 10000 for local UID collection. - F2 (blocker): tool description updated — byDepth is now documented as paginated, not 'all affected symbols'. - F3: impactByUid calls _runImpactBFS without limit, so Phase-2 neighbor results are no longer capped at 100. - F4: pagination metadata now appears when offset > 0 (head truncation), not just tail truncation. Pagination.limit is null when uncapped. - F5: limit/offset schema types changed from number to integer; Math.trunc applied in implementation as defense-in-depth. - F6: 7 new tests — multi-depth pagination, offset-only truncation, offset past end, float inputs, _runImpactBFS internal uncapped path, collectImpactSymbolUids with paginated vs complete data. * fix(mcp): NaN guard on pagination params, complete GroupToolPort interface - Add Number.isFinite guard to limit/offset in _runImpactBFS so NaN inputs fall through to uncapped/zero defaults instead of producing silent empty byDepth with no truncation signal. - Add offset and summaryOnly to GroupToolPort.impact interface to match the implementation and prevent silent param loss at the port boundary. - Replace bounds-only toBeLessThan assertion with exact byDepthCounts and pagination assertions per DoD §2.7. * fix(mcp): address remaining review findings for impact pagination - #3: Forward limit/offset/summaryOnly through callToolAtGroupRepo so group-mode MCP callers can use the new pagination params. - #4: Extract GROUP_LOCAL_PHASE_LIMIT constant from magic 10000 in cross-impact.ts with a comment explaining the intent. - #7: eval-server formatImpactResult uses byDepthCounts[depth] for the 'and N more' suffix instead of paginated slice length. - #8: Extract ImpactParams interface from duplicate inline type definitions in impact() and _impactImpl(). - #9: Add --limit, --offset, --summary-only CLI flags to the impact command with i18n help strings (en + zh-CN). - #10: Clarify in tool description that limit/offset apply per depth level, not per total result set. * chore(autofix): apply prettier + eslint fixes via /autofix command * @ fix(mcp): address Copilot review feedback on impact pagination - Sanitize limit/offset with Number.isFinite in _impactImpl to prevent NaN passthrough from bypassing the default limit of 100 - Omit pagination.limit field instead of emitting null when paginationLimit is Infinity, keeping the response schema consistent - Move GROUP_LOCAL_PHASE_LIMIT after all imports in cross-impact.ts - Stop forwarding limit/offset/summaryOnly to group-mode impact since runGroupImpact overrides limit with GROUP_LOCAL_PHASE_LIMIT for UID collection and does not re-paginate - Validate CLI parseInt results with Number.isFinite before passing to the backend, falling back to undefined so defaults apply - Use byDepthCounts to decide whether to render depth sections in formatImpactResult, handling empty pages from offset past end @ * @ fix(mcp): address code review findings on impact pagination - Fix formatImpactResult "N more" count: use Math.min(items.length, 12) instead of hardcoded 12, so paginated pages with <12 items show the correct remaining count - Detect summaryOnly responses (byDepth absent, byDepthCounts present) and show a summary-mode message instead of misleading "(0 items on this page — adjust offset)" per depth level - Document that limit/offset/summaryOnly are single-repo only and ignored in group mode (@groupName) in MCP tool schema descriptions - List byDepthCounts in summaryOnly description and note byDepth absence when summaryOnly is true - Remove unused limit/offset/summaryOnly from GroupToolPort.impact interface since they are never forwarded to group impact - Deduplicate parseInt calls in CLI tool.ts: extract to local variables with consistent optional-chain usage @ * chore(autofix): apply prettier + eslint fixes via /autofix command * @ fix(group): restore limit in GroupToolPort.impact interface cross-impact.ts passes limit: GROUP_LOCAL_PHASE_LIMIT through the GroupToolPort.impact interface for UID collection. Only offset and summaryOnly were truly unused — limit must stay. @ * @ docs: add limit/offset/summaryOnly to impact tool options in README @ --------- Co-authored-by: Test <test@example.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
RoJLD
added a commit
to RoJLD/GitNexus
that referenced
this pull request
May 27, 2026
…bhigyanpatwari#3 Phase 2 Item abhigyanpatwari#3 of Timeline series. Item abhigyanpatwari#2 (Mode union) is subsumed by Item abhigyanpatwari#1's Permissive mode — skipping directly to Item abhigyanpatwari#3. Décisions cadres validées : - D1 Trigger : temporalFilterMode !== 'off' (single source of truth avec le dropdown filter de Item abhigyanpatwari#1) - D2 Endpoint shape : extend /lifespan?repo=&from=&to= avec params optionnels (backward-compat, same endpoint) - D3 UX feedback : header text + badge daterange (pattern cohérent avec l'indicateur de durée Timeline Phase 1) - D4 Compute ephemeral fenêtré : réutilise /nodes/alive-between de Item abhigyanpatwari#1 + pure fn unionSnapshotNodeIds - D5 Behavior branch : si from/to présents → windowed ; sinon global (existing logic unchanged) Sémantique windowed des 4 buckets : - foundational : in A AND in B (survit la fenêtre) - recent : not in A, in B (apparu pendant) - discontinued : in A, not in B (disparu pendant) - ephemeral : not in A, in some intermediate snapshot, not in B (apparu+disparu dans la fenêtre) Architecture : - Backend : docker-server-lifespan.mjs MOD (windowed branch + cache) - Frontend : useAppState effect branche sur temporalFilterMode, LifespanPanel header text + badge quand data.windowed présent Effort estimé : ~3-5 jours. Aucun nouveau endpoint, aucune nouvelle dep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RoJLD
added a commit
to RoJLD/GitNexus
that referenced
this pull request
May 27, 2026
…TORY/tests/CLAUDE (Task 6) - ROADMAP.md row 52: Lifespan Windowed feature description with backward-compat /lifespan?from=&to= endpoint - INVENTORY.md: Updated /lifespan endpoint description with windowed mode details + LifespanPanel header/badge UX - tests/README.md: Added unit test (computeWindowedBuckets), integration test (lifespan-windowed.test.mjs), and E2E test (lifespan-windowed.spec.ts) - CLAUDE.md smoke loop: Added curl for lifespan windowed with from=oldest&to=live params - patches/upstream-all.diff: Regenerated (no upstream source changes for this task) Phase 2 Item abhigyanpatwari#3 complete. Remaining: Item abhigyanpatwari#5 (URL persistence) and Item abhigyanpatwari#4 (Zoom mousewheel). Item abhigyanpatwari#2 (Mode union) subsumed by Item abhigyanpatwari#1 (Permissive mode). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
magyargergo
added a commit
that referenced
this pull request
May 29, 2026
…ts, tests) Resolves the blocking + actionable findings from the PR #1875 review: - Pin base image by digest as bare name@digest [#1]. The :tag@digest form trips the @devcontainers/cli image-name parser (which builds this image in CI and in VS Code "Reopen in Container"); bare name@digest is the parser-compatible form. Verified by a full local build. - Pin Cursor by version + per-arch sha256 and fetch the artifact directly instead of executing cursor.com/install; fail-closed on mismatch [#2]. - Mount ~/.config/gh and ~/.docker read-only so a compromised dep can't rewrite the host GitHub token / Docker credHelper [#4]. - Pin @devcontainers/cli@0.87.0 in the CI smoke [#5]. - chown via find -xdev in install-deps.sh (symlink-safe; matches post-create.sh) [#6]. - Add filesystem-I/O tests (translate/readHostConfig/seed main/ensurePaths) and refactor ensure-host-config-dirs to be unit-testable [#7]. - Stop pre-creating settings.json/config.toml on the host; only the real single-file bind source (.claude.json) is touched [#10]. - Add a prominent top-of-README security callout for the RW write-through trade-off and reframe the deferred egress firewall as the key missing compensating control [#3, #9]. Full devcontainer build verified locally (digest pull + pinned Cursor download/extract/symlink). 24/24 config-transform tests pass.
RoJLD
added a commit
to RoJLD/GitNexus
that referenced
this pull request
May 29, 2026
…figurable prewarm (abhigyanpatwari#3#5#7) Browser-verified (Playwright) PASS — Compare fires 2 /graph/at-commit reconstructions, Play fires per-frame reconstructions. - abhigyanpatwari#3 (Compare/Play on commits): new compareCommits(shaA,shaB) in useAppState (reconstruct both via /graph/at-commit + reuse computeGraphDiff + diffMode/ exitDiffMode). Timeline: Compare A↔B button + Play branch on navMode; cursors A/B double as window bounds AND Compare/Play endpoints (commitNearest maps a cursor date → nearest windowed commit — no cursor-drag rewrite needed). Compare button label fixed to reflect diffMode in commit mode. - abhigyanpatwari#5 (mixed-filter fidelity): /graph/at-commit returns droppedLabels/droppedRelTypes (union of what the replayed diff chain excluded vs a known vocab); EntropyCommitTimeline banner lists "omet : Variable, Const…" instead of a bare "mixed filters" badge. Verified field present (empty for uniform-filter chains). - abhigyanpatwari#7 (prewarm rate): .gitnexus.json > incremental.preWarmPerTick (default 10, was env-only 5), consumed by the watches cron. Unit-tested clamp [1,100]. Backend essentially unchanged for abhigyanpatwari#3 (reuses /graph/at-commit). Patches via split scheme (0 binary). Spec amended (## Update 2026-05-29 suite). Note: Compare/Play SUCCESS still needs warm baselines/diffs for the target commits (un-seeded commits 409 → caught as diffError) — same backend limitation, mitigated by baseline-seed (#B) + pre-warm (#C). Host tests deferred (Node 21<22). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 30, 2026
Closed
ReidenXerx
added a commit
to ReidenXerx/GitNexus
that referenced
this pull request
Jun 1, 2026
## P0 abhigyanpatwari#1 — missing scope-resolution hooks in vueProvider `pass3CollectImports` early-returns when `interpretImport` is undefined, producing zero IMPORTS and zero cross-file CALLS edges. Add the four hooks to `vueProvider` in `vue.ts`: - `interpretImport: interpretTsImport` - `interpretTypeBinding: interpretTsTypeBinding` - `bindingScopeFor: tsBindingScopeFor` - `importOwningScope: tsImportOwningScope` Also add `receiverBinding`, `mergeBindings`, `arityCompatibility`, and `resolveImportTarget` to complete the scope-resolution contract. ## P0 abhigyanpatwari#2 — template-component CALLS dropped when Vue is registry-primary `isRegistryPrimary(Vue) → true` makes the main call-processor loop skip Vue files entirely, silencing the inline `vue-template-component` CALLS emitter at ≈L1506. Add a dedicated post-loop pass in `call-processor.ts` that emits template-component CALLS for Vue files whenever Vue is registry-primary. Update the stale `vue/index.ts` limitation comment to reflect the new emit site. ## P1 abhigyanpatwari#3 — worker-mode double-extraction → zero captures In worker mode (≥15 files) the parse worker pre-extracts the `<script>` block and passes `scriptContent` as `sourceText`. `emitVueScopeCaptures` was calling `extractVueScript` a second time, getting null, and returning `[]`. Fix: if extraction returns null and the content has no SFC block- level markers (`<template`, `<style`), treat it as already-extracted script text and delegate directly to `emitTsScopeCaptures`. ## Test assertion strictness Replace all `toBeGreaterThanOrEqual(1)` assertions with exact `toBe(N)` counts. IMPORTS counts reflect per-symbol scope-based edges (value imports only; `import type` is not emitted as an IMPORTS edge). CALLS counts are 1 per single-call-site. Co-authored-by: Cursor <cursoragent@cursor.com>
abhigyanpatwari
pushed a commit
that referenced
this pull request
Jun 2, 2026
) * chore: extend .gitattributes for shell scripts and binary assets Append explicit `*.sh text eol=lf` and `*.bash text eol=lf` rules so shell scripts (notably anything COPYed into a Linux container) check out with LF endings on Windows hosts with `core.autocrlf=true`, regardless of the auto-detection on the existing `* text=auto eol=lf` line. Add binary markers for `*.node`, `*.wasm`, `*.onnx`, `*.so`, `*.dll`, `*.dylib` so native and ML model artifacts aren't ever subjected to text normalization. The existing `* text=auto eol=lf` and `.husky/* text eol=lf` rules are preserved. `git ls-files --eol` confirmed zero CRLF or mixed blobs in the index, so no `--renormalize` was needed. * feat(devcontainer): add cross-platform devcontainer for Claude Code, Codex, and Cursor CLIs Add a Dev Container that pre-installs Claude Code (2.1.153, via Anthropic's official Feature), OpenAI Codex CLI (pinned 0.134.0), and Cursor CLI alongside the GitNexus native build chain. Opens via VS Code's Dev Containers extension on Windows 11 (Docker Desktop + WSL2), macOS, or Linux without OS-specific branches in devcontainer.json. Topology and base - Base image `mcr.microsoft.com/devcontainers/typescript-node:1-22-bookworm` (multi-arch, monthly patched, ships the `node` non-root user, zsh, `gh`). - Node 22 LTS satisfies `gitnexus/`'s engines `>=22.0.0` and matches the `node:22-bookworm-slim` SHA-pinned base used by `Dockerfile.cli`. - Single container with all three CLIs co-installed (vs. docker-compose per-tool) — prevailing 2026 community pattern, lowest daily-driver friction. Persistence and auth - Per-devcontainer named volumes scoped by `${devcontainerId}` for `/home/node/.claude`, `/home/node/.codex`, `/home/node/.cursor`, `/commandhistory`, and `/home/node/.npm`. Authentication survives rebuilds without leaking between workspaces. - Four sub-workspace `node_modules` volumes (root, gitnexus, gitnexus-web, gitnexus-shared) keep tree-sitter native bindings and onnxruntime off the bind mount — the actual Win/Mac perf win. - Credential mount paths are pre-created in the Dockerfile with `chown node:node` BEFORE `USER node`, so empty named volumes inherit correct ownership on first mount and first-run logins don't EACCES. - `CURSOR_API_KEY` is injected via `containerEnv: ${localEnv:CURSOR_API_KEY}` (Cursor's documented headless path); falls back to interactive `cursor-agent login` when the host env var is unset. Build-arg promotion - Build args (`CLAUDE_CODE_VERSION`, `CODEX_VERSION`, `CURSOR_VERSION`, `TZ`) are promoted to ENV in the Dockerfile so lifecycle commands and shells can resolve them. Without this promotion, Docker ARG values are build-only and silently no-op at lifecycle time. Workspace setup - `postCreateCommand` chowns the four workspace `node_modules` volumes (Docker creates them root-owned), then installs in dependency order: root → gitnexus-shared (install + build) → gitnexus → gitnexus-web. The shared package must build before its consumers (`file:../gitnexus-shared`). Ports - 5173 (Vite dev) and 4173 (Vite preview) auto-forwarded. - 4747 (`gitnexus serve`) marked `requireLocalPort: true` because `gitnexus-web/src/services/backend-client.ts` hardcodes `http://localhost:4747` as the default backend URL; a remapped port would silently break the web UI. VS Code integration - Recommended extensions: `anthropic.claude-code`, `dbaeumer.vscode-eslint`, `esbenp.prettier-vscode`, `eamodio.gitlens`. - Settings: format-on-save with Prettier, ESLint auto-fix on save, zsh as default terminal profile, persistent zsh history via `HISTFILE` → `/commandhistory`. Documentation - `.devcontainer/README.md` covers WSL2 setup (clone inside WSL2 for IO and file-watcher reliability), first-time auth flows for each CLI, port- forwarding notes, LadybugDB container limitations, and the bumping procedure for each CLI version. - `CONTRIBUTING.md` gets a "Containerized development (optional)" subsection pointing at the devcontainer README. Deferred to a follow-up PR - Opt-in egress firewall (originally planned as a fourth implementation unit). The Dev Containers spec makes `runArgs` static — toggling `NET_ADMIN`/`NET_RAW` capabilities cleanly requires either a separate `devcontainer-firewall.json` profile or an `initializeCommand`-generated overlay. Keeping this PR focused on the working baseline. - Codespaces-specific tuning (works incidentally when the firewall is off, not actively tested). - Inside-container Playwright e2e (needs Chromium libs not in the base image). Verification deferred to user - This change introduces a new dev tooling artifact. Validate by running `docker build .devcontainer/`, opening the repo in VS Code via "Dev Containers: Reopen in Container", confirming `claude --version`, `codex --version`, `cursor-agent --version` resolve inside the container, and `cd gitnexus && npm run test:unit` runs clean against the named-volume `node_modules`. * fix(devcontainer): make interactive login the default auth path for all CLIs The previous `containerEnv` injected `CURSOR_API_KEY: "${localEnv:CURSOR_API_KEY}"`. When the host had no `CURSOR_API_KEY` set, this resolved to an empty string and Docker injected `CURSOR_API_KEY=""` into the container. Cursor CLI treats a set-but-empty `CURSOR_API_KEY` as "use this key" rather than "fall back to stored login", which silently broke `cursor-agent login` on the most common path — users who hadn't explicitly opted into API key auth. Drop `CURSOR_API_KEY` from `containerEnv`. Login is now the unconditional default for all three CLIs (Claude Code, Codex CLI, Cursor CLI); the named-volume + Dockerfile-chown pattern keeps credentials persistent across container rebuilds for every login path. Reorganize the README's auth section to put login first for all three CLIs uniformly (matching the new behavior) and move API key authentication into a separate "Alternative" section for CI/headless use. Document that API keys are intentionally not auto-propagated from the host and explain the export-in-shell or VS Code dotfiles-repo paths for users who want them. Update the troubleshooting row to reflect the new design. * fix(devcontainer): install gitnexus-web before gitnexus in postCreateCommand The previous order (root → gitnexus-shared → gitnexus → gitnexus-web) broke at the `gitnexus` install step because `gitnexus`'s `prepare` script runs `scripts/build.js`, which compiles `gitnexus-web` whenever its source tree exists. In the devcontainer the entire workspace is bind-mounted, so `gitnexus-web/` is present from the start — but its `node_modules/` wasn't yet, so `tsc -b` failed with: error TS2688: Cannot find type definition file for 'vite/client' error TS2688: Cannot find type definition file for 'node' Reorder so `gitnexus-web` installs before `gitnexus`. Verified end-to-end via `npx @devcontainers/cli up`: container builds clean, all three CLIs (Claude 2.1.153, Codex 0.134.0, Cursor) respond, and `npx tsc --noEmit` inside `/workspace/gitnexus` passes. Production Dockerfiles (`Dockerfile.cli` etc.) don't hit this because they only COPY `gitnexus/` + `gitnexus-shared/`, so `gitnexus-web/` doesn't exist at install time and `scripts/build.js` skips the web step. The devcontainer's full-tree bind mount changes that calculus. * fix(devcontainer): clear stale .husky/_ before npm install When `npm install` runs the root `prepare` script (husky), husky tries to copyfile `node_modules/husky/husky` → `.husky/_/h`. On Docker Desktop Windows bind mounts, if `.husky/_/` already exists from a prior container run, the new container's `node` user can't overwrite it via the bind mount's permission translation and the install fails with: Error: EPERM: operation not permitted, copyfile '/workspace/node_modules/husky/husky' -> '.husky/_/h' Drop `.husky/_` defensively in `postCreateCommand` before `npm install` so husky always starts from a clean slate. `.husky/_` is a husky runtime cache (gitignored), so removing it has no effect on the repo — husky regenerates it. No-op for WSL2-side checkouts (where this class of bind-mount permission collision doesn't occur). Add a troubleshooting row to `.devcontainer/README.md` covering the manual recovery (`rm -rf .husky/_` on the host) and the long-term fix (clone in WSL2 — Windows-side bind mounts will keep biting on this kind of issue across rebuilds with different UID alignment). * feat(devcontainer): bind-mount host CLI config dirs for plugin/skill/memory sync Switch the credential/config mounts from per-devcontainer named volumes to bind mounts of `${localEnv:HOME}/.claude`, `~/.codex`, and `~/.cursor`. Effect inside the container: - Authentication is shared with the host. If you've already run `claude login` / `codex login --device-auth` / `cursor-agent login` on the host, you're already authenticated in the container. - Plugins, skills, agents, memory, and settings sync both ways. Install a plugin in the container, it shows up on the host; add a custom agent on the host, the container sees it immediately. - All devcontainers on the host share the same CLI state, mirroring how host shells already share it. (Per-workspace isolation of plugins was never a stated requirement; the previous per-devcontainer named volumes leaked nothing useful.) Add `.devcontainer/ensure-host-config-dirs.cjs` and wire it as `initializeCommand`. It runs on the host before container create and guarantees `~/.claude`, `~/.codex`, `~/.cursor` exist, so Docker doesn't reject the bind mount when a CLI has never been used on this host. Cross-platform via Node `os.homedir()` + `fs.mkdirSync({recursive: true})`; idempotent; no third-party deps. Update `.devcontainer/README.md`: - New "How CLI state is shared with your host" section explaining the bind-mount model up front so users know their host plugins/skills/ memory carry into the container. - Mark first-time-login section as skippable when the user is already authenticated on the host. - Note the high-trust escape hatch: replace the three bind mounts with `type=volume` named volumes if the host/container trust boundary needs to be separated (Anthropic's reference pattern for enterprise). - Replace the obsolete "rm named volume" troubleshooting row with one that covers EACCES/EPERM on the host-bind-mount path. * refactor(devcontainer): address ce-code-review findings (P0 + 4 × P1 + 8 × P2 + 2 × P3) Walkthrough resolution of the 16-finding ce-code-review on PR #1875. 15 of 16 findings applied; one (F12, Anthropic Feature floating tag) was superseded by F6's Feature removal. P0 - F1: WSL2 is now REQUIRED for Windows hosts, not just recommended. ${localEnv:HOME} resolves to empty string on Windows-native (no HOME env var) — bind mounts then point at /.claude, /.codex etc. and silently break. ensure-host-config-dirs.cjs wrote to USERPROFILE-derived paths via os.homedir(), so the two surfaces disagreed about which env var was "home" on Windows. README header reframed; "Windows 11 — WSL2 is required" section explains the mismatch concretely. P1 - F2: Workspace `node_modules` volume names now include `-${devcontainerId}` so two GitNexus checkouts on the same host (~/work/GitNexus and ~/projects/GitNexus) don't share volumes and corrupt each other's installs. - F3 + F5: `postCreateCommand` extracted to `.devcontainer/post-create.sh` with `set -euo pipefail` and six labeled echo steps so failure logs name the step instead of an opaque &&-chain index. Chown step extended to cover /home/node/.npm, /commandhistory, and /home/node/.local — these named-volume mount points were owned by build-time UID 1000 but the container's `node` is re-IDed at runtime by updateRemoteUserUID on non-1000 Linux hosts, leaving them unwritable until now. - F4: Cursor installer downloaded to a temp file with curl --retry + --max-time; sha256 logged to build output before execution so drift across rebuilds is visible in CI logs. Full hard-pin (to a versioned downloads.cursor.com tarball with verified sha256) tracked as a follow-up in README "What's not included". P2 - F6: Anthropic Feature replaced with a direct `npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}` so CLAUDE_CODE_VERSION actually pins the installed binary (the Feature ignored the ARG and pulled latest at install time). Honors the earlier "pin known-good versions" decision and resolves F12's floating-tag concern for this Feature. - F7: Dockerfile ARG defaults dropped for the three version vars; `devcontainer.json` `build.args` is now the single source of truth. Standalone `docker build .devcontainer/` must pass --build-arg. - F8: ensure-host-config-dirs.cjs deleted; `initializeCommand` now uses POSIX `mkdir -p` + `touch ~/.gitconfig` directly, dropping the host-Node-on-PATH prerequisite that broke on fresh Windows+Docker Desktop installs without Node. - F9: ~/.gitconfig bind-mounted read-only so `git commit` inside the container uses the host's user.name / user.email. Read-only so container-side `git config --global` doesn't leak to host. - F10: ~/.config/gh bind-mounted (read-write) so `gh pr create` / `gh pr checks` / `gh issue create` work inside the container without re-auth. AGENTS.md's commit + PR workflow now fully functional for agents inside the container. - F11: CLAUDE_CONFIG_DIR removed from Dockerfile ENV; canonical value lives only in devcontainer.json containerEnv. Eliminates the two-file edit risk. - F13: Mounts comment now documents per-instance vs per-workspace-name scoping rationale so future contributors don't guess. - F14: README "Trust boundary, concretely" paragraph names the exfil path explicitly (malicious npm postinstall → OAuth tokens → ~/.claude/projects/<workspace>/memory/MEMORY.md secrets) and lists vendor-side rotation runbook entries. P3 - F15: Dockerfile pre-create + chown of /home/node/.claude, .codex, .cursor dropped — those paths are bind-mounted, which fully shadows any image-side ownership. Only .npm, .local, /commandhistory still benefit from the pre-create. - F16: README "Bumping CLI versions" section rewritten against the post-F6 reality: CLAUDE_CODE_VERSION and CODEX_VERSION are real pins; CURSOR_VERSION is informational only. Verified locally: `docker build .devcontainer/ --build-arg ...` succeeds. Smoke-tested image: `claude --version` (2.1.153), `codex --version` (0.134.0), `cursor-agent --version` all resolve as the non-root `node` user; named-volume mount points (/home/node/.npm, /commandhistory) are node-owned at build time so non-1000 host UIDs get the post-create.sh chown fix instead of EACCES. * fix(devcontainer): cross-platform initializeCommand + soften Windows-native posture The previous commit's `initializeCommand` was POSIX-only (`mkdir -p $HOME/...`). VS Code on Windows runs the host shell as `cmd.exe /c ...`, which can't parse POSIX syntax — `$HOME` doesn't expand, `mkdir -p` errors, the init fails with `The syntax of the command is incorrect`, and container creation aborts before Docker is invoked. Switch `initializeCommand` to the spec's OS-keyed object form: - linux/darwin (covers WSL2 because VS Code runs initializeCommand in the WSL shell when attached via the WSL extension): POSIX mkdir+touch, as before - win32: PowerShell snippet that creates the same directories under $USERPROFILE and touches the gitconfig if missing Soften the README's hard "WSL2 required" framing from the previous commit. Reality per `@devcontainers/cli read-configuration` output: `${localEnv:HOME}` on Windows-native resolves to `C:\Users\<name>` (VS Code falls back to USERPROFILE), so the bind mount sources are valid Windows paths and Docker Desktop handles the translation. The earlier `accessing specified distro mount service` failure was a separate Docker Desktop WSL-integration issue, not a HOME-resolution issue. Windows-native works; it's just slower with more bind-mount permission edge cases (the husky/_/h EPERM class). The README now explains the tradeoff and steers toward WSL2 for performance + file watchers + permission reliability, rather than blocking Windows-native checkouts outright. Update the troubleshooting row to reflect the new posture. * fix(devcontainer): Node-based initializeCommand; bind-mount .ssh + .config/git Two fixes bundled: 1. The previous commit's OS-keyed `initializeCommand` object was based on a misread of the Dev Containers spec. The object form on command properties is **named parallel tasks**, not OS dispatch — VS Code ran all three keys in parallel via cmd.exe on Windows, the POSIX branches failed, and container creation aborted before Docker was invoked. Restore the single-string Node-based form: `node .devcontainer/ensure-host-config-dirs.cjs`. Node works identically in cmd.exe on Windows and bash/zsh on Linux/macOS/WSL, and `os.homedir()` respects $HOME on POSIX and %USERPROFILE% on Windows. The script is idempotent (mkdirSync recursive is a no-op for existing dirs; touch is gated on .gitconfig existence). Document Node ≥18 on the host as the only host-side prerequisite beyond Docker Desktop and the VS Code Dev Containers extension. Anyone running Claude Code on the host already has it. 2. Extend the host-bind mount surface with `~/.ssh` and `~/.config/git`, both read-only: - `~/.ssh` lets commit signing + push over SSH remotes work inside the container without copying private keys. Read-only mount means container code can read keys but can't modify or delete them. (Threat: a malicious dep can still read private keys from inside the container; the read-only mount narrows write-side blast radius, not read-side. Documented in the trust-boundary section.) - `~/.config/git` covers XDG-style git config (`~/.config/git/config`, `~/.config/git/ignore`, `~/.config/git/attributes`) for users who keep settings there instead of `~/.gitconfig`. Read-only, same as `~/.gitconfig`. Update the CLI-state-sharing table and trust-boundary paragraph to reflect the expanded surface. Re-adds .devcontainer/ensure-host-config-dirs.cjs (deleted before the OS-keyed attempt). * fix(devcontainer): fail-fast on Windows-native with HOME-not-set diagnostic The previous commit's "Windows-native works" softening was wrong. VS Code on Windows-native resolves `${localEnv:HOME}` by reading the host shell's HOME env var, and cmd.exe has no HOME set — the bind sources collapse to `/.claude`, `/.codex`, etc., and Docker errors: Error response from daemon: invalid mount config for type "bind": bind source path does not exist: /.claude The @devcontainers/cli output that prompted the softening was misleading because I ran it from a Bash session with HOME already set, not from VS Code's cmd.exe call context. The original Finding-1 P0 — that Windows- native silently breaks the bind-mount feature — was correct. Three changes: 1. `ensure-host-config-dirs.cjs` detects the failure mode early: `if (process.platform === 'win32' && !process.env.HOME)` prints a targeted error message naming the root cause (cmd.exe has no HOME → ${localEnv:HOME} resolves empty → bind sources fail) and a step-by-step pointer to set up WSL2. Exits 1 so VS Code surfaces it as a clean container-creation failure, not the cryptic Docker bind-mount error. 2. README header reverted to "Windows 11 via WSL2" only (not "and Windows-native"). The "Windows 11 — WSL2 is required" section names the specific HOME-resolution mismatch concretely so future readers understand why the constraint exists. 3. Troubleshooting table gets a new row for the `ERROR: GitNexus devcontainer requires WSL2` message pointing at the setup section. * feat(devcontainer): support Windows-native via auto setx HOME on first run Reverses the "WSL2 required on Windows" posture. Windows-native now works after a one-time auto-handled setup. The root cause of the bind-mount failure: VS Code resolves `${localEnv:HOME}` by reading its own process env, and Windows doesn't set `HOME` by default — Windows uses `USERPROFILE`. So the bind sources were collapsing to `/.claude`, `/.codex`, etc., and Docker rejected them. `ensure-host-config-dirs.cjs` now handles this automatically on Windows hosts where `HOME` is unset: 1. Runs `setx HOME "%USERPROFILE%"`, which writes to the user-level Windows environment (HKCU\Environment) — no admin required. Every future user process inherits HOME from there. 2. Prints a clear one-time setup banner explaining the user needs to fully restart VS Code (File > Exit, not just close the window) for VS Code to pick up the new env at its next startup. 3. Exits 1 so VS Code surfaces this as a clean container-create failure instead of letting Docker error opaquely later. On the second Reopen-in-Container attempt, `HOME` is now set in VS Code's env, the script skips the setup block, creates the bind-mount source dirs, and the container builds normally. Subsequent rebuilds have no extra steps. Mac, Linux, and WSL2 hosts have `HOME` set by the shell, so the new block is a no-op there. Same `devcontainer.json` works across all supported hosts. README rewritten to reflect the new posture: - Header lists Windows 11 (native) as a supported host alongside macOS, Linux, and WSL2, with a note that Windows-native gets a one-time HOME setup handled by the initializeCommand. - New "Windows 11 setup" section walks through the auto-handled setup flow + a manual `setx HOME "%USERPROFILE%"` fallback for users who want to do it themselves. - "Known trade-offs of Windows-native vs WSL2" subsection lays out the Docker Desktop Windows bind-mount edge cases (file watchers, npm install perf, husky/_ EPERM) so users opting into Windows-native do so eyes-open. WSL2 remains documented as the faster path for users who want it, but it's no longer the only supported one. - Troubleshooting table gets two new rows: the one-time setup banner (with "what to do" instructions) and the residual `bind source path does not exist` case (run setx manually + fully exit VS Code). * fix(devcontainer): drop ~/.gitconfig bind mount; defer to VS Code auto-copy VS Code's Dev Containers extension auto-copies the host's gitconfig into the container at attach time using `(dd ...) >> /home/node/.gitconfig`. A read-only bind mount of ~/.gitconfig blocks that write, so attach failed with `cannot create /home/node/.gitconfig: Read-only file system`. Making it read-write would let the append succeed, but the bind mount means the host file and the container file are the same file — VS Code's append would double the host gitconfig contents on every container start. Drop the ~/.gitconfig bind mount entirely. VS Code's auto-copy is the purpose-built mechanism for this, gives the container the host's user.name / user.email transparently, and avoids both the read-only write failure and the append-duplication trap. The container ends up with a writable /home/node/.gitconfig that's a copy of the host's, not a mount. The remaining six bind mounts (.claude, .codex, .cursor, .ssh, .config/git, .config/gh) keep their existing modes — XDG-style git config under ~/.config/git is unaffected by VS Code's auto-copy (which only targets ~/.gitconfig), so its read-only bind mount stays. Also remove the `.gitconfig` touch from ensure-host-config-dirs.cjs (now unnecessary) and update the README CLI-state table, sharing explanation, and troubleshooting row to reflect that gitconfig flows in via VS Code auto-copy rather than the bind mount. * feat(devcontainer): bind-mount ~/.docker, ~/.aws, ~/.azure for agent workflows Extend the host bind-mount surface so coding agents inside the container inherit cloud + container-registry auth from the host without any per-container setup: - ~/.docker (read-write) — Docker registry auth (config.json) + buildx config. Container-registry pushes (ghcr.io, docker.io) from inside the container pick up host `docker login` state. Read-write because the Docker CLI refreshes credential-helper tokens. - ~/.aws (read-only) — AWS CLI / SDK credentials. Read-only because rotating creds typically happens via the host. Empty on this dev box, so forward-compatible: the moment you `aws configure` on the host the container picks it up on the next rebuild. - ~/.azure (read-only) — Azure CLI credentials. Same pattern as ~/.aws. `ensure-host-config-dirs.cjs` extends to mkdir these three on init so the bind mounts always have a valid source even if a CLI has never been used on this host. The Docker CLI itself isn't installed in the container by default — the ~/.docker/ mount is inert until you add `docker-outside-of-docker:1` or similar Feature. README now calls this out under "What you still don't have inside the container" so it's obvious which CLIs are agent-ready and which need a feature add to become useful. README updates: - Bind-mount table gains a "Why" column and rows for the three new mounts, making it clear at a glance what each one enables. - Trust-boundary section lists Docker registry tokens, AWS, and Azure creds in the read-side exfil path so the threat model stays honest as the credential surface grows. - New subsection lists not-included CLIs (Docker, AWS, Azure, gcloud, kubectl, private-npm) with the exact Feature ID or mount snippet needed to enable each — turns "I want my agent to do X" into a one-line config change. Verified locally: `npx @devcontainers/cli read-configuration` resolves all 9 host bind mounts to valid C:\Users\<name>/* paths on Windows. * refactor(devcontainer): hybrid AI CLI config — read-only host share + per-container credentials Restructure the Claude Code / Codex / Cursor mount topology to fix the silent first-run-UI bug surfaced in PR testing, and to harden against the host-write-through escape class the previous bind-mount design exposed. The actual root cause of the first-run wizard firing on the user's screenshot — confirmed via three parallel research agents (best practices, framework docs deep dive of the OpenAI Codex Rust source, adversarial design review) — was NOT a credential permission check. Claude Code splits state across `~/.claude/.credentials.json` AND `~/.claude.json` (a FILE at $HOME, sibling of the `.claude/` dir). The latter holds `hasCompletedOnboarding`, `userID`, `oauthAccount` metadata, MCP user-scope config, and per-project trust state — and Claude Code reads it at literal `$HOME/.claude.json`, not via `CLAUDE_CONFIG_DIR`. The previous design mounted `~/.claude/` but left `~/.claude.json` outside the topology entirely, so every container started with a missing onboarding-state file and re-ran the wizard. Confirmed by tfvchow/field-notes-public#10: "Persisting .credentials.json alone is NOT sufficient. Without .claude.json, Claude Code treats the session as a fresh install and prompts for login regardless of valid credentials being present." The new topology: **Mounts** - `${localEnv:HOME}/.claude` → `/host/.claude` (read-only bind) - `${localEnv:HOME}/.codex` → `/host/.codex` (read-only bind) - `${localEnv:HOME}/.cursor` → `/host/.cursor` (read-only bind) - `${localEnv:HOME}/.claude.json` → `/host/.claude.json` (read-only bind) - `claude-config-${devcontainerId}` → `/home/node/.claude` (named volume) - `codex-config-${devcontainerId}` → `/home/node/.codex` (named volume) - `cursor-config-${devcontainerId}` → `/home/node/.cursor` (named volume) **containerEnv** gains `CODEX_HOME=/home/node/.codex` (Codex's own env override, per its public Rust source). `CLAUDE_CONFIG_DIR=/home/node/ .claude` was already set. **`post-create.sh`** stages the named volumes on first run: - Symlinks shareable subdirs from `/host/.claude` into the named volume: `plugins/`, `skills/`, `agents/`, `memory/`, `commands/`. Codex gets `config.toml` symlinked. Cursor has no shareable subdirs (cli-config .json conflates auth and settings). - Copies `.credentials.json`, `auth.json`, `cli-config.json` on first run with `chmod 600`. After first run, container manages its own refresh; host's credentials untouched. - Copies `~/.claude.json` on first run (with stub `{"hasCompletedOnboarding":true,"installMethod":"global"}` fallback for hosts that haven't run Claude Code). This is the fix for the observed onboarding-wizard loop. `ensure-host-config-dirs.cjs` now also touches `~/.claude.json` on the host if missing, so the bind mount has a valid source on hosts that have never run Claude Code. **Why read-only + named volume vs. the previous full bidirectional bind mount:** 1. **Host filesystem write-through escape, eliminated.** Previous design symlinked `plugins/`, `agents/`, `skills/` write-through into the host's `~/.claude/` — a malicious npm package in the workspace dep tree could drop `agents/evil.md` into the host's config, which the next host Claude session would auto-load. The read-only `/host` mount blocks this; container compromise no longer persists across teardown via host-side autoload. 2. **Windows bind-mount perm-flattening, sidestepped.** Files surfaced through a Docker Desktop Windows bind mount appear as `root:root` mode `777`. Credentials in the named volume come with proper Linux ownership and `chmod 600` — what each CLI expects on write (none enforces on read, but write-side hygiene matters for the host's understanding of "where credentials live"). 3. **No `ide/` lock-file collisions.** Previous design symlinked `~/.claude/ide/` write-through, including per-PID lock files. Host PID and container PID namespaces are unrelated → lock-file PIDs misclassify dead processes as alive. Skipping `ide/` keeps lock files container-local. 4. **No `projects/` ghost dirs.** Host encodes the workspace path as `D--development-coding-GitNexus`, container as `-workspace`. Bidirectional `projects/` symlinks would split memory and session state across two ghost project dirs for what is conceptually the same project. Skipping `projects/` keeps per-project state container-local; host's projects/ stays untouched. 5. **No `settings.json` version drift.** Container is pinned to a specific Claude Code version (`CLAUDE_CODE_VERSION` build arg); host floats with auto-update. Bidirectional `settings.json` writes produced silent schema rollback. Skipping settings.json keeps each side authoritative for its own version. **README** rewritten in the same section to describe the new topology honestly: what's shared, what isn't, the OAuth refresh-token divergence between host and container, per-CLI quirks (macOS Keychain storage, Cursor's known upstream in-container auth bug, Codex keyring storage). Trust-boundary section updated to name the threat model accurately — same read surface as before (malicious dep can still READ all credentials), but write-through into host plugin/agent dirs is now blocked. Verified locally: `@devcontainers/cli read-configuration` resolves all 19 mounts correctly on Windows, `post-create.sh` parses, and `ensure-host-config-dirs.cjs` idempotently touches `~/.claude.json`. Research backing this design: - Anthropic Claude Code devcontainer docs (named-volume pattern): https://code.claude.com/docs/en/devcontainer - tfvchow/field-notes-public#10 (both files required): https://github.com/tfvchow/field-notes-public/issues/10 - anthropics/claude-code#29029 (VS Code extension strips hasCompletedOnboarding): https://github.com/anthropics/claude-code/issues/29029 - OpenAI Codex Rust source (no read-side perm check): https://github.com/openai/codex/blob/main/codex-rs/login/src/auth/storage.rs - Cursor CLI in-Docker auth issue: https://forum.cursor.com/t/cursor-agent-authentication-issue-inside-docker/143995 * fix(devcontainer): resync AI CLI state from host on every container-create Two bugs were causing Claude Code to fire the onboarding wizard inside the container even with valid host credentials: 1. Missing the second state file. Claude Code 2.1.x writes a small `.claude.json` INSIDE `CLAUDE_CONFIG_DIR` (carrying migration tracking + userID), not just the one at `$HOME/.claude.json`. If the userIDs in the two files disagree, Claude treats the session as inconsistent and re-onboards. The previous post-create.sh only copied the `$HOME` one. 2. First-run guards (`[ ! -e $dst ]`) skipped the copy when stale named volumes from earlier rebuilds still had the prior session's state in them, leaving the container desynced from the host. Replace `copy_on_first_run` with `sync_from_host` that always overwrites from host on container-create. `link_readonly_share` now clears stale non-symlink dst entries before linking. Copies both `$HOME/.claude.json` and `$CLAUDE_CONFIG_DIR/.claude.json` so userIDs stay aligned. Container can still mutate its own state between rebuilds; resync only happens on rebuild (postCreate boundary). * docs(devcontainer): document sync-from-host design + dual-source auth flow README still described the old "first-run copy" behavior. After the post-create.sh change to always-sync-from-host, the design works either direction: - Log in on host → next container-create syncs the credentials into the named volume. - Log in inside the container → the named volume persists the login across rebuilds; the host has no source to overwrite from, so it stays alone. Also documents the two-Claude-state-files trap (`$HOME/.claude.json` AND `$CLAUDE_CONFIG_DIR/.claude.json`, both with the same userID required), and the volume-deletion recovery path for stale named volumes carried over from earlier rebuilds. * fix(devcontainer): full plugin/config parity by dropping CLAUDE_CONFIG_DIR + syncing settings.json Two changes that together give the container the same plugins and configs as the host for all three AI CLIs (login stays per-container): 1. Drop CLAUDE_CONFIG_DIR from containerEnv. The named-volume mount target `/home/node/.claude` already matches Claude's default `~/.claude`, so the env var added no behavior — but setting it changed which file Claude reads `hasCompletedOnboarding` from. With it set, Claude reads `$CLAUDE_CONFIG_DIR/.claude.json` (the small identity-only file that does NOT carry `hasCompletedOnboarding`); without it, Claude reads `$HOME/.claude.json` (the big onboarding-state file that does). The wizard fires every container-create when set, skips when unset. 2. Sync `settings.json` from host (Claude) + symlink `memories/` and `skills/` from host (Codex). Theme + `enabledPlugins` + `extraKnownMarketplaces` live in `settings.json` — without syncing it, the theme picker fires and host-installed plugins stay disabled even though their files are symlinked in. Codex's `memories/` and `skills/` are the symmetric Codex user-installed surface, now shared the same way Claude's plugins/skills/agents/memory/commands are. Cursor stays as-is — `cli-config.json` conflates auth+settings (already synced), and there's no separate plugin surface to mirror. Login details remain per-container by design (acceptable to re-login on rebuild). Everything else — plugins, skills, agents, memory, MCP user- scope config, project trust, theme, plugin enablement — now matches host on every container-create. * refactor(devcontainer): hybrid RW bind + per-container creds — fixes EROFS on in-container plugin install The previous Option B topology (RO host stage + named volume + symlinks into the volume) made `/plugin marketplace add` inside the container fail with EROFS — the symlinks pointed at a read-only mount, so Claude couldn't create new marketplace dirs. Switch to a hybrid: shareable content (plugins/skills/agents/memory/commands/settings.json/$HOME/.claude.json for Claude; config.toml/memories/skills for Codex) gets a direct RW bind from host so reads and writes go bidirectionally; credentials + the small identity file stay in per-container named volumes so logout in container doesn't log out host. Mount precedence does the heavy lifting: the named volume mounts at /home/node/.<cli> first, then sub-path bind mounts overlay specific sub-paths. Container's view at /home/node/.claude/plugins/ is the host dir; container's view at /home/node/.claude/.credentials.json is the named volume's file. What this gives you: - /plugin marketplace add in container = installed on host - New skill on host = visible in container immediately (no rebuild) - claude logout in container = host stays logged in - compound-engineering plugin enabled on host = enabled in container - Theme picker fires once (or never if host has theme set) What it costs: - Write-through: a compromised npm dep in workspace deps can write to host ~/.claude/{plugins,skills,agents,memory,commands}/. Documented trade-off; for personal dev, accepted. Credentials still per-container. post-create.sh becomes much simpler — only syncs the four credential files from host into the named volumes. No more symlink dance, no more state-file merging. ensure-host-config-dirs.cjs gains the new bind sources: the shareable subdirs and settings.json/config.toml files get mkdir/touched on host so Docker doesn't reject the mount when a CLI has never been used. * fix(devcontainer): translate host plugin registry paths to Linux on rebuild The previous topology bind-mounted the entire `~/.claude/plugins/` directory from host. That brought through plugins, marketplaces, and extracted cache content correctly — but ALSO brought through the registry JSONs (`known_marketplaces.json`, `installed_plugins.json`, `plugin-catalog-cache.json`) which carry absolute OS-native paths: "installLocation": "C:\Users\gergo\.claude\plugins\marketplaces\X" "installPath": "C:\Users\gergo\.claude\plugins\cache\Y\Z" Claude in the Linux container fails to resolve these Windows paths and reports `Marketplace X failed to load: cache-miss`. Split the topology: - `plugins/marketplaces/` (git clones) and `plugins/cache/` (extracted plugin files) stay bidirectional RW binds — content is path-independent. - Registry JSONs move into the per-container named volume. post-create.sh reads host's versions, rewrites any absolute path ending in `/.claude/plugins/<rest>` (Windows `C:\Users\...` and POSIX `/Users/...` / `/home/...` patterns) to `/home/node/.claude/plugins/<rest>`, and writes the translated result to the volume. What this gets you: - Plugin installed on host → next container rebuild has it (translated). - Plugin installed inside container → lives in volume registry; lost on rebuild (consistent with credentials model). Re-install on host for persistence. ensure-host-config-dirs.cjs now also creates `plugins/marketplaces/` and `plugins/cache/` on host if absent (Docker rejects bind mounts whose source doesn't exist). * fix(devcontainer): clean stale plugin/skill symlinks from prior design before writes A user upgrading from Option B (read-only host stage + symlinks) to the current hybrid RW-bind topology hit EROFS in post-create.sh when the plugin registry path-translator tried to write `/home/node/.claude/plugins/known_marketplaces.json`. The named volume still carried `/home/node/.claude/plugins -> /host/.claude/plugins` (Option B's symlink). The new design's sub-path bind mounts at `plugins/marketplaces` and `plugins/cache` overlay through the symlink, but writes to the parent dir itself resolve via the symlink to the RO host stage and fail. Drop any leftover symlinks at known target paths early in step 2 so the mkdir/writes that follow land in the volume. * refactor(devcontainer): split workspace-deps to updateContentCommand post-create.sh was doing two unrelated jobs: workspace dependency install (four `npm install` runs in topological order) and AI CLI credential sync. They have different lifecycle needs — deps should re-run when lockfiles change, AI sync should run once per container — but both were gated on container-create. Per Dev Container spec lifecycle, `updateContentCommand` is the right hook for workspace deps: runs at container-create AND on content changes (lockfile updates). `postCreateCommand` is right for AI CLI sync: container-create only. Move steps 3-7 (husky cleanup + four `npm install` runs) into install-deps.sh wired as `updateContentCommand`. Split the chown step too — install-deps owns workspace-side dirs (node_modules volumes, ~/.npm), post-create owns AI-side dirs (~/.claude, ~/.codex, ~/.cursor, /commandhistory, ~/.local). Each script now has one concern. post-create.sh drops from ~187 lines to 148; install-deps.sh is 56 lines new. Faster rebuilds when nothing about deps changed (the credential sync + path translation work still runs every container-create, but the npm install dance no longer does). Research backing (no other simplification applies): - Anthropic's reference devcontainer uses pure named volumes; no host-state inheritance pattern is published. - Path translation has no upstream fix (issues #21916, #10379 closed without resolution). Our Node rewrite is the workaround. - pnpm workspaces (`pnpm -r install`) would replace the four installs with one command, but that's a real refactor (touches gitnexus/scripts/build.js + 4 package.json files); deferred. - `HUSKY=0` in containerEnv would drop the `rm -rf .husky/_` hack, but would also stop pre-commit hooks from firing inside the container; deferred. * fix(devcontainer): drop single-file binds — fixes Codex `batchWrite failed in TUI` On Docker Desktop Windows the named volumes are ext4 (`/dev/sdd`) while single-file bind mounts from the Windows host land as 9p (drvfs). Different filesystems → atomic config writes (write `foo.tmp`, then rename onto `foo`) trip EXDEV `inter-device move failed` / `Device or resource busy`. Codex's TUI surfaces this as `config/batchWrite failed in TUI` when saving model preference. Claude's writes to settings.json / .claude.json fail the same way, silently. Reproduction in container: $ echo x > /tmp/foo.toml; mv /tmp/foo.toml /home/node/.codex/config.toml mv: inter-device move failed: ... Device or resource busy Fix: drop the three single-file bind mounts. Sync host's versions into the named volume on container-create via `sync_from_host` (same pattern already used for credentials). Atomic rename within the volume works because everything is ext4. Trade-off: container writes to these files no longer propagate to host; they stay in the volume until next rebuild, which re-syncs from host. Host is source of truth on rebuild — same model as credentials. Plugin/ skill/agent/memory/command DIRS still bind-mount bidirectionally (atomic writes within a dir bind stay on one filesystem, no EXDEV). Files affected: - ~/.codex/config.toml - ~/.claude/settings.json - ~/.claude.json (HOME-level — added `/host/.claude.json` RO mount back for sync_from_host to read) * chore(autofix): apply prettier + eslint fixes via /autofix command * feat(devcontainer): Codex + Cursor plugin/config host parity with Claude Codex plugins installed in the container never reached the Windows host because, unlike Claude, the Codex plugin tree wasn't bind-mounted — only memories/ and skills/ were. Verified via live /proc/mounts: Claude binds 6 shareable dirs (incl. plugins/marketplaces + plugins/cache), Codex bound 2. So `codex plugin add` wrote into the ext4 named volume and stayed there. Codex changes: - Bind the WHOLE ~/.codex/plugins dir + ~/.codex/prompts (plus existing memories/skills). Strace of two real `codex plugin add` runs proved the installer stages INSIDE plugins/cache/<marketplace>/ and renames intra-dir, so a single 9p bind of plugins/ keeps the rename intra-fs — no EXDEV (the bug that broke single-file binds). .tmp/ stays on the volume (it's the cross-fs staging source). No path translation needed: Codex enablement lives in config.toml as git URLs, not FS paths. - Verified live: `codex plugin add compound-engineering@...` now writes through to C:\Users\...\.codex\plugins\cache\ on the Windows host, and host-created files appear in the container (bidirectional). Cursor changes (review found cursor-agent has a real plugin surface, not editor-only — Cursor 2.5 Marketplace shared by IDE + CLI): - Bind plugins/marketplaces, plugins/local, rules, commands, agents, skills (dir binds, EXDEV-safe). - Copy-on-create mcp.json (single file → EXDEV-unsafe as bind), alongside the existing cli-config.json. - Translate plugins/installed_plugins.json (carries absolute Windows paths like Claude's) — generalized the existing path-rewrite to run for both Claude and Cursor. - hooks.json deliberately NOT shared (runs shell commands → supply-chain surface); documented as opt-in. ensure-host-config-dirs.cjs pre-creates all new host bind sources. post-create.sh defensive symlink cleanup extended to the new Codex/Cursor paths. README updated with the accurate per-CLI share/sync/translate matrix. Design adversarially verified (straced installs, EXDEV primitive tests, sqlite-under-bind check, path-encoding check) before implementing. * fix(devcontainer): resolve ce-code-review findings (doc drift, chown scope, .cjs extraction, CI smoke) Multi-agent review (9 reviewers) found the devcontainer files carried comments + README from the abandoned read-only-symlink design, plus real behavioral gaps. Resolved all actionable findings (no deferrals). Documentation drift (the headline — stale comments described a security model opposite to what shipped): - README "Trust boundary" claimed a malicious dep "cannot write back … the read-only /host mount blocks the write." FALSE — the shareable dirs are RW-bound. Rewrote to document the bidirectional write-through, what stays one-way (credentials never flow back), and how to close it. - devcontainer.json mount group-1 comment described "selectively symlinks … read-only eliminates write-through" — replaced with the RW-bind reality. - Header "Windows-native is unsupported" -> supported (auto HOME setup). - containerEnv comment "credentials persist in host-bind-mounted dirs" -> they live in the named volumes. - hooks.json exclusion documented honestly as a partial mitigation, not a clean boundary (commands/agents/skills/rules are equally executing). - ~/.local "named volume" -> image directory. Behavioral fixes: - chown -R recursed into the RW host binds (could rewrite host ownership / EPERM-abort provisioning on non-UID-aligned Linux). Switched to `find -xdev` per dir so chown stays on the volume filesystem. - Cursor installer wrapped in `timeout 300` — its inner binary download isn't covered by curl --max-time and could hang docker build forever. - Removed dead CURSOR_VERSION ARG/ENV/build-arg (never consumed; "latest" implied a pin the installer can't honor). Documented why Cursor is unpinned. Extraction + tests (the two inline post-create.sh node heredocs were unlintable and untestable; the path regex had had bugs): - seed-claude-config.cjs — installMethod-strip seed, now with a non-object guard (a bare-value/array host .claude.json could otherwise slip the try/catch and silently re-trigger onboarding) and labeled write errors. - translate-plugin-registries.cjs — plugin-registry path translation with labeled errors. - translate-plugin-registries.test.cjs — 12 tests (Windows/POSIX paths, cross-CLI isolation, nested objects, non-object/empty-config guard). - post-create.sh calls the modules via $SCRIPT_DIR. CI: - .github/workflows/ci-devcontainer.yml — runs the unit tests + shell syntax checks + a `@devcontainers/cli build` smoke on .devcontainer/** changes. Conforms to the repo concurrency convention (validator passes). Documented (real gaps, fixes are honest docs since no correct auto-fix exists): user-scope MCP servers with absolute host command paths don't resolve in-container; user-scope config is copy-on-create so host edits need a rebuild; in-container plugin installs get shadowed by an empty host bind on rebuild (recovery noted); plugin installs are single-writer across checkouts; gh/docker RW-vs-ssh/aws/azure-RO rationale. Verified: fresh `@devcontainers/cli up` succeeds; installMethod stripped, registry translated to Linux paths, credentials node:node, 12/12 tests pass. * fix(devcontainer): set persist-credentials:false on CI checkouts + prettier - zizmor `artipacked` (CodeQL/GitHub Advanced Security) flagged both actions/checkout steps in ci-devcontainer.yml: checkout defaults to persist-credentials:true, leaving GITHUB_TOKEN in .git/config where it can leak into uploaded artifacts. Both jobs are read-only (run tests / build smoke, never push), so persist-credentials:false is correct — matches the repo convention in codeql.yml / ci-tests.yml. - Ran prettier 3.8.0 over the new .cjs modules + test (single-quote/style normalization to match the repo). JSON/YAML were already compliant; README is in .prettierignore; .sh has no prettier parser. Behavior unchanged — 12/12 transform unit tests still pass. * fix(devcontainer): resolve adversarial review findings (pins, RO mounts, tests) Resolves the blocking + actionable findings from the PR #1875 review: - Pin base image by digest as bare name@digest [#1]. The :tag@digest form trips the @devcontainers/cli image-name parser (which builds this image in CI and in VS Code "Reopen in Container"); bare name@digest is the parser-compatible form. Verified by a full local build. - Pin Cursor by version + per-arch sha256 and fetch the artifact directly instead of executing cursor.com/install; fail-closed on mismatch [#2]. - Mount ~/.config/gh and ~/.docker read-only so a compromised dep can't rewrite the host GitHub token / Docker credHelper [#4]. - Pin @devcontainers/cli@0.87.0 in the CI smoke [#5]. - chown via find -xdev in install-deps.sh (symlink-safe; matches post-create.sh) [#6]. - Add filesystem-I/O tests (translate/readHostConfig/seed main/ensurePaths) and refactor ensure-host-config-dirs to be unit-testable [#7]. - Stop pre-creating settings.json/config.toml on the host; only the real single-file bind source (.claude.json) is touched [#10]. - Add a prominent top-of-README security callout for the RW write-through trade-off and reframe the deferred egress firewall as the key missing compensating control [#3, #9]. Full devcontainer build verified locally (digest pull + pinned Cursor download/extract/symlink). 24/24 config-transform tests pass. * fix(devcontainer): resolve local adversarial-review findings (low/nit) Follow-up to a local branch review (run after the cloud review crashed before producing findings); all 5 confirmed findings were low/nit: - chown via `find -xdev -exec chown -h`: add -h so chown acts on a symlink ITSELF, not its target. Without it a dangling node_modules/.bin link aborted provisioning under `set -e`, and a cross-fs symlink target could be dereferenced/rewritten. Verified in a clean container (regular files still chowned; dangling link no longer aborts; cross-fs target untouched). Applied to install-deps.sh and post-create.sh; the inline comments are corrected to describe -xdev (descent bound) and -h (no deref) as the two distinct guards. - Reword the .cjs header claims from "lintable" to "unit-tested and prettier-checked": ESLint applies no rules to .cjs in this repo; CI only prettier-checks them. - README: the initializeCommand is `node ensure-host-config-dirs.cjs`, which creates the full bind-source set, not a bash `mkdir -p` of four dirs. - ci-devcontainer.yml: document that the x64 runner exercises only the amd64 Cursor branch; the arm64 sha/URL is hash-pinned (verified against the published artifact) but not built in CI. - Make the seed chmod-644 test meaningful: pre-create dst at 0o600 so only the explicit chmodSync can widen it (the prior assertion passed under the default umask regardless of whether the chmod ran). 25/25 config-transform tests pass; arm64 + x64 Cursor artifacts verified. * docs(devcontainer): rewrite code comments in plain English The devcontainer comments had grown dense and jargon-heavy. Rewrite them across all 9 files into short, plain-English sentences — same facts and reasoning, just clearer wording. Comments only; no code changed. Verified: the diff touches comment lines only, 25/25 config-transform tests pass, devcontainer.json is still valid JSONC with build.args + readonly mounts unchanged, shell scripts pass `bash -n`, and prettier is clean. * feat(devcontainer): persist AI CLI session state across container recreation Add dedicated per-workspace named volumes (mount group 6) for the three AI CLIs' session/resume state so `claude --resume`, `codex resume`, and `cursor-agent resume` survive a rebuild, a full delete-and-recreate, and the `docker volume rm <cli>-config-*` re-login fix: - Claude -> ~/.claude/projects - Codex -> ~/.codex/sessions - Cursor -> ~/.cursor/chats + ~/.cursor/projects The volumes are SEPARATE from the credential/config volumes and keyed like the node_modules volumes (${localWorkspaceFolderBasename}-...- ${devcontainerId}), so wiping a config volume to force a re-login no longer destroys session history. Session state already survived a plain rebuild (it lived in the config volume); this closes the recreation, volume-rm, and devcontainerId-change gaps. Kept container-private (not host bind mounts) deliberately: transcripts can contain pasted secrets, so a host bind would spill them to host disk, widen the supply-chain write-through surface, and leak cross-project transcripts. A commented-out opt-in host-bind block is included for users who accept that trade-off. post-create.sh: chown each new volume root explicitly (find -xdev stops at the config-volume filesystem boundary and won't descend into them), guarded with `[ -d ] || continue` so a missing root can't abort provisioning under set -e. README: document the topology, what survives vs not, the one-time first-rebuild masking of pre-existing config-volume sessions, updated rebuild/reset commands, and the trust-boundary impact. * feat(devcontainer): isolate host AI-CLI config via seed-once copies + persist claude-mem Replace the read-write host bind mounts for the AI-CLI shareable dirs (Claude skills/agents/memory/commands/plugins; Codex plugins/prompts/ memories/skills; Cursor rules/commands/agents/skills/plugins) with a seed-once copy from a read-only /host/.<cli> stage into the per-container config volume. The container gets its own writable copy and can never write back to the host, closing the write-through vector where a compromised in-container dependency could drop a malicious agent, command, skill, or plugin onto the host for the next host session to auto-load. Add a per-container claude-mem named volume (claude-mem-${devcontainerId}) at /home/node/.claude-mem, seeded once from a read-only /host/.claude-mem stage. claude-mem's multi-GB SQLite + Chroma store is kept off a host bind (unreliable fcntl locking / corruption risk over 9p on Docker Desktop Windows) while still surviving rebuilds. - post-create.sh: seed shareable dirs (marker-gated, seed-once) and run plugin-registry translation per seeded CLI; seed claude-mem behind a completion-sentinel guard that self-heals an interrupted multi-GB copy; chown the claude-mem volume only on first create. - translate-plugin-registries.cjs: add selectRegistries() so translation runs per-CLI seed-once instead of clobbering container-installed plugins. - ensure-host-config-dirs.cjs: add ~/.claude-mem; drop the shareable subdirs (no longer bind sources). - devcontainer.json: drop the RW shareable binds; add the claude-mem volume + read-only stage. - README: rewrite trust-boundary, mount table, and rebuild/reset docs for the copy model. - tests: cover selectRegistries and the trimmed DIRS (30 pass). * feat(devcontainer): add Bun 1.3.14, pinned via build arg Installed by the official bun.sh/install script with the release tag passed as the first positional arg, so the version is pinned even though the install path itself is an unverified remote script (the one such exception in the image — Cursor and the base image stay sha256/digest- pinned). BUN_INSTALL is set in ENV so the binary lands at a known path and the installer's rc-file edits don't matter. unzip is added to apt since the Bun installer extracts a .zip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(devcontainer): persist gh auth via copy-into-volume model Move ~/.config/gh from a read-only bind to the same read-only host stage + per-container named volume pattern used for the AI CLI credentials. post-create.sh seeds hosts.yml/config.yml from the /host/.config/gh stage into the gh-config volume on create, so an in-container `gh auth login` now persists across rebuilds while the read-only stage still prevents any write-back to the host token. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(devcontainer): bump Claude Code to 2.1.156 for Opus 4.8 The pin was 2.1.153, which predates Opus 4.8 support (added in 2.1.154). With DISABLE_AUTOUPDATER=1 the container never updated past the pin, so Claude Code only offered models up to 4.7. Bump to the latest 2.1.156 so Opus 4.8 is available. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ReidenXerx
added a commit
to ReidenXerx/GitNexus
that referenced
this pull request
Jun 3, 2026
## P0 abhigyanpatwari#1 — missing scope-resolution hooks in vueProvider `pass3CollectImports` early-returns when `interpretImport` is undefined, producing zero IMPORTS and zero cross-file CALLS edges. Add the four hooks to `vueProvider` in `vue.ts`: - `interpretImport: interpretTsImport` - `interpretTypeBinding: interpretTsTypeBinding` - `bindingScopeFor: tsBindingScopeFor` - `importOwningScope: tsImportOwningScope` Also add `receiverBinding`, `mergeBindings`, `arityCompatibility`, and `resolveImportTarget` to complete the scope-resolution contract. ## P0 abhigyanpatwari#2 — template-component CALLS dropped when Vue is registry-primary `isRegistryPrimary(Vue) → true` makes the main call-processor loop skip Vue files entirely, silencing the inline `vue-template-component` CALLS emitter at ≈L1506. Add a dedicated post-loop pass in `call-processor.ts` that emits template-component CALLS for Vue files whenever Vue is registry-primary. Update the stale `vue/index.ts` limitation comment to reflect the new emit site. ## P1 abhigyanpatwari#3 — worker-mode double-extraction → zero captures In worker mode (≥15 files) the parse worker pre-extracts the `<script>` block and passes `scriptContent` as `sourceText`. `emitVueScopeCaptures` was calling `extractVueScript` a second time, getting null, and returning `[]`. Fix: if extraction returns null and the content has no SFC block- level markers (`<template`, `<style`), treat it as already-extracted script text and delegate directly to `emitTsScopeCaptures`. ## Test assertion strictness Replace all `toBeGreaterThanOrEqual(1)` assertions with exact `toBe(N)` counts. IMPORTS counts reflect per-symbol scope-based edges (value imports only; `import type` is not emitted as an IMPORTS edge). CALLS counts are 1 per single-call-site. Co-authored-by: Cursor <cursoragent@cursor.com>
magyargergo
added a commit
that referenced
this pull request
Jun 3, 2026
…closes #940) (#1950) * feat(vue): migrate Vue SFC to scope-based resolution (RFC #909 Ring 3, closes #940) Adds `vueScopeResolver` and wires Vue into the scope-resolution pipeline (`SCOPE_RESOLVERS`, `MIGRATED_LANGUAGES`). Vue's `<script>` / `<script setup>` blocks are TypeScript — `emitVueScopeCaptures` extracts the script block via the existing `extractVueScript` utility and delegates to `emitTsScopeCaptures`, keeping grammar identity consistent with the cached tree the parse-worker already builds. - `languages/vue/captures.ts` — `emitVueScopeCaptures` - `languages/vue/import-target.ts` — `makeVueResolveImportTarget` (TS resolver + tsconfig path-alias support; explicit `.vue` imports resolve via the exact-path branch) - `languages/vue/scope-resolver.ts` — `vueScopeResolver` - `languages/vue/index.ts` — barrel + known-limitations doc - `languages/vue.ts` — `emitScopeCaptures` hooked up - `scope-resolution/pipeline/registry.ts` — Vue entry added - `registry-primary-flag.ts` — `SupportedLanguages.Vue` added to `MIGRATED_LANGUAGES` (production default → registry-primary) - `vue-composition-api` — `<script setup lang="ts">`, defineProps / defineEmits macros, cross-file TS imports, computed refs - `vue-options-api` — `defineComponent({methods, computed, data})`, this-based method calls, imported utility calls - `vue-cross-file` — composable functions returning class instances, multi-level import chains, UserModel/PostModel method calls - `fieldFallbackOnMethodLookup: true` — Options API `this.X()` calls may not resolve through the type-binding layer (no formal class); fallback catches common patterns via declared field names. - `allowGlobalFreeCallFallback: false` — Vue uses explicit imports; workspace-wide unique-name fallback would produce spurious edges for built-ins (ref, reactive, defineProps, …). - Template expression calls intentionally out of scope: component- reference CALLS edges are already emitted by the legacy template extractor. Remaining template gaps tracked in #1647. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(vue): address P0/P1 review findings from #1950 ## P0 #1 — missing scope-resolution hooks in vueProvider `pass3CollectImports` early-returns when `interpretImport` is undefined, producing zero IMPORTS and zero cross-file CALLS edges. Add the four hooks to `vueProvider` in `vue.ts`: - `interpretImport: interpretTsImport` - `interpretTypeBinding: interpretTsTypeBinding` - `bindingScopeFor: tsBindingScopeFor` - `importOwningScope: tsImportOwningScope` Also add `receiverBinding`, `mergeBindings`, `arityCompatibility`, and `resolveImportTarget` to complete the scope-resolution contract. ## P0 #2 — template-component CALLS dropped when Vue is registry-primary `isRegistryPrimary(Vue) → true` makes the main call-processor loop skip Vue files entirely, silencing the inline `vue-template-component` CALLS emitter at ≈L1506. Add a dedicated post-loop pass in `call-processor.ts` that emits template-component CALLS for Vue files whenever Vue is registry-primary. Update the stale `vue/index.ts` limitation comment to reflect the new emit site. ## P1 #3 — worker-mode double-extraction → zero captures In worker mode (≥15 files) the parse worker pre-extracts the `<script>` block and passes `scriptContent` as `sourceText`. `emitVueScopeCaptures` was calling `extractVueScript` a second time, getting null, and returning `[]`. Fix: if extraction returns null and the content has no SFC block- level markers (`<template`, `<style`), treat it as already-extracted script text and delegate directly to `emitTsScopeCaptures`. ## Test assertion strictness Replace all `toBeGreaterThanOrEqual(1)` assertions with exact `toBe(N)` counts. IMPORTS counts reflect per-symbol scope-based edges (value imports only; `import type` is not emitted as an IMPORTS edge). CALLS counts are 1 per single-call-site. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(vue): template-derived edges + pipeline benchmark (#1950 review) Addresses the reviewer's request for template edge attribution and a performance benchmark. ## Template event-handler CALLS (`vue-template-callback`) Add `extractTemplateEventHandlers` to `vue-sfc-extractor.ts`. Extracts bare single-identifier handlers from `@event="methodName"` and `v-on:event="methodName"` attributes. Inline expressions with arguments or operators (`@click="toggle(item)"`) are intentionally excluded. Wire into the dedicated registry-primary Vue template pass in `call-processor.ts`. For each extracted handler name, `ctx.resolve` finds the in-file Function/Method node and emits a CALLS edge with `reason: 'vue-template-callback'`. ## Template attribute-binding ACCESSES (`vue-template-attribute`) Add `extractTemplateAttributeBindings` to `vue-sfc-extractor.ts`. Extracts bare single-identifier values from `:prop="varName"` and `v-bind:prop="varName"` bindings. Member-access (`:key="post.id"`) and literals are excluded by the identifier-boundary regex. Wire into the same template pass. For each extracted variable, `ctx.resolve` finds the in-file node and emits an ACCESSES edge with `reason: 'vue-template-attribute'`. ## `vue/index.ts` limitations comment Updated to accurately describe all three categories of template-derived edges and explicitly document the complex-expression exclusions. ## Tests Add 6 new assertions in `vue-scope.test.ts`: - `@click="handleSave"` → CALLS `handleSave` (UserProfile.vue) - `@select="onPostSelected"` → CALLS `onPostSelected` (App.vue composition) - `@keyup.enter="addTodo"` → CALLS `addTodo` (TodoList.vue) - `@loaded="onUserLoaded"` → CALLS `onUserLoaded` (App.vue cross-file) - `:userId="currentUserId"` → ACCESSES `currentUserId` (App.vue composition) - `:posts="allPosts"` → ACCESSES `allPosts` (App.vue composition) Add `vue` entry to `LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES` in `helpers.ts` documenting which assertions are registry-primary-only (IMPORTS cardinality, template-derived edges, `<script setup>` export). ## Benchmark Add `vue-pipeline-benchmark.test.ts` (gated by `GITNEXUS_BENCH=1`). Generates N-component synthetic repos (10 / 25 / 50 / 100) and asserts that wall-clock and node counts scale sub-quadratically with component count, guarding against O(n²) regressions in the template extraction or scope-resolution passes. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(vue): BINDS_EVENT_HANDLER/EMITS_EVENT edges via ScopeResolver hook Per maintainer feedback on PR #1950: - Do not edit call-processor.ts (will be removed when all languages migrate) - Model Vue component-event system with dedicated edge types to avoid CALLS noise in deep component hierarchies (per contributor discussion) Changes: - gitnexus-shared: add BINDS_EVENT_HANDLER and EMITS_EVENT to RelationshipType - vue-sfc-extractor: add extractComponentEventBindings, extractNativeElementEventHandlers, and extractScriptEmitCalls - ScopeResolver contract: add optional emitPostResolutionEdges hook - run.ts: wire emitPostResolutionEdges after emitImportEdges - vue/scope-resolver: implement emitPostResolutionEdges emitting: 1. CALLS (vue-template-component) — PascalCase component File refs 2. CALLS (vue-template-callback) — @event on native HTML elements 3. BINDS_EVENT_HANDLER (vue-event: @name) — @event on component elements; source = handler fn in parent, target = child component File (not CALLS) 4. EMITS_EVENT (vue-emit: name) — emit() calls; self-loop on component File, joinable with BINDS_EVENT_HANDLER via Cypher for impact tracing 5. ACCESSES (vue-template-attribute) — :prop="var" bindings - call-processor.ts: revert dedicated Vue post-loop pass; moved to scope resolver - Tests and parity expected-failures updated accordingly Co-authored-by: Cursor <cursoragent@cursor.com> * fix(vue): close review gaps in scope/parity extraction Resolve the new PR #1950 review findings by widening Vue scope context to include TS/JS import closures, fixing BINDS_EVENT_HANDLER endpoint assertions, hardening emit/event extraction to avoid comment/property false positives, supporting kebab-case component tags, and ensuring parity runs include vue-scope suites. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(vue): address second review round — regex safety, emit coverage, arch Closes items raised in the Jun 2 review comment on PR #1950. Correctness fixes: - ReDoS mitigation: bound attribute-capture spans to [^>]{0,512}? in all three template tag regexes to prevent pathological backtracking. - Kebab-case misclassified as native: added (?![A-Za-z0-9-]) negative lookahead to NATIVE_TAG_RE so <post-list> is no longer split as native tag `post` with attrs `-list ...`. - Hyphenated event names dropped: widened TAG_EVENT_RE from [\w:.]+ to [\w:.-]+ so @user-loaded and @update:model-value are captured. - this.$emit silently dropped: collectBareEmitEventNames now allows this.$emit(...) by looking back past the '.' to verify preceding token is exactly `this`; socket.emit etc. remain blocked. - Event names with colon rejected: extended validator to accept update:modelValue and update:model-value patterns. Architecture fix: - Moved collectVueScopeFilePaths out of shared phase.ts into a new collectScopeContextPaths optional hook on ScopeResolver, keeping shared pipeline code language-agnostic. vueScopeResolver implements the hook. - Fixed memory leak: preExtractedByPath cleanup now iterates filePaths (all context files) not just primaryFilePaths (only .vue files). Cleanup: - Removed unused extractTemplateEventHandlers and duplicate EVENT_HANDLER_RE. - Fixed skipped comment numbers in emitPostResolutionEdges (1,2,4,5,6 -> 1-6). - Updated vue/index.ts: four categories -> five (added EMITS_EVENT). - Fixed gitnexus-shared EMITS_EVENT JSDoc to reflect File->File reality. Tests: 7 new unit tests covering hyphenated events, this.$emit, kebab-case native-tag exclusion, and update:modelValue event name validation. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(vue): eliminate double file-read and per-file template re-scans Two performance fixes from the self-review pass: 1. **No more double read of .vue files in phase.ts**: primary files were previously read once for `collectScopeContextPaths` (via `entryFileContents`) and again in the blanket `readFileContents(filePaths)` call. Now the primary-file map is passed directly and only the extra context files (TS/JS import closure) require a second I/O round-trip. 2. **Single template parse per .vue file in emitPostResolutionEdges**: previously each of the five extractor functions (components, native handlers, component event bindings, emit calls, attribute bindings) ran `TEMPLATE_RE.exec(content)` independently — five full-file scans per `.vue` file. Replaced with a new `extractVueTemplateEdgeData` batching helper that parses the template and script blocks once and feeds all five extractors from the pre-extracted content. emitPostResolutionEdges now calls a single function and destructures the results. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(parity): exclude TypeScript HOC/HOF/JSX scope-resolver tests from legacy DAG parity gate Three test files introduced in prior PRs exercise scope-resolver-only correctness wins: HOC-wrapped const declarations, HOF-callback caller attribution, and JSX-as-call CALLS edges. The parity runner's ${slug}-*.test.ts glob now picks them up, causing typescript [legacy] failures in CI. Fix: convert each file to use createResolverParityIt('typescript') and register all 26 legacy-failing test names in LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.typescript with explanatory comments. Legacy mode: 11+11+4 tests skipped, zero failures. Registry-primary mode: all 37 tests pass as before. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(test): remove registry-primary-flag unit tests after migration complete All languages are now in MIGRATED_LANGUAGES; the per-language flip tests are no longer needed. Addresses PR #1950 review feedback. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
This was referenced Jun 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
updated readme