DO NOT MERGE — retrospective Copilot review of merged PR #35 (058f87e, KKO provenance gate) - #39
Conversation
* W12: make ontology/kko/PROVENANCE.md a gate instead of a record
The engine's KKO provenance is the best in the estate — source, licence, version IRI,
byte count, sha256 — and until now it was verified by NOTHING. The digest was a correct
line in a Markdown file, connected to no check.
That matters beyond this repo: prophet-platform vendors byte-identical copies of this
TBox and now asserts the SAME constant at import. The engine, the artifact's source of
truth, was the one copy where drift would have gone unnoticed.
scripts/check-kko-provenance.mjs (npm run check:kko, wired into ts-ci on every PR)
asserts three things:
1. kko-2.10.n3 still hashes to the SHA-256 the record declares, and is still the
declared byte length. The record is the source of the expectation, so record and
artifact cannot silently disagree.
2. The record pins a COMMIT. It previously cited "@ master" — a moving reference: the
same retrieval later can return different bytes while claiming this provenance.
Now pinned to 3f888b397255b69d1439fd95823e97011ed9440b (verified against upstream
2026-07-29; master has not moved since 2019-04-09, which is exactly why the
difference stays invisible until it isn't).
3. ts/src/kko-data.ts is exactly what scripts/gen-kko.mjs reproduces from that .n3 —
so the ontology the engine SHIPS derives from the file whose provenance we publish.
A hand-edit, or a re-vendor without regeneration, otherwise leaves the runtime
ontology diverged from its stated source. Same doctrine as the existing ts/dist
staleness gate.
Read-only with respect to ts/src: it regenerates into memory and compares, never writes.
Teeth verified — each check fails on a real tamper:
1. length-preserving :Monads -> :Places re-parent (valid Turtle, still 327,797 bytes) -> refused
2. commit pin reverted to "master" -> refused
3. hand-edited label in the generated kko-data.ts -> refused
clean tree -> exit 0
Note: the commit-pin check deliberately strips the sha256 from the document before
searching for a 40-hex commit — a naive scan matches a 40-char window INSIDE the 64-char
digest and would have been satisfied by the very value it is meant to be independent of.
* review: least-privilege token for ts-ci, and gate the paths the gate guards
Two findings from review of #35.
1. CodeQL actions/missing-workflow-permissions fired on the new kko-provenance
job (alert 36, ts-ci.yml:92). Accepted. Fixed at WORKFLOW level rather than
on the new job, because the same rule was already open against this file on
`main` for build-and-verify-dist (alert 33, ts-ci.yml:24) — a job-level
block would have closed the alert this PR introduced and left the one it
inherited. Declaring it once also means a job added later inherits
least-privilege instead of silently getting the default write-scoped token.
Both jobs only read the checkout and install from the public registry, so
`contents: read` is sufficient; verified both still pass locally.
2. `on: push` was path-filtered to package.json / ts/** / the workflow itself.
The kko-provenance job guards ontology/kko/** and scripts/**, so on a push
that touched ONLY those paths the gate did not run at all. It always runs on
pull_request (no path filter there), which is the merge path, so this was
not a hole in practice — but the trigger should cover what the job guards.
Added ontology/** and scripts/**.
Gate-fires proof (all six failure branches, each restored afterwards):
artifact digest drift -> rc=1
commit pin replaced by @master -> rc=1
kko-data.ts hand-edited -> rc=1
declared byte count wrong -> rc=1
declared sha256 removed -> rc=1
source repo no longer recorded -> rc=1
untouched tree -> rc=0
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an enforced provenance “gate” for the vendored KKO ontology to prevent silent drift between the recorded provenance, the vendored .n3, and the generated kko-data.ts.
Changes:
- Introduces
scripts/check-kko-provenance.mjsto validate SHA-256/bytes, ensure provenance is commit-pinned, and ensurets/src/kko-data.tsis reproducible from the.n3. - Wires the check into npm scripts (
check:kko) and GitHub Actions (kko-provenancejob). - Updates
ontology/kko/PROVENANCE.mdto pin an upstream commit and document the enforced checks.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/check-kko-provenance.mjs | New guard script enforcing provenance + reproducibility invariants. |
| package.json | Adds check:kko npm script to run the guard. |
| ontology/kko/PROVENANCE.md | Pins upstream commit and documents enforcement/regeneration checklist. |
| .github/workflows/ts-ci.yml | Runs the provenance guard in CI and expands push path filters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const withoutDigest = doc.split(declared).join('') | ||
| const commitPin = (withoutDigest.match(/\b([0-9a-f]{40})\b/) || [])[1] |
| const declaredBytes = Number(((doc.match(/\*\*Bytes:\*\*\s*([\d,]+)/) || [])[1] || '0').replace(/,/g, '')) | ||
| if (declaredBytes && bytes.length !== declaredBytes) | ||
| die(`vendored KKO TBox is ${bytes.length} bytes, PROVENANCE.md declares ${declaredBytes}`) |
| const rows = onto.classes | ||
| .map((c) => { | ||
| const label = c.label !== undefined ? `, label: ${JSON.stringify(c.label)}` : '' | ||
| return ` { iri: ${JSON.stringify(c.iri)}${label}, subClassOf: ${JSON.stringify(c.subClassOf)} },` | ||
| }) | ||
| .join('\n') | ||
|
|
||
| const expected = `// GENERATED by scripts/gen-kko.mjs from ontology/kko/kko-2.10.n3 — do not edit by hand. | ||
| // KKO ${onto.version} · ${onto.classes.length} classes. Regenerate after re-vendoring KKO. | ||
| import type { KkoClass } from './kko' | ||
|
|
||
| export const KKO_VERSION = ${JSON.stringify(onto.version)} | ||
| export const KKO_CLASSES: KkoClass[] = [ | ||
| ${rows} | ||
| ] | ||
| ` |
| if (readFileSync(dataPath, 'utf8') !== expected) | ||
| die('ts/src/kko-data.ts is NOT what scripts/gen-kko.mjs produces from ontology/kko/kko-2.10.n3.\n' + | ||
| ' The ontology the engine ships has diverged from the vendored file whose provenance we publish.\n' + | ||
| " Run: node --import tsx scripts/gen-kko.mjs then commit ts/src/kko-data.ts") |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
scripts/check-kko-provenance.mjs:55
- This currently treats a missing
**Bytes:**field as0and silently skips the byte-length verification (if (declaredBytes && ...)). Given this script is intended as a provenance gate, it should fail if**Bytes:**is absent/unparseable, not downgrade the check. Consider explicitly detecting a missing match and callingdie(...), then comparing unconditionally once parsed.
const declaredBytes = Number(((doc.match(/\*\*Bytes:\*\*\s*([\d,]+)/) || [])[1] || '0').replace(/,/g, ''))
if (declaredBytes && bytes.length !== declaredBytes)
die(`vendored KKO TBox is ${bytes.length} bytes, PROVENANCE.md declares ${declaredBytes}`)
scripts/check-kko-provenance.mjs:92
- This duplicates
scripts/gen-kko.mjs’s output formatting inline. That means any future formatting/header change in the generator will require careful synchronized updates here (or the gate risks blocking legitimate generator changes). To reduce drift, consider factoring the generator’s render logic into a shared module (e.g., export arenderKkoDataTs(onto)function) used by bothgen-kko.mjsand this check, so the gate validates exactly what the generator produces without duplicating templates.
const rows = onto.classes
.map((c) => {
const label = c.label !== undefined ? `, label: ${JSON.stringify(c.label)}` : ''
return ` { iri: ${JSON.stringify(c.iri)}${label}, subClassOf: ${JSON.stringify(c.subClassOf)} },`
})
.join('\n')
const expected = `// GENERATED by scripts/gen-kko.mjs from ontology/kko/kko-2.10.n3 — do not edit by hand.
// KKO ${onto.version} · ${onto.classes.length} classes. Regenerate after re-vendoring KKO.
import type { KkoClass } from './kko'
export const KKO_VERSION = ${JSON.stringify(onto.version)}
export const KKO_CLASSES: KkoClass[] = [
${rows}
]
`
scripts/check-kko-provenance.mjs:97
- A raw string equality check is sensitive to line endings (CRLF vs LF) and potentially a final trailing newline. This can cause local failures on Windows checkouts (e.g., with
core.autocrlf) even when the semantic content matches what the generator would write. If you want the gate to be platform-robust while still strict, normalize both strings (e.g., convert\\r\\n→\\n, and optionally enforce/normalize a single trailing newline) before comparison.
if (readFileSync(dataPath, 'utf8') !== expected)
die('ts/src/kko-data.ts is NOT what scripts/gen-kko.mjs produces from ontology/kko/kko-2.10.n3.\n' +
' The ontology the engine ships has diverged from the vendored file whose provenance we publish.\n' +
" Run: node --import tsx scripts/gen-kko.mjs then commit ts/src/kko-data.ts")
| const withoutDigest = doc.split(declared).join('') | ||
| const commitPin = (withoutDigest.match(/\b([0-9a-f]{40})\b/) || [])[1] | ||
| if (!commitPin) | ||
| die('ontology/kko/PROVENANCE.md pins no commit sha. "@ master" is a MOVING reference — the same ' + | ||
| 'retrieval later can return different bytes and still claim this provenance. Pin the commit.') |
Evaluation — and this PR should be CLOSED, not mergedThis is a retrospective review container for already-merged #35 (058f87e). Its purpose was to get Copilot's eyes on the KKO provenance gate after the fact; that has now happened twice. It must not be merged — merging it would replay merged history against Copilot coverage: 4 of 4 files — the only one of tonight's four PRs with complete coverage — though the review still carries the degraded-mode banner. Two rounds landed: 5 visible comments and 3 in the Real — and the sharpest of the night
const declaredBytes = Number(((doc.match(/\*\*Bytes:\*\*\s*([\d,]+)/) || [])[1] || '0').replace(/,/g, ''))
if (declaredBytes && bytes.length !== declaredBytes)A missing or unparseable — the byte count in that line is the actual file length, not the declared one, so the output reads as though verification happened when the record no longer declares anything to verify. For a gate whose stated job is that "the record and the artifact cannot silently disagree", a record that omits the field is the one case it must reject, and it is the one case it accepts. The irony is worth recording: eight lines below sits the comment "A checker that validates itself validates nothing; this repo's estate has been bitten by exactly that shape before." The author was alert to the defect in one place and blind to it in the next. Fix: Real, lower severity
Rejected
DispositionNone of this is fixed in this PR, because this PR is not a place to fix things. Carrying the byte-check fix and the commit-pin anchor into a PR against |
Review complete — closing, not mergingSame terminal state as #38, and for the same reason: this is byte-for-byte the already-merged content of #35 ( Copilot left 5 inline comments and 3 more inside the Findings the review produced, and where they went
Items 3 and 4 are carried forward to the estate backlog by this comment. Neither is a correctness hole in the gate today; both are ways the gate produces a wrong answer for the wrong reason later — 3 by blocking good changes, 4 by failing on a platform difference. Branches
|
DO NOT MERGE — retrospective review only
This PR exists only to obtain a Copilot review of content that is already on
main.PR #35 ("W12: make ontology/kko/PROVENANCE.md a gate instead of a record") merged as
058f87e73255cac57f26ede50fc1f8501aab2897on 2026-07-30 without a Copilot review. This repo doesnot auto-request Copilot, and Copilot cannot review a closed PR.
The diff below is byte-for-byte PR #35: base is
e4ae226(mainimmediately before the merge), headis
058f87e(the merge commit itself). Nothing here is new work.review/pr33-retrospective→e4ae226review/pr35-retrospective→058f87emain. Delete both once the review is captured.What already happened on #35
This is a missing second opinion, not a known defect.
Where a reviewer's attention is most useful
scripts/check-kko-provenance.mjsis a gate, so its failure modes matter more than its size:that sufficient if
PROVENANCE.mdever carries a second 64-hex digest?if (declaredBytes && ...)— omitting**Bytes:**from the recordsilently skips the check rather than failing.
kko-data.tsas a template string; does itstay faithful to
scripts/gen-kko.mjsif that generator changes?