diff --git a/.claude/skills/gitnexus/gitnexus-debugging/SKILL.md b/.claude/skills/gitnexus/gitnexus-debugging/SKILL.md index 9834f94b7..80f9c0ec5 100644 --- a/.claude/skills/gitnexus/gitnexus-debugging/SKILL.md +++ b/.claude/skills/gitnexus/gitnexus-debugging/SKILL.md @@ -16,10 +16,10 @@ description: "Use when the user is debugging a bug, tracing an error, or asking ## Workflow ``` -1. query({query: ""}) → Find related execution flows +1. query({search_query: ""}) → Find related execution flows 2. context({name: ""}) → See callers/callees/processes 3. READ gitnexus://repo/{name}/process/{name} → Trace execution flow -4. cypher({query: "MATCH path..."}) → Custom traces if needed +4. cypher({statement: "MATCH path..."}) → Custom traces if needed ``` > If "Index is stale" → run `node .gitnexus/run.cjs analyze` in terminal. @@ -51,7 +51,7 @@ description: "Use when the user is debugging a bug, tracing an error, or asking **query** — find code related to error: ``` -query({query: "payment validation error"}) +query({search_query: "payment validation error"}) → Processes: CheckoutFlow, ErrorHandling → Symbols: validatePayment, handlePaymentError, PaymentException ``` @@ -75,7 +75,7 @@ RETURN [n IN nodes(path) | n.name] AS chain ## Example: "Payment endpoint returns 500 intermittently" ``` -1. query({query: "payment error handling"}) +1. query({search_query: "payment error handling"}) → Processes: CheckoutFlow, ErrorHandling → Symbols: validatePayment, handlePaymentError diff --git a/.claude/skills/gitnexus/gitnexus-exploring/SKILL.md b/.claude/skills/gitnexus/gitnexus-exploring/SKILL.md index ccf684c28..f483c2fd6 100644 --- a/.claude/skills/gitnexus/gitnexus-exploring/SKILL.md +++ b/.claude/skills/gitnexus/gitnexus-exploring/SKILL.md @@ -18,7 +18,7 @@ description: "Use when the user asks how code works, wants to understand archite ``` 1. READ gitnexus://repos → Discover indexed repos 2. READ gitnexus://repo/{name}/context → Codebase overview, check staleness -3. query({query: ""}) → Find related execution flows +3. query({search_query: ""}) → Find related execution flows 4. context({name: ""}) → Deep dive on specific symbol 5. READ gitnexus://repo/{name}/process/{name} → Trace full execution flow ``` @@ -50,7 +50,7 @@ description: "Use when the user asks how code works, wants to understand archite **query** — find execution flows related to a concept: ``` -query({query: "payment processing"}) +query({search_query: "payment processing"}) → Processes: CheckoutFlow, RefundFlow, WebhookHandler → Symbols grouped by flow with file locations ``` @@ -68,7 +68,7 @@ context({name: "validateUser"}) ``` 1. READ gitnexus://repo/my-app/context → 918 symbols, 45 processes -2. query({query: "payment processing"}) +2. query({search_query: "payment processing"}) → CheckoutFlow: processPayment → validateCard → chargeStripe → RefundFlow: initiateRefund → calculateRefund → processRefund 3. context({name: "processPayment"}) diff --git a/.claude/skills/gitnexus/gitnexus-guide/SKILL.md b/.claude/skills/gitnexus/gitnexus-guide/SKILL.md index 2a1e76b02..7f90f4e6d 100644 --- a/.claude/skills/gitnexus/gitnexus-guide/SKILL.md +++ b/.claude/skills/gitnexus/gitnexus-guide/SKILL.md @@ -36,10 +36,12 @@ For any task involving code understanding, debugging, impact analysis, or refact | `context` | 360-degree symbol view — categorized refs, processes it participates in | | `impact` | Symbol blast radius — what breaks at depth 1/2/3 with confidence | | `detect_changes` | Git-diff impact — what do your current changes affect | -| `check` | Check graph invariants such as circular imports | | `rename` | Multi-file coordinated rename with confidence-tagged edits | | `cypher` | Raw graph queries (read `gitnexus://repo/{name}/schema` first) | -| `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | +| `explain` | Persisted taint findings — source→sink data flows (needs `analyze --pdg`) | +| `pdg_query` | Control/data dependence — what gates X (CDG) / where Y flows (REACHING_DEF); needs `analyze --pdg` | +| `check` | Check graph invariants such as circular imports | +| `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | ### Paginating `list_repos` @@ -72,6 +74,25 @@ list_repos { offset: 400 } → repos 401–437, hasMore false Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). Out-of-range or malformed `limit`/`offset` (non-integer, `limit` outside `[1, 200]`, `offset < 0`) are rejected with a clear error — `limit` above the max is rejected, not silently capped. The order is deterministic (lower-cased name, then path), so paging never skips or duplicates an entry while the registry is unchanged. +### Taint findings (`explain`) + +`explain` returns intra-procedural taint findings (`TAINTED` edges) recorded by `gitnexus analyze --pdg` — each with a sink category (command-injection, code-injection, path-traversal, sql-injection, xss), source/sink lines, and the ordered hop path with the variable carried on each hop. + +- `explain {}` — enumerate all findings for the repo (bounded by `limit`, deterministic order) +- `explain { target: "src/vuln.ts" }` — findings in a file (suffix path match accepted) +- `explain { target: "runUserCommand" }` — findings in a function (resolved like `context`; ambiguous names return ranked candidates) + +A repo indexed without `--pdg` returns a clear "no taint layer" note. Caveats: findings are intra-procedural only — cross-function, closure/callback, property/field, and implicit flows are not modeled, so the absence of a finding is **not** proof of safety. `SANITIZES` (sanitizer-kill) edges are queryable via `cypher`. + +### Control & data dependence (`pdg_query`) + +`pdg_query` reads the control/data-dependence layers `gitnexus analyze --pdg` records (CDG + REACHING_DEF, basic-block granular) — the control/data analog of `explain`. It is **always anchored** (a `target` file path or symbol, resolved like `context`) and has two modes: + +- `pdg_query { mode: "controls", target: "..." }` — CDG: "under what condition does X run?". Each edge is a controlling predicate block → dependent block with the branch sense (`'T'`/`'F'`) in `reason`; an edge into an early `return`/`throw` is flagged `guard: true` (guard-clause discovery — the sense depends on the predicate, so don't filter guards by a fixed label). +- `pdg_query { mode: "flows", target: "...", variable?: "..." }` — REACHING_DEF def→use edges within the function; pass `variable` to trace one binding. + +A repo indexed without `--pdg` returns a "no PDG layer" note (or "status unknown" when the layer can't be confirmed). Intra-procedural only — cross-function flow is taint's domain (`explain`). The raw CDG/REACHING_DEF edges are also queryable via `cypher`. See the `gitnexus-pdg-query` skill for the full query surface. + ## Resources Reference Lightweight reads (~100-500 tokens) for navigation: diff --git a/.claude/skills/gitnexus/gitnexus-pdg-query/SKILL.md b/.claude/skills/gitnexus/gitnexus-pdg-query/SKILL.md new file mode 100644 index 000000000..f2fcd7d3b --- /dev/null +++ b/.claude/skills/gitnexus/gitnexus-pdg-query/SKILL.md @@ -0,0 +1,89 @@ +--- +name: gitnexus-pdg-query +description: "Use when querying or extending GitNexus's PDG control/data-dependence surface (the `pdg_query` MCP tool, CDG/REACHING_DEF edges), or reasoning about \"what controls X\" / \"where does Y flow\" / guard clauses. Examples: \"what guards this statement?\", \"trace this variable within the function\", \"why is the pdg_query result empty?\", \"add a CDG query\"." +--- + +# PDG query surface with GitNexus + +Expert knowledge for the `pdg_query` MCP tool and the control/data-dependence +edges it reads — the opt-in `--pdg` program-dependence layers. Read this before +touching `gitnexus/src/mcp/local/local-backend.ts` (`_pdgQueryImpl`) or the +`pdg_query` tool def, or when explaining a `pdg_query` result. + +## When to Use + +- "Under what condition does this statement run?" (guarding predicates). +- "Where does this variable flow inside the function?" (def→use). +- Guard-clause discovery (early-return guards — subsumes the #559 heuristic). +- Extending or reviewing `pdg_query` / the CDG / REACHING_DEF read path. +- Debugging an empty or surprising `pdg_query` result. + +## The layered substrate (build order) + +`pdg_query` runs **on** the same graph taint runs on. Each layer is opt-in +behind `--pdg`; a default `analyze` run records none of them (byte-identical). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L5 CDG Ferrante control dependence (post-dominators) (M5 #2085) +``` + +All three are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table +(keyed by the `type` property). There is **no** `Function → BasicBlock` edge. + +## The two modes + +- `pdg_query({ mode: 'controls', target })` — CDG. For the anchored function, + each edge: controlling predicate block → dependent block + branch sense in + `label` (`'T'` = predicate's true/taken arm, `'F'` = false/fall-through). An + edge into an early-return/throw block is flagged `guard: true`. +- `pdg_query({ mode: 'flows', target, variable? })` — REACHING_DEF def→use + edges; `variable` filters to one binding. + +`target` is **required** — a file path or a symbol/function name (resolved like +`context()`). There is no anchorless mode (see below). + +## The corrected guard-clause Cypher + +The RFC #567 §2 form (`[:CDG {label:'F'}]`) does **not** run as written. Edges +are values of the single `CodeRelation` table's `type` property, and the branch +sense is in `reason`, NOT a `label` column: + +```cypher +MATCH (pred:BasicBlock)-[r:CodeRelation {type: 'CDG'}]->(dep:BasicBlock) +WHERE dep.text STARTS WITH 'return' OR dep.text STARTS WITH 'throw' +RETURN pred.startLine, r.reason AS branch, dep.startLine, dep.text +``` + +`r.reason` is the sense the predicate took to reach the early exit. For +`if (!ok) return;` the return rides the predicate's **true** arm (`'T'`) and the +protected body rides the **false** arm (`'F'`) — polarity depends on the guard, +so don't hard-code one sense. + +## Gotchas (the load-bearing ones) + +- **Always anchored + LIMIT-bounded.** LadybugDB has no rel-property index, so + an unanchored `[:CDG*]`/`[:REACHING_DEF*]` path scan is unbounded. `pdg_query` + requires `target` and bounds the page; raw `cypher` callers must anchor on a + file id-prefix or symbol span themselves. +- **BasicBlock↔symbol join is reconstructed.** No `Function→BasicBlock` edge: + the block is matched by its id-prefix (`BasicBlock:::…`) + plus `startLine` within the symbol's span. BasicBlock `startLine` is **1-based** + while the symbol node's `startLine`/`endLine` are **0-based**, so **both** bounds + are shifted `+1` (`[symStart+1, symEnd+1]`): the upper `+1` keeps a guard/def/use + on the function's **final line**, the lower `+1` excludes an adjacent function's + block on the line directly **above**. Same-line / nested functions anchor coarsely. +- **No PDG layer ⇒ a note, not an error.** If the repo wasn't indexed with + `--pdg` the tool returns `{ results: [], note: "no PDG layer …" }` (cheap meta + probe on `RepoMeta.pdg.maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`). +- **CDG labels are binary in M5/M6.** Every `switch`-case arm is `'T'`; per-case + conditions are not yet distinguished. +- **Intra-procedural only.** Cross-function flow is taint's domain (`explain`). + +## Mirror, don't fork + +`_pdgQueryImpl` is the front half of `_explainImpl` (WAL wrapper, meta no-layer +probe, limit validation, `resolveSymbolCandidates` anchoring) with CDG/ +REACHING_DEF instead of TAINTED — and none of taint's path-codec / interproc +`TAINT_PATH` machinery. Reuse those shared helpers; do not re-implement them. diff --git a/.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md b/.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md index e13c04e14..90c8c324d 100644 --- a/.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md +++ b/.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md @@ -17,7 +17,7 @@ description: "Use when the user wants to rename, extract, split, move, or restru ``` 1. impact({target: "X", direction: "upstream"}) → Map all dependents -2. query({query: "X"}) → Find execution flows involving X +2. query({search_query: "X"}) → Find execution flows involving X 3. context({name: "X"}) → See all incoming/outgoing refs 4. Plan update order: interfaces → implementations → callers → tests ``` diff --git a/.claude/skills/gitnexus/gitnexus-taint-analysis/SKILL.md b/.claude/skills/gitnexus/gitnexus-taint-analysis/SKILL.md new file mode 100644 index 000000000..9bffffdac --- /dev/null +++ b/.claude/skills/gitnexus/gitnexus-taint-analysis/SKILL.md @@ -0,0 +1,178 @@ +--- +name: gitnexus-taint-analysis +description: "Use when working on, reviewing, or extending GitNexus's CFG/taint/PDG subsystem (the `--pdg` layers), or when reasoning about source→sink data-flow findings. Examples: \"How does taint analysis work here?\", \"Why didn't explain find this flow?\", \"Add a new sink/source\", \"Review the interprocedural taint code\"." +--- + +# CFG & Taint Analysis with GitNexus + +Expert knowledge for the opt-in `--pdg` program-analysis subsystem: control-flow +graphs, reaching definitions, and intra- + inter-procedural taint. Read this +before touching `gitnexus/src/core/ingestion/cfg/**` or +`gitnexus/src/core/ingestion/taint/**`, or when explaining a finding. + +## When to Use + +- "How does the taint engine work / why is this flow (not) reported?" +- Adding a source, sink, or sanitizer to the model. +- Extending or reviewing the CFG / reaching-defs / taint / summary code. +- Understanding the `explain` MCP tool's findings (intra- vs inter-procedural). +- Debugging a false positive or false negative in `--pdg` output. + +## The layered substrate (build order) + +Taint runs **on** the graph, not beside it. Each layer is opt-in behind `--pdg` +and a default `analyze` run is **byte-identical** (the golden parity gate is the +hard floor for every change here). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L3 Taint (intra) source→sink over RD facts, minus sanitizers (M3 #2083) +L4 Taint (inter) per-function summaries composed over CALLS (M4 #2084) +``` + +- **Worker-built, main-thread-solved.** The parse worker builds each function's + CFG + harvests def/use + call-site facts onto `ParsedFile.cfgSideChannel` + (plain, structured-clone-safe data — never AST nodes). The main thread runs + the pure solvers. NEVER re-parse on the main thread (re-introduces the #1983 + OOM). +- **In-phase emit (KTD1).** L1–L4-harvest all run INSIDE the scope-resolution + pdg window (`scope-resolution/pipeline/run.ts`, gated `input.pdg === true`), + because the disk-backed ParsedFile store is cleared when that phase ends — a + standalone post-`mro` phase would read empty data. The cross-function fixpoint + (L4) is the exception: it runs in its OWN registered phase (`taintSummaries`) + AFTER scope-resolution, because it needs the COMPLETE call graph, and consumes + small plain summary data threaded out via `ScopeResolutionOutput`. +- **Pure-solver contract.** `computeReachingDefs`, `computeTaintFlows`, + `harvestFunctionSummary`, and `solveInterprocTaint` are pure and deterministic + (no graph, no I/O, no logger; sorted outputs). Snapshot tests and + content-derived edge ids depend on it. + +## Intra-procedural taint (L3) + +Forward reachability over RD facts from matched **sources** to matched **sinks**, +killed by **sanitizers**. Key design points worth internalizing: + +- **Occurrence-tagged sites.** A flat per-arg binding set cannot tell + `exec(escape(x))` (safe) from `exec(x)` (finding); the harvest records nested + call structure (`SiteRecord.parent`/via-tags) so sanitizer interposition is + precise. +- **Kind-set sanitizer model.** A taint carries a set of *neutralized* + `SinkKind`s; a sink fires unless its kind is in the set. So `escape(req.body)` + suppresses `res.send` (xss) but STILL fires `db.query` (sql) — a kind-blind + kill would be a suppressed live injection (the forbidden FN direction). + `path.basename(t)` neutralizes path-traversal only, not command-injection. +- **Statement-level finding identity.** NOT block-pair (block conflation drops + distinct findings; `exec(req.body, req.query)` is two findings). +- Persisted as `TAINTED` edges (BasicBlock→BasicBlock); the path rides the + `reason` column via the shared versioned codec (`taint/path-codec.ts`). + +## Interprocedural taint (L4) — the functional/summary method + +The production approach (Sharir-Pnueli 1981; the same shape as Meta's Pysa and +Mariana Trench, and FB Infer) — NOT full IFDS tabulation. Each function is +reduced to a compact **summary**, and summaries are composed over the already- +resolved `CALLS` graph. + +**Summary shape** (`taint/summary-model.ts`, whole-parameter granularity): + +| Edge | Meaning | Analogue | +|------|---------|----------| +| `param→return` | a param flows to the return value | TITO — **reserved** (the floor already covers its recall; precision pass deferred) | +| `param→callee-arg` | a param flows into arg *j* of a call (carries the path's neutralized sink kinds) | TITO into callee | +| `param→sink` | a param reaches a modelled sink | partial/triggered sink | +| `source→return` | the function generates+returns a source | generative — **composed** via the caller's `callResults` | +| `source→callee-arg` | a generated source flows into a call | fixpoint SEED | +| `callResults` | a user-function call's result flows to a sink/return/callee-arg in the caller | composes with callee `source→return` | + +**The fixpoint** (`taint/interproc-solver.ts`): the unit is `(function, +parameter, source)`. Seed from `source→callee-arg`, propagate via +`param→callee-arg`, fire a finding when a tainted param meets `param→sink`. + +- **Cycle-safe by monotonicity.** The tainted-set is monotone over a finite + lattice (`fn × param × source`), so the worklist converges — a recursive call + just re-proposes an already-visited entry. SCC condensation would only refine + processing order; correctness/termination don't require it. +- **Source-discriminated state (load-bearing).** Key the state by the SOURCE + too. Keying only by `(fn, param)` collapses multi-source flows: a sink param + tainted by source A is marked visited and a later flow from source B is dropped + before firing — the recurring multi-source bug class. (Bit M3; bit M4 U9.) +- **Name-based call join.** Match a summary's call-arg edge to a `CALLS` edge by + CALLEE NAME, not call-site line — line-base parity (CFG 1-based vs reference + site) is fragile; the callee identity is exact and context-insensitivity + taints the callee's param identically at every call site. +- Persisted as `TAINT_PATH` edges (Function→Function), function-level hop chain + in `reason` via the same codec; confidence < the intra-procedural 1.0. + +**Context-insensitivity** is the accepted trade-off at this tier: one summary +per function, return/call-site merging accepted (security-conservative). Expect +some FP from merging; the bigger FN sources are unmodeled features (below). + +## Known false-negative classes (documented, deferred) + +The largest is **closures/callbacks** (`arr.forEach(() => sink(y))`) — taint +into a callback is dropped without per-library models (true of CodeQL's JS libs +too). Also deferred: field/property flows (`obj.x = taint; sink(obj.y)`), +field-sensitive access paths, guard-style sanitizers, implicit/control-dependence +flows, promise/async-await threading, and **destructured/rest params before a +tainted simple param** (the summary port index is the binding ordinal, not the +formal arg position — needs a formal-param index threaded from the worker +`BindingEntry`). The interprocedural join is also context-insensitive: when one +caller invokes two distinct **same-named callees**, a flow into one +over-attributes to both (sound — over-report, never a missed flow). Absence of a +finding is NOT proof of safety. + +## GitNexus-specific gotchas + +- **Function↔CFG join.** `FunctionCfg.functionStartLine` is 1-based; `Function`/ + `Method` node `startLine` is 0-based — join at `startLine - 1`. Function nodes + have no column, so same-line functions (`{a:()=>x(), b:()=>y()}`) are + ambiguous → drop (the summary driver counts `unresolved`) rather than + cross-wire. +- **No rel-property index (S1).** Kuzu has no secondary index on relationship + properties, and unanchored `[:TAINTED*]`/`[:TAINT_PATH*]` queries explode. + TAINT_PATH is therefore MATERIALIZED + anchored at analyze time, never + traversed live; `explain` reads it source-anchored + LIMIT-guarded. +- **`explain` is the only discovery surface.** `TAINTED`/`TAINT_PATH` are + deliberately OUT of `VALID_RELATION_TYPES` (impact's allow-list) and the web + schema (pinned in `security.test.ts`). `explain` enumerates both layers + (cross-function findings carry `interprocedural: true`). +- **One shared codec.** Both the emit path and `explain` import + `taint/path-codec.ts`. Two hand-rolled copies of a wire format drift — never + fork it. New metadata extends the format WITHIN the version when writer + + reader ship together. +- **Cache versioning.** A worker-harvest shape change bumps the parse-cache pdg + NAMESPACE (`pdg:N`), NOT `SCHEMA_BUMP` (which cold-invalidates every user). + Persisted-graph/config changes ride `RepoMeta.pdg`'s key-union mismatch → + full writeback. Model content rides `taintModelVersion`. + +## Adding a source / sink / sanitizer + +Edit the language model in `taint/typescript-model.ts` (registered via the +explicit `registerBuiltinTaintModels` seam, keyed by `SupportedLanguages`). The +spec is hashable data (no functions). A sanitizer's `neutralizes` lists the +EXACT sink kinds it defends — never a blanket kill. Add a fixture + assert the +finding (or its absence) in `test/unit/taint/` (real-source harness: +`test/helpers/ts-cfg-harness.ts`); the end-to-end proof is +`test/integration/cfg/`. + +## Validation checklist for any `--pdg` change + +``` +1. tsc clean (schema additions are exhaustiveness-checked; watch the + api.ts getNodeQuery runtime read-path if a node label is added). +2. Targeted vitest by directory (test/unit/taint, test/unit/cfg, + test/integration/cfg) — verify by ISOLATION, not full-suite exit + (known load-flakes). `node scripts/build.js` before worker/integration runs. +3. Flag-off golden byte-identical (pipeline-graph-golden.test.ts). +4. bench/cfg/measure.mjs --check (no fingerprint drift / budget regression). +5. detect_changes() before commit; impact({direction:'upstream'}) before + editing shared symbols (KnowledgeGraph, RepoMeta, RelationshipType, codec). +``` + +## Prior art (for deeper design questions) + +Sharir & Pnueli 1981 (functional approach); Reps-Horwitz-Sagiv IFDS (POPL 1995); +FlowDroid/StubDroid (access-path summaries); Pysa & Mariana Trench (TITO / +propagations, parallel SCC fixpoint); CodeQL Models-as-Data (the richest port +notation, incl. callback ports); Infer (content-keyed incremental summaries). diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 620a5483c..c57c0d6aa 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -22,26 +22,10 @@ # --format '{{json .Manifest.Digest}}' FROM mcr.microsoft.com/devcontainers/typescript-node@sha256:7c2e711a4f7b02f32d2da16192d5e05aa7c95279be4ce889cff5df316f251c1d -# Build args. We deliberately set no version defaults here. devcontainer.json -# `build.args` is the single source of truth for versions. A standalone -# `docker build .devcontainer/` (for example, a CI smoke test) must pass each -# version with --build-arg. Without a default, the build fails loudly instead of -# silently drifting from the version pinned in devcontainer.json. -ARG CLAUDE_CODE_VERSION -ARG CODEX_VERSION -# Cursor is pinned by version plus a per-arch tarball sha256 hash. The install -# step below verifies that hash. All three values live in devcontainer.json -# build.args. They follow the same rule as the others: one source of truth, and -# no default so the build fails loudly if a value is missing. -ARG CURSOR_VERSION -ARG CURSOR_SHA256_X64 -ARG CURSOR_SHA256_ARM64 # Bun is installed via the official remote script (bun.sh/install), pinned by -# version. UNLIKE Cursor and the npm packages, this install path runs an -# UNVERIFIED remote script — there is no tarball-hash check. Chosen explicitly -# at request time over the pin-by-sha256 alternative for install-script -# simplicity. To harden later, switch to a pinned tarball + per-arch sha256 in -# the Cursor style (release artifacts at github.com/oven-sh/bun/releases). +# version. Claude Code and Cursor also use official install scripts (no version +# to pin). To harden Bun: switch to a pinned tarball + per-arch sha256 +# (release artifacts at github.com/oven-sh/bun/releases). ARG BUN_VERSION ARG TZ=UTC ARG USERNAME=node @@ -50,10 +34,7 @@ ARG USERNAME=node # read them. We deliberately do not set CLAUDE_CONFIG_DIR here. Its one true # value lives in devcontainer.json `containerEnv`, and the runtime value wins # anyway. -ENV CLAUDE_CODE_VERSION=${CLAUDE_CODE_VERSION} \ - CODEX_VERSION=${CODEX_VERSION} \ - CURSOR_VERSION=${CURSOR_VERSION} \ - BUN_VERSION=${BUN_VERSION} \ +ENV BUN_VERSION=${BUN_VERSION} \ BUN_INSTALL=/home/${USERNAME}/.bun \ TZ=${TZ} \ DEVCONTAINER=true \ @@ -86,51 +67,19 @@ RUN mkdir -p \ USER ${USERNAME} -# Install Claude Code and the Codex CLI globally, as the `node` user. The base -# image sets /usr/local/share/npm-global as the npm-global prefix and makes the -# `npm` group writable by `node`. So `npm install -g` works without sudo. Both -# versions come from build args. To upgrade, bump them in devcontainer.json and -# rebuild. -RUN npm install -g \ - @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} \ - @openai/codex@${CODEX_VERSION} +# Install Claude Code via the official native installer. Downloads the latest +# self-contained binary for the running platform and places it at +# ~/.local/bin/claude — no Node.js runtime dependency, no version to pin. +RUN curl -fsSL https://claude.ai/install.sh | bash -# Install the Cursor CLI. It is pinned and hash-verified, and we run no remote -# script. The cursor.com/install script just detects os/arch, downloads a -# versioned tarball from -# downloads.cursor.com/lab////agent-cli-package.tar.gz, -# extracts it, and symlinks `agent`/`cursor-agent` into ~/.local/bin. We do that -# ourselves against a PINNED version plus a per-arch sha256 hash. So the build -# runs no unverified remote code. This matches how we pin the base image and npm -# packages by digest (issue #1451). The download is fail-closed: if the hash -# does not match, the build aborts. -# -# To bump: set CURSOR_VERSION and both CURSOR_SHA256_* in devcontainer.json -# build.args. Get each arch's hash with: -# curl -fSL https://downloads.cursor.com/lab//linux//agent-cli-package.tar.gz | sha256sum -# -# TARGETARCH is the per-platform build arg that BuildKit sets automatically. It -# must be (re)declared in this stage to be visible. When the build is a -# non-BuildKit `docker build`, TARGETARCH is unset, so we fall back to `dpkg -# --print-architecture`. -ARG TARGETARCH -RUN set -eux; \ - arch="${TARGETARCH:-$(dpkg --print-architecture)}"; \ - case "$arch" in \ - amd64) cursor_arch=x64; cursor_sha="${CURSOR_SHA256_X64}";; \ - arm64) cursor_arch=arm64; cursor_sha="${CURSOR_SHA256_ARM64}";; \ - *) echo "unsupported architecture for Cursor: $arch" >&2; exit 1;; \ - esac; \ - url="https://downloads.cursor.com/lab/${CURSOR_VERSION}/linux/${cursor_arch}/agent-cli-package.tar.gz"; \ - curl -fSL --retry 3 --max-time 120 -o /tmp/cursor.tgz "$url"; \ - echo "${cursor_sha} /tmp/cursor.tgz" | sha256sum -c -; \ - dir="/home/${USERNAME}/.local/share/cursor-agent/versions/${CURSOR_VERSION}"; \ - install -d "$dir" "/home/${USERNAME}/.local/bin"; \ - tar --strip-components=1 -xzf /tmp/cursor.tgz -C "$dir"; \ - test -x "$dir/cursor-agent"; \ - ln -sf "$dir/cursor-agent" "/home/${USERNAME}/.local/bin/agent"; \ - ln -sf "$dir/cursor-agent" "/home/${USERNAME}/.local/bin/cursor-agent"; \ - rm -f /tmp/cursor.tgz +# Install the Codex CLI globally via npm. No version pinned — @latest at build +# time. (Codex has no native binary installer; npm is the canonical method.) +RUN npm install -g @openai/codex + +# Install the Cursor agent CLI via the official install script. Downloads the +# latest agent-cli-package for the running platform and places `cursor-agent` +# and `agent` into ~/.local/bin — no version or hash to pin. +RUN curl -fsSL https://cursor.com/install | bash # Install Bun via the official remote installer, pinned by version. The first # positional arg to `bash` is the release tag (`bun-vX.Y.Z`), so a specific diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index a7160c8d5..7b0f7e219 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -14,16 +14,6 @@ "dockerfile": "Dockerfile", "context": ".", "args": { - "CLAUDE_CODE_VERSION": "2.1.156", - "CODEX_VERSION": "0.134.0", - // Cursor: a pinned version plus one sha256 hash per CPU arch. The - // Dockerfile checks the tarball against the hash at build time, so it - // never runs a remote install script. Bump all three values together. - // Re-hash each arch with: - // curl -fSL https://downloads.cursor.com/lab//linux//agent-cli-package.tar.gz | sha256sum - "CURSOR_VERSION": "2026.05.28-a70ca7c", - "CURSOR_SHA256_X64": "7f8b6a09393e0b84b288cc6952b292fc98d15775f644cc01b0b9aa4f04b268df", - "CURSOR_SHA256_ARM64": "05a0ab361e038729aba25fe7f407531b3e8432912e499d0bffdf1dda0e7833e9", // Bun: pinned by version. Installed by the official bun.sh/install // script, which accepts the release tag as its first positional arg // (`bash -s bun-vX.Y.Z`). UNLIKE Cursor, the install path runs an @@ -324,17 +314,6 @@ // dependency explicit instead of silently following the default. "containerEnv": { "CODEX_HOME": "/home/node/.codex", - "DISABLE_AUTOUPDATER": "1", - // post-create.sh removes `installMethod` from the seeded ~/.claude.json so - // the npm-global binary detects its own install method. This is a backup - // safeguard for Claude Code issue #17289. The install-checks routine probes - // ~/.local/bin/claude just because that directory EXISTS. It does exist - // here, because Cursor drops agent and cursor-agent symlinks there. So even - // when installMethod is non-native, the routine reports a false "claude - // command not found at ~/.local/bin/claude". DISABLE_AUTOUPDATER does NOT - // turn that routine off. DISABLE_INSTALLATION_CHECKS is its dedicated kill - // switch. - "DISABLE_INSTALLATION_CHECKS": "1", "HISTFILE": "/commandhistory/.zsh_history" }, diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index 58c9f8892..fce605153 100644 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -141,15 +141,12 @@ sync_from_host /host/.codex/config.toml /home/node/.codex/config.toml 644 # Seed $HOME/.claude.json from the host, but NOT as a straight copy. That file # mixes two kinds of state. Some is portable account and onboarding state we # want to keep: hasCompletedOnboarding, oauthAccount, userID, projects, -# tipsHistory. The rest describes how Claude is installed on the host, and that -# part is never valid here. This image installs Claude with `npm install -g`, -# but the host's `installMethod` (for example "native") makes Claude look for -# ~/.local/bin/claude and fail with -# "claude command not found at /home/node/.local/bin/claude". The fix strips the -# machine-specific fields and forces hasCompletedOnboarding, while handling a -# host file that isn't a JSON object. That logic lives in seed-claude-config.cjs -# so it can be unit-tested and prettier-checked -# (translate-plugin-registries.test.cjs). +# tipsHistory. The rest describes how Claude is installed on the HOST, and that +# part is never valid here — for example the host's `installMethod` value only +# makes sense for the host's binary. The fix strips the machine-specific fields +# and forces hasCompletedOnboarding, while handling a host file that isn't a +# JSON object. That logic lives in seed-claude-config.cjs so it can be +# unit-tested and prettier-checked (translate-plugin-registries.test.cjs). node "$SCRIPT_DIR/seed-claude-config.cjs" # Codex auth. Some hosts store credentials in the OS keyring instead of on disk diff --git a/.github/scripts/check-tree-sitter-upgrade-readiness.py b/.github/scripts/check-tree-sitter-upgrade-readiness.py index 35d86d766..269afa7b2 100644 --- a/.github/scripts/check-tree-sitter-upgrade-readiness.py +++ b/.github/scripts/check-tree-sitter-upgrade-readiness.py @@ -1,27 +1,14 @@ #!/usr/bin/env python3 -"""Monitor tree-sitter 0.25 upgrade readiness. +"""Monitor tree-sitter 0.25 upgrade readiness — two things Dependabot can't see: -Tracks two things Dependabot cannot see: + 1. Peer-dep compatibility: when every grammar's *latest npm release* accepts + tree-sitter@0.25.0 (so we can upgrade without --legacy-peer-deps). + 2. Vendored upstream drift: whether a vendored grammar's upstream parser.c moved. - 1. Peer-dep compatibility. Each tree-sitter-* grammar declares a peer - dependency on the tree-sitter runtime. We want to know when every - grammar's *latest npm release* satisfies tree-sitter@0.25.0 so we - can upgrade without --legacy-peer-deps. - - 2. Vendored upstream drift. vendor/tree-sitter-proto/ is a snapshot of - coder3101/tree-sitter-proto's parser.c. When upstream moves, we want - to know whether we can pick it up. - -Invoked from .github/workflows/tree-sitter-upgrade-readiness.yml daily. -Runs locally too: - - python3 .github/scripts/check-tree-sitter-upgrade-readiness.py - -Outputs Markdown to stdout. Exit 0 when every grammar is upgrade-ready -and the vendored proto is in sync. Exit 1 when blockers remain (the -workflow uses this to open or update a tracking issue). - -No external deps -- stdlib only, so it runs on any vanilla runner. +Invoked daily from tree-sitter-upgrade-readiness.yml; runs locally too. Outputs +Markdown to stdout; exit 1 when blockers remain (the workflow upserts a tracking +issue). stdlib-only — runs on any vanilla runner. + python3 .github/scripts/check-tree-sitter-upgrade-readiness.py [--offline | --assert-current] """ from __future__ import annotations @@ -38,6 +25,11 @@ import urllib.request REPO_ROOT = pathlib.Path(__file__).resolve().parents[2] GITNEXUS_DIR = REPO_ROOT / "gitnexus" +# Offline mode (--offline flag or GITNEXUS_TS_READINESS_OFFLINE=1): skip ALL network +# so the script + tests run hermetically. npm columns render "n/a (offline)"; +# vendored ABIs are still read from the repo. The read-path mirror of --assert-current. +OFFLINE = os.environ.get("GITNEXUS_TS_READINESS_OFFLINE", "") not in ("", "0", "false") + # ── Upgrade target ────────────────────────────────────────────────────── # The runtime version we want to upgrade TO. Update this when the goal # changes (e.g. once 0.25 lands and we target 0.26). @@ -78,15 +70,11 @@ GRAMMARS: dict[str, tuple[str, str, str]] = { "tree-sitter-proto": ("coder3101/tree-sitter-proto", "main", "src/parser.c"), } -# Grammars deliberately held below npm latest. The readiness report surfaces -# these so reviewers can tell intentional pins apart from drift, and so the -# context for each pin (which issue motivated it) is visible at a glance. -# Add an entry whenever you pin a grammar below npm latest. +# npm-installed grammars deliberately held below npm latest (surfaced so reviewers +# can tell intentional pins from drift). Add an entry when you pin an npm grammar. +# VENDORED grammars carry their hold in .github/vendored-grammars.json instead, so a +# vendored grammar's hold lives in one place — tree-sitter-c's is there, not here. INTENTIONAL_PINS: dict[str, str] = { - "tree-sitter-c": ( - "#1242 — last release built against the tree-sitter@0.21 ABI; " - "tree-sitter-c@0.23.x prebuilds segfault on Windows under tree-sitter@0.21.1" - ), "tree-sitter-cpp": ( "#1242 — last 0.23.x release before tree-sitter-cpp added a runtime " "dep on the broken-ABI tree-sitter-c@^0.23.1; pinning here removes " @@ -95,6 +83,56 @@ INTENTIONAL_PINS: dict[str, str] = { } +def load_vendored_manifest() -> dict[str, dict]: + """Load the shared vendored-grammar manifest (.github/vendored-grammars.json). + + The single source of truth — shared with update-vendored-grammars.mjs — for + which grammars are *vendored* (shipped from gitnexus/vendor/, not npm) + and any policy ``hold`` (e.g. tree-sitter-c, #1242/#858). Membership routes a + grammar to the vendored branch, which reads its ABI from the repo instead of + node_modules (the #858 source of the old bare ``?``). Returns + ``{ name: {"hold": str | None} }``; upstream-drift coords stay in ``GRAMMARS``. + """ + manifest_path = REPO_ROOT / ".github" / "vendored-grammars.json" + # Fail loud with a pointer, not a bare traceback: this runs at module import, + # so a missing/corrupt manifest would otherwise crash both the script and any + # test that imports it with an opaque FileNotFoundError/JSONDecodeError. + try: + data = json.loads(manifest_path.read_text(encoding="utf-8")) + except FileNotFoundError as exc: + raise SystemExit( + f"vendored-grammars manifest not found at {manifest_path}. " + f"It is the shared source of truth for vendored grammars " + f"(see CONTRIBUTING.md → CI automation contracts)." + ) from exc + except json.JSONDecodeError as exc: + raise SystemExit( + f"vendored-grammars manifest at {manifest_path} is not valid JSON: {exc}." + ) from exc + out: dict[str, dict] = {} + for key, g in (data.get("grammars") or {}).items(): + name = g.get("name") + if not name: + raise SystemExit( + f"vendored-grammars manifest entry {key!r} is missing a 'name' field " + f"({manifest_path})." + ) + # Defense-in-depth (#2187): `name` is joined into gitnexus/vendor/, so + # reject anything not a plain grammar name before it can traverse ("../etc"). + if not re.fullmatch(r"tree-sitter-[a-z0-9-]+", name): + raise SystemExit( + f"vendored-grammars manifest entry {key!r} has an invalid grammar " + f"name {name!r} (must match tree-sitter-[a-z0-9-]+)." + ) + out[name] = {"hold": g.get("hold")} + return out + + +# Vendored set + holds, keyed by full grammar name (e.g. "tree-sitter-c"). +VENDORED: dict[str, dict] = load_vendored_manifest() +VENDORED_NAMES: frozenset[str] = frozenset(VENDORED) + + # ── Helpers ───────────────────────────────────────────────────────────── def _load_package_json() -> dict: @@ -134,6 +172,8 @@ def npm_view_json(pkg: str) -> dict | None: being available (it's a batch file on Windows which complicates subprocess calls). """ + if OFFLINE: + return None url = f"https://registry.npmjs.org/{pkg}/latest" try: req = urllib.request.Request(url, headers={"Accept": "application/json"}) @@ -190,6 +230,8 @@ def fetch_text(url: str, timeout: int = 8) -> str | None: Adds an Authorization header for github.com URLs when GITHUB_TOKEN is set (raises the rate limit from 60 to 5 000 requests/hour). """ + if OFFLINE: + return None headers: dict[str, str] = {} # Parse the URL and check the hostname rather than substring-matching # on the full URL string (CodeQL py/incomplete-url-substring-sanitization). @@ -231,14 +273,8 @@ def md_h(text: str, level: int = 2) -> str: def _first_sentence(text: str) -> str: - """Return the leading sentence of a free-form rationale string. - - Vendor package.json `_vendoredBy` fields often look like - ". . Do NOT ." — the - first sentence is what reviewers actually want to read; the rest is - noise in this context. Match a sentence-ending '.' followed by - whitespace; fall back to the whole string if nothing matches. - """ + """Return the leading sentence of a `_vendoredBy` rationale (the rest tails off + into install-script breadcrumbs); fall back to the whole string.""" text = text.strip() match = re.search(r"\.\s+[A-Z]", text) return text[: match.start() + 1] if match else text @@ -262,21 +298,26 @@ def range_includes(spec: str | None, version: str) -> bool: return spec.strip() == version.strip() -def is_vendored_pin(spec: str | None) -> bool: - return bool(spec) and spec.startswith(("file:", "git", "http")) +def vendored_abi_from_repo(name: str, parser_path: str) -> int | None: + """Read a vendored grammar's ABI directly from gitnexus/vendor/. + + Local-only (no network) — the offline half of ``vendored_drift_summary``, + factored out so the hermetic ``--assert-current`` gate can introspect vendored + ABIs without triggering the upstream-drift fetches it never uses (#858 review). + """ + vendor_dir = GITNEXUS_DIR / "vendor" / name + vendored_parser = vendor_dir / parser_path + if not vendored_parser.is_file(): + vendored_parser = vendor_dir / "src" / "parser.c" + return extract_language_version(vendored_parser) def vendored_drift_summary( name: str, upstream_repo: str, upstream_branch: str, parser_path: str ) -> dict: - """Inspect a vendored grammar under gitnexus/vendor/. - - Returns the vendored package.json's ``version`` and ``_vendoredBy`` - fields (which carry the human rationale for vendoring), the vendored - parser's ABI, and a comparison against upstream main. We deliberately - rely on ``_vendoredBy`` rather than a parallel registry in this - script: the rationale belongs next to the vendored sources, not in - a daily-running CI script. + """Inspect a vendored grammar under gitnexus/vendor/: returns its + package.json ``version`` + ``_vendoredBy`` (the rationale, kept next to the + sources), the vendored ABI, and a comparison against upstream main. """ vendor_dir = GITNEXUS_DIR / "vendor" / name pkg: dict = {} @@ -290,7 +331,7 @@ def vendored_drift_summary( vendored_parser = vendor_dir / parser_path if not vendored_parser.is_file(): vendored_parser = vendor_dir / "src" / "parser.c" - vendored_abi = extract_language_version(vendored_parser) + vendored_abi = vendored_abi_from_repo(name, parser_path) upstream_url = ( f"https://raw.githubusercontent.com/{upstream_repo}/" @@ -302,10 +343,13 @@ def vendored_drift_summary( sha_text = fetch_text( f"https://api.github.com/repos/{upstream_repo}/commits/{upstream_branch}" ) - upstream_sha = "?" + # Labeled fallback rather than a bare "?": in CI this fetch succeeds, but + # offline (or on a transient API miss) the report should say *why* it's + # blank instead of leaving a placeholder (#858). + upstream_sha = "unknown" if sha_text: try: - upstream_sha = json.loads(sha_text).get("sha", "?")[:12] + upstream_sha = json.loads(sha_text).get("sha", "unknown")[:12] except json.JSONDecodeError: pass @@ -321,7 +365,9 @@ def vendored_drift_summary( return { "name": name, - "vendored_version": pkg.get("version", "?"), + # Labeled fallback, never a bare "?": a vendor package.json should always + # carry a version, but if one is missing the report says so plainly (#858). + "vendored_version": pkg.get("version") or "unknown", "vendored_by": pkg.get("_vendoredBy"), "vendored_abi": vendored_abi, "upstream_repo": upstream_repo, @@ -336,27 +382,14 @@ def vendored_drift_summary( def assert_current() -> int: - """Assert every grammar's ABI is loadable by the CURRENT runtime. - - Unlike the readiness report (which probes the npm registry + upstream - main for the *target* runtime), this mode is hermetic and offline: it - reads only what's checked out / installed locally and asserts each - grammar's compiled ABI lies within the current runtime's - ``RUNTIME_ABI_RANGES`` window. It is the static half of the #1922 ABI - gate; the runtime load-smoke (`parser-loader-abi.test.ts`) is the - dynamic half. - - Coverage, reusing the existing helpers: - - npm-installed grammars: ABI from node_modules//. - - vendored grammars (dart/proto/swift): ABI via ``vendored_drift_summary``. - - Swift is prebuilt-only (no parser.c) → not introspectable here; - treated as "covered by the runtime load-smoke", not asserted. - - INTENTIONAL_PINS are honored: a pinned grammar is expected to sit at - an ABI the current runtime loads (that's *why* it's pinned), so it is - asserted like any other rather than skipped. + """Assert every grammar's compiled ABI loads on the CURRENT runtime. + The hermetic/offline static half of the #1922 ABI gate (the runtime + load-smoke is the dynamic half): reads only local files — npm ABIs from + node_modules/, vendored ABIs from gitnexus/vendor/ via + ``vendored_abi_from_repo`` (no network). A prebuilt-only vendor (no + parser.c) is skipped; INTENTIONAL_PINS are asserted like any other grammar. Returns 0 when every introspectable grammar is in range, 1 otherwise. - Prints a plain-text (non-Markdown) report so CI logs stay readable. """ current_runtime = read_current_runtime() abi_range = RUNTIME_ABI_RANGES.get(current_runtime) @@ -380,13 +413,20 @@ def assert_current() -> int: for name, (upstream_repo, upstream_branch, parser_path) in sorted(GRAMMARS.items()): pinned_spec = pinned_versions.get(name, "—") - pin_note = f" [intentional pin: {pinned_spec}]" if name in INTENTIONAL_PINS else "" + if name in VENDORED_NAMES and VENDORED[name].get("hold"): + pin_note = " [vendored, held]" + elif name in INTENTIONAL_PINS: + pin_note = f" [intentional pin: {pinned_spec}]" + else: + pin_note = "" - if is_vendored_pin(pinned_spec): - v = vendored_drift_summary(name, upstream_repo, upstream_branch, parser_path) - abi = v["vendored_abi"] + # Vendored grammars: ABI read locally from the repo via vendored_abi_from_repo + # (NOT vendored_drift_summary, which fetches upstream — this gate is hermetic), + # so the offline #1922 gate covers them instead of skipping them (#858/#2187). + if name in VENDORED_NAMES: + abi = vendored_abi_from_repo(name, parser_path) if abi is None: - # Prebuilt-only vendor (e.g. tree-sitter-swift): no parser.c to + # Prebuilt-only vendor (e.g. a binary-only grammar): no parser.c to # introspect. The runtime load-smoke covers it instead. skipped.append(f"{name} (vendored, prebuilt — covered by load-smoke)") continue @@ -453,32 +493,18 @@ def _classify_grammar( ) -> dict: """Decide a single primary disposition + a separate bump-now hint. - Buckets are mutually exclusive and ordered by what a reviewer should - look at first: - - fetch_failed : npm registry fetch failed (treat as blocker, but - surface separately so reviewers don't confuse it - with an upstream block) - - intentional : pinned in INTENTIONAL_PINS — explicit choice - - ready : npm-latest peer dep already accepts the target - runtime; nothing to do - - waiting : main has a fix (ABI 15 or relaxed peer) but no - published npm release yet - - blocked : peer dep too tight on both npm and main - - Independently of bucket, `bump_now` reports whether reviewers can - move the pin forward today without touching the runtime — we only - suggest it when npm-latest's peer dep also accepts our *current* - runtime, otherwise the bump would break `npm install`. + Mutually-exclusive buckets, ordered by reviewer priority: ``fetch_failed`` + (npm fetch failed — surfaced apart from upstream blocks), ``intentional`` + (in INTENTIONAL_PINS), ``ready`` (npm-latest peer accepts the target), + ``waiting`` (a fix on main, unpublished), ``blocked`` (peer too tight on + both). ``bump_now`` is independent: True only when npm-latest's peer also + accepts our *current* runtime (else the bump would break ``npm install``). """ - is_vendored = is_vendored_pin(pinned_spec) - behind_latest = ( - not is_vendored - and npm_version != "?" - and not range_includes(pinned_spec, npm_version) - ) - # Intentional pins must never appear as actionable bumps — by definition - # we're holding them back on purpose. The pin can only be lifted by - # editing INTENTIONAL_PINS and package.json together. + # Only npm-path grammars reach this function — vendored grammars are routed + # to the vendored branch in main() and `continue` before classification. + behind_latest = npm_version != "?" and not range_includes(pinned_spec, npm_version) + # Intentional pins are never actionable bumps (held on purpose; lifted only by + # editing INTENTIONAL_PINS + package.json together). bump_now = behind_latest and current_compat and name not in INTENTIONAL_PINS if fetch_failed: @@ -496,6 +522,9 @@ def _classify_grammar( "name": name, "pinned_spec": pinned_spec or "—", "npm_version": npm_version, + # Display form for the disposition prose, laundering a "?" (a malformed 200 + # npm response lacking `version`) so it never shows bare, like the matrix cell. + "npm_version_label": "unknown" if npm_version == "?" else npm_version, "peer_range": peer_range, "target_compat": target_compat, "current_compat": current_compat, @@ -503,15 +532,105 @@ def _classify_grammar( "behind_latest": behind_latest, "bump_now": bump_now, "bucket": bucket, - "is_vendored": is_vendored, } +def _render_vendored_section( + vendored_grammars: list[dict], + target_abi_range: tuple[int, int], + blockers: dict[str, str], +) -> list[str]: + """Render the 'Vendored parsers' prose block. Appends any runtime-side blocker + (upstream ABI beyond the target range) to ``blockers`` in place; returns the + markdown lines (empty when nothing is vendored). Extracted from main() so that + function coordinates named render phases rather than inlining them (#2187).""" + if not vendored_grammars: + return [] + # Hoisted out of the list literal below: an implicit string concatenation + # inside a list display trips CodeQL py/implicit-string-concatenation-in-list + # (it reads as a possibly-missing comma between elements). + intro = ( + "These grammars ship from `gitnexus/vendor/` rather than the npm " + "registry. Their compatibility is governed by the **vendored " + "ABI** (must lie in the target runtime's range), not by a peer-" + "dep negotiation. The rationale for each vendored copy lives in " + "its own `package.json` `_vendoredBy` field." + ) + lines = [md_h(f"Vendored parsers ({len(vendored_grammars)})", 2), intro, ""] + for v in sorted(vendored_grammars, key=lambda v: v["name"]): + sync_label = "in sync with upstream" if v["in_sync"] else "diverged from upstream" + if v["abi_state"] == "in_range": + abi_label = f"ABI `{v['vendored_abi']}` (in target range)" + elif v["abi_state"] == "prebuilt": + abi_label = "ABI `prebuilt` (binary-only vendor, source not introspectable)" + else: + abi_label = ( + f"ABI `{v['vendored_abi']}` (**outside** target range " + f"{target_abi_range[0]}..{target_abi_range[1]})" + ) + # Never a bare "?": when upstream parser.c can't be read (generated at build, + # or a transient fetch miss), use the neutral `n/a` token (#858). + upstream_abi_str = ( + f"ABI `{v['upstream_abi']}`" if v["upstream_abi"] is not None else "ABI `n/a`" + ) + lines.append( + f"- **`{v['name']}`** `{v['vendored_version']}` — {abi_label}, " + f"upstream `{v['upstream_repo']}@{v['upstream_sha']}` " + f"{upstream_abi_str} · {sync_label}" + ) + if v.get("hold"): + lines.append(f" - **Held:** {v['hold']}") + if v["vendored_by"]: + # First sentence only — vendor _vendoredBy fields tail off into noise. + lines.append(f" - **Why vendored:** {_first_sentence(v['vendored_by'])}") + # Action: regen iff upstream ABI exceeds vendored AND stays within target; + # beyond target is a runtime-side blocker. Prebuilt-only vendors get a + # manual-refresh action driven by the in-sync flag instead. + if v["abi_state"] == "prebuilt": + if not v["in_sync"]: + lines.append( + " - **Action:** check whether upstream has shipped a new " + "prebuilt release; this vendor ships binary-only artefacts." + ) + elif v["upstream_abi"] and v["vendored_abi"] and v["upstream_abi"] > v["vendored_abi"]: + if v["upstream_abi"] <= target_abi_range[1]: + lines.append( + f" - **Action:** after upgrading to tree-sitter@{TARGET_RUNTIME}, " + f"regenerate `parser.c` from upstream `{v['upstream_sha']}`." + ) + else: + lines.append( + f" - **Action:** wait for a runtime supporting ABI " + f"{v['upstream_abi']}; current target ({TARGET_RUNTIME}) only " + f"goes up to ABI {target_abi_range[1]}." + ) + blockers[f"vendored-{v['name']}-abi"] = ( + f"vendored {v['name']}: upstream ABI {v['upstream_abi']} outside target range" + ) + elif not v["in_sync"]: + lines.append( + " - **Action:** review upstream changes; vendored copy may " + "need a refresh (no ABI bump required)." + ) + lines.append("") + return lines + + def main() -> int: blockers: dict[str, str] = {} lines: list[str] = [] + # Label for npm/upstream values we couldn't determine: in --offline mode the + # fetch was deliberately skipped (not "failed"), so say so honestly. + miss_label = "offline" if OFFLINE else "fetch failed" lines.append(md_h("Tree-sitter 0.25 upgrade readiness", 1)) lines.append("") + if OFFLINE: + lines.append( + "> **Offline mode** — npm registry + upstream GitHub checks were skipped. " + "npm-installed grammars show as unverified; vendored-grammar ABIs are read " + "from `gitnexus/vendor/`." + ) + lines.append("") current_runtime = read_current_runtime() current_abi_range = RUNTIME_ABI_RANGES.get(current_runtime, (0, 0)) @@ -525,10 +644,9 @@ def main() -> int: ) lines.append("") - # First pass: gather raw data + classification per grammar. We render - # the human-friendly buckets first, then the raw matrix in a
- # block at the end. Status text in the matrix is preserved verbatim - # so the workflow's row-diff change-detection keeps working. + # First pass: gather + classify per grammar. Human buckets render first, then + # the raw matrix in a
block (Status text preserved verbatim so the + # workflow's row-diff change-detection keeps working). grammar_rows: list[dict] = [] raw_matrix: list[str] = [ "| Grammar | Pinned | npm latest | Peer dep | Satisfies 0.25? | ABI | Upstream ABI | Status |", @@ -540,17 +658,17 @@ def main() -> int: for name, (upstream_repo, upstream_branch, parser_path) in sorted(GRAMMARS.items()): pinned_spec = pinned_versions.get(name, "—") - # Vendored grammars don't have an "npm latest" we install from — - # we ship our own copy under gitnexus/vendor/. Treat them - # as a separate kind of artefact: their readiness for the runtime - # upgrade depends on the vendored ABI being in the target range, - # not on a peer-dep negotiation. - if is_vendored_pin(pinned_spec): + # Vendored grammars are classified by manifest membership (NOT a file: pin + # heuristic — they aren't in package.json at all, the #858 misrouting bug). + # Their readiness is governed by the vendored ABI, read from the repo, not a + # peer-dep negotiation. npm-latest columns get sentinels. + if name in VENDORED_NAMES: v = vendored_drift_summary(name, upstream_repo, upstream_branch, parser_path) v["pinned_spec"] = pinned_spec - # Three-state classification: in-range, out-of-range, or - # not-introspectable (e.g. tree-sitter-swift ships only - # prebuilt .node binaries, no parser.c — assume compatible). + hold = VENDORED[name].get("hold") + v["hold"] = hold + # Three-state ABI classification: in-range, out-of-range, or + # not-introspectable (e.g. a prebuilt-only vendor with no parser.c). if v["vendored_abi"] is None: v["target_compat"] = True v["abi_state"] = "prebuilt" @@ -567,13 +685,37 @@ def main() -> int: f"vendored `{name}`: ABI {v['vendored_abi']} outside target range " f"{target_abi_range[0]}..{target_abi_range[1]}" ) + # A held vendored grammar (e.g. tree-sitter-c, #1242/#858) is frozen below + # a runtime upgrade: in-range ABI or not, keep it a blocker until the hold + # (from the manifest) is lifted — same treatment as npm INTENTIONAL_PINS. + if hold: + v["target_compat"] = False + status = "Vendored — held" + # Compose with any out-of-range reason rather than overwriting it: + # both share the blockers[name] key, and the ABI-out-of-range + # detail would otherwise be lost from the blockers summary. + hold_reason = f"vendored `{name}` held: {hold}" + prior = blockers.get(name) + blockers[name] = f"{prior}; {hold_reason}" if prior else hold_reason + # Cell sentinels: never emit a bare "?". A vendored grammar's ABI is + # the real LANGUAGE_VERSION when introspectable, else a labeled token. + vendored_abi_cell = ( + str(v["vendored_abi"]) if v["vendored_abi"] is not None else "prebuilt" + ) + # A None upstream ABI means the upstream parser.c couldn't be read — + # either it is generated at build time (e.g. swift) or the fetch + # missed. We can't tell which here, so use a neutral label rather + # than asserting "generated at build". Never a bare "?". + upstream_abi_cell = ( + str(v["upstream_abi"]) if v["upstream_abi"] is not None else "n/a" + ) # Keep vendored grammars in the raw matrix so the workflow's - # row-diff change-detection picks up status transitions on - # them too. npm-only columns get sentinels. + # row-diff change-detection picks up status transitions on them too. + # npm-only columns get sentinels. raw_matrix.append( f"| `{name}` | {pinned_spec} | (vendored) | (vendored) | " f"{'Yes' if v['target_compat'] else '**No**'} | " - f"{v['vendored_abi'] or '?'} | {v['upstream_abi'] or '?'} | {status} |" + f"{vendored_abi_cell} | {upstream_abi_cell} | {status} |" ) vendored_grammars.append(v) continue @@ -593,7 +735,7 @@ def main() -> int: peer_optional = ts_meta.get("optional", False) if peer_range else True if fetch_failed: - peer_display = "? (fetch failed)" + peer_display = f"n/a ({miss_label})" target_compat = False current_compat = False else: @@ -609,7 +751,9 @@ def main() -> int: # Fallback to default location. installed_parser = GITNEXUS_DIR / "node_modules" / name / "src" / "parser.c" installed_abi = extract_language_version(installed_parser) - abi_display = str(installed_abi) if installed_abi else "?" + # Labeled sentinel, never a bare "?": CI's `npm ci` populates node_modules, + # but if it's absent say so plainly rather than leaving a placeholder (#858). + abi_display = str(installed_abi) if installed_abi else "n/a (not installed)" # Check upstream (main/master branch) ABI for unreleased work. upstream_url = ( @@ -618,23 +762,19 @@ def main() -> int: ) upstream_text = fetch_text(upstream_url) upstream_abi = extract_abi_from_text(upstream_text) if upstream_text else None - upstream_abi_display = str(upstream_abi) if upstream_abi else "?" + upstream_abi_display = str(upstream_abi) if upstream_abi else "n/a" # Status text + upstream-progress detection. The Status column # values are preserved as-is to keep the workflow's row-diff # change-detection working on the raw matrix below. upstream_progress: str | None = None if fetch_failed: - status = "Unknown (fetch failed)" - blockers[name] = f"`{name}`: npm registry fetch failed — could not verify peer dep" + status = f"Unknown ({miss_label})" + reason = "checks skipped (offline)" if OFFLINE else "npm registry fetch failed" + blockers[name] = f"`{name}`: {reason} — could not verify peer dep" elif name in INTENTIONAL_PINS: - # An intentional pin is, by definition, a held-back grammar: - # whatever npm-latest's peer dep says, our shipped version is - # the one whose ABI/peer must accept the target runtime, and - # the pin entry exists precisely because it does not. Treat - # it as a blocker until the pin is lifted (entry removed from - # INTENTIONAL_PINS), at which point this grammar falls back - # to standard classification on the next run. + # A held-back grammar: treated as a blocker until the pin is lifted + # (entry removed from INTENTIONAL_PINS), then reclassified next run. status = "Intentionally pinned" blockers[name] = ( f"`{name}` intentionally pinned at `{pinned_spec}` " @@ -675,8 +815,11 @@ def main() -> int: pinned_spec = pinned_versions.get(name, "—") compat_icon = "Yes" if target_compat else "**No**" + # "?" stays the internal fetch-failed sentinel (compared above); render a + # labeled token in the matrix so the report never shows a bare "?" (#858). + npm_version_cell = f"n/a ({miss_label})" if npm_version == "?" else npm_version raw_matrix.append( - f"| `{name}` | {pinned_spec} | {npm_version} | {peer_display} | " + f"| `{name}` | {pinned_spec} | {npm_version_cell} | {peer_display} | " f"{compat_icon} | {abi_display} | {upstream_abi_display} | {status} |" ) @@ -726,7 +869,8 @@ def main() -> int: lines.append(f"- {len(by_bucket['waiting'])} waiting on an upstream npm release") lines.append(f"- {len(by_bucket['blocked'])} blocked on upstream (no fix even on main)") if by_bucket['fetch_failed']: - lines.append(f"- {len(by_bucket['fetch_failed'])} could not be checked (npm registry unreachable)") + why = "checks skipped in offline mode" if OFFLINE else "npm registry unreachable" + lines.append(f"- {len(by_bucket['fetch_failed'])} could not be checked ({why})") if bump_now: lines.append( f"- **{len(bump_now)} bump candidate(s) you can take TODAY** (npm-latest " @@ -745,7 +889,7 @@ def main() -> int: lines.append("") for r in sorted(bump_now, key=lambda r: r["name"]): lines.append( - f"- `{r['name']}`: `{r['pinned_spec']}` → `{r['npm_version']}` " + f"- `{r['name']}`: `{r['pinned_spec']}` → `{r['npm_version_label']}` " f"(peer `{r['peer_range'] or 'none'}`)" ) lines.append("") @@ -768,7 +912,7 @@ def main() -> int: "These grammars' npm-latest peer dep already accepts the target runtime. No action needed for the upgrade.", by_bucket["ready"], lambda r: ( - f"- `{r['name']}` — pinned `{r['pinned_spec']}`, npm latest `{r['npm_version']}`" + f"- `{r['name']}` — pinned `{r['pinned_spec']}`, npm latest `{r['npm_version_label']}`" + (" _(also a bump candidate — see above)_" if r["bump_now"] else "") ), ) @@ -784,7 +928,7 @@ def main() -> int: reason = INTENTIONAL_PINS.get(r["name"], "(no rationale recorded)") lines.append( f"- `{r['name']}` pinned at `{r['pinned_spec']}` " - f"(npm latest `{r['npm_version']}`)\n {reason}" + f"(npm latest `{r['npm_version_label']}`)\n {reason}" ) lines.append("") @@ -794,7 +938,7 @@ def main() -> int: "We can move forward as soon as upstream cuts a release.", by_bucket["waiting"], lambda r: ( - f"- `{r['name']}@{r['npm_version']}` — peer `{r['peer_range'] or 'none'}`. " + f"- `{r['name']}@{r['npm_version_label']}` — peer `{r['peer_range'] or 'none'}`. " f"_{r['upstream_progress']}_" ), ) @@ -804,90 +948,23 @@ def main() -> int: "Peer dep is too tight on both the latest npm release and on upstream main. " "These need an upstream issue/PR before we can proceed.", by_bucket["blocked"], - lambda r: ( - f"- `{r['name']}@{r['npm_version']}` — peer `{r['peer_range'] or 'none'}`" - + (" _(vendored)_" if r["is_vendored"] else "") - ), + lambda r: f"- `{r['name']}@{r['npm_version_label']}` — peer `{r['peer_range'] or 'none'}`", ) _emit_bucket( "Could not check", - "npm registry fetch failed for these grammars. Re-run the workflow to retry.", + ( + "Checks were skipped because the report ran in `--offline` mode. " + "Re-run online to verify these grammars." + if OFFLINE + else "npm registry fetch failed for these grammars. Re-run the workflow to retry." + ), by_bucket["fetch_failed"], lambda r: f"- `{r['name']}` (pinned `{r['pinned_spec']}`)", ) # ── Vendored parsers ──────────────────────────────────────────── - if vendored_grammars: - lines.append(md_h(f"Vendored parsers ({len(vendored_grammars)})", 2)) - lines.append( - "These grammars ship from `gitnexus/vendor/` rather than the npm " - "registry. Their compatibility is governed by the **vendored " - "ABI** (must lie in the target runtime's range), not by a peer-" - "dep negotiation. The rationale for each vendored copy lives in " - "its own `package.json` `_vendoredBy` field." - ) - lines.append("") - for v in sorted(vendored_grammars, key=lambda v: v["name"]): - sync_label = ( - "in sync with upstream" if v["in_sync"] else "diverged from upstream" - ) - if v["abi_state"] == "in_range": - abi_label = f"ABI `{v['vendored_abi']}` (in target range)" - elif v["abi_state"] == "prebuilt": - abi_label = "ABI `prebuilt` (binary-only vendor, source not introspectable)" - else: - abi_label = ( - f"ABI `{v['vendored_abi']}` (**outside** target range " - f"{target_abi_range[0]}..{target_abi_range[1]})" - ) - upstream_abi_str = ( - f"ABI `{v['upstream_abi']}`" if v["upstream_abi"] else "ABI `?`" - ) - lines.append( - f"- **`{v['name']}`** `{v['vendored_version']}` — {abi_label}, " - f"upstream `{v['upstream_repo']}@{v['upstream_sha']}` " - f"{upstream_abi_str} · {sync_label}" - ) - if v["vendored_by"]: - # Show the first sentence — vendor package.json fields tend - # to start with the rationale and tail off into install- - # script breadcrumbs that aren't useful in this report. - rationale = _first_sentence(v["vendored_by"]) - lines.append(f" - **Why vendored:** {rationale}") - # Action computation: needs regen iff upstream ABI exceeds - # vendored AND is still within target range. If upstream ABI - # exceeds the target, that's a runtime-side blocker. For - # prebuilt-only vendors we can't drive this from source ABI; - # the action is a manual upstream-binary refresh, surfaced - # via the in-sync flag instead. - if v["abi_state"] == "prebuilt": - if not v["in_sync"]: - lines.append( - " - **Action:** check whether upstream has shipped a new " - "prebuilt release; this vendor ships binary-only artefacts." - ) - elif v["upstream_abi"] and v["vendored_abi"] and v["upstream_abi"] > v["vendored_abi"]: - if v["upstream_abi"] <= target_abi_range[1]: - lines.append( - f" - **Action:** after upgrading to tree-sitter@{TARGET_RUNTIME}, " - f"regenerate `parser.c` from upstream `{v['upstream_sha']}`." - ) - else: - lines.append( - f" - **Action:** wait for a runtime supporting ABI " - f"{v['upstream_abi']}; current target ({TARGET_RUNTIME}) only " - f"goes up to ABI {target_abi_range[1]}." - ) - blockers[f"vendored-{v['name']}-abi"] = ( - f"vendored {v['name']}: upstream ABI {v['upstream_abi']} outside target range" - ) - elif not v["in_sync"]: - lines.append( - " - **Action:** review upstream changes; vendored copy may " - "need a refresh (no ABI bump required)." - ) - lines.append("") + lines.extend(_render_vendored_section(vendored_grammars, target_abi_range, blockers)) # ── Raw matrix (for completeness + workflow row-diff) ──────────── lines.append(md_h("Full grammar matrix", 2)) @@ -911,6 +988,11 @@ if __name__ == "__main__": sys.stdout.reconfigure(encoding="utf-8") # type: ignore[attr-defined] except Exception: pass + # `--offline` skips all network so the readiness report renders hermetically + # (vendored ABIs from the repo; npm columns marked unverified). Useful for + # air-gapped runs and deterministic tests. + if "--offline" in sys.argv[1:]: + OFFLINE = True # `--assert-current` is the offline CI gate (#1922): assert every grammar's # ABI loads on the CURRENT runtime. Bare invocation keeps the original # target-runtime readiness report behaviour. diff --git a/.github/scripts/test_check_tree_sitter_upgrade_readiness.py b/.github/scripts/test_check_tree_sitter_upgrade_readiness.py new file mode 100644 index 000000000..c36e68d07 --- /dev/null +++ b/.github/scripts/test_check_tree_sitter_upgrade_readiness.py @@ -0,0 +1,394 @@ +#!/usr/bin/env python3 +"""Tests for check-tree-sitter-upgrade-readiness.py. + +Stdlib-only (``unittest`` + ``unittest.mock``) to match the script under test, +which is deliberately dependency-free so it runs on any vanilla runner. Run with: + + python3 -m unittest .github/scripts/test_check_tree_sitter_upgrade_readiness.py + +(pytest also discovers ``unittest.TestCase`` classes, so a future pytest CI job +picks these up unchanged.) + +These tests lock in the #858 fix: the 5 vendored grammars +(c/swift/kotlin/dart/proto) are classified from the shared manifest +(.github/vendored-grammars.json), their ABI is read from gitnexus/vendor/, +and the report never renders a bare ``?`` placeholder. All network is mocked. +""" +from __future__ import annotations + +import contextlib +import importlib.util +import io +import json +import pathlib +import re +from unittest import TestCase, main, mock + +# ── Load the hyphenated script as a module ─────────────────────────────── +_SCRIPTS_DIR = pathlib.Path(__file__).resolve().parent +_SCRIPT = _SCRIPTS_DIR / "check-tree-sitter-upgrade-readiness.py" +_REPO_ROOT = _SCRIPTS_DIR.parents[1] +_MANIFEST = _REPO_ROOT / ".github" / "vendored-grammars.json" + +_spec = importlib.util.spec_from_file_location("readiness_under_test", _SCRIPT) +readiness = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(readiness) # type: ignore[union-attr] + +# The exact row-diff regex the workflow's change-detection bot uses +# (.github/workflows/tree-sitter-upgrade-readiness.yml) — byte-identical so a matrix +# format change that would silently break change-detection fails here. Group 2 is +# ONLY the Status cell ([^|]+? before the final `|$`). +_ROW_DIFF_RE = re.compile(r"\| `(tree-sitter-[^`]+)` \|.*\| ([^|]+?) \|$", re.M) + + +def _physical_vendor_grammars() -> set[str]: + vendor = _REPO_ROOT / "gitnexus" / "vendor" + return { + p.name + for p in vendor.iterdir() + if p.is_dir() and p.name.startswith("tree-sitter-") + } + + +def _render_report() -> tuple[str, int]: + """Run main() with network mocked to mirror PRODUCTION; return (md, exit_code). + + - npm grammars resolve to a permissive "Ready" peer dep, so the ONLY blocker + left is the held vendored tree-sitter-c — letting us assert the hold is + load-bearing (exit code stays non-zero because of it). + - npm_view_json records its calls so we can prove vendored grammars are never + npm-queried. + - fetch_text mirrors the real workflow: upstream parser.c resolves to a real + ABI (committed upstream), commit endpoints return a sha — EXCEPT swift's + upstream, whose parser.c is generated at build time and so is unreachable + (None). That single miss exercises the labeled-sentinel path; every other + cell must be a real value, never a bare '?'. + """ + npm_calls: list[str] = [] + + def fake_npm_view_json(pkg: str): + npm_calls.append(pkg) + return {"version": "9.9.9", "peerDependencies": {"tree-sitter": "^0.25.0"}} + + def fake_fetch_text(url: str, timeout: int = 8): + if "parser.c" in url: + # swift's upstream parser.c is generated at build time → unreachable; + # the others ship a committed parser.c. + if "alex-pinkus" in url: + return None + return "#define LANGUAGE_VERSION 14\n#define STATE_COUNT 1\n" + if "/commits/" in url: + return json.dumps({"sha": "0123456789abcdef"}) + # package.json (relaxed-peer probe) etc. — not needed for these assertions. + return None + + buf = io.StringIO() + with mock.patch.object(readiness, "npm_view_json", side_effect=fake_npm_view_json), \ + mock.patch.object(readiness, "fetch_text", side_effect=fake_fetch_text), \ + contextlib.redirect_stdout(buf): + code = readiness.main() + report = buf.getvalue() + _render_report.last_npm_calls = npm_calls # type: ignore[attr-defined] + return report, code + + +class ManifestClassification(TestCase): + def test_manifest_matches_physical_vendor_dirs(self): + """Consistency guard: the manifest set == the gitnexus/vendor/tree-sitter-* + dirs. Vendoring a grammar without a manifest entry (or vice-versa) fails — + this is what keeps the two tree-sitter workflows aligned (#858).""" + manifest_names = { + g["name"] + for g in json.loads(_MANIFEST.read_text())["grammars"].values() + } + self.assertEqual(manifest_names, _physical_vendor_grammars()) + + def test_vendored_names_loaded_from_manifest(self): + self.assertEqual(set(readiness.VENDORED_NAMES), _physical_vendor_grammars()) + # npm-installed grammars must NOT be classified vendored. + self.assertNotIn("tree-sitter-cpp", readiness.VENDORED_NAMES) + self.assertNotIn("tree-sitter-go", readiness.VENDORED_NAMES) + + def test_c_carries_a_hold_cpp_does_not(self): + self.assertTrue(readiness.VENDORED["tree-sitter-c"]["hold"]) + self.assertNotIn("tree-sitter-c", readiness.INTENTIONAL_PINS) + # cpp stays an npm intentional pin. + self.assertIn("tree-sitter-cpp", readiness.INTENTIONAL_PINS) + + def test_vendored_names_are_a_subset_of_GRAMMARS(self): + # The report + --assert-current iterate the hardcoded GRAMMARS dict for + # upstream-drift coords. A vendored grammar present in the manifest but + # missing from GRAMMARS would be silently dropped from both — re-creating + # the cross-workflow divergence the manifest exists to kill (#858). Guard it. + missing = set(readiness.VENDORED_NAMES) - set(readiness.GRAMMARS) + self.assertEqual(missing, set(), f"manifest grammars missing from GRAMMARS: {missing}") + + def test_missing_manifest_raises_a_clear_error(self): + import pathlib + import tempfile + + with tempfile.TemporaryDirectory() as d: + with mock.patch.object(readiness, "REPO_ROOT", pathlib.Path(d)): + with self.assertRaises(SystemExit) as ctx: + readiness.load_vendored_manifest() + self.assertIn("vendored-grammars manifest", str(ctx.exception)) + + def test_malformed_manifest_raises_a_clear_error(self): + import pathlib + import tempfile + + with tempfile.TemporaryDirectory() as d: + gh = pathlib.Path(d) / ".github" + gh.mkdir() + (gh / "vendored-grammars.json").write_text("{ not valid json", encoding="utf-8") + with mock.patch.object(readiness, "REPO_ROOT", pathlib.Path(d)): + with self.assertRaises(SystemExit) as ctx: + readiness.load_vendored_manifest() + self.assertIn("not valid JSON", str(ctx.exception)) + + def test_path_traversal_grammar_name_is_rejected(self): + import pathlib + import tempfile + + bad = '{"grammars": {"evil": {"name": "../etc"}}}' + with tempfile.TemporaryDirectory() as d: + gh = pathlib.Path(d) / ".github" + gh.mkdir() + (gh / "vendored-grammars.json").write_text(bad, encoding="utf-8") + with mock.patch.object(readiness, "REPO_ROOT", pathlib.Path(d)): + with self.assertRaises(SystemExit) as ctx: + readiness.load_vendored_manifest() + self.assertIn("invalid grammar name", str(ctx.exception)) + + +class AssertCurrent(TestCase): + """The offline #1922 ABI gate (--assert-current) must stay hermetic — it reads + vendored ABIs from the repo, never the network. (Regression guard: a prior + revision routed vendored grammars through vendored_drift_summary, which fetches + upstream parser.c + commit sha, silently breaking the 'hermetic and offline' + contract — #858 review.)""" + + def _run_assert_current(self): + import urllib.request + + def explode(*a, **k): + raise AssertionError("--assert-current attempted a network call") + + buf = io.StringIO() + with mock.patch.object(urllib.request, "urlopen", side_effect=explode), \ + contextlib.redirect_stdout(buf): + code = readiness.assert_current() + return buf.getvalue(), code + + def test_assert_current_is_network_free_and_passes(self): + report, code = self._run_assert_current() # raises if any urlopen fires + self.assertEqual(code, 0) + # All 5 vendored grammars are introspected from the repo (ABI 14), not skipped. + for name in readiness.VENDORED_NAMES: + self.assertIn(f"{name}: vendored ABI", report) + + def test_assert_current_fails_an_out_of_range_vendored_abi(self): + # vendored_abi_from_repo is the local-read injection point: force one + # grammar out of the current runtime's ABI window and assert the gate trips. + real = readiness.vendored_abi_from_repo + + def fake(name, parser_path): + return 99 if name == "tree-sitter-dart" else real(name, parser_path) + + import urllib.request + buf = io.StringIO() + with mock.patch.object(readiness, "vendored_abi_from_repo", side_effect=fake), \ + mock.patch.object(urllib.request, "urlopen", side_effect=AssertionError("network")), \ + contextlib.redirect_stdout(buf): + code = readiness.assert_current() + self.assertEqual(code, 1) + self.assertIn("tree-sitter-dart", buf.getvalue()) + self.assertIn("outside current runtime range", buf.getvalue()) + + +class ReportRendering(TestCase): + @classmethod + def setUpClass(cls): + cls.report, cls.code = _render_report() + cls.rows = dict(_ROW_DIFF_RE.findall(cls.report)) + + def test_no_bare_question_mark_anywhere(self): + # The only legitimate '?' is the "Satisfies 0.25?" column header. + sanitized = self.report.replace("Satisfies 0.25?", "Satisfies 0.25") + self.assertNotIn("?", sanitized, "report still contains a bare '?' placeholder") + + def test_malformed_npm_version_renders_unknown_in_prose_not_bare_question(self): + # A successful (200) npm /latest response that omits `version` leaves + # npm_version == "?"; the grammar is still bucketed (fetch did not fail), so + # its disposition PROSE line must show the labeled sentinel, never a bare '?'. + def fake_npm(pkg: str): + if pkg == "tree-sitter-go": + return {"peerDependencies": {"tree-sitter": "^0.25.0"}} # no 'version' + return {"version": "9.9.9", "peerDependencies": {"tree-sitter": "^0.25.0"}} + + def fake_fetch(url: str, timeout: int = 8): + if "parser.c" in url and "alex-pinkus" not in url: + return "#define LANGUAGE_VERSION 14\n" + if "/commits/" in url: + return json.dumps({"sha": "0123456789abcdef"}) + return None + + buf = io.StringIO() + with mock.patch.object(readiness, "npm_view_json", side_effect=fake_npm), \ + mock.patch.object(readiness, "fetch_text", side_effect=fake_fetch), \ + contextlib.redirect_stdout(buf): + readiness.main() + report = buf.getvalue() + sanitized = report.replace("Satisfies 0.25?", "Satisfies 0.25") + self.assertNotIn("?", sanitized) + # The Ready bucket prose line for go shows the labeled 'unknown', not '?'. + self.assertRegex(report, r"`tree-sitter-go`.*npm latest `unknown`") + + def test_every_vendored_grammar_shows_numeric_abi_not_question_mark(self): + for name in readiness.VENDORED_NAMES: + row = self._matrix_row(name) + cells = [c.strip() for c in row.strip().strip("|").split("|")] + abi_cell = cells[5] # Grammar|Pinned|npm|Peer|Satisfies|ABI|UpstreamABI|Status + self.assertRegex( + abi_cell, r"^\d+$", + f"{name} ABI cell is '{abi_cell}', expected a number (read from vendor/)", + ) + + def test_proto_is_never_npm_queried(self): + # github-only vendored grammars must skip the npm peer-dep path entirely, + # which is what removes the old "? (fetch failed)" for tree-sitter-proto. + self.assertNotIn("tree-sitter-proto", _render_report.last_npm_calls) + self.assertNotIn("tree-sitter-dart", _render_report.last_npm_calls) + self.assertNotIn("Could not check", self.report) + self.assertNotIn("fetch failed", self.report) + + def test_held_c_renders_held_and_keeps_exit_nonzero(self): + # Status is the last matrix cell (the row-diff regex captures the whole + # tail, not just status, so read the cell directly). + cells = [c.strip() for c in self._matrix_row("tree-sitter-c").strip().strip("|").split("|")] + self.assertEqual(cells[-1], "Vendored — held") + self.assertIn("**Held:**", self.report) + # With every npm grammar mocked to "Ready", the ONLY remaining blocker is + # the held c — so a non-zero exit proves the hold is treated as a blocker. + self.assertEqual(self.code, 1) + + def test_upstream_abi_miss_uses_labeled_sentinel(self): + # swift's upstream parser.c is unreachable (mocked None), so its + # upstream-ABI cell is the labeled 'n/a' token, never a bare '?'. + cells = [c.strip() for c in self._matrix_row("tree-sitter-swift").strip().strip("|").split("|")] + self.assertEqual(cells[6], "n/a") # Upstream ABI column + + def test_row_diff_regex_captures_all_fifteen_grammar_statuses(self): + # The change-detection bot keys on this regex: group 1 = grammar name, + # group 2 = the Status cell ONLY (not the whole tail). It must match every + # row after the format change so status transitions keep being detected. + self.assertEqual(len(self.rows), 15) + for name in readiness.VENDORED_NAMES: + self.assertIn(name, self.rows) + # group 2 is the Status cell — held c renders exactly "Vendored — held", + # and no captured status contains a pipe (proves cell-scoped capture). + self.assertEqual(self.rows["tree-sitter-c"], "Vendored — held") + for status in self.rows.values(): + self.assertNotIn("|", status) + + def _matrix_row(self, name: str) -> str: + for line in self.report.splitlines(): + if line.startswith(f"| `{name}` |"): + return line + # Explicit terminating raise (not self.fail, which CodeQL doesn't model as + # NoReturn) so the function has no implicit fall-through return (CodeQL 754). + raise AssertionError(f"no matrix row for {name}") + + +class OfflineMode(TestCase): + """--offline must render the report touching ZERO network — vendored ABIs come + from the repo, npm columns are marked unverified. This is what makes the + network-dependent report deterministically testable in air-gapped CI.""" + + def _render_offline(self): + import urllib.request + + def explode(*a, **k): + raise AssertionError("network call attempted in --offline mode") + + buf = io.StringIO() + with mock.patch.object(readiness, "OFFLINE", True), \ + mock.patch.object(urllib.request, "urlopen", side_effect=explode), \ + contextlib.redirect_stdout(buf): + code = readiness.main() + return buf.getvalue(), code + + def test_offline_touches_no_network_and_still_renders(self): + report, code = self._render_offline() # raises if any urlopen fires + self.assertIn("Offline mode", report) + # Vendored grammars are introspected from the repo → real ABI 14, not a miss. + for name in readiness.VENDORED_NAMES: + row = next(l for l in report.splitlines() if l.startswith(f"| `{name}` |")) + cells = [c.strip() for c in row.strip().strip("|").split("|")] + self.assertRegex(cells[5], r"^\d+$", f"{name} vendored ABI missing offline") + + def test_offline_marks_npm_grammars_offline_not_fetch_failed(self): + report, _ = self._render_offline() + self.assertIn("(offline)", report) + self.assertNotIn("fetch failed", report) # honest: skipped, not failed + + def test_offline_report_has_no_bare_question_mark(self): + report, _ = self._render_offline() + sanitized = report.replace("Satisfies 0.25?", "Satisfies 0.25") + self.assertNotIn("?", sanitized) + + +class VendoredAbiBranches(TestCase): + """main()'s vendored-ABI classification reads through vendored_abi_from_repo + (the same local-read seam --assert-current uses), so a single patch drives the + out-of-range and prebuilt-only branches that no real vendor dir can trigger + today (all ship parser.c at ABI 14).""" + + def _render_with_vendored_abi(self, override): + """Render main() with the standard production-faithful network mock plus a + vendored_abi_from_repo override (dict: name -> int|None; others read real).""" + real = readiness.vendored_abi_from_repo + + def abi_seam(name, parser_path): + return override[name] if name in override else real(name, parser_path) + + def fake_npm(pkg): + return {"version": "9.9.9", "peerDependencies": {"tree-sitter": "^0.25.0"}} + + def fake_fetch(url, timeout=8): + if "parser.c" in url and "alex-pinkus" not in url: + return "#define LANGUAGE_VERSION 14\n" + if "/commits/" in url: + return json.dumps({"sha": "0123456789abcdef"}) + return None + + buf = io.StringIO() + with mock.patch.object(readiness, "vendored_abi_from_repo", side_effect=abi_seam), \ + mock.patch.object(readiness, "npm_view_json", side_effect=fake_npm), \ + mock.patch.object(readiness, "fetch_text", side_effect=fake_fetch), \ + contextlib.redirect_stdout(buf): + code = readiness.main() + return buf.getvalue(), code + + def _row(self, report, name): + line = next(l for l in report.splitlines() if l.startswith(f"| `{name}` |")) + return [c.strip() for c in line.strip().strip("|").split("|")] + + def test_out_of_range_vendored_abi_is_a_blocker(self): + # Force tree-sitter-dart's vendored ABI outside the target range (13–15). + report, code = self._render_with_vendored_abi({"tree-sitter-dart": 99}) + cells = self._row(report, "tree-sitter-dart") + self.assertEqual(cells[-1], "Vendored (ABI out of range)") + self.assertEqual(cells[5], "99") + self.assertEqual(code, 1) # out-of-range vendored grammar is a blocker + + def test_prebuilt_only_vendored_abi_renders_prebuilt_not_question(self): + # vendored_abi None (a future binary-only vendor with no parser.c). + report, _ = self._render_with_vendored_abi({"tree-sitter-dart": None}) + cells = self._row(report, "tree-sitter-dart") + self.assertEqual(cells[5], "prebuilt") # labeled, never a bare '?' + self.assertEqual(cells[4], "Yes") # prebuilt is assumed target-compatible + + +if __name__ == "__main__": + main() diff --git a/.github/scripts/update-vendored-grammars.mjs b/.github/scripts/update-vendored-grammars.mjs index 957200e77..769310bd7 100644 --- a/.github/scripts/update-vendored-grammars.mjs +++ b/.github/scripts/update-vendored-grammars.mjs @@ -42,17 +42,52 @@ const COMPATIBLE_ABI = new Set([13, 14]); // tree-sitter@0.21.1 LANGUAGE_VERSION // github grammars (no usable npm release) track the default branch HEAD. A `hold` // reason makes a grammar report-only: updates are detected + surfaced but never // auto-applied (c is ABI-pinned and must not move without a runtime upgrade). -const GRAMMARS = { - c: { - name: 'tree-sitter-c', - npm: 'tree-sitter-c', - hold: 'ABI-pinned at 0.21.4 (#1242/#858) — needs a tree-sitter runtime upgrade before bumping', - }, - swift: { name: 'tree-sitter-swift', npm: 'tree-sitter-swift' }, - kotlin: { name: 'tree-sitter-kotlin', npm: 'tree-sitter-kotlin' }, - dart: { name: 'tree-sitter-dart', github: 'UserNobody14/tree-sitter-dart' }, - proto: { name: 'tree-sitter-proto', github: 'coder3101/tree-sitter-proto' }, -}; +// +// The vendored set lives in .github/vendored-grammars.json — the SHARED source of +// truth this monitor and .github/scripts/check-tree-sitter-upgrade-readiness.py both +// read, so the two tree-sitter workflows can never disagree about which grammars are +// vendored or where their upstream lives. We reshape the manifest's +// `{ upstream: { npm | github } }` form into the flat `{ npm? , github? }` shape the +// rest of this script consumes. This is a local file read (import-safe, no network). +const MANIFEST = path.join(REPO_ROOT, '.github', 'vendored-grammars.json'); +// `raw` is injectable for testing; production reads the manifest file. +function loadManifestGrammars(raw = null) { + if (raw === null) { + // Fail loud with a pointer, not a bare ENOENT/SyntaxError: this runs at import. + try { + raw = JSON.parse(fs.readFileSync(MANIFEST, 'utf8')); + } catch (e) { + throw new Error( + `Could not load the vendored-grammars manifest at ${MANIFEST} ` + + `(shared source of truth — see CONTRIBUTING.md → CI automation contracts): ${e.message}`, + ); + } + } + return Object.fromEntries( + Object.entries(raw.grammars || {}).map(([key, g]) => { + if (!g.name) + throw new Error(`manifest entry '${key}' is missing a 'name' field (${MANIFEST})`); + // Defense-in-depth: `name` is joined into gitnexus/vendor/ paths (and + // apply() WRITES there), so reject anything that isn't a plain grammar name + // before it can traverse the filesystem (#2187). + if (!/^tree-sitter-[a-z0-9-]+$/.test(g.name)) + throw new Error( + `manifest entry '${key}' has an invalid grammar name '${g.name}' ` + + `(must match tree-sitter-[a-z0-9-]+)`, + ); + return [ + key, + { + name: g.name, + ...(g.upstream?.npm ? { npm: g.upstream.npm } : {}), + ...(g.upstream?.github ? { github: g.upstream.github } : {}), + ...(g.hold ? { hold: g.hold } : {}), + }, + ]; + }), + ); +} +const GRAMMARS = loadManifestGrammars(); const sh = (cmd, args, opts = {}) => execFileSync(cmd, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], ...opts }).trim(); @@ -62,6 +97,26 @@ const clean = (v) => .replace(/^[v^~]/, '') .trim(); +// Shared "is the candidate newer than what we ship?" check, used by BOTH detect() +// and apply() so they can never disagree. up.version is the comparable identity for +// both kinds: a plain semver for npm, and the `-g` provenance string for +// github (which apply() also writes to package.json). detect() previously compared +// the bare sha7 for github, so after the bot re-vendored a github grammar once it +// reported a perpetual false "update available" while apply() saw "already current" +// (#2187 review). Comparing up.version on both sides removes that asymmetry. +const isNewer = (up, have) => !have || up.version !== have; + +// apply() throws this (instead of calling process.exit) so its error branches are +// exercisable in-process by tests; the CLI entrypoint maps `.code` back to the +// original exit code, keeping the monitor's subprocess contract identical (#2187). +class ApplyExit extends Error { + constructor(message, code) { + super(message); + this.name = 'ApplyExit'; + this.code = code; + } +} + function vendoredVersion(g) { const p = path.join(VENDOR, g.name, 'package.json'); return clean(JSON.parse(fs.readFileSync(p, 'utf8')).version); @@ -136,22 +191,30 @@ function readAbi(srcRoot) { return null; // unknown (e.g. parser.c only generated at build time) } -function detect() { +// `deps` injects the network/filesystem seams (vendoredVersion / resolveUpstream / +// fetchSource / readAbi) so the classification logic — newer-detection, the ABI +// gate, and the policy-hold gate — can be unit-tested offline with fixtures, never +// touching live npm/GitHub. Production passes nothing and gets the real functions. +function detect(deps = {}) { + const getVendored = deps.vendoredVersion || vendoredVersion; + const resolveUp = deps.resolveUpstream || resolveUpstream; + const fetchSrc = deps.fetchSource || fetchSource; + const readAbiFn = deps.readAbi || readAbi; const report = []; for (const [key, g] of Object.entries(GRAMMARS)) { - const have = vendoredVersion(g); + const have = getVendored(g); let up; try { - up = resolveUpstream(g); + up = resolveUp(g); } catch (err) { report.push({ grammar: key, error: String(err.message || err) }); continue; } - const newer = up.kind === 'npm' ? up.version !== have : !have || up.ref.slice(0, 7) !== have; + const newer = isNewer(up, have); let abi = null; if (newer) { try { - abi = readAbi(fetchSource(g, up.ref)); + abi = readAbiFn(fetchSrc(g, up.ref)); } catch { /* fetch/abi best-effort; null = unknown */ } @@ -190,34 +253,48 @@ const copyFile = (srcRoot, dest, rel) => { * notice), LICENSE, and prebuilds/ (the build workflow refreshes those). Bumps the * stripped vendor package.json version + provenance — never re-introduces * scripts/dependencies (#836/#1728). Returns the new version. + * + * opts.dryRun resolves + ABI-validates the candidate but writes NOTHING — it logs + * what it would re-vendor and returns the version, so the flow can be rehearsed + * (locally or in CI) without mutating gitnexus/vendor/. opts.deps injects the + * network/fs seams for offline testing (same shape as detect()). */ -function apply(key) { +function apply(key, opts = {}) { + const dryRun = opts.dryRun || false; + const deps = opts.deps || {}; + const getVendored = deps.vendoredVersion || vendoredVersion; + const resolveUp = deps.resolveUpstream || resolveUpstream; + const fetchSrc = deps.fetchSource || fetchSource; + const readAbiFn = deps.readAbi || readAbi; const g = GRAMMARS[key]; - if (!g) { - console.error(`unknown grammar '${key}'`); - process.exit(2); - } - if (g.hold) { - console.error( + if (!g) throw new ApplyExit(`unknown grammar '${key}'`, 2); + if (g.hold) + throw new ApplyExit( `${key}: report-only (${g.hold}); not auto-applied. Re-vendor manually if intended.`, + 3, ); - process.exit(3); - } - const have = vendoredVersion(g); - const up = resolveUpstream(g); - const newer = up.kind === 'npm' ? up.version !== have : !have || up.version !== have; + const have = getVendored(g); + const up = resolveUp(g); + const newer = isNewer(up, have); if (!newer) { + // Already current: nothing to apply. Return (exit 0 via the CLI) — NOT an error. console.error(`${key}: already current (${have}); nothing to apply.`); - process.exit(0); + return have; } - const srcRoot = fetchSource(g, up.ref); - const abi = readAbi(srcRoot); - if (abi == null || !COMPATIBLE_ABI.has(abi)) { - console.error( + const srcRoot = fetchSrc(g, up.ref); + const abi = readAbiFn(srcRoot); + if (abi == null || !COMPATIBLE_ABI.has(abi)) + throw new ApplyExit( `${key}: candidate ${up.version} is ABI ${abi ?? 'unknown'} — not tree-sitter@0.21.1 ` + `compatible (need 13/14); refusing to re-vendor. Handle manually.`, + 3, ); - process.exit(3); + + if (dryRun) { + console.log( + `${key}: [dry-run] would re-vendor ${g.name} → ${up.version} (ABI ${abi}); no files written.`, + ); + return up.version; } const dest = path.join(VENDOR, g.name); @@ -256,11 +333,31 @@ function apply(key) { // makes live network calls, so importing must be side-effect-free. const isMain = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href; if (isMain) { - if (process.argv[2] === '--apply') { - apply(process.argv[3]); + const args = process.argv.slice(2); + const dryRun = args.includes('--dry-run'); + if (args[0] === '--apply') { + // `--apply [--dry-run]` — --dry-run previews without writing. + // Map apply()'s thrown ApplyExit back to the original exit codes (0/2/3) so + // the monitor workflow's subprocess (which only distinguishes zero vs non-zero) + // sees identical behavior. + try { + apply(args[1], { dryRun }); + } catch (e) { + console.error(e.message); + process.exit(e instanceof ApplyExit ? e.code : 1); + } } else { process.stdout.write(JSON.stringify(detect(), null, 2) + '\n'); } } -export { detect, apply, resolveUpstream, readAbi, vendoredVersion, GRAMMARS, COMPATIBLE_ABI }; +export { + detect, + apply, + resolveUpstream, + readAbi, + vendoredVersion, + loadManifestGrammars, + GRAMMARS, + COMPATIBLE_ABI, +}; diff --git a/.github/vendored-grammars.json b/.github/vendored-grammars.json new file mode 100644 index 000000000..2777e26a0 --- /dev/null +++ b/.github/vendored-grammars.json @@ -0,0 +1,26 @@ +{ + "_comment": "Single source of truth for the VENDORED SET + policy holds, read by BOTH .github/scripts/update-vendored-grammars.mjs (weekly auto-PR bot) and .github/scripts/check-tree-sitter-upgrade-readiness.py (daily readiness report -> issue #858). The monitor also resolves each grammar's upstream from the `upstream` field here; the readiness report reads vendored ABIs from gitnexus/vendor//src/parser.c and keeps its own upstream-drift coords. A consistency-guard test asserts this set equals the gitnexus/vendor/tree-sitter-* directories. See CONTRIBUTING.md.", + "grammars": { + "c": { + "name": "tree-sitter-c", + "upstream": { "npm": "tree-sitter-c" }, + "hold": "ABI-pinned at 0.21.4 (#1242/#858) — needs a tree-sitter runtime upgrade before bumping" + }, + "swift": { + "name": "tree-sitter-swift", + "upstream": { "npm": "tree-sitter-swift" } + }, + "kotlin": { + "name": "tree-sitter-kotlin", + "upstream": { "npm": "tree-sitter-kotlin" } + }, + "dart": { + "name": "tree-sitter-dart", + "upstream": { "github": "UserNobody14/tree-sitter-dart" } + }, + "proto": { + "name": "tree-sitter-proto", + "upstream": { "github": "coder3101/tree-sitter-proto" } + } + } +} diff --git a/.github/workflows/grammar-update-monitor.yml b/.github/workflows/grammar-update-monitor.yml index ee14c093c..f6166de82 100644 --- a/.github/workflows/grammar-update-monitor.yml +++ b/.github/workflows/grammar-update-monitor.yml @@ -14,6 +14,13 @@ name: Vendored grammar update monitor # never auto-bumped — a maintainer re-vendors it deliberately after a runtime # upgrade. # +# The vendored set + per-grammar upstream coords + the tree-sitter-c hold live in +# .github/vendored-grammars.json — the SHARED source of truth this monitor and +# tree-sitter-upgrade-readiness.yml both read, so the two workflows can never +# disagree about which grammars are vendored (#858). This monitor additionally +# resolves each grammar's upstream from it; the readiness report reads vendored +# ABIs from gitnexus/vendor/ and keeps its own upstream-drift coords. +# # Concurrency convention: see CONTRIBUTING.md -> "GitHub Actions — Concurrency Convention". on: diff --git a/.github/workflows/tree-sitter-upgrade-readiness.yml b/.github/workflows/tree-sitter-upgrade-readiness.yml index 1eca8861d..bd887319f 100644 --- a/.github/workflows/tree-sitter-upgrade-readiness.yml +++ b/.github/workflows/tree-sitter-upgrade-readiness.yml @@ -1,12 +1,21 @@ name: Tree-sitter Upgrade Readiness # Monitors readiness for upgrading tree-sitter to 0.25.x. Tracks: -# 1. Peer-dep compatibility — can each grammar install cleanly with -# tree-sitter@0.25.0 without --legacy-peer-deps? -# 2. Vendored proto drift — has coder3101/tree-sitter-proto moved -# ahead of our vendored snapshot? +# 1. Peer-dep compatibility — can each NPM-installed grammar install cleanly +# with tree-sitter@0.25.0 without --legacy-peer-deps? +# 2. Vendored grammars — each grammar in .github/vendored-grammars.json +# (c/swift/kotlin/dart/proto) is classified by its vendored ABI, read +# straight from gitnexus/vendor//src/parser.c (NOT node_modules, +# which is never populated for vendored grammars — that mismatch is why +# the report used to render bare "?" placeholders, #858). # See .github/scripts/check-tree-sitter-upgrade-readiness.py for the logic. # +# .github/vendored-grammars.json is the SHARED source of truth for the vendored +# SET + policy holds: this readiness report and grammar-update-monitor.yml both +# read it, so the two workflows can never disagree about which grammars are +# vendored. (The monitor also resolves upstreams from it; this report keeps its +# own upstream-drift coords and reads vendored ABIs from gitnexus/vendor/.) +# # Concurrency convention: see CONTRIBUTING.md → "GitHub Actions — Concurrency Convention". on: @@ -18,6 +27,8 @@ on: pull_request: paths: - '.github/scripts/check-tree-sitter-upgrade-readiness.py' + - '.github/scripts/test_check_tree_sitter_upgrade_readiness.py' + - '.github/vendored-grammars.json' - '.github/workflows/tree-sitter-upgrade-readiness.yml' concurrency: @@ -28,14 +39,18 @@ permissions: contents: read jobs: - readiness: + report: name: Check upgrade readiness runs-on: ubuntu-latest timeout-minutes: 10 + # Least privilege: rendering the report needs no write. The issue mutation + # lives in the schedule-only `upsert-issue` job below, so PR runs (incl. forks) + # never receive `issues: write` (#2187 review). permissions: contents: read - # Needed to open/update the tracking issue on scheduled runs. - issues: write + outputs: + report: ${{ steps.readiness.outputs.report }} + exit_code: ${{ steps.readiness.outputs.exit_code }} steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -43,6 +58,17 @@ jobs: with: build: 'false' + # Guard the readiness script's logic (vendored classification, no bare "?", + # the manifest⇄vendor-dir consistency guard). Stdlib-only, so no extra deps; + # node_modules is populated by setup-gitnexus above, which the npm-path ABI + # reads need. Runs only on validation events (PR / manual), not the daily + # scheduled report. + - name: Run readiness script unit tests + if: github.event_name != 'schedule' + shell: bash + working-directory: .github/scripts + run: python3 -m unittest test_check_tree_sitter_upgrade_readiness -v + - name: Run upgrade readiness check id: readiness shell: bash @@ -54,10 +80,15 @@ jobs: code=$? set -e echo "exit_code=$code" >> "$GITHUB_OUTPUT" + # Unguessable per-run heredoc delimiter: the report includes the manifest's + # `hold` field, which a fork PR can edit — a fixed delimiter (e.g. DRIFT_EOF) + # in a hold value could close the heredoc early and inject $GITHUB_OUTPUT keys. + # A random hex delimiter the report cannot contain neutralizes that. + DELIM="DRIFT_EOF_$(openssl rand -hex 16)" { - echo 'report<> "$GITHUB_OUTPUT" echo "=== Report ===" cat drift-report.md @@ -69,13 +100,22 @@ jobs: run: | echo "::warning::Tree-sitter 0.25 upgrade has blockers. See job output for the full readiness report." - - name: Upsert tracking issue on scheduled runs - if: > - github.event_name == 'schedule' && - steps.readiness.outputs.exit_code != '0' + # Issue mutation is isolated here so `issues: write` is only ever granted on the + # scheduled run (never on PRs). Consumes the report + exit_code via job outputs. + upsert-issue: + name: Upsert tracking issue + needs: report + if: github.event_name == 'schedule' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + issues: write + steps: + - name: Upsert tracking issue on blockers + if: needs.report.outputs.exit_code != '0' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: - REPORT: ${{ steps.readiness.outputs.report }} + REPORT: ${{ needs.report.outputs.report }} with: script: | const title = 'Tree-sitter 0.25 upgrade readiness'; @@ -105,7 +145,11 @@ jobs: // | `tree-sitter-foo` | ... | Blocking | const parseRows = (md) => { const map = {}; - for (const m of md.matchAll(/\| `(tree-sitter-[^`]+)` \|.*?\| (\S+(?:\s\S+)*?) \|$/gm)) { + // Group 2 captures ONLY the Status cell ([^|]+? before the final + // `|$`), so change-detection fires on status transitions, not on + // unrelated cell drift (e.g. an upstream-ABI bump). Mirror this in + // _ROW_DIFF_RE in test_check_tree_sitter_upgrade_readiness.py. + for (const m of md.matchAll(/\| `(tree-sitter-[^`]+)` \|.*\| ([^|]+?) \|$/gm)) { map[m[1]] = m[2].trim(); } return map; @@ -152,10 +196,8 @@ jobs: core.info(`Opened issue #${created.number}`); } - - name: Close tracking issue on clean scheduled runs - if: > - github.event_name == 'schedule' && - steps.readiness.outputs.exit_code == '0' + - name: Close tracking issue on clean runs + if: needs.report.outputs.exit_code == '0' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | diff --git a/AGENTS.md b/AGENTS.md index 64d264126..285a12186 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,7 +83,7 @@ This project is indexed by GitNexus as **GitNexus** (26675 symbols, 35395 relati - **MUST run impact analysis before editing any symbol.** Before modifying a function, class, or method, run `impact({target: "symbolName", direction: "upstream"})` and report the blast radius (direct callers, affected processes, risk level) to the user. - **MUST run `detect_changes()` before committing** to verify your changes only affect expected symbols and execution flows. - **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. -- When exploring unfamiliar code, use `query({query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. +- When exploring unfamiliar code, use `query({search_query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. - When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `context({name: "symbolName"})`. ## Never Do diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b3319f172..4aa854a88 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -41,6 +41,8 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). | `route_map` | API route → handler → consumer mappings | | `tool_map` | MCP/RPC tool definitions and handlers | | `shape_check` | Response shape vs consumer property access mismatches | +| `explain` | Persisted taint findings (source→sink data flows) — needs `analyze --pdg` | +| `pdg_query` | Control/data dependence — CDG (`mode: controls`) / REACHING_DEF (`mode: flows`) — needs `analyze --pdg` | | `group_list` | List repo groups or details for one group | | `group_sync` | Rebuild group Contract Registry (`contracts.json`) and bridge graph | @@ -204,9 +206,17 @@ Language-agnostic scope-resolution resolver. This is the resolution path for eve Orchestrator: `runScopeResolution(input, provider)` in `scope-resolution/pipeline/run.ts`. Pipeline phase: `scopeResolutionPhase` in `scope-resolution/pipeline/phase.ts` — iterates the registered `SCOPE_RESOLVERS` over the worker-serialized `ParsedFile`s. (Per-language `emitScopeCaptures` hooks may reuse a cached Tree via the orchestrator's `treeCache`, but in worker-pool runs that cache is empty — Trees can't cross MessageChannels — so they consume the pre-extracted `ParsedFile` instead; § Performance notes.) -### Optional CFG/PDG emission (`--pdg`, #2081 M1) +### Optional CFG/PDG emission (`--pdg`, #2081–#2086) -On a `--pdg` run, the parse worker builds a per-function control-flow graph from the tree-sitter AST (`LanguageProvider.cfgVisitor`; TypeScript/JavaScript in M1) and serializes it onto `ParsedFile.cfgSideChannel` as plain data. Scope-resolution then emits `BasicBlock` nodes + `CFG` edges from that side-channel **inside Phase 4 of `runScopeResolution`, while the disk-backed ParsedFile store is still live** — the only window where the worker-built CFGs are loaded (the store is cleared right after the phase returns). A standalone post-`mro` phase would read an empty store, so the CFG emit deliberately lives in-phase, mirroring the `applyCaptureSideChannel` pattern. The opt-in is off by default (graph byte-identical), folded into the parse-cache key (a pdg-off warm cache is never reused on a `--pdg` run), and bounded by a per-function edge cap that logs any dropped edges. Edge *kind* (`seq`/`cond-true`/`loop-back`/…) rides in the `CFG` relationship's `reason` (CFG is a single `CodeRelation` type, not one type per kind). See `core/ingestion/cfg/`. +On a `--pdg` run the parse worker builds a per-function control-flow graph from the tree-sitter AST (`LanguageProvider.cfgVisitor`; TypeScript/JavaScript today) and serializes it onto `ParsedFile.cfgSideChannel` as plain data. Scope-resolution then emits the program-dependence layers from that side-channel **inside Phase 4 of `runScopeResolution`, while the disk-backed ParsedFile store is still live** — the only window where the worker-built CFGs are loaded (the store is cleared right after the phase returns). A standalone post-`mro` phase would read an empty store, so the emit deliberately lives in-phase, mirroring the `applyCaptureSideChannel` pattern. The opt-in is off by default (graph byte-identical), folded into the parse-cache key (a pdg-off warm cache is never reused on a `--pdg` run), and each layer is bounded by a per-function edge cap that logs any dropped edges. All layers are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table, keyed by `type`; there is **no** `Function → BasicBlock` edge — the symbol↔block join is reconstructed from the BasicBlock id prefix + line span. The layers build on each other: + +- **M1 — CFG** (#2081): `BasicBlock` nodes + `CFG` edges. Edge *kind* (`seq`/`cond-true`/`loop-back`/…) rides the `reason` column (CFG is one `CodeRelation` type, not one per kind). +- **M2 — REACHING_DEF** (#2082): GEN/KILL def→use data dependence from a pure fixpoint solver; the variable name rides `reason`. +- **M3/M4 — TAINTED / SANITIZES / TAINT_PATH** (#2083–#2084): intra- and inter-procedural taint (source→sink) — the `explain` tool's data. +- **M5 — CDG** (#2085): Ferrante control dependence over a Cooper–Harvey–Kennedy post-dominator tree (the EXIT-rooted reverse CFG); branch sense (`'T'`/`'F'`) rides `reason`. A CFG whose EXIT is unreachable from some block is skipped for CDG (post-dominance would be unsound) while its CFG/REACHING_DEF layers are kept. +- **M6 — read surface** (#2086): the `pdg_query` MCP tool answers "what gates X?" (CDG, `mode: controls`) and "where does Y flow?" (REACHING_DEF, `mode: flows`); `explain` is the taint consumer. Both are always anchored + `LIMIT`-bounded (LadybugDB has no rel-property index) and share one `resolveBlockAnchor` helper. These PDG edge types are deliberately kept out of the default `VALID_RELATION_TYPES` / web schema. + +See `core/ingestion/cfg/` (emit + the pure CFG / post-dominator / control-dependence / reaching-defs / taint passes) and `mcp/local/local-backend.ts` (`_pdgQueryImpl`, `_explainImpl`, the shared `resolveBlockAnchor`). ### `ScopeResolver` contract @@ -383,6 +393,8 @@ Defined in `lbug/schema.ts`. Separate node tables per type, single `CodeRelation **Relation types** (`CodeRelation.type`): CONTAINS, DEFINES, CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, ACCESSES, METHOD_OVERRIDES, METHOD_IMPLEMENTS, MEMBER_OF, STEP_IN_PROCESS, HANDLES_ROUTE, FETCHES, HANDLES_TOOL, ENTRY_POINT_OF. +**Optional `--pdg` additions** (off by default, opt-in via `gitnexus analyze --pdg`; see _Optional CFG/PDG emission_ above): a `BasicBlock` node table, plus the PDG relation types `CFG`, `REACHING_DEF`, `CDG`, `TAINTED`, `SANITIZES`, and `TAINT_PATH` on the same `CodeRelation` table. These are deliberately kept out of the default `VALID_RELATION_TYPES` / web graph schema — query them via `cypher`, `explain`, or `pdg_query`. + ## Embeddings and search **Embeddings** (`src/core/embeddings/`): Snowflake arctic-embed-xs (384D). Embeddable: File, Function, Class, Method, Interface. Incremental via SHA1 content hash. Separate `Embedding` table. diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b34046ad..1bf60be80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,16 +4,6 @@ All notable changes to GitNexus will be documented in this file. ## [Unreleased] -### Fixed - -- **Hook db-lock probe no longer strands unkillable `lsof`/`ps` orphans** — the probe's `lsof`/`ps` subprocesses are now wrapped in a self-tested coreutils `timeout`/`gtimeout` (`timeout -k 1 …`), so a hook SIGKILLed by the runner's 10s timeout can no longer leave `lsof` running forever (orphan lifetime bounded at ~3s); `acquireHookSlot` now also gates the probe itself, capping concurrent probes at 3 per repo. Opt out with `GITNEXUS_HOOK_TIMEOUT_PATH=disabled`. (#2163) - -### Changed -- Migrated from KuzuDB to LadybugDB v0.15 (`@ladybugdb/core`, `@ladybugdb/wasm-core`) -- Renamed all internal paths from `kuzu` to `lbug` (storage: `.gitnexus/kuzu` → `.gitnexus/lbug`) -- Added automatic cleanup of stale KuzuDB index files -- LadybugDB v0.15 requires explicit VECTOR extension loading for semantic search - ## [1.5.3] - 2026-04-01 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index f2bf1e487..7350cfb09 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,7 @@ This project is indexed by GitNexus as **GitNexus** (26675 symbols, 35395 relati - **MUST run impact analysis before editing any symbol.** Before modifying a function, class, or method, run `impact({target: "symbolName", direction: "upstream"})` and report the blast radius (direct callers, affected processes, risk level) to the user. - **MUST run `detect_changes()` before committing** to verify your changes only affect expected symbols and execution flows. - **MUST warn the user** if impact analysis returns HIGH or CRITICAL risk before proceeding with edits. -- When exploring unfamiliar code, use `query({query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. +- When exploring unfamiliar code, use `query({search_query: "concept"})` to find execution flows instead of grepping. It returns process-grouped results ranked by relevance. - When you need full context on a specific symbol — callers, callees, which execution flows it participates in — use `context({name: "symbolName"})`. ## Never Do diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 848884be4..278dd72d2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -144,6 +144,15 @@ Re-invoking `/autofix` after a successful apply is a safe no-op — the workflow **Sensitive paths.** The apply workflow refuses any patch that touches `.github/` (workflow files, CODEOWNERS, dependabot config). A malicious PR could ship a custom prettier or ESLint config that reformats workflow YAML; if accepted, those edits would be pushed under `contents: write` without human review. Apply formatter changes to files under `.github/` manually in a normal commit so they get the same review every other workflow change gets. +### Vendored tree-sitter grammars + +`.github/vendored-grammars.json` is the **single source of truth** for the vendored tree-sitter grammar **set** and each grammar's policy `hold` (the ones shipped from `gitnexus/vendor/` rather than installed from npm). It lists each grammar's name, upstream coords (`npm` or `github`), and any `hold`. The monitor resolves upstreams from it; the readiness report keeps its own upstream-drift coords and reads vendored ABIs from `gitnexus/vendor/`. Two workflows read it: + +- `grammar-update-monitor.yml` (`.github/scripts/update-vendored-grammars.mjs`) — weekly; opens auto-PRs re-vendoring ABI-compatible upstream updates. +- `tree-sitter-upgrade-readiness.yml` (`.github/scripts/check-tree-sitter-upgrade-readiness.py`) — daily; renders the tree-sitter-0.25 readiness report (issue #858), reading each vendored grammar's ABI from `gitnexus/vendor//src/parser.c`. + +Sharing the manifest keeps the two aligned: a consistency-guard test asserts the manifest set equals the `gitnexus/vendor/tree-sitter-*` directories. **When you vendor a new grammar (or remove one), update `.github/vendored-grammars.json` in the same change** — otherwise that guard fails CI and the readiness report regresses to `?` placeholders. + ## AI-assisted contributions If you use coding agents, follow project context files (e.g. `AGENTS.md`, `CLAUDE.md`) and avoid drive-by refactors unrelated to the issue. Prefer incremental, test-backed changes. diff --git a/README.md b/README.md index aea1079ee..ab01db810 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,7 @@ To configure MCP for your editor, run `npx gitnexus setup` once — or set it up ### MCP Setup -`gitnexus setup` auto-detects your editors and writes the correct global MCP config. You only need to run it once. +`gitnexus setup` auto-detects your editors and writes the correct global MCP config. You only need to run it once. To configure only selected integrations, pass `--coding-agent`/`-c` with a comma-separated list or repeat the option, for example `gitnexus setup -c cursor,codex`. ### Editor Support @@ -224,7 +224,7 @@ args = ["-y", "gitnexus@latest", "mcp"] ### CLI Commands ```bash -gitnexus setup # Configure MCP for your editors (one-time) +gitnexus setup # Configure MCP for detected editors (one-time; use -c to select) gitnexus uninstall # Preview removal of GitNexus MCP/skills/hooks (add --force to apply) gitnexus analyze [path] # Index a repository (or update stale index) gitnexus analyze --repair-fts # Fast path: rebuild/verify only FTS indexes on existing index data @@ -360,7 +360,7 @@ It is opt-in and a no-op without `UNDERSTAND_QUICKLY_TOKEN` — a fine-grained G | `group_query` | Search execution flows across all repos in a group | — | | `group_status` | Check staleness of repos in a group | — | -> When only one repo is indexed, the `repo` parameter is optional. With multiple repos, specify which one: `query({query: "auth", repo: "my-app"})`. +> When only one repo is indexed, the `repo` parameter is optional. With multiple repos, specify which one: `query({search_query: "auth", repo: "my-app"})`. **Resources** for instant context: @@ -738,7 +738,7 @@ gitnexus impact get_embeddings --uid "Function:src/embed.py:get_embeddings" # e ### Process-Grouped Search ``` -query({query: "authentication middleware"}) +query({search_query: "authentication middleware"}) processes: - summary: "LoginFlow" diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index 5274ef4c0..2cf133cd9 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -15,7 +15,10 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); const { acquireHookSlot } = require('./hook-lock.js'); -const { hasGitNexusDbLockedByGitNexusServer } = require('./hook-db-lock-probe.cjs'); +const { + hasGitNexusDbLockedByGitNexusServer, + resolveUnixGuardTimeout, +} = require('./hook-db-lock-probe.cjs'); const { formatAnalyzeCommand } = require('./resolve-analyze-cmd.cjs'); /** @@ -196,18 +199,90 @@ function extractPattern(toolName, toolInput) { return null; } +// Debounce for the unguarded-CLI diagnostic below (#2163 follow-up review): +// at most one line per (short-lived) hook process, even if a future change +// runs the CLI more than once. +let unguardedCliWarned = false; + /** * Spawn a gitnexus CLI command synchronously. * Detects binary on PATH once, then runs exactly once. * * SECURITY: Never use shell: true with user-controlled arguments. * On Windows, invoke gitnexus.cmd directly (no shell needed). + * + * Unix orphan containment (#2163 follow-up): the augment CLI is the + * longest-lived hook child (inner spawnSync timeout 7s locally, 12s via + * npx), so on Unix every CLI-running branch gets the same SIGKILL-surviving + * coreutils `timeout` wrapper as the probe's lsof/ps (the cheap which/where + * PATH check stays unwrapped). The wrapper budget is ceil(inner/1000)+1 + * seconds — STRICTLY greater than the inner spawnSync timeout, so on the + * supervised path Node's SIGTERM always fires first and the existing + * error/status contract is untouched. Once the hook itself has been + * SIGKILLed (exactly the orphan case the wrapper exists for), the guard + * semantics differ per branch: + * - direct exec (GITNEXUS_HOOK_CLI_PATH / PATH-installed `gitnexus`; the + * CLI is the guard's CHILD): `-k 1` TERM-first — a SIGTERM-immune CLI + * can hold the guard ~1s past the inner timeout before the `-k` SIGKILL + * escalation reaps it. + * - npx (the CLI is a GRANDCHILD: guard → npx → CLI): `-s KILL` — the + * budget expiry SIGKILLs the whole process group outright. TERM-first + * would kill only the obedient npx parent, making `timeout` reap it and + * return before the `-k` escalation ever fires, stranding a + * SIGTERM-immune CLI grandchild unbounded (reproduced on coreutils + * 9.x). `-k 1` is retained alongside `-s KILL` as a harmless belt: with + * `-s KILL` the `-k` escalation signal is also KILL. Two residual gaps + * on this branch, both bounded by "no worse than pre-fix" (where the + * grandchild received no signal at all): the group-wide SIGKILL is + * coreutils semantics — a busybox `timeout` passes the self-test (it + * has `-k` and propagates exit status) but signals only its direct + * child, so a busybox guard cannot reach the grandchild; and on the + * SUPERVISED path (hook alive, inner spawnSync timeout SIGTERMs the + * guard) coreutils forwards TERM rather than the `-s` signal, npx dies, + * and the guard exits before any KILL fires — so a SIGTERM-immune CLI + * grandchild still escapes in those two cases. + * If the sibling probe predates the resolveUnixGuardTimeout export (version + * skew), the adapter degrades to the unwrapped invocation instead of + * throwing. Windows is deliberately NOT wrapped — there is no coreutils + * timeout to resolve there and the resolver's self-test spawns /bin/sh — so + * on win32 (the gitnexus.cmd / npx.cmd paths) and whenever the guard + * resolves to null (e.g. macOS without Homebrew coreutils — reported once + * under GITNEXUS_DEBUG) the argv stays byte-identical to the pre-wrap + * invocation. */ function runGitNexusCli(args, cwd, timeout) { const isWin = process.platform === 'win32'; + // Version-skew guard (#2163 follow-up review): an older sibling probe + // without the resolveUnixGuardTimeout export must degrade to the unwrapped + // invocation — a TypeError here would be swallowed by the caller's catch + // and silently kill the augment. + const guard = + isWin || typeof resolveUnixGuardTimeout !== 'function' ? null : resolveUnixGuardTimeout(); + if (!isWin && !guard && !unguardedCliWarned && isDebugEnabled()) { + // Diagnose the "stays unwrapped" Unix paths once per hook process: no + // usable coreutils timeout/gtimeout (e.g. macOS without Homebrew + // coreutils), GITNEXUS_HOOK_TIMEOUT_PATH=disabled, or probe skew above. + unguardedCliWarned = true; + process.stderr.write( + '[GitNexus hook] no usable timeout/gtimeout guard; augment CLI child runs unguarded\n', + ); + } const hookCli = process.env.GITNEXUS_HOOK_CLI_PATH; if (hookCli !== undefined && String(hookCli).trim() && fs.existsSync(String(hookCli))) { - return spawnSync(process.execPath, [String(hookCli), ...args], { + const [cmd, cmdArgs] = guard + ? [ + guard, + [ + '-k', + '1', + String(Math.ceil(timeout / 1000) + 1), + process.execPath, + String(hookCli), + ...args, + ], + ] + : [process.execPath, [String(hookCli), ...args]]; + return spawnSync(cmd, cmdArgs, { encoding: 'utf-8', timeout, cwd, @@ -231,7 +306,12 @@ function runGitNexusCli(args, cwd, timeout) { } if (useDirectBinary) { - return spawnSync(isWin ? 'gitnexus.cmd' : 'gitnexus', args, { + // A non-null guard implies non-Windows, so the wrapped arm can hardcode + // plain `gitnexus` (the guard resolves it via PATH, like spawnSync does). + const [cmd, cmdArgs] = guard + ? [guard, ['-k', '1', String(Math.ceil(timeout / 1000) + 1), 'gitnexus', ...args]] + : [isWin ? 'gitnexus.cmd' : 'gitnexus', args]; + return spawnSync(cmd, cmdArgs, { encoding: 'utf-8', timeout, cwd, @@ -239,8 +319,27 @@ function runGitNexusCli(args, cwd, timeout) { windowsHide: true, }); } - // npx fallback needs shell on Windows since npx is a .cmd script - return spawnSync(isWin ? 'npx.cmd' : 'npx', ['-y', 'gitnexus', ...args], { + // npx fallback needs shell on Windows since npx is a .cmd script. The + // wrapped arm leads with `-s KILL` (NOT TERM-first like the direct + // branches above): the CLI here is a grandchild behind npx — see the + // docblock. + const [cmd, cmdArgs] = guard + ? [ + guard, + [ + '-s', + 'KILL', + '-k', + '1', + String(Math.ceil((timeout + 5000) / 1000) + 1), + 'npx', + '-y', + 'gitnexus', + ...args, + ], + ] + : [isWin ? 'npx.cmd' : 'npx', ['-y', 'gitnexus', ...args]]; + return spawnSync(cmd, cmdArgs, { encoding: 'utf-8', timeout: timeout + 5000, cwd, diff --git a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs index 5c67804b1..948581110 100644 --- a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs +++ b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs @@ -3,14 +3,36 @@ * with a command line that looks like a GitNexus MCP/serve server? * * Backends (no user-installed Sysinternals): - * - Linux: scan procfs under /proc (per-PID fd entries) via stat(2) (dev+inode); works without lsof; - * optional lsof fallback when proc scan finds nothing. + * - Linux: cmdline-first procfs scan under /proc, no lsof at all (#2180). Three + * phases, cheapest first: (0) read /proc//comm — a tiny task->comm read + * that never touches the target's mm — and keep only PIDs whose comm is a + * plausible node/gitnexus server; (1) read up to GITNEXUS_HOOK_PROC_CMDLINE_MAX + * bytes of /proc//cmdline via openSync+readSync (bounded, so a D-state + * holder stuck on mmap_lock or a giant argv can't wedge the hook) and prefilter + * with isGitNexusServerCommand; (2) only for the 0..N survivors, stat their + * /proc//fd/* and compare dev+inode against the target lbug. The lbug + * handle is fd-visible (a @ladybugdb/core property), so this finds every real + * owner without scanning every fd of every process. * - macOS / *BSD / etc.: trusted lsof + ps (absolute paths first). * - Windows: Restart Manager (rstrtmgr) via bundled PowerShell script + * Win32_Process for command lines; trusted powershell.exe under %SystemRoot%. * - * Fail-open on most errors; fail-closed only on lsof ETIMEDOUT (Unix) or - * PowerShell ETIMEDOUT (Windows), matching the hook contract. + * Fail matrix: + * - Linux proc scan: owner found -> fail-closed (skip augment); budget exhausted + * (GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS) -> fail-CLOSED (#2180). This is a + * deliberate change from the old "timeout -> fail-open then try lsof" path. + * End-to-end the busy-host outcome is unchanged: the old code's lsof fallback + * ETIMEDOUT'd on the very hosts where the scan ran out of budget and ALSO + * failed closed there — the lsof leg only ever added 1-2s of dead work plus + * the orphan-storm risk it caused (#2163). What changes is that an overloaded + * host now self-throttles immediately (the throttle the incident needed) + * instead of paying for a doomed lsof. Mid-load hosts that used to fall + * through to a successful lsof now answer from the scan directly (faster) or, + * if even the scan can't finish in budget, fail closed (self-throttle) — a + * bounded, documented tradeoff, never an orphan. + * - macOS / other Unix: fail-open on most errors; fail-closed only on lsof + * ETIMEDOUT, matching the hook contract. + * - Windows: fail-closed only on PowerShell ETIMEDOUT. * * Unix subprocess containment contract (#2163): * - lsof/ps are wrapped in coreutils `timeout`/`gtimeout` when a working @@ -20,13 +42,18 @@ * it 1s later — orphan lifetime is bounded at ~3s instead of unbounded. * - GITNEXUS_HOOK_TIMEOUT_PATH: the sentinel value `disabled` switches the * wrapper off deterministically; any other value is adopted only when it - * exists AND passes a one-shot `-k` self-test — otherwise resolution FALLS - * THROUGH to the built-in candidate list (first self-test pass wins), so - * no malformed value of any shape can silently disable orphan containment. + * exists AND passes a one-shot `-k` exit-propagation self-test — otherwise + * resolution FALLS THROUGH to the built-in candidate list (first self-test + * pass wins), so no malformed value of any shape can silently disable + * orphan containment. * - The gitnexus server is lazy-open + sticky-hold: an idle MCP server holds * ZERO lbug fds until the repo's first MCP query, then keeps the fd open. * A probe before that first query is therefore always false — a known, * pre-existing race, not a bug in this probe. + * - resolveUnixGuardTimeout is exported so the hook adapters can wrap the + * `gitnexus augment` CLI child — the longest-lived hook subprocess (7s + * local / 12s npx inner budgets) — in the same guard; see runGitNexusCli + * in the adapters (#2163 follow-up). */ const fs = require('fs'); @@ -41,6 +68,16 @@ function isGitNexusServerCommand(command) { return hasServerMode && hasGitNexus; } +// GITNEXUS_DEBUG-gated stderr diagnostics. Reuses the exact gating predicate the +// Windows ps1-load warning already uses (===' 1' / ==='true') so there is one +// debug convention in this file, and writes via process.stderr.write (NOT a +// spawn) so it never perturbs the windowsHide spawn-count invariant. +function debugLog(msg) { + if (process.env.GITNEXUS_DEBUG === '1' || process.env.GITNEXUS_DEBUG === 'true') { + process.stderr.write(`[GitNexus hook] ${msg}\n`); + } +} + function resolveHookBinary(tool) { const envKey = tool === 'lsof' ? 'GITNEXUS_HOOK_LSOF_PATH' : 'GITNEXUS_HOOK_PS_PATH'; const fromEnv = process.env[envKey]; @@ -70,38 +107,53 @@ let unixGuardTimeoutCache; /** * Resolve a coreutils `timeout`/`gtimeout` binary to wrap lsof/ps with - * (#2163). Dead code on Windows (the win32 dispatch returns earlier). + * (#2163). Unix-only by contract: the probe's win32 dispatch returns before + * reaching it, and the exported callers (the adapters' runGitNexusCli, + * #2163 follow-up) must check the platform first — the self-test below + * spawns /bin/sh. The memoized result is module-wide, so probe and adapter + * share one lazy self-test per hook process. * * GITNEXUS_HOOK_TIMEOUT_PATH semantics: the sentinel `disabled` turns the * wrapper off; any other value is only a CANDIDATE — an existing file path - * is tried first, but it must pass the `-k` self-test to be adopted. On any - * failure (non-existent path, directory, non-executable file, wrapper - * without `-k` support, …) resolution falls through to the built-in - * candidates below, tried in order, first self-test pass wins. This is - * strictly stronger than the sibling GITNEXUS_HOOK_LSOF_PATH / - * GITNEXUS_HOOK_PS_PATH overrides (which only check existence): no bad env - * value of ANY shape can silently disable orphan containment. + * is tried first, but it must pass the `-k` exit-propagation self-test to + * be adopted. On any failure (non-existent path, directory, non-executable + * file, wrapper without `-k` support, always-exit-0 stub, …) resolution + * falls through to the built-in candidates below, tried in order, first + * self-test pass wins. This is strictly stronger than the sibling + * GITNEXUS_HOOK_LSOF_PATH / GITNEXUS_HOOK_PS_PATH overrides (which only + * check existence): no bad env value of ANY shape can silently disable + * orphan containment. * * Lazy self-test: candidates are probed only when the lsof/ps fallback is * first reached, and the result is memoized. A candidate is adopted only - * when `timeout -k 1 1 /bin/sh -c :` exits 0. This rejects wrappers that do - * not support the coreutils `-k` flag — busybox <1.34, toybox, broken - * symlinks — which would otherwise exit with a usage error without ever - * running lsof, silently converting the lsof-ETIMEDOUT fail-closed contract - * into fail-open (#1492 regression). Only when EVERY candidate fails does - * the probe fall back to the unwrapped status quo (memoized null). - * busybox ≥1.34 passes the test and is fully usable (capability, not - * identity, decides). + * when `timeout -k 1 1 /bin/sh -c 'exit 42'` exits 42 — i.e. it must RUN + * the wrapped command AND PROPAGATE its exit status. This rejects two + * failure shapes: wrappers without the coreutils `-k` flag — busybox <1.34, + * toybox, broken symlinks — which would exit with a usage error without + * ever running lsof, silently converting the lsof-ETIMEDOUT fail-closed + * contract into fail-open (#1492 regression); and always-exit-0 stubs + * (/bin/true shapes), which would otherwise be adopted and "succeed" every + * wrapped spawn instantly without running it — a constant no-owner probe + * answer and, worse, a silently dead augment (status 0, empty stderr passes + * the adapters' success check with no context; #2163 follow-up review). + * Only when EVERY candidate fails does the probe fall back to the unwrapped + * status quo (memoized null). busybox ≥1.34 passes the test and is fully + * usable for everything THIS file spawns (lsof/ps are the guard's direct + * children) and for the adapters' direct-exec arm. The adapters' npx arm + * additionally relies on coreutils' process-GROUP signalling for its + * `-s KILL` grandchild reaping; busybox signals only its direct child, and + * this self-test deliberately does not probe that capability — see the + * adapter docblocks for the residual-gap statement. */ function passesGuardSelfTest(guard) { try { - const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', ':'], { + const selfTest = spawnSync(guard, ['-k', '1', '1', '/bin/sh', '-c', 'exit 42'], { encoding: 'utf-8', timeout: 3000, stdio: ['ignore', 'ignore', 'ignore'], windowsHide: true, }); - return !selfTest.error && selfTest.status === 0; + return !selfTest.error && selfTest.status === 42; } catch { return false; } @@ -222,59 +274,325 @@ function hasGitNexusServerOwnerWindows(dbPathAbs, myPid) { return false; } -function readLinuxCmdline(pidStr) { +// The procfs root every Linux scan path reads from. Production is always /proc; +// GITNEXUS_HOOK_PROC_ROOT only exists so unit tests can inject a fixture tree +// (comm + cmdline + fd symlinks) and assert the three-phase logic without +// scanning the real, ~hundreds-of-process /proc of the test host. +// +// Test-only gate (F4): the override is honored ONLY under a test runner — +// vitest injects VITEST="true" and NODE_ENV="test" into every worker (verified; +// a production hook is `node .cjs` with neither set). Without the gate, a +// production env that accidentally leaked GITNEXUS_HOOK_PROC_ROOT (pointing at an +// empty/bad tree) would make readdirSync find no pids -> 'not-owned' -> Linux +// owner detection silently OFF (fail-OPEN: augment races the real server for the +// lbug, the #1492 class). Gating to the test signal makes that leak inert in +// production (always /proc) while the fake-procfs unit tests, which run under +// vitest, still inject freely. Unset env (or non-test context) => /proc, so the +// production path is byte-for-byte the historical behavior. +function isTestContext() { + return ( + process.env.VITEST === 'true' || process.env.VITEST === '1' || process.env.NODE_ENV === 'test' + ); +} +function getProcRoot() { + if (!isTestContext()) return '/proc'; + const raw = process.env.GITNEXUS_HOOK_PROC_ROOT; + return raw && String(raw).trim() ? String(raw) : '/proc'; +} + +// Max bytes read from /proc//cmdline in Phase 1. Bounded by default so a +// D-state holder wedged on mmap_lock, or a process with a pathological multi-MB +// argv, can't stall the hook. 16 KiB comfortably clears a realistic +// `node mcp` line +// (the `mcp`/`serve` mode token lives at the very tail, so the cap must be large +// enough to reach it — see PROC_CMDLINE_FLOOR escalation below). Overridable for +// tests; never goes below PROC_CMDLINE_FLOOR. +const PROC_CMDLINE_FLOOR = 4096; +function getCmdlineMaxBytes() { + const raw = process.env.GITNEXUS_HOOK_PROC_CMDLINE_MAX; + // Number() (not parseInt) so "8e3" reads as 8000, not 8 (parseInt stops at + // 'e'). The `raw && String(raw).trim()` guard keeps empty/whitespace on the + // default; trailing garbage ("8abc") now -> NaN -> default (stricter). + const n = raw && String(raw).trim() ? Number(String(raw).trim()) : NaN; + if (Number.isFinite(n) && n >= PROC_CMDLINE_FLOOR) return n; + return 16384; +} + +// Phase 0 comm prefilter. /proc//comm is the kernel task->comm string, +// capped at 16 bytes INCLUDING the trailing NUL — i.e. at most 15 visible +// chars, truncated by the kernel with no marker. So a process whose real name +// is longer than 15 chars shows a 15-char prefix here. The match below is +// therefore truncation-safe in BOTH directions (a whitelist name that is a +// prefix of comm, or comm that is a prefix of a whitelist name, both count) to +// guarantee we never drop a real owner at this cheap stage — Phase 2's dev+ino +// fd check is the real authority; Phase 0/1 only exist to skip the overwhelming +// majority (kernel threads, shells, editors) cheaply. +// +// The whitelist is calibrated against what a real `gitnexus mcp`/`serve` server +// actually reports for comm. Observed on production hosts: the server renames +// its main thread, so comm reads `MainThread` (via @ladybugdb/core's +// worker_threads setup), NOT `node` — omitting it would blind the probe to +// every real server (#1492-class owner miss). We also keep the plausible +// launcher/runtime basenames in case a future build does not rename the thread. +// Conservative by design: over-collecting a few extra candidates only costs a +// bounded number of Phase 1 cmdline reads. +const COMM_CANDIDATES = ['node', 'gitnexus', 'bun', 'deno', 'npm', 'npx', 'MainThread']; +function commLooksLikeServer(comm) { + const c = comm.trim(); + if (!c) return false; + for (const name of COMM_CANDIDATES) { + if (name === c || name.startsWith(c) || c.startsWith(name)) return true; + } + return false; +} + +function readProcComm(procRoot, pidStr) { try { - return fs.readFileSync(`/proc/${pidStr}/cmdline`, 'utf8').replace(/\0+/g, ' ').trim(); + return fs + .readFileSync(path.join(procRoot, pidStr, 'comm'), 'utf8') + .replace(/\0+/g, '') + .trim(); } catch { return ''; } } -function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { +// Timeout sentinel for readLinuxCmdline (F3). MUST be distinct from the +// "unreadable/empty" return value (''): '' flows through isGitNexusServerCommand +// as a NON-candidate (both regexes are false on ''), so the Phase 1 caller +// `continue`s past it — correct for a raced/openSync-failed pid, but a FAIL-OPEN +// bug if it ever meant "I ran out of budget mid-read" (a real owner whose +// escalation timed out would be silently dropped, racing the lbug -> #1492). A +// unique Symbol can never collide with any cmdline string, so the caller can +// branch on it explicitly and map a mid-read timeout to the tri-state 'timeout' +// (fail-CLOSED) instead of swallowing it as a non-candidate. +const CMDLINE_TIMEOUT = Symbol('gitnexus.cmdline.timeout'); + +// Bounded /proc//cmdline read for Phase 1. openSync+readSync (not +// readFileSync) so a D-state holder cannot stall the hook on a huge or +// never-EOF argv: we read at most `cap` bytes and stop. cmdline separates argv +// with NULs; convert to spaces for isGitNexusServerCommand. +// +// Owner-miss guard for the 4 KB cap: the `gitnexus` token usually sits in the +// first path component while the `mcp`/`serve` mode token is the LAST argv, so +// a naive 4 KB read could clip the mode token off a server launched with a very +// long interpreter path and silently miss a real owner. We mitigate two ways: +// (a) the default cap (16 KiB) already clears realistic lines; (b) if the first +// read fills the cap AND already contains the `gitnexus` token but no mode +// token yet, we keep reading in bounded chunks (up to a hard ceiling) until the +// mode token appears or the file ends — so a genuine server is never missed for +// want of a few more bytes, while non-candidates still pay only the initial +// bounded read. +// +// Budget (F3): the escalation loop above is the one place a SINGLE pathological +// candidate could read up to HARD_CEIL (256 KiB) before the next scan-level +// budget check, weakening the timeout contract. `outOfBudget` (the scan's shared +// deadline callback) is checked once per escalation iteration; on expiry we +// return CMDLINE_TIMEOUT (NOT '') so the caller can fail-closed honestly rather +// than mistake the partial read for a non-candidate. Reads that simply can't +// open / error out still return '' (genuinely "not a readable candidate"). +function readLinuxCmdline(procRoot, pidStr, cap, outOfBudget) { + const file = path.join(procRoot, pidStr, 'cmdline'); + let fd; + try { + fd = fs.openSync(file, 'r'); + } catch { + return ''; + } + try { + const HARD_CEIL = 262144; // 256 KiB absolute ceiling for the escalation path + let collected = Buffer.alloc(0); + let offset = 0; + let chunkCap = cap; + for (;;) { + // allocUnsafe is safe here: readSync fills exactly [0, bytes), only + // buf.subarray(0, bytes) is consumed, and Buffer.concat deep-copies that + // slice into `collected`, so the uninitialized tail never reaches decode. + const buf = Buffer.allocUnsafe(chunkCap); + const bytes = fs.readSync(fd, buf, 0, chunkCap, offset); + if (bytes <= 0) break; + collected = Buffer.concat([collected, buf.subarray(0, bytes)]); + offset += bytes; + const text = collected.toString('utf8').replace(/\0+/g, ' '); + // Stop early when we can already decide "owner": has both the gitnexus + // token and a mode token. Keep going only when gitnexus is present but + // the mode token might be just past the boundary. + const hasGitNexus = + /(?:^|[/\\\s])gitnexus(?:\.cmd)?(?:\s|$)/.test(text) || + /node_modules[/\\]gitnexus[/\\]/.test(text); + const hasMode = /(?:^|\s)(mcp|serve)(?:\s|$)/.test(text); + if (hasMode) break; // decided (positive); isGitNexusServerCommand re-checks below + if (bytes < chunkCap) break; // EOF: full cmdline read, definitive + if (!hasGitNexus) break; // not a candidate; do not escalate the read + if (offset >= HARD_CEIL) break; // bounded escalation only + // Budget gate the escalation: a single huge-argv candidate must not burn + // the whole scan deadline before we re-check. Return the timeout sentinel + // (never '') so the caller fails closed instead of treating us as a + // non-candidate. The sole caller (linuxProcScanFindGitNexusServer) always + // passes outOfBudget, so no presence guard is needed. + if (outOfBudget()) return CMDLINE_TIMEOUT; + chunkCap = cap; // keep reading more in cap-sized chunks + } + return collected.toString('utf8').replace(/\0+/g, ' ').trim(); + } catch { + return ''; + } finally { + try { + fs.closeSync(fd); + } catch { + /* ignore */ + } + } +} + +function resolveLinuxProcBudgetMs() { const raw = process.env.GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS; - const budget = Number(raw && String(raw).trim()) ? Number.parseInt(String(raw), 10) : 1200; + // Gate on the STRING's emptiness, NOT the parsed number's truthiness — the + // old `Number(raw && trim()) ? ... : 1200` form treated "0" as falsy and + // silently fell back to 1200 (#2180). Use Number() (not parseInt) so "16e3" + // reads as 16000, not 16 (parseInt stops at 'e'). The `&& String(raw).trim()` + // guard is load-bearing: without it a set-but-empty/whitespace value would be + // `Number("")===0` => budget 0 => immediate fail-CLOSED timeout (augment + // permanently skipped). With it, ''/whitespace => NaN => 1200 default, while a + // finite "0" still parses to an explicit, deterministic "no budget" => + // immediate timeout. Non-numeric / unset => default 1200. + const n = raw != null && String(raw).trim() ? Number(String(raw).trim()) : NaN; + if (!Number.isFinite(n)) return 1200; + return n; // may be <= 0, meaning "out of budget on the first check" +} + +// Returns one of: 'owned' (a non-self process with a GitNexus-server cmdline +// holds the target lbug fd), 'not-owned' (scan completed, no such owner), or +// 'timeout' (the per-scan budget was exhausted before a verdict). The name is +// pinned by a source-contract test; only the return TYPE changed (#2180: +// boolean -> tri-state, so the dispatcher can fail-closed on 'timeout'). +function linuxProcScanFindGitNexusServer(dbPathAbs, myPid) { + const budget = resolveLinuxProcBudgetMs(); + // A non-positive budget is an explicit, deterministic "no time to scan" => + // immediate timeout (the #2180 test vector, and the only correct reading of + // the fixed parse: "0" must NOT mean 1200). Returning before any procfs read + // keeps it instantaneous regardless of host load. + if (budget <= 0) return 'timeout'; + const procRoot = getProcRoot(); + const cmdlineCap = getCmdlineMaxBytes(); const start = Date.now(); + const outOfBudget = () => Date.now() - start > budget; + let targetStat; try { targetStat = fs.statSync(dbPathAbs); } catch { - return false; + // Caller already existsSync'd the path; a stat failure here is a transient + // race, treat as no owner (historical semantics). + return 'not-owned'; } + let procEntries; try { - procEntries = fs.readdirSync('/proc', { withFileTypes: true }); + procEntries = fs.readdirSync(procRoot, { withFileTypes: true }); } catch { - return false; + return 'not-owned'; } + + // Phase 0 + Phase 1: collect the few PIDs whose comm AND cmdline look like a + // GitNexus server, without touching any fd yet. + const candidates = []; for (const ent of procEntries) { - if (Date.now() - start > budget) return false; + if (outOfBudget()) return 'timeout'; if (!ent.isDirectory() || !/^\d+$/.test(ent.name)) continue; const pid = Number.parseInt(ent.name, 10); if (!Number.isFinite(pid) || pid === myPid) continue; - const fdDir = path.join('/proc', ent.name, 'fd'); + + // Phase 0: cheap comm prefilter. + const comm = readProcComm(procRoot, ent.name); + if (!comm) continue; // unreadable comm (kernel thread, raced exit) -> skip + if (!commLooksLikeServer(comm)) continue; + + // Phase 1: bounded cmdline read + isGitNexusServerCommand prefilter. + if (outOfBudget()) return 'timeout'; + const cmdline = readLinuxCmdline(procRoot, ent.name, cmdlineCap, outOfBudget); + // F3: a mid-read budget timeout returns the CMDLINE_TIMEOUT sentinel (a + // Symbol, never a string). Fail CLOSED on it rather than letting it fall + // through isGitNexusServerCommand as a non-candidate — a real owner whose + // escalation timed out must not be silently dropped (would fail-OPEN). + if (cmdline === CMDLINE_TIMEOUT) return 'timeout'; + if (!isGitNexusServerCommand(cmdline)) continue; + candidates.push(ent.name); + } + + // Phase 2: only now stat the fds of the (typically 0-2) survivors. + for (const pidStr of candidates) { + if (outOfBudget()) return 'timeout'; + const fdDir = path.join(procRoot, pidStr, 'fd'); let fds; try { fds = fs.readdirSync(fdDir); - } catch { + } catch (err) { + // F1: the old code returned 'owned' for EVERY non-ENOENT error. That was + // a correctness bug: /proc//fd is owner-only (mode 0500), so a + // cross-user/root `gitnexus mcp` serving a DIFFERENT repo passes Phase 0+1 + // (its cmdline matches) and then EACCES'es here — yet its dev+ino was + // NEVER compared against THIS lbug. Claiming 'owned' lets it permanently, + // silently suppress augment for a repo it does not actually lock. We now + // distinguish the failure shapes (all still fail-closed where we can't + // prove non-ownership, but 'timeout' is the HONEST verdict for + // "inconclusive", not the false-positive 'owned'): + const code = err && err.code; + if (code === 'ENOENT') { + // Process raced away between the candidate scan and now -> genuinely no + // longer an owner. Move on. + continue; + } + if (code === 'EACCES' || code === 'EPERM') { + // Permission-denied fd dir: cannot read fds, so ownership is + // UNVERIFIABLE. Fail closed honestly via 'timeout' (the dispatcher maps + // timeout -> true, same protective skip as before) WITHOUT lying that we + // confirmed ownership. Do NOT degrade to not-owned/fail-open: if this + // really is the owner, fail-open re-opens the #1492 lbug race; augment + // is optional context, so a conservative skip costs little. + debugLog( + `fd dir unreadable for candidate pid ${pidStr} (${code}); ownership ` + + `unverifiable, probe inconclusive -> fail-closed (timeout)`, + ); + return 'timeout'; + } + if (code === 'EIO' || code === 'ESTALE') { + // Genuine transient I/O against this candidate's fd dir — not evidence + // it does NOT hold the lbug. Treat as inconclusive and fail closed + // (timeout) rather than continue, so a real owner mid-I/O-blip is not + // dropped (would fail-open). + debugLog( + `fd dir transient I/O error for candidate pid ${pidStr} (${code}); ` + + `probe inconclusive -> fail-closed (timeout)`, + ); + return 'timeout'; + } + // Any other shape (ENOTDIR — fd path is not a directory at all, so this + // is not a plausible live-procfs owner — and the long tail) is treated as + // "this candidate is not an owner": move to the next candidate instead of + // the old blanket 'owned'. If no other candidate owns the lbug the scan + // ends not-owned (dispatcher fail-open) — acceptable because ENOTDIR means + // the fd entry is structurally not a real /proc//fd. + debugLog( + `fd dir not a readable directory for candidate pid ${pidStr} ` + + `(${code || 'unknown'}); treating candidate as non-owner -> continue`, + ); continue; } - let holds = false; for (const fd of fds) { - if (Date.now() - start > budget) return false; + if (outOfBudget()) return 'timeout'; try { const st = fs.statSync(path.join(fdDir, fd)); if (st.dev === targetStat.dev && st.ino === targetStat.ino) { - holds = true; - break; + return 'owned'; } } catch { - /* ignore */ + /* fd raced closed; ignore */ } } - if (!holds) continue; - if (isGitNexusServerCommand(readLinuxCmdline(ent.name))) return true; } - return false; + + return 'not-owned'; } function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { @@ -350,8 +668,13 @@ function hasGitNexusDbLockedByGitNexusServer(dbPath, myPid) { } if (process.platform === 'linux') { - if (linuxProcScanFindGitNexusServer(dbPathAbs, myPid)) return true; - return unixLsofPsFindGitNexusServer(dbPathAbs, myPid); + // #2180: cmdline-first procfs scan, no lsof. 'timeout' fails CLOSED + // (overloaded host self-throttles — the throttle the orphan-storm incident + // needed; the old lsof fallback ETIMEDOUT'd and failed closed on these same + // hosts anyway, only slower and with the orphan risk). 'not-owned' is the + // only false. See the fail matrix in the file header. + const verdict = linuxProcScanFindGitNexusServer(dbPathAbs, myPid); + return verdict !== 'not-owned'; } return unixLsofPsFindGitNexusServer(dbPathAbs, myPid); @@ -359,4 +682,30 @@ function hasGitNexusDbLockedByGitNexusServer(dbPath, myPid) { module.exports = { hasGitNexusDbLockedByGitNexusServer, + // Exported for white-box unit tests that must assert the tri-state verdict + // ('owned' | 'not-owned' | 'timeout') directly — the dispatcher collapses + // timeout and owned to the same boolean true, so the boolean API alone cannot + // distinguish the F1 EACCES->timeout fix from the old EACCES->owned bug. The + // Probe interface already declares this optional. Linux-only by contract; the + // name is pinned by a source-contract test. + linuxProcScanFindGitNexusServer, + // #2163 follow-up: the hook adapters wrap the augment CLI in the same + // guard. Returns a self-tested wrapper path — the built-in candidates are + // always absolute; a GITNEXUS_HOOK_TIMEOUT_PATH override is adopted as the + // exact string that passed the self-test. Same string is also the same + // RESOLUTION for absolute paths and for slashless names (PATH lookup is + // cwd-independent); a slash-containing RELATIVE override, however, is + // existsSync-checked and self-tested against this process's cwd while the + // adapters spawn the CLI with a `cwd` option (chdir-before-exec), so such + // a value can pass here yet ENOENT at the augment call site — set the + // override to an absolute path. Returns null when the wrapper is + // disabled/unavailable. Never call on win32 (see its JSDoc). + resolveUnixGuardTimeout, + // Exported for white-box unit tests of the numeric-env parsing (#2183 review): + // Number()-not-parseInt so "16e3" reads as 16000, plus the empty/whitespace + // guard that keeps a set-but-empty budget on the 1200 default instead of an + // immediate fail-closed timeout. Tested directly because the values are + // otherwise only observable indirectly through scan timing/escalation. + getCmdlineMaxBytes, + resolveLinuxProcBudgetMs, }; diff --git a/gitnexus-claude-plugin/skills/gitnexus-debugging/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-debugging/SKILL.md index 9834f94b7..80f9c0ec5 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-debugging/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-debugging/SKILL.md @@ -16,10 +16,10 @@ description: "Use when the user is debugging a bug, tracing an error, or asking ## Workflow ``` -1. query({query: ""}) → Find related execution flows +1. query({search_query: ""}) → Find related execution flows 2. context({name: ""}) → See callers/callees/processes 3. READ gitnexus://repo/{name}/process/{name} → Trace execution flow -4. cypher({query: "MATCH path..."}) → Custom traces if needed +4. cypher({statement: "MATCH path..."}) → Custom traces if needed ``` > If "Index is stale" → run `node .gitnexus/run.cjs analyze` in terminal. @@ -51,7 +51,7 @@ description: "Use when the user is debugging a bug, tracing an error, or asking **query** — find code related to error: ``` -query({query: "payment validation error"}) +query({search_query: "payment validation error"}) → Processes: CheckoutFlow, ErrorHandling → Symbols: validatePayment, handlePaymentError, PaymentException ``` @@ -75,7 +75,7 @@ RETURN [n IN nodes(path) | n.name] AS chain ## Example: "Payment endpoint returns 500 intermittently" ``` -1. query({query: "payment error handling"}) +1. query({search_query: "payment error handling"}) → Processes: CheckoutFlow, ErrorHandling → Symbols: validatePayment, handlePaymentError diff --git a/gitnexus-claude-plugin/skills/gitnexus-exploring/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-exploring/SKILL.md index ccf684c28..f483c2fd6 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-exploring/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-exploring/SKILL.md @@ -18,7 +18,7 @@ description: "Use when the user asks how code works, wants to understand archite ``` 1. READ gitnexus://repos → Discover indexed repos 2. READ gitnexus://repo/{name}/context → Codebase overview, check staleness -3. query({query: ""}) → Find related execution flows +3. query({search_query: ""}) → Find related execution flows 4. context({name: ""}) → Deep dive on specific symbol 5. READ gitnexus://repo/{name}/process/{name} → Trace full execution flow ``` @@ -50,7 +50,7 @@ description: "Use when the user asks how code works, wants to understand archite **query** — find execution flows related to a concept: ``` -query({query: "payment processing"}) +query({search_query: "payment processing"}) → Processes: CheckoutFlow, RefundFlow, WebhookHandler → Symbols grouped by flow with file locations ``` @@ -68,7 +68,7 @@ context({name: "validateUser"}) ``` 1. READ gitnexus://repo/my-app/context → 918 symbols, 45 processes -2. query({query: "payment processing"}) +2. query({search_query: "payment processing"}) → CheckoutFlow: processPayment → validateCard → chargeStripe → RefundFlow: initiateRefund → calculateRefund → processRefund 3. context({name: "processPayment"}) diff --git a/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md index cacc4e886..7f90f4e6d 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md @@ -38,7 +38,10 @@ For any task involving code understanding, debugging, impact analysis, or refact | `detect_changes` | Git-diff impact — what do your current changes affect | | `rename` | Multi-file coordinated rename with confidence-tagged edits | | `cypher` | Raw graph queries (read `gitnexus://repo/{name}/schema` first) | -| `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | +| `explain` | Persisted taint findings — source→sink data flows (needs `analyze --pdg`) | +| `pdg_query` | Control/data dependence — what gates X (CDG) / where Y flows (REACHING_DEF); needs `analyze --pdg` | +| `check` | Check graph invariants such as circular imports | +| `list_repos` | Discover indexed repos (paginated — `limit`/`offset`) | ### Paginating `list_repos` @@ -71,6 +74,25 @@ list_repos { offset: 400 } → repos 401–437, hasMore false Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). Out-of-range or malformed `limit`/`offset` (non-integer, `limit` outside `[1, 200]`, `offset < 0`) are rejected with a clear error — `limit` above the max is rejected, not silently capped. The order is deterministic (lower-cased name, then path), so paging never skips or duplicates an entry while the registry is unchanged. +### Taint findings (`explain`) + +`explain` returns intra-procedural taint findings (`TAINTED` edges) recorded by `gitnexus analyze --pdg` — each with a sink category (command-injection, code-injection, path-traversal, sql-injection, xss), source/sink lines, and the ordered hop path with the variable carried on each hop. + +- `explain {}` — enumerate all findings for the repo (bounded by `limit`, deterministic order) +- `explain { target: "src/vuln.ts" }` — findings in a file (suffix path match accepted) +- `explain { target: "runUserCommand" }` — findings in a function (resolved like `context`; ambiguous names return ranked candidates) + +A repo indexed without `--pdg` returns a clear "no taint layer" note. Caveats: findings are intra-procedural only — cross-function, closure/callback, property/field, and implicit flows are not modeled, so the absence of a finding is **not** proof of safety. `SANITIZES` (sanitizer-kill) edges are queryable via `cypher`. + +### Control & data dependence (`pdg_query`) + +`pdg_query` reads the control/data-dependence layers `gitnexus analyze --pdg` records (CDG + REACHING_DEF, basic-block granular) — the control/data analog of `explain`. It is **always anchored** (a `target` file path or symbol, resolved like `context`) and has two modes: + +- `pdg_query { mode: "controls", target: "..." }` — CDG: "under what condition does X run?". Each edge is a controlling predicate block → dependent block with the branch sense (`'T'`/`'F'`) in `reason`; an edge into an early `return`/`throw` is flagged `guard: true` (guard-clause discovery — the sense depends on the predicate, so don't filter guards by a fixed label). +- `pdg_query { mode: "flows", target: "...", variable?: "..." }` — REACHING_DEF def→use edges within the function; pass `variable` to trace one binding. + +A repo indexed without `--pdg` returns a "no PDG layer" note (or "status unknown" when the layer can't be confirmed). Intra-procedural only — cross-function flow is taint's domain (`explain`). The raw CDG/REACHING_DEF edges are also queryable via `cypher`. See the `gitnexus-pdg-query` skill for the full query surface. + ## Resources Reference Lightweight reads (~100-500 tokens) for navigation: diff --git a/gitnexus-claude-plugin/skills/gitnexus-pdg-query/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-pdg-query/SKILL.md new file mode 100644 index 000000000..f2fcd7d3b --- /dev/null +++ b/gitnexus-claude-plugin/skills/gitnexus-pdg-query/SKILL.md @@ -0,0 +1,89 @@ +--- +name: gitnexus-pdg-query +description: "Use when querying or extending GitNexus's PDG control/data-dependence surface (the `pdg_query` MCP tool, CDG/REACHING_DEF edges), or reasoning about \"what controls X\" / \"where does Y flow\" / guard clauses. Examples: \"what guards this statement?\", \"trace this variable within the function\", \"why is the pdg_query result empty?\", \"add a CDG query\"." +--- + +# PDG query surface with GitNexus + +Expert knowledge for the `pdg_query` MCP tool and the control/data-dependence +edges it reads — the opt-in `--pdg` program-dependence layers. Read this before +touching `gitnexus/src/mcp/local/local-backend.ts` (`_pdgQueryImpl`) or the +`pdg_query` tool def, or when explaining a `pdg_query` result. + +## When to Use + +- "Under what condition does this statement run?" (guarding predicates). +- "Where does this variable flow inside the function?" (def→use). +- Guard-clause discovery (early-return guards — subsumes the #559 heuristic). +- Extending or reviewing `pdg_query` / the CDG / REACHING_DEF read path. +- Debugging an empty or surprising `pdg_query` result. + +## The layered substrate (build order) + +`pdg_query` runs **on** the same graph taint runs on. Each layer is opt-in +behind `--pdg`; a default `analyze` run records none of them (byte-identical). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L5 CDG Ferrante control dependence (post-dominators) (M5 #2085) +``` + +All three are `BasicBlock → BasicBlock` edges in the single `CodeRelation` table +(keyed by the `type` property). There is **no** `Function → BasicBlock` edge. + +## The two modes + +- `pdg_query({ mode: 'controls', target })` — CDG. For the anchored function, + each edge: controlling predicate block → dependent block + branch sense in + `label` (`'T'` = predicate's true/taken arm, `'F'` = false/fall-through). An + edge into an early-return/throw block is flagged `guard: true`. +- `pdg_query({ mode: 'flows', target, variable? })` — REACHING_DEF def→use + edges; `variable` filters to one binding. + +`target` is **required** — a file path or a symbol/function name (resolved like +`context()`). There is no anchorless mode (see below). + +## The corrected guard-clause Cypher + +The RFC #567 §2 form (`[:CDG {label:'F'}]`) does **not** run as written. Edges +are values of the single `CodeRelation` table's `type` property, and the branch +sense is in `reason`, NOT a `label` column: + +```cypher +MATCH (pred:BasicBlock)-[r:CodeRelation {type: 'CDG'}]->(dep:BasicBlock) +WHERE dep.text STARTS WITH 'return' OR dep.text STARTS WITH 'throw' +RETURN pred.startLine, r.reason AS branch, dep.startLine, dep.text +``` + +`r.reason` is the sense the predicate took to reach the early exit. For +`if (!ok) return;` the return rides the predicate's **true** arm (`'T'`) and the +protected body rides the **false** arm (`'F'`) — polarity depends on the guard, +so don't hard-code one sense. + +## Gotchas (the load-bearing ones) + +- **Always anchored + LIMIT-bounded.** LadybugDB has no rel-property index, so + an unanchored `[:CDG*]`/`[:REACHING_DEF*]` path scan is unbounded. `pdg_query` + requires `target` and bounds the page; raw `cypher` callers must anchor on a + file id-prefix or symbol span themselves. +- **BasicBlock↔symbol join is reconstructed.** No `Function→BasicBlock` edge: + the block is matched by its id-prefix (`BasicBlock:::…`) + plus `startLine` within the symbol's span. BasicBlock `startLine` is **1-based** + while the symbol node's `startLine`/`endLine` are **0-based**, so **both** bounds + are shifted `+1` (`[symStart+1, symEnd+1]`): the upper `+1` keeps a guard/def/use + on the function's **final line**, the lower `+1` excludes an adjacent function's + block on the line directly **above**. Same-line / nested functions anchor coarsely. +- **No PDG layer ⇒ a note, not an error.** If the repo wasn't indexed with + `--pdg` the tool returns `{ results: [], note: "no PDG layer …" }` (cheap meta + probe on `RepoMeta.pdg.maxCdgEdgesPerFunction` / `maxReachingDefEdgesPerFunction`). +- **CDG labels are binary in M5/M6.** Every `switch`-case arm is `'T'`; per-case + conditions are not yet distinguished. +- **Intra-procedural only.** Cross-function flow is taint's domain (`explain`). + +## Mirror, don't fork + +`_pdgQueryImpl` is the front half of `_explainImpl` (WAL wrapper, meta no-layer +probe, limit validation, `resolveSymbolCandidates` anchoring) with CDG/ +REACHING_DEF instead of TAINTED — and none of taint's path-codec / interproc +`TAINT_PATH` machinery. Reuse those shared helpers; do not re-implement them. diff --git a/gitnexus-claude-plugin/skills/gitnexus-refactoring/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-refactoring/SKILL.md index e13c04e14..90c8c324d 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-refactoring/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-refactoring/SKILL.md @@ -17,7 +17,7 @@ description: "Use when the user wants to rename, extract, split, move, or restru ``` 1. impact({target: "X", direction: "upstream"}) → Map all dependents -2. query({query: "X"}) → Find execution flows involving X +2. query({search_query: "X"}) → Find execution flows involving X 3. context({name: "X"}) → See all incoming/outgoing refs 4. Plan update order: interfaces → implementations → callers → tests ``` diff --git a/gitnexus-claude-plugin/skills/gitnexus-taint-analysis/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-taint-analysis/SKILL.md new file mode 100644 index 000000000..9bffffdac --- /dev/null +++ b/gitnexus-claude-plugin/skills/gitnexus-taint-analysis/SKILL.md @@ -0,0 +1,178 @@ +--- +name: gitnexus-taint-analysis +description: "Use when working on, reviewing, or extending GitNexus's CFG/taint/PDG subsystem (the `--pdg` layers), or when reasoning about source→sink data-flow findings. Examples: \"How does taint analysis work here?\", \"Why didn't explain find this flow?\", \"Add a new sink/source\", \"Review the interprocedural taint code\"." +--- + +# CFG & Taint Analysis with GitNexus + +Expert knowledge for the opt-in `--pdg` program-analysis subsystem: control-flow +graphs, reaching definitions, and intra- + inter-procedural taint. Read this +before touching `gitnexus/src/core/ingestion/cfg/**` or +`gitnexus/src/core/ingestion/taint/**`, or when explaining a finding. + +## When to Use + +- "How does the taint engine work / why is this flow (not) reported?" +- Adding a source, sink, or sanitizer to the model. +- Extending or reviewing the CFG / reaching-defs / taint / summary code. +- Understanding the `explain` MCP tool's findings (intra- vs inter-procedural). +- Debugging a false positive or false negative in `--pdg` output. + +## The layered substrate (build order) + +Taint runs **on** the graph, not beside it. Each layer is opt-in behind `--pdg` +and a default `analyze` run is **byte-identical** (the golden parity gate is the +hard floor for every change here). + +``` +L1 CFG per-function basic blocks + control-flow edges (M1 #2081) +L2 REACHING_DEF GEN/KILL def→use data dependence (pure solver) (M2 #2082) +L3 Taint (intra) source→sink over RD facts, minus sanitizers (M3 #2083) +L4 Taint (inter) per-function summaries composed over CALLS (M4 #2084) +``` + +- **Worker-built, main-thread-solved.** The parse worker builds each function's + CFG + harvests def/use + call-site facts onto `ParsedFile.cfgSideChannel` + (plain, structured-clone-safe data — never AST nodes). The main thread runs + the pure solvers. NEVER re-parse on the main thread (re-introduces the #1983 + OOM). +- **In-phase emit (KTD1).** L1–L4-harvest all run INSIDE the scope-resolution + pdg window (`scope-resolution/pipeline/run.ts`, gated `input.pdg === true`), + because the disk-backed ParsedFile store is cleared when that phase ends — a + standalone post-`mro` phase would read empty data. The cross-function fixpoint + (L4) is the exception: it runs in its OWN registered phase (`taintSummaries`) + AFTER scope-resolution, because it needs the COMPLETE call graph, and consumes + small plain summary data threaded out via `ScopeResolutionOutput`. +- **Pure-solver contract.** `computeReachingDefs`, `computeTaintFlows`, + `harvestFunctionSummary`, and `solveInterprocTaint` are pure and deterministic + (no graph, no I/O, no logger; sorted outputs). Snapshot tests and + content-derived edge ids depend on it. + +## Intra-procedural taint (L3) + +Forward reachability over RD facts from matched **sources** to matched **sinks**, +killed by **sanitizers**. Key design points worth internalizing: + +- **Occurrence-tagged sites.** A flat per-arg binding set cannot tell + `exec(escape(x))` (safe) from `exec(x)` (finding); the harvest records nested + call structure (`SiteRecord.parent`/via-tags) so sanitizer interposition is + precise. +- **Kind-set sanitizer model.** A taint carries a set of *neutralized* + `SinkKind`s; a sink fires unless its kind is in the set. So `escape(req.body)` + suppresses `res.send` (xss) but STILL fires `db.query` (sql) — a kind-blind + kill would be a suppressed live injection (the forbidden FN direction). + `path.basename(t)` neutralizes path-traversal only, not command-injection. +- **Statement-level finding identity.** NOT block-pair (block conflation drops + distinct findings; `exec(req.body, req.query)` is two findings). +- Persisted as `TAINTED` edges (BasicBlock→BasicBlock); the path rides the + `reason` column via the shared versioned codec (`taint/path-codec.ts`). + +## Interprocedural taint (L4) — the functional/summary method + +The production approach (Sharir-Pnueli 1981; the same shape as Meta's Pysa and +Mariana Trench, and FB Infer) — NOT full IFDS tabulation. Each function is +reduced to a compact **summary**, and summaries are composed over the already- +resolved `CALLS` graph. + +**Summary shape** (`taint/summary-model.ts`, whole-parameter granularity): + +| Edge | Meaning | Analogue | +|------|---------|----------| +| `param→return` | a param flows to the return value | TITO — **reserved** (the floor already covers its recall; precision pass deferred) | +| `param→callee-arg` | a param flows into arg *j* of a call (carries the path's neutralized sink kinds) | TITO into callee | +| `param→sink` | a param reaches a modelled sink | partial/triggered sink | +| `source→return` | the function generates+returns a source | generative — **composed** via the caller's `callResults` | +| `source→callee-arg` | a generated source flows into a call | fixpoint SEED | +| `callResults` | a user-function call's result flows to a sink/return/callee-arg in the caller | composes with callee `source→return` | + +**The fixpoint** (`taint/interproc-solver.ts`): the unit is `(function, +parameter, source)`. Seed from `source→callee-arg`, propagate via +`param→callee-arg`, fire a finding when a tainted param meets `param→sink`. + +- **Cycle-safe by monotonicity.** The tainted-set is monotone over a finite + lattice (`fn × param × source`), so the worklist converges — a recursive call + just re-proposes an already-visited entry. SCC condensation would only refine + processing order; correctness/termination don't require it. +- **Source-discriminated state (load-bearing).** Key the state by the SOURCE + too. Keying only by `(fn, param)` collapses multi-source flows: a sink param + tainted by source A is marked visited and a later flow from source B is dropped + before firing — the recurring multi-source bug class. (Bit M3; bit M4 U9.) +- **Name-based call join.** Match a summary's call-arg edge to a `CALLS` edge by + CALLEE NAME, not call-site line — line-base parity (CFG 1-based vs reference + site) is fragile; the callee identity is exact and context-insensitivity + taints the callee's param identically at every call site. +- Persisted as `TAINT_PATH` edges (Function→Function), function-level hop chain + in `reason` via the same codec; confidence < the intra-procedural 1.0. + +**Context-insensitivity** is the accepted trade-off at this tier: one summary +per function, return/call-site merging accepted (security-conservative). Expect +some FP from merging; the bigger FN sources are unmodeled features (below). + +## Known false-negative classes (documented, deferred) + +The largest is **closures/callbacks** (`arr.forEach(() => sink(y))`) — taint +into a callback is dropped without per-library models (true of CodeQL's JS libs +too). Also deferred: field/property flows (`obj.x = taint; sink(obj.y)`), +field-sensitive access paths, guard-style sanitizers, implicit/control-dependence +flows, promise/async-await threading, and **destructured/rest params before a +tainted simple param** (the summary port index is the binding ordinal, not the +formal arg position — needs a formal-param index threaded from the worker +`BindingEntry`). The interprocedural join is also context-insensitive: when one +caller invokes two distinct **same-named callees**, a flow into one +over-attributes to both (sound — over-report, never a missed flow). Absence of a +finding is NOT proof of safety. + +## GitNexus-specific gotchas + +- **Function↔CFG join.** `FunctionCfg.functionStartLine` is 1-based; `Function`/ + `Method` node `startLine` is 0-based — join at `startLine - 1`. Function nodes + have no column, so same-line functions (`{a:()=>x(), b:()=>y()}`) are + ambiguous → drop (the summary driver counts `unresolved`) rather than + cross-wire. +- **No rel-property index (S1).** Kuzu has no secondary index on relationship + properties, and unanchored `[:TAINTED*]`/`[:TAINT_PATH*]` queries explode. + TAINT_PATH is therefore MATERIALIZED + anchored at analyze time, never + traversed live; `explain` reads it source-anchored + LIMIT-guarded. +- **`explain` is the only discovery surface.** `TAINTED`/`TAINT_PATH` are + deliberately OUT of `VALID_RELATION_TYPES` (impact's allow-list) and the web + schema (pinned in `security.test.ts`). `explain` enumerates both layers + (cross-function findings carry `interprocedural: true`). +- **One shared codec.** Both the emit path and `explain` import + `taint/path-codec.ts`. Two hand-rolled copies of a wire format drift — never + fork it. New metadata extends the format WITHIN the version when writer + + reader ship together. +- **Cache versioning.** A worker-harvest shape change bumps the parse-cache pdg + NAMESPACE (`pdg:N`), NOT `SCHEMA_BUMP` (which cold-invalidates every user). + Persisted-graph/config changes ride `RepoMeta.pdg`'s key-union mismatch → + full writeback. Model content rides `taintModelVersion`. + +## Adding a source / sink / sanitizer + +Edit the language model in `taint/typescript-model.ts` (registered via the +explicit `registerBuiltinTaintModels` seam, keyed by `SupportedLanguages`). The +spec is hashable data (no functions). A sanitizer's `neutralizes` lists the +EXACT sink kinds it defends — never a blanket kill. Add a fixture + assert the +finding (or its absence) in `test/unit/taint/` (real-source harness: +`test/helpers/ts-cfg-harness.ts`); the end-to-end proof is +`test/integration/cfg/`. + +## Validation checklist for any `--pdg` change + +``` +1. tsc clean (schema additions are exhaustiveness-checked; watch the + api.ts getNodeQuery runtime read-path if a node label is added). +2. Targeted vitest by directory (test/unit/taint, test/unit/cfg, + test/integration/cfg) — verify by ISOLATION, not full-suite exit + (known load-flakes). `node scripts/build.js` before worker/integration runs. +3. Flag-off golden byte-identical (pipeline-graph-golden.test.ts). +4. bench/cfg/measure.mjs --check (no fingerprint drift / budget regression). +5. detect_changes() before commit; impact({direction:'upstream'}) before + editing shared symbols (KnowledgeGraph, RepoMeta, RelationshipType, codec). +``` + +## Prior art (for deeper design questions) + +Sharir & Pnueli 1981 (functional approach); Reps-Horwitz-Sagiv IFDS (POPL 1995); +FlowDroid/StubDroid (access-path summaries); Pysa & Mariana Trench (TITO / +propagations, parallel SCC fixpoint); CodeQL Models-as-Data (the richest port +notation, incl. callback ports); Infer (content-keyed incremental summaries). diff --git a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs index ab495be84..d497a16d9 100644 --- a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs +++ b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs @@ -241,7 +241,18 @@ function main() { if (!pattern || pattern.length < 3) return; const release = acquireHookSlot(gitNexusDir); - if (!release) return; + if (!release) { + // Normal skip path: all per-repo hook slots are held by concurrent + // sessions. Stays silent by default; surfaced only under the cursor + // hook's own GITNEXUS_DEBUG (truthy) convention. NOTE: unlike the + // claude/plugin/antigravity adapters this integration does not install + // hook-db-lock-probe.cjs, so its augment child is not guard-wrapped + // yet — tracked on the #2163 follow-up list ("cursor probe"). + if (process.env.GITNEXUS_DEBUG) { + process.stderr.write('[GitNexus] augment skipped: hook slots saturated\n'); + } + return; + } const cliPath = resolveCliPath(); let result = ''; diff --git a/gitnexus-cursor-integration/skills/gitnexus-debugging/SKILL.md b/gitnexus-cursor-integration/skills/gitnexus-debugging/SKILL.md index a88b76430..cde6a6a3a 100644 --- a/gitnexus-cursor-integration/skills/gitnexus-debugging/SKILL.md +++ b/gitnexus-cursor-integration/skills/gitnexus-debugging/SKILL.md @@ -15,10 +15,10 @@ description: Trace bugs through call chains using knowledge graph ## Workflow ``` -1. query({query: ""}) → Find related execution flows +1. query({search_query: ""}) → Find related execution flows 2. context({name: ""}) → See callers/callees/processes 3. READ gitnexus://repo/{name}/process/{name} → Trace execution flow -4. cypher({query: "MATCH path..."}) → Custom traces if needed +4. cypher({statement: "MATCH path..."}) → Custom traces if needed ``` > If "Index is stale" → run `node .gitnexus/run.cjs analyze` in terminal. @@ -49,7 +49,7 @@ description: Trace bugs through call chains using knowledge graph **query** — find code related to error: ``` -query({query: "payment validation error"}) +query({search_query: "payment validation error"}) → Processes: CheckoutFlow, ErrorHandling → Symbols: validatePayment, handlePaymentError, PaymentException ``` @@ -71,7 +71,7 @@ RETURN [n IN nodes(path) | n.name] AS chain ## Example: "Payment endpoint returns 500 intermittently" ``` -1. query({query: "payment error handling"}) +1. query({search_query: "payment error handling"}) → Processes: CheckoutFlow, ErrorHandling → Symbols: validatePayment, handlePaymentError diff --git a/gitnexus-cursor-integration/skills/gitnexus-exploring/SKILL.md b/gitnexus-cursor-integration/skills/gitnexus-exploring/SKILL.md index 73df1353a..993a38481 100644 --- a/gitnexus-cursor-integration/skills/gitnexus-exploring/SKILL.md +++ b/gitnexus-cursor-integration/skills/gitnexus-exploring/SKILL.md @@ -17,7 +17,7 @@ description: Navigate unfamiliar code using GitNexus knowledge graph ``` 1. READ gitnexus://repos → Discover indexed repos 2. READ gitnexus://repo/{name}/context → Codebase overview, check staleness -3. query({query: ""}) → Find related execution flows +3. query({search_query: ""}) → Find related execution flows 4. context({name: ""}) → Deep dive on specific symbol 5. READ gitnexus://repo/{name}/process/{name} → Trace full execution flow ``` @@ -48,7 +48,7 @@ description: Navigate unfamiliar code using GitNexus knowledge graph **query** — find execution flows related to a concept: ``` -query({query: "payment processing"}) +query({search_query: "payment processing"}) → Processes: CheckoutFlow, RefundFlow, WebhookHandler → Symbols grouped by flow with file locations ``` @@ -65,7 +65,7 @@ context({name: "validateUser"}) ``` 1. READ gitnexus://repo/my-app/context → 918 symbols, 45 processes -2. query({query: "payment processing"}) +2. query({search_query: "payment processing"}) → CheckoutFlow: processPayment → validateCard → chargeStripe → RefundFlow: initiateRefund → calculateRefund → processRefund 3. context({name: "processPayment"}) diff --git a/gitnexus-cursor-integration/skills/gitnexus-refactoring/SKILL.md b/gitnexus-cursor-integration/skills/gitnexus-refactoring/SKILL.md index 76c9d3351..fbf193182 100644 --- a/gitnexus-cursor-integration/skills/gitnexus-refactoring/SKILL.md +++ b/gitnexus-cursor-integration/skills/gitnexus-refactoring/SKILL.md @@ -16,7 +16,7 @@ description: Plan safe refactors using blast radius and dependency mapping ``` 1. impact({target: "X", direction: "upstream"}) → Map all dependents -2. query({query: "X"}) → Find execution flows involving X +2. query({search_query: "X"}) → Find execution flows involving X 3. context({name: "X"}) → See all incoming/outgoing refs 4. Plan update order: interfaces → implementations → callers → tests ``` diff --git a/gitnexus-shared/src/graph/types.ts b/gitnexus-shared/src/graph/types.ts index 86abc9eba..085c27d03 100644 --- a/gitnexus-shared/src/graph/types.ts +++ b/gitnexus-shared/src/graph/types.ts @@ -157,7 +157,21 @@ export type RelationshipType = | 'SANITIZES' /** Materialized source→sink taint path. Working name — final name/representation * is confirmed when M3/M4 emits it; no persisted edge exists before then. */ - | 'TAINT_PATH'; + | 'TAINT_PATH' + /** Control-dependence edge (PDG, issue #2085 M5): block `dependent` (target) + * executes only because the branch at block `controller` (source) took a + * given side. The branch sense (`'T'` | `'F'`) rides the relation's existing + * `reason` column — mirroring how `CFG` stores its edge kind there — since + * the single `CodeRelation` table has no dedicated label column. */ + | 'CDG' + /** Debug-only post-dominator-tree edge (#2085 M5): a block → its immediate + * post-dominator, emitted behind the `GITNEXUS_PDG_EMIT_POST_DOMINATE` env + * flag for inspection. Never emitted in a normal `--pdg` run. Note: as a + * member of this exported union it is a forward-compatibility commitment — + * removing it later is a breaking schema change — and it is deliberately + * excluded from `VALID_RELATION_TYPES` so it never enters impact-style + * symbol-space traversal (same posture as the taint substrate edges). */ + | 'POST_DOMINATE'; export interface GraphNode { id: string; diff --git a/gitnexus-shared/src/lbug/schema-constants.ts b/gitnexus-shared/src/lbug/schema-constants.ts index d022ba5c4..875f74d2e 100644 --- a/gitnexus-shared/src/lbug/schema-constants.ts +++ b/gitnexus-shared/src/lbug/schema-constants.ts @@ -77,6 +77,12 @@ export const REL_TYPES = [ 'TAINTED', 'SANITIZES', 'TAINT_PATH', + // Control dependence (PDG, issue #2085 M5) — CDG carries its 'T'|'F' branch + // label in the relation's `reason` column; POST_DOMINATE is debug-only + // (behind GITNEXUS_PDG_EMIT_POST_DOMINATE). Both are BasicBlock→BasicBlock, + // reusing the existing FROM BasicBlock TO BasicBlock pair in RELATION_SCHEMA. + 'CDG', + 'POST_DOMINATE', ] as const; export type RelType = (typeof REL_TYPES)[number]; diff --git a/gitnexus-web/src/App.tsx b/gitnexus-web/src/App.tsx index 68f7d4968..6141104b5 100644 --- a/gitnexus-web/src/App.tsx +++ b/gitnexus-web/src/App.tsx @@ -10,7 +10,7 @@ import { StatusBar } from './components/StatusBar'; import { FileTreePanel } from './components/FileTreePanel'; import { CodeReferencesPanel } from './components/CodeReferencesPanel'; import { getActiveProviderConfig } from './core/llm/settings-service'; -import { createKnowledgeGraph } from './core/graph/graph'; +import { buildGraphFromConnectResult } from './lib/apply-connect-result'; import { connectToServer, fetchRepos, @@ -21,6 +21,7 @@ import { type BackendRepo, } from './services/backend-client'; import { ERROR_RESET_DELAY_MS } from './config/ui-constants'; +import { parseSkipGraphParam } from './lib/graph-load-decision'; import { formatBackendError } from './i18n/error-messages'; import { useTranslation } from 'react-i18next'; @@ -30,6 +31,8 @@ const AppContent = () => { viewMode, setViewMode, setGraph, + setGraphMode, + setChatOnlyNodeCount, setProgress, setProjectName, progress, @@ -66,15 +69,14 @@ const AppContent = () => { setProjectName(projectName); setCurrentRepo(projectName); - // Build KnowledgeGraph from server data for visualization - const graph = createKnowledgeGraph(); - for (const node of result.nodes) { - graph.addNode(node); - } - for (const rel of result.relationships) { - graph.addRelationship(rel); - } - setGraph(graph); + // Build KnowledgeGraph from server data for visualization. In chat-only + // mode the graph download was skipped, so the shared builder keeps an + // empty (but non-null) graph and flags the mode so the UI shows the + // chat-only empty state, with the node count captured for its notice. + const built = buildGraphFromConnectResult(result); + setGraph(built.graph); + setGraphMode(built.graphMode); + setChatOnlyNodeCount(built.graphMode === 'chatOnly' ? built.nodeCount : null); // Persist the active project in the URL for bookmarkability and F5 refresh resilience const urlObj = new URL(window.location.href); @@ -84,10 +86,11 @@ const AppContent = () => { // Transition directly to exploring view setViewMode('exploring'); - // Initialize agent with backend queries, then start embeddings + // Initialize agent with backend queries, then start embeddings. Pass the + // chat-only flag so the agent's prompt matches the loaded/skipped graph (#2178). try { if (getActiveProviderConfig()) { - await initializeAgent(projectName); + await initializeAgent(projectName, { chatOnly: result.graphSkipped }); } startEmbeddingsWithFallback(); } catch (err) { @@ -97,6 +100,8 @@ const AppContent = () => { [ setViewMode, setGraph, + setGraphMode, + setChatOnlyNodeCount, setProjectName, setCurrentRepo, initializeAgent, @@ -116,6 +121,9 @@ const AppContent = () => { const params = new URLSearchParams(window.location.search); const serverUrlParam = params.get('server'); const projectParam = params.get('project'); + // `?skipGraph=1` forces chat-only, `?skipGraph=0` forces a full graph; + // absent → auto-detect by node count. Bookmarkable / survives F5 (#2178). + const skipGraphParam = parseSkipGraphParam(params.get('skipGraph')); if (!serverUrlParam && !projectParam) return; autoConnectRan.current = true; @@ -162,15 +170,19 @@ const AppContent = () => { }, undefined, projectParam || undefined, - { awaitAnalysis: true }, // enable backend hold-queue for repos still being analyzed + { awaitAnalysis: true, skipGraph: skipGraphParam }, // hold-queue + chat-only control (#2178) ); }; tryConnect() .then(async (result) => { + // Set serverBaseUrl BEFORE handleServerConnect: the latter transitions + // to 'exploring' (rendering the chat-only overlay + its "Load graph + // anyway" button) and then awaits agent init, leaving a window where + // loadGraphAnyway would silently no-op on a still-null serverBaseUrl. + setServerBaseUrl(baseUrl); await handleServerConnect(result); setProgress(null); - setServerBaseUrl(baseUrl); fetchRepos() .then((repos) => setAvailableRepos(repos)) .catch((e) => console.warn('Failed to fetch repo list:', e)); @@ -261,6 +273,9 @@ const AppContent = () => { try { const repos = await fetchRepos(); setAvailableRepos(repos); + // Auto-detect by size for a freshly-analyzed repo (#2178). A stale + // ?skipGraph from a previously-viewed repo must NOT leak in here — + // that would bypass the size guard and could re-trigger the hang. const result = await connectToServer(url, undefined, undefined, repoName); await handleServerConnect(result); setServerBaseUrl(normalizeServerUrl(url)); diff --git a/gitnexus-web/src/components/DropZone.tsx b/gitnexus-web/src/components/DropZone.tsx index 521b64115..389f99869 100644 --- a/gitnexus-web/src/components/DropZone.tsx +++ b/gitnexus-web/src/components/DropZone.tsx @@ -206,6 +206,10 @@ export const DropZone = ({ onServerConnect }: DropZoneProps) => { const abortController = new AbortController(); abortControllerRef.current = abortController; try { + // Landing-screen repo selection auto-detects by size (#2178). The + // ?skipGraph URL param is a bookmark hint for the initial auto-connect + // only; honoring a stale value for a different repo here would risk the + // hang it is meant to prevent. const result = await connectToServer( detectedBackendUrl, (p, downloaded, total) => { diff --git a/gitnexus-web/src/components/GraphCanvas.tsx b/gitnexus-web/src/components/GraphCanvas.tsx index d0880dbe1..70030eb88 100644 --- a/gitnexus-web/src/components/GraphCanvas.tsx +++ b/gitnexus-web/src/components/GraphCanvas.tsx @@ -27,6 +27,8 @@ import type { GraphNode } from 'gitnexus-shared'; import { QueryFAB } from './QueryFAB'; import Graph from 'graphology'; import { useTranslation } from 'react-i18next'; +import { LARGE_GRAPH_NODE_THRESHOLD } from '../config/ui-constants'; +import { shouldConfirmGraphLoad } from '../lib/graph-load-decision'; export interface GraphCanvasHandle { focusNode: (nodeId: string) => void; @@ -55,6 +57,9 @@ export const GraphCanvas = forwardRef((_, ref) => { animatedNodes, graphViewMode, setGraphViewMode, + graphMode, + chatOnlyNodeCount, + loadGraphAnyway, } = useAppState(); const [hoveredNodeName, setHoveredNodeName] = useState(null); @@ -193,7 +198,10 @@ export const GraphCanvas = forwardRef((_, ref) => { // Update Sigma graph when KnowledgeGraph changes useEffect(() => { - if (!graph) return; + // Skip layout work in chat-only mode: `graph` is non-null but empty, the + // overlay covers the canvas, and this guard also future-proofs against a + // transient where a populated graph is set while mode is still chat-only. + if (!graph || graphMode === 'chatOnly') return; let sigmaGraph: Graph; @@ -218,7 +226,7 @@ export const GraphCanvas = forwardRef((_, ref) => { } setSigmaGraph(sigmaGraph); - }, [graph, nodeById, setSigmaGraph, graphViewMode]); + }, [graph, graphMode, nodeById, setSigmaGraph, graphViewMode]); // Update node visibility when filters change useEffect(() => { @@ -256,6 +264,37 @@ export const GraphCanvas = forwardRef((_, ref) => { resetZoom(); }, [setSelectedNode, setSigmaSelectedNode, resetZoom]); + // Chat-only mode (#2178): the graph download was skipped. `chatOnlyNodeCount` + // comes from app state (captured at connect time), so it is authoritative and + // available immediately — not derived from the async `availableRepos` list. + const handleLoadGraphAnyway = useCallback(() => { + // Warn before re-triggering a potentially browser-hanging download. Confirm + // whenever the count is large OR unknown — never silently re-load a graph we + // can't size, which would risk re-introducing the original #2178 hang. Skip + // the prompt only when the count is known to be below the threshold (a small + // repo force-skipped via ?skipGraph=1). + const needsConfirm = shouldConfirmGraphLoad(chatOnlyNodeCount, LARGE_GRAPH_NODE_THRESHOLD); + if (needsConfirm) { + // Fail SAFE, not open: if there's no usable confirm dialog (some embedded + // webviews) or it throws, treat it as declined rather than loading a + // graph we couldn't warn about (#2178). + const canPrompt = typeof window !== 'undefined' && typeof window.confirm === 'function'; + if (!canPrompt) return; + let confirmed = false; + try { + confirmed = window.confirm( + chatOnlyNodeCount != null + ? t('canvas.chatOnly.loadAnywayWarning', { count: chatOnlyNodeCount.toLocaleString() }) + : t('canvas.chatOnly.loadAnywayWarningUnknown'), + ); + } catch { + return; + } + if (!confirmed) return; + } + void loadGraphAnyway(); + }, [chatOnlyNodeCount, loadGraphAnyway, t]); + return (
{/* Background gradient */} @@ -324,6 +363,32 @@ export const GraphCanvas = forwardRef((_, ref) => { className="sigma-container h-full w-full cursor-grab active:cursor-grabbing" /> + {/* Chat-only empty state (#2178): graph download was skipped for a large + project. Chat works normally; offer an explicit "load anyway" escape. */} + {graphMode === 'chatOnly' && ( +
+
+

+ {t('canvas.chatOnly.title')} +

+

+ {chatOnlyNodeCount != null + ? t('canvas.chatOnly.descriptionWithCount', { + count: chatOnlyNodeCount.toLocaleString(), + }) + : t('canvas.chatOnly.description')} +

+

{t('canvas.chatOnly.citationNote')}

+ +
+
+ )} + {/* Hovered node tooltip - only show when NOT selected */} {hoveredNodeName && !sigmaSelectedNode && (
diff --git a/gitnexus-web/src/components/Header.tsx b/gitnexus-web/src/components/Header.tsx index 3fae0c48f..03bdd6041 100644 --- a/gitnexus-web/src/components/Header.tsx +++ b/gitnexus-web/src/components/Header.tsx @@ -27,6 +27,7 @@ import { EmbeddingStatus } from './EmbeddingStatus'; import { RepoAnalyzer } from './RepoAnalyzer'; import { LanguageSwitcher } from './LanguageSwitcher'; import { translateProgressMessage } from '../i18n/progress'; +import { formatBackendError } from '../i18n/error-messages'; // Color mapping for node types in search results const NODE_TYPE_COLORS: Record = { @@ -58,10 +59,11 @@ export const Header = ({ onAnalyzeComplete, onReposChanged, }: HeaderProps) => { - const { t } = useTranslation(['common', 'header']); + const { t } = useTranslation(['common', 'header', 'errors']); const { projectName, graph, + graphMode, openChatPanel, isRightPanelOpen, rightPanelTab, @@ -72,6 +74,7 @@ export const Header = ({ const [isRepoDropdownOpen, setIsRepoDropdownOpen] = useState(false); const [showAnalyzer, setShowAnalyzer] = useState(false); const [reanalyzing, setReanalyzing] = useState(null); // repo name being re-analyzed + const [deleteError, setDeleteError] = useState(null); // surfaced when a delete is rejected (e.g. origin-blocked 403) const [reanalyzeProgress, setReanalyzeProgress] = useState(null); const reanalyzeSseRef = useRef(null); const repoDropdownRef = useRef(null); @@ -305,6 +308,7 @@ export const Header = ({ setReanalyzeProgress(null); reanalyzeSseRef.current = null; } + setDeleteError(null); try { await deleteRepo(repo.name); const updated = await fetchRepos(); @@ -317,7 +321,11 @@ export const Header = ({ window.location.reload(); } } catch (err) { + // Surface the failure instead of silently no-opping — + // e.g. an origin-blocked 403 when driving a local + // backend from the hosted UI. console.error('Failed to delete repo:', err); + setDeleteError(formatBackendError(err, t)); } }} className="cursor-pointer rounded p-1 text-text-muted/0 transition-all group-hover:text-text-muted hover:!text-red-400" @@ -330,6 +338,13 @@ export const Header = ({
)} + {/* Surfaced delete failure (e.g. origin-blocked 403) */} + {deleteError && ( +
+ {deleteError} +
+ )} + {/* Re-analyze progress bar */} {reanalyzing && reanalyzeProgress && (
@@ -453,8 +468,9 @@ export const Header = ({ ✨ - {/* Stats */} - {graph && ( + {/* Stats — hidden in chat-only mode, where the empty-but-non-null graph + would otherwise show a misleading "0 nodes / 0 edges" (#2178). */} + {graph && graphMode !== 'chatOnly' && (
{t('common:counts.nodes', { count: nodeCount })} {t('common:counts.edges', { count: edgeCount })} diff --git a/gitnexus-web/src/components/RightPanel.tsx b/gitnexus-web/src/components/RightPanel.tsx index 193d2c1e6..9b1fa5ab3 100644 --- a/gitnexus-web/src/components/RightPanel.tsx +++ b/gitnexus-web/src/components/RightPanel.tsx @@ -23,6 +23,7 @@ export const RightPanel = () => { isRightPanelOpen, setRightPanelOpen, graph, + graphMode, addCodeReference, // LLM / chat state chatMessages, @@ -283,6 +284,14 @@ export const RightPanel = () => {
+ {/* Chat-only notice: the graph wasn't loaded for this large project, so + inline node citations won't pin in the (absent) graph view (#2178). */} + {graphMode === 'chatOnly' && ( +
+ {t('chat:chatOnly.banner')} +
+ )} + {/* Status / errors */} {agentError && (
@@ -292,7 +301,7 @@ export const RightPanel = () => { )} {/* Messages */} -
+
{chatMessages.length === 0 ? (
@@ -417,7 +426,7 @@ export const RightPanel = () => { onKeyDown={handleKeyDown} placeholder={t('chat:input.placeholder')} rows={1} - className="scrollbar-thin min-h-[36px] flex-1 resize-none border-none bg-transparent text-sm text-text-primary outline-none placeholder:text-text-muted" + className="min-h-[36px] flex-1 resize-none scrollbar-thin border-none bg-transparent text-sm text-text-primary outline-none placeholder:text-text-muted" style={{ height: '36px', overflowY: 'hidden' }} />