scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-04 02:31:36 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7
2136 commits 115 branches 1023 tags 246 MiB
Commit graph

3 commits

Author SHA1 Message Date
Gergő Magyar
ebbcd5b0b4
fix(mcp): name the indexed ref on hot read tool staleness (#3291) (#3293) 2026-09-16 07:29:04 +00:00
Ankit Verma
8bd71c8335
feat(staleness): report diverged and unknown index state instead of fresh (#3257) 
* feat(staleness): report diverged and unknown index state instead of fresh

A staleness check collapsed every git failure into { isStale: false,
commitsBehind: 0 }. On a branch-pinned serve clone, a failed re-index
leaves the recorded commit orphaned by the --depth 1 fetch; once the
reflog expires and gc prunes it, rev-list fails and the index silently
reads as fresh while still behind.

checkStaleness / checkStalenessAsync now return an additive status:
current, behind, diverged (rev-list failed but HEAD resolved and differs
from lastCommit) or unknown (HEAD unresolvable, no lastCommit, timeout).
isStale and commitsBehind keep their values in every case.

One payload builder (core/staleness-status.ts) feeds MCP list_repos, the
hot read tools and /api/repos + /api/repo. diverged carries a hint and no
invented count; unknown appears on listings only. The helpers live in a
pure module so existing vi.mock stubs of git-staleness stay valid.

gitnexus group status no longer prints "-1 commits behind".

Fixes #3256

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

* Address PR review feedback (#3257)

- Document the `current` arm `fromHead` can return after a failed `rev-list`.
  The `staleness-status.ts` status list, the `fromHead` docstring and the
  `stalenessForTool` comment each named only `diverged`/`unknown`, so all three
  described a contract the helpers do not have.
- Document both `unknown` rows on the group status type: no recorded commit
  (`indexStale: true`, `commitsBehind: -1`) and a git probe that could not
  answer (`indexStale: false`, `commitsBehind: 0`), which do not agree on
  either field.
- Update both shipped `gitnexus-guide` copies to the new wire shape
  (`{ status, commitsBehind?, hint? }`), add a `diverged` example carrying no
  count, and state `unknown` is reported only by the `list_repos` listing.
  Presence now means "not `current`", not "behind N". Pinned in
  shipped-skills-sync.
- Lock the timed-out `rev-list` short-circuit with staleness-timeout.test.ts:
  it asserts `unknown` from exactly one git spawn, so removing the `killed`
  guard (which would probe HEAD again and double the #3232 bound) now fails.

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

* Claim only the uncountable gap in the diverged hint (#3257)

`fromHead` reaches the `diverged` branch on any non-timeout `rev-list` failure
where `rev-parse HEAD` resolves to a different SHA. It compares the two SHAs and
runs no reachability check, so a transient object-read failure lands there while
`lastCommit` is still an ancestor of HEAD — and the hint told the operator the
commit was gone from history. `staleness-status.ts` already documents `diverged`
as only "provably not at HEAD; the count is unknown", so the string was also the
one place contradicting its own contract.

The hint now states what the check established and hedges the usual cause:
"Index is not at HEAD and the commit gap could not be counted — the recorded
commit may no longer be in this clone's history."

Updated with it: the `staleness.test.ts` diverged assertion, and the `diverged`
example plus its lead-in sentence in both shipped `gitnexus-guide` copies (still
byte-identical). `mcp/resources.ts` needs no change — it interpolates
`staleness.hint`, holding no copy of the text.

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

* test(staleness): cover the remaining #3256 review test gaps

The tri-review's lower-priority gaps, each now locked:

- staleness-fallback.test.ts: after a failed (not timed-out) rev-list,
  both helpers report `current` when HEAD alone still resolves to the
  indexed commit, `diverged` when it resolves elsewhere, and `unknown`
  when it cannot be read, each from exactly rev-list + rev-parse. This
  complements staleness-timeout.test.ts: a timeout spawns once, any other
  failure probes HEAD.
- list_repos carries the listing-only `unknown` and a count-free
  `diverged`; `current` carries nothing.
- group status: a resolvable repo with no recorded commit composes to
  indexStale: true, commitsBehind: -1, status: unknown and renders as
  "STALE (? commits behind)", through the real groupStatus path rather
  than a hand-built row.

Each test was checked against a mutation of the code it guards (the
fromHead current arm, the group no-commit literal, list_repos
includeUnknown); every mutation fails its test.

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

---------

Co-authored-by: tech-admin3 <tech-admin@kodnest.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
 2026-09-11 08:07:37 +01:00
Gergő Magyar
df0110b06f
fix: index staleness — false-stale status after analyze (#2668) + inline staleness in query/context/impact/cypher tools (#2655) (#2683) 
* fix(analyzer): case-stabilize runner-identity path fields so status isn't false-stale (#2668)

`gitnexus status` reported a freshly-analyzed, untouched repo as stale on
Windows (econia/aptos-core, 1.6.10-aptos.0). `status`'s up-to-date check gates
on `runnerIdentityIsCurrent`, which deep-compares the stamped runner identity
against a freshly recomputed one. That comparison includes `build.rootPath`,
`dependencyRuntime.manifestPath`/`lockfilePath`, and `runtime.executablePath`
(only `invokedArtifact` is stripped), and `identityCacheKey` hashes
packageRoot/buildRoot — all derived from paths that flow through
`realpathSync.native`, which canonicalizes 8.3 names and symlinks but does NOT
normalize the Windows drive-letter case. When `analyze` and `status` are
launched under different drive-letter casing (`c:\...` vs `C:\...`, plausible
across CLI shim / npx / server-worker entries), the two identities differ by
that one byte and `status` reports stale.

Fix: `normalizeAnalyzerRootPath(p, platform)` uppercases the Windows drive
letter (POSIX no-op, platform-explicit for testability; preserves a `\\?\`
extended-length prefix), applied at the single upstream source —
`resolveBuildRoot`'s returned `{packageRoot, buildRoot}` — so every derived
identity path field and the cache key inherit a case-stable root, plus at
`runtime.executablePath` (process.execPath is the same compared class). The
`runnerIdentityIsCurrent` gate is kept intact: a genuine analyzer change still
differs in `build.digest`/`dependencyRuntime`, and analyze still rebuilds on
real mismatch.

Note: the drive-letter divergence was not reproduced on a Windows host (none
available); the mechanical chain is verified in source and the fix is a correct
defensive normalization that is a no-op on POSIX. If a `status --json` identity
field-diff later shows `build.digest`/`dependencyRuntime`/`cliVersion`
diverging instead, that indicates a genuinely different install (where "stale"
is correct), not this bug.

Migration: on Windows, an existing index stamped under the old (non-normalized)
casing mismatches the normalized recompute once, triggering a single forced
full re-analyze on first upgrade (and a one-time identity-cache recompute).
One-time, Windows-only, POSIX no-op.

Tests: pure `normalizeAnalyzerRootPath` unit tests (drive-letter uppercase,
idempotence, drive-only scope, `\\?\` extended-length prefix, POSIX no-op).

* feat(mcp): surface index staleness in query/context/impact/cypher tool responses (#2655)

`checkStalenessAsync` already computes how many commits an index is behind the
checkout's HEAD, and `list_repos` returns it as `staleness: {commitsBehind,
hint}`. But the four hot read tools an agent actually calls in a session —
`query`, `context`, `impact`, `cypher` — never surfaced it: `resolveRepo` only
runs `maybeWarnSiblingDrift` (stderr, sibling-clone drift only), so a direct
tool call gave zero indication the index might be behind HEAD.

Thread the existing signal into those four tools at the single `callTool`
dispatch chokepoint (after the one `resolveRepo`), reusing the `list_repos`
`{commitsBehind, hint}` shape:

- `stalenessForTool` computes `checkStalenessAsync` behind an in-flight-promise
  cache (5s TTL) keyed by lbugPath, so N concurrent tool calls share one
  `git rev-list` and flat/branch handles (same repoPath, different lastCommit)
  don't collide. The cache entry is evicted with the repo's other per-index
  state when the repo leaves the registry.
- `withToolStaleness` skips the `git` spawn entirely for results that can't
  carry the field (via `canCarryStaleness`), so error-returning calls pay
  nothing.
- `attachToolStaleness` adds a `staleness` field to an object result only when
  the index is behind HEAD. It NEVER changes an existing result's shape:
  raw-array results (non-tabular cypher rows) are returned untouched, because
  the CLI's `--limit` and other consumers branch on `Array.isArray`; error
  envelopes and already-annotated results are left as-is. Non-blocking:
  `checkStalenessAsync` swallows git failures to `{isStale:false}`, so a git
  error just omits the field — it never fails the tool.

Deliberately out of scope: `@group`-targeted calls forward to
`callToolAtGroupRepo` before the chokepoint (multi-repo, single-commit
staleness is ill-defined); the legacy `search`/`explore` aliases; and
`list_repos` / the `context` resource, which already carry the signal.

Tests: `attachToolStaleness` branch matrix (stale object -> field; fresh ->
unchanged; raw array -> unchanged; error envelope -> unchanged; idempotent;
non-object -> unchanged; null-safe) and a flat-vs-branch cache-key regression
test that fails when the cache is keyed by repoPath.

* test(mcp): cover staleness tool-signal edge cases + harden the freshness boundary (#2655)

Addresses the coverage gaps the review flagged on the #2655 staleness signal,
plus one defensive guard so a failing freshness check can never fail a tool.

Production (defense-in-depth, no behavior change on the happy path):
- withToolStaleness now awaits stalenessForTool with a `.catch(() => undefined)`
  so a rejection degrades to no-staleness instead of failing query/cypher/
  context/impact.
- stalenessForTool wraps the check in `Promise.resolve(...).catch(...)` that
  evicts the cache entry on rejection — a transient failure isn't served as a
  permanently-rejecting promise for the rest of the TTL window, and the
  `Promise.resolve` wrap makes the boundary robust to a non-thenable return
  (a no-op for the real async checkStalenessAsync). A resolving promise is
  never evicted, so happy-path dedup is unchanged.

Tests (gitnexus/test/unit/calltool-dispatch.test.ts):
- F1: a rejecting checkStalenessAsync leaves the tool payload intact with no
  staleness field, and a later call recovers (proves the entry isn't poisoned).
  Written first and confirmed to fail without the guard.
- F2: staleness attaches on query/context/impact object results and on cypher's
  tabular {markdown,row_count}; a raw-array cypher result keeps its shape.
- F3: drift guard — exactly query/cypher/context/impact route through
  stalenessForTool; explain/pdg_query/detect_changes/check do not.
- F4: the per-index cache dedupes within TOOL_STALENESS_TTL_MS and recomputes
  after it expires (driven via a Date.now spy, not fake timers).

Tests (gitnexus/test/unit/analyzer-identity.test.ts):
- F5: the produced identity's build.rootPath and runtime.executablePath are
  normalizer-stable, guarding that both call sites thread through
  normalizeAnalyzerRootPath (trivial on POSIX, a real regression guard on
  Windows CI). Plus a source comment noting the one-time Windows re-analyze on
  first upgrade.

* test(mcp): run #2668 guard on Windows CI, document staleness field, cover staleness edge cases

Addresses the review follow-ups on the staleness work:

- Wire test/unit/analyzer-identity.test.ts into scripts/cross-platform-tests.ts
  (PLATFORM_LOGIC). Its "identity path fields are normalizer-stable" fixpoint is
  the Windows regression guard for the #2668 drive-letter normalization, but
  normalizeAnalyzerRootPath is a POSIX no-op, so the guard was only ever running
  (trivially green) on the Ubuntu full-suite and never on the windows-latest
  matrix where it actually bites. Now it runs where it matters.

- Document the inline `staleness` field on query/context/impact/cypher responses
  in the gitnexus-guide skill (both the .claude source and the shipped
  gitnexus-claude-plugin mirror, kept in sync).

- Add three staleness tests that pin behavior the prior tests only implied:
  * @group-routed calls never get the signal (forwarded before the wrapping
    switch) — locks the intentional skip so it can't silently flip.
  * one in-flight freshness check is shared across truly concurrent calls
    (two dispatched before checkStalenessAsync settles → a single spawn), not
    just sequential reuse of an already-resolved value.
  * a late rejection from a superseded cache entry does not evict the newer
    entry that replaced it after the TTL rolled over (the `=== entry`
    object-identity guard).

The defensive stack in stalenessForTool/withToolStaleness (Promise.resolve
wrap + guarded evict + outer catch) is retained deliberately: the wrap is
load-bearing for the tests (a sibling describe's vi.resetAllMocks() makes the
mock return undefined), and the guarded evict closes the superseded-entry edge
now covered above.

* fix(test): split the #2668 normalization guard into a portable cross-platform file

Registering analyzer-identity.test.ts on the Windows/macOS matrix (previous
commit) surfaced four pre-existing failures in that file on macOS 3/3 and
windows 3/3. They are not new breakage: those fixture tests compare identity
fields against the RAW temp-dir path while the identity resolves through
realpathSync.native, so on macOS `/var/folders/...` is received as
`/private/var/folders/...`. The file was simply never portable — it had only
ever run in the Ubuntu full-suite. Reproduced locally by pointing TMPDIR at a
symlink: the same four tests fail, and pass again without it.

Move only the portable assertions — the pure `normalizeAnalyzerRootPath` cases
(explicit `platform` argument) and the identity fixpoint guard (which compares
each field against ITSELF normalized, never against the fixture path) — into
test/unit/analyzer-identity-path-normalization.test.ts, and register that file
on the matrix instead. The #2668 Windows regression guard still runs where it
actually bites, without dragging four symlink-sensitive tests onto runners they
were never written for.

Verified: the new file passes with TMPDIR behind a symlink (the macOS
condition); the heavy file is back to Ubuntu-only.

* fix(test): keep the cross-platform #2668 file fixture-free so Windows stays green

The split file still carried the fixture-based fixpoint guard, which fails on
windows-latest:

  Invoked analyzer artifact is absent from the validated build:
    D:\a\...\node_modules\vitest\dist\workers\forks.js

Cause is a pre-existing cross-drive defect in this module's `isInside()`, not the
#2668 change. The GH Windows runner keeps the repo on D: and temp fixtures on C:.
`path.win32.relative('C:\\...fixture', 'D:\\...forks.js')` cannot express a
relative path across drives, so it returns the absolute target — which does not
start with '..', so `isInside()` reports true. `resolveInvokedArtifact` therefore
treats the vitest fork worker as the invoked artifact, it is absent from the
fixture's validated build, and identity resolution throws. (Verified directly:
`isInside` returns true cross-drive and false for the same-drive control.)

Keep the cross-platform file strictly pure — only `normalizeAnalyzerRootPath`
assertions with an explicit `platform` argument, no fixture and no filesystem —
so it is green on every runner while still exercising the transform on real
Windows. The fixture-based threading guard moves back to analyzer-identity.test.ts
(Ubuntu-only), where the rest of that file's fixture tests already live, with a
comment recording why it cannot be on the matrix.

The underlying `isInside()` cross-drive bug is left untouched here (out of scope
for this PR) but is worth its own fix: it also guards the trusted cache directory
and the identity-cache path-escape check in validateIdentityCache, where a false
"inside" verdict weakens validation on multi-drive Windows setups.

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
 2026-07-25 07:21:44 +01:00