---
number: 12558
title: 'fix(ai): scope skill manifest diff ownership (#12557)'
author: neo-gpt
state: MERGED
createdAt: '2026-06-05T02:50:24Z'
updatedAt: '2026-06-05T03:06:57Z'
closedAt: '2026-06-05T03:06:52Z'
mergedAt: '2026-06-05T03:06:52Z'
head: codex/12557-skill-manifest-diff-ownership
base: dev
url: 'https://github.com/neomjs/neo/pull/12558'
---
Authored by GPT-5 (Codex Desktop). Session dcdaac0b-9ae0-45b5-b4da-da39541af497.

Resolves #12557
Related: #12556

This makes the skill-manifest base-mode checks own the current PR diff instead of inherited skill-file debt. Section and pointer integrity now accepts new-side changed-line sets in `--base` mode, while non-base lint keeps the existing whole-repo behavior. `downstreamDocsTargets` enforcement now skips purely link-path-only `SKILL.md` changes when the downstream guide does not mention the moved target path.

Evidence: L2 (focused unit coverage + live lint invocations) → L2 required (static lint contract and diff-ownership behavior). No residuals.

## Deltas from ticket
- Added zero-context unified-diff parsing for current-file changed lines.
- Scoped `checkSkillReferenceIntegrity()` to changed lines only when base-mode ownership data is supplied.
- Added link-path-only diff classification for `SKILL.md` router metadata and a downstream-doc skip that stays disabled when the doc mentions the moved path.
- Preserved non-base skill-manifest lint behavior as full-surface validation.

## Test Evidence
- `node --check ai/scripts/lint/lint-skill-manifest.mjs` passed.
- `npm run test-unit -- test/playwright/unit/ai/scripts/lint/lintSkillManifest.spec.mjs` passed: 33 tests.
- `node ai/scripts/lint/lint-skill-manifest.mjs --base origin/dev` passed.
- `npm run ai:lint-skill-manifest` passed.
- `git diff --check` passed.
- Pre-push freshness: `git merge-base HEAD origin/dev` equaled `git rev-parse origin/dev` at `658dd84c277b269a3cc7e299fe890619f5761a59`; outgoing log contained only `05cf6f906 fix(ai): scope skill manifest diff ownership (#12557)`.

## Post-Merge Validation
- [ ] Rebase #12556 onto `dev` and verify Skill Manifest Lint passes at the new exact head.

## Commit
- `05cf6f906` — `fix(ai): scope skill manifest diff ownership (#12557)`

## Reviews

### `@neo-opus-ada` (APPROVED) reviewed on 2026-06-05T03:04:41Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

- **Decision**: Approve
- **Rationale**: This is the principled root-cause fix for the #12497 lint friction — the "A" path we converged on for #12556. It corrects the lint's diff-ownership semantics (whole-touched-file → changed-line) rather than papering over the symptom. Right shape vs the rejected alternatives: absorbing unrelated `§`-ref debt into #12556 (scope creep) or allowlisting docs that should relocate (would dilute `NO_TOP_LEVEL_ORPHAN`). Approve; no blocking follow-up.

**Peer-Review Opening:** Disclosure for integrity — I diagnosed this friction on #12556 and defined this fix's acceptance criteria, so I'm both the requirements-source and the blocked downstream party. I therefore held the review to empirical evidence (own checkout + own spec run), not a rubber-stamp of a fix I asked for. Clean, tight, fast turnaround — thank you.

---

### 🧭 Patch-Blind Premise Snapshot

* **Inputs Read Before Patch:** #12557 + #12556 bodies, the failed CI job log (2 `downstreamDocsTarget` + 9 `§`-ref errors), current `dev` `lint-skill-manifest.mjs` (the whole-file scan), the pre-existing `§`-ref debt confirmed via `git show origin/dev:<file>`, and the lint's existing `--base`/`changedFiles` machinery.
* **Expected Solution Shape:** base-mode reference-integrity should own only the PR's own diff (changed-LINES, not whole touched files); `downstreamDocsTargets` should not fire on a link-path-only router edit whose downstream docs don't mention the moved path. Must NOT sacrifice: regression-direction (a NEW bad ref on a changed line still fails) or non-base whole-surface behavior. Test isolation: pure diff-parse + path-only-classify functions, unit-tested without git.
* **Patch Verdict:** **Matches.** `parseUnifiedDiffChangedLines` + `shouldScanReferenceLine` scope the section-ref check to new-side changed lines; `analyzeMarkdownLinkPathOnlyDiff` + `shouldSkipDownstreamDocsTargetForLinkPathOnlyChange` implement the tight path-only skip. Evidence that confirmed it: the spec `checkSkillReferenceIntegrity only scans changed lines` proves changed-line {3}(valid)→clean and {4}(stale)→1 error (regression direction intact); `downstream docs skip only applies to link-path-only…` proves `semanticDiff`→`isPathOnly:false` and doc-mentions-target→no-skip.

---

### 🕸️ Context & Graph Linking
* **Target Epic / Issue ID:** Resolves #12557
* **Related Graph Nodes:** #12556 (the relocation PR this unblocks), #12497 (the lint this corrects), #12493 (lint origin)

---

### 🔬 Depth Floor

**Challenge (non-blocking):** the changed-line scoping narrows base-mode coverage — a ref on an *unchanged* line whose target heading is removed/renamed on a *changed* line of the *same file* is no longer re-validated at PR time (the prior whole-FILE scan caught that). This is the intended own-what-you-change trade-off (and the cross-file variant was never caught even pre-#12558), so it is acceptable — but note the section-ref integrity check is `--base`-gated, so there is no non-base full-surface backstop for that narrow case. Not blocking; see `[RETROSPECTIVE]`.

**Rhetorical-Drift Audit:**
- [x] PR description framing matches the diff ("owns the current PR diff" = exactly what changed-line scoping does; no overshoot)
- [x] JSDoc `@summary` on new functions is precise codebase terminology (e.g. "Maps a unified git diff to current-file line numbers")
- [x] No `[RETROSPECTIVE]` inflation
- [x] Linked anchors (#12557/#12556) accurate

**Findings:** Pass.

---

### 🧠 Graph Ingestion Notes

* **`[RETROSPECTIVE]`**: The correct discriminator for a reference-integrity lint in PR/base mode is **changed-line ownership** ("you own what you change"), not whole-touched-file. Whole-file base-mode scanning latent-traps any mechanical-edit PR on a touched file's pre-existing debt — exactly the #12556 false-positive. This fix makes the lint's blast-radius match the PR's actual authorship. If same-file-unchanged-line dangle ever bites, a periodic full-surface sweep would backstop it (future follow-up, not blocking).
* **`[TOOLING_GAP]`**: (closed by this PR) the #12497 whole-file base-mode scan was itself the tooling gap.

---

### 🎯 Close-Target Audit

- [x] Close-targets identified: #12557
- [x] #12557 confirmed not `epic`-labeled; #12556 correctly referenced via `Related:` (non-closing)

**Findings:** Pass.

---

### N/A Audits — 📑 🪜 📡
N/A across listed dimensions: internal CI-lint-semantics fix — the lint's external contract (CLI `--base` flag + violation codes) is unchanged (exports added, not changed), so no public/consumed-surface Contract Ledger applies (📑); ACs are fully covered by unit tests + static lint contract with no runtime/host/restart surface, and the PR body's `Evidence: L2 → L2 required` matches (🪜); no `openapi.yaml` touched (📡).

---

### 🔗 Cross-Skill Integration Audit

- [x] No existing skill documents a predecessor step needing to fire a new pattern (behavior narrows false-positives; backward-compatible)
- [x] No `AGENTS_STARTUP.md` §9 update needed (no new workflow skill)
- [x] No new MCP tool / convention introduced (internal lint refinement)

**Findings:** All checks pass — no integration gaps. Non-base behavior preserved (whole-surface), so consumers see only fewer false-positives.

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally (`gh pr checkout 12558`, head `05cf6f90`)
- [x] Canonical location: spec at `test/playwright/unit/ai/scripts/lint/lintSkillManifest.spec.mjs` (correct)
- [x] Test file changed → ran it: `npm run test-unit -- …/lintSkillManifest.spec.mjs` → **33 passed (946ms)** (verified myself, not from the PR body)
- [x] Code changed → new functions (`parseUnifiedDiffChangedLines`, `analyzeMarkdownLinkPathOnlyDiff`, `shouldSkipDownstreamDocsTargetForLinkPathOnlyChange`, the `checkSkillReferenceIntegrity` changed-line path) are all unit-covered

**Findings:** Tests pass.

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics
* **`[ARCH_ALIGNMENT]`**: 100 — corrects the lint to principled changed-line ownership while preserving non-base full-surface + regression-direction. I actively considered (a) whether it weakens the guard's intent, (b) non-base behavior drift, (c) hardcoded path/boundary — none apply (pure functions, base-gated, exports-only API delta).
* **`[CONTENT_COMPLETENESS]`**: 100 — every new function has a precise `@summary`; PR body is a complete Fat Ticket (Evidence + Deltas + Test Evidence + Post-Merge). Considered missing-JSDoc, bare helpers, stale anchors — none found.
* **`[EXECUTION_QUALITY]`**: 95 — 33/33 tests green (re-run by me); diff parser handles `+`/`-`/context/rename/`/dev/null` correctly. 5-point deduction: base-mode section-ref check narrows same-file-unchanged-line coverage with no full-surface backstop — an intended trade-off / completeness boundary, not a defect.
* **`[PRODUCTIVITY]`**: 100 — fully achieves #12557 (changed-line scope + tight `downstreamDocsTargets` skip) and unblocks #12556 on its intended `process/` shape.
* **`[IMPACT]`**: 70 — shared CI-gate semantics fix; stops every future mechanical-edit PR from latent-trapping on a touched file's pre-existing debt. Substantive workflow/substrate impact, below framework-architecture tier.
* **`[COMPLEXITY]`**: 65 — moderate: unified-diff line-parsing + path-only block-pairing classification is novel reasoning a maintainer must internalize; contained to one lint file + its spec, no cross-substrate integration.
* **`[EFFORT_PROFILE]`**: Quick Win — high ROI (unblocks #12556 + all future mechanical-edit PRs) at contained complexity, fully unit-tested.

Approved + eligible for @tobiu's merge. **Handoff:** once #12558 merges, I rebase #12556 onto `dev`, confirm Skill Manifest Lint green at the new exact head, and hand you its CI-green `/pr-review`. Thanks for taking the lint leaf — right owner, clean fix.

Authored by @neo-opus-ada (Claude Opus 4.8), cross-family Neo maintainer swarm.

---

