Skip to content

fix(sync): recover index freshness after lock contention (#1959) - #2043

Merged
colbymchenry merged 6 commits into
mainfrom
fix/1959-lock-freshness-exclude
Sep 27, 2026
Merged

colbymchenry merged 6 commits into
mainfrom
fix/1959-lock-freshness-exclude

Conversation

@colbymchenry

Copy link
Copy Markdown
Owner

Problem

Long-running writers lost their locks after two minutes, while exhausted watcher retries left MCP serving a frozen index. The contributor freshness guard missed paths containing spaces or Unicode and silently skipped paths beyond its limit.

Fix

Build on PRs #1978 and #1980, preserving authorship. Keep live locks protected, throttle watcher recovery, and retain degraded state until full reconciliation succeeds. Carry structured contributing paths and indexed hashes through query workers; refuse changed, deleted, or unverified files with success-shaped guidance. Bound freshness measurements and expose structured status data. Add regression coverage for both Git exclusion sources; their implementation was already fixed on main.

Validation

  • ../../repro/1959/repro.sh "$PWD": exit 1 before, exit 0 after rebuilding.
  • npx tsc && npm run copy-assets
  • npx tsc --noEmit
  • npx vitest run __tests__/mcp*.test.ts __tests__/daemon*.test.ts __tests__/writer-lock.test.ts __tests__/concurrent-locking.test.ts __tests__/query-pool.test.ts __tests__/watch-policy.test.ts __tests__/cli-unlock.test.ts __tests__/integration/mcp-input-limits.test.ts --maxWorkers=3 --minWorkers=1: 33 suites passed.
  • Targeted security, watcher, extraction, freshness, and explore budget/session suites passed on macOS.
  • Linux Docker validation passed, including final freshness deadline checks.
  • Windows regressions passed; one unrelated writer-lock test assumes PID 1 is alive and fails identically with origin/main code and test.

Fixes #1959

🤖 Generated with Claude Code

danusha2345 and others added 6 commits September 27, 2026 09:12
… degradation (#1959)

Explore already refuses to render source from a file that changed while
auto-sync was off. search, callers, callees and impact answer from the same
frozen graph but kept replying, naming the changed file as if its edges were
current. They now take the same refusal: the indexed files an answer names
as locations are hashed against the index, and a drifted one is named
instead of the answer. codegraph_node keeps its own drift gate, which serves
the changed file's current bytes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep a live writer's lock regardless of its age.
Re-arm lock-degraded watchers with throttling and retain stale state until full reconciliation succeeds.
Validate contributing files through structured paths and hashes, refusing incomplete checks without tool errors.
Report bounded freshness measurements and cover Git fallback exclusions.

Co-authored-by: danusha2345 <danusha2345@users.noreply.github.com>
Co-authored-by: danusha2345 <ewidusoc498@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@colbymchenry
colbymchenry merged commit 2268a09 into main Sep 27, 2026
danusha2345 pushed a commit to danusha2345/codegraph that referenced this pull request Sep 27, 2026
…mchenry#1910)

colbymchenry#2043's answer-freshness check hashes each contributing file whole and
compares it with the stored contentHash. For a file over the index's size
limit the stored hash is the size stamp, so an unchanged oversize file
would read as stale. Compare it by its stamp, without reading it, like
the viewer and the MCP drift gate do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bompus added a commit to bompus/codegraph that referenced this pull request Sep 28, 2026
The fork keeps its kernel-only extraction and resolution, its synthesis
refresh on incremental sync, and its migrations v10-16. Upstream's TS
resolver files (name-matcher, js-builtins, cpp-constructor,
cpp-macro-visibility, synthesis-stage) and the Scala wasm grammar stay
deleted. The kernel walkers carry the ported upstream kernel changes:
NodeIdAllocator column suffixes, Python body docstrings, Scala objects as
modules, the C++ constructor and macro-name helpers, and the TS/JS
expression-receiver and curried-wrapper cases.

Taken from upstream:
- read-only CodeGraph.open/openSync (colbymchenry#1963), the shared project lease for
  MCP engines, and the query-pool and daemon fixes
- sync throws LockUnavailableError when the file lock is held (colbymchenry#2014);
  open() destroys the instance if its sync fails
- watcher lock-contention re-arm, explore miss diagnostics, the Scala
  grammar bump (colbymchenry#1823), and the vitest run-TMPDIR setup (colbymchenry#1879)
- pending-reference recovery counts on SyncResult and in `codegraph sync`
  output (colbymchenry#1360), ported onto the fork's orphan sweep
- `this.#field.method()` call edges (colbymchenry#1987), ported into the kernel's
  field-type matcher and method-call receiver pattern

Fork-side adjustments:
- edges keep the fork's `ORDER BY kind, id`. Upstream's
  `kind, target, line, col` tie-break (and a position-first variant)
  dropped CG-36's answer cluster and colbymchenry#1711's named gaps; the db-perf
  ordering test now checks kind precedence and insertion-order ties
- a gap marker that can't afford its full name list names fewer symbols
  plus `+N more` instead of going bare (colbymchenry#2057 budgeting kept)
- the lease catch-up sync defers synthesis, as the fork's engine did
- enableWatcherAfterHandover nudges the existing lease instead of
  acquiring a second one
- dropped idx_edges_synthesis_site from the bulk-load index list (the
  fork schema has no such index)
- Python and Go function refs stay bare-named in both the kernel and
  the generic extractor until colbymchenry#1820 is ported to both
- tests: the colbymchenry#1963 daemon test waits for attach before connecting; the
  Windows zip-archive cases run only where `zip` exists; the watcher and
  generated-flag tests keep their fork fixtures
- golden dumps re-baselined; torture-multilang now covers 42 files

Port follow-ups (tests skipped with a "kernel port pending" note):
- colbymchenry#1820 receiver-path function refs (Python, Go)
- colbymchenry#1838/colbymchenry#1839/colbymchenry#2035 C++ macro names and constructor calls
- colbymchenry#1932/colbymchenry#2034 alias-binding object member boundaries
- Scala companion resolution; colbymchenry#1857 Go bare calls; colbymchenry#1986 ABI evidence;
  expression receivers; HAR excluded imports; value+interface pairs
- colbymchenry#2033 synthesis_inputs convergence model (7 tests)
- colbymchenry#2043 stale-refusal banner policy needs a decision
bompus added a commit to bompus/codegraph that referenced this pull request Sep 28, 2026
…answer (#244)

Upstream colbymchenry#2043 made search, callers, callees, impact and explore withhold
the whole answer when live watching had stopped and any contributing file
changed after its last sync. Rendered source is already re-read from disk
and drift-guarded (colbymchenry#1474), so only graph facts from those files can lag.
The answer is now served with a notice naming the changed or unverified
files (structuredContent.freshness is kept). The DISABLED and RECOVERING
banners, the status section and the server instructions say what may lag
instead of telling the agent to read files itself.
danusha2345 pushed a commit to danusha2345/codegraph that referenced this pull request Sep 28, 2026
…mchenry#1910)

colbymchenry#2043's answer-freshness check hashes each contributing file whole and
compares it with the stored contentHash. For a file over the index's size
limit the stored hash is the size stamp, so an unchanged oversize file
would read as stale. Compare it by its stamp, without reading it, like
the viewer and the MCP drift gate do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry added a commit that referenced this pull request Sep 29, 2026
….ts is not TypeScript (#1910) (#2082)

* fix(extraction): an MPEG-TS video named .ts is not TypeScript (#1910)

`.ts` mapped to TypeScript by extension alone, so MPEG transport-stream
fixtures (`testdata/*.ts`) were parsed by tree-sitter: ~28 s of CPU for a
900 KB clip, minutes for a directory of them, for zero symbols.

Recognise the stream from the head of the file — the 0x47 sync byte at
offsets 0, 188, 376 and 564 plus a NUL byte, which every stream carries in
its first packets and UTF-8 source never does — and drop it at discovery:
not indexed, not parsed, not counted, not tallied as an unsupported
language. The check reads under 1 KB and only for `.ts` files; the batch
reader sniffs the bytes it already read, and the single-file path (sync,
watcher) does the same head read, so a clip handed in by name is skipped
too. The skip happens before parse dispatch, so no kernel mirror is needed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(extraction): a video .ts is never pending on the git path or a scoped sync

Review found two places that decide "is this a source file" without the
MPEG-TS check the scan applies: the git fast path's candidate loop in
getChangedFiles and the scoped-sync path filter (watcher events). An
untracked clip in a git repo was reported as `added`, skipped by sync
without a record, and reported as `added` again on every status; a tracked
.ts that became a clip stayed `modified` with its stale nodes. Both now
treat the clip as non-source: removed when tracked, ignored otherwise.

Regression tests fail without this change and pass with it, on both arms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(extraction): require a binary head before calling a .ts file video (#1910)

Review on #1915: the sniff took four aligned 0x47 sync bytes plus any NUL
as proof of a transport stream, so TypeScript with a `G` at four 188-byte
strides and one raw NUL in a comment was dropped unindexed. The sniff now
needs the sync byte on sixteen consecutive packets (3 KB of head) and at
least 1/64 of the head to be control bytes. Compressed payload carries
8-20% of them (checked on ffmpeg-made H.264/AAC, MPEG-2 and MP2 streams);
source text carries none. A shorter clip is cheap to parse anyway.

Also drops the benchmark figures from the changelog entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(extraction): a file over the size limit is never read (#1910)

The size gate ran after `readFile`, so a committed video or blob fixture was
decoded in full — 3.4 GB of RSS for a 400 MB file, `Invalid string length`
past ~512 MB — only to be stored as skipped. It was then read again by every
pass that scans files by name: the framework detectors' `readFile` and the
resolver's cached reader, three full passes on the reporter's fixtures.

Stat first, everywhere a source file is read by path: the batch reader and
`extractFile` store an oversize file with a size stamp in place of its
content, change detection hashes the same stamp (so a same-size rewrite of a
file nothing is indexed from is not a change, while crossing the limit in
either direction is), and both by-name readers return null above the limit.
The limit moves to `src/file-limits.ts` so the three sites share one number.

strace on a sparse 400 MB `.ts`: 153 603 reads before, 0 after; indexing it
next to a real file costs no measurable RSS. Stacked on #1914, which shares
the batch-reader hunk.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* refactor(extraction): one-purpose size-stamp reader, comments in operation order

`readSourceOrStamp` took a `stats` argument no caller passed and returned
stats no caller read; it now returns the text or stamp change detection
hashes. The batch reader's two comments are back in the order its code runs:
the size gate, then the MPEG-TS head check. No behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(extraction): bound the read itself, not only the stat before it (#1910)

A file can grow between the stat that gates the size limit and the read
that follows (a log, a download, a build output being written). Every
source read now goes through readBoundedSource/readBoundedSourceSync: the
open descriptor is re-checked, and the read stops one byte past the limit,
so an oversize file is still stamped rather than decoded. The extraction
batch reader, single-file extraction, framework detectors and the resolver's
file cache all use it. Ported from the maintainer's hardening of this fix
in #1919.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(extraction): bound the parse-retry reads too (#1910)

The retry passes for files whose worker crashed or timed out re-read the
source with an unbounded fsp.readFile. The file can have grown since the
first, bounded read; an oversize file is now skipped by the retry instead
of being decoded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(extraction): check an oversize file against its size stamp everywhere (#1910)

Review on #1915:

- The viewer (readFileShape, hasDriftedOnDisk, the source endpoint) and
  MCP's drift gate hashed a file's real bytes, but the index stores the
  size stamp for a file over the limit, so every unchanged 1-8 MB file read
  as "changed on disk". oversizeStamp moves to file-limits.ts with an
  indexedHashInput helper, and all four checks hash what the index stored.
  An oversize file is now compared without being read.
- git-index-currency's "keeps a committed path pending when sync cannot
  read it" injected its failure only into readFileSync, which the bounded
  reader no longer calls, so it failed. The failure now reaches openSync
  too.
- The changelog entry drops its benchmark figures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(mcp): check an oversize answer file against its size stamp (#1910)

#2043's answer-freshness check hashes each contributing file whole and
compares it with the stored contentHash. For a file over the index's size
limit the stored hash is the size stamp, so an unchanged oversize file
would read as stale. Compare it by its stamp, without reading it, like
the viewer and the MCP drift gate do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* perf(extraction): judge a .ts clip from the bytes it is read for, not at discovery (#1910)

Discovery sniffed the head of every `.ts` file on every full scan — an
open + 3 KB read + close per file, before anything asked for its content.
On a 10k-file TypeScript tree that took a warm-cache scan from ~105 ms to
~243 ms; on a slow disk or a network share it is a random read per file
per scan, the access pattern #1231 and the SMB reports (#1014, #448) are
about.

The bytes are now judged where they are read anyway, so the check costs no
I/O and runs only for a file that is new or changed: the batch reader (as
before), `indexFile`, the sync reconcile (an untracked clip is skipped, a
tracked file that became one is removed through the same removal path as
a deletion), and both `getChangedFiles` paths. The scan lists `.ts` files
by name again; a test pins that discovery opens none of them on the walk
or the git path. Scan time is back to main's (~98 ms), and a full index is
unchanged within noise.

Also collapses the four #1910 changelog bullets (#1914 and #1915 each added
both) to two, without the benchmark figures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: danusha2345 <danusha2345@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: danusha2345 <ewidusoc498@gmail.com>
@colbymchenry
colbymchenry deleted the fix/1959-lock-freshness-exclude branch October 10, 2026 06:15
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.

serve --mcp answers from a frozen index after lock contention; FileLock goes stale under a live holder; walk fallback ignores .git/info/exclude (1.6.0)

2 participants