Repository navigation
feat(upstream): catch up, batch 1 — twelve fixes that apply as written - #596
Merged
Merged
Conversation
…odegraph_node (colbymchenry#3030) codegraph_node's multi-definition answer rendered a name's definitions in index order (getNodesByName: file_path, start_line) until its caps ran out: 16 bodies within 12,000 characters, then 20 list lines, then a bare "+N more". Forty definitions under an early-sorting folder hid the only one in another language or a late folder: on the triage fixture (40 TypeScript `num` in aaa_client/, one Python `num` in zzz_server/), zzz_server/format.py appeared nowhere. Without includeCode, a long list was cut by the output cap at "... (output truncated)". The bodies still fill in index order, so every overload that rendered before still renders. On top of them, each language with no body gets its first hand-written definition rendered, while the whole answer stays under the output cap. The list of the rest fills its 20 slots in a new definitionCapOrder (the first definition of each language, then of each top-level folder within a language, then the rest, each tier in index order) and is shown in index order; the overflow line names what it leaves out by language and folder, like "+365 more: 365 typescript in aaa_client/". The list-only answer is unchanged unless it is longer than the output cap; then it fills in the same order and names the rest the same way. Generated stubs stay behind hand-written definitions. Promoting each folder's first definition into the bodies, as first tried, cost Alamofire's `validate` three Validation.swift overloads to a Tests/ helper, and `adapt`, `urlSession`, `encode` and `cancel` one or two Source/ bodies each, so folders get early list slots only. Validation: new node-overload-spread tests (6): 5 fail on main's source (the fits-every-cap guard passes on both) and all pass with the fix. A/B of every multi-definition name, with and without includeCode, main vs fix: gin 171 names, 170 identical (`_` only names its overflow); Alamofire 706 names, 702 identical. No body that rendered before is lost: `cancel` adds the jQuery `cancel` from docs/ as a 13th body, and `request`, `urlSession` and `encode` swap one Source/ list line for the first Tests/ one and name their overflow. Alamofire's `validate`, `task` and `didCompleteTask` and gin's `reset`, `Bind` and `Render` are byte-identical. Reimplements PR colbymchenry#1657 by @ferrine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…olbymchenry#3043) Both extractors created a Go var/const node from its var_spec or const_spec without exportedness: the TS Go branch passed no isExported (the declaration-level value is read off the var_declaration, which has no name), and the kernel's extract_variable set none. Every such node was unexported, so findGoPackageMember, which looks up pkg.Name among the package's exported declarations, skipped it: a call through `var Hook = func...` from another package linked to nothing, though the same call inside the package linked to the variable. Both engines now mark the node exported by its own name's case, read from the spec through goExtractor.isExported / is_exported, as functions, methods and types already were. Validation: - __tests__/go-package-value-exported.test.ts, 16 tests (extraction on the kernel and wasm, LF and CRLF; cross-package calls on both paths): 12 fail on the base main, 6 with only the TS half (the kernel checks), all pass with the fix. - A/B, main vs fix, kernel path: gin 108 var/const flags, 0 edges; prometheus 625 flags, 0 edges; etcd 806 flags and 12 new calls, all right (e2e.GetVersionFromBinary x11 and model.NonDeterministicModel, both `var X = func(...)`); flask and express byte-identical. The fix's kernel and wasm graphs are byte-identical on all three Go repos, and the wasm A/B matches. - kernel-parity --lang go: gin 99/99, prometheus 755/755, etcd 1065/1105 with 40 deferred on main too; 0 diffs. - etcd index time, median of 3 interleaved: 13.4 s main, 12.6 s fix. Reimplements PR colbymchenry#1521 by ericquan8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… unused types package (colbymchenry#3044) devalue 5.9.1 (root, through svelte) and 5.8.1 (site/, through astro) fall in seven advisories published 2026-10-01 and fixed in 5.9.3, among them GHSA-j22f-vq7h-c4qm (CVE-2026-92708, high). postcss 8.5.6 (root, through vite) and 8.5.15 (site/) fall in GHSA-r28c-9q8g-f849 (CVE-2026-73646, high, fixed in 8.5.18) and GHSA-fxqj-rqcc-2cmp (fixed in 8.5.23); the root copy also in GHSA-6g55-p6wh-862q and GHSA-qx2v-qp2m-jg93. All of these are build-time dependencies. Lockfile-only updates inside the existing ranges: npm update devalue and npm update postcss, in the root and in site/. devalue goes to 5.9.4 and postcss to 8.5.32, whose own ranges move nanoid to 3.3.20 and source-map-js to 1.2.2, which clears their advisories as well (GHSA-28wg-ghj8-5hjv, GHSA-2v37-7h3g-55p8, GHSA-68fv-2mgg-jv7q). The @types/better-sqlite3 devDependency goes: nothing has imported better-sqlite3 since the switch to node:sqlite. In site/, npm 10 also drops five "peer": true markers that the npm which wrote that lockfile had added; they are metadata only, and npm ci installs the same tree. Validation: - npm audit: root 14 advisories before, 10 after; site/ 13 before, 9 after. devalue, postcss, nanoid and source-map-js are gone from both. - npm run build on main's lockfile and on this one: all 1,264 files of dist/, the viewer included, are byte-identical. The site builds its 21 pages, and its 101-file dist/ is byte-identical too. - The release bundle's install (npm ci --omit=dev --ignore-scripts on the staged lockfile) installs the same 12 packages, file for file. - All 8 changed resolved/integrity entries match the npm registry's own metadata and sit on registry.npmjs.org. No git, URL or tarball sources, and no new install scripts. devalue, postcss and nanoid carry provenance from their own repositories' release workflows. Raised by PR 2885 (@johnhldiver-beep) and PR 1477 (@anupamme). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> [skip actions]
…colbymchenry#3050) codegraph applies .gitignore rules itself, through the ignore package: on the filesystem walk for every .gitignore, and in git repositories on top of git ls-files for its built-in defaults merged with the root .gitignore and git's exclude files. codegraph.json's include, exclude and includeIgnored patterns go through it too. ignore 7.0.5 read several forms differently from git: a backslash escape (\d was a regex digit), POSIX classes and a ] or ! first in a bracket, and a run of three asterisks. A lone ! line matched every path, so it re-included everything the rules and the built-in defaults leave out, node_modules included. 2,031 of 200,000 random patterns also threw at match time, which readGitignorePatterns has to guard against (issue 682). ignore 7.0.14, with the range raised to ^7.0.14, reads all of these as git does, and none of the 200,000 patterns throws. Its code diff from 7.0.5 was read in full: pattern translation only, with no I/O, no dependencies and no install scripts. The lockfile entry matches the npm registry's own metadata on registry.npmjs.org. The package has no provenance, like every release its maintainer has published. Validation: - New __tests__/gitignore-pattern-parity.test.ts checks scanDirectory against git's own answer for these patterns, on the filesystem walk and in a git repository: its 4 tests fail with 7.0.5 and pass with 7.0.14. - scanDirectory on react, astro and django, in git mode and in walk mode on a copy without .git, returns the same 4,631, 4,946 and 3,027 files with either version. - On a fixture of the changed forms, 7.0.14 leaves out exactly what git leaves out in both modes, node_modules aside, which codegraph's defaults always exclude. With a lone ! line, 7.0.5 kept 16 of the 18 files on the walk, node_modules among them, where git keeps 6. Without that line it differed from git on 8 files on the walk and on 2 in git mode. - The gitignore and scope test files (extraction, git-ignored-dirs-scope, the include, exclude and includeIgnored configs, src-build-folders and others): 853 passed. Raised by PR 2885 (@johnhldiver-beep). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry#3056) Issue colbymchenry#2979: FileLock.acquire() read the lock file, saw a dead PID, unlinked the file and created its own with an exclusive create. Two processes that read the same dead PID could both succeed: the first one replaced the stale lock, then the second one, acting on what it had read a moment earlier, unlinked the first one's live lock and created its own. Both returned from acquire() and wrote to the index as its only writer. The same steal happened in two narrower ways: a lock file read in the instant between its exclusive create and the write of its PID was empty, so it counted as stale, and a PID the process may not signal (EPERM: another user's process, or an elevated one on Windows) counted as dead. The fix keeps the lock file and its format (a bare PID) and changes how it is taken: - A free lock costs one create, as before. The lock file is created with its PID already in it: a private file hard-linked into place, which fails if the lock exists. Where hard links are missing it falls back to the exclusive create, and an empty lock younger than 10 s counts as held, since its creator is still writing its PID. - A stale lock (dead PID, malformed, or empty and old) is replaced only by the process holding <lock>.break, itself a FileLock, after reading the lock again there, the way Mercurial breaks its locks. A break lock a crashed process left is broken the same way, one level up, at most three levels. - EPERM from a signal-0 probe means the holder is alive. Validation: __tests__/filelock-stale-takeover.test.ts runs two real processes with a barrier: the slow one stops right after reading the dead PID and resumes only once the other holds the lock. On main both held it, 5 runs of 5; with the fix the slow one is refused and the lock stays the holder's. The other cases cover the slow one getting in once the lock is free, a live and a dead break lock, the empty-lock window, EPERM (mocked, and for real on Windows with the System process), and no leftover files. Fixes colbymchenry#2979 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry#3058) Issue colbymchenry#2980: opening an index whose schema is behind read its version, then ran each pending migration in a deferred transaction. When several processes opened it at the same moment, those that read the version just before another one migrated applied the same migration again, and their open failed: on this Windows machine with "UNIQUE constraint failed: schema_versions.version", since the version was already recorded, and in the report with SQLITE_BUSY, which a deferred transaction that has already read gets at once when it comes to write. Each migration now runs in a transaction that takes the write lock up front (BEGIN IMMEDIATE, a new option of the adapter's transaction()), so a process that finds another one migrating waits it out on busy_timeout, and reads the version again inside it, skipping a migration already applied. The version the caller read stays a floor, as before: a database built from schema.sql records only v1 and the current version, and tests replay migrations above a version they rewound to. An open of a current index takes no lock and waits for nothing, as before. Validation: __tests__/migration-concurrent-opens.test.ts. A second process holds the write lock with version 16 recorded but uncommitted, so the open reads 15: main fails with the UNIQUE error after waiting 1.1 s; the fix waits, opens and leaves the migration to the other process. Three processes released at once from a stdin barrier on an index with 150k unresolved references: main fails in round 0, the fix opens all three in both rounds. On excalidraw's real index, 8 rounds of three openers: main 7 rounds with failures, the fix 0. The single process and the current-index cases pass on both. Fixes colbymchenry#2980 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eanly (colbymchenry#3060) Issue colbymchenry#2991: a host can stop reading an MCP stdio server's output while it keeps the server's stdin open and stays alive. The next response the server wrote then failed (EPIPE), and with no listener on stdout Node handed the error to the last-resort fatal handler, which exits 1 at once. That skipped the shutdown that stops the engine, releases the writer lock and waits for query workers still starting, which on Windows can crash a process that exits under them. Stdin, the host's PID and the liveness watchdog all still look healthy, so nothing else noticed. A new treatStdoutFailureAsShutdown, the twin of the stdin teardown from issue colbymchenry#799, routes a stdout 'error' (and the 'close' after it) into the owner's own shutdown, at most once, destroys stdout so later writes are dropped, and keeps its listener so a write queued before the failure cannot escape during the shutdown. Backpressure is left alone. It is wired into the direct-mode transport and into the local-handshake proxy, whose shutdown closes only this session's daemon connection and stops the engines it owns. The legacy socket proxy (runProxy) has had no caller since PR #580 and is left as it is. Validation: __tests__/mcp-stdout-failure.test.ts runs a real direct-mode serve --mcp, and the local proxy with an in-process engine and with a stand-in shared daemon. After the test destroys its end of stdout and sends one more request, main exits 1 in all three, the server with "Uncaught exception: Error: EPIPE"; with the fix all three exit 0, the server's writer.pid is released, the proxy's engine stop runs, and the daemon closes only the proxy's connection and still answers its other client. __tests__/stdout-teardown.test.ts covers the guard itself. proxy-shutdown, stdin-teardown and mcp-initialize pass. Fixes colbymchenry#2991 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext script
A self-closing `<script ... />`, such as Astro's
`<script is:inline set:html={code} />` or a third-party
`<script src="/analytics.js" />` in Svelte's `<svelte:head>`, matched the
extractors' script regex as an opening tag, so the match ran on to the next
`</script>`. The real script that followed was parsed with the markup in
between, failed with "parse produced no symbols", and contributed nothing.
The covered-range regexes had the same shape, so the template scans also
skipped the markup between the two tags, and a self-closing `<style />` hid
the markup after it.
The opening-tag pattern now matches a `/>` ending on its own as an empty
tag: in the Astro extractor's script blocks and covered ranges, the Svelte
extractor's script blocks and both of its covered-range scans, and the
viewer's highlight regions.
With a fix of ours: a self-closing tag has no body, so the covered-range
scans skip it, and a call in its attributes, like
`set:html={serializeJsonLd(schema)}`, is read as on main. Matched as a
range, such a tag hid those calls even where no script followed it.
Validation on main bc9291a (Windows 11):
- Red/green, 6 tests: 4 from the PRs, plus 2 of ours for an attribute call
on a self-closing script in Astro and in Svelte. With main's
astro-extractor.ts, svelte-extractor.ts and syntax-tokens.ts all 6 fail;
with the PRs' code as written the 2 of ours fail; with this commit all 6
pass.
- Svelte 5.56.10's own compiler reads the Svelte fixture as an empty head
script plus an instance script, as the extractor now does.
- A/B against main on 7 Astro repos, with flask and express as controls.
starlight, diploi.com and both controls are byte-identical, and
devfest-website keeps every edge. Gains: the genkit docsite 2 import
links, nodejsdesignpatterns.com 2 nodes and 2 import links, a portfolio
25 component references, and wm-project-astro-components 3
`serializeJsonLd` calls from JSON-LD scripts plus 1 call in a script main
swallowed. That repo also drops 31 false calls, CSS like
`border-radius: var(...)` that main read as calls after parsing a
swallowed `<style>` block as TypeScript. No other edge changes.
- Index time unchanged: median of 3 interleaved runs on
wm-project-astro-components, 4.67 s on main and 4.52 s with the fix.
Vue's extractor has the same pattern and the same bug; a follow-up shares
one pattern across the three extractors.
Original PRs colbymchenry#2995 and colbymchenry#2997 by @maxmilian.
Refs colbymchenry#2989
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…of nothing (colbymchenry#3069) Full-text search dropped the FTS5 syntax characters ( ) ^ " ' * : from a query before quoting each word, so the words on either side of one were glued together: leaf(entry) became the single prefix term "leafentry"*, which no symbol has, and the LIKE and fuzzy fallbacks don't recover it. codegraph query, the codegraph_search tool and searchNodes() all came back empty for a pasted call like render(props). The index's unicode61 tokenizer splits stored text at anything but letters and digits (a signature's leaf(entry) is stored as leaf and entry), so the query now splits there too: each of those characters becomes a space, the treatment :: already gets (issue #173). Validation: new search-special-characters.test.ts (8 tests) fails 8 of 8 on main and passes with the fix; fts5-fallback, search-field-filters, search-query-parser, cli-query-command and symbol-lookup pass. A query A/B on expressjs/express 0dee2c52, both arms over one index: 12 control queries without such a character between words are byte-identical; 10 pasted calls changed, 7 of them from no results to results led by the function they name (res.send(body), res.json(obj), View.prototype.lookup(name)). Reimplements PR colbymchenry#2987 by @kjgggggg. Fixes colbymchenry#2971 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…params, not a crash (colbymchenry#3071) A tools/call request passes its arguments as an object. MCPSession took params.arguments || {} without checking that, so a string or number went straight to the tool handler, where the session-view check ('_cgExploreSession' in args) threw. The call then came back as a tool result flagged isError: "This is an internal codegraph error, retry the call once; if it persists, continue without codegraph". That reports a client's malformed request as codegraph's own malfunction, with advice to stop using it. An array went through as if it were the arguments. The stdio proxy, serving in process while the daemon is unavailable, had the same unchecked read. Both now read the arguments through one helper, toolCallArguments(), and answer anything but an object with a JSON-RPC invalid-params error (-32602, "Tool arguments must be an object"), as a call with no tool name already is; no tool runs, and the proxy starts no engine for it. Missing, null or another empty value still means no arguments. Validation: new mcp-tool-call-arguments.test.ts covers the session (the direct stdio server and every daemon client) and the real proxy entry point in a child process with the daemon unavailable: red on main (2 of 3 failed; the well-formed control passed), green with the fix. explore-session-state and proxy-shutdown pass. Driving serve --mcp over stdio: on main, codegraph_explore, codegraph_node and codegraph_search with a string argument returned isError "internal codegraph error" and codegraph_status accepted [1], 42 and true; with the fix those six get -32602, while "", null, {} and absent arguments run as before. Reimplements PR colbymchenry#2986 by @kjgggggg. Fixes colbymchenry#2981 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e name (colbymchenry#3080) Liquid allows hyphens in variable names (`{% assign form-field-label = … %}`, `{% capture icon-cart %}`), but the extractor read assign and capture names as \w+. PR colbymchenry#2814, which added capture, made such a name get no node rather than a cut-short one, so hyphenated variables were missing from search and codegraph_explore. Both declarations now read a name as a word character followed by word characters and hyphens. A capture's bare name is matched lazily up to whitespace or the tag's end, so in `{% capture title-%}` the hyphen still belongs to the -%} whitespace control and the name is `title`, while `{% capture title-text-%}` reads `title-text`. A name that goes on with a dot (`ghost.name`, `ghost-name.x`) still gets no node, in both tags. Liquid isn't extracted by the native kernel, so there is no kernel side. Validation: extraction.test.ts gains 5 capture forms (bare, tight -%}, trimmed, quoted, a {% liquid %} line) and an assign test (braced, trimmed with no spaces, a {% liquid %} line); they fail 6 of 6 with main's extractor and pass with the fix, and the cut-short guard now covers dotted names in both tags. A/B, each arm built from its own checkout, on PR colbymchenry#2814's theme set (Shopify/dawn 05222537, Shopify/horizon dfad90d8, Shopify/skeleton-theme a7a655e7, ChumsInc/chums-impulse-dev 4fd6f08e, agencecinq/cinquante-et-un ecb0035a, noclocks/shopify-devstore-theme e1747695, panoply/syncify 98d75fd0, Shopify/theme-tools bb33f49c) and controls expressjs/express 0dee2c52 and psf/requests 3c7baaa0: all ten byte-identical, as none of them uses a hyphenated name. On three themes that do (Zoront22/LOS 58dc2dfc, lucidnz/bootstrapify-1 b36ab7ff, ibarake/bittergracedc e36fa5db) the fix adds 4, 25 and 39 variable nodes, each with its one contains edge: exactly the 68 hyphenated declarations a separate scan finds, each checked to sit on a capture or assign tag naming it. Nothing is removed or retargeted, no unresolved ref changes, and file rows differ only in their node counts. Index time, median of six alternating runs: horizon 3.60 vs 3.64 s, bittergracedc 28.39 vs 28.24 s. Reimplements the part of PR colbymchenry#2504 by @Puma7 that PR colbymchenry#2814 left out. Refs colbymchenry#2498 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y#3081) A new contributor had nothing to start from: the README covers using CodeGraph, and the build and test rules live in AGENTS.md, written for agents. CONTRIBUTING.md now covers building from source, testing a change, preparing a pull request and what happens to it. docs/README.md indexes the guides and design notes, and the README links to both. Every relative link and heading anchor was checked against main. Reimplements PR 1931 by @mixxer. Also folds in the fork-and-upstream setup, `npm run cli`, `npm run test:watch` and the version rule from PR 306 by @marcelovani, and the commit format, the single-test command, the bug-report list and the `/add-lang` pointer from PR 665 by @ymstar. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… to the fork's migration numbers and kernel-only parser
bompus
added
author:anthropic
Owner
Author
|
@coderabbitai full review |
bompus
changed the title
|
Bompus AI Review could not run on f700428: no reviewer seat finished the review (Muse Wonder Full: usage limit reached; SWE-2 Max: failed (exit 3)). Nothing was reviewed. Pushing a new commit or re-running the review tries the seats again. |
bompus
added
author:anthropic
![bompus-ai-review[bot]](https://avatars.githubusercontent.com/u/253635?s=60&v=4){kind=small}
…as required Review of the upstream lock change: a link planted at the private file path was written through. Clear the path, then create with 'wx'. CONTRIBUTING no longer calls the kernel optional.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Twelve upstream fixes (colbymchenry/codegraph, after 60b0d7a) now apply to the fork unchanged: Go exports, overloaded names in
codegraph_node, lock and migration races, MCP shutdown and argument errors,.gitignorereading,<script />in Astro and Svelte, Liquid and search fixes, dependency security updates and the contributing guide.This is batch 1 of bringing the fork up to upstream
cd04b21a. Upstream is 40 commits past the last port point: 12 apply cleanly (this PR), 28 conflict with the fork's Rust resolution kernel and are ported in later batches. The README's "through 60b0d7a" statement moves in the final batch, not here.Applied as upstream wrote them (author kept): colbymchenry#3030, colbymchenry#3043, colbymchenry#3044, colbymchenry#3050, colbymchenry#3056, colbymchenry#3058, colbymchenry#3060, 6177692 (Astro/Svelte
<script />), colbymchenry#3069, colbymchenry#3071, colbymchenry#3080, colbymchenry#3081. One commit adapts two of upstream's new tests to the fork:migration-concurrent-opensassumed upstream's migration 16 builds the module-member index. In the fork that migration is number 22 (the fork's own migrations sit between), so the test now rewinds to version 21 and checks version 22. It failed before the change withindex: truewhere it expectedfalse.mcp-stdout-failureimported the removed WebAssembly runtime flags; the fork has one parser, so the import and its use are gone.Checks: full suite on this branch before the test commit: 944 of 946 files passed, 11,269 tests; the two failing files were the two above, which now pass (7 tests).
tscis clean andnpm ciwas run in this worktree, since two dependency versions change.README rows checked: the fork-versus-upstream pointer (
60b0d7a9) is unchanged on purpose; no number the README quotes moves.