From b2ae8d65227d808e1cb11f966cbbba1cb9f4acea Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 11 Jul 2026 08:05:54 +0000 Subject: [PATCH] fix(skills): apply cross-skill review findings to the gitnexus skill family MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1s: gitnexus-plan Deepen mode now re-anchors before re-pinning (diffs the old evidence pin over every [verified]-claim file and re-reads or downgrades before the header moves — moving the pin without this laundered stale claims as verified); the index-refresh budget is stated once in Phase 1 (one --index-only refresh plus at most one Phase 3 --pdg upgrade per session, Deepen = its own session) with ledger and pdg-slice deferring to it. Contract fixes: gitnexus-work's drift check now covers every file the pack cites (not just files_to_modify) and parses the full pack incl. primary/related symbols and acceptance_criteria (walked in Phase 4 alongside §13); a pre-completed check skips §7 steps already landed and Deepen gains a reconcile-execution-state step, closing the mid-execution route-back loop; pack assumptions must name what to check and how. lfg: Lane 4 passes the merge-base to detect_changes compare (two-dot diff misattributes upstream commits when default advanced), branch-diff is the stated normal case, oversized review findings route to the plan gate instead of overflowing direct mode, the one-fix-cycle cap is explicit on re-run, and headless runs end at the plan gate with the plan as deliverable. work: blank mode narrowed to *gitnexus-plan*.md with a re-execution guard, direct-mode discipline spelled out, branch meaningfulness defined against the plan slug, and the plan document is committed as the branch's docs commit (review diff includes it). Planning-only contract now names the dist/ rebuild as the second permitted state change; Phase 5.1 names the four claim tags; stale AGENTS.md anchors fixed. Known latent issue left untouched: gitnexus/gitnexus-pr-review pairs a three-dot example with a two-dot detect_changes compare — that skill is also shipped by the plugin, so fixing it here would drift the copies; lfg compensates by passing the merge-base. Co-Authored-By: Claude Fable 5 --- .claude/skills/gitnexus-lfg/SKILL.md | 31 ++++++++---- .claude/skills/gitnexus-plan/README.md | 7 +-- .claude/skills/gitnexus-plan/SKILL.md | 39 ++++++++++----- .../references/context-ledger.md | 9 ++-- .../gitnexus-plan/references/context-pack.md | 7 ++- .../gitnexus-plan/references/pdg-slice.md | 7 +-- .claude/skills/gitnexus-work/README.md | 2 +- .claude/skills/gitnexus-work/SKILL.md | 49 +++++++++++++------ 8 files changed, 101 insertions(+), 50 deletions(-) diff --git a/.claude/skills/gitnexus-lfg/SKILL.md b/.claude/skills/gitnexus-lfg/SKILL.md index 2cc1c281e..97c81f371 100644 --- a/.claude/skills/gitnexus-lfg/SKILL.md +++ b/.claude/skills/gitnexus-lfg/SKILL.md @@ -38,6 +38,10 @@ Loop on 1 as many times as the user asks. Do not proceed past the gate without an explicit choice — the gate is the pipeline's only checkpoint and exists precisely because execution is expensive to unwind. +**Headless / non-interactive runs:** no one can answer the gate, so end the +pipeline after Lane 1 — the plan file is the deliverable (gate option 3) — +and say so in the final report. Never auto-proceed to execution. + ## Lane 3 — Work Invoke `gitnexus-work` with the plan path. It re-anchors the plan at HEAD, @@ -47,17 +51,26 @@ Deepen pass and return to the Lane 2 gate rather than pushing through. ## Lane 4 — Review -Invoke `gitnexus-pr-review` on the completed work: +Invoke `gitnexus-pr-review` on the completed work. The pipeline's normal +case is a **branch-diff review** — neither lane pushes, so no PR exists +unless the user made one: -- An open PR exists for the branch (`gh pr view`) → review that PR. -- No PR → review the branch diff against the default branch - (`git diff ...HEAD` + `detect_changes {scope: "compare", - base_ref: ""}`), which the skill supports directly. +- Branch diff (normal): review the branch against its merge-base with the + default branch — `git diff ...HEAD` plus + `detect_changes {scope: "compare", base_ref: "$(git merge-base + HEAD)"}`. Pass the **merge-base**, not the branch name: `compare` runs a + two-dot diff, so a raw `` base misattributes upstream commits to + this branch whenever the default has advanced past the branch point. +- An open PR exists (`gh pr view` — the exception, e.g. a user-supplied + plan on an already-pushed branch) → review that PR instead. -Surface the review verdict and findings to the user. Findings the user wants -fixed → hand them to `gitnexus-work` as a small bounded task (direct mode), -then re-run this lane once. Do not loop unbounded; after one fix cycle, -remaining findings are reported, not silently retried. +Surface the review verdict and findings to the user. Findings the user +wants fixed: those within `gitnexus-work`'s direct-mode bounds (1–2 files, +no architectural decisions) → hand to `gitnexus-work` direct mode; anything +larger → offer the plan gate instead (Deepen the plan with the findings, or +stop). Then re-run this lane's review once. On that re-run, do not start +another fix cycle even if findings remain — report them and point the user +at `/gitnexus-work` (or the plan gate) to continue deliberately. ## Final report diff --git a/.claude/skills/gitnexus-plan/README.md b/.claude/skills/gitnexus-plan/README.md index a55a5cb43..ee9a09298 100644 --- a/.claude/skills/gitnexus-plan/README.md +++ b/.claude/skills/gitnexus-plan/README.md @@ -9,8 +9,8 @@ and the agent's native targeted source verification. | CLI | How to invoke | Adapter file | |-----|---------------|--------------| | **Claude Code** | `/gitnexus-plan ` | `.claude/skills/gitnexus-plan/SKILL.md` | -| **Codex CLI** | Ask: "run gitnexus-plan for " (Codex reads `AGENTS.md`) — or install the user-level prompt below | `AGENTS.md` § Engineering planning | -| **Any AGENTS.md-aware agent** | Ask it to "read `.claude/skills/gitnexus-plan/SKILL.md` and follow it for " | `AGENTS.md` § Engineering planning | +| **Codex CLI** | Ask: "run gitnexus-plan for " (Codex reads `AGENTS.md`) — or install the user-level prompt below | `AGENTS.md` § Engineering planning & execution | +| **Any AGENTS.md-aware agent** | Ask it to "read `.claude/skills/gitnexus-plan/SKILL.md` and follow it for " | `AGENTS.md` § Engineering planning & execution | ``` /gitnexus-plan Add retry support to the ingestion pipeline @@ -111,4 +111,5 @@ of context until the phase that needs them. (taint) or `impact {mode:"pdg"}` inter-procedural reach. - The skill is planning-only by contract: the only repository file it writes is the plan document, and the only other state it may touch is the - `.gitnexus` index store (freshness refresh) — it will not fix what it finds. + `.gitnexus` index store (freshness refresh) plus the analyzer's `dist/` + build output (runner build check) — it will not fix what it finds. diff --git a/.claude/skills/gitnexus-plan/SKILL.md b/.claude/skills/gitnexus-plan/SKILL.md index 52117fd14..2995d6792 100644 --- a/.claude/skills/gitnexus-plan/SKILL.md +++ b/.claude/skills/gitnexus-plan/SKILL.md @@ -21,9 +21,10 @@ consume without repeating the investigation. **This skill plans. It never implements.** Do not modify production code, tests, or configuration while running it. The only repository file it writes is the plan document (a working ledger kept outside the repo is fine). The -one permitted state change besides that is an index refresh via -`analyze --index-only` — it writes only the `.gitnexus` index store, never -repo files. +only other permitted state changes are the freshness gate's: an index +refresh via `analyze --index-only` (writes only the `.gitnexus` index store, +never repo files) and the analyzer `dist/` rebuild that may precede it +(build output only). ## Hard rules @@ -87,8 +88,11 @@ take the widest depth, union the focus areas. rebuild in `index_refresh`. - Stale index → run `node .gitnexus/run.cjs analyze --index-only` (append `--pdg` when the task category will reach Phase 3) and re-read the - context resource. At most **one refresh per planning session**; record - the command and outcome in the ledger's `index_refresh`. + context resource. Refresh budget, stated once here: at most one + `--index-only` refresh in Phase 1 **plus** at most one later `--pdg` + upgrade in Phase 3 (only when Phase 1's refresh lacked `--pdg`) per + planning session — a Deepen run is its own session. Record each command + and outcome in the ledger's `index_refresh`. - Refresh failed or impractical (no write access to the index, prohibitive repo size), or `freshness: accept` was passed → proceed on the stale graph, weight source verification higher, and state the staleness and @@ -163,8 +167,8 @@ and executable behavior → compiler/build/lint output → GitNexus graph and PD ## Phase 5 — Compose the plan 1. Read `references/plan-template.md` and fill all 13 sections from the - ledger, using its claim-tagging convention to distinguish confirmed facts, - evidence-backed inferences, assumptions, and open questions. + ledger, tagging claims with the template's four classes — `[verified]`, + `[graph]`, `[inferred]`, `[assumed]` — and routing open questions to §12. 2. Build the implementation context pack per `references/context-pack.md` (this is section 11 of the plan). 3. Write the document to `docs/plans/YYYY-MM-DD-gitnexus-plan-.md` under the @@ -181,19 +185,28 @@ and executable behavior → compiler/build/lint output → GitNexus graph and PD `/gitnexus-plan deepen ` strengthens an existing plan in place instead of creating a new one: -1. Re-run Phase 1 in full — runner build check, freshness gate, a new HEAD - pin for the evidence header. -2. Escalate to `depth: deep` (impact_depth 3, clusters/processes read) +1. Re-run Phase 1 in full — runner build check, freshness gate (a Deepen + run is its own session, with its own refresh budget). +2. **Re-anchor before re-pinning.** Diff the plan's old evidence pin against + current HEAD for every file backing a `[verified]` claim: unchanged files + keep the tag; changed files get their cited ranges re-read — or the claim + downgraded — *before* the header pin moves to the new HEAD. Moving the + pin without this step silently launders stale claims as verified. +3. Escalate to `depth: deep` (impact_depth 3, clusters/processes read) unless the invocation overrides knobs explicitly. -3. Seed the ledger from the plan's §11 pack, then re-verify: every +4. Seed the ledger from the plan's §11 pack, then re-verify: every `[graph]`/`[inferred]` claim gets a targeted pass toward `[verified]`; every `[assumed]` claim is resolved or kept with its reason; direct (d=1) dependent accounting is re-checked against the refreshed graph; PDG slices are built or expanded for the central functions when the layer is present. -4. Strengthen whatever the deeper pass showed thin — test scenarios, risks, +5. **Reconcile execution state.** If `gitnexus-work` already landed commits + for this plan (a mid-execution route-back), mark the §7 steps present at + HEAD as completed and re-sequence the remainder — the rewritten plan must + be executable from the top without redoing landed steps. +6. Strengthen whatever the deeper pass showed thin — test scenarios, risks, Definition of Done — and carry claim-tag upgrades through the prose. -5. Rewrite the **same file**: same 13 sections, context pack kept in sync, +7. Rewrite the **same file**: same 13 sections, context pack kept in sync, evidence header updated. Summarize the delta in chat: claims upgraded, claims that failed re-verification, sections changed. diff --git a/.claude/skills/gitnexus-plan/references/context-ledger.md b/.claude/skills/gitnexus-plan/references/context-ledger.md index ee749e933..2b5ac1434 100644 --- a/.claude/skills/gitnexus-plan/references/context-ledger.md +++ b/.claude/skills/gitnexus-plan/references/context-ledger.md @@ -20,10 +20,11 @@ context_ledger: verified_at_commit: "" # target repo HEAD, recorded once in Phase 1; # every line citation in the plan pins to it - index_refresh: "" # the one permitted analyze --index-only run: - # command + outcome (or "skipped: "), - # incl. any analyzer dist/ rebuild that - # preceded it; at most one per session + index_refresh: "" # analyze --index-only runs: command + outcome + # (or "skipped: "), incl. any analyzer + # dist/ rebuild that preceded them. Budget is + # owned by SKILL.md Phase 1: one refresh plus + # at most one Phase 3 --pdg upgrade per session established_facts: [] # each with its evidence source diff --git a/.claude/skills/gitnexus-plan/references/context-pack.md b/.claude/skills/gitnexus-plan/references/context-pack.md index 8619fdec1..46dd00385 100644 --- a/.claude/skills/gitnexus-plan/references/context-pack.md +++ b/.claude/skills/gitnexus-plan/references/context-pack.md @@ -48,7 +48,9 @@ implementation_context: # prefer npm/CI scripts that carry their pre-hooks risks: [] - assumptions: [] # faithful condensation of plan §12 assumptions + assumptions: [] # faithful condensation of plan §12 assumptions; + # each entry names WHAT to check and HOW — + # gitnexus-work re-verifies them before executing open_questions: [] # faithful condensation of plan §12 open questions avoid: @@ -67,7 +69,8 @@ implementation_context: ## Stability contract -Field names above are the interface consumed by `gitnexus-work`. Add fields +Field names above are the interface consumed by `gitnexus-work` (fields it +does not act on directly travel as executor context). Add fields freely; do not rename or repurpose existing ones. `assumptions` and `avoid` are load-bearing: an executor treats `assumptions` as things to re-verify cheaply before relying on them, and `avoid` as hard constraints. diff --git a/.claude/skills/gitnexus-plan/references/pdg-slice.md b/.claude/skills/gitnexus-plan/references/pdg-slice.md index eccb1bf14..b82197dcb 100644 --- a/.claude/skills/gitnexus-plan/references/pdg-slice.md +++ b/.claude/skills/gitnexus-plan/references/pdg-slice.md @@ -28,9 +28,10 @@ Contract caveats that shape interpretation: - No `--pdg` layer → the tools return a "no PDG layer" note, not an error. The note is repo-wide: one probe settles it — do not re-probe per function. Under `freshness: strict` (default), run - `node .gitnexus/run.cjs analyze --index-only --pdg` — once per planning - session, only if Phase 1's refresh didn't already carry `--pdg`, and with - Phase 1's runner build check applied first — then re-probe. If the refresh failed, is impractical, or `freshness: accept` was + `node .gitnexus/run.cjs analyze --index-only --pdg` — this is the one + `--pdg` upgrade SKILL.md Phase 1's refresh budget allows (skip it if + Phase 1 already refreshed with `--pdg`; apply the runner build check + first) — then re-probe. If the refresh failed, is impractical, or `freshness: accept` was passed: record "PDG unavailable" in the ledger, skip the slice, say so in plan §5, and recommend the command. Never reconstruct edges from source by hand. diff --git a/.claude/skills/gitnexus-work/README.md b/.claude/skills/gitnexus-work/README.md index 3b53960a7..ed91f647e 100644 --- a/.claude/skills/gitnexus-work/README.md +++ b/.claude/skills/gitnexus-work/README.md @@ -11,7 +11,7 @@ relying on it. | CLI | How to invoke | |-----|---------------| -| **Claude Code** | `/gitnexus-work [plan path]` (blank → newest `docs/plans/*.md`) | +| **Claude Code** | `/gitnexus-work [plan path]` (blank → newest `docs/plans/*gitnexus-plan*.md` in this repo) | | **Codex CLI** | Ask: "run gitnexus-work on " (Codex reads `AGENTS.md`), or install the skill user-level (below) | ### Codex (user-level install) diff --git a/.claude/skills/gitnexus-work/SKILL.md b/.claude/skills/gitnexus-work/SKILL.md index 720f2a132..4f8d789f2 100644 --- a/.claude/skills/gitnexus-work/SKILL.md +++ b/.claude/skills/gitnexus-work/SKILL.md @@ -13,30 +13,39 @@ executor counterpart to the planning-only `gitnexus-plan`. ``` /gitnexus-work # execute this plan -/gitnexus-work # execute the newest docs/plans/*.md +/gitnexus-work # newest docs/plans/*gitnexus-plan*.md here /gitnexus-work # direct mode, see Input triage ``` ## Input triage -- **Plan path** (or blank → newest `docs/plans/*.md`): the normal mode; - continue to Phase 1. +- **Plan path** (or blank → the newest `docs/plans/*gitnexus-plan*.md` under + the current repo root; plans written elsewhere — `out:` override, other + target repo — must be passed by explicit path): the normal mode; continue + to Phase 1. If Phase 1's pre-completed check finds every §7 step of that + newest plan already landed, stop and ask instead of re-executing it. - **Bare task text**: trivial and bounded (1–2 files, no architectural - decisions) → implement directly, still honoring the Execution discipline - below. Anything larger → recommend running `/gitnexus-plan` first; honor - the user's choice if they decline. + decisions) → implement directly with the same discipline: `impact` before + every symbol edit, minimal change, tests when behavior changes, + verification commands taken from the repo's own scripts (package.json / + CI), `detect_changes` before every commit. Anything larger → recommend + running `/gitnexus-plan` first; honor the user's choice if they decline. ## Phase 1 — Load and re-anchor the plan 1. Read the plan document completely. It is a decision artifact, not a script: scope boundaries and `avoid` entries bind you; exact code is yours to write. Never edit the plan body. -2. Parse the §11 `implementation_context` pack: `files_to_modify`, +2. Parse the §11 `implementation_context` pack: `acceptance_criteria`, + `primary_symbols`, `related_symbols`, `files_to_modify`, `execution_path`, `pdg_constraints`, `architectural_patterns`, `tests`, `verification_commands`, `risks`, `assumptions`, `open_questions`, `avoid`. 3. **Drift check.** The plan header pins the commit its evidence was verified at. If HEAD has moved since, diff the pinned commit against HEAD - for the pack's `files_to_modify` — untouched files keep their verified + for **every file the pack cites** — `files_to_modify`, + `primary_symbols`/`related_symbols` files, files named in + `pdg_constraints.affected_statements`, `architectural_patterns[]` + example locations, `tests[].file` — untouched files keep their verified status; changed files get their cited ranges re-read before you rely on them. Material drift (a planned seam no longer exists) → stop and send the plan back through `gitnexus-plan` Deepen mode. @@ -45,11 +54,21 @@ executor counterpart to the planning-only `gitnexus-plan`. depend on it, not something to code around silently. 5. Note `open_questions` — if one blocks a step and the answer materially changes the work, ask the user before that step, not after. +6. **Pre-completed check.** If commits for this plan already exist on the + branch (a prior partial run, or a post-route-back Deepen cycle), verify + which §7 steps have landed at HEAD: those are skipped and reported as + pre-completed, and execution resumes at the first unlanded step. All + steps landed → report that and stop. ## Phase 2 — Environment - On the default branch → create a feature branch named from the plan slug. - Already on a meaningful feature branch → stay on it. + On a feature branch already → stay only if it is meaningful *for this + plan* (name matches the plan slug, or the user confirms); otherwise + branch from here with the slug name. +- If the plan document is not yet committed, commit it now + (`docs(plans): add plan`) — the plan travels with the work it + drives, and the final review diff then includes it. - Confirm the `verification_commands` from the pack actually run in this checkout (dependencies installed, builds present) before starting, not after the last step. @@ -91,9 +110,9 @@ choice isn't obvious. 1. Run the full `verification_commands` suite once, at the end, even if every step already passed individually. -2. Walk plan §13 (Definition of Done) item by item; anything unmet is - either finished now or reported as explicitly unmet — never silently - dropped. +2. Walk plan §13 (Definition of Done) and the pack's `acceptance_criteria` + item by item; anything unmet is either finished now or reported as + explicitly unmet — never silently dropped. 3. Report: steps completed, commits made, deviations from the plan (with why), assumptions that failed re-verification, DoD status, and anything deferred. Test failures are reported with their output, not smoothed @@ -101,8 +120,8 @@ choice isn't obvious. ## Never -- Edit a symbol without the Phase 3 impact check, or commit without +- Skip the Phase 3 gates: no symbol edit without `impact`, no commit without `detect_changes`. - Expand scope beyond the plan — §12's deferred follow-ups stay deferred. -- Mutate the plan document, weaken failing tests, or present unverified - work as verified. +- Mutate the plan body (committing the file verbatim in Phase 2 is not + mutation), weaken failing tests, or present unverified work as verified.