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
15f1ed3f3c
5 changed files with 143 additions and 6 deletions
14
CHANGELOG.md
14
CHANGELOG.md
|
|
@ -49,7 +49,19 @@ First release. Covers the checks that a single repository can answer on its own.
|
||||||
- **The skill** (`skills/repo-standard/`) — the judgement the script cannot
|
- **The skill** (`skills/repo-standard/`) — the judgement the script cannot
|
||||||
encode: what a description should say, why the summary card must never be used
|
encode: what a description should say, why the summary card must never be used
|
||||||
to verify one, what not to retrofit, and where the per-repo boundary is.
|
to verify one, what not to retrofit, and where the per-repo boundary is.
|
||||||
- 77 tests over the pure classifiers, using measured false positives as
|
- **Link level follows the reader.** A dead relative link in a root document —
|
||||||
|
README, CHANGELOG, SECURITY — is an `ERROR`: it is in the shop window. The same
|
||||||
|
link below the root is a `WARN`. Measured across seventeen repositories: 30 of
|
||||||
|
43 findings sat in `shared/`, `docs/plan/` and `.claude/` — session plans,
|
||||||
|
agent working files, and path-traversal test fixtures whose targets are invalid
|
||||||
|
on purpose. All 30 were `ERROR`s, which is how a gate gets switched off. Only
|
||||||
|
the level moves; every finding is still reported with its file and line.
|
||||||
|
- **The repo name comes from the remote, not the directory.** `catalog/` is the
|
||||||
|
working directory of the repo named `ktg-plugin-marketplace`. Reading the
|
||||||
|
basename left it `REPO-UNREGISTERED` with zero checks run — against the one
|
||||||
|
repo every catalog rule depends on. The scp form the forge's clone button hands
|
||||||
|
out is handled, and a bare host is not a repo name.
|
||||||
|
- 84 tests over the pure classifiers, using measured false positives as
|
||||||
reference fixtures.
|
reference fixtures.
|
||||||
|
|
||||||
### Notes
|
### Notes
|
||||||
|
|
|
||||||
10
CLAUDE.md
10
CLAUDE.md
|
|
@ -57,13 +57,21 @@ would recreate, in data, exactly the drift this plugin exists to remove.
|
||||||
check only runs for its author is a broken plugin.
|
check only runs for its author is a broken plugin.
|
||||||
- **Codepoints, not bytes, not UTF-16 units.** Use `[...s].length`. An em-dash
|
- **Codepoints, not bytes, not UTF-16 units.** Use `[...s].length`. An em-dash
|
||||||
exposes only the byte layer; astral characters expose the rest.
|
exposes only the byte layer; astral characters expose the rest.
|
||||||
|
- **The reader decides a link's level, not just what is required.** Root
|
||||||
|
documents are the shop window — a dead link there is an `ERROR`. Below the
|
||||||
|
root it is a `WARN`: that is where session plans, agent working files and
|
||||||
|
path-traversal fixtures with deliberately invalid targets live. Measured, 30
|
||||||
|
of 43 findings were down there and all were `ERROR`s.
|
||||||
|
- **A repo's name is its remote, not its directory.** `catalog/` holds
|
||||||
|
`ktg-plugin-marketplace`. The basename left it unregistered with zero checks
|
||||||
|
run, against the one repo every catalog rule depends on.
|
||||||
- **No hook until the rule is precise.** A blocking gate that fails a correct
|
- **No hook until the rule is precise.** A blocking gate that fails a correct
|
||||||
repository is the mechanism that gets gates switched off.
|
repository is the mechanism that gets gates switched off.
|
||||||
|
|
||||||
## Commands
|
## Commands
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
npm test # 77 tests
|
npm test # 84 tests
|
||||||
node scripts/repo-standard-check.mjs --dir "$PWD" # gate one repo
|
node scripts/repo-standard-check.mjs --dir "$PWD" # gate one repo
|
||||||
node scripts/repo-standard-check.mjs --offline # no network call
|
node scripts/repo-standard-check.mjs --offline # no network call
|
||||||
node scripts/repo-standard-check.mjs --json # machine output
|
node scripts/repo-standard-check.mjs --json # machine output
|
||||||
|
|
|
||||||
|
|
@ -149,7 +149,7 @@ without that, a raw scan turns three dead names into about twenty.
|
||||||
npm test
|
npm test
|
||||||
```
|
```
|
||||||
|
|
||||||
77 tests over the pure classifiers. The reference fixtures are measured false
|
84 tests over the pure classifiers. The reference fixtures are measured false
|
||||||
positives, each with its expected verdict — the six that produced the
|
positives, each with its expected verdict — the six that produced the
|
||||||
three-outcome reference rule, plus the noise sources found by running the gate
|
three-outcome reference rule, plus the noise sources found by running the gate
|
||||||
against a real repository: regexes inside code spans that are markdown links to
|
against a real repository: regexes inside code spans that are markdown links to
|
||||||
|
|
|
||||||
|
|
@ -562,6 +562,17 @@ export function resolveRelative(fromFile, target) {
|
||||||
return out.join('/');
|
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
|
// 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
|
// 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.
|
// 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)) {
|
if (!have.has(resolved)) {
|
||||||
findings.push({
|
findings.push({
|
||||||
level: 'ERROR',
|
level: linkLevelFor(path),
|
||||||
code: 'LINK-INTERNAL-MISSING',
|
code: 'LINK-INTERNAL-MISSING',
|
||||||
bucket: 'broken',
|
bucket: 'broken',
|
||||||
msg: `${path}:${i + 1} — link points at \`${clean}\` (${resolved}), which is not a tracked file`,
|
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' });
|
findings.push({ level: 'OK', code: 'LINKS-INTERNAL', msg: 'every resolvable relative link resolves' });
|
||||||
}
|
}
|
||||||
return findings;
|
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) {
|
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 {
|
try {
|
||||||
return basename(execFileSync('git', ['-C', dir, 'rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim());
|
return basename(execFileSync('git', ['-C', dir, 'rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim());
|
||||||
} catch {
|
} catch {
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,7 @@ import {
|
||||||
checkInstallTruth,
|
checkInstallTruth,
|
||||||
classifyRepo,
|
classifyRepo,
|
||||||
levelOf,
|
levelOf,
|
||||||
|
parseRepoNameFromRemote,
|
||||||
} from './repo-standard-check.mjs';
|
} from './repo-standard-check.mjs';
|
||||||
|
|
||||||
const REGISTER = {
|
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);
|
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({
|
const f = checkInternalLinks({
|
||||||
files: { 'examples/demo/README.md': 'See [gone](gone.md).' },
|
files: { 'examples/demo/README.md': 'See [gone](gone.md).' },
|
||||||
present: ['examples/demo/README.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);
|
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
|
// ------------------------------- stripCode must not swallow nested list items
|
||||||
|
|
||||||
test('a link inside a nested list item is still scanned', () => {
|
test('a link inside a nested list item is still scanned', () => {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue