Skip to content

sync returns silent all-zeros on lock-acquire failure → prints "Already up to date" under contention (index surfaces the same failure as an error); if the silent-skip is intentional the output is still misleading #1361

Description

@inth3shadows

Edit (2026-07-18): softened framing. The certain, code-proven claim is narrow — on a lock-acquire failure sync returns silent all-zeros and the CLI prints Already up to date, so the output is misleading under contention. The broader claim that the silent-skip itself is a defect is an inference, not confirmed: "skip if another process is busy, retry later" is a defensible (if undocumented) design. Title and the "Impact" section below are reworded to reflect that; the "sync-as-recovery is defeated" line is demoted from assertion to conditional. Severity (real bug vs. cosmetic-under-contention) is the maintainer's call — the behavior is not in question, only its interpretation.

Version: 1.4.1

Summary

codegraph sync's file-lock-acquisition failure path returns a zero-valued SyncResult with no error field, so the CLI prints Already up to date and the orphan-ref sweep never runs. Under contention (a lock-holder present, e.g. a long-running codegraph serve --mcp), sync reports Already up to date even though it did nothing — the same failure that index surfaces as a visible error is swallowed silently. The output being misleading under contention is the certain claim. Whether the silent-skip itself is wrong (vs. an intentional "retry later" design) is a separate, softer question — see Impact.

This is distinct from #1325: that issue is codegraph index surfacing the lock failure as a visible database file is in use error. sync swallows the same condition.

Code path

  • src/index.ts:733-735sync():
    try { this.fileLock.acquire(); }
    catch { return { filesChecked: 0, filesAdded: 0, filesModified: 0, filesRemoved: 0, nodesUpdated: 0, durationMs: 0 }; }
    
    Returns silently, before the orphan sweep (~src/index.ts:887). The zero result makes src/bin/codegraph.ts:883 print Already up to date.
  • Contrast — src/index.ts:439 and :715 (index()): the same lock failure returns { success: false, errors: [{ message: "Could not acquire file lock - another process may be indexing" }] } — a visible error (codegraph index fails with "database file is in use" when MCP server is still running #1325).
  • FileLock.acquire() (src/utils.ts:247-252) throws database is locked by another process (PID …) when the lock is fresh (lockAge < STALE_TIMEOUT_MS) and the holder is alive — the throw that sync catches and hides.

Impact

Certain: under contention the CLI prints Already up to date when sync in fact did nothing — a false signal to the user.

Inferred (not confirmed): if a wedged index has pending orphan refs, this silent early-return means sync can't self-heal it while a lock-holder is present, so the sync-as-recovery path (#1191) may not fire when a serve --mcp daemon holds the lock. This assumes silent-skip is unintended; "skip now, the holder or a later sync will heal it" is a defensible design. If it is intended, the misleading Already up to date output (fix C below) still stands on its own.

Reproduction status (honest)

The code path is unambiguous, but a minimal turnkey repro is fiddly: acquire() steals stale locks (utils.ts:239-255), so a lone manual sync usually succeeds; the silent no-op needs a concurrent holder keeping the lock fresh. I did not reproduce the silent no-op at runtime — the stale-lock stealing plus a background watcher's auto-heal made isolation hard. Flagging as code-verified, not runtime-reproduced for the silent-skip; the misleading-output consequence follows directly from the zero result.

Suggested fixes (tradeoffs)

  • A — surface the failure like index() does: return/print Could not acquire file lock — another process is indexing; retry instead of silent zeros. Tradeoff: noisier under contention, but honest (no false Already up to date).
  • B — retry-with-backoff before giving up (sync is idempotent). Tradeoff: adds latency under contention; best UX; heals once the holder releases.
  • C — cheapest / lowest-risk: simply don't print Already up to date when the lock was not acquired; print skipped: index busy. One-line honesty fix that is correct regardless of whether silent-skip is intentional.

Relatedly, if sync retried or waited (B), it would also paper over the misleading-output issue I filed separately (#1360) for the successful sweep case.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions