diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index b5b107741f..e328847024 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -2,7 +2,7 @@ * @supabase/cli # Files that bots update. Either owner satisfies code-owner review; `supabase-oss` approves only -# bot-opened PRs, through bot-pr-auto-merge.yml. +# bot-opened PRs, through bot-pr-auto-merge.yml and sync-repair.yml. /.github/workflows/*.yml @supabase/cli @supabase-oss /package.json @supabase/cli @supabase-oss /apps/*/package.json @supabase/cli @supabase-oss diff --git a/.github/scripts/ai-review/generate-pr-diff.test.ts b/.github/scripts/ai-review/generate-pr-diff.test.ts index 471b00691c..bf58f72002 100644 --- a/.github/scripts/ai-review/generate-pr-diff.test.ts +++ b/.github/scripts/ai-review/generate-pr-diff.test.ts @@ -87,4 +87,31 @@ describe("generatePrDiff", () => { expect(diff).toContain("+feature"); expect(diff).not.toContain("base-only.txt"); }); + + test("diffs the pinned head even after the PR branch moves", () => { + const { seed, checkout } = setupRepository(); + git(seed, "switch", "-c", "feature"); + writeFileSync(join(seed, "reviewed.txt"), "reviewed\n"); + git(seed, "add", "reviewed.txt"); + git(seed, "commit", "-m", "reviewed change"); + publishPullRequest(seed, 88); + const headSha = git(seed, "rev-parse", "HEAD"); + writeFileSync(join(seed, "late.txt"), "pushed during the review\n"); + git(seed, "add", "late.txt"); + git(seed, "commit", "-m", "late change"); + publishPullRequest(seed, 88); + + const outputPath = join(checkout, "pr.diff"); + generatePrDiff({ + repositoryPath: checkout, + prNumber: 88, + baseRef: "develop", + outputPath, + headSha, + }); + + const diff = readFileSync(outputPath, "utf8"); + expect(diff).toContain("+reviewed"); + expect(diff).not.toContain("late.txt"); + }); }); diff --git a/.github/scripts/ai-review/generate-pr-diff.ts b/.github/scripts/ai-review/generate-pr-diff.ts index dc6b84d54f..9d7d78f571 100644 --- a/.github/scripts/ai-review/generate-pr-diff.ts +++ b/.github/scripts/ai-review/generate-pr-diff.ts @@ -7,6 +7,8 @@ export interface GeneratePrDiffOptions { prNumber: number; baseRef: string; outputPath: string; + /** Commit to diff instead of the PR's current head, so a push during the review is not reviewed unseen. */ + headSha?: string; } function runGit(repositoryPath: string, args: string[]): void { @@ -22,19 +24,22 @@ function runGit(repositoryPath: string, args: string[]): void { } } -function validateInputs(prNumber: number, baseRef: string): void { +function validateInputs(prNumber: number, baseRef: string, headSha?: string): void { if (!Number.isSafeInteger(prNumber) || prNumber <= 0) { throw new Error(`Invalid PR number: ${prNumber}`); } if (baseRef.length === 0) { throw new Error("Base ref must not be empty"); } + if (headSha !== undefined && !/^[0-9a-f]{40}$/.test(headSha)) { + throw new Error(`Invalid head SHA: ${headSha}`); + } } export function generatePrDiff(options: GeneratePrDiffOptions): void { const repositoryPath = resolve(options.repositoryPath); const outputPath = resolve(options.outputPath); - validateInputs(options.prNumber, options.baseRef); + validateInputs(options.prNumber, options.baseRef, options.headSha); runGit(repositoryPath, ["check-ref-format", `refs/heads/${options.baseRef}`]); const baseRef = "refs/ai-review/base"; @@ -45,7 +50,7 @@ export function generatePrDiff(options: GeneratePrDiffOptions): void { "--no-tags", "origin", `+refs/heads/${options.baseRef}:${baseRef}`, - `+refs/pull/${options.prNumber}/head:${headRef}`, + `+${options.headSha ?? `refs/pull/${options.prNumber}/head`}:${headRef}`, ]); mkdirSync(dirname(outputPath), { recursive: true }); @@ -78,14 +83,15 @@ export function generatePrDiff(options: GeneratePrDiffOptions): void { } function parseArguments(args: string[]): GeneratePrDiffOptions { - if (args.length !== 2) { - throw new Error("Usage: generate-pr-diff.ts "); + if (args.length < 2 || args.length > 3) { + throw new Error("Usage: generate-pr-diff.ts [head-sha]"); } return { repositoryPath: process.cwd(), prNumber: Number(args[0]), baseRef: args[1] ?? "", outputPath: "/tmp/ai-review/pr.diff", + headSha: args[2] || undefined, }; } diff --git a/.github/scripts/ai-review/post-review.test.ts b/.github/scripts/ai-review/post-review.test.ts index be1442b79e..3f20ecdd1f 100644 --- a/.github/scripts/ai-review/post-review.test.ts +++ b/.github/scripts/ai-review/post-review.test.ts @@ -10,6 +10,7 @@ import { type MarkedEntry, type MergedFinding, type MergedReview, + parseAiReviewState, parseDiffAnchors, partitionFindings, postConsolidatedReview, @@ -535,6 +536,8 @@ describe("renderReviewBody", () => { trigger: "auto", runUrl: "https://example.com/run/9", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; test("shows 'No issues found.' when nothing was posted", () => { @@ -672,6 +675,8 @@ describe("buildReviewPayload", () => { trigger: "manual", runUrl: "https://example.com/run/1", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; test("event is always COMMENT", () => { @@ -686,6 +691,29 @@ describe("buildReviewPayload", () => { expect(payload.body).toContain(AI_REVIEW_MARKER); }); + test("posts on the reviewed head and records every unrefuted finding, inline or not", () => { + const review = makeMergedReview({ + findings: [ + makeFinding({ id: "inline", file: "file.ts", line: 10 }), + makeFinding({ id: "outside", file: "file.ts", line: 99, claim: "x".repeat(70_000) }), + makeFinding({ id: "refuted", adjudication: { verdict: "refuted", reason: "No." } }), + makeFinding({ id: "advisory", category: "next-impact" }), + ], + }); + const payload = buildReviewPayload(review, anchors, { ...footer, reviewsComplete: false }); + + expect(payload.commit_id).toBe(footer.headSha); + expect(payload.comments).toHaveLength(1); + expect(payload.body).toContain("truncated"); + expect(payload.body).not.toContain(footer.modelsFooter); + expect(payload.body).toContain(AI_REVIEW_MARKER); + expect(parseAiReviewState(payload.body)).toEqual({ + head: "a".repeat(40), + complete: false, + findings: 3, + }); + }); + test("the injected models footer appears in the body verbatim", () => { const review = makeMergedReview({ findings: [] }); const payload = buildReviewPayload(review, anchors, footer); @@ -832,6 +860,8 @@ describe("next-impact findings", () => { trigger: "auto", runUrl: "https://example.com/run/2", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; const anchors = parseDiffAnchors(SINGLE_HUNK_DIFF); @@ -883,6 +913,8 @@ describe("foldInlineCommentsIntoBody", () => { trigger: "manual", runUrl: "https://example.com/run/1", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; test("returns the same payload unchanged when there are no inline comments", () => { @@ -1047,6 +1079,8 @@ describe("post flow via injected ReviewIo", () => { trigger: "manual", runUrl: "https://example.com/run/1", modelsFooter: "`claude-fable-5` + `gpt-5.6-sol`", + headSha: "a".repeat(40), + reviewsComplete: true, }; function makeReviewIo( @@ -1208,17 +1242,28 @@ describe("post flow via injected ReviewIo", () => { expect(postedReviews[1]?.body).toContain("Inline comments (GitHub rejected"); }); - test("never retries with the fold when there were no inline comments to fold", async () => { + test("retries a rejected pinned review on the current head, still naming the reviewed commit", async () => { const finding = makeFinding({ file: "file.ts", line: 999 }); // not anchorable -> body-only const review = makeMergedReview({ findings: [finding] }); - const { io, postedReviews } = makeReviewIo({ - postReviewStatuses: [422], - }); + const { io, postedReviews } = makeReviewIo({ postReviewStatuses: [422, 200] }); - await expect(postConsolidatedReview(io, 42, review, SINGLE_HUNK_DIFF, footer)).rejects.toThrow( - /Review POST failed \(status 422\)/, - ); + await postConsolidatedReview(io, 42, review, SINGLE_HUNK_DIFF, footer); + + expect(postedReviews.map((posted) => posted.commit_id)).toEqual([footer.headSha, undefined]); + expect(parseAiReviewState(postedReviews[1]?.body ?? "")?.head).toBe(footer.headSha); + }); + + test("an older workflow's review carries no state, so it never retries a body-only 422", async () => { + const finding = makeFinding({ file: "file.ts", line: 999 }); + const review = makeMergedReview({ findings: [finding] }); + const { io, postedReviews } = makeReviewIo({ postReviewStatuses: [422] }); + const unpinned = { ...footer, headSha: undefined, reviewsComplete: undefined }; + + await expect( + postConsolidatedReview(io, 42, review, SINGLE_HUNK_DIFF, unpinned), + ).rejects.toThrow(/Review POST failed \(status 422\)/); expect(postedReviews).toHaveLength(1); + expect(parseAiReviewState(postedReviews[0]?.body ?? "")).toBeNull(); }); test("throws with GitHub's response body when the retry also 422s, instead of swallowing it", async () => { diff --git a/.github/scripts/ai-review/post-review.ts b/.github/scripts/ai-review/post-review.ts index 6a7d1dbd1e..674ee84f2d 100644 --- a/.github/scripts/ai-review/post-review.ts +++ b/.github/scripts/ai-review/post-review.ts @@ -24,6 +24,29 @@ */ export const AI_REVIEW_MARKER = ""; +const AI_REVIEW_STATE_PATTERN = + /^$/m; + +/** What a posted AI review covered, read from the hidden state line at the top of its body. */ +export interface AiReviewState { + head: string; + /** Whether both the Claude and the Codex review finished. */ + complete: boolean; + /** Findings posted that were not refuted, inline or in the body. */ + findings: number; +} + +export function renderAiReviewState(state: AiReviewState): string { + return ``; +} + +export function parseAiReviewState(body: string): AiReviewState | null { + const match = AI_REVIEW_STATE_PATTERN.exec(body); + if (!match?.[1] || !match[2] || !match[3]) { + return null; + } + return { head: match[1], complete: match[2] === "true", findings: Number(match[3]) }; +} const SUPERSEDED_SUMMARY = "Superseded by a newer AI review"; /** Hidden marker `isSuperseded` checks for. Kept out of the human-readable * `SUPERSEDED_SUMMARY` text, and broken by `sanitizeModelText`, so a model @@ -606,6 +629,9 @@ export interface ReviewFooterInfo { /** e.g. `` `claude-fable-5` + `gpt-5.6-sol` ``. Sourced from * `.github/ai-models.json` through the workflow's model env vars. */ modelsFooter: string; + /** Commit the models reviewed; absent from older workflow copies, whose reviews carry no state and never approve. */ + headSha?: string; + reviewsComplete?: boolean; } /** Renders the full review body: summary, findings table, out-of-diff section, refuted details, stats, and footer. */ @@ -619,7 +645,13 @@ export function renderReviewBody( ); const verdicts = computeVerdictCounts(review.findings); - const sections: string[] = [`## ๐Ÿค– AI Review\n\n${sanitizeModelText(review.summary)}`]; + // The markers lead the body so truncation never drops them. Advisory next-impact findings have no thread, so + // counting them keeps any review that raised one from approving a sync. + const state = footer.headSha + ? `${renderAiReviewState({ head: footer.headSha, complete: footer.reviewsComplete === true, findings: posted.length + review.findings.filter(isAdvisoryNextImpact).length })}\n` + : ""; + const header = `${AI_REVIEW_MARKER}\n${state}`; + const sections: string[] = [`${header}## ๐Ÿค– AI Review\n\n${sanitizeModelText(review.summary)}`]; if (posted.length > 0) { const rows = posted.map( @@ -697,8 +729,6 @@ export function renderReviewBody( `Models: ${footer.modelsFooter} ยท Trigger: \`${footer.trigger}\` ยท [Workflow run](${footer.runUrl})`, "", "This review runs once per PR. A maintainer can request another with a `/ai-review` comment.", - "", - AI_REVIEW_MARKER, ].join("\n"), ); @@ -715,6 +745,7 @@ export interface InlineReviewComment { } export interface ReviewPayload { + commit_id?: string; event: "COMMENT"; body: string; comments: InlineReviewComment[]; @@ -763,7 +794,12 @@ export function buildReviewPayload( const body = renderReviewBody(review, partitioned, footer); // A body-only review has no 422 fold-retry to truncate it later, so // truncate the first payload too rather than posting an oversized body. - return { event: "COMMENT", body: truncateReviewBody(body, footer.runUrl), comments }; + return { + ...(footer.headSha ? { commit_id: footer.headSha } : {}), + event: "COMMENT", + body: truncateReviewBody(body, footer.runUrl), + comments, + }; } /** Folds every inline comment into the review body, for the 422-retry path when GitHub rejects an anchor. */ @@ -903,11 +939,12 @@ export async function postConsolidatedReview( const prior = await listPriorRunsBestEffort(io, prNumber); const result = await io.postReview(prNumber, payload); - if (result.status === 422 && payload.comments.length > 0) { + if (result.status === 422 && (payload.comments.length > 0 || payload.commit_id)) { console.warn( - "Review POST rejected an inline anchor (422); retrying once with comments folded into the body.", + "Review POST rejected (422); retrying once on the current head with comments folded into the body.", ); - const folded = foldInlineCommentsIntoBody(payload); + // A force-push can drop the reviewed commit; the state line still names it, so the review cannot vouch for the new head. + const { commit_id: _rejected, ...folded } = foldInlineCommentsIntoBody(payload); const retryResult = await io.postReview(prNumber, { ...folded, body: truncateReviewBody(folded.body, footer.runUrl), @@ -1138,6 +1175,8 @@ async function runPost(): Promise { const prDiffPath = requireEnv("PR_DIFF_PATH"); const claudeModel = requireEnv("CLAUDE_MODEL"); const codexModel = requireEnv("CODEX_MODEL"); + const headSha = process.env.HEAD_SHA || undefined; + const reviewsComplete = process.env.REVIEWS_COMPLETE === "true"; const raw: unknown = JSON.parse(await Bun.file(mergedReviewPath).text()); assertMergedReview(raw); @@ -1147,6 +1186,8 @@ async function runPost(): Promise { trigger, runUrl, modelsFooter: `\`${claudeModel}\` + \`${codexModel}\``, + headSha, + reviewsComplete, }); console.log(`Posted AI review on PR #${prNumber} (${raw.findings.length} finding(s)).`); } diff --git a/.github/scripts/ai-review/resolve.test.ts b/.github/scripts/ai-review/resolve.test.ts index 07453c1c32..f4d77aea58 100644 --- a/.github/scripts/ai-review/resolve.test.ts +++ b/.github/scripts/ai-review/resolve.test.ts @@ -24,6 +24,7 @@ function makePr(overrides: Partial = {}): PrDetails { authorLogin: PR_AUTHOR, headRepoFullName: REPO, baseRepoFullName: REPO, + headSha: "a".repeat(40), ...overrides, }; } diff --git a/.github/scripts/ai-review/resolve.ts b/.github/scripts/ai-review/resolve.ts index 58df368df1..0cfaafc071 100644 --- a/.github/scripts/ai-review/resolve.ts +++ b/.github/scripts/ai-review/resolve.ts @@ -61,6 +61,8 @@ export interface PrDetails { headRepoFullName: string; /** `owner/name` of the repository the PR targets. */ baseRepoFullName: string; + /** Head commit when the run resolved; every job reviews this commit even if the branch moves. */ + headSha: string; } /** A prior review or issue comment, checked for the dedup marker. */ @@ -85,13 +87,15 @@ export interface ResolveResult { /** Human-readable explanation, present whenever `shouldRun` is false. */ skipReason?: string; trigger: Trigger; + /** Commit every job reviews, present whenever `shouldRun` is true. */ + headSha?: string; } /** No size gate: Claude and Codex review agentically over many turns, so any * PR that clears the draft/bot/fork/dedup checks is reviewed regardless of * size, best-effort within the model's context/turn budget. */ -function decideForPr(trigger: Trigger): ResolveResult { - return { shouldRun: true, trigger }; +function decideForPr(trigger: Trigger, pr: PrDetails): ResolveResult { + return { shouldRun: true, trigger, headSha: pr.headSha }; } /** @@ -156,7 +160,7 @@ export async function resolveDecision(input: ResolveInput, io: ResolveIo): Promi } // A maintainer explicitly asked, so the marker/dedup guard and the // draft/fork/bot skips below don't apply. - return decideForPr(trigger); + return decideForPr(trigger, pr); } if (pr.draft) { @@ -206,7 +210,7 @@ export async function resolveDecision(input: ResolveInput, io: ResolveIo): Promi }; } - return decideForPr(trigger); + return decideForPr(trigger, pr); } function requireEnv(name: string): string { @@ -243,7 +247,7 @@ interface RestPullRequest { state: "open" | "closed"; draft: boolean; user: { login: string; type: string } | null; - head: { repo: { full_name: string } | null }; + head: { sha: string; repo: { full_name: string } | null }; base: { repo: { full_name: string } }; } @@ -274,6 +278,7 @@ function assertRestPullRequest(value: unknown): asserts value is RestPullRequest typeof value.user.type === "string") ) || !isRecordEntry(value.head) || + typeof value.head.sha !== "string" || !( value.head.repo === null || (isRecordEntry(value.head.repo) && typeof value.head.repo.full_name === "string") @@ -314,6 +319,7 @@ async function fetchPullRequest(token: string, base: string, prNumber: number): authorLogin: pr.user?.login ?? "", headRepoFullName: pr.head.repo?.full_name ?? "", baseRepoFullName: pr.base.repo.full_name, + headSha: pr.head.sha, }; } @@ -369,6 +375,7 @@ function writeOutputs(result: ResolveResult, prNumber: number): void { should_run: String(result.shouldRun), pr_number: String(prNumber), head_ref: `refs/pull/${prNumber}/head`, + head_sha: result.headSha ?? "", trigger: result.trigger, }; const lines = Object.entries(entries).map(([name, value]) => { diff --git a/.github/scripts/fast-forward.test.ts b/.github/scripts/fast-forward.test.ts index 008058b89e..c2d152cedc 100644 --- a/.github/scripts/fast-forward.test.ts +++ b/.github/scripts/fast-forward.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { type CheckRun, + AUTO_APPROVER_LOGIN, type FastForwardIo, type GraphqlQuery, type Promotion, @@ -427,6 +428,7 @@ function scenarioIo( const io: FastForwardIo = { git: makeGit(checkout), getPullRequest: async () => pullRequest, + listReviews: async () => [], listCheckRuns: async () => checks, findOpenPullRequest: async (base, head) => { lookups.push(`${base}<-${head}`); @@ -733,6 +735,36 @@ describe("runFastForward sync", () => { expect(() => repo.remoteTip("sync/main-into-develop")).toThrow(); }); + test("lands the sync bot's approval only while it is active and nobody requests changes", async () => { + const review = (login: string, state: string) => ({ user: { login }, state }); + const approved = review(AUTO_APPROVER_LOGIN, "APPROVED"); + const attempt = async (reviews: ReturnType[]) => { + const { repo, head, io } = resolvedSyncScenario(); + const before = repo.remoteTip("develop"); + const outcome = await runFastForward( + { ...io, listReviews: async () => reviews }, + { reviewCommitId: head, approver: AUTO_APPROVER_LOGIN }, + ); + return { status: outcome.status, moved: repo.remoteTip("develop") !== before }; + }; + + expect(await attempt([approved, review("maintainer", "CHANGES_REQUESTED")])).toEqual({ + status: "refused", + moved: false, + }); + expect(await attempt([approved, review(AUTO_APPROVER_LOGIN, "DISMISSED")])).toEqual({ + status: "refused", + moved: false, + }); + expect( + await attempt([ + review("maintainer", "CHANGES_REQUESTED"), + review("maintainer", "APPROVED"), + approved, + ]), + ).toEqual({ status: "fast-forwarded", moved: true }); + }); + test("lands the approved head merged with a target move without pulling in the moved source", async () => { const { repo, head, io, comments } = resolvedSyncScenario(); git(repo.seed, "switch", "main"); diff --git a/.github/scripts/fast-forward.ts b/.github/scripts/fast-forward.ts index 77b9ff82cd..5e1bcf8f6c 100644 --- a/.github/scripts/fast-forward.ts +++ b/.github/scripts/fast-forward.ts @@ -1,3 +1,5 @@ +import { readFileSync } from "node:fs"; + import { analyzeCommits } from "../../apps/cli/scripts/analyze-commits-title.js"; import { type GitRunner, @@ -199,9 +201,36 @@ export type CheckEvaluation = failing: { name: string; conclusion: string }[]; }; +/** Account `Sync repair` approves eligible sync pull requests as. */ +export const AUTO_APPROVER_LOGIN = "supabase-oss"; + +export interface PullRequestReview { + user: { login: string } | null; + state: string; +} + +/** + * Whether anyone's latest approving or blocking review requests changes; comments do not change a verdict. Every + * reviewer counts because `author_association` hides private org membership; a maintainer dismisses a stray one. + */ +export function hasChangesRequested(reviews: PullRequestReview[]): boolean { + return [...latestVerdicts(reviews).values()].includes("CHANGES_REQUESTED"); +} + +function latestVerdicts(reviews: PullRequestReview[]): Map { + const verdicts = new Map(); + for (const review of reviews) { + if (review.user && ["APPROVED", "CHANGES_REQUESTED", "DISMISSED"].includes(review.state)) { + verdicts.set(review.user.login, review.state); + } + } + return verdicts; +} + export interface FastForwardIo { git: GitRunner; getPullRequest(): Promise; + listReviews(): Promise; listCheckRuns(sha: string): Promise; findOpenPullRequest(base: string, head: string): Promise; comment(body: string): Promise; @@ -210,6 +239,8 @@ export interface FastForwardIo { export interface FastForwardInput { /** The commit the approving review was submitted against; a local dry run defaults to the live head. */ reviewCommitId?: string; + /** Login of the approving reviewer. */ + approver?: string; dryRun?: boolean; } @@ -448,6 +479,21 @@ export async function runFastForward( if (pullRequest.draft) { return refuse(io, input, "pull request is a draft"); } + // A change request or a dismissal can land between Sync repair's decision and this run. + if (input.approver === AUTO_APPROVER_LOGIN) { + const reviews = await io.listReviews(); + if ( + hasChangesRequested(reviews) || + latestVerdicts(reviews).get(AUTO_APPROVER_LOGIN) !== "APPROVED" + ) { + return refuse( + io, + input, + "a reviewer requested changes or the approval was dismissed", + `Not fast-forwarding \`${promotion.target}\`: a reviewer requested changes or dismissed \`${AUTO_APPROVER_LOGIN}\`'s approval. A maintainer approves once that is addressed.`, + ); + } + } if (approvedSha !== headSha) { return refuse( io, @@ -567,6 +613,19 @@ function makeIo(token: string, repository: string, prNumber: number): FastForwar git: makeGit(process.cwd()), getPullRequest: () => githubRequest(token, `/repos/${repository}/pulls/${prNumber}`), + async listReviews() { + const reviews: PullRequestReview[] = []; + for (let page = 1; ; page += 1) { + const batch = await githubRequest( + token, + `/repos/${repository}/pulls/${prNumber}/reviews?per_page=100&page=${page}`, + ); + reviews.push(...batch); + if (batch.length < 100) { + return reviews; + } + } + }, listCheckRuns: (sha) => fetchPullRequestCheckRuns( (query, variables) => githubGraphql(token, query, variables), @@ -600,6 +659,15 @@ function parseArguments(args: string[]): { prNumber: number; dryRun: boolean } { return { prNumber, dryRun }; } +function reviewerLogin(event: unknown): string | undefined { + const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null; + const review = isRecord(event) ? event.review : undefined; + const user = isRecord(review) ? review.user : undefined; + const login = isRecord(user) ? user.login : undefined; + return typeof login === "string" ? login : undefined; +} + async function main(): Promise { const { prNumber, dryRun } = parseArguments(process.argv.slice(2)); const repository = process.env.REPOSITORY ?? requireEnv("GITHUB_REPOSITORY"); @@ -608,7 +676,12 @@ async function main(): Promise { if (!reviewCommitId && !dryRun) { throw new Error("REVIEW_COMMIT_ID is required"); } - const outcome = await runFastForward(io, { reviewCommitId, dryRun }); + // Read from the event so a sync PR running an older copy of the workflow still identifies the approver. + const eventPath = process.env.GITHUB_EVENT_PATH; + const approver = eventPath + ? reviewerLogin(JSON.parse(readFileSync(eventPath, "utf8"))) + : undefined; + const outcome = await runFastForward(io, { reviewCommitId, approver, dryRun }); console.log(JSON.stringify(outcome)); if (outcome.status === "refused") { console.error(`::error::${outcome.reason}`); diff --git a/.github/scripts/sync-agent.test.ts b/.github/scripts/sync-agent.test.ts index cb8a51aa18..35fd2fa571 100644 --- a/.github/scripts/sync-agent.test.ts +++ b/.github/scripts/sync-agent.test.ts @@ -37,6 +37,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, failed, run(100, "Require fast-forward", "failure")], threads: [], @@ -50,6 +52,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, cancelled], threads: [], @@ -62,6 +66,8 @@ describe("planRepair", () => { const running = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(99, "CodeQL", null)], threads: [finding], @@ -69,6 +75,8 @@ describe("planRepair", () => { const missing = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: green.slice(1), threads: [finding], @@ -82,6 +90,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [ "@supabase/cli Claude could not resolve this merge: ", ], @@ -99,6 +109,8 @@ describe("planRepair", () => { runs: green, threads: [finding], reviewRunning: false, + aiReview: null, + changesRequested: false, }); expect(plan.action).toBe("skip"); @@ -108,18 +120,22 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(-1, "Check code quality", "failure")], threads: [], }); - expect(plan).toEqual({ action: "skip", reason: "Nothing to repair." }); + expect(plan.action).toBe("skip"); }); test("hands a head a round left failing to a maintainer unless new findings arrive", () => { const state = { head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED, repaired(HEAD)], runs: [...green, run(99, "Check code quality", "failure")], }; @@ -132,6 +148,8 @@ describe("planRepair", () => { const state = { head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, runs: [...green, run(99, "Check code quality", "failure")], threads: [], }; @@ -144,6 +162,8 @@ describe("planRepair", () => { const plan = planRepair({ head: "d".repeat(40), reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED, repaired(HEAD), ""], runs: [...green, run(99, "Check code quality", "failure")], threads: [finding], @@ -156,6 +176,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: true, + aiReview: null, + changesRequested: false, botComments: [RESOLVED], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -168,6 +190,8 @@ describe("planRepair", () => { const plan = planRepair({ head: HEAD, reviewRunning: false, + aiReview: null, + changesRequested: false, botComments: [RESOLVED, ``], runs: [...green, run(99, "Check code quality", "failure")], threads: [], @@ -175,6 +199,29 @@ describe("planRepair", () => { expect(plan.action).toBe("skip"); }); + + test("approves a green head only when both models reviewed it and every finding has an answered thread", () => { + const reviewed = { head: HEAD, complete: true, findings: 2, answeredThreads: 2 }; + const state = { + head: HEAD, + reviewRunning: false, + botComments: [RESOLVED], + runs: [...green, run(100, "Require fast-forward", "failure")], + threads: [], + changesRequested: false, + }; + + expect(planRepair({ ...state, aiReview: reviewed })).toEqual({ action: "approve" }); + for (const plan of [ + planRepair({ ...state, aiReview: reviewed, head: "e".repeat(40) }), + planRepair({ ...state, aiReview: { ...reviewed, complete: false } }), + planRepair({ ...state, aiReview: { ...reviewed, answeredThreads: 1 } }), + planRepair({ ...state, aiReview: reviewed, changesRequested: true }), + planRepair({ ...state, aiReview: null }), + ]) { + expect(plan.action).toBe("skip"); + } + }); }); describe("buildSuggestions", () => { diff --git a/.github/scripts/sync-agent.ts b/.github/scripts/sync-agent.ts index b5427aa171..3d8f7c2ee8 100644 --- a/.github/scripts/sync-agent.ts +++ b/.github/scripts/sync-agent.ts @@ -6,8 +6,19 @@ import { spawnSync } from "node:child_process"; import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { AI_REVIEW_MARKER } from "./ai-review/post-review.ts"; -import { type CheckRun, fetchPullRequestCheckRuns, REQUIRED_CHECKS } from "./fast-forward.ts"; +import { + AI_REVIEW_MARKER, + type AiReviewState, + isSuperseded, + parseAiReviewState, +} from "./ai-review/post-review.ts"; +import { + type CheckRun, + fetchPullRequestCheckRuns, + hasChangesRequested, + type PullRequestReview, + REQUIRED_CHECKS, +} from "./fast-forward.ts"; import { githubGraphql, githubRequest, @@ -50,6 +61,7 @@ export interface ReviewThread { export type RepairPlan = | { action: "skip"; reason: string } | { action: "exhausted" } + | { action: "approve" } | { action: "repair"; failures: CheckRun[]; threads: ReviewThread[] }; function repairMarker(head: string): string { @@ -93,6 +105,10 @@ export function planRepair(state: { runs: CheckRun[]; threads: ReviewThread[]; reviewRunning: boolean; + /** The latest AI review, with how many of its inline threads are resolved or answered by the release bot. */ + aiReview: (AiReviewState & { answeredThreads: number }) | null; + /** Whether any reviewer's latest verdict is "Request changes". */ + changesRequested: boolean; }): RepairPlan { const { head, botComments, threads } = state; if (!botComments.some((body) => body.startsWith(RESOLVE_MARKER))) { @@ -121,7 +137,7 @@ export function planRepair(state: { (run) => !["success", "neutral", "skipped"].includes(run.conclusion ?? ""), ); if (threads.length === 0 && failures.length === 0) { - return { action: "skip", reason: "Nothing to repair." }; + return planApproval(head, state.aiReview, state.changesRequested); } const rounds = botComments.filter((body) => body.startsWith(REPAIR_MARKER)).length; const headRepaired = botComments.some((body) => body.startsWith(repairMarker(head))); @@ -131,6 +147,33 @@ export function planRepair(state: { return { action: "repair", failures, threads }; } +/** + * Approves only a head both models reviewed, where every finding they did not refute has a thread Claude fixed or + * declined. Findings only in the review body have no thread, so they wait for a maintainer. + */ +function planApproval( + head: string, + review: (AiReviewState & { answeredThreads: number }) | null, + changesRequested: boolean, +): RepairPlan { + if (changesRequested) { + return { action: "skip", reason: "A reviewer requested changes." }; + } + if (review?.head !== head) { + return { action: "skip", reason: "Nothing to repair; the AI review did not see this head." }; + } + if (!review.complete) { + return { action: "skip", reason: "Only one model finished the AI review." }; + } + if (review.answeredThreads < review.findings) { + return { + action: "skip", + reason: "AI review findings without an answered thread need a maintainer.", + }; + } + return { action: "approve" }; +} + /** Lines that still carry a conflict marker, as `path:line`. */ export function leftoverConflictMarkers(workDir: string, paths: string[]): string[] { return paths.flatMap((path) => { @@ -522,7 +565,7 @@ query($owner: String!, $name: String!, $number: Int!) { id isResolved comments(first: 50) { - nodes { databaseId body path line author { login } pullRequestReview { body } } + nodes { databaseId body path line author { login } pullRequestReview { databaseId body } } } } } @@ -544,7 +587,7 @@ interface ThreadsPage { path: string; line: number | null; author: { login: string } | null; - pullRequestReview: { body: string } | null; + pullRequestReview: { databaseId: number; body: string } | null; }[]; }; }[]; @@ -553,32 +596,38 @@ interface ThreadsPage { } | null; } -/** Unresolved AI review threads the release bot has not answered; nobody else's comments reach the agent. */ -async function fetchOpenFindings( +/** + * AI review threads, answered when resolved or replied to by the release bot. Only the AI review's own comments are + * read, so nobody else's text reaches the agent. + */ +async function fetchAiReviewThreads( token: string, repository: string, number: number, -): Promise { +): Promise<{ thread: ReviewThread; reviewId: number; answered: boolean }[]> { const [owner, name] = repository.split("/"); const page = await githubGraphql(token, THREADS_QUERY, { owner, name, number }); return (page.repository?.pullRequest?.reviewThreads.nodes ?? []).flatMap((thread) => { const [first, ...rest] = thread.comments.nodes; if ( - thread.isResolved || - !first || - !isLogin(first.author?.login, AI_REVIEW_LOGIN) || - !first.pullRequestReview?.body.includes(AI_REVIEW_MARKER) || - rest.some((comment) => isLogin(comment.author?.login, RELEASE_BOT_LOGIN)) + !first?.pullRequestReview?.body.includes(AI_REVIEW_MARKER) || + !isLogin(first.author?.login, AI_REVIEW_LOGIN) ) { return []; } return [ { - threadId: thread.id, - commentId: first.databaseId, - path: first.path, - line: first.line, - body: first.body, + thread: { + threadId: thread.id, + commentId: first.databaseId, + path: first.path, + line: first.line, + body: first.body, + }, + reviewId: first.pullRequestReview.databaseId, + answered: + thread.isResolved || + rest.some((comment) => isLogin(comment.author?.login, RELEASE_BOT_LOGIN)), }, ]; }); @@ -691,11 +740,20 @@ async function collect(outputDir: string): Promise { token, `/repos/${repository}/issues/${pull.number}/comments`, )), - ...(await listAll<{ user: { login: string } | null; body: string }>( - token, - `/repos/${repository}/pulls/${pull.number}/reviews`, - )), ]; + const reviews = await listAll( + token, + `/repos/${repository}/pulls/${pull.number}/reviews`, + ); + comments.push(...reviews); + const aiReview = reviews.findLast( + (review) => + isLogin(review.user?.login, AI_REVIEW_LOGIN) && + review.body.includes(AI_REVIEW_MARKER) && + !isSuperseded(review.body), + ); + const aiReviewState = aiReview ? parseAiReviewState(aiReview.body) : null; + const aiThreads = await fetchAiReviewThreads(token, repository, pull.number); const plan = planRepair({ head: pull.head.sha, botComments: comments @@ -707,15 +765,25 @@ async function collect(outputDir: string): Promise { pull.number, pull.head.sha, ), - threads: await fetchOpenFindings(token, repository, pull.number), + threads: aiThreads.filter((entry) => !entry.answered).map((entry) => entry.thread), reviewRunning: await isReviewRunning(token, repository, pull.number), + aiReview: + aiReview && aiReviewState + ? { + ...aiReviewState, + answeredThreads: aiThreads.filter( + (entry) => entry.reviewId === aiReview.id && entry.answered, + ).length, + } + : null, + changesRequested: hasChangesRequested(reviews), }); setOutput("action", plan.action); setOutput("pr", String(pull.number)); setOutput("head", pull.head.sha); - if (plan.action === "skip") { - console.log(plan.reason); + if (plan.action === "skip" || plan.action === "approve") { + console.log(plan.action === "skip" ? plan.reason : "Ready for approval."); return; } if (plan.action === "exhausted") { diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index 5140c4142b..582874175f 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -66,6 +66,8 @@ jobs: should_run: ${{ steps.resolve.outputs.should_run }} pr_number: ${{ steps.resolve.outputs.pr_number }} head_ref: ${{ steps.resolve.outputs.head_ref }} + # Every job reviews this commit, so a push during the run is never reported as reviewed. + head_sha: ${{ steps.resolve.outputs.head_sha }} trigger: ${{ steps.resolve.outputs.trigger }} # Read once here so every job in the run uses the same models, even if the file changes mid-run. claude_model: ${{ steps.models.outputs.claude_model }} @@ -116,7 +118,7 @@ jobs: - name: Checkout PR head (untrusted; review subject matter only) uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ needs.resolve.outputs.head_ref }} + ref: ${{ needs.resolve.outputs.head_sha }} path: pr fetch-depth: 1 persist-credentials: false @@ -143,13 +145,14 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} run: | mkdir -p /tmp/ai-review gh pr view "$PR" --repo "$GITHUB_REPOSITORY" \ --json number,title,body,baseRefName,headRefName,additions,deletions,changedFiles \ > /tmp/ai-review/pr.json base_ref=$(jq -r '.baseRefName' /tmp/ai-review/pr.json) - bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" "$HEAD_SHA" if [ -f .github/scripts/ai-review/generate-next-context.ts ]; then bun .github/scripts/ai-review/generate-next-context.ts "$base_ref" fi @@ -287,10 +290,11 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} run: | mkdir -p /tmp/ai-review base_ref=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') - bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" "$HEAD_SHA" if [ -f .github/scripts/ai-review/generate-next-context.ts ]; then bun .github/scripts/ai-review/generate-next-context.ts "$base_ref" fi @@ -381,7 +385,7 @@ jobs: - name: Checkout PR head (untrusted; read-only, for verify-by-reading) uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ needs.resolve.outputs.head_ref }} + ref: ${{ needs.resolve.outputs.head_sha }} path: pr fetch-depth: 1 persist-credentials: false @@ -442,10 +446,11 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR: ${{ needs.resolve.outputs.pr_number }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} run: | mkdir -p /tmp/ai-review base_ref=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName --jq '.baseRefName') - bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" + bun .github/scripts/ai-review/generate-pr-diff.ts "$PR" "$base_ref" "$HEAD_SHA" if [ -f .github/scripts/ai-review/generate-next-context.ts ]; then bun .github/scripts/ai-review/generate-next-context.ts "$base_ref" fi @@ -522,6 +527,8 @@ jobs: name: Post review needs: - resolve + - claude-review + - codex-review - adjudicate # Runs only when adjudication succeeded (it produced the merged review this job posts). # `!cancelled()` is required since an explicit `if` replaces the default "all needed jobs @@ -571,6 +578,8 @@ jobs: MERGED_REVIEW_PATH: /tmp/ai-review/merged-review.json PR_DIFF_PATH: /tmp/ai-review/pr.diff TRIGGER: ${{ needs.resolve.outputs.trigger }} + HEAD_SHA: ${{ needs.resolve.outputs.head_sha }} + REVIEWS_COMPLETE: ${{ needs.claude-review.result == 'success' && needs.codex-review.result == 'success' }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} # CLAUDE_MODEL / CODEX_MODEL are the job-level values from `resolve`, the same ones the # review jobs ran with, so the footer never drifts from what actually ran. diff --git a/.github/workflows/sync-repair.yml b/.github/workflows/sync-repair.yml index 54d3030fb5..80e8ed7e6d 100644 --- a/.github/workflows/sync-repair.yml +++ b/.github/workflows/sync-repair.yml @@ -40,6 +40,10 @@ jobs: pull-requests: read checks: read actions: read + outputs: + action: ${{ steps.collect.outputs.action }} + pr: ${{ steps.collect.outputs.pr }} + head: ${{ steps.collect.outputs.head }} env: OUTPUT_DIR: ${{ github.workspace }}/sync-agent RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} @@ -126,6 +130,11 @@ jobs: --force-with-lease="refs/heads/sync/develop-into-next:$HEAD_SHA" \ "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" HEAD:refs/heads/sync/develop-into-next fi + # The workflow suggestions hold the head before any reply counts a withheld fix as answered. + if [ "$CHANGED" != "" ]; then + (cd trusted && REPO_DIR="$GITHUB_WORKSPACE/work" HEAD_SHA="$(git -C "$GITHUB_WORKSPACE/work" rev-parse HEAD)" \ + bun .github/scripts/sync-agent.ts suggest) + fi if [ -f "$OUTPUT_DIR/replies.json" ]; then jq -c '.[]' "$OUTPUT_DIR/replies.json" | while read -r reply; do gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/comments/$(jq -r .commentId <<<"$reply")/replies" \ @@ -137,8 +146,38 @@ jobs: fi done fi - if [ "$CHANGED" != "" ]; then - cd trusted - REPO_DIR="$GITHUB_WORKSPACE/work" HEAD_SHA="$(git -C "$GITHUB_WORKSPACE/work" rev-parse HEAD)" \ - bun .github/scripts/sync-agent.ts suggest + + # A green head both AI review models saw, with every finding fixed or declined and no change request. + # The approval runs fast-forward.yml, which re-checks the head and its checks before moving `next`. + approve: + name: Approve the reviewed sync + needs: repair + if: needs.repair.outputs.action == 'approve' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: {} + # Holds `AUTO_APPROVE_PR_PAT`; the environment only deploys from the default branch. + environment: auto-approve + steps: + - name: Approve + env: + GH_TOKEN: ${{ secrets.AUTO_APPROVE_PR_PAT }} + PR: ${{ needs.repair.outputs.pr }} + HEAD_SHA: ${{ needs.repair.outputs.head }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + if [ "$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json headRefOid -q .headRefOid)" != "$HEAD_SHA" ]; then + echo "::notice::The sync branch moved since the checks settled; skipping." + exit 0 + fi + login="$(gh api user --jq .login)" + # A dismissed approval on this head means a maintainer took it back, so it is never posted again. + previous="$(gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" \ + --jq ".[] | select(.user.login == \"$login\" and .commit_id == \"$HEAD_SHA\" and (.state == \"APPROVED\" or .state == \"DISMISSED\")) | .state")" + if [ -n "$previous" ]; then + echo "::notice::${login} already approved ${HEAD_SHA} (${previous//$'\n'/, })." + exit 0 fi + gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/reviews" -f commit_id="$HEAD_SHA" -f event=APPROVE \ + -f body="CI is green and the AI review raised nothing the merge introduced, so this approves Claude's resolution of the sync (${RUN_URL})." > /dev/null diff --git a/apps/cli/docs/release-process.md b/apps/cli/docs/release-process.md index 1e5e115a59..96fa0b001f 100644 --- a/apps/cli/docs/release-process.md +++ b/apps/cli/docs/release-process.md @@ -354,11 +354,11 @@ git push origin sync/develop-into-next A `develop-into-next` conflict PR is resolved by Claude (the `claude.opus` model in `.github/ai-models.json`) in the same run. The `Resolve conflicts with Claude` job merges `next` into the sync branch and runs Claude Code with file read, search, and edit tools only: no shell, no web tools, no `.git` edits, and no GitHub write token. A `pnpm-lock.yaml` conflict keeps `next`'s lockfile, which the next job regenerates. `Run quick checks on the resolution` formats the merged tree and runs `check:all` and unit tests in a job with no secrets, and `Fix the quick checks with Claude` fixes what fails once, without re-running them. This catches code on one side that still uses something the other renamed or removed; integration and end-to-end failures are left to the PR's CI and the repair loop below. The jobs holding the Anthropic key never execute the merged tree, and the tree passes between jobs as a git bundle. The `Publish the resolution` job pushes the latest finished stage from a fresh checkout with the release App token, comments the resolution and any check that failed, and starts the AI review. Choices between `develop` and `next` behavior are listed as decisions for `@supabase/cli`. If Claude cannot resolve the merge, it comments why and the PR waits for a person as above. -[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. +[`sync-repair.yml`](../../../.github/workflows/sync-repair.yml) (`Sync repair`) runs when a CI workflow or the AI review finishes, and hourly. Once every check on the PR head has finished and no `AI Review` run for the PR is still going, it gives Claude the failed jobs' log tails and the AI review's unanswered threads. Claude fixes what the merge broke and replies on each finding, declining findings the merge did not introduce. The fix is pushed as a `chore(repo)` commit, fixed threads are resolved, and a round comment is posted. A head gets another round only for new review findings. When a round leaves its head failing, or after two rounds, the bot asks `@supabase/cli` to take over and stops repairing that PR. When every check except `Require fast-forward` is green on a head both AI review models finished reviewing, and every finding they did not refute has a thread Claude fixed or declined, `supabase-oss` approves that head, which fast-forwards `next`. A head moved by a repair commit or a maintainer push is not re-reviewed, and findings outside the diff have no thread, so both wait for a maintainer's approval. To hold a sync, mark the PR as a draft or request changes (any reviewer's change request holds it, so dismiss a stray one); dismissing `supabase-oss`'s approval also stops it approving that head again. `Fast-forward on approval` re-checks change requests before moving `next` on `supabase-oss`'s approval. When it refuses, `supabase-oss` does not approve that head again, so a maintainer approves. The repair loop reads only CI output and the AI review's own threads, never other people's comments, and only acts on PRs Claude resolved. -Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land only through a maintainer's approval like any sync PR. +Workflows on the sync branch run with repository secrets, so Claude never pushes a change under `.github/`. Claude resolves and fixes workflow files like any other, but those edits are posted as a review of suggestions, with a diff for file additions and deletions; a conflicted workflow keeps `develop`'s side until then. A maintainer reviews them and uses **Commit suggestions**, so the workflow change runs only once a person commits it, and repair waits for that. The publish job refuses to push when the merge's `.github/` differs from git's own merge (with `develop`'s side on conflicts) or a later commit touches `.github/`, and the repair token has no `workflows` permission. Claude's other edits run in the sync PR's CI before anyone reviews them, and land through a maintainer's approval or `Sync repair`'s approval under the conditions above. -The PR is authored by the release App, so the person who resolved it can approve it. This is an accepted decision: sync resolvers are trusted maintainers with write access, and the resolution content is not independently reviewed. `bot-pr-auto-merge.yml` excludes both sync heads so the bot never approves or auto-merges them. **Approving the PR fast-forwards the target to the PR head** and deletes the sync branch; do not use the merge button (the repo is squash-only and squashing destroys the merge ancestry the sync relies on). The `Require fast-forward` check fails on these PRs to enforce that. +The PR is authored by the release App, so the person who resolved it can approve it. This is an accepted decision: sync resolvers are trusted maintainers with write access, and the resolution content is not independently reviewed. `bot-pr-auto-merge.yml` excludes both sync heads so the bot never approves or auto-merges them; only `Sync repair` approves, on the conditions above. This is an accepted decision: Claude's resolution and its own judgement of which findings the merge introduced reach `next` without a person reviewing them. **Approving the PR fast-forwards the target to the PR head** and deletes the sync branch; do not use the merge button (the repo is squash-only and squashing destroys the merge ancestry the sync relies on). The `Require fast-forward` check fails on these PRs to enforce that. #### Fast-forward on approval diff --git a/apps/cli/src/command-internal/test-db.integration.test.ts b/apps/cli/src/command-internal/test-db.integration.test.ts index 8b6ccaae37..9572227a20 100644 --- a/apps/cli/src/command-internal/test-db.integration.test.ts +++ b/apps/cli/src/command-internal/test-db.integration.test.ts @@ -3,13 +3,32 @@ import { join } from "node:path"; import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { ConfigProvider, Effect, Exit, Layer, Option } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { mockOutput } from "../../tests/helpers/mocks.ts"; import { + VALID_REF, + VALID_TOKEN, mockCommandSettings, mockTelemetryStateTracked, useTempWorkdir, } from "../../tests/helpers/command-mocks.ts"; +import { assertPermissionDrift } from "../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../tests/helpers/permission-real-layers.ts"; import { cliConfigValuesTestLayer } from "../../tests/helpers/config-values-layer.ts"; import { CliArgs } from "../shared/cli/cli-args.service.ts"; import { DebugFlag, DnsResolverFlag, NetworkIdFlag } from "./global-flags.ts"; @@ -249,6 +268,8 @@ interface SetupOpts { dockerInstalled?: boolean; /** Raw CLI args for `CliArgs` โ€” drives DB target selection (Changed-based). */ args?: ReadonlyArray; + /** Runs the real `DbConfigResolver` against `workdir`. */ + realLayers?: boolean; } function setup(opts: SetupOpts = {}) { @@ -261,7 +282,13 @@ function setup(opts: SetupOpts = {}) { const layer = Layer.mergeAll( cliConfigValuesTestLayer, out.layer, - resolver.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir: opts.workdir ?? "/work/project", + dbConnection: connection.layer, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver.layer, connection.layer, docker.layer, bundled.layer, @@ -759,4 +786,66 @@ describe("test db integration", () => { }); }).pipe(Effect.provide(layer)); }); + + it.effect("mints a login role through the real resolver for --linked --project-ref", () => { + const { layer, connection } = setup({ + args: ["test", "db", "--linked", "--project-ref", VALID_REF], + realLayers: true, + workdir: tempWorkdir.current, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* testDb(flags({ linked: true, local: false, projectRef: Option.some(VALID_REF) })); + expect(connection.connectCalls[0]?.isLocal).toBe(false); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + assertPermissionDrift({ + command: "test db", + activeFlags: { linked: true, "project-ref": VALID_REF }, + requests: asRecordedRequests(requests), + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("makes no Management API request for the default local target", () => { + const { layer } = setup({ + args: ["test", "db"], + realLayers: true, + workdir: tempWorkdir.current, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* testDb(flags()); + expect(requests).toEqual([]); + assertPermissionDrift({ + command: "test db", + requests: asRecordedRequests(requests), + exact: true, + }); + expect(() => + assertPermissionDrift({ + command: "test db", + requests: [ + { + method: "POST", + url: `https://api.supabase.com/v1/projects/${VALID_REF}/cli/login-role`, + }, + ], + }), + ).toThrow(/condition doesn't hold/); + }).pipe( + Effect.scoped, + Effect.provide(layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); diff --git a/apps/cli/src/commands/inspect/db/SIDE_EFFECTS.md b/apps/cli/src/commands/inspect/db/SIDE_EFFECTS.md index 2012ba5ae4..dc4efcbd4d 100644 --- a/apps/cli/src/commands/inspect/db/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/inspect/db/SIDE_EFFECTS.md @@ -27,12 +27,12 @@ no new config reads. ## API Routes (MAY fire on `--linked`, inside the resolver) -| Method | Path | Auth | When | -| -------- | ----------------------------------------- | ------------ | --------------------------------------- | -| `POST` | `/v1/projects/{ref}/login-role` | Bearer token | `--linked`, to create a temp login role | -| `GET` | pooler config endpoint | Bearer token | `--linked`, pooler fallback | -| `GET` | `/v1/projects/{ref}/network-bans` | Bearer token | `--linked`, on pooler retry | -| `DELETE` | `/v1/projects/{ref}/network-bans` (unban) | Bearer token | `--linked`, when a self-ban is detected | +| Method | Path | Auth | When | +| -------- | ------------------------------------------- | ------------ | ------------------------------------------------------------------------------------ | +| `POST` | `/v1/projects/{ref}/cli/login-role` | Bearer token | `--linked` with no database password, to create a temp login role | +| `GET` | `/v1/projects/{ref}/config/database/pooler` | Bearer token | `--linked` with `--project-ref`, direct host unreachable, no usable saved pooler URL | +| `POST` | `/v1/projects/{ref}/network-bans/retrieve` | Bearer token | `--linked`, from the 3rd failed temp-role connect (list bans) | +| `DELETE` | `/v1/projects/{ref}/network-bans` | Bearer token | `--linked`, only when bans are listed (clear a self-ban) | ## Environment Variables diff --git a/apps/cli/src/commands/inspect/db/bloat/bloat.command.ts b/apps/cli/src/commands/inspect/db/bloat/bloat.command.ts index ae3bfdeeb6..d4654ef10a 100644 --- a/apps/cli/src/commands/inspect/db/bloat/bloat.command.ts +++ b/apps/cli/src/commands/inspect/db/bloat/bloat.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbBloat } from "./bloat.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbBloatCommand = Command.make("bloat", INSPECT_DB_FLAGS).pipe( Command.withDescription("Estimates space allocated to a relation that is full of dead tuples."), Command.withShortDescription("Show relation bloat"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbBloat)), Command.provide(inspectDbRuntimeLayer("bloat")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/blocking/blocking.command.ts b/apps/cli/src/commands/inspect/db/blocking/blocking.command.ts index 41a70d31e6..88ab264c32 100644 --- a/apps/cli/src/commands/inspect/db/blocking/blocking.command.ts +++ b/apps/cli/src/commands/inspect/db/blocking/blocking.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbBlocking } from "./blocking.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbBlockingCommand = Command.make("blocking", INSPECT_DB_FLAG "Show queries that are holding locks and the queries that are waiting for them to be released.", ), Command.withShortDescription("Show blocking queries"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbBlocking)), Command.provide(inspectDbRuntimeLayer("blocking")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/cache-hit/cache-hit.command.ts b/apps/cli/src/commands/inspect/db/cache-hit/cache-hit.command.ts index 8da34f98fa..3acae05f79 100644 --- a/apps/cli/src/commands/inspect/db/cache-hit/cache-hit.command.ts +++ b/apps/cli/src/commands/inspect/db/cache-hit/cache-hit.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbCacheHit } from "./cache-hit.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbCacheHitCommand = Command.make("cache-hit", INSPECT_DB_FLA 'Show cache hit rates for tables and indices. Deprecated: use "db-stats" instead.', ), Command.withShortDescription("Show cache hit rates (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbCacheHit)), Command.provide(inspectDbRuntimeLayer("cache-hit")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/calls/calls.command.ts b/apps/cli/src/commands/inspect/db/calls/calls.command.ts index 5d34ef0770..4de81e2e93 100644 --- a/apps/cli/src/commands/inspect/db/calls/calls.command.ts +++ b/apps/cli/src/commands/inspect/db/calls/calls.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbCalls } from "./calls.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbCallsCommand = Command.make("calls", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show queries from pg_stat_statements ordered by total times called."), Command.withShortDescription("Show queries by call count"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbCalls)), Command.provide(inspectDbRuntimeLayer("calls")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/db-stats/db-stats.command.ts b/apps/cli/src/commands/inspect/db/db-stats/db-stats.command.ts index ffd2e726db..f5d6451edc 100644 --- a/apps/cli/src/commands/inspect/db/db-stats/db-stats.command.ts +++ b/apps/cli/src/commands/inspect/db/db-stats/db-stats.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbDbStats } from "./db-stats.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbDbStatsCommand = Command.make("db-stats", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show stats such as cache hit rates, total sizes, and WAL size."), Command.withShortDescription("Show database stats"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbDbStats)), Command.provide(inspectDbRuntimeLayer("db-stats")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/index-sizes/index-sizes.command.ts b/apps/cli/src/commands/inspect/db/index-sizes/index-sizes.command.ts index 39ec9ab979..4c212eae17 100644 --- a/apps/cli/src/commands/inspect/db/index-sizes/index-sizes.command.ts +++ b/apps/cli/src/commands/inspect/db/index-sizes/index-sizes.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbIndexSizes } from "./index-sizes.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbIndexSizesCommand = Command.make("index-sizes", INSPECT_DB 'Show index sizes of individual indexes. Deprecated: use "index-stats" instead.', ), Command.withShortDescription("Show individual index sizes (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbIndexSizes)), Command.provide(inspectDbRuntimeLayer("index-sizes")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/index-stats/index-stats.command.ts b/apps/cli/src/commands/inspect/db/index-stats/index-stats.command.ts index c6bd88942b..952cea8bac 100644 --- a/apps/cli/src/commands/inspect/db/index-stats/index-stats.command.ts +++ b/apps/cli/src/commands/inspect/db/index-stats/index-stats.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbIndexStats } from "./index-stats.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbIndexStatsCommand = Command.make("index-stats", INSPECT_DB "Show combined index size, usage percent, scan counts, and unused status.", ), Command.withShortDescription("Show index stats"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbIndexStats)), Command.provide(inspectDbRuntimeLayer("index-stats")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/index-usage/index-usage.command.ts b/apps/cli/src/commands/inspect/db/index-usage/index-usage.command.ts index 7817e0127a..e0c683be81 100644 --- a/apps/cli/src/commands/inspect/db/index-usage/index-usage.command.ts +++ b/apps/cli/src/commands/inspect/db/index-usage/index-usage.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbIndexUsage } from "./index-usage.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbIndexUsageCommand = Command.make("index-usage", INSPECT_DB 'Show information about the efficiency of indexes. Deprecated: use "index-stats" instead.', ), Command.withShortDescription("Show index efficiency (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbIndexUsage)), Command.provide(inspectDbRuntimeLayer("index-usage")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/inspect-db-command.ts b/apps/cli/src/commands/inspect/db/inspect-db-command.ts index 8f7ad3526e..d3f50334ff 100644 --- a/apps/cli/src/commands/inspect/db/inspect-db-command.ts +++ b/apps/cli/src/commands/inspect/db/inspect-db-command.ts @@ -1,6 +1,8 @@ import { Effect } from "effect"; import { Flag } from "effect/cli"; +import { withDbTarget } from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import type { InspectConnectionFlags } from "./inspect-query.ts"; @@ -31,6 +33,9 @@ export const INSPECT_DB_FLAGS = { ), } as const; +/** The Management API calls every `inspect db` subcommand makes, all through the database-config resolver. */ +export const INSPECT_DB_PERMISSIONS = withDbTarget(dbConfig, { default: "linked" }); + /** * Wraps an `inspect db` handler with the standard command-level pipeline: telemetry * instrumentation with the three connection flags, and the machine-format JSON error envelope. diff --git a/apps/cli/src/commands/inspect/db/inspect-query.integration.test.ts b/apps/cli/src/commands/inspect/db/inspect-query.integration.test.ts index ac87b12ab7..9b02020273 100644 --- a/apps/cli/src/commands/inspect/db/inspect-query.integration.test.ts +++ b/apps/cli/src/commands/inspect/db/inspect-query.integration.test.ts @@ -1,8 +1,32 @@ +import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; -import { Cause, Effect, Exit, Layer, Option } from "effect"; +import { Cause, Effect, Exit, FileSystem, Layer, Option } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { mockOutput } from "../../../../tests/helpers/mocks.ts"; -import { mockTelemetryStateTracked } from "../../../../tests/helpers/command-mocks.ts"; +import { commandTreeFor } from "../../../../tests/helpers/command-flags.ts"; +import { + VALID_REF, + VALID_TOKEN, + mockTelemetryStateTracked, + useTempWorkdir, +} from "../../../../tests/helpers/command-mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { DnsResolverFlag } from "../../../command-internal/global-flags.ts"; import { renderGlamourTable } from "../../../output/glamour-table.ts"; @@ -141,6 +165,8 @@ interface SetupOpts { dnsResolver?: "native" | "https"; /** Raw CLI args slice; drives explicitly-passed flag detection. */ cliArgs?: ReadonlyArray; + /** Runs the real `DbConfigResolver` against the temp `workdir`. */ + realLayers?: { readonly workdir: string }; } function setup(opts: SetupOpts = {}) { @@ -158,7 +184,13 @@ function setup(opts: SetupOpts = {}) { const telemetry = mockTelemetryStateTracked(); const layer = Layer.mergeAll( out.layer, - resolver.layer, + opts.realLayers === undefined + ? resolver.layer + : realDbConfigResolver({ + workdir: opts.realLayers.workdir, + dbConnection: connection.layer, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }), connection.layer, telemetry.layer, Layer.succeed(DnsResolverFlag, opts.dnsResolver ?? "native"), @@ -440,3 +472,170 @@ describe("inspect db query runner", () => { }).pipe(Effect.provide(layer)); }); }); + +describe("inspect db through the real database-config resolver", () => { + const tempRoot = useTempWorkdir("supabase-inspect-db-int-"); + const poolerRequest = `GET /v1/projects/${VALID_REF}/config/database/pooler`; + const loginRoleRequest = `POST /v1/projects/${VALID_REF}/cli/login-role`; + const asLines = (requests: ReadonlyArray) => + requests.map(({ method, path }) => `${method} ${path}`); + + const INSPECT_DB_LEAVES = [...commandTreeFor(undefined)] + .filter(([path, node]) => node.isLeaf && path.startsWith("inspect db ")) + .map(([path]) => path); + // Removed commands fail before any Management API call, so they never reach the shared runner. + const REMOVED_INSPECT_DB_LEAVES = [ + "inspect db table-index-sizes", + "inspect db total-index-size", + "inspect db total-table-sizes", + ]; + const RUNNER_INSPECT_DB_LEAVES = INSPECT_DB_LEAVES.filter( + (command) => !REMOVED_INSPECT_DB_LEAVES.includes(command), + ); + + it.effect( + "every inspect db subcommand declares the calls the shared runner makes on the linked project", + () => { + const { layer, out, connection } = setup({ + rows: [DB_STATS_ROW], + cliArgs: ["inspect", "db", "db-stats", "--project-ref", VALID_REF], + realLayers: { workdir: tempRoot.current }, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* inspectDbDbStats(flags({ projectRef: Option.some(VALID_REF) })); + expect(asLines(requests)).toEqual([poolerRequest, loginRoleRequest]); + expect(connection.connectCalls[0]?.cfg.host).toBe("aws-0-us-east-1.pooler.supabase.com"); + expect(out.stdoutText).toContain("8192 kB"); + expect(INSPECT_DB_LEAVES).toHaveLength(25); + expect(INSPECT_DB_LEAVES).toEqual(expect.arrayContaining(REMOVED_INSPECT_DB_LEAVES)); + for (const command of RUNNER_INSPECT_DB_LEAVES) { + assertPermissionDrift({ + command, + activeFlags: { "project-ref": VALID_REF }, + requests: asRecordedRequests(requests), + exact: true, + }); + } + for (const command of REMOVED_INSPECT_DB_LEAVES) { + assertPermissionDrift({ + command, + activeFlags: { "project-ref": VALID_REF }, + requests: [], + exact: true, + }); + } + }).pipe( + Effect.scoped, + Effect.provide(Layer.mergeAll(layer, BunServices.layer)), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }, + ); + + it.effect("--linked=false still reaches the linked project through the real resolver", () => { + const { layer } = setup({ + rows: [DB_STATS_ROW], + cliArgs: ["inspect", "db", "db-stats", "--linked=false", "--project-ref", VALID_REF], + realLayers: { workdir: tempRoot.current }, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* inspectDbDbStats(flags({ projectRef: Option.some(VALID_REF) })); + expect(asLines(requests)).toEqual([poolerRequest, loginRoleRequest]); + assertPermissionDrift({ + command: "inspect db db-stats", + activeFlags: { linked: false, "project-ref": VALID_REF }, + requests: asRecordedRequests(requests), + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(Layer.mergeAll(layer, BunServices.layer)), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect( + "an empty --project-ref falls back to the saved ref and still fetches the pooler", + () => { + const { layer } = setup({ + rows: [DB_STATS_ROW], + cliArgs: ["inspect", "db", "db-stats", "--project-ref", ""], + realLayers: { workdir: tempRoot.current }, + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* fs.makeDirectory(`${tempRoot.current}/supabase/.temp`, { recursive: true }); + yield* fs.writeFileString(`${tempRoot.current}/supabase/.temp/project-ref`, VALID_REF); + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* inspectDbDbStats(flags({ projectRef: Option.some("") })); + expect(asLines(requests)).toEqual([poolerRequest, loginRoleRequest]); + assertPermissionDrift({ + command: "inspect db db-stats", + activeFlags: { "project-ref": "" }, + requests: asRecordedRequests(requests), + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(Layer.mergeAll(layer, BunServices.layer)), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }, + ); + + it.effect("makes no Management API request for --local", () => { + const { layer, connection } = setup({ + rows: [DB_STATS_ROW], + cliArgs: ["inspect", "db", "db-stats", "--local"], + realLayers: { workdir: tempRoot.current }, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* inspectDbDbStats(flags({ local: true })); + expect(connection.connectCalls).toHaveLength(1); + expect(requests).toEqual([]); + assertPermissionDrift({ + command: "inspect db db-stats", + activeFlags: { local: true }, + requests: [], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("makes no Management API request for --db-url", () => { + const DB_URL = "postgresql://postgres:postgres@127.0.0.1:5432/postgres"; + const { layer, connection } = setup({ + rows: [DB_STATS_ROW], + cliArgs: ["inspect", "db", "db-stats", "--db-url", DB_URL], + realLayers: { workdir: tempRoot.current }, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* inspectDbDbStats(flags({ dbUrl: Option.some(DB_URL) })); + expect(connection.connectCalls).toHaveLength(1); + expect(requests).toEqual([]); + assertPermissionDrift({ + command: "inspect db db-stats", + activeFlags: { "db-url": true }, + requests: [], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); +}); diff --git a/apps/cli/src/commands/inspect/db/locks/locks.command.ts b/apps/cli/src/commands/inspect/db/locks/locks.command.ts index de978e3d73..264c403403 100644 --- a/apps/cli/src/commands/inspect/db/locks/locks.command.ts +++ b/apps/cli/src/commands/inspect/db/locks/locks.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbLocks } from "./locks.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbLocksCommand = Command.make("locks", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show queries which have taken out an exclusive lock on a relation."), Command.withShortDescription("Show exclusive locks"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbLocks)), Command.provide(inspectDbRuntimeLayer("locks")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/long-running-queries/long-running-queries.command.ts b/apps/cli/src/commands/inspect/db/long-running-queries/long-running-queries.command.ts index 4f36336caf..03a740f719 100644 --- a/apps/cli/src/commands/inspect/db/long-running-queries/long-running-queries.command.ts +++ b/apps/cli/src/commands/inspect/db/long-running-queries/long-running-queries.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbLongRunningQueries } from "./long-running-queries.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -11,6 +16,7 @@ export const inspectDbLongRunningQueriesCommand = Command.make( ).pipe( Command.withDescription("Show currently running queries running for longer than 5 minutes."), Command.withShortDescription("Show long-running queries"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbLongRunningQueries)), Command.provide(inspectDbRuntimeLayer("long-running-queries")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/outliers/outliers.command.ts b/apps/cli/src/commands/inspect/db/outliers/outliers.command.ts index 3c0aaaad53..be422ba44f 100644 --- a/apps/cli/src/commands/inspect/db/outliers/outliers.command.ts +++ b/apps/cli/src/commands/inspect/db/outliers/outliers.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbOutliers } from "./outliers.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbOutliersCommand = Command.make("outliers", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show queries from pg_stat_statements ordered by total execution time."), Command.withShortDescription("Show query outliers by time"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbOutliers)), Command.provide(inspectDbRuntimeLayer("outliers")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/replication-slots/replication-slots.command.ts b/apps/cli/src/commands/inspect/db/replication-slots/replication-slots.command.ts index 8bc97c09ed..bd25bcbe04 100644 --- a/apps/cli/src/commands/inspect/db/replication-slots/replication-slots.command.ts +++ b/apps/cli/src/commands/inspect/db/replication-slots/replication-slots.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbReplicationSlots } from "./replication-slots.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -11,6 +16,7 @@ export const inspectDbReplicationSlotsCommand = Command.make( ).pipe( Command.withDescription("Show information about replication slots on the database."), Command.withShortDescription("Show replication slots"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbReplicationSlots)), Command.provide(inspectDbRuntimeLayer("replication-slots")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/role-configs/role-configs.command.ts b/apps/cli/src/commands/inspect/db/role-configs/role-configs.command.ts index a246b0326f..4d52c41f32 100644 --- a/apps/cli/src/commands/inspect/db/role-configs/role-configs.command.ts +++ b/apps/cli/src/commands/inspect/db/role-configs/role-configs.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbRoleConfigs } from "./role-configs.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbRoleConfigsCommand = Command.make("role-configs", INSPECT_ 'Show configuration settings for database roles when they have been modified. Deprecated: use "role-stats" instead.', ), Command.withShortDescription("Show role configs (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbRoleConfigs)), Command.provide(inspectDbRuntimeLayer("role-configs")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/role-connections/role-connections.command.ts b/apps/cli/src/commands/inspect/db/role-connections/role-connections.command.ts index c339fb55ca..a41cde73de 100644 --- a/apps/cli/src/commands/inspect/db/role-connections/role-connections.command.ts +++ b/apps/cli/src/commands/inspect/db/role-connections/role-connections.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbRoleConnections } from "./role-connections.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -13,6 +18,7 @@ export const inspectDbRoleConnectionsCommand = Command.make( 'Show number of active connections for all database roles. Deprecated: use "role-stats" instead.', ), Command.withShortDescription("Show role connections (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbRoleConnections)), Command.provide(inspectDbRuntimeLayer("role-connections")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/role-stats/role-stats.command.ts b/apps/cli/src/commands/inspect/db/role-stats/role-stats.command.ts index 8cb73fcb83..1b1fb3307c 100644 --- a/apps/cli/src/commands/inspect/db/role-stats/role-stats.command.ts +++ b/apps/cli/src/commands/inspect/db/role-stats/role-stats.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbRoleStats } from "./role-stats.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbRoleStatsCommand = Command.make("role-stats", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show information about roles on the database."), Command.withShortDescription("Show role stats"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbRoleStats)), Command.provide(inspectDbRuntimeLayer("role-stats")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/seq-scans/seq-scans.command.ts b/apps/cli/src/commands/inspect/db/seq-scans/seq-scans.command.ts index 4ca03d1c4c..f8a8d667da 100644 --- a/apps/cli/src/commands/inspect/db/seq-scans/seq-scans.command.ts +++ b/apps/cli/src/commands/inspect/db/seq-scans/seq-scans.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbSeqScans } from "./seq-scans.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbSeqScansCommand = Command.make("seq-scans", INSPECT_DB_FLA 'Show number of sequential scans recorded against all tables. Deprecated: use "index-stats" instead.', ), Command.withShortDescription("Show sequential scans (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbSeqScans)), Command.provide(inspectDbRuntimeLayer("seq-scans")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/table-index-sizes/table-index-sizes.command.ts b/apps/cli/src/commands/inspect/db/table-index-sizes/table-index-sizes.command.ts index 0f2d2c4403..dc13590ec2 100644 --- a/apps/cli/src/commands/inspect/db/table-index-sizes/table-index-sizes.command.ts +++ b/apps/cli/src/commands/inspect/db/table-index-sizes/table-index-sizes.command.ts @@ -1,4 +1,5 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { removedCommand } from "../../../../command-internal/removed-command.ts"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; import { commandRuntimeLayer } from "../../../../shared/runtime/command-runtime.layer.ts"; @@ -13,6 +14,11 @@ export const inspectDbTableIndexSizesCommand = Command.make( ).pipe( Command.withDescription("Removed: use `supabase inspect db table-stats` instead."), Command.withShortDescription("Removed: use `inspect db table-stats` instead"), + // Removed: fails before any Management API call, so no flag changes what it calls. + withPermissions({ + operations: [], + noApiEffectFlags: ["db-url", "linked", "local", "project-ref"], + }), Command.withHandler(() => removedCommand("Use `supabase inspect db table-stats` instead.").pipe( withCommandTelemetry(), diff --git a/apps/cli/src/commands/inspect/db/table-record-counts/table-record-counts.command.ts b/apps/cli/src/commands/inspect/db/table-record-counts/table-record-counts.command.ts index 75a867cc77..0c5fbd70b1 100644 --- a/apps/cli/src/commands/inspect/db/table-record-counts/table-record-counts.command.ts +++ b/apps/cli/src/commands/inspect/db/table-record-counts/table-record-counts.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbTableRecordCounts } from "./table-record-counts.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -13,6 +18,7 @@ export const inspectDbTableRecordCountsCommand = Command.make( 'Show estimated number of rows per table. Deprecated: use "table-stats" instead.', ), Command.withShortDescription("Show table record counts (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbTableRecordCounts)), Command.provide(inspectDbRuntimeLayer("table-record-counts")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/table-sizes/table-sizes.command.ts b/apps/cli/src/commands/inspect/db/table-sizes/table-sizes.command.ts index 7337f6c3c2..90986cfab1 100644 --- a/apps/cli/src/commands/inspect/db/table-sizes/table-sizes.command.ts +++ b/apps/cli/src/commands/inspect/db/table-sizes/table-sizes.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbTableSizes } from "./table-sizes.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -10,6 +15,7 @@ export const inspectDbTableSizesCommand = Command.make("table-sizes", INSPECT_DB 'Show table sizes of individual tables without their index sizes. Deprecated: use "table-stats" instead.', ), Command.withShortDescription("Show table sizes (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbTableSizes)), Command.provide(inspectDbRuntimeLayer("table-sizes")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/table-stats/table-stats.command.ts b/apps/cli/src/commands/inspect/db/table-stats/table-stats.command.ts index aeb480d295..f5f6d7bbc6 100644 --- a/apps/cli/src/commands/inspect/db/table-stats/table-stats.command.ts +++ b/apps/cli/src/commands/inspect/db/table-stats/table-stats.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbTableStats } from "./table-stats.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbTableStatsCommand = Command.make("table-stats", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show combined table size, index size, and estimated row count."), Command.withShortDescription("Show table stats"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbTableStats)), Command.provide(inspectDbRuntimeLayer("table-stats")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/total-index-size/total-index-size.command.ts b/apps/cli/src/commands/inspect/db/total-index-size/total-index-size.command.ts index 9dc0a5848e..48a0822074 100644 --- a/apps/cli/src/commands/inspect/db/total-index-size/total-index-size.command.ts +++ b/apps/cli/src/commands/inspect/db/total-index-size/total-index-size.command.ts @@ -1,4 +1,5 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { removedCommand } from "../../../../command-internal/removed-command.ts"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; import { commandRuntimeLayer } from "../../../../shared/runtime/command-runtime.layer.ts"; @@ -13,6 +14,11 @@ export const inspectDbTotalIndexSizeCommand = Command.make( ).pipe( Command.withDescription("Removed: use `supabase inspect db index-stats` instead."), Command.withShortDescription("Removed: use `inspect db index-stats` instead"), + // Removed: fails before any Management API call, so no flag changes what it calls. + withPermissions({ + operations: [], + noApiEffectFlags: ["db-url", "linked", "local", "project-ref"], + }), Command.withHandler(() => removedCommand("Use `supabase inspect db index-stats` instead.").pipe( withCommandTelemetry(), diff --git a/apps/cli/src/commands/inspect/db/total-table-sizes/total-table-sizes.command.ts b/apps/cli/src/commands/inspect/db/total-table-sizes/total-table-sizes.command.ts index 53cfde0d8c..9c985e19fd 100644 --- a/apps/cli/src/commands/inspect/db/total-table-sizes/total-table-sizes.command.ts +++ b/apps/cli/src/commands/inspect/db/total-table-sizes/total-table-sizes.command.ts @@ -1,4 +1,5 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { removedCommand } from "../../../../command-internal/removed-command.ts"; import { withJsonErrorHandling } from "../../../../shared/output/json-error-handling.ts"; import { commandRuntimeLayer } from "../../../../shared/runtime/command-runtime.layer.ts"; @@ -13,6 +14,11 @@ export const inspectDbTotalTableSizesCommand = Command.make( ).pipe( Command.withDescription("Removed: use `supabase inspect db table-stats` instead."), Command.withShortDescription("Removed: use `inspect db table-stats` instead"), + // Removed: fails before any Management API call, so no flag changes what it calls. + withPermissions({ + operations: [], + noApiEffectFlags: ["db-url", "linked", "local", "project-ref"], + }), Command.withHandler(() => removedCommand("Use `supabase inspect db table-stats` instead.").pipe( withCommandTelemetry(), diff --git a/apps/cli/src/commands/inspect/db/traffic-profile/traffic-profile.command.ts b/apps/cli/src/commands/inspect/db/traffic-profile/traffic-profile.command.ts index aa2cefbaa9..69b83cf562 100644 --- a/apps/cli/src/commands/inspect/db/traffic-profile/traffic-profile.command.ts +++ b/apps/cli/src/commands/inspect/db/traffic-profile/traffic-profile.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbTrafficProfile } from "./traffic-profile.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -13,6 +18,7 @@ export const inspectDbTrafficProfileCommand = Command.make( "Show read/write activity ratio for tables based on block I/O operations.", ), Command.withShortDescription("Show traffic profile"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbTrafficProfile)), Command.provide(inspectDbRuntimeLayer("traffic-profile")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/unused-indexes/unused-indexes.command.ts b/apps/cli/src/commands/inspect/db/unused-indexes/unused-indexes.command.ts index 216b96db33..5f1532d157 100644 --- a/apps/cli/src/commands/inspect/db/unused-indexes/unused-indexes.command.ts +++ b/apps/cli/src/commands/inspect/db/unused-indexes/unused-indexes.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbUnusedIndexes } from "./unused-indexes.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbUnusedIndexesCommand = Command.make("unused-indexes", INSPECT_DB_FLAGS).pipe( Command.withDescription('Show indexes with low usage. Deprecated: use "index-stats" instead.'), Command.withShortDescription("Show unused indexes (deprecated)"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbUnusedIndexes)), Command.provide(inspectDbRuntimeLayer("unused-indexes")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/db/vacuum-stats/vacuum-stats.command.ts b/apps/cli/src/commands/inspect/db/vacuum-stats/vacuum-stats.command.ts index dc7e9dd4f3..a4709dcad0 100644 --- a/apps/cli/src/commands/inspect/db/vacuum-stats/vacuum-stats.command.ts +++ b/apps/cli/src/commands/inspect/db/vacuum-stats/vacuum-stats.command.ts @@ -1,6 +1,11 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { inspectDbVacuumStats } from "./vacuum-stats.handler.ts"; -import { INSPECT_DB_FLAGS, inspectDbCommandHandler } from "../inspect-db-command.ts"; +import { + INSPECT_DB_FLAGS, + INSPECT_DB_PERMISSIONS, + inspectDbCommandHandler, +} from "../inspect-db-command.ts"; import { inspectDbRuntimeLayer } from "../db.layers.ts"; import { cliConfigValuesLayer } from "../../../../config/cli-config-values.layer.ts"; import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; @@ -8,6 +13,7 @@ import { withCliConfigFlags } from "../../../../config/cli-config-flags.ts"; export const inspectDbVacuumStatsCommand = Command.make("vacuum-stats", INSPECT_DB_FLAGS).pipe( Command.withDescription("Show statistics related to vacuum operations per table."), Command.withShortDescription("Show vacuum stats"), + withPermissions(INSPECT_DB_PERMISSIONS), Command.withHandler(inspectDbCommandHandler(inspectDbVacuumStats)), Command.provide(inspectDbRuntimeLayer("vacuum-stats")), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/inspect/report/SIDE_EFFECTS.md b/apps/cli/src/commands/inspect/report/SIDE_EFFECTS.md index b686ad1322..26debd732e 100644 --- a/apps/cli/src/commands/inspect/report/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/inspect/report/SIDE_EFFECTS.md @@ -6,14 +6,14 @@ validating those CSVs. ## Files Read -| Path | Format | When | -| ---------------------------------------------- | ---------- | --------------------------------------------------------------------------- | -| `/supabase/config.toml` | TOML | always โ€” `[experimental.inspect.rules]` (custom rules) + `[db]` subtree | -| `/supabase/.env*` (nested) | dotenv | always โ€” `env(VAR)` expansion for `[db]` and rule string fields | -| `/supabase/.temp/pooler-url` | plain text | `--linked` path (pooler connection string) | -| `/supabase/.temp/linked-project.json` | JSON | `--linked` path (resolve linked project ref) | -| `~/.supabase/access-token` | plain text | `--linked` path, when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | -| `//.csv` ร—14 | CSV | read back in-memory for rule evaluation | +| Path | Format | When | +| ------------------------------------------ | ---------- | -------------------------------------------------------------------------------------- | +| `/supabase/config.toml` | TOML | always โ€” `[experimental.inspect.rules]` (custom rules) + `[db]` subtree | +| `/supabase/.env*` (nested) | dotenv | always โ€” `env(VAR)` expansion for `[db]` and rule string fields | +| `/supabase/.temp/pooler-url` | plain text | `--linked` path (pooler connection string) | +| `/supabase/.temp/project-ref` | plain text | `--linked` path, without `--project-ref` or `SUPABASE_PROJECT_ID` (linked project ref) | +| `~/.supabase/access-token` | plain text | `--linked` path, when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | +| `//.csv` ร—14 | CSV | read back in-memory for rule evaluation | A **missing** `config.toml` is fine (defaults apply); a **malformed** file, an invalid value (including a non-string rule field such as `query = 123` or @@ -23,12 +23,11 @@ or an unknown rule key aborts the command before connecting. Rules and `env(VAR) ## Files Written -| Path | Mode | When | -| ------------------------------------------------ | ---- | --------------------------------------------------------------- | -| `//` (directory) | 0755 | always โ€” created recursively | -| `//.csv` ร—14 | 0644 | always โ€” one CSV per inspect query (server-side `COPY ... CSV`) | -| `~/.supabase/telemetry.json` | โ€” | always (telemetry flush) | -| `~/.supabase//linked-project.json` | โ€” | `--linked` path (linked-project cache) | +| Path | Mode | When | +| ------------------------------------------ | ---- | --------------------------------------------------------------- | +| `//` (directory) | 0755 | always โ€” created recursively | +| `//.csv` ร—14 | 0644 | always โ€” one CSV per inspect query (server-side `COPY ... CSV`) | +| `~/.supabase/telemetry.json` | โ€” | always (telemetry flush) | The 14 CSV basenames (underscored, matching the SQL filenames โ€” **not** the `inspect db` command names): `bloat`, `blocking`, `calls`, `db_stats`, diff --git a/apps/cli/src/commands/inspect/report/report.command.ts b/apps/cli/src/commands/inspect/report/report.command.ts index eb7b6a7ca4..ead991a6d1 100644 --- a/apps/cli/src/commands/inspect/report/report.command.ts +++ b/apps/cli/src/commands/inspect/report/report.command.ts @@ -2,6 +2,9 @@ import { Layer } from "effect"; import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { compose, withDbTarget } from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { commandRuntimeLayer } from "../../../shared/runtime/command-runtime.layer.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; @@ -41,6 +44,11 @@ export type InspectReportFlags = CliCommand.Command.Config.Infer; export const inspectReportCommand = Command.make("report", config).pipe( Command.withDescription("Generate a CSV output for all inspect commands."), Command.withShortDescription("Generate a CSV output for all inspect commands"), + withPermissions( + withDbTarget(compose({ operations: [], noApiEffectFlags: ["output-dir"] }, dbConfig), { + default: "linked", + }), + ), Command.withHandler((flags) => inspectReport(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/src/commands/inspect/report/report.integration.test.ts b/apps/cli/src/commands/inspect/report/report.integration.test.ts index b7c1bdce54..721f96bde2 100644 --- a/apps/cli/src/commands/inspect/report/report.integration.test.ts +++ b/apps/cli/src/commands/inspect/report/report.integration.test.ts @@ -2,6 +2,7 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Cause, + ConfigProvider, DateTime, Effect, Exit, @@ -11,13 +12,32 @@ import { Path, PlatformError, } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { mockOutput, mockRuntimeInfo, mockTty } from "../../../../tests/helpers/mocks.ts"; import { + VALID_REF, + VALID_TOKEN, mockCommandSettings, mockTelemetryStateTracked, + useTempWorkdir, withEnvVar, } from "../../../../tests/helpers/command-mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + realDbConfigResolver, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { configValuesLayer } from "../../../../tests/helpers/config-values-layer.ts"; import { DnsResolverFlag } from "../../../command-internal/global-flags.ts"; @@ -133,6 +153,8 @@ interface SetupOpts { workdir?: string; /** Raw CLI args slice, used to detect which flags were explicitly passed. */ cliArgs?: ReadonlyArray; + /** Runs the real `DbConfigResolver` against `workdir`. */ + realLayers?: boolean; } function setupReport(opts: SetupOpts = {}) { @@ -150,7 +172,13 @@ function setupReport(opts: SetupOpts = {}) { const telemetry = mockTelemetryStateTracked(); const layer = Layer.mergeAll( out.layer, - resolver.layer, + opts.realLayers !== true || opts.workdir === undefined + ? resolver.layer + : realDbConfigResolver({ + workdir: opts.workdir, + dbConnection: connection.layer, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }), connection.layer, telemetry.layer, Layer.succeed(DnsResolverFlag, "native"), @@ -164,6 +192,14 @@ function setupReport(opts: SetupOpts = {}) { mockTty({ stdoutIsTty: opts.stdoutIsTty ?? false }), BunServices.layer, configValuesLayer(), + opts.realLayers !== true + ? Layer.empty + : ConfigProvider.layer( + ConfigProvider.fromEnvRecord( + { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + { preserveEmptyStrings: true }, + ), + ), ); return { layer, out, resolver, connection, telemetry }; } @@ -671,3 +707,61 @@ describe("inspect report", () => { }).pipe(Effect.provide(layer)); }); }); + +describe("inspect report through the real database-config resolver", () => { + const tempRoot = useTempWorkdir("supabase-inspect-report-int-"); + + it.effect("mints a login role for the linked project and writes every CSV", () => { + const { layer, connection } = setupReport({ + csvs: DEFAULT_RULE_CSVS, + isLocal: false, + cliArgs: ["inspect", "report", "--project-ref", VALID_REF], + workdir: tempRoot.current, + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + const base = yield* tempDir("supabase-report-out-"); + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* inspectReport(flags({ outputDir: base, projectRef: Option.some(VALID_REF) })); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + const { files } = yield* dateFolderContents(base); + expect(files).toHaveLength(14); + expect(connection.copiedSql).toHaveLength(14); + assertPermissionDrift({ + command: "inspect report", + activeFlags: { "project-ref": VALID_REF, "output-dir": true }, + requests: asRecordedRequests(requests), + exact: true, + }); + }).pipe(Effect.scoped, Effect.provide(layer)); + }); + + it.effect("makes no Management API request for --local", () => { + const { layer, connection } = setupReport({ + csvs: DEFAULT_RULE_CSVS, + cliArgs: ["inspect", "report", "--local"], + workdir: tempRoot.current, + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + const base = yield* tempDir("supabase-report-out-"); + yield* stubManagementApi(requests, () => undefined); + yield* inspectReport(flags({ outputDir: base, local: true })); + const { files } = yield* dateFolderContents(base); + expect(files).toHaveLength(14); + expect(connection.copiedSql).toHaveLength(14); + expect(requests).toEqual([]); + assertPermissionDrift({ + command: "inspect report", + activeFlags: { local: true, "output-dir": true }, + requests: [], + exact: true, + }); + }).pipe(Effect.scoped, Effect.provide(layer)); + }); +}); diff --git a/apps/cli/src/commands/migration/down/down.command.ts b/apps/cli/src/commands/migration/down/down.command.ts index b346fe4392..cbd258ee9c 100644 --- a/apps/cli/src/commands/migration/down/down.command.ts +++ b/apps/cli/src/commands/migration/down/down.command.ts @@ -1,6 +1,14 @@ import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { + compose, + withCondition, + withDbTarget, +} from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { migrationDbRuntimeLayer } from "../migration.layers.ts"; @@ -49,6 +57,16 @@ export type MigrationDownFlags = CliCommand.Command.Config.Infer; export const migrationDownCommand = Command.make("down", config).pipe( Command.withDescription("Resets applied migrations up to the last n versions."), Command.withShortDescription("Resets applied migrations up to the last n versions"), + withPermissions( + withDbTarget( + compose( + { operations: [], noApiEffectFlags: ["last"] }, + dbConfig, + withCondition(linkedProjectCache, { dbTarget: ["linked"] }), + ), + { default: "local" }, + ), + ), Command.withHandler((flags) => migrationDown(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/src/commands/migration/down/down.integration.test.ts b/apps/cli/src/commands/migration/down/down.integration.test.ts index 9badad9e42..ebe4a8d100 100644 --- a/apps/cli/src/commands/migration/down/down.integration.test.ts +++ b/apps/cli/src/commands/migration/down/down.integration.test.ts @@ -2,10 +2,19 @@ import { createHash } from "node:crypto"; import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, FileSystem, Layer, Option, Path } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { VALID_REF, + VALID_TOKEN, mockCommandSettings, mockLinkedProjectCacheTracked, mockTelemetryStateTracked, @@ -14,6 +23,17 @@ import { } from "../../../../tests/helpers/command-mocks.ts"; import { cliConfigValuesTestLayer } from "../../../../tests/helpers/config-values-layer.ts"; import { mockOutput, mockStdin, mockTty } from "../../../../tests/helpers/mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + realLinkedProjectCache, + recordingProjectHttpClient, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { DnsResolverFlag, YesFlag } from "../../../command-internal/global-flags.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; @@ -33,6 +53,8 @@ import type { MigrationDownFlags } from "./down.command.ts"; const LIST_SQL = "SELECT version FROM supabase_migrations.schema_migrations ORDER BY version"; interface SetupOpts { + /** Runs the real `DbConfigResolver` and linked-project cache against `workdir`. */ + readonly realLayers?: boolean; readonly format?: OutputFormat; readonly isTTY?: boolean; readonly pipedInput?: string; @@ -125,12 +147,24 @@ function setup(workdir: string, opts: SetupOpts = {}) { promptProjectRef: () => Effect.succeed(VALID_REF), }); + const http = recordingProjectHttpClient(VALID_REF); const layer = Layer.mergeAll( cliConfigValuesTestLayer, out.layer, telemetry.layer, - cache.layer, - resolver, + opts.realLayers === true + ? realLinkedProjectCache({ + httpClient: http.layer, + cliSettings: mockCommandSettings({ workdir }), + }) + : cache.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir, + dbConnection: connection, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver, connection, projectRef, mockCommandSettings({ workdir }), @@ -145,7 +179,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { ), BunServices.layer, ); - return { layer, out, telemetry, execs, queries, cache }; + return { layer, http, out, telemetry, execs, queries, cache }; } const flags = (over: Partial = {}): MigrationDownFlags => ({ @@ -422,4 +456,79 @@ describe("migration down", () => { expect(queries.some((q) => q.sql.includes("INSERT INTO supabase_migrations"))).toBe(false); }).pipe(Effect.provide(layer)); }); + + it.effect("mints a login role and fills the cache through the real resolver and cache", () => { + const ctx = setup(tmp.current, { + args: ["migration", "down", "--linked", "--project-ref", VALID_REF], + confirm: true, + realLayers: true, + remote: ["20240101000000", "20240102000000"], + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seed(tmp.current, "20240101000000_a.sql"); + yield* migrationDown( + flags({ last: 1, linked: true, local: false, projectRef: Option.some(VALID_REF) }), + ); + expect(ctx.out.stderrText).toContain("Resetting database to version: 20240101000000"); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(ctx.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration down", + activeFlags: { linked: true, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("defaults to the local target and makes no Management API request", () => { + const ctx = setup(tmp.current, { + args: ["migration", "down"], + confirm: true, + realLayers: true, + remote: ["20240101000000", "20240102000000"], + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* seed(tmp.current, "20240101000000_a.sql"); + yield* migrationDown(flags({ last: 1 })); + expect(ctx.out.stderrText).toContain("Resetting database to version: 20240101000000"); + expect(requests).toEqual([]); + expect(ctx.http.calls).toEqual([]); + assertPermissionDrift({ + command: "migration down", + requests: [], + exact: true, + }); + expect(() => + assertPermissionDrift({ + command: "migration down", + requests: [ + { + method: "POST", + url: `https://api.supabase.com/v1/projects/${VALID_REF}/cli/login-role`, + }, + ], + }), + ).toThrow(/condition doesn't hold/); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); diff --git a/apps/cli/src/commands/migration/fetch/fetch.command.ts b/apps/cli/src/commands/migration/fetch/fetch.command.ts index c211abea9f..68e50a7dd3 100644 --- a/apps/cli/src/commands/migration/fetch/fetch.command.ts +++ b/apps/cli/src/commands/migration/fetch/fetch.command.ts @@ -1,6 +1,14 @@ import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { + compose, + withCondition, + withDbTarget, +} from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { migrationDbRuntimeLayer } from "../migration.layers.ts"; @@ -35,6 +43,16 @@ export type MigrationFetchFlags = CliCommand.Command.Config.Infer export const migrationFetchCommand = Command.make("fetch", config).pipe( Command.withDescription("Fetch migration files from history table."), Command.withShortDescription("Fetch migration files from history table"), + withPermissions( + withDbTarget( + compose( + { operations: [], noApiEffectFlags: [] }, + dbConfig, + withCondition(linkedProjectCache, { dbTarget: ["linked"] }), + ), + { default: "linked" }, + ), + ), Command.withHandler((flags) => migrationFetch(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/src/commands/migration/fetch/fetch.integration.test.ts b/apps/cli/src/commands/migration/fetch/fetch.integration.test.ts index daf5aba25a..42054fddeb 100644 --- a/apps/cli/src/commands/migration/fetch/fetch.integration.test.ts +++ b/apps/cli/src/commands/migration/fetch/fetch.integration.test.ts @@ -1,15 +1,35 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, FileSystem, Layer, Option, Path } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { VALID_REF, + VALID_TOKEN, mockCommandSettings, mockLinkedProjectCacheTracked, mockTelemetryStateTracked, useTempWorkdir, } from "../../../../tests/helpers/command-mocks.ts"; import { mockOutput, mockStdin, mockTty } from "../../../../tests/helpers/mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + realLinkedProjectCache, + recordingProjectHttpClient, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { DnsResolverFlag, YesFlag } from "../../../command-internal/global-flags.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; @@ -33,6 +53,8 @@ interface MigrationRow { } interface SetupOpts { + /** Runs the real `DbConfigResolver` and linked-project cache against `workdir`. */ + readonly realLayers?: boolean; readonly format?: OutputFormat; readonly isTTY?: boolean; readonly pipedInput?: string; @@ -104,11 +126,23 @@ function setup(workdir: string, opts: SetupOpts = {}) { promptProjectRef: () => Effect.succeed(VALID_REF), }); + const http = recordingProjectHttpClient(VALID_REF); const layer = Layer.mergeAll( out.layer, telemetry.layer, - cache.layer, - resolver, + opts.realLayers === true + ? realLinkedProjectCache({ + httpClient: http.layer, + cliSettings: mockCommandSettings({ workdir }), + }) + : cache.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir, + dbConnection: connection, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver, connection, projectRef, mockCommandSettings({ workdir }), @@ -123,7 +157,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { ), BunServices.layer, ); - return { layer, out, telemetry, cache }; + return { layer, http, out, telemetry, cache }; } const flags = (over: Partial = {}): MigrationFetchFlags => ({ @@ -455,4 +489,62 @@ describe("migration fetch", () => { expect(cache.cached).toBe(false); }).pipe(Effect.provide(layer)); }); + + it.effect("mints a login role and fills the cache through the real resolver and cache", () => { + const ctx = setup(tmp.current, { + cliArgs: ["migration", "fetch", "--project-ref", VALID_REF], + realLayers: true, + rows: [{ version: "20240101000000", name: "init", statements: ["create table a"] }], + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* migrationFetch(flags({ projectRef: Option.some(VALID_REF) })); + expect(yield* listMigrations(tmp.current)).toEqual(["20240101000000_init.sql"]); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(ctx.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration fetch", + activeFlags: { "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("makes no Management API request for --local through the real resolver", () => { + const ctx = setup(tmp.current, { + cliArgs: ["migration", "fetch", "--local"], + realLayers: true, + rows: [], + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* migrationFetch(flags({ linked: false, local: true })); + expect(requests).toEqual([]); + expect(ctx.http.calls).toEqual([]); + assertPermissionDrift({ + command: "migration fetch", + activeFlags: { local: true }, + requests: [], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); diff --git a/apps/cli/src/commands/migration/list/list.command.ts b/apps/cli/src/commands/migration/list/list.command.ts index 7d3c1caf4b..db60757b30 100644 --- a/apps/cli/src/commands/migration/list/list.command.ts +++ b/apps/cli/src/commands/migration/list/list.command.ts @@ -1,6 +1,14 @@ import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { + compose, + withCondition, + withDbTarget, +} from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withCliConfigFlags } from "../../../config/cli-config-flags.ts"; import { CliConfigKeys } from "../../../config/cli-config-keys.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; @@ -42,6 +50,16 @@ export type MigrationListFlags = CliCommand.Command.Config.Infer; export const migrationListCommand = Command.make("list", config).pipe( Command.withDescription("List local and remote migrations."), Command.withShortDescription("List local and remote migrations"), + withPermissions( + withDbTarget( + compose( + { operations: [], noApiEffectFlags: [] }, + dbConfig, + withCondition(linkedProjectCache, { dbTarget: ["linked"] }), + ), + { default: "linked" }, + ), + ), Command.withHandler((flags) => migrationList(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/src/commands/migration/list/list.integration.test.ts b/apps/cli/src/commands/migration/list/list.integration.test.ts index 6bc521d29f..0952e93453 100644 --- a/apps/cli/src/commands/migration/list/list.integration.test.ts +++ b/apps/cli/src/commands/migration/list/list.integration.test.ts @@ -1,16 +1,36 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, FileSystem, Layer, Option, Path } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { VALID_REF, + VALID_TOKEN, mockCommandSettings, mockLinkedProjectCacheTracked, mockTelemetryStateTracked, useTempWorkdir, } from "../../../../tests/helpers/command-mocks.ts"; import { mockOutput } from "../../../../tests/helpers/mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + realLinkedProjectCache, + recordingProjectHttpClient, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { DnsResolverFlag } from "../../../command-internal/global-flags.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; @@ -33,6 +53,8 @@ interface SetupOpts { readonly isLocal?: boolean; readonly remote?: ReadonlyArray; readonly remoteError?: DbExecError; + /** Runs the real `DbConfigResolver` and linked-project cache against `workdir`. */ + readonly realLayers?: boolean; } function setup(workdir: string, opts: SetupOpts = {}) { @@ -91,11 +113,23 @@ function setup(workdir: string, opts: SetupOpts = {}) { promptProjectRef: () => Effect.succeed(VALID_REF), }); + const http = recordingProjectHttpClient(VALID_REF); const layer = Layer.mergeAll( out.layer, telemetry.layer, - cache.layer, - resolver, + opts.realLayers === true + ? realLinkedProjectCache({ + httpClient: http.layer, + cliSettings: mockCommandSettings({ workdir }), + }) + : cache.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir, + dbConnection: connection, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver, connection, projectRef, mockCommandSettings({ workdir }), @@ -108,6 +142,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { out, telemetry, cache, + http, resolverCalls, }; } @@ -289,4 +324,89 @@ describe("migration list", () => { expect(error).toBeInstanceOf(MigrationsReadError); }).pipe(Effect.provide(layer)); }); + + it.effect("mints a login role and fills the cache through the real resolver and cache", () => { + const ctx = setup(tmp.current, { + args: ["migration", "list", "--project-ref", VALID_REF], + realLayers: true, + remote: ["20240101000000"], + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seedMigrations(tmp.current, ["20240101000000_a.sql"]); + yield* migrationList(flags({ projectRef: Option.some(VALID_REF) })); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(ctx.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration list", + activeFlags: { "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("makes no Management API request for --local through the real resolver", () => { + const ctx = setup(tmp.current, { + args: ["migration", "list", "--local"], + isLocal: true, + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* migrationList(flags({ linked: false, local: true })); + expect(requests).toEqual([]); + expect(ctx.http.calls).toEqual([]); + assertPermissionDrift({ + command: "migration list", + activeFlags: { local: true }, + requests: [], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("fetches the pooler config but mints no login role when --password is given", () => { + const ctx = setup(tmp.current, { + args: ["migration", "list", "--password", "pw", "--project-ref", VALID_REF], + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* migrationList( + flags({ password: Option.some("pw"), projectRef: Option.some(VALID_REF) }), + ); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + ]); + assertPermissionDrift({ + command: "migration list", + activeFlags: { password: true, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); diff --git a/apps/cli/src/commands/migration/new/new.command.ts b/apps/cli/src/commands/migration/new/new.command.ts index 36e2996098..fca7e49391 100644 --- a/apps/cli/src/commands/migration/new/new.command.ts +++ b/apps/cli/src/commands/migration/new/new.command.ts @@ -1,6 +1,7 @@ import { Argument, Command } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { migrationNewRuntimeLayer } from "../migration.layers.ts"; @@ -17,6 +18,7 @@ export type MigrationNewFlags = CliCommand.Command.Config.Infer; export const migrationNewCommand = Command.make("new", config).pipe( Command.withDescription("Create an empty migration script."), Command.withShortDescription("Create an empty migration script"), + withPermissions({ operations: [] }), Command.withHandler((flags) => migrationNew(flags).pipe(withCommandTelemetry(), withJsonErrorHandling), ), diff --git a/apps/cli/src/commands/migration/repair/repair.command.ts b/apps/cli/src/commands/migration/repair/repair.command.ts index 44e020b86c..3d46a5cb40 100644 --- a/apps/cli/src/commands/migration/repair/repair.command.ts +++ b/apps/cli/src/commands/migration/repair/repair.command.ts @@ -1,5 +1,13 @@ import { Argument, Command, Flag } from "effect/cli"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { + compose, + withCondition, + withDbTarget, +} from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withCliConfigFlags } from "../../../config/cli-config-flags.ts"; import { CliConfigKeys } from "../../../config/cli-config-keys.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; @@ -46,6 +54,16 @@ const config = { export const migrationRepairCommand = Command.make("repair", config).pipe( Command.withDescription("Repair the migration history table."), Command.withShortDescription("Repair the migration history table"), + withPermissions( + withDbTarget( + compose( + { operations: [], noApiEffectFlags: ["status"] }, + dbConfig, + withCondition(linkedProjectCache, { dbTarget: ["linked"] }), + ), + { default: "linked" }, + ), + ), Command.withHandler((flags) => migrationRepair({ versions: flags.versions.map(String), diff --git a/apps/cli/src/commands/migration/repair/repair.integration.test.ts b/apps/cli/src/commands/migration/repair/repair.integration.test.ts index a5793e2d10..b09b7924c3 100644 --- a/apps/cli/src/commands/migration/repair/repair.integration.test.ts +++ b/apps/cli/src/commands/migration/repair/repair.integration.test.ts @@ -1,10 +1,19 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, FileSystem, Layer, Option, Path } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { VALID_REF, + VALID_TOKEN, mockCommandSettings, mockLinkedProjectCacheTracked, mockTelemetryStateTracked, @@ -13,6 +22,17 @@ import { withConfigEnv, } from "../../../../tests/helpers/command-mocks.ts"; import { mockOutput, mockStdin, mockTty } from "../../../../tests/helpers/mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + realLinkedProjectCache, + recordingProjectHttpClient, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { DnsResolverFlag, YesFlag } from "../../../command-internal/global-flags.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; @@ -33,6 +53,8 @@ import { MigrationRepairUpdateError } from "./repair.errors.ts"; import { migrationRepair, type MigrationRepairInput } from "./repair.handler.ts"; interface SetupOpts { + /** Runs the real `DbConfigResolver` and linked-project cache against `workdir`. */ + readonly realLayers?: boolean; readonly format?: OutputFormat; readonly isTTY?: boolean; readonly pipedInput?: string; @@ -116,11 +138,23 @@ function setup(workdir: string, opts: SetupOpts = {}) { promptProjectRef: () => Effect.succeed(VALID_REF), }); + const http = recordingProjectHttpClient(VALID_REF); const layer = Layer.mergeAll( out.layer, telemetry.layer, - cache.layer, - resolver, + opts.realLayers === true + ? realLinkedProjectCache({ + httpClient: http.layer, + cliSettings: mockCommandSettings({ workdir }), + }) + : cache.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir, + dbConnection: connection, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver, connection, projectRef, mockCommandSettings({ workdir }), @@ -135,7 +169,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { ), BunServices.layer, ); - return { layer, out, telemetry, execs, queries, cache }; + return { layer, http, out, telemetry, execs, queries, cache }; } const input = (over: Partial = {}): MigrationRepairInput => ({ @@ -473,4 +507,102 @@ describe("migration repair", () => { ); }).pipe(Effect.provide(layer)); }); + + it.effect("mints a login role and fills the cache through the real resolver and cache", () => { + const ctx = setup(tmp.current, { + args: ["migration", "repair", "--project-ref", VALID_REF], + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seedMigration(tmp.current, "20240101000000_init.sql", "create table a;\n"); + yield* migrationRepair( + input({ + versions: ["20240101000000"], + status: "applied", + projectRef: Option.some(VALID_REF), + }), + ); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(ctx.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration repair", + activeFlags: { "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("makes no Management API request for --local through the real resolver", () => { + const ctx = setup(tmp.current, { + args: ["migration", "repair", "--local"], + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* seedMigration(tmp.current, "20240101000000_init.sql", "create table a;\n"); + yield* migrationRepair( + input({ versions: ["20240101000000"], status: "applied", linked: false, local: true }), + ); + expect(requests).toEqual([]); + expect(ctx.http.calls).toEqual([]); + assertPermissionDrift({ + command: "migration repair", + activeFlags: { local: true }, + requests: [], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("fetches the pooler config but mints no login role when --password is given", () => { + const ctx = setup(tmp.current, { + args: ["migration", "repair", "--password", "pw", "--project-ref", VALID_REF], + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seedMigration(tmp.current, "20240101000000_init.sql", "create table a;\n"); + yield* migrationRepair( + input({ + versions: ["20240101000000"], + status: "applied", + password: Option.some("pw"), + projectRef: Option.some(VALID_REF), + }), + ); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + ]); + assertPermissionDrift({ + command: "migration repair", + activeFlags: { password: true, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); diff --git a/apps/cli/src/commands/migration/squash/SIDE_EFFECTS.md b/apps/cli/src/commands/migration/squash/SIDE_EFFECTS.md index c4905f5753..26e11243d9 100644 --- a/apps/cli/src/commands/migration/squash/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/migration/squash/SIDE_EFFECTS.md @@ -74,13 +74,13 @@ live shadow available. ## API Routes -| Method | Path | Auth | Purpose | -| ---------- | ---------------------------------- | ------ | ---------------------------------------------------------- | -| โ€” | โ€” | โ€” | local target: none | -| POST | `/v1/projects/{ref}/roles` | Bearer | `--linked`: temp login role when no password | -| GET | `/v1/projects/{ref}/pooler/config` | Bearer | `--linked`: IPv4 pooler fallback (IPv6-only network) | -| GET/DELETE | `/v1/projects/{ref}/network-bans` | Bearer | `--linked`: unban during pooler login retry | -| GET | `/v1/projects/{ref}` | Bearer | `--linked`: linked-project cache (post-run, unconditional) | +| Method | Path | Auth | Purpose | +| ----------- | ----------------------------------------------------------------------------- | ------ | ---------------------------------------------------------- | +| โ€” | โ€” | โ€” | local target: none | +| POST | `/v1/projects/{ref}/cli/login-role` | Bearer | `--linked`: temp login role when no password | +| GET | `/v1/projects/{ref}/config/database/pooler` | Bearer | `--linked`: IPv4 pooler fallback (IPv6-only network) | +| POST/DELETE | `/v1/projects/{ref}/network-bans/retrieve`, `/v1/projects/{ref}/network-bans` | Bearer | `--linked`: unban during pooler login retry | +| GET | `/v1/projects/{ref}` | Bearer | `--linked`: linked-project cache (post-run, unconditional) | ## Environment Variables diff --git a/apps/cli/src/commands/migration/squash/squash.command.ts b/apps/cli/src/commands/migration/squash/squash.command.ts index 537cb9822a..9491a7cea2 100644 --- a/apps/cli/src/commands/migration/squash/squash.command.ts +++ b/apps/cli/src/commands/migration/squash/squash.command.ts @@ -1,6 +1,14 @@ import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { + compose, + withCondition, + withDbTarget, +} from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withCliConfigFlags } from "../../../config/cli-config-flags.ts"; import { CliConfigKeys } from "../../../config/cli-config-keys.ts"; import { cliConfigValuesLayer } from "../../../config/cli-config-values.layer.ts"; @@ -46,6 +54,16 @@ export type MigrationSquashFlags = CliCommand.Command.Config.Infer migrationSquash(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/src/commands/migration/squash/squash.integration.test.ts b/apps/cli/src/commands/migration/squash/squash.integration.test.ts index 9f8a52e2f6..c12ace13bb 100644 --- a/apps/cli/src/commands/migration/squash/squash.integration.test.ts +++ b/apps/cli/src/commands/migration/squash/squash.integration.test.ts @@ -6,11 +6,20 @@ import { Config, ConfigProvider, Effect, Exit, FileSystem, Layer, Option, Path } import { PlatformError, SystemError } from "effect/PlatformError"; import * as HttpClient from "effect/http/HttpClient"; import * as HttpClientResponse from "effect/http/HttpClientResponse"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { FAKE_SHADOW_CONTAINER_ID, VALID_REF, + VALID_TOKEN, mockCommandSettings, mockLinkedProjectCacheTracked, mockShadowContainerCliSpawner, @@ -27,6 +36,17 @@ import { mockStdin, mockTty, } from "../../../../tests/helpers/mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + realLinkedProjectCache, + recordingProjectHttpClient, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { dockerfileServiceImage } from "../../../shared/services/dockerfile-images.ts"; import { slimImagesEnabled } from "../../../shared/services/slim-images.ts"; import { getRegistryImageUrl } from "../../../command-internal/docker-registry.ts"; @@ -211,6 +231,8 @@ const alwaysReadyHttpClientLayer = Layer.succeed( ); interface SetupOpts { + /** Runs the real `DbConfigResolver` and linked-project cache against `workdir`. */ + readonly realLayers?: boolean; readonly format?: OutputFormat; readonly isTTY?: boolean; readonly pipedInput?: string; @@ -346,6 +368,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { http: () => Effect.void, }); + const http = recordingProjectHttpClient(VALID_REF); const baseLayer = Layer.mergeAll( unusedStackServices, // Listed first so every fake service layer below overrides it; Layer.mergeAll is @@ -354,8 +377,19 @@ function setup(workdir: string, opts: SetupOpts = {}) { cliConfigValuesTestLayer, out.layer, telemetry.layer, - cache.layer, - resolver, + opts.realLayers === true + ? realLinkedProjectCache({ + httpClient: http.layer, + cliSettings: mockCommandSettings({ workdir }), + }) + : cache.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir, + dbConnection: connection, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver, connection, projectRef, spawner.layer, @@ -391,6 +425,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { return { layer, + http, out, telemetry, cache, @@ -1381,6 +1416,110 @@ describe("migration squash", () => { }).pipe(Effect.provide(s.layer)); }, ); + it.effect("mints a login role and fills the cache through the real resolver and cache", () => { + const s = setupRemote({ + args: ["migration", "squash", "--linked", "--project-ref", VALID_REF], + confirm: true, + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seedMigration(tmp.current, "0_init.sql"); + yield* migrationSquash( + flags({ linked: true, local: false, projectRef: Option.some(VALID_REF) }), + ); + expect(s.queries.some((q) => q.sql.includes("INSERT INTO supabase_migrations"))).toBe(true); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(s.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration squash", + activeFlags: { linked: true, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...s.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(s.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("fetches the pooler config but mints no login role when --password is given", () => { + const s = setupRemote({ + args: ["migration", "squash", "--linked", "--password", "pw", "--project-ref", VALID_REF], + confirm: true, + realLayers: true, + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seedMigration(tmp.current, "0_init.sql"); + yield* migrationSquash( + flags({ + linked: true, + local: false, + password: Option.some("pw"), + projectRef: Option.some(VALID_REF), + }), + ); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + ]); + assertPermissionDrift({ + command: "migration squash", + activeFlags: { linked: true, password: true, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...s.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(s.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + }); + + describe("real resolver on the local target", () => { + it.effect("defaults to the local target and makes no Management API request", () => { + const s = setup(tmp.current, { args: ["migration", "squash"], realLayers: true }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* seedMigration(tmp.current, "0_init.sql"); + yield* migrationSquash(flags()); + expect(stdout(s.out)).toContain("Finished supabase migration squash."); + expect(requests).toEqual([]); + expect(s.http.calls).toEqual([]); + assertPermissionDrift({ + command: "migration squash", + requests: [], + exact: true, + }); + expect(() => + assertPermissionDrift({ + command: "migration squash", + requests: [ + { + method: "POST", + url: `https://api.supabase.com/v1/projects/${VALID_REF}/cli/login-role`, + }, + ], + }), + ).toThrow(/condition doesn't hold/); + }).pipe( + Effect.scoped, + Effect.provide(s.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); describe("output formats", () => { diff --git a/apps/cli/src/commands/migration/up/up.command.ts b/apps/cli/src/commands/migration/up/up.command.ts index 66b8ba9c44..9a2750740c 100644 --- a/apps/cli/src/commands/migration/up/up.command.ts +++ b/apps/cli/src/commands/migration/up/up.command.ts @@ -1,6 +1,14 @@ import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { + compose, + withCondition, + withDbTarget, +} from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; import { migrationDbRuntimeLayer } from "../migration.layers.ts"; @@ -39,6 +47,16 @@ export type MigrationUpFlags = CliCommand.Command.Config.Infer; export const migrationUpCommand = Command.make("up", config).pipe( Command.withDescription("Apply pending migrations to local database."), Command.withShortDescription("Apply pending migrations to local database"), + withPermissions( + withDbTarget( + compose( + { operations: [], noApiEffectFlags: ["include-all"] }, + dbConfig, + withCondition(linkedProjectCache, { dbTarget: ["linked"] }), + ), + { default: "local" }, + ), + ), Command.withHandler((flags) => migrationUp(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/src/commands/migration/up/up.integration.test.ts b/apps/cli/src/commands/migration/up/up.integration.test.ts index 8372615e18..2b917f4e79 100644 --- a/apps/cli/src/commands/migration/up/up.integration.test.ts +++ b/apps/cli/src/commands/migration/up/up.integration.test.ts @@ -1,10 +1,19 @@ import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, FileSystem, Layer, Option, Path } from "effect"; +import { vi } from "vitest"; + +vi.mock("node:net", (importOriginal) => + Promise.all([ + import("../../../../tests/helpers/refuse-invalid-db-hosts.ts"), + importOriginal(), + ]).then(([helper, net]) => helper.refuseInvalidDbHosts(net)), +); import { stripAnsi } from "../../../../tests/helpers/ansi.ts"; import { VALID_REF, + VALID_TOKEN, mockCommandSettings, mockLinkedProjectCacheTracked, mockTelemetryStateTracked, @@ -13,6 +22,17 @@ import { } from "../../../../tests/helpers/command-mocks.ts"; import { cliConfigValuesTestLayer } from "../../../../tests/helpers/config-values-layer.ts"; import { mockOutput } from "../../../../tests/helpers/mocks.ts"; +import { assertPermissionDrift } from "../../../../tests/helpers/permission-drift.ts"; +import { + asRecordedRequests, + provideEnv, + realDbConfigResolver, + realLinkedProjectCache, + recordingProjectHttpClient, + serveLinkedDatabase, + stubManagementApi, + type StubbedRequest, +} from "../../../../tests/helpers/permission-real-layers.ts"; import { CliArgs } from "../../../shared/cli/cli-args.service.ts"; import { DnsResolverFlag } from "../../../command-internal/global-flags.ts"; import type { OutputFormat } from "../../../shared/output/types.ts"; @@ -32,6 +52,8 @@ const LIST_SQL = "SELECT version FROM supabase_migrations.schema_migrations ORDE const READ_VAULT = "SELECT id, name FROM vault.secrets WHERE name = ANY($1)"; interface SetupOpts { + /** Runs the real `DbConfigResolver` and linked-project cache against `workdir`. */ + readonly realLayers?: boolean; readonly format?: OutputFormat; readonly args?: ReadonlyArray; readonly remote?: ReadonlyArray; @@ -109,12 +131,24 @@ function setup(workdir: string, opts: SetupOpts = {}) { promptProjectRef: () => Effect.succeed(VALID_REF), }); + const http = recordingProjectHttpClient(VALID_REF); const layer = Layer.mergeAll( cliConfigValuesTestLayer, out.layer, telemetry.layer, - cache.layer, - resolver, + opts.realLayers === true + ? realLinkedProjectCache({ + httpClient: http.layer, + cliSettings: mockCommandSettings({ workdir }), + }) + : cache.layer, + opts.realLayers === true + ? realDbConfigResolver({ + workdir, + dbConnection: connection, + env: { SUPABASE_ACCESS_TOKEN: VALID_TOKEN }, + }) + : resolver, connection, projectRef, mockCommandSettings({ workdir }), @@ -122,7 +156,7 @@ function setup(workdir: string, opts: SetupOpts = {}) { Layer.succeed(CliArgs, { args: opts.args ?? [] }), BunServices.layer, ); - return { layer, out, telemetry, execs, queries, cache }; + return { layer, http, out, telemetry, execs, queries, cache }; } const flags = (over: Partial = {}): MigrationUpFlags => ({ @@ -320,4 +354,107 @@ describe("migration up", () => { expect(error).toBeInstanceOf(MigrationApplyError); }).pipe(Effect.provide(layer)); }); + + it.effect("mints a login role and fills the cache through the real resolver and cache", () => { + const ctx = setup(tmp.current, { + args: ["migration", "up", "--linked", "--project-ref", VALID_REF], + realLayers: true, + remote: [], + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* seed(tmp.current, "20240101000000_a.sql"); + yield* migrationUp(flags({ linked: true, local: false, projectRef: Option.some(VALID_REF) })); + expect(insertedVersions(ctx.queries)).toEqual(["20240101000000"]); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(ctx.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration up", + activeFlags: { linked: true, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("--linked=false still reaches the linked database through the real resolver", () => { + const ctx = setup(tmp.current, { + args: ["migration", "up", "--linked=false", "--project-ref", VALID_REF], + realLayers: true, + remote: [], + }); + const requests: Array = []; + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* stubManagementApi(requests, serveLinkedDatabase(VALID_REF)); + yield* migrationUp(flags({ linked: false, projectRef: Option.some(VALID_REF) })); + expect(requests.map(({ method, path }) => `${method} ${path}`)).toEqual([ + `GET /v1/projects/${VALID_REF}/config/database/pooler`, + `POST /v1/projects/${VALID_REF}/cli/login-role`, + ]); + expect(ctx.http.calls.map(({ method, url }) => `${method} ${url}`)).toEqual([ + `GET https://api.supabase.com/v1/projects/${VALID_REF}`, + ]); + expect(yield* fs.exists(`${tmp.current}/supabase/.temp/linked-project.json`)).toBe(true); + assertPermissionDrift({ + command: "migration up", + activeFlags: { linked: false, "project-ref": VALID_REF }, + requests: [...asRecordedRequests(requests), ...ctx.http.calls], + exact: true, + }); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); + + it.effect("defaults to the local target and makes no Management API request", () => { + const ctx = setup(tmp.current, { + args: ["migration", "up"], + realLayers: true, + remote: [], + }); + const requests: Array = []; + return Effect.gen(function* () { + yield* stubManagementApi(requests, () => undefined); + yield* seed(tmp.current, "20240101000000_a.sql"); + yield* migrationUp(flags()); + expect(insertedVersions(ctx.queries)).toEqual(["20240101000000"]); + expect(requests).toEqual([]); + expect(ctx.http.calls).toEqual([]); + assertPermissionDrift({ + command: "migration up", + requests: [], + exact: true, + }); + expect(() => + assertPermissionDrift({ + command: "migration up", + requests: [ + { + method: "POST", + url: `https://api.supabase.com/v1/projects/${VALID_REF}/cli/login-role`, + }, + ], + }), + ).toThrow(/condition doesn't hold/); + }).pipe( + Effect.scoped, + Effect.provide(ctx.layer), + provideEnv({ SUPABASE_ACCESS_TOKEN: VALID_TOKEN }), + ); + }); }); diff --git a/apps/cli/src/commands/test/db/db.command.ts b/apps/cli/src/commands/test/db/db.command.ts index ccf1ad2949..4152d96d12 100644 --- a/apps/cli/src/commands/test/db/db.command.ts +++ b/apps/cli/src/commands/test/db/db.command.ts @@ -1,5 +1,8 @@ import { Command } from "effect/cli"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; +import { withDbTarget } from "../../../command-internal/command-permissions/model.ts"; +import { dbConfig } from "../../../command-internal/db-config.layer.ts"; import { TEST_DB_DESCRIPTION, TEST_DB_SHORT, @@ -17,6 +20,7 @@ import { withCliConfigFlags } from "../../../config/cli-config-flags.ts"; export const testDbCommand = Command.make("db", testDbConfig).pipe( Command.withDescription(TEST_DB_DESCRIPTION), Command.withShortDescription(TEST_DB_SHORT), + withPermissions(withDbTarget(dbConfig, { default: "local" })), Command.withHandler(runTestDbCommand), Command.provide(testDbRuntimeLayer(["test", "db"])), Command.provide(cliConfigValuesLayer), diff --git a/apps/cli/src/commands/test/new/new.command.ts b/apps/cli/src/commands/test/new/new.command.ts index 02dcd8f661..bf27416af7 100644 --- a/apps/cli/src/commands/test/new/new.command.ts +++ b/apps/cli/src/commands/test/new/new.command.ts @@ -2,6 +2,7 @@ import { Layer } from "effect"; import { Argument, Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; +import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { commandSettingsLayer } from "../../../config/command-settings.layer.ts"; import { debugLoggerLayer } from "../../../command-internal/debug-logger.layer.ts"; import { telemetryStateLayer } from "../../../telemetry/telemetry-state.layer.ts"; @@ -37,6 +38,7 @@ const testNewRuntimeLayer = Layer.mergeAll( export const testNewCommand = Command.make("new", config).pipe( Command.withDescription("Create a new test file."), Command.withShortDescription("Create a new test file"), + withPermissions({ operations: [], noApiEffectFlags: ["template"] }), Command.withHandler((flags) => testNew(flags).pipe( withCommandTelemetry({ diff --git a/apps/cli/tests/helpers/permission-pending-commands.ts b/apps/cli/tests/helpers/permission-pending-commands.ts index e2260745f0..6fc177ae04 100644 --- a/apps/cli/tests/helpers/permission-pending-commands.ts +++ b/apps/cli/tests/helpers/permission-pending-commands.ts @@ -63,33 +63,6 @@ export const PENDING_COMMANDS: ReadonlyArray = [ "init", - "inspect db bloat", - "inspect db blocking", - "inspect db cache-hit", - "inspect db calls", - "inspect db db-stats", - "inspect db index-sizes", - "inspect db index-stats", - "inspect db index-usage", - "inspect db locks", - "inspect db long-running-queries", - "inspect db outliers", - "inspect db replication-slots", - "inspect db role-configs", - "inspect db role-connections", - "inspect db role-stats", - "inspect db seq-scans", - "inspect db table-index-sizes", - "inspect db table-record-counts", - "inspect db table-sizes", - "inspect db table-stats", - "inspect db total-index-size", - "inspect db total-table-sizes", - "inspect db traffic-profile", - "inspect db unused-indexes", - "inspect db vacuum-stats", - "inspect report", - "issue bug", "issue docs", "issue feature", @@ -100,14 +73,6 @@ export const PENDING_COMMANDS: ReadonlyArray = [ "logout", - "migration down", - "migration fetch", - "migration list", - "migration new", - "migration repair", - "migration squash", - "migration up", - "network-bans get", "network-bans remove", @@ -172,9 +137,6 @@ export const PENDING_COMMANDS: ReadonlyArray = [ "telemetry enable", "telemetry status", - "test db", - "test new", - "unlink", "vanity-subdomains activate", diff --git a/packages/stack/src/HostProcess.integration.test.ts b/packages/stack/src/HostProcess.integration.test.ts index ef55751574..1f618349a2 100644 --- a/packages/stack/src/HostProcess.integration.test.ts +++ b/packages/stack/src/HostProcess.integration.test.ts @@ -24,8 +24,6 @@ import { ChildProcess, ChildProcessSpawner } from "effect/process"; import * as HttpClient from "effect/http/HttpClient"; import * as HttpClientRequest from "effect/http/HttpClientRequest"; import * as FetchHttpClient from "effect/http/FetchHttpClient"; -// oxlint-disable-next-line effecttsgo/node-builtin-import -- reads the spawned owner's own argv. -import { execFileSync } from "node:child_process"; // oxlint-disable-next-line effecttsgo/node-builtin-import -- integration verifies exact-port reopening. import * as Net from "node:net"; import { fileURLToPath } from "node:url"; @@ -89,17 +87,22 @@ const savedStack = ( /** The live command line of a running process, read the POSIX or Windows way. */ const processCommandLine = (pid: number) => - process.platform === "win32" - ? execFileSync( - "powershell", - [ - "-NoProfile", - "-Command", - `(Get-CimInstance Win32_Process -Filter "ProcessId=${pid}").CommandLine`, - ], - { encoding: "utf8" }, - ) - : execFileSync("ps", ["-ww", "-p", String(pid), "-o", "args="], { encoding: "utf8" }); + Effect.gen(function* () { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + return yield* spawner.string( + process.platform === "win32" + ? ChildProcess.make( + "powershell", + [ + "-NoProfile", + "-Command", + `(Get-CimInstance Win32_Process -Filter "ProcessId=${pid}").CommandLine`, + ], + { stdin: "ignore" }, + ) + : ChildProcess.make("ps", ["-ww", "-p", String(pid), "-o", "args="], { stdin: "ignore" }), + ); + }); /** A loopback listener that accepts connections and never answers, counting each one. */ const silentListener = Effect.acquireRelease( @@ -608,13 +611,15 @@ it.live( register: savedStack(root, "argv-secrecy", registerMarker), requestedCreations, }); - yield* Effect.sync(() => { - const argv = processCommandLine(access.endpoint.pid); - expect(argv).not.toContain(marker); - expect(argv).not.toContain(registerMarker); - }).pipe(Effect.ensuring(bestEffortShutdown(root)(access))); + const argv = yield* processCommandLine(access.endpoint.pid).pipe( + Effect.ensuring(bestEffortShutdown(root)(access)), + ); + expect(argv.trim()).not.toBe(""); + expect(argv).not.toContain(marker); + expect(argv).not.toContain(registerMarker); }), ).pipe(Effect.provide(Layer.merge(NodeServices.layer, NodeHttpClient.layerNodeHttp))), + { timeout: 120_000 }, ); it.live("hands the owner a private startup payload file that it deletes once read", () => diff --git a/packages/stack/src/Owner.analytics.integration.test.ts b/packages/stack/src/Owner.analytics.integration.test.ts index f66bb12b8a..2ede6efb82 100644 --- a/packages/stack/src/Owner.analytics.integration.test.ts +++ b/packages/stack/src/Owner.analytics.integration.test.ts @@ -348,7 +348,7 @@ it.live( const storedIds = (ids: ReadonlyArray) => Effect.gen(function* () { const table = yield* eventsTable(internalDatabaseUrl, source); - if (table === undefined) return []; + if (table === undefined) return undefined; const rows = yield* query<{ readonly id: string }>( internalDatabaseUrl, `SELECT id::text AS id FROM ${table} WHERE id IN (${ids.map((id) => `'${id}'`).join(", ")})`, @@ -359,10 +359,10 @@ it.live( const awaitStored = (label: string, id: string, attempts: number) => storedIds([id]).pipe( Effect.filterOrFail( - (rows) => rows.length > 0, - () => + (rows) => rows !== undefined && rows.length > 0, + (rows) => new NotStoredError({ - message: `the ${label} event ${id} was not stored after ${attempts + 1} polls`, + message: `the ${label} event ${id} was not stored after ${attempts + 1} polls: ${rows === undefined ? "no events table" : "no row"}`, }), ), Effect.retry({ schedule: Schedule.spaced("500 millis"), times: attempts }), diff --git a/packages/stack/src/Owner.logs.integration.test.ts b/packages/stack/src/Owner.logs.integration.test.ts index aee8840e38..839290c235 100644 --- a/packages/stack/src/Owner.logs.integration.test.ts +++ b/packages/stack/src/Owner.logs.integration.test.ts @@ -306,6 +306,7 @@ describe("owner persisted logs", () => { }); yield* owner.rpc.startService({ id: mail.id }); yield* owner.rpc.readyService({ id: mail.id }); + yield* firstOutput(owner.rpc.readLogs({ id: mail.id, follow: true })); const history = Array.from( yield* owner.rpc.readLogs({ id: mail.id, follow: false }).pipe(Stream.runCollect), ); diff --git a/packages/stack/src/namespace/drivers/sqlite-pin.ts b/packages/stack/src/namespace/drivers/sqlite-pin.ts index f7d496bede..ad5f5eec6b 100644 --- a/packages/stack/src/namespace/drivers/sqlite-pin.ts +++ b/packages/stack/src/namespace/drivers/sqlite-pin.ts @@ -19,10 +19,10 @@ const ensurePinTable = (connection: DatabaseSync): void => { /** * Takes a SHARED read lock: a deferred `BEGIN` plus a real `SELECT`, held until the connection * commits or closes. Multiple SHARED holders coexist; only a concurrent EXCLUSIVE holder (a - * retirement sweep) contends, surfacing as SQLite's busy errcode under `busy_timeout = 0`. + * retirement sweep) contends, surfacing as SQLite's busy errcode once `busyTimeoutMillis` passes. */ -export const takeSharedLockSync = (connection: DatabaseSync): void => { - connection.exec("PRAGMA busy_timeout = 0"); +export const takeSharedLockSync = (connection: DatabaseSync, busyTimeoutMillis = 0): void => { + connection.exec(`PRAGMA busy_timeout = ${busyTimeoutMillis}`); ensurePinTable(connection); connection.exec("BEGIN"); try { diff --git a/packages/stack/src/runtime/NativeProcess.ts b/packages/stack/src/runtime/NativeProcess.ts index 2e0cf2aeae..184607a5f3 100644 --- a/packages/stack/src/runtime/NativeProcess.ts +++ b/packages/stack/src/runtime/NativeProcess.ts @@ -359,9 +359,25 @@ export const spawnNativeProcess = Effect.fn("NativeProcess.spawn")(function* ( const parsedGroupId = Option.isSome(groupIdLine) ? Number(groupIdLine.value) : Number.NaN; if (!Number.isSafeInteger(parsedGroupId) || parsedGroupId <= 0) { yield* Stream.run(Stream.empty, handle.getInputFd(3)).pipe(Effect.ignore); - yield* handle.exitCode.pipe(Effect.ignore, Effect.timeout("5 seconds"), Effect.ignore); + const stderr = yield* Effect.all( + [ + handle.stderr.pipe( + Stream.decodeText, + Stream.runFold( + () => "", + (text, chunk) => (text + chunk).slice(-4096), + ), + ), + handle.exitCode.pipe(Effect.ignore), + ], + { concurrency: "unbounded" }, + ).pipe( + Effect.map(([text]) => text.trim()), + Effect.timeout("5 seconds"), + Effect.orElseSucceed(() => ""), + ); return yield* new NativeProcessError({ - message: "Native launcher did not report a valid workload process group", + message: `Native launcher did not report a valid workload process group${stderr === "" ? "" : `: ${stderr}`}`, executable: spec.executable, }); } diff --git a/packages/stack/src/runtime/native-launcher.ts b/packages/stack/src/runtime/native-launcher.ts index d5dbe05a82..42c01d5632 100644 --- a/packages/stack/src/runtime/native-launcher.ts +++ b/packages/stack/src/runtime/native-launcher.ts @@ -80,7 +80,7 @@ const decodeSpec = (bytes: Buffer): LaunchSpec | undefined => { const pinArtifactGeneration = (lockPath: string): (() => void) | undefined => { try { const connection = new DatabaseSync(lockPath); - takeSharedLockSync(connection); + takeSharedLockSync(connection, 2_000); // oxlint-disable-next-line effecttsgo/global-date -- this standalone process has no Effect Clock service to source "now" from. const now = new Date(); utimesSync(lockPath, now, now); diff --git a/packages/stack/src/runtime/native-process.integration.test.ts b/packages/stack/src/runtime/native-process.integration.test.ts index da8c8b2c82..3e7e031e74 100644 --- a/packages/stack/src/runtime/native-process.integration.test.ts +++ b/packages/stack/src/runtime/native-process.integration.test.ts @@ -857,7 +857,9 @@ describe("native process group cleanup", () => { if (Exit.isFailure(result)) { const error = Option.getOrUndefined(Cause.findErrorOption(result.cause)); expect(error).toMatchObject({ - message: "Native launcher did not report a valid workload process group", + message: expect.stringMatching( + /^Native launcher did not report a valid workload process group: Native workload failed to start: ENOENT/u, + ), }); } }), diff --git a/packages/stack/src/services/Realtime.ts b/packages/stack/src/services/Realtime.ts index 231b8e3d13..cf5de65e93 100644 --- a/packages/stack/src/services/Realtime.ts +++ b/packages/stack/src/services/Realtime.ts @@ -33,6 +33,18 @@ export const initializationCommand = { containerEntrypoint: "/app/bin/prepare", } satisfies StartupCommand & { readonly containerEntrypoint: string }; +const nativeReadinessOutput: NonNullable["nativeReadinessOutput"]> = ( + line, + endpoints, +) => { + const http = endpoints.get("http"); + if (http === undefined || /\bfailed\b/i.test(line)) return false; + return ( + line.includes("Running RealtimeWeb.Endpoint") && + new RegExp(`:${http.port}\\s+\\(http\\)(?:\\s|$)`).test(line) + ); +}; + export const makeSpec = (): ProcessRecipeSpec => ({ service: "realtime", executable: "bin/server", @@ -88,5 +100,6 @@ export const makeSpec = (): ProcessRecipeSpec => ({ Effect.succeed(context.container ? ["-s", "-g", "--", "/app/bin/server"] : []), mounts: () => Effect.succeed([]), startupCommands: [initializationCommand], + nativeReadinessOutput, containerEntrypoint: () => "/usr/bin/tini", }); diff --git a/packages/stack/src/services/Realtime.unit.test.ts b/packages/stack/src/services/Realtime.unit.test.ts index 534dd0f8ed..bd2edb5d9a 100644 --- a/packages/stack/src/services/Realtime.unit.test.ts +++ b/packages/stack/src/services/Realtime.unit.test.ts @@ -1,5 +1,6 @@ import { expect, it } from "@effect/vitest"; import { Effect } from "effect"; +import type { ServiceEndpoint } from "./Recipe.ts"; import { makeSpec, type Creation } from "./Realtime.ts"; const realtimeEnv = (ipVersion: Creation["config"]["ipVersion"]) => @@ -27,3 +28,31 @@ it.effect("connects to the database over IPv4 whichever family Realtime binds", }); }), ); + +it.effect("requires the selected HTTP port on a successful native endpoint bind line", () => + Effect.sync(() => { + const readinessOutput = makeSpec().nativeReadinessOutput; + const endpoints: ReadonlyMap = new Map([ + ["http", { kind: "tcp" as const, host: "127.0.0.1", port: 4000 }], + ]); + + expect( + readinessOutput?.( + "Running RealtimeWeb.Endpoint with cowboy 2.18.0 at 127.0.0.1:4000 (http)", + endpoints, + ), + ).toBe(true); + expect( + readinessOutput?.( + "Failed to start Ranch listener RealtimeWeb.Endpoint.HTTP in :ranch_tcp:listen([ip: {127, 0, 0, 1}, port: 4000]) for reason :eaddrinuse (address already in use)", + endpoints, + ), + ).toBe(false); + expect( + readinessOutput?.( + "Running RealtimeWeb.Endpoint with cowboy 2.18.0 at 127.0.0.1:40001 (http)", + endpoints, + ), + ).toBe(false); + }), +);