diff --git a/.agents/skills/contribution-triage/SKILL.md b/.agents/skills/contribution-triage/SKILL.md index 21ba3b7a6566a..49a62c5e905e6 100644 --- a/.agents/skills/contribution-triage/SKILL.md +++ b/.agents/skills/contribution-triage/SKILL.md @@ -111,6 +111,8 @@ trusted policy and reassess changed submission evidence; neither example grants Do not automatically close a PR if a maintainer in the trusted `TRIAGE_EXEMPTIONS.td` list has commented or submitted a review, including on earlier heads, or if `triage:keep-open` is present. +Comments that triage itself posted, such as an earlier notice or question, are not maintainer +involvement, even when they were posted from a maintainer's account on that maintainer's behalf. Leave protected PRs open for maintainer decision. This does not grant an author exemption, eligibility, or review approval. @@ -126,7 +128,17 @@ comment text but do not write to GitHub. Keep the eligibility finding separate f This is a routing exception, not a claim that the PR passed eligibility or correctness review. - **Closure warranted.** The assessment is complete and establishes a specific policy violation. Prepare a clear explanation under [closure and reconsideration](../../../CONTRIBUTING.md#closure-and-reconsideration), - post it, and close the PR. Verify that the explanation is present and the PR is closed. The comment + post it, then read the repository variable `PR_LIMITS_ENFORCE` + (`gh variable get PR_LIMITS_ENFORCE -R pingdotgg/t3code`). When it prints `true`, apply + `triage:closing` as the last thing you do to the PR, verify that the explanation and the label + are present, and do not close the PR yourself: + [pr-limits.yml](../../../.github/workflows/pr-limits.yml) closes it 72 hours after the label. Any + activity on the PR after the label, including a further comment or label from you, removes the + label; treat an author's reply or push as new evidence, reassess, and apply the label again only + if the finding still holds. When the command prints any other value, or fails saying the variable + was not found, the variable is off and nothing would close a labeled PR: close it yourself and + verify that the explanation is present and the PR is closed. Only another failure, such as a + permission error, means the variable cannot be read; then report the closure as incomplete. The comment must name the violated rule, cite supporting submission evidence, link the maintained guide section, and give a concrete remedy. Missing contributor evidence can warrant closure; explain what must be established. Missing product approval calls for maintainer discussion, not an agent's product decision. @@ -173,9 +185,10 @@ description, evidence, approvals, existing triage comments, PR open/closed state label when relevant. Reassess changes that could invalidate the finding. Reuse an existing explanation only if it still matches the current finding; avoid duplicate comments, closures, or label applications. -For closure, establish that the required explanation is posted before closing. If posting fails or its -result is ambiguous, read back the comments before retrying or proceeding. If the comment succeeds but -closure fails, preserve the comment and retry only the unfinished closure after checking current state. +For closure, establish that the required explanation is posted before applying `triage:closing` or +closing. If posting fails or its result is ambiguous, read back the comments before retrying or +proceeding. If the comment succeeds but the label or the closure fails, preserve the comment and retry +only that unfinished step after checking current state. Handle a failed or ambiguous label application the same way: inspect labels before any retry. If access or state still cannot be verified, report an incomplete action and the step needed to resume. Do not retry blindly or claim success for an unverified action. diff --git a/.github/fork/upstream-tag b/.github/fork/upstream-tag index b9875402aa8ac..b60052e304c1c 100644 --- a/.github/fork/upstream-tag +++ b/.github/fork/upstream-tag @@ -1 +1 @@ -v0.1.9-nightly.20261111.4515 +v0.1.10-nightly.20261112.4564 diff --git a/.github/scripts/pr-limits.cjs b/.github/scripts/pr-limits.cjs new file mode 100644 index 0000000000000..77c09785e4104 --- /dev/null +++ b/.github/scripts/pr-limits.cjs @@ -0,0 +1,320 @@ +const fs = require("node:fs"); + +const HOUR_MS = 60 * 60 * 1000; +const OPEN_LIMIT = 3; +const CONFLICT_IDLE_MS = 7 * 24 * HOUR_MS; +const NOTICE_MS = 72 * HOUR_MS; +// A first run meets the whole backlog. These keep one run from flooding watchers or GitHub's +// limit on how fast a token may comment. +const MAX_REQUESTS_PER_RUN = 100; +const MAX_CLOSES_PER_RUN = 50; +// A last look that ends in "not today" closes nothing, so the looks need a cap of their own. +const MAX_RECHECKS_PER_RUN = 150; +const WRITE_PAUSE_MS = 1_000; +// Measured here: about two and a half minutes until every open PR has an answer. +const MERGEABLE_WAIT_MS = 150_000; +const READ_ATTEMPTS = 4; +const READ_RETRY_MS = 5_000; +// GitHub stamps a label event and the PR's updatedAt a second or two apart. +const SAME_MOMENT_MS = 15_000; + +const REBASE_LABEL = "needs-rebase"; +const CLOSING_LABEL = "triage:closing"; +const KEEP_OPEN_LABEL = "triage:keep-open"; +const UNVOUCHED_LABEL = "vouch:unvouched"; +const WRITE_PERMISSIONS = new Set(["admin", "maintain", "write"]); +const EXEMPTIONS_PATH = ".github/TRIAGE_EXEMPTIONS.td"; +const VOUCHED_PATH = ".github/VOUCHED.td"; +const LOGIN_PATTERN = /^github:([A-Za-z0-9](?:[A-Za-z0-9]|-(?=[A-Za-z0-9])){0,38})$/; + +// `labels` matches a PR that carries any one of them. +const SWEEP_QUERY = ` + query ($owner: String!, $repo: String!, $labels: [String!], $cursor: String) { + repository(owner: $owner, name: $repo) { + pullRequests(states: OPEN, labels: $labels, first: 50, after: $cursor) { + pageInfo { hasNextPage endCursor } + nodes { + number + mergeable + baseRefName + updatedAt + author { login } + labels(first: 100) { nodes { name } } + timelineItems(itemTypes: [LABELED_EVENT], last: 20) { + nodes { ... on LabeledEvent { createdAt label { name } } } + } + } + } + } + }`; + +const RECHECK_QUERY = ` + query ($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + state + mergeable + updatedAt + labels(first: 100) { nodes { name } } + } + } + }`; + +const AUTHOR_OPEN_PRS_QUERY = ` + query ($q: String!) { + search(query: $q, type: ISSUE, first: 50) { + nodes { ... on PullRequest { number } } + } + }`; + +const pause = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +// GitHub answers a heavy list request with a 502 now and then, and one of those must not end the +// run. Only reads go through here: repeating a read is safe, repeating a comment is not. +async function read(github, query, variables) { + for (let attempt = 1; ; attempt += 1) { + try { + return await github.graphql(query, variables); + } catch (error) { + const transient = typeof error.status === "number" && error.status >= 500; + if (attempt === READ_ATTEMPTS || !transient) throw error; + await pause(attempt * READ_RETRY_MS); + } + } +} + +// Fails closed: a missing, empty, or malformed list must never read as "nobody is exempt". +function parseExemptions(text) { + const logins = new Set(); + for (const raw of text.split("\n")) { + const line = raw.trim(); + if (line === "" || line.startsWith("#")) continue; + const match = LOGIN_PATTERN.exec(line); + if (!match) throw new Error(`${EXEMPTIONS_PATH}: malformed entry "${line}"`); + const login = match[1].toLowerCase(); + if (logins.has(login)) throw new Error(`${EXEMPTIONS_PATH}: duplicate entry "${line}"`); + logins.add(login); + } + if (logins.size === 0) throw new Error(`${EXEMPTIONS_PATH}: no entries`); + return logins; +} + +// VOUCHED.td: one `github:login` per line, `#` comments, and `-github:login reason` to denounce. +function vouchedLogins(text) { + const logins = new Set(); + for (const raw of text.split("\n")) { + const entry = raw.trim().split(/\s+/)[0]; + if (entry.startsWith("github:")) logins.add(entry.slice("github:".length).toLowerCase()); + } + if (logins.size === 0) throw new Error(`${VOUCHED_PATH}: no entries`); + return logins; +} + +function labeledAt(pr, name) { + const last = pr.timelineItems.nodes.filter((event) => event.label?.name === name).at(-1); + return last ? Date.parse(last.createdAt) : undefined; +} + +// The whole policy for the two windows. Activity is anything that moves the PR's updatedAt: a +// push, a comment, a review, a label. Nobody can set that time, so nothing here trusts the author. +function decide(pr, exemptions, now) { + const labels = new Set(pr.labels.nodes.map((label) => label.name)); + if (labels.has(KEEP_OPEN_LABEL)) return "skip"; + const updatedAt = Date.parse(pr.updatedAt); + + if (labels.has(CLOSING_LABEL)) { + const since = labeledAt(pr, CLOSING_LABEL); + if (since === undefined) return "unknown-notice"; + // Any activity after the notice sends the PR back to triage. + if (updatedAt - since > SAME_MOMENT_MS) return "clear-notice"; + return now - since >= NOTICE_MS ? "close-notice" : "skip"; + } + + const exempt = exemptions.has((pr.author?.login ?? "").toLowerCase()); + const subject = labels.has(UNVOUCHED_LABEL) && !exempt; + if (labels.has(REBASE_LABEL)) { + if (!subject || pr.mergeable === "MERGEABLE") return "clear-rebase"; + // Activity restarts the seven days and keeps the label, so a PR is asked to rebase once. + const idle = now - updatedAt >= CONFLICT_IDLE_MS; + return idle && pr.mergeable === "CONFLICTING" ? "close-conflict" : "skip"; + } + return subject && pr.mergeable === "CONFLICTING" ? "request-rebase" : "skip"; +} + +function guideLink(context, anchor) { + return `https://github.com/${context.repo.owner}/${context.repo.repo}/blob/main/CONTRIBUTING.md#${anchor}`; +} + +async function removeLabel(github, context, number, name) { + try { + await github.rest.issues.removeLabel({ ...context.repo, issue_number: number, name }); + } catch (error) { + if (error.status !== 404) throw error; + } +} + +async function closeWithComment(github, context, number, body) { + await github.rest.issues.createComment({ ...context.repo, issue_number: number, body }); + await github.rest.pulls.update({ ...context.repo, pull_number: number, state: "closed" }); +} + +async function sweep({ github, context, core, dryRun }) { + const exemptions = parseExemptions(fs.readFileSync(EXEMPTIONS_PATH, "utf8")); + const repo = { owner: context.repo.owner, repo: context.repo.repo }; + + const list = async () => { + const prs = []; + for (let cursor = null; ;) { + const page = ( + await read(github, SWEEP_QUERY, { + ...repo, + cursor, + labels: [UNVOUCHED_LABEL, REBASE_LABEL, CLOSING_LABEL], + }) + ).repository.pullRequests; + prs.push(...page.nodes); + if (!page.pageInfo.hasNextPage) return prs; + cursor = page.pageInfo.endCursor; + } + }; + // GitHub computes mergeability lazily and forgets it whenever the base moves. Asking starts the + // computation, so ask once more. A PR that is still unknown waits for tomorrow's run. + let prs = await list(); + if (prs.some((pr) => pr.mergeable === "UNKNOWN")) { + await pause(MERGEABLE_WAIT_MS); + prs = await list(); + } + + const now = Date.now(); + // Start further down the list each day. The step is no larger than a run's cap, so a cap that + // is reached cannot leave the same PRs waiting behind the same others every day. + const start = (Math.floor(now / (24 * HOUR_MS)) * MAX_RECHECKS_PER_RUN) % (prs.length || 1); + prs = [...prs.slice(start), ...prs.slice(0, start)]; + const count = {}; + let rechecks = 0; + for (const pr of prs) { + const { number } = pr; + let action = decide(pr, exemptions, now); + if (action === "skip") continue; + if (action === "unknown-notice") { + core.warning(`#${number}: cannot tell when ${CLOSING_LABEL} was applied, leaving it open.`); + continue; + } + if (action === "request-rebase" && (count[action] ?? 0) >= MAX_REQUESTS_PER_RUN) continue; + if (action.startsWith("close")) { + if ((count["close-conflict"] ?? 0) + (count["close-notice"] ?? 0) >= MAX_CLOSES_PER_RUN) { + continue; + } + if (rechecks >= MAX_RECHECKS_PER_RUN) continue; + rechecks += 1; + // The list is minutes old by now. Closing is the one step worth a second look. + const fresh = (await read(github, RECHECK_QUERY, { ...repo, number })).repository.pullRequest; + if (fresh.state !== "OPEN") continue; + // A merge to the base can resolve a conflict without touching the PR, so this is re-read + // too. An answer that is not ready yet reads as "not conflicting" and waits for tomorrow. + action = decide({ ...pr, ...fresh }, exemptions, now); + if (!action.startsWith("close")) continue; + } + + count[action] = (count[action] ?? 0) + 1; + core.info(`#${number} ${action}${dryRun ? " (dry run)" : ""}`); + if (dryRun) continue; + + if (action === "request-rebase") { + // The request goes first: a label without it would start a clock nobody was told about. + await github.rest.issues.createComment({ + ...context.repo, + issue_number: number, + body: [ + `This PR conflicts with \`${pr.baseRefName}\`. Rebase it or merge \`${pr.baseRefName}\` into it. If it still conflicts after 7 days with no activity, it will be closed automatically.`, + "", + `See the [conflict window](${guideLink(context, "conflict-window")}).`, + ].join("\n"), + }); + await github.rest.issues.addLabels({ + ...context.repo, + issue_number: number, + labels: [REBASE_LABEL], + }); + } else if (action === "clear-rebase") { + await removeLabel(github, context, number, REBASE_LABEL); + } else if (action === "clear-notice") { + await removeLabel(github, context, number, CLOSING_LABEL); + } else { + const conflict = action === "close-conflict"; + await closeWithComment( + github, + context, + number, + conflict + ? [ + `Closing: this PR still conflicts with \`${pr.baseRefName}\` and has had no activity for 7 days since the rebase request.`, + "", + `To continue, rebase the branch and open a new PR that links this one. See the [conflict window](${guideLink(context, "conflict-window")}).`, + ].join("\n") + : [ + "Closing: 72 hours have passed since the triage notice above with no activity on this PR.", + "", + `To ask for reconsideration, address the notice and comment here, or open a focused replacement PR that links this one. See [closure and reconsideration](${guideLink(context, "closure-and-reconsideration")}).`, + ].join("\n"), + ); + // Without this a maintainer's reopen would be closed again by the next run. + await removeLabel(github, context, number, conflict ? REBASE_LABEL : CLOSING_LABEL); + } + await pause(WRITE_PAUSE_MS); + } + + const summary = Object.entries(count).map(([action, n]) => `${n} ${action}`); + core.info( + `${dryRun ? "Dry run. " : ""}${prs.length} PRs read: ${summary.join(", ") || "nothing to do"}.`, + ); + return count; +} + +async function enforceOpenLimit({ github, context, core, dryRun }) { + const pr = context.payload.pull_request; + const author = pr.user.login; + // Who is trusted is decided here rather than by the vouch action, which reports its own failed + // lookups as "not vouched". Every read below throws on failure, and a throw closes nothing. + if (pr.user.type === "Bot") return; + if (parseExemptions(fs.readFileSync(EXEMPTIONS_PATH, "utf8")).has(author.toLowerCase())) return; + if (vouchedLogins(fs.readFileSync(VOUCHED_PATH, "utf8")).has(author.toLowerCase())) return; + const { data: access } = await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: author, + }); + if (WRITE_PERMISSIONS.has(access.permission)) return; + if (pr.labels.some((label) => label.name === KEEP_OPEN_LABEL)) return; + // Someone else reopening a PR is a maintainer overriding the limit. + const reopened = context.payload.action === "reopened"; + if (reopened && context.payload.sender.login.toLowerCase() !== author.toLowerCase()) return; + + const q = `repo:${context.repo.owner}/${context.repo.repo} is:pr is:open author:${author} sort:created-asc`; + const { search } = await read(github, AUTHOR_OPEN_PRS_QUERY, { q }); + // A new PR counts only the older ones, so two opened together cannot close each other. + const others = search.nodes.filter( + (node) => node.number && (reopened ? node.number !== pr.number : node.number < pr.number), + ).length; + if (others < OPEN_LIMIT) { + core.info(`PR #${pr.number}: ${author} has ${others} other open PRs.`); + return; + } + if (dryRun) { + core.notice(`Dry run. PR #${pr.number} would close: ${author} has ${others} other open PRs.`); + return; + } + + await closeWithComment( + github, + context, + pr.number, + [ + `Closing: ${author} already has ${OPEN_LIMIT} or more open PRs here, which is the limit for contributors who are not vouched.`, + "", + `Once one of them is merged or closed, open this again. See the [open PR limit](${guideLink(context, "open-pr-limit")}).`, + ].join("\n"), + ); + core.info(`PR #${pr.number}: closed, ${author} has ${others} other open PRs.`); +} + +module.exports = { decide, enforceOpenLimit, parseExemptions, sweep }; diff --git a/.github/scripts/test-delta.cjs b/.github/scripts/test-delta.cjs new file mode 100644 index 0000000000000..c9789f644d272 --- /dev/null +++ b/.github/scripts/test-delta.cjs @@ -0,0 +1,78 @@ +// Counts the test cases a pull request adds and removes, from a zero-context diff, so PR Size can +// say when a change grows the suite. Counting declarations keeps the number deterministic, unlike +// CI timings, which move with runner load. +const MARKER = ""; +const NET_CASES_TO_REPORT = 20; +const TEST_FILE = /\.(?:test|spec)\.[cm]?[jt]sx?$/; +// `<` admits type arguments, as in `it.each(...)`, which may span lines. +const TEST_CASE = /^\s*(?:it|test)(?:\.\w+)*\s*[(<]/; +const isBrowserTest = (path) => path.includes(".browser.test."); + +function kindOf(path) { + if (isBrowserTest(path)) return "browser"; + if (path.startsWith("apps/server/") || path.includes(".integration.")) return "server"; + return "unit"; +} + +/** @param {string} diff output of `git diff -U0 base...head` */ +function countTestDelta(diff) { + const kinds = { browser: 0, server: 0, unit: 0 }; + const newBrowserFiles = []; + let path = null; + // A browser test file is new when it is added, or renamed from a file that was not one. + let file = null; + const flush = () => { + if (file?.isNew && isBrowserTest(file.target)) newBrowserFiles.push(file.target); + }; + for (const line of diff.split("\n")) { + if (line.startsWith("diff --git ")) { + flush(); + path = null; + file = { target: line.replace(/^diff --git a\/\S+ b\//, ""), isNew: false }; + continue; + } + if (line.startsWith("new file mode") && file) file.isNew = true; + if (line.startsWith("rename from ") && file) file.isNew = !isBrowserTest(line.slice(12)); + if (line.startsWith("--- ")) { + const source = line.slice(4).replace(/^a\//, ""); + path = TEST_FILE.test(source) ? source : null; + continue; + } + if (line.startsWith("+++ ")) { + const target = line.slice(4).replace(/^b\//, ""); + // A deleted file's target is /dev/null; keep its old path to count the removed cases. + if (target !== "/dev/null") path = TEST_FILE.test(target) ? target : null; + continue; + } + if (line.startsWith("new file mode") || !path) continue; + const sign = line[0] === "+" ? 1 : line[0] === "-" ? -1 : 0; + if (sign !== 0 && TEST_CASE.test(line.slice(1))) kinds[kindOf(path)] += sign; + } + flush(); + return { kinds, net: kinds.browser + kinds.server + kinds.unit, newBrowserFiles }; +} + +/** Returns the comment body, or null when the change does not grow the suite enough to mention. */ +function render(delta) { + if (delta.net < NET_CASES_TO_REPORT && delta.newBrowserFiles.length === 0) return null; + const signed = (n) => (n > 0 ? `+${n}` : String(n)); + const lines = [ + MARKER, + `This PR changes the test suite by **${signed(delta.net)} test cases** ` + + `(browser ${signed(delta.kinds.browser)}, unit ${signed(delta.kinds.unit)}, server ${signed(delta.kinds.server)}).`, + ]; + if (delta.newBrowserFiles.length > 0) { + lines.push( + "", + `New browser test files: ${delta.newBrowserFiles.map((file) => `\`${file}\``).join(", ")}.`, + "Each browser test file costs CI seconds of module loading before its first test runs; a unit test costs milliseconds.", + ); + } + lines.push( + "", + "Keep tests that would fail for a real reason, and test logic below the UI where it can be. See [Testing](https://github.com/pingdotgg/t3code/blob/main/docs/internals/testing.md).", + ); + return lines.join("\n"); +} + +module.exports = { MARKER, countTestDelta, render }; diff --git a/.github/scripts/test-delta.test.cjs b/.github/scripts/test-delta.test.cjs new file mode 100644 index 0000000000000..82a3e00862d2f --- /dev/null +++ b/.github/scripts/test-delta.test.cjs @@ -0,0 +1,59 @@ +const assert = require("node:assert/strict"); +const test = require("node:test"); + +const { countTestDelta, render } = require("./test-delta.cjs"); + +const diff = `diff --git a/apps/web/src/Panel.browser.test.tsx b/apps/web/src/Panel.browser.test.tsx +new file mode 100644 +--- /dev/null ++++ b/apps/web/src/Panel.browser.test.tsx +@@ -0,0 +1,4 @@ ++it("opens", async () => {}); ++ it.each<{ n: number }>([{ n: 1 }])("renders $n", async () => {}); ++// it("is a comment, still counted only when it starts the line") +diff --git a/apps/server/src/thing.test.ts b/apps/server/src/thing.test.ts +--- a/apps/server/src/thing.test.ts ++++ b/apps/server/src/thing.test.ts +@@ -3,2 +3,1 @@ +-it.effect("old", () => {}); +-test("older", () => {}); ++ it.effect("new", () => {}); +diff --git a/packages/shared/src/gone.test.ts b/packages/shared/src/gone.test.ts +deleted file mode 100644 +--- a/packages/shared/src/gone.test.ts ++++ /dev/null +@@ -1,1 +0,0 @@ +-it("goes", () => {}); +diff --git a/apps/web/src/Menu.test.tsx b/apps/web/src/Menu.browser.test.tsx +similarity index 100% +rename from apps/web/src/Menu.test.tsx +rename to apps/web/src/Menu.browser.test.tsx +diff --git a/apps/web/src/Old.browser.test.tsx b/apps/web/src/New.browser.test.tsx +similarity index 100% +rename from apps/web/src/Old.browser.test.tsx +rename to apps/web/src/New.browser.test.tsx +diff --git a/apps/web/src/Panel.tsx b/apps/web/src/Panel.tsx +--- a/apps/web/src/Panel.tsx ++++ b/apps/web/src/Panel.tsx +@@ -1 +1 @@ ++it("is not a test file", () => {}); +`; + +test("counts cases by kind, including deleted files, and finds new or renamed browser files", () => { + assert.deepEqual(countTestDelta(diff), { + kinds: { browser: 2, server: -1, unit: -1 }, + net: 0, + newBrowserFiles: ["apps/web/src/Panel.browser.test.tsx", "apps/web/src/Menu.browser.test.tsx"], + }); +}); + +test("reports only growth worth a reviewer's attention", () => { + const quiet = { kinds: { browser: 0, server: 3, unit: 5 }, net: 8, newBrowserFiles: [] }; + assert.equal(render(quiet), null); + const growth = { kinds: { browser: 4, server: 10, unit: 12 }, net: 26, newBrowserFiles: [] }; + assert.match(render(growth), /\*\*\+26 test cases\*\* \(browser \+4, unit \+12, server \+10\)/); + assert.match( + render(countTestDelta(diff)), + /New browser test files: `apps\/web\/src\/Panel\.browser\.test\.tsx`/, + ); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5343dbb754ff8..6f75c92562d03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -326,7 +326,7 @@ jobs: run: python3 -B .github/scripts/stage-preview-bundle.test.py - name: Test release checks - run: node --test .github/scripts/check-nightly-release.test.cjs .github/scripts/vercel-deployment-output.test.cjs .github/scripts/mobile-preview-comment.test.cjs + run: node --test .github/scripts/check-nightly-release.test.cjs .github/scripts/vercel-deployment-output.test.cjs .github/scripts/mobile-preview-comment.test.cjs .github/scripts/test-delta.test.cjs - name: Finish installing browser secret helper build libraries timeout-minutes: 5 @@ -355,16 +355,22 @@ jobs: # The web suite is the largest outside the server. In the shared Test job it # started last and ran alone for about 40s after everything else finished. - # release.yml mirrors this test, test_web, and test_server split. + # Browser tests are bound by the Vite server transforming modules for each + # file, not by Chromium, so a second runner (and Vite server) halves the job + # that gated every run. release.yml mirrors this test, test_web, and test_server split. test_web: permissions: contents: read id-token: write - name: Test Web + name: Test Web ${{ matrix.shard }} needs: changes if: ${{ !cancelled() && needs.changes.outputs.test_web != 'false' }} runs-on: blacksmith-8vcpu-ubuntu-2404 timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + shard: [1, 2] steps: - name: Checkout uses: actions/checkout@v6 @@ -406,7 +412,7 @@ jobs: fi - name: Test - run: vp run --filter @t3tools/web test + run: vp run --filter @t3tools/web test --shard ${{ matrix.shard }}/${{ strategy.job-total }} # apps/server sets `fileParallelism: false`, so its test files run strictly # one at a time. Sharding spreads them over separate runners instead of @@ -481,6 +487,17 @@ jobs: node apps/server/node_modules/playwright-core/cli.js install-deps chromium fi + # Each server test file runs in its own Vitest process, and a fresh install starts + # Vitest's content-keyed module cache empty, so every file re-transformed its imports. + # Only pushes to main save it, so pull requests never evict each other's entries. + - name: Restore Vitest module cache + if: steps.changes.outputs.test_server != 'false' + uses: actions/cache/restore@v6 + with: + path: node_modules/.vitest-cache + key: vitest-modules-server-${{ matrix.shard }}-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-${{ github.sha }} + restore-keys: vitest-modules-server-${{ matrix.shard }}-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}- + - name: Test if: steps.changes.outputs.test_server != 'false' env: @@ -489,15 +506,14 @@ jobs: run: >- vp run test:cached -- --shard ${{ matrix.shard }}/${{ strategy.job-total }} --uncached apps/server/integration/transferBudgetV2.integration.test.ts - --verify-hits 1 --report "${{ runner.temp }}/test-cache-${{ matrix.shard }}.json" apps/server + --verify-hits 1 apps/server - - name: Upload test cache measurements - if: always() && steps.changes.outputs.test_server != 'false' - uses: actions/upload-artifact@v7 + - name: Save Vitest module cache + if: github.event_name == 'push' && steps.changes.outputs.test_server != 'false' + uses: actions/cache/save@v6 with: - name: test-cache-${{ matrix.shard }} - path: ${{ runner.temp }}/test-cache-${{ matrix.shard }}.json - if-no-files-found: error + path: node_modules/.vitest-cache + key: vitest-modules-server-${{ matrix.shard }}-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-${{ github.sha }} # integration/transferBudgetV2.integration.test.ts writes the report; exactly one shard # produces these files. Gating the upload on their presence keeps a diff --git a/.github/workflows/pr-limits.yml b/.github/workflows/pr-limits.yml new file mode 100644 index 0000000000000..75943062735cd --- /dev/null +++ b/.github/workflows/pr-limits.yml @@ -0,0 +1,72 @@ +name: PR Limits + +# Deterministic PR rules from CONTRIBUTING.md: the open PR limit, the conflict +# window, and the 72 hour close window that follows a triage notice. +# +# Nothing is labeled, commented on, or closed until the repository variable +# PR_LIMITS_ENFORCE is "true". Until then every run logs what it would do. A +# sweep reads the variable once when it starts; to stop one, cancel the run. + +on: + pull_request_target: + types: [opened, reopened] + schedule: + - cron: "23 5 * * *" + workflow_dispatch: + inputs: + dry_run: + description: Log what the sweep would do without labeling, commenting, or closing. + type: boolean + default: true + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + open-limit: + name: Open PR limit + if: github.event_name == 'pull_request_target' + runs-on: ubuntu-24.04 + steps: + # pull_request_target has a write token. Load the script from the default + # branch and never run PR code. Cone mode also checks out the files beside + # .github/scripts, which is where TRIAGE_EXEMPTIONS.td and VOUCHED.td live. + - name: Checkout trusted scripts + uses: actions/checkout@v6 + with: + ref: ${{ github.event.repository.default_branch }} + sparse-checkout: .github/scripts + + - name: Close PRs over the limit + uses: actions/github-script@v8 + env: + DRY_RUN: ${{ vars.PR_LIMITS_ENFORCE != 'true' }} + with: + script: | + const limits = require("./.github/scripts/pr-limits.cjs"); + await limits.enforceOpenLimit({ github, context, core, dryRun: process.env.DRY_RUN === "true" }); + + sweep: + name: Conflict and close windows + if: github.event_name != 'pull_request_target' + runs-on: ubuntu-24.04 + concurrency: + group: pr-limits-sweep + cancel-in-progress: false + steps: + - name: Checkout trusted scripts + uses: actions/checkout@v6 + with: + ref: ${{ github.event.repository.default_branch }} + sparse-checkout: .github/scripts + + - name: Request rebases and close what went unanswered + uses: actions/github-script@v8 + env: + DRY_RUN: ${{ vars.PR_LIMITS_ENFORCE != 'true' || (github.event_name == 'workflow_dispatch' && inputs.dry_run) }} + with: + script: | + const limits = require("./.github/scripts/pr-limits.cjs"); + await limits.sweep({ github, context, core, dryRun: process.env.DRY_RUN === "true" }); diff --git a/.github/workflows/pr-size.yml b/.github/workflows/pr-size.yml index af557dff62df8..cb0c9637fcef7 100644 --- a/.github/workflows/pr-size.yml +++ b/.github/workflows/pr-size.yml @@ -293,3 +293,55 @@ jobs: core.info( `PR #${issueNumber}: ${nonTestChangedLines} non-test lines, ${testChangedLines} test lines, ${changedLines} effective lines -> ${nextLabelName} (${classification})`, ); + # Reads the script from the base checkout above; the PR's own commits are only diffed. + - name: Report test suite growth + uses: actions/github-script@v8 + with: + script: | + const { execFileSync } = require("node:child_process"); + const { MARKER, countTestDelta, render } = require( + `${process.env.GITHUB_WORKSPACE}/.github/scripts/test-delta.cjs`, + ); + + const issueNumber = context.payload.pull_request.number; + const diff = execFileSync( + "git", + ["diff", "-U0", `${context.payload.pull_request.base.sha}...refs/remotes/pr-size/${issueNumber}`], + { encoding: "utf8", maxBuffer: 256 * 1024 * 1024 }, + ); + const delta = countTestDelta(diff); + const body = render(delta); + core.info(`PR #${issueNumber}: test cases ${JSON.stringify(delta.kinds)}, net ${delta.net}`); + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber, + per_page: 100, + }); + const existing = comments.find( + (comment) => comment.user?.type === "Bot" && comment.body?.startsWith(MARKER), + ); + if (body && existing) { + if (existing.body !== body) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } + } else if (body) { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber, + body, + }); + } else if (existing) { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + }); + } diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c56c76be7e0e7..8827876b3829f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -300,11 +300,15 @@ jobs: # Web browser tests time out when they share a runner with the other package # suites: Chromium gets too little CPU and diff workers miss their 10s start budget. test_web: - name: Release web tests + name: Release web tests ${{ matrix.shard }} needs: [preflight] if: ${{ !failure() && !cancelled() && needs.preflight.result == 'success' }} runs-on: blacksmith-8vcpu-ubuntu-2404 timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + shard: [1, 2] steps: - name: Checkout uses: actions/checkout@v6 @@ -344,10 +348,11 @@ jobs: fi - name: Test - run: vp run --filter @t3tools/web test + run: vp run --filter @t3tools/web test --shard ${{ matrix.shard }}/${{ strategy.job-total }} # Browser tests run source modules; this boots the production chunk graph. - name: Boot the built web client + if: matrix.shard == 1 run: | vp run --filter @t3tools/web build node apps/web/scripts/verify-built-boot.ts @@ -1482,7 +1487,18 @@ jobs: git add apps/server/package.json apps/desktop/package.json apps/web/package.json packages/contracts/package.json pnpm-lock.yaml git commit --no-verify -m "chore(release): prepare $RELEASE_TAG" - git push origin HEAD:main + + # main moves while this job runs, so a rejected push rebases and retries. + for attempt in 1 2 3 4 5; do + if git push origin HEAD:main; then + exit 0 + fi + echo "Push rejected (attempt $attempt). Rebasing onto latest main." + git pull --rebase origin main + done + + echo "Could not push the version bump after 5 attempts." >&2 + exit 1 announce_discord: name: Announce release on Discord diff --git a/.macroscope/check-run-agents/ui-consistency.md b/.macroscope/check-run-agents/ui-consistency.md index 905823d1d1a9e..4326d4e8391b7 100644 --- a/.macroscope/check-run-agents/ui-consistency.md +++ b/.macroscope/check-run-agents/ui-consistency.md @@ -44,7 +44,7 @@ The reference for how this codebase wants UI built is the composer banner system - `ComposerBanner.tsx` exports one object of small slot components (`Root`, `Row`, `Icon`, `Content`, `Actions`, `Dismiss`, `Children`, and so on). Each slot owns its own Tailwind classes, exposes a `data-slot` attribute, takes `className` and spreads the rest of its props, and uses `cn` so callers can adjust without overriding. Variants and density are props on `Root`, not ad hoc class strings at call sites. Where a slot needs a real control it renders `Button` from `ui/` rather than a styled `