From 2111b559cc7898d2d1b4be001778d77b86d547e4 Mon Sep 17 00:00:00 2001 From: Callum Flack <749885+callumflack@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:58:18 +1000 Subject: [PATCH 1/5] feat(cli): prepare and submit connector PRs BUI-942 Assisted-by: AI Signed-off-by: Callum Flack <749885+callumflack@users.noreply.github.com> --- src/cli/connector-pr-prep.ts | 547 +++++++++++++++++++++++++++++ src/cli/connector-pr-submit.ts | 375 ++++++++++++++++++++ src/cli/index.ts | 4 +- test/cli/connector-pr-prep.test.ts | 413 ++++++++++++++++++++++ 4 files changed, 1338 insertions(+), 1 deletion(-) create mode 100644 src/cli/connector-pr-prep.ts create mode 100644 src/cli/connector-pr-submit.ts create mode 100644 test/cli/connector-pr-prep.test.ts diff --git a/src/cli/connector-pr-prep.ts b/src/cli/connector-pr-prep.ts new file mode 100644 index 0000000..dfec1f8 --- /dev/null +++ b/src/cli/connector-pr-prep.ts @@ -0,0 +1,547 @@ +import { spawnSync } from "node:child_process"; +import { readFileSync, realpathSync } from "node:fs"; +import { isDeepStrictEqual } from "node:util"; +import type { Command } from "commander"; +import { z } from "zod"; + +import { CliExitCode } from "../core/exit-codes.js"; +import { + connectorPrTarget, + SubmitError, + submitConnectorPr, +} from "./connector-pr-submit.js"; +import { isNewerVersion } from "./update-check.js"; + +const INDEX_PATH = "connector-implementation-index.json"; +const commitId = /^[a-f0-9]{40}(?:[a-f0-9]{24})?$/; +const directoryKey = /^[a-z0-9]+(?:_[a-z0-9]+)*$/; +const version = /^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)$/; +const line = z + .string() + .min(1) + .max(300) + .regex(/^[^\p{Cc}\p{Zl}\p{Zp}]+$/u); + +const evidenceSchema = z.strictObject({ + kind: z.literal("synthetic"), + commit: z.string().regex(commitId), + checkedAt: z.iso.datetime(), + checks: z + .array( + z.strictObject({ + name: line, + result: z.enum(["passed", "failed", "not run"]), + }), + ) + .min(1) + .max(25), +}); + +const manifestSchema = z.looseObject({ + connector_key: z.string(), + version: z.string().regex(version), + streams: z.array(z.looseObject({ name: line })).min(1), +}); + +const indexSchema = z.strictObject({ + version: z.literal(1), + connectors: z.array( + z.looseObject({ + manifest: z.looseObject({ connector_key: z.string() }), + }), + ), +}); + +interface PrepOptions { + checkout: string; + base: string; + head: string; + summary: string; + evidence?: string; + confirm?: string; +} + +class PrepError extends Error { + constructor( + readonly code: string, + message: string, + ) { + super(message); + } +} + +function localGit(checkout: string) { + // A caller's Git environment must not redirect the explicit checkout. + const env = Object.fromEntries( + Object.entries(process.env).filter(([name]) => !name.startsWith("GIT_")), + ); + return (...args: string[]): string => { + const result = spawnSync( + "git", + [ + "--no-pager", + "--no-optional-locks", + "--no-replace-objects", + "-C", + checkout, + ...args, + ], + { + env: { ...env, GIT_NO_LAZY_FETCH: "1" }, + maxBuffer: 4 * 1024 * 1024, + timeout: 10_000, + }, + ); + if (result.status !== 0) { + throw new PrepError( + "local_git_failed", + "Git could not read the local checkout or requested revisions. Check the checkout, refs and diff size.", + ); + } + return new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode( + result.stdout, + ); + }; +} + +function prepare(key: string, options: PrepOptions) { + if (!directoryKey.test(key) || !line.safeParse(options.summary).success) { + throw new PrepError( + "invalid_input", + "Use the connector's snake_case directory name and a one-line change summary.", + ); + } + const checkout = realpathSync(options.checkout); + const git = localGit(checkout); + if ( + realpathSync(git("rev-parse", "--show-toplevel").trimEnd()) !== checkout + ) { + throw new PrepError( + "invalid_checkout", + "Pass the root of the PDP-Connect checkout.", + ); + } + const resolveCommit = (ref: string) => + git("rev-parse", "--verify", "--end-of-options", ref + "^{commit}").trim(); + const base = resolveCommit(options.base); + const head = resolveCommit(options.head); + if (git("merge-base", base, head).trim() !== base) { + throw new PrepError( + "base_not_ancestor", + "Rebase the selected commit onto the intended PDP-Connect base before preparing the diff.", + ); + } + + const prefix = "connectors/" + key + "/"; + const comparison = [ + "--no-ext-diff", + "--no-textconv", + "--no-renames", + "--no-relative", + base, + head, + "--", + ]; + const changed = git("diff", "--name-only", "-z", ...comparison) + .split("\0") + .filter(Boolean); + if ( + !changed.some((file) => file.startsWith(prefix)) || + changed.some( + (file) => + (file !== INDEX_PATH && !file.startsWith(prefix)) || + /\p{Cc}/u.test(file), + ) + ) { + throw new PrepError( + "connector_diff_required", + "Select a commit range changing only the chosen connector and its generated index entry.", + ); + } + // Raw modes expose symlinks and Git links, including deletions. Neither + // belongs in this review format. + const raw = git("diff", "--raw", "-z", ...comparison).split("\0"); + if ( + raw.some( + (entry, index) => + index % 2 === 0 && + entry && + !/^:(?:000000|100644|100755) (?:000000|100644|100755) /.test(entry), + ) + ) { + throw new PrepError( + "non_regular_file", + "The connector diff must contain only regular files.", + ); + } + if ( + git("diff", "--numstat", "-z", ...comparison) + .split("\0") + .some((entry) => entry.startsWith("-\t")) + ) { + throw new PrepError( + "binary_diff", + "The connector diff must contain only text files.", + ); + } + const manifestPath = prefix + "manifest.json"; + const current = manifestSchema.parse( + JSON.parse(git("show", head + ":" + manifestPath)), + ); + const previous = git("ls-tree", base, "--", manifestPath) + ? manifestSchema.parse(JSON.parse(git("show", base + ":" + manifestPath))) + : null; + if ( + current.connector_key !== key.replaceAll("_", "-") || + (previous && previous.connector_key !== current.connector_key) + ) { + throw new PrepError( + "connector_key_mismatch", + "Keep the manifest connector key consistent with the directory and the base manifest.", + ); + } + if (previous && !isNewerVersion(previous.version, current.version)) { + throw new PrepError( + "version_bump_required", + "Increase the connector manifest version before preparing this update.", + ); + } + + const beforeIndex = indexSchema.parse( + JSON.parse(git("show", base + ":" + INDEX_PATH)), + ); + const afterIndex = indexSchema.parse( + JSON.parse(git("show", head + ":" + INDEX_PATH)), + ); + const isSelected = ( + entry: z.infer["connectors"][number], + ) => entry.manifest.connector_key === current.connector_key; + const selected = afterIndex.connectors.filter(isSelected); + if ( + selected.length !== 1 || + selected[0].entry !== "./" + prefix + "index.ts" || + !isDeepStrictEqual(selected[0].manifest, current) + ) { + throw new PrepError( + "stale_connector_index", + "Regenerate the PDP-Connect implementation index so its entry matches the selected manifest.", + ); + } + if ( + !isDeepStrictEqual( + beforeIndex.connectors.filter((entry) => !isSelected(entry)), + afterIndex.connectors.filter((entry) => !isSelected(entry)), + ) + ) { + throw new PrepError( + "unrelated_index_change", + "Keep other connectors' implementation index entries unchanged.", + ); + } + + const evidence = options.evidence + ? evidenceSchema.safeParse( + JSON.parse(readFileSync(options.evidence, "utf8")), + ) + : undefined; + if (evidence && !evidence.success) { + throw new PrepError( + "invalid_evidence", + "Supply a reviewed synthetic summary with kind, commit, checkedAt and checks containing name and result.", + ); + } + const verification = evidence?.data ?? null; + if (verification && verification.commit !== head) { + throw new PrepError( + "stale_evidence", + "The evidence names a different commit. Check the selected revision and prepare a fresh summary.", + ); + } + const evidenceGaps = [ + ...(verification ? [] : ["No verification summary supplied."]), + ...(verification?.checks + .filter((check) => check.result !== "passed") + .map((check) => check.name + ": " + check.result) ?? []), + "Live provider behavior and Vana app availability remain unverified.", + ]; + const prBody = [ + "## What this does", + "", + options.summary, + "", + "Connector: " + + current.connector_key + + " " + + (previous ? previous.version + " → " : "") + + current.version, + "Streams: " + current.streams.map((stream) => stream.name).join(", "), + "", + "## How it was verified", + "", + ...(verification + ? [ + "Reported synthetic checks at " + + verification.checkedAt + + " on " + + head + + ".", + "", + ...verification.checks.map( + (check) => "- " + check.name + " — " + check.result, + ), + "", + ] + : []), + ...evidenceGaps.map((gap) => "- " + gap), + "", + "Assisted-by: AI", + "", + ].join("\n"); + + return { + target: connectorPrTarget, + baseBranch: "main", + base, + head, + connector: current.connector_key, + version: current.version, + previousVersion: previous?.version ?? null, + streams: current.streams.map((stream) => stream.name), + files: changed, + verification, + evidenceGaps, + prBody, + diff: git( + "diff", + "--no-color", + "--src-prefix=a/", + "--dst-prefix=b/", + "--unified=3", + ...comparison, + ), + }; +} + +function prepareSubmission( + options: PrepOptions, + preview: ReturnType, +) { + const git = localGit(options.checkout); + const commit = git("cat-file", "commit", preview.head); + const headers = commit.slice(0, commit.indexOf("\n\n")); + const parents = headers + .split("\n") + .filter((entry) => entry.startsWith("parent ")); + if ( + !preview.previousVersion || + preview.head.length !== 40 || + parents.length !== 1 || + parents[0] !== "parent " + preview.base + ) { + throw new PrepError( + "single_connector_update_required", + "Submit one commit updating an existing connector, directly on the reviewed PDP-Connect main revision.", + ); + } + if ( + !git( + "show", + "-s", + "--format=%(trailers:key=Signed-off-by,valueonly)", + preview.head, + ).trim() + ) { + throw new PrepError( + "signoff_required", + "PDP-Connect requires a Signed-off-by trailer. Sign off the commit and refresh its evidence.", + ); + } + if (!preview.verification) { + throw new PrepError( + "evidence_required", + "Supply a reviewed synthetic verification summary before submitting.", + ); + } + const submission = submitConnectorPr( + { + ...preview, + title: options.summary, + objects: git( + "rev-parse", + "--path-format=absolute", + "--git-path", + "objects", + ).trimEnd(), + }, + options.confirm, + ); + return submission.kind === "confirmation_required" + ? { ...submission, ...preview, title: options.summary, commit } + : submission; +} + +function terminalText(value: string): string { + return value.replace( + /[\u0000-\u0008\u000b-\u001f\u007f-\u009f]/g, + (char) => "\\u" + char.charCodeAt(0).toString(16).padStart(4, "0"), + ); +} + +function printPreview(preview: ReturnType) { + process.stdout.write( + terminalText( + "Target: " + + preview.target + + " (" + + preview.baseBranch + + ")\n" + + "Base: " + + preview.base + + "\n" + + "Head: " + + preview.head + + "\n" + + "Files:\n" + + preview.files.map((file) => " " + file).join("\n") + + "\n\nDraft PR body\n\n" + + preview.prBody + + "\nReview this diff and evidence before sharing.\n\n" + + preview.diff, + ), + ); +} + +function printSubmission(submission: ReturnType) { + if (submission.kind === "pull_request") { + const state = + submission.draft && submission.state === "open" + ? "draft" + : submission.state; + process.stdout.write( + (submission.reused ? "Existing GitHub PR (" : "GitHub PR (") + + state + + "): " + + submission.url + + "\n" + + (submission.draft && submission.state === "open" + ? "When ready, mark the draft ready for review on GitHub.\n" + : "Follow the review and respond to requests on GitHub.\n"), + ); + return; + } + process.stdout.write( + terminalText( + "Public GitHub submission preview\n" + + "GitHub account: @" + + submission.login + + "\n" + + "Fork: " + + submission.fork + + " (" + + submission.branch + + ")\n" + + "Draft PR title: " + + submission.title + + "\n\n" + + "Commit metadata and message\n\n" + + submission.commit + + "\n", + ), + ); + printPreview(submission); + process.stdout.write( + "\nTo publish this commit to your fork and open a draft PR, rerun with --confirm " + + submission.confirmation + + "\n", + ); +} + +function registerCommand( + parent: Command, + getOptions: () => { json?: boolean }, + submit: boolean, +) { + const command = parent + .command(submit ? "submit-pr " : "prepare-pr ") + .description( + submit + ? "Review and explicitly submit a committed connector update as a GitHub draft PR" + : "Preview a committed PDP-Connect connector diff and draft PR body locally", + ) + .requiredOption( + "--checkout ", + "Local data-connectors checkout root", + ) + .requiredOption( + "--base ", + "Local revision of the intended upstream main base", + ) + .option("--head ", "Connector revision to review", "HEAD") + .requiredOption("--summary ", "One-line description for the draft PR") + .option( + "--evidence ", + "Reviewed JSON with kind: synthetic, commit, checkedAt and checks [{name, result}]", + ) + .addHelpText( + "after", + submit + ? "\nUses GitHub CLI sign-in and an existing public fork. Run without --confirm to review.\n" + : "\nUses committed files only. Review the diff and evidence before sharing them.\n", + ); + if (submit) + command.option( + "--confirm ", + "Confirm the exact GitHub submission preview", + ); + command.action((key: string, options: PrepOptions) => { + const json = getOptions().json; + try { + const preview = prepare(key, options); + const submission = submit + ? prepareSubmission(options, preview) + : undefined; + if (json) { + process.stdout.write(JSON.stringify(submission ?? preview) + "\n"); + } else if (submission) { + printSubmission(submission); + } else { + process.stdout.write("Local connector PR preview\n"); + printPreview(preview); + } + process.exitCode = + submission?.kind === "confirmation_required" + ? CliExitCode.CONFIRMATION_REQUIRED + : CliExitCode.OK; + } catch (error) { + const failure = + error instanceof PrepError || error instanceof SubmitError + ? error + : new PrepError( + submit ? "invalid_submission" : "invalid_local_input", + submit + ? "Could not read the local submission or GitHub response. Check the checkout and retry the same submission." + : "Check the checkout path, committed manifest, and reviewed evidence JSON.", + ); + if (json) { + process.stdout.write( + JSON.stringify({ + error: { code: failure.code, message: failure.message }, + }) + "\n", + ); + } else { + process.stderr.write(failure.message + "\n"); + } + process.exitCode = + failure instanceof SubmitError ? failure.exitCode : CliExitCode.FAILURE; + } + }); +} + +/** Register the local preview and public submission commands. */ +export function registerConnectorPrCommands( + parent: Command, + getOptions: () => { json?: boolean }, +): void { + registerCommand(parent, getOptions, false); + registerCommand(parent, getOptions, true); +} diff --git a/src/cli/connector-pr-submit.ts b/src/cli/connector-pr-submit.ts new file mode 100644 index 0000000..5a62af6 --- /dev/null +++ b/src/cli/connector-pr-submit.ts @@ -0,0 +1,375 @@ +import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { z } from "zod"; + +import { CliExitCode } from "../core/exit-codes.js"; + +const repository = "PDP-Connect/data-connectors"; +/** The public repository that owns connector contributions. */ +export const connectorPrTarget = "https://github.com/" + repository; +const loginSchema = z.string().regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/i); + +interface Submission { + base: string; + baseBranch: string; + head: string; + title: string; + prBody: string; + diff: string; + objects: string; +} + +/** An actionable error from the GitHub submission flow. */ +export class SubmitError extends Error { + constructor( + readonly code: string, + message: string, + readonly exitCode: CliExitCode = CliExitCode.FAILURE, + ) { + super(message); + } +} + +function githubEnvironment() { + const env = { ...process.env }; + // Collection and CI tokens must not select the account publishing this PR. + delete env.GITHUB_TOKEN; + delete env.GITHUB_PERSONAL_ACCESS_TOKEN; + delete env.GH_TOKEN; + delete env.GITHUB_ENTERPRISE_TOKEN; + delete env.GH_ENTERPRISE_TOKEN; + delete env.GH_DEBUG; + delete env.DEBUG; + return { ...env, GH_PROMPT_DISABLED: "1" }; +} + +function gh(env: NodeJS.ProcessEnv, args: string[], input?: string) { + return spawnSync("gh", args, { + env, + input, + encoding: "utf8", + timeout: 30_000, + maxBuffer: 4 * 1024 * 1024, + stdio: [input === undefined ? "ignore" : "pipe", "pipe", "pipe"], + }); +} + +function api( + env: NodeJS.ProcessEnv, + endpoint: string, + body?: Record, +): unknown { + const result = gh( + env, + [ + "api", + "--hostname", + "github.com", + "--method", + body === undefined ? "GET" : "POST", + endpoint, + ...(body === undefined ? [] : ["--input", "-"]), + ], + body === undefined ? undefined : JSON.stringify(body), + ); + if (result.status !== 0) { + throw new SubmitError( + "github_request_failed", + "GitHub did not confirm the request. Check GitHub access and retry the same submission.", + ); + } + return JSON.parse(result.stdout); +} + +function githubLogin(env: NodeJS.ProcessEnv) { + const result = gh(env, [ + "api", + "--hostname", + "github.com", + "user", + "--jq", + ".login", + ]); + if ( + result.error && + "code" in result.error && + result.error.code === "ENOENT" + ) { + throw new SubmitError( + "github_cli_missing", + "Install GitHub CLI (gh), then run gh auth login --hostname github.com.", + CliExitCode.CONFIRMATION_REQUIRED, + ); + } + const login = loginSchema.safeParse(result.stdout?.trim()); + if (result.status !== 0 || !login.success) { + throw new SubmitError( + "github_sign_in_required", + "Could not verify a signed-in GitHub account. Check connectivity and run gh auth login --hostname github.com if signed out.", + CliExitCode.CONFIRMATION_REQUIRED, + ); + } + return login.data; +} + +function pushCommit( + env: NodeJS.ProcessEnv, + submission: Submission, + fork: string, + branch: string, +) { + // Use only the reviewed commit's object store. Checkout and user Git config + // can otherwise supply push hooks, URL rewrites, or other credentials. + const directory = mkdtempSync(join(tmpdir(), "vana-connector-push-")); + const gitEnv = Object.fromEntries( + Object.entries(env).filter(([name]) => !name.startsWith("GIT_")), + ); + Object.assign(gitEnv, { + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: process.platform === "win32" ? "NUL" : "/dev/null", + GIT_TERMINAL_PROMPT: "0", + GIT_NO_LAZY_FETCH: "1", + }); + const git = (args: string[], objects?: string) => + spawnSync("git", ["--no-replace-objects", "-C", directory, ...args], { + env: objects ? { ...gitEnv, GIT_OBJECT_DIRECTORY: objects } : gitEnv, + stdio: "ignore", + timeout: 120_000, + }); + try { + if ( + git(["init", "--quiet", "--bare", "--template=", "--object-format=sha1"]) + .status !== 0 + ) { + throw new SubmitError( + "git_init_failed", + "Could not prepare the temporary Git push.", + ); + } + const ref = "refs/heads/" + branch; + if ( + git( + [ + "-c", + "credential.helper=!gh auth git-credential", + "push", + "--no-follow-tags", + "--force-with-lease=" + ref + ":", + fork + ".git", + submission.head + ":" + ref, + ], + submission.objects, + ).status !== 0 + ) { + throw new SubmitError( + "connector_push_failed", + "Git did not confirm the push to your fork. Check write access and retry the same submission.", + ); + } + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +const pullNumber = z.number().int().positive(); +const pullSchema = z.object({ + number: pullNumber, + html_url: z.string(), + state: z.enum(["open", "closed"]), + draft: z.boolean(), + merged_at: z.string().nullable(), + user: z.object({ login: loginSchema }), + head: z.object({ sha: z.string(), label: z.string() }), + base: z.object({ ref: z.string() }), +}); + +/** Preview or submit a connector update under the builder's GitHub account. */ +export function submitConnectorPr( + submission: Submission, + confirmation?: string, +) { + const env = githubEnvironment(); + const login = githubLogin(env); + const fork = "https://github.com/" + login + "/data-connectors"; + const branch = "vana/connector-" + submission.head; + const headRef = login + ":" + branch; + const digest = createHash("sha256") + .update( + JSON.stringify({ + login, + fork, + branch, + target: connectorPrTarget, + base: submission.base, + head: submission.head, + baseBranch: submission.baseBranch, + title: submission.title, + body: submission.prBody, + diff: submission.diff, + draft: true, + }), + ) + .digest("hex"); + + if (confirmation === undefined) { + return { + kind: "confirmation_required" as const, + login, + fork, + branch, + confirmation: digest, + }; + } + if (confirmation !== digest) { + throw new SubmitError( + "confirmation_mismatch", + "The GitHub account or submission changed. Review the preview again before confirming.", + CliExitCode.CONFIRMATION_REQUIRED, + ); + } + + // Pin this operation to the reviewed login even if gh's active user + // changes during the push. The token stays in subprocess pipes and env. + const credential = gh(env, [ + "auth", + "token", + "--hostname", + "github.com", + "--user", + login, + ]); + const token = credential.stdout?.trim(); + if (credential.status !== 0 || !token) { + throw new SubmitError( + "github_sign_in_required", + "Could not use the confirmed GitHub account. Sign in again with gh auth login --hostname github.com.", + CliExitCode.CONFIRMATION_REQUIRED, + ); + } + const authenticated = { ...env, GH_TOKEN: token }; + if (githubLogin(authenticated) !== login) { + throw new SubmitError( + "github_account_changed", + "The GitHub account changed. Review the preview again.", + CliExitCode.CONFIRMATION_REQUIRED, + ); + } + + const pulls = "repos/" + repository + "/pulls"; + const lookup = + pulls + + "?" + + new URLSearchParams({ + state: "all", + head: headRef, + base: submission.baseBranch, + }); + const receipt = (pr: z.infer, reused: boolean) => { + if ( + pr.user.login !== login || + pr.head.label !== headRef || + pr.head.sha !== submission.head || + pr.base.ref !== submission.baseBranch || + pr.html_url !== connectorPrTarget + "/pull/" + pr.number + ) { + throw new SubmitError( + "existing_pull_request_changed", + "The GitHub PR for this branch differs from the reviewed submission. Check it on GitHub before trying again.", + ); + } + return { + kind: "pull_request" as const, + repository, + login, + number: pr.number, + url: pr.html_url, + state: pr.merged_at ? "merged" : pr.state, + draft: pr.draft, + reused, + }; + }; + const findExisting = () => { + const [pr] = z + .array(z.object({ number: pullNumber })) + .parse(api(authenticated, lookup)); + if (!pr) return undefined; + return receipt( + pullSchema.parse(api(authenticated, pulls + "/" + pr.number)), + true, + ); + }; + + // A prior attempt may have created the PR and lost its response. Resolve + // the stable branch before any push or create so retry can stay read-only. + const existing = findExisting(); + if (existing) return existing; + + const forkResponse = gh(authenticated, [ + "api", + "--hostname", + "github.com", + "repos/" + login + "/data-connectors", + ]); + const forkDetails = z + .object({ + full_name: z.string(), + private: z.literal(false), + parent: z.object({ full_name: z.string() }), + permissions: z.object({ push: z.literal(true) }), + }) + .safeParse( + forkResponse.status === 0 ? JSON.parse(forkResponse.stdout) : null, + ); + if ( + !forkDetails.success || + forkDetails.data.full_name.toLowerCase() !== + (login + "/data-connectors").toLowerCase() || + forkDetails.data.parent.full_name.toLowerCase() !== repository.toLowerCase() + ) { + throw new SubmitError( + "github_fork_required", + "A writable public fork under your GitHub account is required. Fork PDP-Connect/data-connectors on GitHub, then retry.", + CliExitCode.CONFIRMATION_REQUIRED, + ); + } + const base = z + .object({ object: z.object({ sha: z.string() }) }) + .parse( + api( + authenticated, + "repos/" + repository + "/git/ref/heads/" + submission.baseBranch, + ), + ); + if (base.object.sha !== submission.base) { + throw new SubmitError( + "upstream_base_changed", + "PDP-Connect main changed. Rebase the connector commit, refresh its evidence, and review the new submission.", + ); + } + + pushCommit(authenticated, submission, fork, branch); + try { + const pr = pullSchema.parse( + api(authenticated, pulls, { + title: submission.title, + body: submission.prBody, + base: submission.baseBranch, + head: headRef, + draft: true, + }), + ); + return receipt(pr, false); + } catch { + // GitHub may have created the PR before the response was lost. The same + // branch lets a fresh invocation recover too, even if this lookup fails. + const recovered = findExisting(); + if (recovered) return recovered; + throw new SubmitError( + "pull_request_unconfirmed", + "The branch was pushed. GitHub did not confirm a PR. Retry the same submission to recover it.", + ); + } +} diff --git a/src/cli/index.ts b/src/cli/index.ts index 817051b..c1a2097 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -110,6 +110,7 @@ import { import { CliExitCode } from "../core/exit-codes.js"; import { registerAppCommands } from "./app/index.js"; import { registerConnectorBuildCommands } from "./connector-build.js"; +import { registerConnectorPrCommands } from "./connector-pr-prep.js"; import { checkRegisteredServers, lookupRegisteredServers, @@ -616,7 +617,7 @@ Examples: const connectors = program .command("connectors") - .description("Build and register local Collection Profile connectors"); + .description("Build, register, and submit Collection Profile connectors"); connectors.addHelpText( "after", ` @@ -635,6 +636,7 @@ Examples: }); registerConnectorBuildCommands(connectors, () => parsedOptions); + registerConnectorPrCommands(connectors, () => parsedOptions); connectors .command("add ") diff --git a/test/cli/connector-pr-prep.test.ts b/test/cli/connector-pr-prep.test.ts new file mode 100644 index 0000000..8218233 --- /dev/null +++ b/test/cli/connector-pr-prep.test.ts @@ -0,0 +1,413 @@ +import { execFileSync } from "node:child_process"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; + +import { runCli } from "../../src/cli/index.js"; + +let root: string; +let checkout: string; +let base: string; +let head: string; +let previousExitCode: typeof process.exitCode; + +function git(...args: string[]) { + return execFileSync("git", ["-C", checkout, ...args], { encoding: "utf8" }); +} + +function commit() { + git("add", "."); + git( + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + "commit", + "--quiet", + "--signoff", + "-m", + "Fixture change", + ); + return git("rev-parse", "HEAD").trim(); +} + +function writeConnector(version: string, otherVersion = "1.0.0") { + const manifest = { + connector_key: "example", + version, + streams: [{ name: "items" }], + }; + writeFileSync( + path.join(checkout, "connectors/example/manifest.json"), + JSON.stringify(manifest), + ); + writeFileSync( + path.join(checkout, "connector-implementation-index.json"), + JSON.stringify({ + version: 1, + connectors: [ + { entry: "./connectors/example/index.ts", manifest }, + { + entry: "./connectors/other/index.ts", + manifest: { connector_key: "other", version: otherVersion }, + }, + ], + }), + ); +} + +beforeEach(() => { + previousExitCode = process.exitCode; + root = mkdtempSync(path.join(os.tmpdir(), "vana-prepare-pr-")); + checkout = path.join(root, "data-connectors"); + mkdirSync(path.join(checkout, "connectors/example"), { recursive: true }); + vi.stubEnv("VANA_HOME", path.join(root, "vana")); + git("init", "--quiet"); + writeConnector("1.0.0"); + writeFileSync( + path.join(checkout, "connectors/example/index.ts"), + "export const test = 'before';\n", + ); + base = commit(); + writeConnector("1.0.1"); + writeFileSync( + path.join(checkout, "connectors/example/index.ts"), + "export const test = 'after';\n", + ); + head = commit(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + process.exitCode = previousExitCode; + rmSync(root, { recursive: true, force: true }); +}); + +async function preview( + evidenceCommit: string, + command = "prepare-pr", + ...args: string[] +) { + const evidence = path.join(root, "synthetic.json"); + writeFileSync( + evidence, + JSON.stringify({ + kind: "synthetic", + commit: evidenceCommit, + checkedAt: "2026-11-10T00:00:00Z", + checks: [{ name: "Fixture test", result: "passed" }], + }), + ); + const stdout = vi.spyOn(process.stdout, "write").mockReturnValue(true); + const code = await runCli([ + "node", + "vana", + "connectors", + command, + "example", + "--checkout", + checkout, + "--base", + base, + "--summary", + "Fix example.", + "--evidence", + evidence, + "--json", + ...args, + ]); + const result = JSON.parse( + stdout.mock.calls.map(([chunk]) => String(chunk)).join(""), + ); + stdout.mockRestore(); + return { code, result }; +} + +it("previews the committed change and evidence without including or modifying dirty files", async () => { + const entry = path.join(checkout, "connectors/example/index.ts"); + writeFileSync(entry, "export const test = 'uncommitted';\n"); + git("add", "connectors/example/index.ts"); + writeFileSync(path.join(checkout, "private.txt"), "private example data"); + const statusBefore = git("status", "--porcelain"); + const { code, result } = await preview(head); + + expect(code).toBe(0); + expect(result).toMatchObject({ + target: "https://github.com/PDP-Connect/data-connectors", + base, + head, + connector: "example", + version: "1.0.1", + files: [ + "connector-implementation-index.json", + "connectors/example/index.ts", + "connectors/example/manifest.json", + ], + }); + expect(result.diff).toContain("+export const test = 'after';"); + expect(result.diff).not.toContain("uncommitted"); + expect(result.diff).not.toContain("private.txt"); + expect(result.prBody).toContain("Reported synthetic checks"); + expect(result.prBody).toContain("Fixture test — passed"); + expect(git("status", "--porcelain")).toBe(statusBefore); + expect(git("rev-parse", "HEAD").trim()).toBe(head); +}); + +it("rejects evidence for another commit without emitting a patch", async () => { + const { code, result } = await preview(base); + + expect(code).toBe(1); + expect(result.error.code).toBe("stale_evidence"); + expect(result).not.toHaveProperty("diff"); +}); + +it("rejects another connector's change in the shared implementation index", async () => { + writeConnector("1.0.1", "1.0.1"); + head = commit(); + const { code, result } = await preview(head); + + expect(code).toBe(1); + expect(result.error.code).toBe("unrelated_index_change"); + expect(result).not.toHaveProperty("diff"); +}); + +it("rejects unrelated committed files rather than hiding them from the preview", async () => { + writeFileSync(path.join(checkout, "unrelated.txt"), "out of scope"); + head = commit(); + const { code, result } = await preview(head); + + expect(code).toBe(1); + expect(result.error.code).toBe("connector_diff_required"); + expect(result).not.toHaveProperty("diff"); +}); + +function githubFixture() { + const bin = path.join(root, "bin"); + const fork = path.join(root, "fork.git"); + const receipt = path.join(root, "pull.json"); + const realGit = execFileSync("which", ["git"], { encoding: "utf8" }).trim(); + execFileSync(realGit, ["init", "--quiet", "--bare", fork]); + mkdirSync(bin); + vi.stubEnv("PATH", bin + path.delimiter + process.env.PATH); + vi.stubEnv("VANA_TEST_BASE", base); + vi.stubEnv("VANA_TEST_REAL_GIT", realGit); + vi.stubEnv("VANA_TEST_FORK", fork); + vi.stubEnv("VANA_TEST_RECEIPT", receipt); + vi.stubEnv("VANA_TEST_LOGIN", "fixture-builder"); + vi.stubEnv("GH_TOKEN", "ambient-ci-token"); + vi.stubEnv("GITHUB_TOKEN", "ambient-collection-token"); + writeFileSync( + path.join(bin, "git"), + String.raw`#!/usr/bin/env node +const { spawnSync } = require("node:child_process"); +const args = process.argv.slice(2); +const remote = args.indexOf("https://github.com/fixture-builder/data-connectors.git"); +if (args.includes("push")) { + if (process.env.VANA_TEST_FORK_READ_ONLY) process.exit(1); + if (remote === -1 || process.env.GH_TOKEN !== "fixture-only-token") process.exit(1); + args[remote] = process.env.VANA_TEST_FORK; +} +const result = spawnSync(process.env.VANA_TEST_REAL_GIT, args, { + env: { ...process.env, GIT_ALLOW_PROTOCOL: "file" }, + stdio: "inherit", +}); +process.exit(result.status ?? 1); +`, + { mode: 0o755 }, + ); + writeFileSync( + path.join(bin, "gh"), + String.raw`#!/usr/bin/env node +const fs = require("node:fs"); +const { execFileSync } = require("node:child_process"); +const env = process.env; +const args = process.argv.slice(2); +const root = "repos/PDP-Connect/data-connectors"; +const login = env.VANA_TEST_LOGIN; +const receipt = env.VANA_TEST_RECEIPT; +const send = (data) => process.stdout.write(JSON.stringify(data)); +if (env.GITHUB_TOKEN || (env.GH_TOKEN && env.GH_TOKEN !== "fixture-only-token")) { + process.exit(1); +} +if (args[0] === "auth" && args[1] === "token" && args.includes(login)) { + process.stdout.write("fixture-only-token"); +} else if (args.includes("user") && login) { + process.stdout.write(login); +} else if (args.includes("repos/" + login + "/data-connectors")) { + send({ + full_name: login + "/data-connectors", + private: false, + parent: { full_name: "PDP-Connect/data-connectors" }, + permissions: { push: true }, + }); +} else if (args.includes(root + "/git/ref/heads/main")) { + send({ object: { sha: env.VANA_TEST_BASE } }); +} else if (args.includes(root + "/pulls") && args.includes("POST")) { + if (fs.existsSync(receipt)) throw new Error("Duplicate create"); + const submitted = JSON.parse(fs.readFileSync(0, "utf8")); + const branch = submitted.head.slice(login.length + 1); + const pushed = execFileSync(env.VANA_TEST_REAL_GIT, [ + "--git-dir", env.VANA_TEST_FORK, "rev-parse", "refs/heads/" + branch, + ], { encoding: "utf8" }).trim(); + const pr = { + number: 42, + html_url: "https://github.com/PDP-Connect/data-connectors/pull/42", + state: "open", + draft: submitted.draft, + merged_at: null, + user: { login }, + head: { sha: pushed, label: submitted.head }, + base: { ref: submitted.base }, + }; + fs.writeFileSync(receipt, JSON.stringify({ submitted, pr })); + if (env.VANA_TEST_LOSE_RESPONSE) process.exit(1); + send(pr); +} else if (args.includes(root + "/pulls/42")) { + send(JSON.parse(fs.readFileSync(receipt, "utf8")).pr); +} else if (args.some((arg) => arg.startsWith(root + "/pulls?"))) { + const query = new URLSearchParams(args.find((arg) => arg.includes("/pulls?")).split("?")[1]); + const pr = fs.existsSync(receipt) ? JSON.parse(fs.readFileSync(receipt, "utf8")).pr : null; + if (pr && env.VANA_TEST_LOSE_RESPONSE === "and-lookup") process.exit(1); + send(pr && query.get("head") === pr.head.label && query.get("base") === pr.base.ref && + query.get("state") === "all" ? [{ number: pr.number }] : []); +} else { + process.exit(1); +} +`, + { mode: 0o755 }, + ); + return { + receipt, + refs: () => + execFileSync( + realGit, + ["--git-dir", fork, "for-each-ref", "--format=%(objectname)"], + { + encoding: "utf8", + }, + ).trim(), + }; +} + +const withShellFixtures = it.skipIf(process.platform === "win32"); + +withShellFixtures( + "requires a GitHub login and explicit confirmation bound to the public preview", + async () => { + const fixture = githubFixture(); + vi.stubEnv("VANA_TEST_LOGIN", ""); + const signedOut = await preview(head, "submit-pr"); + expect(signedOut).toMatchObject({ + code: 7, + result: { error: { code: "github_sign_in_required" } }, + }); + + vi.stubEnv("VANA_TEST_LOGIN", "fixture-builder"); + const reviewed = await preview(head, "submit-pr", "--yes"); + expect(reviewed).toMatchObject({ + code: 7, + result: { + kind: "confirmation_required", + login: "fixture-builder", + fork: "https://github.com/fixture-builder/data-connectors", + target: "https://github.com/PDP-Connect/data-connectors", + title: "Fix example.", + }, + }); + expect(reviewed.result.diff).toContain("+export const test = 'after';"); + expect(reviewed.result.prBody).toContain("Fixture test — passed"); + + vi.stubEnv("VANA_TEST_LOGIN", "another-account"); + const changed = await preview( + head, + "submit-pr", + "--confirm", + reviewed.result.confirmation, + ); + expect(changed).toMatchObject({ + code: 7, + result: { error: { code: "confirmation_mismatch" } }, + }); + expect(fixture.refs()).toBe(""); + expect(existsSync(fixture.receipt)).toBe(false); + }, +); + +withShellFixtures.each(["no loss", "response", "and-lookup"])( + "returns one draft PR when creating or recovering a submission (%s)", + async (loss) => { + const fixture = githubFixture(); + const reviewed = await preview(head, "submit-pr"); + vi.stubEnv("VANA_TEST_LOSE_RESPONSE", loss === "no loss" ? "" : loss); + writeFileSync( + path.join(checkout, "private.txt"), + "uncommitted private data", + ); + + const confirmed = await preview( + head, + "submit-pr", + "--confirm", + reviewed.result.confirmation, + ); + if (loss === "and-lookup") { + expect(confirmed.code).toBe(1); + } else { + expect(confirmed).toMatchObject({ + code: 0, + result: { number: 42, draft: true, reused: loss !== "no loss" }, + }); + } + expect(fixture.refs()).toBe(head); + + const saved = readFileSync(fixture.receipt, "utf8"); + expect(JSON.parse(saved).submitted).toMatchObject({ + title: "Fix example.", + body: expect.stringContaining("Fixture test — passed"), + base: "main", + draft: true, + }); + vi.stubEnv("VANA_TEST_LOSE_RESPONSE", ""); + vi.stubEnv("VANA_TEST_FORK_READ_ONLY", "1"); + const recovered = await preview( + head, + "submit-pr", + "--confirm", + reviewed.result.confirmation, + ); + expect(recovered).toMatchObject({ + code: 0, + result: { + number: 42, + url: "https://github.com/PDP-Connect/data-connectors/pull/42", + reused: true, + }, + }); + expect(readFileSync(fixture.receipt, "utf8")).toBe(saved); + expect(fixture.refs()).toBe(head); + }, +); + +it("refuses to publish a commit range whose intermediate contents were absent from the preview", async () => { + const privateFile = path.join(checkout, "private.txt"); + writeFileSync(privateFile, "private example data"); + commit(); + rmSync(privateFile); + head = commit(); + const reviewed = await preview(head, "submit-pr"); + expect(reviewed).toMatchObject({ + code: 1, + result: { error: { code: "single_connector_update_required" } }, + }); +}); From b176030492fcb00bd54ab92878057c0ae7db5687 Mon Sep 17 00:00:00 2001 From: Callum Flack <749885+callumflack@users.noreply.github.com> Date: Tue, 10 Nov 2026 17:05:20 +1000 Subject: [PATCH 2/5] test(cli): reject write attempts during PR recovery BUI-942 Assisted-by: AI Signed-off-by: Callum Flack <749885+callumflack@users.noreply.github.com> --- test/cli/connector-pr-prep.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/cli/connector-pr-prep.test.ts b/test/cli/connector-pr-prep.test.ts index 8218233..1ae751d 100644 --- a/test/cli/connector-pr-prep.test.ts +++ b/test/cli/connector-pr-prep.test.ts @@ -196,6 +196,7 @@ function githubFixture() { const bin = path.join(root, "bin"); const fork = path.join(root, "fork.git"); const receipt = path.join(root, "pull.json"); + const writes = path.join(root, "write-attempts.log"); const realGit = execFileSync("which", ["git"], { encoding: "utf8" }).trim(); execFileSync(realGit, ["init", "--quiet", "--bare", fork]); mkdirSync(bin); @@ -204,6 +205,7 @@ function githubFixture() { vi.stubEnv("VANA_TEST_REAL_GIT", realGit); vi.stubEnv("VANA_TEST_FORK", fork); vi.stubEnv("VANA_TEST_RECEIPT", receipt); + vi.stubEnv("VANA_TEST_WRITES", writes); vi.stubEnv("VANA_TEST_LOGIN", "fixture-builder"); vi.stubEnv("GH_TOKEN", "ambient-ci-token"); vi.stubEnv("GITHUB_TOKEN", "ambient-collection-token"); @@ -214,6 +216,7 @@ const { spawnSync } = require("node:child_process"); const args = process.argv.slice(2); const remote = args.indexOf("https://github.com/fixture-builder/data-connectors.git"); if (args.includes("push")) { + require("node:fs").appendFileSync(process.env.VANA_TEST_WRITES, "push\n"); if (process.env.VANA_TEST_FORK_READ_ONLY) process.exit(1); if (remote === -1 || process.env.GH_TOKEN !== "fixture-only-token") process.exit(1); args[remote] = process.env.VANA_TEST_FORK; @@ -236,6 +239,10 @@ const args = process.argv.slice(2); const root = "repos/PDP-Connect/data-connectors"; const login = env.VANA_TEST_LOGIN; const receipt = env.VANA_TEST_RECEIPT; +const createRequest = args.includes(root + "/pulls") && args.includes("POST"); +if (createRequest || (args[0] === "pr" && args[1] === "create")) { + fs.appendFileSync(env.VANA_TEST_WRITES, "create PR\n"); +} const send = (data) => process.stdout.write(JSON.stringify(data)); if (env.GITHUB_TOKEN || (env.GH_TOKEN && env.GH_TOKEN !== "fixture-only-token")) { process.exit(1); @@ -253,7 +260,7 @@ if (args[0] === "auth" && args[1] === "token" && args.includes(login)) { }); } else if (args.includes(root + "/git/ref/heads/main")) { send({ object: { sha: env.VANA_TEST_BASE } }); -} else if (args.includes(root + "/pulls") && args.includes("POST")) { +} else if (createRequest) { if (fs.existsSync(receipt)) throw new Error("Duplicate create"); const submitted = JSON.parse(fs.readFileSync(0, "utf8")); const branch = submitted.head.slice(login.length + 1); @@ -289,6 +296,7 @@ if (args[0] === "auth" && args[1] === "token" && args.includes(login)) { ); return { receipt, + writeAttempts: () => readFileSync(writes, "utf8"), refs: () => execFileSync( realGit, @@ -380,6 +388,7 @@ withShellFixtures.each(["no loss", "response", "and-lookup"])( }); vi.stubEnv("VANA_TEST_LOSE_RESPONSE", ""); vi.stubEnv("VANA_TEST_FORK_READ_ONLY", "1"); + expect(fixture.writeAttempts()).toBe("push\ncreate PR\n"); const recovered = await preview( head, "submit-pr", @@ -394,6 +403,7 @@ withShellFixtures.each(["no loss", "response", "and-lookup"])( reused: true, }, }); + expect(fixture.writeAttempts()).toBe("push\ncreate PR\n"); expect(readFileSync(fixture.receipt, "utf8")).toBe(saved); expect(fixture.refs()).toBe(head); }, From ecdd089583c1d238c440385bf42c78353d17b3e1 Mon Sep 17 00:00:00 2001 From: Callum Flack <749885+callumflack@users.noreply.github.com> Date: Wed, 11 Nov 2026 13:28:18 +1000 Subject: [PATCH 3/5] feat(cli): submit new connectors with review evidence BUI-942 BUI-952 Assisted-by: AI Signed-off-by: Callum Flack <749885+callumflack@users.noreply.github.com> --- skills/create-connector/SKILL.md | 72 ++++++ skills/registry.json | 2 +- src/cli/connector-pr-prep.ts | 166 +++++++++++-- test/cli/connector-pr-prep.test.ts | 366 ++++++++++++++++++++++++++++- 4 files changed, 577 insertions(+), 29 deletions(-) diff --git a/skills/create-connector/SKILL.md b/skills/create-connector/SKILL.md index 675b786..eaf8987 100644 --- a/skills/create-connector/SKILL.md +++ b/skills/create-connector/SKILL.md @@ -119,3 +119,75 @@ MCP; one-off acceptance needs only `--from`. Remove an unwanted override with `vana connectors remove `. App consent, reading and delivery belong to the Builder workflow. Native availability, signed release and publication are separate work, not a side effect of a local test. + +## Submit a connector for review + +When the builder explicitly asks to submit a new connector or an improvement, +run the upstream checks, then prepare its public handoff. These commands read +reported evidence. They do not execute the test commands in the evidence file. + +1. Keep one signed-off commit directly on the reviewed upstream main revision. + Change only the selected connector and its generated implementation-index + entry. New connectors need a unique key, manifest and `index.ts`. Updates + need a higher manifest version. +2. Save reviewed synthetic evidence outside the connector commit using the + example below. Replace the commit, date, commands, results, source and + record with observed values. This content becomes public with the PR. +3. Run `vana connectors prepare-pr --checkout /path/to/data-connectors --base --head --summary "" --evidence /path/to/reviewed-evidence.json --json`. + Review the full committed diff and PR description. Older evidence summaries + remain previewable, with missing review evidence listed. +4. Sign in with `gh auth login --hostname github.com` if needed. The builder + needs a writable public fork of `PDP-Connect/data-connectors`. A GitHub data + connection does not select the account used to submit code. +5. Replace `prepare-pr` with `submit-pr` using the same arguments. It shows the + GitHub account, public destination, exact commit, diff and evidence, then + exits with code 7 and a confirmation digest. Show the preview to the builder. + Global `--yes` does not approve publication. +6. After explicit approval, rerun with `--confirm ` to push the reviewed + commit and open a draft PR. Changed evidence or account needs a new review. + If interrupted, retry the same confirmed submission. It finds the existing + PR before any write and returns its URL. +7. The builder marks the draft ready and responds to reviewers on GitHub. + Report submission, approval, merge, signing, publication and Vana app + availability only when each is actually verified. + +An example evidence file for a new connector follows. + +```json +{ + "kind": "synthetic", + "commit": "", + "checkedAt": "2026-11-11T00:00:00Z", + "environment": "Node 24.22.1, Linux, synthetic provider fixture", + "checks": [ + { + "name": "Synthetic collection and schema test", + "result": "passed", + "command": "node --import tsx --test connectors/example/index.test.ts", + "exitCode": 0, + "output": "tests 1\npass 1\nfail 0" + } + ], + "sample": { + "source": { "id": "item-1", "title": "Example item" }, + "after": { "id": "item-1", "title": "Example item" } + }, + "storedData": "Initial upstream connector. Local test records are unchanged. IDs use the source item ID; the first collection starts without a cursor." +} +``` + +For an update, add `sample.before` with the old output for the same input, or +`null` if no record was emitted. Explain existing-record effects in `storedData`, +including IDs, types, cursors and any required reimport. Keep secrets, personal +records and raw private captures out of every field. + +Each check reports `passed`, `failed` or `not run`, its exact command, exit +status and output. A passed check needs status 0. A failed check needs a nonzero +status. A check not run uses `exitCode: null` and explains why in `output`. +For upstream failures, include the base SHA, environment, command and result in +the reviewed output. Keep failed and skipped checks visible. Synthetic evidence +does not establish live provider behavior or app availability. + +Commands and the environment support 300 characters each. Outputs and the +stored-data explanation support 12,000 characters each. The full description +must fit within 60,000 UTF-8 bytes. Review output before sharing it. diff --git a/skills/registry.json b/skills/registry.json index fb67f7a..1541c18 100644 --- a/skills/registry.json +++ b/skills/registry.json @@ -32,7 +32,7 @@ "id": "create-connector", "name": "Create Connector", "description": "Build or repair a Collection Profile connector with the current agent and shared Factory checks", - "version": "1.4.0", + "version": "1.5.0", "files": { "skill": "create-connector/SKILL.md" } diff --git a/src/cli/connector-pr-prep.ts b/src/cli/connector-pr-prep.ts index dfec1f8..d3451ad 100644 --- a/src/cli/connector-pr-prep.ts +++ b/src/cli/connector-pr-prep.ts @@ -22,19 +22,48 @@ const line = z .max(300) .regex(/^[^\p{Cc}\p{Zl}\p{Zp}]+$/u); +const evidenceText = z + .string() + .min(1) + .max(12_000) + .regex(/^[^\u0000-\u0008\u000b-\u001f\u007f-\u009f]*$/); +const recordSchema = z.record(z.string(), z.json()); +const checkFields = { + name: line, + command: line.optional(), + output: evidenceText.optional(), +}; +const checkSchema = z.discriminatedUnion("result", [ + z.strictObject({ + ...checkFields, + result: z.literal("passed"), + exitCode: z.literal(0).optional(), + }), + z.strictObject({ + ...checkFields, + result: z.literal("failed"), + exitCode: z.number().int().min(1).max(255).optional(), + }), + z.strictObject({ + ...checkFields, + result: z.literal("not run"), + exitCode: z.null().optional(), + }), +]); const evidenceSchema = z.strictObject({ kind: z.literal("synthetic"), commit: z.string().regex(commitId), checkedAt: z.iso.datetime(), - checks: z - .array( - z.strictObject({ - name: line, - result: z.enum(["passed", "failed", "not run"]), - }), - ) - .min(1) - .max(25), + environment: line.optional(), + checks: z.array(checkSchema).min(1).max(25), + sample: z + .strictObject({ + source: z.json(), + before: recordSchema.nullable().optional(), + after: recordSchema, + }) + .optional(), + storedData: evidenceText.optional(), }); const manifestSchema = z.looseObject({ @@ -104,6 +133,13 @@ function localGit(checkout: string) { }; } +function codeBlock(value: string, language: string) { + const fence = "`".repeat( + Math.max(3, ...(value.match(/`+/g) ?? []).map((run) => run.length + 1)), + ); + return [fence + language, value, fence].join("\n"); +} + function prepare(key: string, options: PrepOptions) { if (!directoryKey.test(key) || !line.safeParse(options.summary).success) { throw new PrepError( @@ -216,6 +252,18 @@ function prepare(key: string, options: PrepOptions) { const isSelected = ( entry: z.infer["connectors"][number], ) => entry.manifest.connector_key === current.connector_key; + if (!previous && beforeIndex.connectors.some(isSelected)) { + throw new PrepError( + "connector_key_conflict", + "The upstream index already declares this connector key. Resolve the identity before submitting a new connector.", + ); + } + if (!git("ls-tree", head, "--", prefix + "index.ts")) { + throw new PrepError( + "connector_entry_missing", + "Commit the connector index.ts entry before preparing its PR.", + ); + } const selected = afterIndex.connectors.filter(isSelected); if ( selected.length !== 1 || @@ -247,7 +295,7 @@ function prepare(key: string, options: PrepOptions) { if (evidence && !evidence.success) { throw new PrepError( "invalid_evidence", - "Supply a reviewed synthetic summary with kind, commit, checkedAt and checks containing name and result.", + "Supply reviewed synthetic evidence. Check the schema in vana skills show create-connector --json.", ); } const verification = evidence?.data ?? null; @@ -257,8 +305,26 @@ function prepare(key: string, options: PrepOptions) { "The evidence names a different commit. Check the selected revision and prepare a fresh summary.", ); } + const reviewGaps = [ + ...(verification?.environment ? [] : ["Missing the test environment."]), + ...(verification?.checks.flatMap((check) => + check.command && check.output && check.exitCode !== undefined + ? [] + : ["Missing command, exit status or output for " + check.name + "."], + ) ?? []), + ...(verification?.sample + ? [] + : ["Missing a reviewed synthetic source item and output record."]), + ...(previous && verification?.sample?.before === undefined + ? ["Missing the previous output record, or null if none was emitted."] + : []), + ...(verification?.storedData + ? [] + : ["Missing the effect on existing records, IDs and cursors."]), + ]; const evidenceGaps = [ ...(verification ? [] : ["No verification summary supplied."]), + ...reviewGaps, ...(verification?.checks .filter((check) => check.result !== "passed") .map((check) => check.name + ": " + check.result) ?? []), @@ -274,6 +340,7 @@ function prepare(key: string, options: PrepOptions) { " " + (previous ? previous.version + " → " : "") + current.version, + "Change: " + (previous ? "Connector update" : "New connector"), "Streams: " + current.streams.map((stream) => stream.name).join(", "), "", "## How it was verified", @@ -286,19 +353,71 @@ function prepare(key: string, options: PrepOptions) { head + ".", "", - ...verification.checks.map( - (check) => "- " + check.name + " — " + check.result, - ), + ...(verification.environment + ? ["Environment: " + verification.environment, ""] + : []), + ...verification.checks.flatMap((check) => [ + "- " + check.name + " — " + check.result, + ...(check.command ? ["", codeBlock(check.command, "sh")] : []), + ...(check.exitCode !== undefined + ? [ + "", + "Exit status: " + + (check.exitCode === null ? "not run" : check.exitCode), + ] + : []), + ...(check.output ? ["", codeBlock(check.output, "text")] : []), + "", + ]), "", ] : []), ...evidenceGaps.map((gap) => "- " + gap), "", + ...(verification?.sample + ? [ + "## Synthetic source item and output", + "", + "Source item", + "", + codeBlock( + JSON.stringify(verification.sample.source, null, 2), + "json", + ), + "", + ...(previous && verification.sample.before !== undefined + ? [ + "Previous output, with null meaning no record was emitted", + "", + codeBlock( + JSON.stringify(verification.sample.before, null, 2), + "json", + ), + "", + ] + : []), + "Output at the submitted commit", + "", + codeBlock(JSON.stringify(verification.sample.after, null, 2), "json"), + "", + ] + : []), + ...(verification?.storedData + ? ["## Existing data", "", verification.storedData, ""] + : []), "Assisted-by: AI", "", ].join("\n"); + if (Buffer.byteLength(prBody, "utf8") > 60_000) { + throw new PrepError( + "review_evidence_too_large", + "Keep the reviewed PR description and evidence within 60,000 UTF-8 bytes.", + ); + } + return { + change: previous ? ("update" as const) : ("new" as const), target: connectorPrTarget, baseBranch: "main", base, @@ -310,6 +429,7 @@ function prepare(key: string, options: PrepOptions) { files: changed, verification, evidenceGaps, + reviewGaps, prBody, diff: git( "diff", @@ -333,14 +453,13 @@ function prepareSubmission( .split("\n") .filter((entry) => entry.startsWith("parent ")); if ( - !preview.previousVersion || preview.head.length !== 40 || parents.length !== 1 || parents[0] !== "parent " + preview.base ) { throw new PrepError( - "single_connector_update_required", - "Submit one commit updating an existing connector, directly on the reviewed PDP-Connect main revision.", + "single_connector_commit_required", + "Submit one commit adding or updating a connector, directly on the reviewed PDP-Connect main revision.", ); } if ( @@ -362,6 +481,13 @@ function prepareSubmission( "Supply a reviewed synthetic verification summary before submitting.", ); } + if (preview.reviewGaps.length) { + throw new PrepError( + "review_evidence_required", + preview.reviewGaps.join(" ") + + " Read vana skills show create-connector --json.", + ); + } const submission = submitConnectorPr( { ...preview, @@ -465,7 +591,7 @@ function registerCommand( .command(submit ? "submit-pr " : "prepare-pr ") .description( submit - ? "Review and explicitly submit a committed connector update as a GitHub draft PR" + ? "Review and explicitly submit a new connector or update as a GitHub draft PR" : "Preview a committed PDP-Connect connector diff and draft PR body locally", ) .requiredOption( @@ -480,13 +606,17 @@ function registerCommand( .requiredOption("--summary ", "One-line description for the draft PR") .option( "--evidence ", - "Reviewed JSON with kind: synthetic, commit, checkedAt and checks [{name, result}]", + "Reviewed synthetic checks, commands, output, sample records and stored-data effects", ) .addHelpText( "after", submit ? "\nUses GitHub CLI sign-in and an existing public fork. Run without --confirm to review.\n" : "\nUses committed files only. Review the diff and evidence before sharing them.\n", + ) + .addHelpText( + "after", + "\nEvidence schema and submission steps: vana skills show create-connector --json\n", ); if (submit) command.option( diff --git a/test/cli/connector-pr-prep.test.ts b/test/cli/connector-pr-prep.test.ts index 1ae751d..1d5f02f 100644 --- a/test/cli/connector-pr-prep.test.ts +++ b/test/cli/connector-pr-prep.test.ts @@ -94,21 +94,38 @@ afterEach(() => { rmSync(root, { recursive: true, force: true }); }); +function reviewEvidence(commit: string) { + return { + kind: "synthetic", + commit, + checkedAt: "2026-11-11T00:00:00Z", + environment: "Node 24.22.1, synthetic fixture", + checks: [ + { + name: "Fixture test", + result: "passed", + command: "node --test connectors/example/index.test.cjs", + exitCode: 0, + output: "tests 1\npass 1\nfail 0", + }, + ], + sample: { + source: { id: "item-1", text: "Demo\\, review" }, + before: { id: "item-1", text: "Demo\\, review" }, + after: { id: "item-1", text: "Demo, review" }, + }, + storedData: + "IDs and types stay the same. Existing records require reimport; the cursor skips past records.", + }; +} + async function preview( evidenceCommit: string, command = "prepare-pr", ...args: string[] ) { const evidence = path.join(root, "synthetic.json"); - writeFileSync( - evidence, - JSON.stringify({ - kind: "synthetic", - commit: evidenceCommit, - checkedAt: "2026-11-10T00:00:00Z", - checks: [{ name: "Fixture test", result: "passed" }], - }), - ); + writeFileSync(evidence, JSON.stringify(reviewEvidence(evidenceCommit))); const stdout = vi.spyOn(process.stdout, "write").mockReturnValue(true); const code = await runCli([ "node", @@ -418,6 +435,335 @@ it("refuses to publish a commit range whose intermediate contents were absent fr const reviewed = await preview(head, "submit-pr"); expect(reviewed).toMatchObject({ code: 1, - result: { error: { code: "single_connector_update_required" } }, + result: { error: { code: "single_connector_commit_required" } }, + }); +}); + +function makeNewConnector(baseHasKey = false) { + git("checkout", "--quiet", "--detach", base); + rmSync(path.join(checkout, "connectors/example"), { recursive: true }); + const indexPath = path.join(checkout, "connector-implementation-index.json"); + const index = JSON.parse(readFileSync(indexPath, "utf8")); + if (!baseHasKey) { + index.connectors = index.connectors.filter( + (entry: { manifest: { connector_key: string } }) => + entry.manifest.connector_key !== "example", + ); + } + writeFileSync(indexPath, JSON.stringify(index)); + base = commit(); + mkdirSync(path.join(checkout, "connectors/example")); + writeConnector("0.1.0"); + writeFileSync( + path.join(checkout, "connectors/example/index.ts"), + String.raw`import { readFileSync } from "node:fs"; +const item = JSON.parse(readFileSync(0, "utf8")); +process.stdout.write(JSON.stringify({ id: item.id, text: item.text.replaceAll("\\,", ",") })); +`, + ); + const source = { id: "item-1", text: "Demo\\, review" }; + writeFileSync( + path.join(checkout, "connectors/example/source.json"), + JSON.stringify(source), + ); + writeFileSync( + path.join(checkout, "connectors/example/index.test.cjs"), + String.raw`const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const { readFileSync } = require("node:fs"); +const { join } = require("node:path"); +const { execFileSync } = require("node:child_process"); +test("emits the synthetic record with its stable ID and decoded text", () => { + const input = readFileSync(join(__dirname, "source.json"), "utf8"); + const output = execFileSync(process.execPath, ["--experimental-strip-types", join(__dirname, "index.ts")], { input, encoding: "utf8" }); + assert.deepEqual(JSON.parse(output), { id: "item-1", text: "Demo, review" }); +}); +`, + ); + head = commit(); + const output = execFileSync( + process.execPath, + ["--test", "connectors/example/index.test.cjs"], + { + cwd: checkout, + encoding: "utf8", + timeout: 10_000, + }, + ); + const record = JSON.parse( + execFileSync( + process.execPath, + ["--experimental-strip-types", "connectors/example/index.ts"], + { + cwd: checkout, + input: JSON.stringify(source), + encoding: "utf8", + timeout: 10_000, + }, + ), + ); + const evidence = path.join(root, "new-connector-evidence.json"); + writeFileSync( + evidence, + JSON.stringify({ + ...reviewEvidence(head), + environment: "Node " + process.version + ", synthetic fixture", + checks: [ + { + name: "Synthetic record test", + result: "passed", + command: "node --test connectors/example/index.test.cjs", + exitCode: 0, + output, + }, + ], + sample: { source, after: record }, + storedData: + "Initial upstream connector. Existing local test records are unchanged. IDs use the source item ID.", + }), + ); + return evidence; +} + +withShellFixtures( + "submits a new connector with review evidence and recovers it without writes", + async () => { + const evidence = makeNewConnector(); + const fixture = githubFixture(); + const reviewed = await preview(head, "submit-pr", "--evidence", evidence); + expect(reviewed.code).toBe(7); + expect(reviewed.result).toMatchObject({ + kind: "confirmation_required", + change: "new", + previousVersion: null, + version: "0.1.0", + }); + expect(reviewed.result.prBody).toContain("New connector"); + expect(reviewed.result.prBody).toContain( + "node --test connectors/example/index.test.cjs", + ); + expect(reviewed.result.prBody).toContain("Exit status: 0"); + expect(reviewed.result.prBody).toContain("Demo, review"); + expect(fixture.refs()).toBe(""); + const confirmed = await preview( + head, + "submit-pr", + "--evidence", + evidence, + "--confirm", + reviewed.result.confirmation, + ); + expect(confirmed).toMatchObject({ + code: 0, + result: { number: 42, draft: true, reused: false }, + }); + const submitted = JSON.parse( + readFileSync(fixture.receipt, "utf8"), + ).submitted; + expect(submitted.body).toContain(head); + expect(submitted.body).toContain("tests 1"); + expect(submitted.body).toContain("Demo, review"); + vi.stubEnv("VANA_TEST_FORK_READ_ONLY", "1"); + expect( + await preview( + head, + "submit-pr", + "--evidence", + evidence, + "--confirm", + reviewed.result.confirmation, + ), + ).toMatchObject({ + code: 0, + result: { number: 42, reused: true }, + }); + expect(fixture.writeAttempts()).toBe("push\ncreate PR\n"); + expect(fixture.refs()).toBe(head); + }, +); + +withShellFixtures.each([ + [ + "missing environment", + { environment: undefined }, + "review_evidence_required", + ], + ["missing sample", { sample: undefined }, "review_evidence_required"], + [ + "missing stored-data effect", + { storedData: undefined }, + "review_evidence_required", + ], + [ + "missing command and output", + { checks: [{ name: "Fixture test", result: "passed" }] }, + "review_evidence_required", + ], + [ + "missing previous output", + { sample: { source: {}, after: {} } }, + "review_evidence_required", + ], + [ + "contradictory exit status", + { + checks: [ + { + name: "Fixture test", + result: "passed", + command: "node --test", + exitCode: 1, + output: "failed", + }, + ], + }, + "invalid_evidence", + ], +])("blocks submission with %s before any write", async (_name, patch, code) => { + const fixture = githubFixture(); + const evidence = path.join(root, "incomplete.json"); + writeFileSync( + evidence, + JSON.stringify({ ...reviewEvidence(head), ...(patch as object) }), + ); + expect( + await preview(head, "submit-pr", "--evidence", evidence), + ).toMatchObject({ + code: 1, + result: { error: { code } }, + }); + expect(fixture.refs()).toBe(""); + expect(existsSync(fixture.receipt)).toBe(false); +}); + +it("keeps legacy evidence previewable and makes its review gaps explicit", async () => { + const evidence = path.join(root, "legacy.json"); + writeFileSync( + evidence, + JSON.stringify({ + kind: "synthetic", + commit: head, + checkedAt: "2026-11-11T00:00:00Z", + checks: [{ name: "Old summary", result: "passed" }], + }), + ); + const { code, result } = await preview( + head, + "prepare-pr", + "--evidence", + evidence, + ); + expect(code).toBe(0); + expect(result.prBody).toContain("Old summary"); + expect(result.reviewGaps).toContain( + "Missing a reviewed synthetic source item and output record.", + ); +}); + +withShellFixtures( + "requires fresh confirmation if evidence changes after review", + async () => { + const fixture = githubFixture(); + const reviewed = await preview(head, "submit-pr"); + const evidence = path.join(root, "edited.json"); + writeFileSync( + evidence, + JSON.stringify({ + ...reviewEvidence(head), + storedData: "Updated migration explanation after review.", + }), + ); + expect( + await preview( + head, + "submit-pr", + "--evidence", + evidence, + "--confirm", + reviewed.result.confirmation, + ), + ).toMatchObject({ + code: 7, + result: { error: { code: "confirmation_mismatch" } }, + }); + expect(fixture.refs()).toBe(""); + expect(existsSync(fixture.receipt)).toBe(false); + }, +); + +it("rejects a new connector whose key is already declared upstream", async () => { + makeNewConnector(true); + const { code, result } = await preview(head); + expect(code).toBe(1); + expect(result.error.code).toBe("connector_key_conflict"); +}); + +it("rejects a connector with no committed entry point", async () => { + rmSync(path.join(checkout, "connectors/example/index.ts")); + head = commit(); + const { code, result } = await preview(head); + expect(code).toBe(1); + expect(result.error.code).toBe("connector_entry_missing"); +}); + +it("keeps failed and skipped checks visible and safely quotes review output", async () => { + const evidence = path.join(root, "reported-results.json"); + const passed = reviewEvidence(head).checks[0]; + writeFileSync( + evidence, + JSON.stringify({ + ...reviewEvidence(head), + checks: [ + passed, + { + ...passed, + name: "Known upstream failure", + result: "failed", + exitCode: 1, + output: "````\nfailed on the documented baseline\n````", + }, + { + ...passed, + name: "Live provider", + result: "not run", + exitCode: null, + output: "No live provider access was used.", + }, + ], + }), + ); + const { code, result } = await preview( + head, + "prepare-pr", + "--evidence", + evidence, + ); + expect(code).toBe(0); + expect(result.prBody).toContain("Fixture test — passed"); + expect(result.prBody).toContain("Known upstream failure — failed"); + expect(result.prBody).toContain("Live provider — not run"); + expect(result.prBody).toContain("Exit status: not run"); + expect(result.prBody).toContain( + "`````text\n````\nfailed on the documented baseline\n````\n`````", + ); + expect(result.evidenceGaps).toContain( + "Live provider behavior and Vana app availability remain unverified.", + ); +}); + +it("rejects evidence that cannot fit into the public PR description", async () => { + const evidence = path.join(root, "oversized.json"); + writeFileSync( + evidence, + JSON.stringify({ + ...reviewEvidence(head), + sample: { source: {}, before: {}, after: { text: "x".repeat(60_000) } }, + }), + ); + expect( + await preview(head, "prepare-pr", "--evidence", evidence), + ).toMatchObject({ + code: 1, + result: { error: { code: "review_evidence_too_large" } }, }); }); From 6a53e285ccbf61fb0875047ffd4757a73bb0e7b4 Mon Sep 17 00:00:00 2001 From: Callum Flack <749885+callumflack@users.noreply.github.com> Date: Wed, 11 Nov 2026 14:01:33 +1000 Subject: [PATCH 4/5] fix(cli): allow scoped new-connector registration rows BUI-942 Assisted-by: AI Signed-off-by: Callum Flack <749885+callumflack@users.noreply.github.com> --- skills/create-connector/SKILL.md | 8 +- src/cli/connector-pr-prep.ts | 137 ++++++++++++++++++++++++- test/cli/connector-pr-prep.test.ts | 154 ++++++++++++++++++++++++++++- 3 files changed, 292 insertions(+), 7 deletions(-) diff --git a/skills/create-connector/SKILL.md b/skills/create-connector/SKILL.md index eaf8987..1b1de60 100644 --- a/skills/create-connector/SKILL.md +++ b/skills/create-connector/SKILL.md @@ -127,9 +127,11 @@ run the upstream checks, then prepare its public handoff. These commands read reported evidence. They do not execute the test commands in the evidence file. 1. Keep one signed-off commit directly on the reviewed upstream main revision. - Change only the selected connector and its generated implementation-index - entry. New connectors need a unique key, manifest and `index.ts`. Updates - need a higher manifest version. + Change the selected connector and its generated implementation-index entry. + For a new connector, put its required conformance roster, sequential-await + coverage-gap and publication inventory entries first in their upstream\n declarations. + Keep the other rows intact. New connectors need a unique key, manifest and + `index.ts`. Updates need a higher manifest version. 2. Save reviewed synthetic evidence outside the connector commit using the example below. Replace the commit, date, commands, results, source and record with observed values. This content becomes public with the PR. diff --git a/src/cli/connector-pr-prep.ts b/src/cli/connector-pr-prep.ts index d3451ad..afcab95 100644 --- a/src/cli/connector-pr-prep.ts +++ b/src/cli/connector-pr-prep.ts @@ -13,6 +13,44 @@ import { import { isNewerVersion } from "./update-check.js"; const INDEX_PATH = "connector-implementation-index.json"; +const registrations = new Map([ + [ + "packages/polyfill-connectors/src/connector-conformance-roster.ts", + { + header: + /^export const REAL_UNLISTED_CONNECTORS: Record = \{\n/m, + row: /^[\t ]+(?[a-z0-9_]+): \{ testFile: "(?[^"\\\r\n]+)" \},\n/gm, + single: true, + }, + ], + [ + "packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts", + { + header: + /^export const NO_AWAIT_IN_LOOPS_ALLOWLIST: readonly NoAwaitInLoopsAllowlistEntry\[\] =\n\t\[\n/m, + row: /^\t\t\{\n\t\t\tpath: "(?[^"\\\r\n]+)",\n\t\t\tline: [1-9]\d*,\n\t\t\tcolumn: [1-9]\d*,\n\t\t\tcategory: "[a-z_]+",\n\t\t\tnote: "[^"\\\r\n]+",\n\t\t\},\n/gm, + single: false, + }, + ], + [ + "connectors/_conformance/coverage-conformance-drivers.ts", + { + header: + /^export const KNOWN_UNEXERCISED_COVERAGE: ReadonlySet = new Set\(\[\n/m, + row: /^\t\/\/ [^\r\n\u2028\u2029]+\n\t"(?[a-z0-9_-]+\.[a-z0-9_]+)",\n/gm, + single: false, + }, + ], + [ + "scripts/connector-publish-allowlist.mjs", + { + header: + /^export const CONNECTOR_PUBLISH_INVENTORY = Object\.freeze\(\n \[\n/m, + row: /^ \["(?[a-z0-9_]+)", "(?[a-z0-9-]+)"\],\n/gm, + single: true, + }, + ], +]); const commitId = /^[a-f0-9]{40}(?:[a-f0-9]{24})?$/; const directoryKey = /^[a-z0-9]+(?:_[a-z0-9]+)*$/; const version = /^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)$/; @@ -133,6 +171,75 @@ function localGit(checkout: string) { }; } +function validNewConnectorRegistration( + git: ReturnType, + file: string, + key: string, + requiredStreams: string[], + base: string, + head: string, +): boolean { + const rule = registrations.get(file); + if (!rule) return false; + const baseMode = git("ls-tree", base, "--", file).slice(0, 6); + if (!baseMode || baseMode !== git("ls-tree", head, "--", file).slice(0, 6)) { + return false; + } + + const before = git("show", base + ":" + file); + const after = git("show", head + ":" + file); + const declaration = rule.header.exec(before); + if (!declaration) return false; + + // Both files are executable TypeScript. Permit only data rows inserted + // directly after the named declaration, and preserve every other byte. + const start = declaration.index + declaration[0].length; + const suffix = before.slice(start); + if ( + after.length <= before.length || + after.slice(0, start) !== before.slice(0, start) || + !after.endsWith(suffix) + ) { + return false; + } + const inserted = after.slice(start, after.length - suffix.length); + const rows = [...inserted.matchAll(rule.row)]; + if ( + rows.map((row) => row[0]).join("") !== inserted || + (rule.single && rows.length !== 1) + ) { + return false; + } + const unregisteredStreams = new Set( + requiredStreams.map((stream) => key + "." + stream), + ); + return rows.every(({ groups }) => { + if (groups?.manifest) { + return ( + groups.manifest === key && groups.connector === key.replaceAll("_", "-") + ); + } + if (groups?.stream) return unregisteredStreams.delete(groups.stream); + const path = groups?.path; + if (!path?.startsWith("connectors/" + key + "/")) return false; + if ( + !/^[a-zA-Z0-9_./-]+$/.test(path) || + path.split("/").some((part) => !part || part === "." || part === "..") + ) { + return false; + } + if ( + rule.single && + (groups?.key !== key || !/\.test\.[cm]?[jt]s$/.test(path)) + ) { + return false; + } + return /^100(?:644|755) blob [a-f0-9]+\t/.test( + git("ls-tree", head, "--", path), + ); + }); +} + function codeBlock(value: string, language: string) { const fence = "`".repeat( Math.max(3, ...(value.match(/`+/g) ?? []).map((run) => run.length + 1)), @@ -185,13 +292,15 @@ function prepare(key: string, options: PrepOptions) { !changed.some((file) => file.startsWith(prefix)) || changed.some( (file) => - (file !== INDEX_PATH && !file.startsWith(prefix)) || + (file !== INDEX_PATH && + !file.startsWith(prefix) && + !registrations.has(file)) || /\p{Cc}/u.test(file), ) ) { throw new PrepError( "connector_diff_required", - "Select a commit range changing only the chosen connector and its generated index entry.", + "Select a commit range changing only the chosen connector, generated index entry and allowed registration rows.", ); } // Raw modes expose symlinks and Git links, including deletions. Neither @@ -243,6 +352,30 @@ function prepare(key: string, options: PrepOptions) { ); } + const requiredStreams = current.streams + .filter((stream) => stream.required === true) + .map((stream) => stream.name); + for (const file of changed.filter((path) => registrations.has(path))) { + if ( + previous || + !validNewConnectorRegistration( + git, + file, + key, + requiredStreams, + base, + head, + ) + ) { + throw new PrepError( + "invalid_connector_registration", + "Only add registration rows for the selected new connector in " + + file + + ".", + ); + } + } + const beforeIndex = indexSchema.parse( JSON.parse(git("show", base + ":" + INDEX_PATH)), ); diff --git a/test/cli/connector-pr-prep.test.ts b/test/cli/connector-pr-prep.test.ts index 1d5f02f..1cad471 100644 --- a/test/cli/connector-pr-prep.test.ts +++ b/test/cli/connector-pr-prep.test.ts @@ -45,7 +45,7 @@ function writeConnector(version: string, otherVersion = "1.0.0") { const manifest = { connector_key: "example", version, - streams: [{ name: "items" }], + streams: [{ name: "items", required: true }], }; writeFileSync( path.join(checkout, "connectors/example/manifest.json"), @@ -439,6 +439,72 @@ it("refuses to publish a commit range whose intermediate contents were absent fr }); }); +const rosterFile = + "packages/polyfill-connectors/src/connector-conformance-roster.ts"; +const awaitListFile = + "packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts"; +const publishFile = "scripts/connector-publish-allowlist.mjs"; +const publishHeader = + "export const CONNECTOR_PUBLISH_INVENTORY = Object.freeze(\n [\n"; +const coverageFile = "connectors/_conformance/coverage-conformance-drivers.ts"; +const coverageHeader = + "export const KNOWN_UNEXERCISED_COVERAGE: ReadonlySet = new Set([\n"; +const rosterHeader = + "export const REAL_UNLISTED_CONNECTORS: Record = {\n"; +const awaitListHeader = + "export const NO_AWAIT_IN_LOOPS_ALLOWLIST: readonly NoAwaitInLoopsAllowlistEntry[] =\n\t[\n"; +const newRosterRow = + '\texample: { testFile: "connectors/example/index.test.cjs" },\n'; +const awaitRow = (connector: string) => + [ + "\t\t{", + '\t\t\tpath: "connectors/' + connector + '/index.ts",', + "\t\t\tline: 2,", + "\t\t\tcolumn: 3,", + '\t\t\tcategory: "dependent_pagination",', + '\t\t\tnote: "Read the next page after the cursor arrives.",', + "\t\t},", + "", + ].join("\n"); + +function writeRegistrationFixture(added: boolean) { + const roster = path.join(checkout, rosterFile); + const awaitList = path.join(checkout, awaitListFile); + mkdirSync(path.dirname(roster), { recursive: true }); + mkdirSync(path.dirname(awaitList), { recursive: true }); + writeFileSync( + roster, + rosterHeader + + (added ? newRosterRow : "") + + '\tother: { testFile: "connectors/other/index.test.ts" },\n};\n', + ); + writeFileSync( + awaitList, + awaitListHeader + + (added ? awaitRow("example") : "") + + awaitRow("other") + + "\t];\n", + ); + const coverage = path.join(checkout, coverageFile); + mkdirSync(path.dirname(coverage), { recursive: true }); + writeFileSync( + coverage, + coverageHeader + + (added + ? '\t// Development collector fixture. Shared driver is pending.\n\t"example.items",\n' + : "") + + '\t"other.items",\n]);\n', + ); + const publish = path.join(checkout, publishFile); + mkdirSync(path.dirname(publish), { recursive: true }); + writeFileSync( + publish, + publishHeader + + (added ? ' ["example", "example"],\n' : "") + + ' ["other", "other"],\n ],\n);\n', + ); +} + function makeNewConnector(baseHasKey = false) { git("checkout", "--quiet", "--detach", base); rmSync(path.join(checkout, "connectors/example"), { recursive: true }); @@ -451,6 +517,7 @@ function makeNewConnector(baseHasKey = false) { ); } writeFileSync(indexPath, JSON.stringify(index)); + writeRegistrationFixture(false); base = commit(); mkdirSync(path.join(checkout, "connectors/example")); writeConnector("0.1.0"); @@ -480,7 +547,14 @@ test("emits the synthetic record with its stable ID and decoded text", () => { }); `, ); + writeRegistrationFixture(true); head = commit(); +} + +function captureNewConnectorEvidence() { + const source = JSON.parse( + readFileSync(path.join(checkout, "connectors/example/source.json"), "utf8"), + ); const output = execFileSync( process.execPath, ["--test", "connectors/example/index.test.cjs"], @@ -528,7 +602,8 @@ test("emits the synthetic record with its stable ID and decoded text", () => { withShellFixtures( "submits a new connector with review evidence and recovers it without writes", async () => { - const evidence = makeNewConnector(); + makeNewConnector(); + const evidence = captureNewConnectorEvidence(); const fixture = githubFixture(); const reviewed = await preview(head, "submit-pr", "--evidence", evidence); expect(reviewed.code).toBe(7); @@ -539,6 +614,14 @@ withShellFixtures( version: "0.1.0", }); expect(reviewed.result.prBody).toContain("New connector"); + expect(reviewed.result.files).toEqual( + expect.arrayContaining([ + rosterFile, + awaitListFile, + coverageFile, + publishFile, + ]), + ); expect(reviewed.result.prBody).toContain( "node --test connectors/example/index.test.cjs", ); @@ -580,6 +663,7 @@ withShellFixtures( expect(fixture.writeAttempts()).toBe("push\ncreate PR\n"); expect(fixture.refs()).toBe(head); }, + 15_000, ); withShellFixtures.each([ @@ -767,3 +851,69 @@ it("rejects evidence that cannot fit into the public PR description", async () = result: { error: { code: "review_evidence_too_large" } }, }); }); + +it.each([ + [ + "a mismatched publication identity", + publishFile, + '["example", "example"]', + '["example", "other"]', + ], + [ + "a different connector's coverage entry", + coverageFile, + '"example.items"', + '"other.items"', + ], + [ + "an undeclared coverage stream", + coverageFile, + '"example.items"', + '"example.missing"', + ], + [ + "another roster entry", + rosterFile, + 'other: { testFile: "connectors/other/index.test.ts" }', + 'other: { testFile: "connectors/other/replaced.test.ts" }', + ], + [ + "a roster target outside the connector", + rosterFile, + "connectors/example/index.test.cjs", + "connectors/other/index.test.ts", + ], + [ + "an uncommitted roster test", + rosterFile, + "connectors/example/index.test.cjs", + "connectors/example/missing.test.ts", + ], + ["another allowlist entry", awaitListFile, awaitRow("other"), ""], + [ + "an allowlist path outside the connector", + awaitListFile, + "connectors/example/index.ts", + "connectors/example/../other/index.ts", + ], + [ + "a registration with additional code", + awaitListFile, + "line: 2,", + "line: readProcessEnv(),", + ], +])( + "rejects %s in a new-connector registration", + async (_name, file, before, after) => { + makeNewConnector(); + const target = path.join(checkout, file); + const text = readFileSync(target, "utf8"); + expect(text).toContain(before); + writeFileSync(target, text.replace(before, after)); + head = commit(); + const { code, result } = await preview(head); + expect(code).toBe(1); + expect(result.error.code).toBe("invalid_connector_registration"); + expect(result).not.toHaveProperty("diff"); + }, +); From 11df55e39e6f7cb7f5c4651ff2b93987e3a3ba76 Mon Sep 17 00:00:00 2001 From: Callum Flack <749885+callumflack@users.noreply.github.com> Date: Wed, 11 Nov 2026 14:19:23 +1000 Subject: [PATCH 5/5] fix(cli): validate generated connector registrations BUI-942 Assisted-by: AI Signed-off-by: Callum Flack <749885+callumflack@users.noreply.github.com> --- skills/create-connector/SKILL.md | 3 +- src/cli/connector-pr-prep.ts | 197 ++++++++++++++++++++++++++--- test/cli/connector-pr-prep.test.ts | 70 ++++++++++ 3 files changed, 251 insertions(+), 19 deletions(-) diff --git a/skills/create-connector/SKILL.md b/skills/create-connector/SKILL.md index 1b1de60..c299c9d 100644 --- a/skills/create-connector/SKILL.md +++ b/skills/create-connector/SKILL.md @@ -130,7 +130,8 @@ reported evidence. They do not execute the test commands in the evidence file. Change the selected connector and its generated implementation-index entry. For a new connector, put its required conformance roster, sequential-await coverage-gap and publication inventory entries first in their upstream\n declarations. - Keep the other rows intact. New connectors need a unique key, manifest and + Regenerate the consent and credential tables and update the manifest count + by one. Keep the other rows intact. New connectors need a unique key, manifest and `index.ts`. Updates need a higher manifest version. 2. Save reviewed synthetic evidence outside the connector commit using the example below. Replace the commit, date, commands, results, source and diff --git a/src/cli/connector-pr-prep.ts b/src/cli/connector-pr-prep.ts index afcab95..923f1ab 100644 --- a/src/cli/connector-pr-prep.ts +++ b/src/cli/connector-pr-prep.ts @@ -13,7 +13,23 @@ import { import { isNewerVersion } from "./update-check.js"; const INDEX_PATH = "connector-implementation-index.json"; -const registrations = new Map([ +const GENERATED_ROOT = "packages/polyfill-connectors/src/generated/"; +const CONSENT_AGGREGATE = GENERATED_ROOT + "consent-time-fields.generated.ts"; +const MANIFEST_COUNT_TEST = + "packages/polyfill-connectors/src/public-listing-manifest-honesty.test.ts"; +const consentHeader = + /^export const CONSENT_TIME_FIELDS: Readonly<\n\tRecord>>\n> = \{\n/m; +const consentRow = + /^\t(?[a-z0-9_]+): \{\n(?(?:\t\t(?:[a-zA-Z0-9_]+|"[a-zA-Z0-9_-]+"): (?:null|\{ field: "[a-zA-Z0-9_.-]+", format: "date(?:-time)?" \}),\n)+)\t\},\n/gm; + +interface RegistrationRule { + header: RegExp; + row: RegExp; + single: boolean; + generated?: "consent" | "secret"; +} + +const registrations = new Map([ [ "packages/polyfill-connectors/src/connector-conformance-roster.ts", { @@ -50,6 +66,25 @@ const registrations = new Map([ single: true, }, ], + [ + CONSENT_AGGREGATE, + { + header: consentHeader, + row: consentRow, + single: true, + generated: "consent", + }, + ], + [ + GENERATED_ROOT + "static-secret-registry.generated.ts", + { + header: + /^export const GENERATED_STATIC_SECRET_REGISTRY: Readonly<\n\tRecord\n> = Object\.freeze\(\{\n/m, + row: /^\t(?[a-z0-9_]+|"[a-z0-9-]+"): \{\n\t\tcredentialKind: "(?[a-z_]+)",\n\t\tsecretEnvVars: \[(?"[A-Z][A-Z0-9_]*"(?:, "[A-Z][A-Z0-9_]*")*)\],\n\t\},\n/gm, + single: true, + generated: "secret", + }, + ], ]); const commitId = /^[a-f0-9]{40}(?:[a-f0-9]{24})?$/; const directoryKey = /^[a-z0-9]+(?:_[a-z0-9]+)*$/; @@ -171,16 +206,126 @@ function localGit(checkout: string) { }; } +function consentSlicePath(key: string): string { + return GENERATED_ROOT + "consent-time-fields/" + key + ".generated.ts"; +} + +function registrationFile(file: string, key: string): boolean { + return ( + registrations.has(file) || + file === MANIFEST_COUNT_TEST || + file === consentSlicePath(key) + ); +} + +function validGeneratedRow( + groups: Record, + kind: "consent" | "secret", + key: string, + manifest: z.infer, +): boolean { + if (kind === "consent") { + if (groups.generatedKey !== key) return false; + const fields = groups.fields.trim().split("\n"); + return ( + isDeepStrictEqual( + fields + .map((line) => line.trim().split(": ")[0].replace(/^"|"$/g, "")) + .sort(), + manifest.streams.map((stream) => stream.name).sort(), + ) && + fields.every((line) => { + const [name, value] = line.trim().split(": "); + if (value === "null,") return true; + const field = line.match(/field: "([^"]+)"/)?.[1]; + return ( + field === + manifest.streams.find( + (stream) => stream.name === name.replace(/^"|"$/g, ""), + )?.consent_time_field + ); + }) + ); + } + const connectorKey = groups.generatedKey.replace(/^"|"$/g, ""); + if (connectorKey !== manifest.connector_key) return false; + const capture = z + .looseObject({ + setup: z.looseObject({ + credential_capture: z.looseObject({ + kind: z.string(), + fields: z.array( + z.looseObject({ + secret: z.boolean().optional(), + env: z.array(z.string()).optional(), + }), + ), + }), + }), + }) + .safeParse(manifest); + if (!capture.success) return false; + const credential = capture.data.setup.credential_capture; + return ( + groups.kind === credential.kind && + isDeepStrictEqual( + (JSON.parse("[" + groups.env + "]") as string[]).sort(), + [ + ...new Set( + credential.fields + .filter((field) => field.secret) + .flatMap((field) => field.env ?? []), + ), + ].sort(), + ) + ); +} + +function validConsentSlice( + git: ReturnType, + key: string, + manifest: z.infer, + base: string, + head: string, +): boolean { + const file = consentSlicePath(key); + if (git("ls-tree", base, "--", file)) return false; + const aggregate = git("show", head + ":" + CONSENT_AGGREGATE); + const rows = [...aggregate.matchAll(consentRow)].filter( + ({ groups }) => + groups && validGeneratedRow(groups, "consent", key, manifest), + ); + if (rows.length !== 1) return false; + const table = consentHeader.exec(aggregate)?.[0]; + if (!table) return false; + const module = git("show", head + ":" + file); + const expected = + 'import type { ConsentTimeField } from "../../time-range.ts";\n\n' + + 'export const CONSENT_BOUND_CONNECTOR: string | null = "' + + key + + '";\n\n' + + table + + rows[0][0] + + "};\n"; + return ( + module.endsWith(expected) && + /^(?:(?:\/\/[^\r\n\u2028\u2029]*)?\n)*$/.test( + module.slice(0, -expected.length), + ) + ); +} + function validNewConnectorRegistration( git: ReturnType, file: string, key: string, - requiredStreams: string[], + manifest: z.infer, base: string, head: string, ): boolean { - const rule = registrations.get(file); - if (!rule) return false; + if (file === consentSlicePath(key)) { + return validConsentSlice(git, key, manifest, base, head); + } const baseMode = git("ls-tree", base, "--", file).slice(0, 6); if (!baseMode || baseMode !== git("ls-tree", head, "--", file).slice(0, 6)) { return false; @@ -188,12 +333,36 @@ function validNewConnectorRegistration( const before = git("show", base + ":" + file); const after = git("show", head + ":" + file); + if (file === MANIFEST_COUNT_TEST) { + const countLine = /^\tassert\.equal\(names\.length, (\d+)\);$/m; + const match = before.match(countLine); + return ( + !!match && + before.replace( + countLine, + "\tassert.equal(names.length, " + (Number(match[1]) + 1) + ");", + ) === after + ); + } + const rule = registrations.get(file); + if (!rule) return false; const declaration = rule.header.exec(before); if (!declaration) return false; // Both files are executable TypeScript. Permit only data rows inserted // directly after the named declaration, and preserve every other byte. const start = declaration.index + declaration[0].length; + const generated = rule.generated; + if (generated) { + return [...after.matchAll(rule.row)].some( + (row) => + row.index >= start && + row.groups && + validGeneratedRow(row.groups, generated, key, manifest) && + after.slice(0, row.index) + after.slice(row.index + row[0].length) === + before, + ); + } const suffix = before.slice(start); if ( after.length <= before.length || @@ -211,7 +380,9 @@ function validNewConnectorRegistration( return false; } const unregisteredStreams = new Set( - requiredStreams.map((stream) => key + "." + stream), + manifest.streams + .filter((stream) => stream.required === true) + .map((stream) => key + "." + stream.name), ); return rows.every(({ groups }) => { if (groups?.manifest) { @@ -294,7 +465,7 @@ function prepare(key: string, options: PrepOptions) { (file) => (file !== INDEX_PATH && !file.startsWith(prefix) && - !registrations.has(file)) || + !registrationFile(file, key)) || /\p{Cc}/u.test(file), ) ) { @@ -352,20 +523,10 @@ function prepare(key: string, options: PrepOptions) { ); } - const requiredStreams = current.streams - .filter((stream) => stream.required === true) - .map((stream) => stream.name); - for (const file of changed.filter((path) => registrations.has(path))) { + for (const file of changed.filter((path) => registrationFile(path, key))) { if ( previous || - !validNewConnectorRegistration( - git, - file, - key, - requiredStreams, - base, - head, - ) + !validNewConnectorRegistration(git, file, key, current, base, head) ) { throw new PrepError( "invalid_connector_registration", diff --git a/test/cli/connector-pr-prep.test.ts b/test/cli/connector-pr-prep.test.ts index 1cad471..0aed72a 100644 --- a/test/cli/connector-pr-prep.test.ts +++ b/test/cli/connector-pr-prep.test.ts @@ -46,6 +46,12 @@ function writeConnector(version: string, otherVersion = "1.0.0") { connector_key: "example", version, streams: [{ name: "items", required: true }], + setup: { + credential_capture: { + kind: "personal_access_token", + fields: [{ secret: true, env: ["SYNTHETIC_TOKEN"] }], + }, + }, }; writeFileSync( path.join(checkout, "connectors/example/manifest.json"), @@ -443,6 +449,15 @@ const rosterFile = "packages/polyfill-connectors/src/connector-conformance-roster.ts"; const awaitListFile = "packages/polyfill-connectors/scripts/no-await-in-loops-allowlist.ts"; +const generatedRoot = "packages/polyfill-connectors/src/generated/"; +const consentAggregate = generatedRoot + "consent-time-fields.generated.ts"; +const consentSlice = generatedRoot + "consent-time-fields/example.generated.ts"; +const staticSecret = generatedRoot + "static-secret-registry.generated.ts"; +const manifestCount = + "packages/polyfill-connectors/src/public-listing-manifest-honesty.test.ts"; +const consentHeader = + "export const CONSENT_TIME_FIELDS: Readonly<\n\tRecord>>\n> = {\n"; +const consentRow = "\texample: {\n\t\titems: null,\n\t},\n"; const publishFile = "scripts/connector-publish-allowlist.mjs"; const publishHeader = "export const CONNECTOR_PUBLISH_INVENTORY = Object.freeze(\n [\n"; @@ -503,6 +518,37 @@ function writeRegistrationFixture(added: boolean) { (added ? ' ["example", "example"],\n' : "") + ' ["other", "other"],\n ],\n);\n', ); + const consent = path.join(checkout, consentAggregate); + const secret = path.join(checkout, staticSecret); + mkdirSync(path.dirname(consent), { recursive: true }); + const otherConsent = "\tother: {\n\t\titems: null,\n\t},\n"; + writeFileSync( + consent, + consentHeader + otherConsent + (added ? consentRow : "") + "};\n", + ); + const secretHeader = + "export const GENERATED_STATIC_SECRET_REGISTRY: Readonly<\n\tRecord\n> = Object.freeze({\n"; + const secretRow = + '\texample: {\n\t\tcredentialKind: "personal_access_token",\n\t\tsecretEnvVars: ["SYNTHETIC_TOKEN"],\n\t},\n'; + writeFileSync(secret, secretHeader + (added ? secretRow : "") + "});\n"); + const count = path.join(checkout, manifestCount); + writeFileSync( + count, + "test('manifest inventory', () => {\n\tassert.equal(names.length, " + + (added ? 2 : 1) + + ");\n});\n", + ); + if (added) { + const slice = path.join(checkout, consentSlice); + mkdirSync(path.dirname(slice), { recursive: true }); + writeFileSync( + slice, + '// Generated for example.\n\nimport type { ConsentTimeField } from "../../time-range.ts";\n\nexport const CONSENT_BOUND_CONNECTOR: string | null = "example";\n\n' + + consentHeader + + consentRow + + "};\n", + ); + } } function makeNewConnector(baseHasKey = false) { @@ -853,6 +899,30 @@ it("rejects evidence that cannot fit into the public PR description", async () = }); it.each([ + [ + "a generated credential not declared by the connector", + staticSecret, + '["SYNTHETIC_TOKEN"]', + '["OTHER_TOKEN"]', + ], + [ + "an unrelated stream in the generated consent row", + consentAggregate, + "items: null", + "unexpected: null", + ], + [ + "executable code in the consent slice", + consentSlice, + "// Generated for example.", + "console.log(process.env);", + ], + [ + "an incorrect manifest count increment", + manifestCount, + "names.length, 2", + "names.length, 3", + ], [ "a mismatched publication identity", publishFile,