mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-10 03:27:59 +00:00
fix(skills): apply cross-skill review findings to the gitnexus skill family
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 <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar
parent
a70e531222
commit
b2ae8d6522
8 changed files with 101 additions and 50 deletions
|
|
@ -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 <default>...HEAD` + `detect_changes {scope: "compare",
|
||||
base_ref: "<default>"}`), which the skill supports directly.
|
||||
- Branch diff (normal): review the branch against its merge-base with the
|
||||
default branch — `git diff <default>...HEAD` plus
|
||||
`detect_changes {scope: "compare", base_ref: "$(git merge-base <default>
|
||||
HEAD)"}`. Pass the **merge-base**, not the branch name: `compare` runs a
|
||||
two-dot diff, so a raw `<default>` 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
|
||||
|
||||
|
|
|
|||
|
|
@ -9,8 +9,8 @@ and the agent's native targeted source verification.
|
|||
| CLI | How to invoke | Adapter file |
|
||||
|-----|---------------|--------------|
|
||||
| **Claude Code** | `/gitnexus-plan <task>` | `.claude/skills/gitnexus-plan/SKILL.md` |
|
||||
| **Codex CLI** | Ask: "run gitnexus-plan for <task>" (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 <task>" | `AGENTS.md` § Engineering planning |
|
||||
| **Codex CLI** | Ask: "run gitnexus-plan for <task>" (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 <task>" | `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.
|
||||
|
|
|
|||
|
|
@ -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-<slug>.md` under the
|
||||
|
|
@ -181,19 +185,28 @@ and executable behavior → compiler/build/lint output → GitNexus graph and PD
|
|||
`/gitnexus-plan deepen <plan-path>` 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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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: <reason>"),
|
||||
# incl. any analyzer dist/ rebuild that
|
||||
# preceded it; at most one per session
|
||||
index_refresh: "" # analyze --index-only runs: command + outcome
|
||||
# (or "skipped: <reason>"), 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
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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 <plan path>" (Codex reads `AGENTS.md`), or install the skill user-level (below) |
|
||||
|
||||
### Codex (user-level install)
|
||||
|
|
|
|||
|
|
@ -13,30 +13,39 @@ executor counterpart to the planning-only `gitnexus-plan`.
|
|||
|
||||
```
|
||||
/gitnexus-work <plan path> # execute this plan
|
||||
/gitnexus-work # execute the newest docs/plans/*.md
|
||||
/gitnexus-work # newest docs/plans/*gitnexus-plan*.md here
|
||||
/gitnexus-work <small task text> # 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 <slug> 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.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue