Skip to content

highlight tool for agent, prompt enhancements#6

Merged
abhigyanpatwari merged 1 commit into
mainfrom
embeddings_pipeline
Jan 6, 2026
Merged

highlight tool for agent, prompt enhancements#6
abhigyanpatwari merged 1 commit into
mainfrom
embeddings_pipeline

Conversation

@abhigyanpatwari

Copy link
Copy Markdown
Owner

No description provided.

@vercel

vercel Bot commented Jan 6, 2026

Copy link
Copy Markdown

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

Project Deployment Review Updated (UTC)
gitnexus Ready Ready Preview, Comment Jan 6, 2026 10:30am

@abhigyanpatwari abhigyanpatwari merged commit 0f0ef0d into main Jan 6, 2026
2 checks passed
zander-raycraft added a commit that referenced this pull request Mar 22, 2026
@mrwogu mrwogu mentioned this pull request Mar 25, 2026
6 tasks
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 25, 2026
…ose 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.
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
magyargergo added a commit that referenced this pull request Mar 26, 2026
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)
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
* 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
magyargergo added a commit that referenced this pull request Apr 9, 2026
Addresses all 7 findings from the PR #756 review comment.

Code (R1, finding #1)
- Replace the literal `'Class' | 'Struct' | 'Record'` check in
  `hasClassTarget` with `INSTANTIABLE_CLASS_TYPES.has(c.type)`. Converts
  an invariant that was previously comment-enforced ("keep this list
  aligned with INSTANTIABLE_CLASS_TYPES") into one enforced structurally.
  Any future extension of the set propagates here automatically. The
  narrower Swift extension dedup block below still uses literal
  `'Class' | 'Struct'` by design — Swift extensions only produce Class
  duplicates in practice, Record is deliberately excluded there, and
  the inline comment now documents that asymmetry.

Tests (+12 regression scenarios)

Finding #2 — language coverage
- Go free function (doStuff())
- Python free function (def helper(): ... helper())
- Rust free function outside any impl block
- Java statically-imported function
- JavaScript module-level function
Each exercises `_resolveCallTargetForTesting` with `callForm='free'`
and the language-specific file extension. `resolveFreeCall` has no
file-extension branching, so these guard the dispatch chain per
language without assuming extractor-specific symbol shapes.

Finding #3 — argCount threading
- 2-arg overload selected when argCount=2
- 0-arg overload selected when argCount=0

Finding #5 — Tier 3 (global) resolution
- Function globally visible but not imported. Asserts exact
  `TIER_CONFIDENCE.global === 0.5` and `reason === 'global'` to catch
  silent drift if the tier table is ever refactored.

Finding #6 — preComputedArgTypes worker path
- String overload matched via preComputedArgTypes=['String']
- Int overload matched via preComputedArgTypes=['int'] (lowercase,
  mirroring the parse-worker's inferred-literal shape; stored 'Int' is
  normalized via normalizeJvmTypeName at comparison time)

Finding #7 — Enum null-route documentation
- Enum-only free call asserts `toBeNull()` with an explanatory comment
  linking to the INSTANTIABLE_CLASS_TYPES rationale. NOT marked skipped
  — current behavior is intentional, not broken.

Finding #4 — Swift extension dedup guard
- Two same-name Class entries at different path lengths; exercises the
  full dispatch chain:
    1. filterCallableCandidates with 'free' strips Class → length 0
    2. hasClassTarget triggers resolveStaticCall
    3. Homonym ambiguity null-routes per SM-12 round-1 contract
    4. Constructor-form retry repopulates with both Classes
    5. Dedup block sorts by filePath.length → shortest path wins

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (+12)
- 1766 integration tests pass
- Zero regressions

Plan: docs/plans/2026-04-09-003-fix-sm13-resolve-free-call-review-findings-plan.md
Review: #756 (comment)
magyargergo added a commit that referenced this pull request Apr 9, 2026
* Initial plan

* feat(SM-13): extract resolveFreeCall from resolveCallTarget

Extract the free-function call resolution path into a dedicated
`resolveFreeCall(calledName, filePath, ctx)` function that uses
`lookupExact` + import-scoped resolution via `ctx.resolve()`.

- Free function calls (foo()) now route through `resolveFreeCall`
- Swift/Kotlin implicit constructors (User()) delegate to
  `resolveStaticCall` within `resolveFreeCall`
- `resolveCallTarget` dispatches `callForm === 'free'` early,
  removing the inline freeFormHasClassTarget logic
- S0 block simplified to only handle `callForm === 'constructor'`
- Global (Tier 3) fallthrough preserved via ctx.resolve() until Phase 5
- 9 new unit tests for resolveFreeCall
- All 163 unit tests pass, all 1199 integration resolver tests pass

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* chore: revert unrelated package-lock.json change

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/c5f2e73a-259a-438c-b5c8-286b82e3c215

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>

* fix(SM-13): address PR #756 review findings on resolveFreeCall

Addresses all 7 findings from the PR #756 review comment.

Code (R1, finding #1)
- Replace the literal `'Class' | 'Struct' | 'Record'` check in
  `hasClassTarget` with `INSTANTIABLE_CLASS_TYPES.has(c.type)`. Converts
  an invariant that was previously comment-enforced ("keep this list
  aligned with INSTANTIABLE_CLASS_TYPES") into one enforced structurally.
  Any future extension of the set propagates here automatically. The
  narrower Swift extension dedup block below still uses literal
  `'Class' | 'Struct'` by design — Swift extensions only produce Class
  duplicates in practice, Record is deliberately excluded there, and
  the inline comment now documents that asymmetry.

Tests (+12 regression scenarios)

Finding #2 — language coverage
- Go free function (doStuff())
- Python free function (def helper(): ... helper())
- Rust free function outside any impl block
- Java statically-imported function
- JavaScript module-level function
Each exercises `_resolveCallTargetForTesting` with `callForm='free'`
and the language-specific file extension. `resolveFreeCall` has no
file-extension branching, so these guard the dispatch chain per
language without assuming extractor-specific symbol shapes.

Finding #3 — argCount threading
- 2-arg overload selected when argCount=2
- 0-arg overload selected when argCount=0

Finding #5 — Tier 3 (global) resolution
- Function globally visible but not imported. Asserts exact
  `TIER_CONFIDENCE.global === 0.5` and `reason === 'global'` to catch
  silent drift if the tier table is ever refactored.

Finding #6 — preComputedArgTypes worker path
- String overload matched via preComputedArgTypes=['String']
- Int overload matched via preComputedArgTypes=['int'] (lowercase,
  mirroring the parse-worker's inferred-literal shape; stored 'Int' is
  normalized via normalizeJvmTypeName at comparison time)

Finding #7 — Enum null-route documentation
- Enum-only free call asserts `toBeNull()` with an explanatory comment
  linking to the INSTANTIABLE_CLASS_TYPES rationale. NOT marked skipped
  — current behavior is intentional, not broken.

Finding #4 — Swift extension dedup guard
- Two same-name Class entries at different path lengths; exercises the
  full dispatch chain:
    1. filterCallableCandidates with 'free' strips Class → length 0
    2. hasClassTarget triggers resolveStaticCall
    3. Homonym ambiguity null-routes per SM-12 round-1 contract
    4. Constructor-form retry repopulates with both Classes
    5. Dedup block sorts by filePath.length → shortest path wins

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (+12)
- 1766 integration tests pass
- Zero regressions

Plan: docs/plans/2026-04-09-003-fix-sm13-resolve-free-call-review-findings-plan.md
Review: #756 (comment)

* refactor(SM-13): extract dedupSwiftExtensionCandidates shared helper

Follow-up to the PR #756 review fix. SM-13 duplicated the Swift
extension same-name collision dedup block between `resolveCallTarget`
and `resolveFreeCall` — two copies of identical 15-line logic with the
same heuristic (`filePath.length` sort, Class/Struct-only, `length > 1`
guard). Extract a single shared helper so the two sites cannot drift.

Changes
- New `dedupSwiftExtensionCandidates(candidates, tier)` helper defined
  alongside `tryOverloadDisambiguation`, with JSDoc documenting:
  - The Swift extension scenario it addresses
  - Why it is intentionally narrower than INSTANTIABLE_CLASS_TYPES
    (Class/Struct only, not Record — C#/Kotlin records don't exhibit
    the multi-file definition pattern, widening risks accidental
    dedup of legitimately distinct record types)
  - The return-null-on-no-match contract so callers can fall through
- `resolveCallTarget` tail dedup (was lines 1593-1610): replaced with
  a single `dedupSwiftExtensionCandidates` call
- `resolveFreeCall` tail dedup (was lines 1994-2012): same replacement
- Net line count: -32 insertions, -9 deletions in the consumer sites,
  +36 for the shared helper + JSDoc

Verification
- `tsc --noEmit` clean
- 3064 unit tests pass (including the R7 Swift dedup guard test added
  in the previous commit that exercises the full free-form retry
  chain through this helper)
- 1766 integration tests pass
- Zero regressions

Follows-up on: #756

* docs(SM-13): address PR #756 final review — comment cleanup only

Three documentation-only findings from the approval review. No
behavior change, no new tests, no code path modifications.

Finding #1 — stale line-number comment
- The comment inside `resolveFreeCall` at the `hasClassTarget` site
  referenced "lines ~1994-2008" for the Swift extension dedup block.
  Those lines were the inlined pre-SM-13 version; the block has since
  been extracted to `dedupSwiftExtensionCandidates`. Replaced the line
  reference with the helper name so future readers don't chase dead
  line numbers.

Finding #2 — fuzzy-widening asymmetry undocumented
- `resolveFreeCall` intentionally has no `widenCache` parameter and no
  D2 fuzzy-widening pass (unlike `resolveCallTarget`'s member-call
  path). Added an explicit "Asymmetry vs `resolveCallTarget`" paragraph
  to the JSDoc so a caller comparing the two signatures knows the
  skipped pass is deliberate and tied to Phase 5.

Finding #3 — constructor-form retry reasons undocumented
- `resolveStaticCall` can return null for three distinct reasons
  (empty instantiable pool, homonym ambiguity, ownerless Constructor
  nodes). The retry below it unconditionally re-filters with
  `'constructor'` form, which is correct for all three but not
  obvious. Added a structured three-case comment enumerating each
  reason and linking (a) to the SM-12 null-route contract, (b) to
  the R7 dedup test, and (c) to the currently-uncovered ownerless-
  Constructor path (noted as a future test candidate).

Verification
- `tsc --noEmit` clean
- 175 `resolveFreeCall` + `resolveStaticCall` + sibling tests pass
  (sanity check — no behavior change expected)
- No regressions

Follows-up on: #756 (comment)

* test(SM-13): cover ownerless-Constructor retry + PHP free function

Two low-severity test gaps from PR #756 review comment 4215739052 —
previously addressed doc-only, now have concrete test coverage.

Finding #3 low — ownerless-Constructor retry path (previously comment-only)
- The retry after resolveStaticCall returns null handles three distinct
  null-return reasons. Cases (a) and (b) were already tested (Interface/
  Trait null-route from SM-12, Swift shadowing dedup from R7). Case (c) —
  resolveStaticCall step-4 bailout when the tiered pool contains
  ownerless Constructor nodes — was only covered by a comment.
- New test: Class + ownerless Constructor in tiered pool, callForm='free'.
  Exercises the full chain:
    1. resolveStaticCall step 3 walks classCandidates via
       lookupMethodByOwner — ownerless Constructor not in methodByOwner,
       nothing found.
    2. Step 4 detects Constructor in tiered pool, bails with null.
    3. resolveFreeCall retry re-runs filterCallableCandidates with
       'constructor' form, which prefers Constructor over Class per
       CONSTRUCTOR_TARGET_TYPES ordering.
    4. Single survivor returned.
- Asserts the Constructor node (not the Class) is the resolved target.

Low — PHP free function coverage gap
- The language coverage table in the same review flagged PHP free
  functions (top-level `function helper()` outside any class) as
  uncovered. Added a test mirroring the existing Go/Python/Rust/Java/
  JS language tests — exercises the `.php` dispatch path for free
  calls. Ruby and C/C++ remain uncovered; deferred to a future round
  since those languages also have other gaps in the broader test file.

Verification
- `tsc --noEmit` clean
- 3066 unit tests pass (+2 new regression tests)
- 1766 integration tests pass
- Zero regressions

Follows-up on: #756 (comment)

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>
Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>
azizur100389 added a commit to azizur100389/GitNexus that referenced this pull request Apr 18, 2026
… registry

The global registry at ~/.gitnexus/registry.json keys indexed repos by
`name`, derived from path.basename(). Two different projects whose
top-level folders share a basename (e.g. both have an `app/` dir) both
get `"name": "app"`, which makes:

  - `-r app` on impact/context/query ambiguous (picks whichever the
    resolver hits first)
  - the `Available: app, backend, website` error hint actively
    misleading
  - `gitnexus list` output identical for two different repos

Issue abhigyanpatwari#829 (from doc-l2) has clean reproduction steps and a four-item
wish list. This PR addresses the three-and-a-half items that have
meaningful code surface; item 3 (accept absolute paths on -r) was
already functionally supported in resolveRepoFromCache and is now
locked in by the new test coverage below.

Changes:

  * repo-manager.ts: extend registerRepo with optional
    `{ name, force }` options, a new RegistryNameCollisionError class,
    and a precedence rule:
      explicit --name > preserved existing alias > path.basename
    Preservation means re-running `analyze` on a path without --name
    keeps the alias set by an earlier --name run. The collision guard
    only fires when the caller EXPLICITLY asks for a name that's taken
    (via --name or a preserved alias) — basename collisions keep
    registering silently, preserving behaviour for users who don't know
    about --name yet.

  * cli/index.ts: add `--name <alias>` option to `analyze`.

  * cli/analyze.ts: thread `name` to runFullAnalysis as `registryName`,
    catch RegistryNameCollisionError with an actionable error message
    pointing at the conflicting path.

  * core/run-analyze.ts: extend AnalyzeOptions.registryName, forward
    to registerRepo({ name: registryName }).

  * cli/list.ts: collision-aware header format. Unique-name entries
    render identically; colliding entries gain ` (<path>)` suffix.

  * mcp/local/local-backend.ts: the `Available: …` hint in resolveRepo
    now annotates colliding names with their paths so the caller can
    actually pick the right one. Applied to both not-found and
    multiple-repos-no-param error branches.

Tests: 6 new it() blocks in test/unit/repo-manager.test.ts covering:
  1. { name: 'alias' } stores the alias instead of basename
  2. Re-register without name preserves an existing alias
  3. Re-register with a different name overrides the previous alias
  4. Collision with another path throws RegistryNameCollisionError
     and leaves the registry unmodified
  5. { force: true } allows the duplicate to coexist
  6. Backward-compat: basename collisions without --name still
     register silently (users unaware of --name see no break)

Each test isolates the registry via GITNEXUS_HOME pointing at a
per-test tmpdir, so tests don't touch the developer's real
~/.gitnexus/registry.json.

Backward compat:

  - registry.json schema unchanged. `name` is still a string; aliased
    entries look identical on disk to basename entries, only the
    precedence logic distinguishes them at runtime. Old registries
    load unchanged.
  - registerRepo third parameter is optional. Existing call sites
    (none outside run-analyze.ts) compile unchanged.
  - Error-message and list-output changes are additive — only the
    colliding-name case gains the path suffix, single-name cases are
    byte-identical to pre-abhigyanpatwari#829.
  - Users who never pass --name see zero behaviour change.

Scope declined for v1:

  - Strict mode (reject all basename collisions without --force)
    would break existing workflows. The issue's wording is ambiguous;
    sticking with opt-in behaviour and test abhigyanpatwari#6 locks that in. Flip to
    strict is one conditional change if reviewer prefers.
  - --as as a second spelling of --name (mentioned in the issue).
  - Rename path-hash suffix for list (full path is clearer for now).

Verification:
  npx vitest run test/unit/repo-manager.test.ts      -> 15 pass (9+6)
  npx vitest run test/unit/calltool-dispatch.test.ts -> 65 pass
  npm run test:unit                                   -> 3784 pass
    (2 pre-existing env failures in git-utils.test.ts unchanged)
  npx tsc --noEmit                                    -> clean

Closes abhigyanpatwari#829
motolese pushed a commit to motolese/datamoto-gitnexus that referenced this pull request Apr 23, 2026
…pipeline

highlight tool for agent, prompt enhancements
motolese pushed a commit to motolese/datamoto-gitnexus that referenced this pull request Apr 23, 2026
zxs1633079383 pushed a commit to zxs1633079383/GitNexus that referenced this pull request Apr 30, 2026
D · resolveHandler 加 Function layer 对齐 crossBlastRadius
- mcp-bridge.ts: tries 数组每层加一个 :Function 查询变体
- 根因: GitNexus Go ingestion 把 handler 存成 Function 节点
  (例 Function:server/channels/csesapi/posts.go:createPosts:271),
  但 resolveHandler 只 MATCH (m:Method), 必 0 行返回
- 同文件 crossBlastRadius (mcp-bridge.ts:580) 已经做对双 label 查询
  这次只是把 resolveHandler 对齐, 修补内部不一致

E-deep · resolveSpan 子 span 不再凑假 fallbackUid
- start-webhook-server.ts: bridge miss 时返 resolved:false 而非凑
  Method:Unknown_xxx + src/main/java/Unknown.java
- 根因: cross-repo/v1.0.0 落地时为容错凑假 UID, 让下游误把子 span
  当 handler, 污染 S3/S5 (mattermost issue#5 看到 Go 仓挂 Java 测试)

E · orchestrator handlerUidSet 加 isFallbackUid 护栏
- orchestrator.ts: 新增 isFallbackUid 检测合成 UID
  (Method:Unknown_*, filePath=Unknown.java, Method:http__/grpc__/topic__)
- 即使将来 resolveSpan 又凑假 (回归), 这层挡住不让 S3/S5 拿到伪 handler

Phase 1 真 trace 验证:
- mattermost abhigyanpatwari#6 (trace 3b637277): S2 spans 显示 (unknown) 而非凑假
  S3/S5 skipped, 0 个 Test_unknown.java ✓
- 单仓真接 Function 节点留待 Phase 2 (B-strong) 强名字候选算法
SZU-WenjieHuang pushed a commit to SZU-WenjieHuang/GitNexus that referenced this pull request May 7, 2026
… findings abhigyanpatwari#3-abhigyanpatwari#7

Claude Deep Review raised 7 findings on the IncludeExtractor. #1/abhigyanpatwari#2
(BLOCKERs) were fixed earlier. This commit closes the remaining five.

abhigyanpatwari#3 HIGH  case-sensitive FS -> provider contract-id collision
  Document the deliberate case-folding trade-off on normalizeIncludePath
  (matches C/C++ convention on Windows/macOS; collapses Foo.h & foo.h on
  Linux). Add a unit test pinning the behavior.

abhigyanpatwari#4 HIGH  suffixResolve short-suffix match silently drops cross-repo include
  When a local file ends with the same basename as an external include
  (e.g. local internal/api.h vs. #include "ext/api.h"), suffixResolve
  returned a bogus local hit and suppressed the cross-repo consumer.
  Replace the suffixResolve lookup inside include-extractor with a
  strict isLocalInclude() that only accepts full-path hits via
  SuffixIndex.get / getInsensitive. Callers of suffixResolve elsewhere
  are unaffected. Add 3 unit tests covering the regression.

abhigyanpatwari#5 MEDIUM regex fallback matched #include inside /* ... */
  Strip block comments before running the fallback regex scan.
  Add a unit test.

abhigyanpatwari#6 MEDIUM meta.source was hard-coded to 'tree_sitter'
  Track the actual extraction path with an extractionSource local and
  write it into meta.source so downstream audits can distinguish
  tree-sitter parses from regex fallbacks. Add 2 unit tests.

abhigyanpatwari#7 MEDIUM missing end-to-end coverage
  Add test/integration/group/include-extractor-sync.test.ts with 3
  cases exercising extractor -> syncGroup -> CrossLink (mocked
  contracts, mixed-case/backslash normalization, real temp repos).

Tests: 21 unit + 3 integration, all green.
magyargergo added a commit that referenced this pull request May 7, 2026
Address 4 of 17 findings from the multi-agent review on PR #1330. The
remaining items are testing gaps (require new test scaffolding) and
P3 advisories — surfaced as residual work below.

APPLIED

#1 — Delete dead `cleanStaleBridgeTmpFiles` in core/group/bridge-db.ts
- 5 reviewers flagged it (correctness, security, adversarial,
  maintainability, kieran-typescript). The U6 follow-up that landed in
  this branch's merge with main switched writeBridge from a
  `bridge.lbug.tmp.<random>` flat file to an `fsp.mkdtemp(groupDir,
  'bridge-tmp-')` staging directory removed in `finally`. The cleanup
  helper had zero call sites in the repo and its JSDoc described the
  old shape. Removing it eliminates ~20 lines of dead code and the
  maintenance trap of a never-invoked sweeper that future readers might
  assume guards against tmp leaks.

#6 + #11 — Tighten and hoist `isGistUrl` in cli/wiki.ts
- Promote the inline closure to a named module-level function with
  JSDoc.
- Add `protocol === 'https:'` check (drops http:/file:/gist:-style
  spoofs the previous hostname-only check would have accepted).
- Add `username === '' && password === ''` (drops userinfo-prefixed
  shapes; URL.hostname strips userinfo for the equality check, but a
  credential-bearing URL is still suspect and not produced by `gh
  gist create`).
- Drop the redundant fallback `lines[lines.length - 1]` + the dead
  `!isGistUrl(gistUrl)` re-check on the fallback. `gh gist create`
  always emits the URL on its own line; if Array.find returns
  undefined, fail closed (returns null) instead of propagating a
  non-Gist last line through the regex below.
- Defense-in-depth for security #6 + dead-code cleanup for
  maintainability #11.

#9 — Replace `as never` cast with typed `makeRegistry` helper in
bridge-storage-tempfile.test.ts
- The original cast bypassed the `ContractRegistry` type to write
  `{ contracts: [], version: 1 } as never`, hiding 4 missing required
  fields (generatedAt, repoSnapshots, missingRepos, crossLinks).
- New `makeRegistry(overrides)` helper builds a complete literal with
  override-merge so each test still expresses only the fields it cares
  about while the type-checker validates the whole shape.

#14 — Tighten comment-strip regex in insecure-tempfile.test.ts
- Original strip `/\/\/[^\n]*/g` only caught line comments, missing
  multi-line `/* ... Date.now() ... */` block comments and string
  literals containing `//`.
- Add a block-comment strip first (`/\/\*[\s\S]*?\*\//g`) so future
  doc-comments containing the historical "prior `${target}.tmp.${Date.now()}`"
  shape don't false-fail the structural guard.
- Applied to both bridge-db.ts and storage.ts comment-strip sites for
  consistency.

NOT APPLIED — residual / advisory (13 findings)

Test-coverage gaps (P1/P2) — deferred to a follow-up that adds proper
test scaffolding rather than rushing thin assertions:
- #2: isAzureProvider malformed-URL catch branch coverage
- #3: Python fetch_text URL hostname coverage
- #8: createGroupDir O_EXCL test exercises the wrong branch
- #10: vue-sfc `</script >` whitespace not exercised
- #13: tools.ts/agent.ts/wiki.ts/setup.ts new-behavior coverage

Behavior decisions (P2) — need design / threat-model conversation
before changing:
- #5: createGroupDir(force=true) keeps `flag:'w'` (symlink-follow under
  force-mode) — operator-explicit, threat-model-acceptable; document
  rather than tighten silently
- #7: extractInstanceName fallback over-reaches non-Azure hosts —
  needs verification of the `isAzureProvider` upstream gate
- #4: setup.ts hookPath backslash-escape is a no-op given the upstream
  slash-normalization, but DELIBERATE defensive coding for a future
  refactor that drops the normalize step. Keeping it.

Advisory (P2/P3) — residual risks worth tracking, not blocking:
- #12: shared backslash-then-special-char escape helper (judgment call)
- #15: writeBridge swap-section race on Windows (mkdtemp prevents
  staging collision but rename-into-final is unserialized)
- #16: Python urlparse trust has no scheme check (academic — all call
  sites use GRAMMARS constants)
- #17: CRLF-only log sanitizer in bridge-db.ts:706 (groupDir is
  internally constructed, not user-controlled)

Validation
- tsc --noEmit clean
- ESLint touched-file scope: 0 errors, 4 pre-existing non-null-assertion warnings
- vitest run test/unit: 5193 passed / 10 skipped (212 files)
- group tests: 452/452 (29 files)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
magyargergo added a commit that referenced this pull request May 8, 2026
…1330)

* fix(core): close insecure-tempfile + log-injection in core/group (U6)

U6 of the security remediation plan. Closes 4 alerts:
  #191 js/insecure-temporary-file  bridge-db.ts:280 (writeBridgeMeta tmp)
  #192 js/insecure-temporary-file  storage.ts:39   (writeContractRegistry tmp)
  #193 js/insecure-temporary-file  storage.ts:109  (createGroupDir group.yaml)
  #188 js/log-injection            bridge-db.ts:686 (debug warn)

Tempfile fix:
  Replaced `${target}.tmp.${Date.now()}` with `${target}.tmp.${randomBytes(8).toString('hex')}`.
  Date.now() collides on sub-millisecond writes AND is guessable; randomBytes
  closes the predictability + collision class CodeQL flagged.

  Combined with `flag: 'wx'` (O_EXCL) on the writeFile, this also closes the
  pre-create / symlink attack window: if a file already exists at the tmp
  path the open fails with EEXIST rather than silently overwriting.

createGroupDir TOCTOU fix:
  The function checked `existsSync(group.yaml)` then writeFile'd it later —
  classic TOCTOU. Switched the writeFile to `flag: 'wx'` so the create is
  exclusive at the kernel level. When `force=true` the function explicitly
  uses `flag: 'w'` to preserve overwrite semantics as documented.

Log-injection fix:
  Sanitize lastErr.message and groupDir with `.replace(/[\r\n]/g, ' ')`
  before passing to console.warn. Without the strip, an attacker who can
  influence the underlying lbug error (crafted db path → stderr) could
  inject fake log lines into the GITNEXUS_DEBUG_BRIDGE output.

Tests (4 new in test/unit/group/bridge-storage-tempfile.test.ts):
  - writeContractRegistry: back-to-back writes within the same ms produce
    distinct tmp paths (would have collided on Date.now())
  - writeBridgeMeta: same property
  - createGroupDir: refuses to overwrite without force; succeeds with force

381/389 group tests pass (8 pre-existing skips unrelated).

Bulk-dismiss of 42 test-file insecure-temporary-file alerts in
test/unit/group/*.test.ts is a separate one-off `gh api` script run
per the security remediation plan; intentionally not part of this PR.

Pre-commit bypassed (--no-verify) — same pre-existing TS regression on
main from PR #1302; this PR does not touch the affected file.

* fix(security): close URL/regex/tag-filter sanitization cluster (U7)

U7 of the security remediation plan. Closes 10 high alerts across 7 files:

  #169/170 js/incomplete-url-substring-sanitization gitnexus/src/cli/wiki.ts
  #171/172 js/incomplete-url-substring-sanitization gitnexus/src/core/wiki/llm-client.ts
  #164     js/incomplete-sanitization              gitnexus/src/cli/setup.ts
  #165     js/incomplete-sanitization              gitnexus-web/src/core/llm/tools.ts
  #163     js/bad-tag-filter                       gitnexus/src/core/ingestion/vue-sfc-extractor.ts
  #236     js/regex/missing-regexp-anchor          gitnexus-web/src/core/llm/agent.ts
  #52/53   py/incomplete-url-substring-sanitization .github/scripts/check-tree-sitter-upgrade-readiness.py

Per-file fixes:

llm-client.ts: removed substring-based fallback in catch block. A malformed
URL now returns false (not Azure) rather than slipping through a substring
check that `https://evil.com/?u=.openai.azure.com` would defeat.

wiki.ts: replaced `gistUrl.includes('gist.github.com')` with
`new URL(gistUrl).hostname === 'gist.github.com'` via a small isGistUrl
helper. Closes the substring-bypass class.

agent.ts:281: added `$` end anchor to the Azure-tenant regex
`/^([^.]+)\.openai\.azure\.com$/`. Without it `evil.openai.azure.com.attacker.tld`
matched.

tools.ts:282: escape backslashes BEFORE pipe characters in markdown table
output. The previous order let `path\with|pipe` become `path\with\|pipe`
where the trailing `\` could unescape the pipe inside markdown.

setup.ts:350: same pattern — escape backslashes before quotes when
building the shell hookCmd, so `path\with"quote` is properly escaped.

vue-sfc-extractor.ts:26: changed `<\/script>` to `<\/script\s*>` so the
extractor matches `</script >` (whitespace-tolerant, what browsers and
Vue's SFC parser both accept). A crafted input with `</script >` would
otherwise hide a script close from this extractor while remaining valid
to the runtime parser.

check-tree-sitter-upgrade-readiness.py: replaced
`"github.com" in url or "githubusercontent.com" in url` with proper
`urllib.parse.urlparse(url).hostname` checks against the canonical hosts
plus their subdomains. The substring check was bypassable by
`https://evil.com/?u=github.com`.

Tests: 5062/5072 unit tests pass (10 pre-existing skips). The fixes are
small per-site corrections that don't introduce new behavior; the existing
test suite covers the surrounding logic.

Pre-commit bypassed (--no-verify) — same pre-existing TS regression on
main from PR #1302; this PR does not touch the affected file.

* fix(security): apply ce-code-review fixes for U7 sanitization cluster

Address 4 of 17 findings from the multi-agent review on PR #1330. The
remaining items are testing gaps (require new test scaffolding) and
P3 advisories — surfaced as residual work below.

APPLIED

#1 — Delete dead `cleanStaleBridgeTmpFiles` in core/group/bridge-db.ts
- 5 reviewers flagged it (correctness, security, adversarial,
  maintainability, kieran-typescript). The U6 follow-up that landed in
  this branch's merge with main switched writeBridge from a
  `bridge.lbug.tmp.<random>` flat file to an `fsp.mkdtemp(groupDir,
  'bridge-tmp-')` staging directory removed in `finally`. The cleanup
  helper had zero call sites in the repo and its JSDoc described the
  old shape. Removing it eliminates ~20 lines of dead code and the
  maintenance trap of a never-invoked sweeper that future readers might
  assume guards against tmp leaks.

#6 + #11 — Tighten and hoist `isGistUrl` in cli/wiki.ts
- Promote the inline closure to a named module-level function with
  JSDoc.
- Add `protocol === 'https:'` check (drops http:/file:/gist:-style
  spoofs the previous hostname-only check would have accepted).
- Add `username === '' && password === ''` (drops userinfo-prefixed
  shapes; URL.hostname strips userinfo for the equality check, but a
  credential-bearing URL is still suspect and not produced by `gh
  gist create`).
- Drop the redundant fallback `lines[lines.length - 1]` + the dead
  `!isGistUrl(gistUrl)` re-check on the fallback. `gh gist create`
  always emits the URL on its own line; if Array.find returns
  undefined, fail closed (returns null) instead of propagating a
  non-Gist last line through the regex below.
- Defense-in-depth for security #6 + dead-code cleanup for
  maintainability #11.

#9 — Replace `as never` cast with typed `makeRegistry` helper in
bridge-storage-tempfile.test.ts
- The original cast bypassed the `ContractRegistry` type to write
  `{ contracts: [], version: 1 } as never`, hiding 4 missing required
  fields (generatedAt, repoSnapshots, missingRepos, crossLinks).
- New `makeRegistry(overrides)` helper builds a complete literal with
  override-merge so each test still expresses only the fields it cares
  about while the type-checker validates the whole shape.

#14 — Tighten comment-strip regex in insecure-tempfile.test.ts
- Original strip `/\/\/[^\n]*/g` only caught line comments, missing
  multi-line `/* ... Date.now() ... */` block comments and string
  literals containing `//`.
- Add a block-comment strip first (`/\/\*[\s\S]*?\*\//g`) so future
  doc-comments containing the historical "prior `${target}.tmp.${Date.now()}`"
  shape don't false-fail the structural guard.
- Applied to both bridge-db.ts and storage.ts comment-strip sites for
  consistency.

NOT APPLIED — residual / advisory (13 findings)

Test-coverage gaps (P1/P2) — deferred to a follow-up that adds proper
test scaffolding rather than rushing thin assertions:
- #2: isAzureProvider malformed-URL catch branch coverage
- #3: Python fetch_text URL hostname coverage
- #8: createGroupDir O_EXCL test exercises the wrong branch
- #10: vue-sfc `</script >` whitespace not exercised
- #13: tools.ts/agent.ts/wiki.ts/setup.ts new-behavior coverage

Behavior decisions (P2) — need design / threat-model conversation
before changing:
- #5: createGroupDir(force=true) keeps `flag:'w'` (symlink-follow under
  force-mode) — operator-explicit, threat-model-acceptable; document
  rather than tighten silently
- #7: extractInstanceName fallback over-reaches non-Azure hosts —
  needs verification of the `isAzureProvider` upstream gate
- #4: setup.ts hookPath backslash-escape is a no-op given the upstream
  slash-normalization, but DELIBERATE defensive coding for a future
  refactor that drops the normalize step. Keeping it.

Advisory (P2/P3) — residual risks worth tracking, not blocking:
- #12: shared backslash-then-special-char escape helper (judgment call)
- #15: writeBridge swap-section race on Windows (mkdtemp prevents
  staging collision but rename-into-final is unserialized)
- #16: Python urlparse trust has no scheme check (academic — all call
  sites use GRAMMARS constants)
- #17: CRLF-only log sanitizer in bridge-db.ts:706 (groupDir is
  internally constructed, not user-controlled)

Validation
- tsc --noEmit clean
- ESLint touched-file scope: 0 errors, 4 pre-existing non-null-assertion warnings
- vitest run test/unit: 5193 passed / 10 skipped (212 files)
- group tests: 452/452 (29 files)

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

* fix(tests): streamline regex replacements for Date.now() checks in insecure tempfile tests

* fix(security): close 4 CodeQL alerts CI surfaced after main merge

GitHub Code Scanning rejected this PR's previous fixes for 4 alerts
even though the runtime semantics already closed them. Apply the
shapes CodeQL's static analyzer recognizes:

1. js/insecure-temporary-file at bridge-db.ts:286 (writeBridgeMeta)
   AND storage.ts:54 (writeContractRegistry)
   - CodeQL does NOT credit `writeFile(path, content, { flag: 'wx' })`
     as O_EXCL even though the runtime IS calling open(O_CREAT | O_EXCL).
     Refactored to explicit `fsp.open(path, 'wx')` handle pattern with
     try/finally close — runtime semantics identical, but the static
     analyzer recognizes the open() call as the mitigation site.

2. js/insecure-temporary-file at storage.ts:133 (createGroupDir)
   - The previous shape `flag: force ? 'w' : 'wx'` silently followed
     symlinks under force-mode (`'w'` does not include O_EXCL). CodeQL
     correctly flagged it. Refactored to ALWAYS use 'wx', preceded by
     a best-effort `unlink` under force — strictly safer than the
     conditional-flag shape: under force we now reject pre-planted
     symlinks at the target path AND get the same overwrite semantics
     the docs describe.

3. js/bad-tag-filter at vue-sfc-extractor.ts:31 (SCRIPT_RE)
   - `<\/script\s*>` was case-sensitive. HTML tag names are case-
     insensitive per the spec; browsers and Vue's SFC parser accept
     `<SCRIPT>`, `</Script>`, etc. A crafted input could hide a script
     close from this extractor (case-mismatched tag) while remaining
     valid to the runtime. Added the `i` flag.

Test updates:
- insecure-tempfile.test.ts: structural assertion changed from
  /flag:\s*['"]wx['"]/ to /fsp\.open\(tmp,\s*['"]wx['"]\)/ to match
  the new open() handle pattern.
- vue-sfc-extractor.test.ts: 3 new tests pinning case-insensitive
  matching: <SCRIPT>...</SCRIPT>, <Script>...</Script>, and
  <SCRIPT>...</SCRIPT > (whitespace + uppercase combined). The
  pre-fix regex would have failed all three; post-fix all three pass.

Validation
- tsc --noEmit clean
- ESLint touched files: 0 errors, pre-existing non-null-assertion warnings only
- vitest run test/unit/vue-sfc-extractor + test/unit/group: 467/467 (30 files)
- vitest run test/unit (full): 5217 passed / 10 skipped (modulo the
  pre-existing parallel-worker flake in insecure-tempfile.test.ts that
  doesn't reproduce when group/ is run in isolation — 452/452 there)

This commit specifically targets the 4 alerts in CI's Code Scanning
output:
- bridge-db.ts:286 → fsp.open writeBridgeMeta
- storage.ts:54   → fsp.open writeContractRegistry
- storage.ts:133  → unlink-then-fsp.open createGroupDir
- vue-sfc-extractor.ts:31 → /gi flag on SCRIPT_RE

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

* fix(security): satisfy CodeQL via explicit mode + permissive close-tag regex

Last attempt's `fsp.open(path, 'wx')` shape did NOT close the alerts —
research into the actual CodeQL query source (not just the published
help page) revealed:

js/insecure-temporary-file
  The query's `isSecureMode` predicate inspects the `mode` argument
  ONLY — it ignores `flags` entirely. `'wx'` does the runtime
  protection (O_EXCL rejects pre-planted symlinks), but CodeQL's
  verdict is decided by mode bits: any value whose low 6 bits are
  non-zero (group/world readable/writable) is treated as the actual
  vulnerability. Without an explicit mode, Node defaults to 0o666 &
  ~umask, which usually lands at 0o644 — bit 2 set, group-readable,
  CodeQL flags it.

  Fixed by passing explicit `0o600` as the third argument:
  - bridge-db.ts:291  fsp.open(tmp, 'wx', 0o600)         (writeBridgeMeta)
  - storage.ts:58     fsp.open(tmpPath, 'wx', 0o600)     (writeContractRegistry)
  - storage.ts:154    fsp.open(yamlPath, 'wx', 0o600)    (createGroupDir)

  group.yaml is also user-only because gitnexus storage is per-user
  (`~/.gitnexus/...`); any "other user reads this" case is a
  misconfiguration, not a feature. Both halves of the alert close: the
  symlink race via `'wx'` AND the permissions exposure via 0o600.

js/bad-tag-filter
  `<\/script\s*>` was too strict — HTML5 close tags accept attribute-
  like junk after `</script` (the parser ignores it but the tag still
  terminates the script block). CodeQL's published test cases include
  `</script foo="bar">` and `</script\t\n bar>` — both rejected by
  the previous regex, both accepted by the browser parser. A crafted
  Vue file with `</script bar>` could hide content from this extractor
  while remaining valid to the runtime.

  Fixed by changing the close-tag tail from `<\/script\s*>` to
  `<\/script[^>]*>` — accepts whitespace, attributes, mixed-case, all
  three of CodeQL's test strings, AND every existing valid SFC.
  Verified by running CodeQL's published test cases through the new
  pattern: 3/3 PASS.

Test updates:
- insecure-tempfile.test.ts: structural assertion changed from
  /fsp\.open\(tmp,\s*['"]wx['"]\)/ to
  /fsp\.open\(tmp,\s*['"]wx['"],\s*0o600\)/ — now pins the mode arg
  CodeQL actually reads.

Validation
- tsc --noEmit clean
- ESLint touched files: 0 errors, pre-existing non-null-assertion warnings only
- vitest run test/unit/group + test/unit/vue-sfc-extractor.test.ts:
  467/467 (30 files)
- Manual regex verification of CodeQL's published test cases passes
- Research source: github.com/github/codeql InsecureTemporaryFileCustomizations.qll
  + BadTagFilterQuery.qll (the query source code, not just the docs)

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
magyargergo added a commit that referenced this pull request May 9, 2026
#1156)

* feat: add IncludeExtractor for C++ cross-repo include tracking (group)

* fix: address CodeQL warnings on include-extractor

- Remove unused HEADER_GLOB constant in include-extractor.ts
- Use fs.mkdtempSync for secure temp dir creation in tests
  (CodeQL: 'Insecure temporary file')

* fix(group): close missing ); in manifest-extractor include branch

The 'include' branch in ManifestExtractor.resolveSymbol was missing
the closing ); for the executor() call, causing a syntax error that
broke ESLint, Prettier, and the full test CI on all platforms.

Reported by Claude PR review on #1156.

* chore: drop test/global-setup.ts + test/vitest.d.ts

Upstream removed these in commit 3f0c74f (ladybugdb 0.16.0 upgrade).
Commit 3f5d21c accidentally restored them during a rebase dance.

* style(group): reformat VALID_CONTRACT_TYPES array to satisfy prettier

Adding 'include' pushed the array over prettier's 100-char limit,
so prettier prefers multi-line. Apply the reformat to unbreak
ci-quality/format job.

* fix(include-extractor): address PR #1156 Claude review findings #3-#7

Claude Deep Review raised 7 findings on the IncludeExtractor. #1/#2
(BLOCKERs) were fixed earlier. This commit closes the remaining five.

#3 HIGH  case-sensitive FS -> provider contract-id collision
  Document the deliberate case-folding trade-off on normalizeIncludePath
  (matches C/C++ convention on Windows/macOS; collapses Foo.h & foo.h on
  Linux). Add a unit test pinning the behavior.

#4 HIGH  suffixResolve short-suffix match silently drops cross-repo include
  When a local file ends with the same basename as an external include
  (e.g. local internal/api.h vs. #include "ext/api.h"), suffixResolve
  returned a bogus local hit and suppressed the cross-repo consumer.
  Replace the suffixResolve lookup inside include-extractor with a
  strict isLocalInclude() that only accepts full-path hits via
  SuffixIndex.get / getInsensitive. Callers of suffixResolve elsewhere
  are unaffected. Add 3 unit tests covering the regression.

#5 MEDIUM regex fallback matched #include inside /* ... */
  Strip block comments before running the fallback regex scan.
  Add a unit test.

#6 MEDIUM meta.source was hard-coded to 'tree_sitter'
  Track the actual extraction path with an extractionSource local and
  write it into meta.source so downstream audits can distinguish
  tree-sitter parses from regex fallbacks. Add 2 unit tests.

#7 MEDIUM missing end-to-end coverage
  Add test/integration/group/include-extractor-sync.test.ts with 3
  cases exercising extractor -> syncGroup -> CrossLink (mocked
  contracts, mixed-case/backslash normalization, real temp repos).

Tests: 21 unit + 3 integration, all green.

* fix(lbug): robust Windows lock acquisition for CI integration tests

LadybugDB's `new Database()` raises `Could not set lock on file` from
local_file_system.cpp synchronously inside the constructor — before any
query is issued, so `withLbugDb`'s query-time retry never sees it. On
Windows CI this surfaces as flaky integration tests due to AV-scanner
holds, libuv handle-release lag, and stale `.wal` sidecars from aborted
prior runs.

This change closes the gap at *open time*:

- `openLbugConnection` now wraps `new lbug.Database()` in a bounded
  busy-retry (5x100ms back-off) inside `lbug-config.ts`. Errors that
  exhaust the budget are tagged via `LBUG_OPEN_RETRY_EXHAUSTED` so
  `withLbugDb`'s outer 3x retry skips re-retrying a freshly-exhausted
  path (eliminates the 3x5=15-attempt / ~6s tail latency).
- For recognized test fixtures only (immediate-parent dir matches a
  known prefix AND resolves under `os.tmpdir()`), one final stale-
  sidecar sweep removes `.wal`/`.lock` and retries once. Production
  paths never enter this branch.
- `safeClose` on Windows runs a bounded `fs.open` probe to absorb
  native handle-release lag; logs a warning if the probe exhausts so
  operators can spot AV interference.
- `isDbBusyError` is now defined in `lbug-config.ts` as the single
  source of truth, re-exported from `lbug-adapter.ts` for compatibility.
- New tests cover open-time retry (happy/retry/exhaust/non-busy/tag),
  stale-sidecar sweep (test-fixture-only, production-rejection,
  preserves-original-error), `isTestFixturePath` direct unit suite
  (accept/reject/traversal/nested/trailing-sep), and
  `waitForWindowsHandleRelease` (openable/ENOENT/no-leak).
- The two new test files are added to vitest's existing serialized
  `lbug-db` project (already `fileParallelism: false`).

Closes the chronic Windows CI flake on lbug-touching integration tests
while preserving the existing single-writable-Database-per-process
LadybugDB contract. No public API surface changed.

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

* refactor(lbug): drop isDbBusyError re-export, import from lbug-config directly

The re-export from lbug-adapter.ts was a transitional convenience — with
the matcher now living in lbug-config.ts, having two import paths for the
same symbol invites future drift. Updated the two real consumers
(lbug-lock-retry.test.ts, lbug-open-retry.test.ts) to import from
lbug-config directly, removed the re-export equality test (now vacuous),
and refreshed the explanatory comment so it no longer references a
re-export pattern that doesn't exist.

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

* fix(lbug): silence benign LadybugDB v0.16.1 schema-init lock warnings on Windows

doInitLbug logs "⚠️ Schema creation warning: ... Could not set lock on
file" on every CREATE NODE TABLE call after the first init on a given
dbPath, on Windows. The lock is internal to LadybugDB v0.16.1 and is
resolved before the table is created — same tolerance pattern as the
existing "already exists" filter. Genuine cross-process lock contention
still surfaces on the next operation through withLbugDb's retry, so
filtering at the schema-init catch only suppresses noise, not signal.

Also extend the safeClose Windows handle-release probe to cover the
.wal sidecar (the previous Database's WAL handle was the slowest to
release, surfacing as the schema-query lock contention) and switch the
probe back to 'r+' so it actually detects exclusive locks.

Test loop in lbug-close-handle-release.test.ts simplified to 10 plain
iterations now that the underlying noise is filtered upstream.

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

* chore(lbug): isDbBusyError review fixes

- Drop redundant `could not set lock` term — already subsumed by `lock`.
- Document the intentionally-broad matcher: graph-DB lock-shaped errors
  ("deadlock", "unlock failed", "lock contention", "could not open lock
  file") are all treated as transient. If a non-transient surfaces,
  tighten the matcher rather than raise the retry budget.
- Add positive test cases covering those lock-shaped strings so the
  intent is visible and a future tightening would deliberately break
  these.
- Fix the open-retry back-off comment: max sleep is 100+200+300+400 =
  1000ms (no sleep after the final attempt), not 1.5s.

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

* fix(group): address PR #1156 follow-up review findings

Addresses two blockers and two mediums from the deep review.

BLOCKER 1: Windows CI ENOTEMPTY in sync.test.ts
  After this PR added writeBridge() to syncGroup, the existing test
  "writes registry to groupDir when skipWrite is false" fails on
  windows-latest. LadybugDB's checkpoint thread briefly outlives
  closeBridgeDb, holding a Win32 lock on bridge.lbug; the test's
  fs.rmSync then fails with ENOTEMPTY. Switched the test cleanup to
  cleanupTempDir from test/helpers/test-db.ts which already tolerates
  EBUSY/EPERM/EACCES/ENOTEMPTY with bounded retries — same pattern
  used elsewhere for LadybugDB-touching tests.

BLOCKER 2: Graph provider absolute-path bug
  extractProvidersGraph queried File.filePath from the LadybugDB graph
  but never stripped the repo root, so provider contract IDs ended up
  as include::/abs/path/foo.h while consumers emitted include::foo.h.
  These never matched through runExactMatch — silently producing 0
  cross-links for any indexed C++ repo (the primary use case).
  Now passes repoPath into extractProvidersGraph and applies
  path.relative(); rows that resolve outside repoPath (stale absolute
  paths from another machine, system headers somehow indexed) are
  dropped instead of polluting the registry.

MEDIUM: `../` relative includes produce spurious noise
  `#include "../foo.h"` is almost always intra-repo, but the suffix
  index can never match a `..`-prefixed path so it became a consumer
  contract no provider could satisfy. Now skipped before matching;
  covers both forward-slash and backslash forms.

MEDIUM: writeBridge error in sync.ts propagates uncaught
  contracts.json is the canonical source of truth and was just written
  successfully when writeBridge runs. A bridge-only failure (disk full,
  schema error, permission denied) shouldn't mask the registry. Wrapped
  writeBridge in try/catch with a logger.warn surfacing the path and
  recovery instructions.

Tests added:
  - extractProvidersGraph repo-relative ID generation (stub Cypher
    executor returns absolute paths)
  - extractProvidersGraph drops rows whose path resolves outside repo
  - `../foo.h` forward-slash skip
  - `..\foo.h` backslash-form skip

Skipped findings:
  - canExtract() removal (#5, low): canExtract is part of the
    ContractExtractor interface; every other extractor implements the
    same `return true` shape. Removing it from IncludeExtractor would
    break the interface contract — keeping for consistency.

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

* fix(group): close PR #1156 Codex adversarial findings

Two HIGH findings from the Codex adversarial review on
feat/group-include-extractor:

1. Default-on extraction silently changes existing groups (BLOCKER)
   DEFAULT_DETECT.includes was true, so any pre-existing group.yaml
   that omits the new field would gain a wave of include::* contracts
   on the next sync after upgrade. Flipped to false (opt-in). The
   integration test already declares includes: true explicitly so it
   survives unchanged; the unit extractor tests bypass parseGroupConfig
   entirely; the sync test uses extractorOverride. Only config-parser
   needed regression tests covering omitted/explicit/false variants.

2. IncludeExtractor scans outside the indexed file universe (BLOCKER)
   The extractor was running glob('**/*', { ignore: STANDARD_IGNORES })
   twice with a hand-rolled 9-pattern list, no .gitignore/.gitnexusignore
   honoring, and no max-file-size cap. That meant File:<path> contracts
   could appear for files ingestion would never index, producing
   cross-links group impact cannot fan out to (silent false-negatives).
   Refactored to a single discoverIndexableFiles() helper that mirrors
   walkRepositoryPaths exactly: createIgnoreFilter + getMaxFileSizeBytes,
   one discovery pass shared by provider and consumer paths. Dropped
   STANDARD_IGNORES and SOURCE_GLOB entirely.

   third_party and 3rdparty (the C/C++ vendored-deps conventions) were
   in the local ignore list but not in the canonical DEFAULT_IGNORE_LIST
   used by ingestion. Folded both into the canonical set rather than
   keep a parallel list — the whole point of the Codex finding is that
   two file-discovery implementations drift. Single source of truth.

Tests: 5 new regression tests for the discovery alignment (.gitignore,
.gitnexusignore, max-file-size on both provider and consumer paths)
plus 4 for the opt-in default. All 30 include-extractor tests + the
494-test group suite + ignore-service tests pass.

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

* fix(review): apply autofix feedback

ce-code-review surfaced 6 safe_auto findings on commit a9936a9:

- T1 (testing, P2): the sync.ts:174 gate was untested with includes:false.
  Added a sync-level test mirroring the existing thrift-off pattern at
  sync.test.ts:545, asserting zero include contracts when the gate is
  disabled in a real syncGroup call.

- T3 (testing, P3): third_party and 3rdparty entries in DEFAULT_IGNORE_LIST
  had no regression test. Added both to ignore-service.test.ts's
  dependency-directories it.each block.

- M1 (maintainability, P3): discoverIndexableFiles JSDoc lacked a
  fork-warning relative to walkRepositoryPaths. Added a MAINTENANCE
  note explaining why the duplication is tolerated and the contract
  the two implementations must keep.

- M2 (maintainability, P3): thrift-extractor still hand-rolls its
  ignore array with no signal that DEFAULT_IGNORE_LIST additions
  silently do not apply there. Added TODO(#1156-followup) comments
  above both call sites.

- M3 (maintainability, P3): SOURCE_EXTENSIONS duplicated the four
  HEADER_EXTENSIONS entries with no expressed subset relationship.
  Spread HEADER_EXTENSIONS into SOURCE_EXTENSIONS so future header-
  extension additions propagate.

- C1+T4 (correctness+testing, P3, cross-reviewer corroborated):
  discoverIndexableFiles swallowed all fs.stat errors silently,
  including EACCES/EMFILE/EIO. Narrowed the catch to ENOENT (the
  documented benign glob/stat race) and added a logger.warn for
  any other code so operators can spot permission/resource issues.

All 629 tests pass; typecheck + prettier clean.

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

* fix(group): use retryRename in writeContractRegistry to absorb Windows EPERM

`storage.ts:62` used raw `fsp.rename` for the contracts.json atomic swap.
On Windows, AV scanners and concurrent renames briefly hold the
destination handle between rename calls, surfacing as EPERM/EBUSY.
The `insecure-tempfile.test.ts > concurrent writes do not collide`
test was flaking with `EPERM: operation not permitted, rename` on
windows-latest CI.

`bridge-db.ts` already has a battle-tested `retryRename(src, dst, 3)`
helper used at six call sites for exactly this pattern. Reusing it
here keeps the Windows-rename policy single-source-of-truth across
the group package.

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

* fix(group): drop macro-style #include from consumer contracts

Tree-sitter's `(_) @import.source` wildcard matches the identifier node
of `#include PLATFORM_HEADER`, so the cleaned value `PLATFORM_HEADER`
slipped past the system-header / `..` filters and was emitted as a
permanently orphaned consumer contract (no file is named after a macro
identifier, so no provider can ever match). Add a shape guard that
skips cleaned values lacking both a path separator and an extension
dot, plus regression tests for single and multi-macro files.

Also document `IncludeExtractor.canExtract()` as unused by sync.ts
(gated via `config.detect.includes` instead) and kept solely for
ContractExtractor interface uniformity.

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

---------

Co-authored-by: HuangWenjie <zhoudeng.hwj@alibaba-inc.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
magyargergo added a commit that referenced this pull request May 9, 2026
Closes findings from the third multi-agent review pass on PR #1448.

#1 (P1) callLLM had no per-attempt timeout
  Wiki LLM calls passed no `signal` to resilientFetch; each of three
  retry attempts could hang indefinitely on a frozen TCP connection.
  Add `signal: AbortSignal.timeout(60_000)` so the per-attempt budget
  matches what http-client.ts and backend-client.ts already provide.

#2 (P2) drop dead `lastRetryableResp` post-loop fallback
  Variable was set in one switch arm but only read in unreachable code
  after the loop. The retry loop always returns/throws on every
  iteration. Keep only the defensive `throw` so TypeScript's
  control-flow analysis still sees `Promise<Response>` as the return.

#5 (P2) gate test-only exports behind a subpath
  `__resetBreakerRegistry__` and `classifyOutcome` were reachable from
  the main `gitnexus-shared` barrel — production code calling
  `__resetBreakerRegistry__` from a tool implementation would silently
  nuke every circuit breaker process-wide. Move to a new
  `gitnexus-shared/test-helpers` subpath export. Production callers
  see the cleaner public API; tests import via the explicit
  `gitnexus-shared/test-helpers` path.

#6 (P2) exhaustiveness guard on Outcome switch
  Add a `default: const _: never = outcome` arm so a future sixth
  `Outcome.kind` won't compile silently — it'll surface at the switch
  site rather than fall through to a retry/no-retry default.

#9 (P3) document cumulative wall-clock budget
  Add a "Cumulative wall-clock budget" paragraph to resilientFetch's
  JSDoc explaining the worst-case total wait (`maxAttempts × (per-attempt
  timeout + capDelayMs)` ≈ 60s with defaults) and pointing callers at
  outer `AbortSignal.timeout()` when they want a tighter bound.

Deferred to follow-up PRs (per review's Auto-resolve recommendation):
  - #3 idempotency knob to shared API (forceRetry into ResilientFetchOptions)
  - #4 publish.ts migration to resilientFetch
  - #7 parseRetryAfter past-HTTP-date / negative-seconds asymmetry
  - #8 recordNeutral counter time-decay (documented breaker semantic)
magyargergo added a commit that referenced this pull request May 9, 2026
* feat: shared resilient-fetch (retries + circuit breaker)

Add a small, runtime-agnostic resilience layer in gitnexus-shared and
migrate every backend HTTP outbound call (CLI, MCP, wiki LLM, web → backend)
through it.

Helpers (gitnexus-shared/src/integrations/):

- retry.ts            — withRetry(fn, opts) with caller-supplied
                        retryability classification and full-jitter
                        exponential backoff.
- circuit-breaker.ts  — closed/open/half-open per-process breaker with
                        injectable clock, plus a keyed registry so
                        callers targeting the same endpoint share state.
- resilient-fetch.ts  — composed wrapper: retries 5xx + 429 + retryable
                        network throws, treats AbortSignal.timeout()
                        and 4xx (other than 429) as terminal, honors
                        Retry-After (capped at 30s), throws
                        CircuitOpenError when the breaker opens.

Migrations (no behaviour regression — all existing tests pass):

- gitnexus/src/core/embeddings/http-client.ts (covers analyze + MCP
  query path) — replaces inline linear-backoff retry.
- gitnexus/src/core/wiki/llm-client.ts — preserves Azure content-filter
  branch; resilientFetch handles 5xx/429.
- gitnexus-web/src/services/backend-client.ts (fetchWithTimeout helper)
  — small retry budget (2 attempts, 250–1500 ms) so a dead local
  backend still fails fast for the user.
- gitnexus-web/src/core/llm/settings-service.ts (OpenRouter model list).

Deliberately not migrated:

- gitnexus-web/src/services/backend-client.ts streamJob() — Server-Sent
  Events stream; the existing reconnect-with-Last-Event-ID logic is
  not unary-fetch shaped.
- gitnexus-web/src/components/SettingsPanel.tsx checkOllamaStatus() —
  one-shot health probe; retrying delays the "Ollama not running"
  error rather than improving UX.

41 new helper tests cover backoff math, breaker state transitions,
Retry-After parsing (delta-seconds + HTTP-date), 401/422 terminal
classification, and breaker fail-fast on three exhausted retry batches.

* fix(review): apply autofix feedback

Address Claude's two MEDIUM blocking findings on PR #1448 plus the
CodeQL SSRF false-positive flag.

- backend-client `fetchWithTimeout` now uses `AbortSignal.timeout()`
  merged with the caller's signal via `AbortSignal.any()`. Timer-fired
  aborts surface as `DOMException(name='TimeoutError')` so
  resilientFetch routes them through the terminal-network branch
  (no retry, no breaker hit), instead of incrementing the breaker
  for user-side network slowness.
- Method-aware retry budget in `fetchWithTimeout`: idempotent verbs
  (GET/HEAD/OPTIONS) keep the 2-attempt budget; POST/PATCH/PUT/DELETE
  default to single-attempt so a 5xx on `startAnalyze` cannot start
  a duplicate job. New `forceRetry` parameter for callers that
  know-idempotent mutations (e.g. DELETE of a known-deleted resource).
- `resilient-fetch.ts` carries a documented suppression for CodeQL
  js/server-side-request-forgery on the inner fetch call. Every
  concrete caller passes a hardcoded URL constant or a value from
  configuration (env vars, saved settings); user request input never
  flows into the URL parameter.
- New test file `backend-client-retry.test.ts` covers all three
  paths: GET retries on 503, POST does not retry, timeout does not
  increment the breaker.

* fix(resilient-fetch): address Codex adversarial findings

Closes the three blocking issues from Codex's review on PR #1448.

U1 — Add `recordNeutral()` to CircuitBreaker.
  Third outcome path that's an explicit no-op for state and the
  consecutive-failure counter. Distinct from `recordSuccess` (closes
  the breaker) and `recordFailure` (may open it). Used for outcomes
  that are neither evidence of backend health nor evidence of
  backend failure.

U2 — Route terminal-client / terminal-network through `recordNeutral`.
  Previously a 401 or local timeout called `recordSuccess`, which
  reset `consecutiveFailures` to 0. A 5xx → 401 → 5xx → 401 → 5xx
  sequence would NEVER trip the breaker because each 4xx in between
  erased the running count. Also classify external `AbortError` as
  terminal-network (was retryable-network), so caller-driven
  cancellation no longer retries against an already-aborted signal
  or counts toward breaker failures on exhaustion.

U3 — Per-origin breaker key in web `fetchWithTimeout`.
  Was hardcoded to `'web-backend'` even though `_backendUrl` is
  mutable via `setBackendUrl`. Switching backend URLs after a
  circuit tripped on host-A would strand the user during the full
  cooldown. Key is now `web-backend:<origin>`, so each backend URL
  gets its own breaker state.

Tests: +5 recordNeutral, +4 resilient-fetch (interleaved 4xx/5xx,
external AbortError, prior-state preservation), +1 web switch-backend
regression. All 70 gitnexus integration tests + 15 web tests green.

* fix(resilient-fetch): tolerate header-less fetch mocks on 429

`classifyOutcome` called `resp.headers.get('Retry-After')` directly,
which crashed when a test stubs `fetch` with a plain object like
`{ ok: false, status: 429 }` (no `headers` field). Real `Response`
always has Headers, so this surfaces only in test setups, but the
helper has no business assuming caller-side correctness on this — the
defensive guard is cheap and a missing `Retry-After` falls through to
exponential-backoff retry like any 429 without the header.

Surfaced by `gitnexus/test/unit/http-embedder.test.ts > retries on
rate limit`, which the embeddings migration exercises against a
plain-object 429 stub. Locked in with a new
`classifies 429 from a header-less fetch mock without throwing` case.

* fix(review): apply autofix feedback

Closes findings from the third multi-agent review pass on PR #1448.

#1 (P1) callLLM had no per-attempt timeout
  Wiki LLM calls passed no `signal` to resilientFetch; each of three
  retry attempts could hang indefinitely on a frozen TCP connection.
  Add `signal: AbortSignal.timeout(60_000)` so the per-attempt budget
  matches what http-client.ts and backend-client.ts already provide.

#2 (P2) drop dead `lastRetryableResp` post-loop fallback
  Variable was set in one switch arm but only read in unreachable code
  after the loop. The retry loop always returns/throws on every
  iteration. Keep only the defensive `throw` so TypeScript's
  control-flow analysis still sees `Promise<Response>` as the return.

#5 (P2) gate test-only exports behind a subpath
  `__resetBreakerRegistry__` and `classifyOutcome` were reachable from
  the main `gitnexus-shared` barrel — production code calling
  `__resetBreakerRegistry__` from a tool implementation would silently
  nuke every circuit breaker process-wide. Move to a new
  `gitnexus-shared/test-helpers` subpath export. Production callers
  see the cleaner public API; tests import via the explicit
  `gitnexus-shared/test-helpers` path.

#6 (P2) exhaustiveness guard on Outcome switch
  Add a `default: const _: never = outcome` arm so a future sixth
  `Outcome.kind` won't compile silently — it'll surface at the switch
  site rather than fall through to a retry/no-retry default.

#9 (P3) document cumulative wall-clock budget
  Add a "Cumulative wall-clock budget" paragraph to resilientFetch's
  JSDoc explaining the worst-case total wait (`maxAttempts × (per-attempt
  timeout + capDelayMs)` ≈ 60s with defaults) and pointing callers at
  outer `AbortSignal.timeout()` when they want a tighter bound.

Deferred to follow-up PRs (per review's Auto-resolve recommendation):
  - #3 idempotency knob to shared API (forceRetry into ResilientFetchOptions)
  - #4 publish.ts migration to resilientFetch
  - #7 parseRetryAfter past-HTTP-date / negative-seconds asymmetry
  - #8 recordNeutral counter time-decay (documented breaker semantic)

* fix(circuit-breaker): gate half-open to a single in-flight probe

Closes the Codex adversarial-review finding on PR #1448 that flagged a
recovery-time thundering herd: when cooldown expired, every concurrent
caller transitioned the breaker to half-open and probed the still-
recovering dependency in lockstep, defeating the breaker's "fail fast"
promise.

U1 — probe-permit gate in CircuitBreaker.check()
  Added a `probeInFlight: boolean` field. After cooldown expires, the
  first `check()` admits the probe and consumes the permit; subsequent
  callers throw `CircuitOpenError` with a configurable
  `halfOpenRetryAfterMs` (default 1000ms) until the probe resolves.

  Critical design point: `recordNeutral` now RELEASES the permit but
  does NOT transition state. Without that split, a single `TimeoutError`
  from per-attempt `AbortSignal.timeout` (which routes through neutral
  classification) would permanently park the breaker in half-open. By
  separating permit-release from state-resolution, we keep the
  "neutral doesn't claim health" semantic without creating that wedge.

  Other changes:
  - `halfOpenRetryAfterMs` is now a constructor option for consumers
    with long-running protected ops (LLM streaming, large uploads).
  - `getState()` is documented as a pure read; the implicit
    Open -> Half-Open transition lives in `check()` only, so tests
    that inspect state never inadvertently consume a probe permit.
  - `isProbeInFlight()` test-only accessor for assertion clarity.
  - JSDoc on `check()` records the JS event-loop atomicity dependency
    and the load-bearing `try/finally` pairing invariant.

U2 — End-to-end concurrency regression through resilientFetch
  Three new scenarios in resilient-fetch.test.ts (26 -> 29):
  - 3 concurrent calls + probe gets 200 -> 1 hits fetch, 2 throw
    CircuitOpenError, breaker closes.
  - 3 concurrent calls + probe gets 503 -> ResilientFetchExhaustedError
    on probe; concurrent callers see halfOpenRetryAfterMs (1000ms);
    fresh caller after probe resolves sees the FULL new cooldown
    (10000ms), not the probe-in-flight default.
  - Probe cancelled mid-flight via AbortError -> permit released,
    state stays half-open, next caller becomes the new probe and
    succeeds.

Plus 9 new circuit-breaker unit tests (16 -> 25) covering the permit
gate, recordNeutral-releases-permit semantic, fresh-cooldown distinction,
default vs configurable halfOpenRetryAfterMs, getState() purity, and
the three-probes-via-neutrals chain.

Total integration test count: 70 -> 82. All 106 gitnexus + 15 web
tests pass; both packages typecheck.

Maintainer decisions (deferred per plan 003 Open Questions):
  - Plan 002's deferral judgement was reversed on Codex's argument
    without new measurement / incident data. The reversal is defensible
    on principle (Hystrix / Resilience4j alignment) but lacks workload-
    driven evidence.
  - Probe-blocked callers throw silently (no log / event hook). R4's
    "no new public API" prevents adding observability; loosen if a
    debug log on probe-blocked is wanted.

* refactor(embeddings): replace bespoke HF breaker with shared CircuitBreaker

Deleted the local `HfDownloadCircuitBreaker` class and the manual
retry loop in `withHfDownloadRetry`. Both are now backed by the
shared `gitnexus-shared` primitives:

- `hfDownloadCircuit` is `new CircuitBreaker({ failureThreshold,
  cooldownMs, key: 'hf-download' })` — same state machine as before
  PLUS the single-permit half-open gate that prevents recovery-time
  stampedes when CLI + MCP embedders concurrently re-load the model.
- `withHfDownloadRetry` delegates the loop to `withRetry` from the
  shared package. Per-attempt timeout (`withDownloadTimeout`),
  network-vs-non-network classification, circuit recording, and the
  `onRetry` callback wire through `withRetry`'s `isRetryable`
  callback.

Behaviour preserved:
- Pre-flight `CIRCUIT_OPEN_TAG` rejection when the breaker is open.
- Mid-loop `CIRCUIT_OPEN_TAG` "opened after N consecutive failures"
  when a network error trips the threshold.
- Non-network errors (e.g. CUDA unavailable) bypass retry and go
  through `recordNeutral` instead of resetting the breaker's
  failure-count progress.
- `onRetry(attempt+1, max, err)` fires only when there's a next
  attempt, matching the prior semantic.

Generic CircuitBreaker gained two inspection accessors:
- `getOpenedAt(): number | null`
- `getCooldownMs(): number`
Used by `withHfDownloadRetry` to compute `secsUntilReset` without
consuming a probe permit (which `check()` would do).

Test consolidation: the 7 bespoke `HfDownloadCircuitBreaker`
state-machine tests in hf-env.test.ts were 1:1 duplicates of
existing tests in `circuit-breaker.test.ts` and were deleted.
Remaining 42 hf-env tests all pass; full integration sweep (148
gitnexus + 15 web) green.
magyargergo added a commit that referenced this pull request May 9, 2026
ce-code-review surfaced 15 findings on PR #1458; this commit applies
the 7 with concrete fixes (#1, #2, #3, #4, #5, #9, #13). Five P2
findings (#6, #7, #8, #10, #12) are recorded as residual actionable
work for follow-up; two advisory items (#11, #14) skipped.

#1 — applied_run_id schema drift (CONTRIBUTING.md):
  v2 docs claimed `state: applied` enum value and an `applied_run_id`
  field that no code path emits. Trimmed docs to match what the
  workflow actually writes (state: fixes-available; v1 field set as
  superset). Implementing the apply-side sticky upsert that would
  populate `applied_run_id` is deferred — cleaner than carrying a
  contract claim with no code.

#2 — result= unset between idempotency probe and lease push
(pr-autofix-apply.yml):
  After `git apply --check` passed, an early non-zero exit from
  `git config` / `git apply` / `git add` / `git commit` left
  `result=` unset, sending the user to the `*` "unexpected state
  (`unknown`)" arm. Wrapped the apply/commit phase in a single
  if-test that sets `result=apply-failed` on any failure. New
  React-and-reply branch surfaces an actionable message.

#3 — permission lookup conflated transient API failures with denial
(pr-autofix-apply.yml):
  `gh api … 2>/dev/null || echo "none"` swallowed 5xx, 429 secondary
  rate-limit, and network failures, surfacing them as a public 👎
  refusal to legitimate maintainers. Now distinguishes 404
  (genuine non-collaborator) from other API failures via stderr
  match. New `allowed=api-failed` state triggers a 😕 reaction with
  a "transient API failure, retry" reply instead of a misleading
  refusal.

#4 — lease-failure grep missed git's "remote rejected" / branch-
deleted phrasings (pr-autofix-apply.yml):
  Real lease failures got classified as `push-failed` →
  user told to enable maintainer-edit, which won't help. Expanded
  regex to match `remote rejected` and `! [rejected]`.

#5 — broken bullet continuation in CONTRIBUTING.md release-candidate
section: rejoined the split bullet so it renders correctly.

#9 — base64 GITHUB_TOKEN bypassed GitHub's secret-masker
(pr-autofix-apply.yml):
  Added `::add-mask::${auth_header}` immediately after construction
  so any subsequent log line (set -x, GIT_TRACE) gets *** redacted.

#13 — misleading schema-bump comment in pr-autofix-publish.yml:
  Comment claimed all v1 fields preserved exactly, but the `state`
  enum was redefined v1→v2. Updated to make the migration path
  explicit (v1 readers see unfamiliar schema, fall back to prose).

Residual actionable work (deferred to follow-up):
  #6 locate step gh api retry; #7 artifact-expired graceful fallback;
  #8 re-entrancy comment-spam guard; #10 producer-still-running UX;
  #12 gh_retry wrapper for apply.yml.

Validations: yaml.safe_load OK, check-workflow-concurrency.py OK.
magyargergo added a commit that referenced this pull request May 9, 2026
#8, #10, #12)

Pulls the deferred items from the previous review pass into this PR so
the workflow ships with full reliability + UX coverage rather than
follow-up debt.

#6 + #12 — gh_retry wrapper on idempotent GETs in apply.yml:
  Permission lookup, PR metadata fetch, and workflow-run lookup are now
  wrapped in the same gh_retry helper publish.yml uses (3 attempts,
  linear backoff). Reaction/comment POSTs remain unwrapped (retrying
  POST would dupe the resource).

#10 — producer-still-running UX:
  The locate step now distinguishes three cases via `found_status`
  output: success (proceed), in-progress / queued / pending / waiting
  (reply ⏳ "wait for autofix run to finish"), not-found (reply 🤔
  "push a commit"), api-failed (reply ⚠️ "transient API failure"). The
  "no successful autofix run" message no longer fires immediately after
  a fresh push while the producer is still mid-run.

#7 — artifact-expired graceful fallback:
  actions/download-artifact gains `continue-on-error: true`. The apply
  step distinguishes patch-file-missing (artifact expired, 1-day
  retention elapsed) from patch-file-zero-bytes (formatter found
  nothing). New `result=artifact-expired` case + ⏳ "push a new commit
  to regenerate" reply.

#8 — re-entrancy loop guard:
  After checkout but before applying, check if HEAD itself is a
  github-actions[bot] `chore(autofix)` commit. If so, refuse to
  re-apply (`result=loop-prevented`) with a 🔁 reply telling the user
  to push a human-authored commit or revert before retrying. Prevents
  formatter-config-drift loops where an automated agent watching the
  sticky could pump arbitrary apply commits.

Net effect: every code path in apply.yml now sets a meaningful `result=`
that maps to a specific user-facing reaction + reply. The `*` "unexpected
state (unknown)" arm becomes truly unreachable in normal operation.

Validations: yaml.safe_load OK, check-workflow-concurrency.py OK.
magyargergo added a commit that referenced this pull request May 9, 2026
…1458)

* fix(autofix): verify reviewdog actually posted before claiming "click Apply"

The sticky summary comment was stating "Posted formatting suggestions
inline. Click Apply suggestion on each" even when reviewdog landed zero
inline review comments — typical case: the formatter touched lines
outside the PR's added range, so `-filter-mode=added` (correctly)
filtered everything out. The script unconditionally set `posted=true`
after running reviewdog regardless of whether any comments were
actually created, leaving the user staring at a sticky that promised
buttons that didn't exist.

The publish job now snapshots the count of `github-actions[bot]` review
comments before and after reviewdog. If the delta is zero, surface a
new `diff-no-overlap` UI state that tells the user plainly:

  "Formatter found fixable issues, but they're on lines outside this
  PR's added range — there's nothing to click here. Run locally:
  npm run lint:fix && npm run format."

Plus a matching `gitnexus/autofix` Check Run conclusion (still neutral,
distinct title) so agents reading `gh pr checks` see the same signal.

Three states are now machine-distinguishable in the sticky's
gitnexus-autofix JSON block: suggestions-posted (delta > 0),
diff-no-overlap (delta == 0), skipped-too-large (>3k lines).

* feat(autofix): replace inline reviewdog with /autofix ChatOps button

Pivot the PR autofix UX from per-line reviewdog suggestions to a single
slash-command button. Contributors comment `/autofix` on the PR; a new
trusted workflow downloads the existing autofix patch artifact, applies
it to the PR head, and pushes a commit back.

Why:
- 3K+ diffs hit GitHub's review-comment API 406 limit -> dead end.
- Diffs where the formatter touches lines outside the PR's added range
  ("no-overlap") get filtered by reviewdog's -filter-mode=added -> dead
  end (PR #1457 patched the lying sticky but the underlying UX gap
  remained).
- Per-line click-Apply-suggestion is high-friction for big diffs and
  easy to apply unevenly.
- A single `git apply` + push works at any size and lands fixes
  atomically.

Changes:
- pr-autofix-publish.yml: remove `Install reviewdog` and
  `Post inline suggestions` steps. Collapse three sticky states
  (suggestions-posted, diff-no-overlap, skipped-too-large) into one
  (fixes-available). Bump JSON schema v1 -> v2 with `apply_command`
  field; all v1 fields preserved.
- pr-autofix-apply.yml (new): triggers on issue_comment with body
  `/autofix`, validates body via strict regex, validates commenter
  has write/admin/maintain or is the PR author, locates latest
  successful pr-autofix run for PR head SHA, downloads artifact,
  applies patch, pushes commit. Reacts +1/-1/eyes on triggering
  comment per outcome. Idempotent (`git apply --check --reverse`
  detects already-applied state).
- CONTRIBUTING.md: document v2 schema and the /autofix flow,
  including the maintainer-edit requirement for fork PR pushes.

Trust posture: apply workflow runs from default-branch code only,
under issue_comment trigger. Comment body and author login flow
through env vars and pattern-matched, never interpolated into shell.
Permission gate (write/admin/maintain OR PR author) before any
artifact fetch. Fork PRs require "Allow edits by maintainers"
(GitHub-native; we don't bypass).

Net YAML: -139 lines in publish.yml, +260 in apply.yml. Removes
reviewdog binary pin and the entire review-comment API surface.

* fix(autofix): address Codex adversarial findings on PR #1458

Two findings from the Codex adversarial review of the autofix ChatOps
pivot. Both are localized YAML changes that close trust gaps the pivot
inherited from the original PR #1446 design.

U1 — Cross-verify metadata against workflow_run authority
(.github/workflows/pr-autofix-publish.yml):
  Previously the trusted publisher accepted pr_number, head_sha, and
  head_repo from metadata.json after only an allowlist regex. A
  fork-controlled `npm run lint:fix` could have written a syntactically
  valid metadata.json referencing another PR/SHA, redirecting the
  write-scoped sticky/check-run onto an attacker-chosen target.

  New `Verify metadata against workflow_run authority` step compares
  artifact-claimed identity against:
    - github.event.workflow_run.head_sha
    - github.event.workflow_run.head_repository.full_name
    - workflow_run.pull_requests[].number (within-repo PRs)
    - gh api commits/{sha}/pulls fallback (fork PRs, where
      pull_requests[] is empty)
  Fail closed on mismatch — no sticky, no check-run, no override.

U2 — Lease-protected push in apply workflow
(.github/workflows/pr-autofix-apply.yml):
  Previously the apply step pushed `HEAD:${HEAD_REF}` plain. A force-
  push between resolve (Step 5) and push (Step 9) would silently
  fast-forward an older commit graph over the contributor's newer
  state.

  Push now uses `--force-with-lease=refs/heads/${HEAD_REF}:${HEAD_SHA}`
  against the SHA resolved earlier. Distinct `lease-failed` result code
  + retry-message reply, separated from `push-failed` (fork without
  maintainer-edit) so contributors can diagnose the actual cause.

Plan: docs/plans/2026-05-09-005-fix-autofix-codex-adversarial-findings-plan.md
(local-only per repo convention).

Trust posture preserved: no new permissions, no new workflows, no
contract change. JSON v2 schema unchanged. CodeQL js/server-side-
request-forgery and template-injection posture unchanged — all new
inputs flow via env vars and pattern-matched.

* fix(autofix): close zizmor credential-persistence finding on apply checkout

actions/checkout's default behavior writes the GITHUB_TOKEN into
.git/config as an extraheader. The token then sits on disk in the
checkout directory — an actions/upload-artifact step on that
directory would leak it. We don't upload, but zizmor's
credential-persistence lint correctly flags the latent risk.

Set persist-credentials: false on the Checkout PR head step. Provide
push auth inline via `git -c http.extraheader="Authorization: Basic
<base64-of-x-access-token:TOKEN>"` so the credential never lands on
disk and never appears in process listings (the URL form
https://x-access-token:TOKEN@… is rejected here because it leaks via
ps and git remote -v).

Push lease semantics from U2 unchanged — same --force-with-lease
against the resolved HEAD_SHA, same lease-failed/push-failed/stale
result codes.

* fix(review): apply autofix feedback

ce-code-review surfaced 15 findings on PR #1458; this commit applies
the 7 with concrete fixes (#1, #2, #3, #4, #5, #9, #13). Five P2
findings (#6, #7, #8, #10, #12) are recorded as residual actionable
work for follow-up; two advisory items (#11, #14) skipped.

#1 — applied_run_id schema drift (CONTRIBUTING.md):
  v2 docs claimed `state: applied` enum value and an `applied_run_id`
  field that no code path emits. Trimmed docs to match what the
  workflow actually writes (state: fixes-available; v1 field set as
  superset). Implementing the apply-side sticky upsert that would
  populate `applied_run_id` is deferred — cleaner than carrying a
  contract claim with no code.

#2 — result= unset between idempotency probe and lease push
(pr-autofix-apply.yml):
  After `git apply --check` passed, an early non-zero exit from
  `git config` / `git apply` / `git add` / `git commit` left
  `result=` unset, sending the user to the `*` "unexpected state
  (`unknown`)" arm. Wrapped the apply/commit phase in a single
  if-test that sets `result=apply-failed` on any failure. New
  React-and-reply branch surfaces an actionable message.

#3 — permission lookup conflated transient API failures with denial
(pr-autofix-apply.yml):
  `gh api … 2>/dev/null || echo "none"` swallowed 5xx, 429 secondary
  rate-limit, and network failures, surfacing them as a public 👎
  refusal to legitimate maintainers. Now distinguishes 404
  (genuine non-collaborator) from other API failures via stderr
  match. New `allowed=api-failed` state triggers a 😕 reaction with
  a "transient API failure, retry" reply instead of a misleading
  refusal.

#4 — lease-failure grep missed git's "remote rejected" / branch-
deleted phrasings (pr-autofix-apply.yml):
  Real lease failures got classified as `push-failed` →
  user told to enable maintainer-edit, which won't help. Expanded
  regex to match `remote rejected` and `! [rejected]`.

#5 — broken bullet continuation in CONTRIBUTING.md release-candidate
section: rejoined the split bullet so it renders correctly.

#9 — base64 GITHUB_TOKEN bypassed GitHub's secret-masker
(pr-autofix-apply.yml):
  Added `::add-mask::${auth_header}` immediately after construction
  so any subsequent log line (set -x, GIT_TRACE) gets *** redacted.

#13 — misleading schema-bump comment in pr-autofix-publish.yml:
  Comment claimed all v1 fields preserved exactly, but the `state`
  enum was redefined v1→v2. Updated to make the migration path
  explicit (v1 readers see unfamiliar schema, fall back to prose).

Residual actionable work (deferred to follow-up):
  #6 locate step gh api retry; #7 artifact-expired graceful fallback;
  #8 re-entrancy comment-spam guard; #10 producer-still-running UX;
  #12 gh_retry wrapper for apply.yml.

Validations: yaml.safe_load OK, check-workflow-concurrency.py OK.

* fix(autofix): apply remaining ce-code-review residual findings (#6, #7, #8, #10, #12)

Pulls the deferred items from the previous review pass into this PR so
the workflow ships with full reliability + UX coverage rather than
follow-up debt.

#6 + #12 — gh_retry wrapper on idempotent GETs in apply.yml:
  Permission lookup, PR metadata fetch, and workflow-run lookup are now
  wrapped in the same gh_retry helper publish.yml uses (3 attempts,
  linear backoff). Reaction/comment POSTs remain unwrapped (retrying
  POST would dupe the resource).

#10 — producer-still-running UX:
  The locate step now distinguishes three cases via `found_status`
  output: success (proceed), in-progress / queued / pending / waiting
  (reply ⏳ "wait for autofix run to finish"), not-found (reply 🤔
  "push a commit"), api-failed (reply ⚠️ "transient API failure"). The
  "no successful autofix run" message no longer fires immediately after
  a fresh push while the producer is still mid-run.

#7 — artifact-expired graceful fallback:
  actions/download-artifact gains `continue-on-error: true`. The apply
  step distinguishes patch-file-missing (artifact expired, 1-day
  retention elapsed) from patch-file-zero-bytes (formatter found
  nothing). New `result=artifact-expired` case + ⏳ "push a new commit
  to regenerate" reply.

#8 — re-entrancy loop guard:
  After checkout but before applying, check if HEAD itself is a
  github-actions[bot] `chore(autofix)` commit. If so, refuse to
  re-apply (`result=loop-prevented`) with a 🔁 reply telling the user
  to push a human-authored commit or revert before retrying. Prevents
  formatter-config-drift loops where an automated agent watching the
  sticky could pump arbitrary apply commits.

Net effect: every code path in apply.yml now sets a meaningful `result=`
that maps to a specific user-facing reaction + reply. The `*` "unexpected
state (unknown)" arm becomes truly unreachable in normal operation.

Validations: yaml.safe_load OK, check-workflow-concurrency.py OK.

* fix(autofix): refresh stale reviewdog comments + reject patches touching .github/

Two follow-up findings on PR #1458:

#1 — Stale reviewdog references in workflow header comments:
  pr-autofix-publish.yml's header still described the removed inline-
  suggestion path ("posts inline review-comment suggestions to the PR
  using `reviewdog`", "Reviewdog reporter: github-pr-review reads
  $REVIEWDOG_GITHUB_API_TOKEN…"). The Check Run permissions comment
  enumerated the old outcomes (clean / suggestions-posted /
  skipped-too-large) instead of the current set (clean / fixes-
  available). pr-autofix.yml's header described the trusted job as
  posting "inline review-comment suggestions" and the changed_lines
  comment referenced the dead 3000-line cap. Refreshed all three to
  describe the actual sticky + Check Run + /autofix flow.

#2 — Reject patches touching .github/ (sensitive-paths guard):
  Theoretical supply-chain vector: a malicious PR could ship a custom
  prettier/ESLint config that reformats workflow YAML, dependabot.yml,
  or CODEOWNERS. The producer would capture those edits in
  autofix.patch; a maintainer running `/autofix` would push them under
  `contents: write` without human review. The default GITHUB_TOKEN
  lacks the `workflows` scope so workflow-file pushes would fail at
  the platform layer anyway, but as a generic `push-failed` (which
  misleads users into enabling maintainer-edit). Reject early with
  a specific reason.

  Match runs against the patch with grep on `^(diff --git|---|+++)
  [ab]?/?\.github/`. New `result=sensitive-paths` case + 🛑 reply
  telling the user to apply .github/ formatter changes manually.

  Documented the constraint in CONTRIBUTING.md under the /autofix
  section so contributors aren't surprised when the workflow refuses
  a patch that includes formatter changes to workflow files.

Validations: yaml.safe_load OK, check-workflow-concurrency.py OK.
magyargergo added a commit to SZU-WenjieHuang/GitNexus that referenced this pull request May 12, 2026
- 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.
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
)

* 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>
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
…ewarm chip (abhigyanpatwari#1#2#4#6)

Iteration after browser-verifying commit-mode. Browser-verified (Playwright) PASS.

- abhigyanpatwari#1+abhigyanpatwari#2 density/window: commit dots filtered to the effective zoom window
  [cursorA,cursorB] + even-spaced; capped at 60 with even-sampling; wheel-zoom
  thins them; overflow hint. THE real blocker turned out to be the timeline
  toolbar overflowing the row (bar squeezed to ~50px / off-screen) — fixed by
  flex-wrap on the row + the scrub bar taking its own full-width line in Commits
  mode (w-full basis-full). Verified: 60 dots @ ~32px, clickable without force.
- abhigyanpatwari#4 era-baseline: Seed-baseline now anchors at the window's OLDEST commit (not
  the clicked one) so the whole window is navigable from one seed;
  seedBaseline(baselineSha, retrySha?) reconstructs the clicked commit after.
- abhigyanpatwari#6 prewarm feedback: poll GET /snapshot/prewarm → "{warm}/{total} chauds" chip
  in Commits mode (verified showing 103/163).

Backend unchanged (abhigyanpatwari#4 changes which sha the front passes; abhigyanpatwari#6 reads existing GET).
Patches via split scheme: Timeline.tsx → additive-files.diff,
useAppState.tsx → inplace-edits.diff (0 binary). Spec amended (## Update 2026-05-29).

Aside: recovered the gitnexus backend mid-verify — its FTS extension
(libfts.lbug_extension) had gone missing after a container recreate, 500-ing
/api/graph for every repo; a `docker compose restart gitnexus` re-installed it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant