Skip to content

orb(content-lane): a standalone provider-file EDIT auto-merges; the same edit as a companion #9666

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

runSurfaceReview (src/review/content-lane/orchestrator.ts:301-384) handles a provider file two
different ways depending on whether it arrives alone or alongside an entry file.

Companion path — proves the provider is a debut before auto-deciding
(src/review/content-lane/orchestrator.ts:351-358):

// A companion recognized by path alone … is only a genuine DEBUT provider when it's absent at base …
// A non-null base means this PR is editing an existing, already-registered provider record alongside an
// unrelated entry — a materially different, more sensitive shape that needs a human, not the automatic
// debut-provider merge/close flow below.
if (companionProviderFile !== null && providerBaseRaw !== null) {
  return { verdict: "manual", summary: NON_DEBUT_COMPANION_SUMMARY };
}

Standalone path — never reads base at all
(src/review/content-lane/orchestrator.ts:329-336):

if (scope.isProvider) {
  const assessProvider = spec.assessProviderEntry;
  if (!assessProvider) return { verdict: "manual", summary: NO_VALIDATOR_PROVIDER_SUMMARY };
  const headRaw = await input.loadFile(directFile, "head");
  return fromProvider(assessProvider(safeParseJson(headRaw), input.opts));
}

classifyRegistryPrScope sets isProviderPr = entryFiles.length === 0 && providerFiles.length === 1
(src/review/content-lane/registry-logic.ts:792) with no added-vs-modified distinction, and
fromProvider (src/review/content-lane/orchestrator.ts:135-140) is decisive: ok means merge.

So a PR whose only change is an edit to an already-registered
registry/providers/<slug>.json — repointing website_url to an attacker-controlled host, swapping a
name, rewriting any field the shape validator does not pin — auto-merges as long as the document
still validates, while the byte-identical edit riding alongside an entry file is held for a human as
"materially different, more sensitive." Nothing upstream compensates:
runRegistrySurfaceGate (src/review/content-lane-wire.ts:212-223) only defers to the generic gate on
an unreadable file, not on a modified one.

Requirements

  • The scope.isProvider branch in runSurfaceReview must read the provider file at BOTH "head" and
    "base" and, when the base read returns a non-null body, return
    { verdict: "manual", summary: NON_DEBUT_COMPANION_SUMMARY } — the identical verdict, summary
    constant, and reasoning the companion path already uses.
  • The two reads must be issued concurrently via Promise.all, matching
    src/review/content-lane/orchestrator.ts:345-350.
  • A null base (a genuine debut provider submission) must keep today's behaviour exactly:
    fromProvider(assessProvider(...)), merge or close.
  • The existing !spec.assessProviderEntry short-circuit must still run BEFORE any fetch, so a
    spec with no provider validator still pays no I/O.

⚠️ Required pattern: mirror the companion guard at src/review/content-lane/orchestrator.ts:351-358
exactly — same providerBaseRaw !== null test, same NON_DEBUT_COMPANION_SUMMARY constant, same
verdict: "manual". What does NOT satisfy this issue: inventing a second summary constant or a new
reason code for the standalone case; making the standalone path CLOSE instead of hold; gating the new
guard behind a spec flag or env var; or "fixing" it in classifyRegistryPrScope by reclassifying a
modified provider file as mixed-files (that would close the PR outright rather than hold it, and the
classifier has no access to base content).

Deliverables

  • runSurfaceReview in src/review/content-lane/orchestrator.ts returns
    { verdict: "manual", summary: NON_DEBUT_COMPANION_SUMMARY } for an entry-free PR whose single
    provider file has a non-null base, asserted by a new named case in
    test/unit/content-lane-orchestrator.test.ts using the injected loadFile.
  • The same test file asserts an entry-free provider PR with a null base still returns the
    fromProvider verdict (merge for a valid document, close for an invalid one) — two cases.
  • A test asserts the provider head/base reads happen via a single Promise.all (assert both refs
    were requested for one call, e.g. by recording loadFile invocations).
  • A test asserts a spec with assessProviderEntry: undefined returns
    NO_VALIDATOR_PROVIDER_SUMMARY with zero loadFile calls.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for
example adding the base read and the manual verdict (Deliverable 1) without the null-base
non-regression cases (Deliverable 2) — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted, and vitest.config.ts's
coverage.include covers src/**/*.ts — this file is measured. Both arms of the new
providerBaseRaw !== null conditional need a test, as do both arms of the pre-existing
!assessProvider short-circuit on this path. The named regression test in Deliverable 1 is required.

Expected Outcome

Editing an already-registered provider record is routed to a human on every path, not only when it
happens to ride alongside an entry submission. The content lane's own stated rule — "a non-null base
means this PR is editing an existing, already-registered provider record … needs a human" — is enforced
uniformly.

Links & Resources

  • src/review/content-lane/orchestrator.ts:329-336 (the defect), :135-140, :345-358 (the sibling guard)
  • src/review/content-lane/registry-logic.ts:777-800 (classifyRegistryPrScope)
  • src/review/content-lane-wire.ts:212-223 (why nothing upstream compensates)
  • test/unit/content-lane-orchestrator.test.ts

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions