fix(gate): the reader sets a link's level, and a repo's name is its remote
Two defects that only org-wide measurement exposes. Both were found by running the gate against seventeen repositories, not by reading the code. Link level follows the reader. 30 of 43 LINK-INTERNAL-MISSING findings sat in `shared/`, `docs/plan/` and `.claude/` -- session plans, agent working files, and path-traversal fixtures whose targets are invalid on purpose. All 30 were ERRORs. portfolio-optimiser scored 19 ERRORs and 15 of them were noise. A gate that wrong gets switched off, so root documents stay ERROR and everything below becomes WARN. Only the level moves; the finding keeps its file and line. The OK line had to move with it: it asserted "every resolvable relative link resolves" whenever no ERROR was present, which would have printed it beside a pile of WARNs saying the opposite. The repo name comes from `git remote get-url origin`. `catalog/` is the working directory of `ktg-plugin-marketplace`; the basename left it REPO-UNREGISTERED with zero checks run -- against the one repo every catalog rule depends on. The scp form is handled because the forge's clone button hands it out, and a bare host is not a repo name (that test caught a real bug: `https://host/` parsed as a repo named after the host). Re-measured: portfolio-optimiser 19 ERROR -> 4, all four genuine. catalog now reports as `ktg-plugin-marketplace [catalog]` and runs its full rule set. Still v0.1.0: nothing is pushed, no tag exists, no consumer has seen it. 77 -> 84 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EeNsGsWukggfmLQ926WZPx
This commit is contained in:
parent
cf2382287a
commit
9fcceb7522
5 changed files with 143 additions and 6 deletions
|
|
@ -562,6 +562,17 @@ export function resolveRelative(fromFile, target) {
|
|||
return out.join('/');
|
||||
}
|
||||
|
||||
// Who the reader is decides the level. A dead link in a root document — README,
|
||||
// CHANGELOG, SECURITY — is in the shop window and blocks a stranger. The same
|
||||
// link three directories down is in a session plan, an agent working file, or a
|
||||
// test fixture whose target is invalid ON PURPOSE. Measured across the org: 30
|
||||
// of 43 findings sat below the root, and every one of them was an ERROR. A gate
|
||||
// that is wrong that often gets switched off, so the level moves — and only the
|
||||
// level. The finding is still reported, with its file and line.
|
||||
function linkLevelFor(path) {
|
||||
return String(path).includes('/') ? 'WARN' : 'ERROR';
|
||||
}
|
||||
|
||||
// Relative file links only. Anchor resolution depends on per-renderer heading
|
||||
// slug rules and is a rabbit hole; external URLs need the network. Both are
|
||||
// deliberately out — a check that is sometimes wrong teaches people to ignore it.
|
||||
|
|
@ -591,7 +602,7 @@ export function checkInternalLinks({ files, present }) {
|
|||
}
|
||||
if (!have.has(resolved)) {
|
||||
findings.push({
|
||||
level: 'ERROR',
|
||||
level: linkLevelFor(path),
|
||||
code: 'LINK-INTERNAL-MISSING',
|
||||
bucket: 'broken',
|
||||
msg: `${path}:${i + 1} — link points at \`${clean}\` (${resolved}), which is not a tracked file`,
|
||||
|
|
@ -600,7 +611,9 @@ export function checkInternalLinks({ files, present }) {
|
|||
}
|
||||
});
|
||||
}
|
||||
if (!findings.some((f) => f.level === 'ERROR')) {
|
||||
// The OK line asserts that every link resolved. Keying it on ERROR alone would
|
||||
// have printed it beside a pile of WARN findings saying the opposite.
|
||||
if (!findings.some((f) => f.code === 'LINK-INTERNAL-MISSING')) {
|
||||
findings.push({ level: 'OK', code: 'LINKS-INTERNAL', msg: 'every resolvable relative link resolves' });
|
||||
}
|
||||
return findings;
|
||||
|
|
@ -706,7 +719,32 @@ function gitFiles(dir) {
|
|||
}
|
||||
}
|
||||
|
||||
// The remote is ground truth for what a repo is CALLED; the directory is only
|
||||
// where it happens to sit. `catalog/` is the working directory of the repo named
|
||||
// `ktg-plugin-marketplace`, and deriving the name from the basename left it
|
||||
// REPO-UNREGISTERED — zero checks run against the one repo the catalog rule
|
||||
// depends on. Handles the scp form too: the forge's clone button hands it out.
|
||||
export function parseRepoNameFromRemote(url) {
|
||||
const raw = String(url ?? '').trim();
|
||||
if (!raw) return null;
|
||||
const path = raw.includes('://') ? raw.split('://')[1] : raw;
|
||||
const segments = path.replace(/\/+$/, '').split(/[/:]/).filter(Boolean);
|
||||
// A bare host is not a repository. Without this, `https://the-forge/` parsed
|
||||
// as a repo named after the host and every class rule matched the wrong thing.
|
||||
if (segments.length < 2) return null;
|
||||
const name = segments.pop().replace(/\.git$/, '');
|
||||
return name || null;
|
||||
}
|
||||
|
||||
function repoNameFrom(dir) {
|
||||
try {
|
||||
const remote = execFileSync('git', ['-C', dir, 'remote', 'get-url', 'origin'], {
|
||||
encoding: 'utf8',
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
});
|
||||
const fromRemote = parseRepoNameFromRemote(remote);
|
||||
if (fromRemote) return fromRemote;
|
||||
} catch { /* no remote yet — a repo before its first push is the ordinary case */ }
|
||||
try {
|
||||
return basename(execFileSync('git', ['-C', dir, 'rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim());
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ import {
|
|||
checkInstallTruth,
|
||||
classifyRepo,
|
||||
levelOf,
|
||||
parseRepoNameFromRemote,
|
||||
} from './repo-standard-check.mjs';
|
||||
|
||||
const REGISTER = {
|
||||
|
|
@ -689,7 +690,7 @@ test('a link from a nested README to its sibling resolves', () => {
|
|||
assert.equal(f.filter((x) => x.level === 'ERROR').length, 0);
|
||||
});
|
||||
|
||||
test('a nested link to a genuinely absent sibling is still an ERROR', () => {
|
||||
test('a nested link to a genuinely absent sibling is still reported', () => {
|
||||
const f = checkInternalLinks({
|
||||
files: { 'examples/demo/README.md': 'See [gone](gone.md).' },
|
||||
present: ['examples/demo/README.md'],
|
||||
|
|
@ -697,6 +698,84 @@ test('a nested link to a genuinely absent sibling is still an ERROR', () => {
|
|||
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-MISSING'), true);
|
||||
});
|
||||
|
||||
// ------------------------------------------- link level follows the reader
|
||||
// Measured: 30 of 43 LINK-INTERNAL-MISSING across the org sat in `shared/`,
|
||||
// `docs/plan/` and `.claude/` — path-traversal test fixtures with deliberately
|
||||
// invalid targets, internal session plans, agent working files. Only 12 were in
|
||||
// a README. Every one of those 30 was an ERROR, which is how a gate gets
|
||||
// switched off. Who the reader is decides what is required: the repo root is
|
||||
// the shop window, everything below it is internal.
|
||||
|
||||
test('a dead link in a root document blocks a stranger — ERROR', () => {
|
||||
const f = checkInternalLinks({ files: { 'README.md': '[e](docs/gone.md)' }, present: ['README.md'] });
|
||||
const hit = f.find((x) => x.code === 'LINK-INTERNAL-MISSING');
|
||||
assert.equal(hit.level, 'ERROR');
|
||||
assert.equal(hit.bucket, 'broken');
|
||||
});
|
||||
|
||||
test('a dead link below the root is a WARN, not an ERROR', () => {
|
||||
// `shared/examples/nav-golden-escape/` in portfolio-optimiser links at
|
||||
// `../../../../etc/passwd` ON PURPOSE — it is the fixture for a path-traversal
|
||||
// test. Nineteen ERRORs against that repo were almost all this.
|
||||
const f = checkInternalLinks({
|
||||
files: { 'docs/plan/session.md': '[x](../gone.md)' },
|
||||
present: ['docs/plan/session.md'],
|
||||
});
|
||||
const hit = f.find((x) => x.code === 'LINK-INTERNAL-MISSING');
|
||||
assert.equal(hit.level, 'WARN');
|
||||
assert.equal(hit.bucket, 'broken');
|
||||
});
|
||||
|
||||
test('only the level moves — an internal dead link is never silently dropped', () => {
|
||||
const f = checkInternalLinks({
|
||||
files: { 'docs/a.md': '[x](gone.md)' },
|
||||
present: ['docs/a.md'],
|
||||
});
|
||||
assert.equal(f.filter((x) => x.code === 'LINK-INTERNAL-MISSING').length, 1);
|
||||
assert.match(f.find((x) => x.code === 'LINK-INTERNAL-MISSING').msg, /docs\/a\.md:1/);
|
||||
});
|
||||
|
||||
test('WARN-only links must not also report that every link resolves', () => {
|
||||
// The OK line said "every resolvable relative link resolves" whenever there
|
||||
// was no ERROR. Degrading below-root links to WARN would have made that line
|
||||
// appear beside its own counter-evidence.
|
||||
const f = checkInternalLinks({
|
||||
files: { 'docs/a.md': '[x](gone.md)' },
|
||||
present: ['docs/a.md'],
|
||||
});
|
||||
assert.equal(f.some((x) => x.code === 'LINKS-INTERNAL'), false);
|
||||
});
|
||||
|
||||
// ------------------------------------------------ the repo name is the remote
|
||||
// `catalog/` is the working directory of the repo named `ktg-plugin-marketplace`.
|
||||
// Deriving the name from the directory basename left it REPO-UNREGISTERED and
|
||||
// ran zero checks against the one repo the whole catalog rule depends on.
|
||||
|
||||
test('the repo name comes from the remote, not the directory', () => {
|
||||
assert.equal(
|
||||
parseRepoNameFromRemote('ssh://git@git.fromaitochitta.com/open/ktg-plugin-marketplace.git'),
|
||||
'ktg-plugin-marketplace',
|
||||
);
|
||||
assert.equal(
|
||||
parseRepoNameFromRemote('https://git.fromaitochitta.com/open/repo-standard.git'),
|
||||
'repo-standard',
|
||||
);
|
||||
});
|
||||
|
||||
test('remote parsing handles the scp form and a missing .git suffix', () => {
|
||||
// The forge UI hands out the scp form from its clone button.
|
||||
assert.equal(parseRepoNameFromRemote('git@git.fromaitochitta.com:open/okr.git'), 'okr');
|
||||
assert.equal(parseRepoNameFromRemote('https://git.fromaitochitta.com/open/okr'), 'okr');
|
||||
assert.equal(parseRepoNameFromRemote('https://git.fromaitochitta.com/open/okr/'), 'okr');
|
||||
});
|
||||
|
||||
test('an absent or unparseable remote yields null, so the caller can fall back', () => {
|
||||
assert.equal(parseRepoNameFromRemote(''), null);
|
||||
assert.equal(parseRepoNameFromRemote(null), null);
|
||||
assert.equal(parseRepoNameFromRemote(' '), null);
|
||||
assert.equal(parseRepoNameFromRemote('https://git.fromaitochitta.com/'), null);
|
||||
});
|
||||
|
||||
// ------------------------------- stripCode must not swallow nested list items
|
||||
|
||||
test('a link inside a nested list item is still scanned', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue