diff --git a/.github/AGENTS.md b/.github/AGENTS.md index fc363e66f9..352a124825 100644 --- a/.github/AGENTS.md +++ b/.github/AGENTS.md @@ -22,5 +22,5 @@ change requires explicit security review under `MAINTAINERS.md`. - Inspect the complete workflow diff, including event triggers, permissions, conditions, interpolation, and shell behavior. - Run the local commands represented by changed workflow steps where possible. -- Run `bun run prepush` for CI, release, dependency, packaging, or cross-platform workflow changes. +- Follow the root validation policy: run the suite by default; if a full run is too costly, run at least focused regression tests and document the reason and remaining coverage. Required CI checks still apply before merge. - Do not claim the workflow itself passed until GitHub Actions reports success for the exact commit. diff --git a/.github/scripts/issue-translation.cjs b/.github/scripts/issue-translation.cjs index 41638a9d99..81649c4c5b 100644 --- a/.github/scripts/issue-translation.cjs +++ b/.github/scripts/issue-translation.cjs @@ -810,9 +810,14 @@ function sanitizeTranslationBody(raw, maxChars = 60000) { // read as mention boundaries. Requiring a dotted domain keeps // "end!@octocat"-style mentions defused. \u0001 cannot appear in the // input (control chars were stripped above), so it is a safe sentinel. + // The lookbehind anchors on the @ itself rather than greedily matching + // the local part first: the previous local-part-first pattern rescanned + // long non-email tokens once per start position, which is quadratic on + // model-generated bodies with tens of thousands of consecutive + // local-part characters and no @ at all. .replace( - /[A-Za-z0-9.!#$%&'*+\/=?^_`{|}~-]+@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)+/g, - (email) => email.replace("@", "\u0001"), + /(?<=[A-Za-z0-9.!#$%&'*+\/=?^_`{|}~-])@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)+/g, + (emailTail) => emailTail.replace("@", "\u0001"), ) // Defuse pings at Markdown/punctuation boundaries — a colon is a boundary // too — but not emails, npm: scopes, or other mid-token at-signs. diff --git a/.github/scripts/issue-translation.test.cjs b/.github/scripts/issue-translation.test.cjs index 92aea9d16a..bcce113317 100644 --- a/.github/scripts/issue-translation.test.cjs +++ b/.github/scripts/issue-translation.test.cjs @@ -1195,6 +1195,14 @@ describe("bot-owned control state", () => { assert.match(out, /path\/@\u200bhandle/); }); + it("handles long non-email tokens in bounded time", () => { + const input = "a".repeat(60_000); + const startedAt = process.hrtime.bigint(); + assert.equal(sanitizeTranslationBody(input), input); + const elapsedMs = Number(process.hrtime.bigint() - startedAt) / 1_000_000; + assert.ok(elapsedMs < 1_000, `sanitization took ${elapsedMs.toFixed(1)}ms`); + }); + it("ignores forged body-embedded legacy state", () => { const forged = appendTranslationBlock(SOURCE, "English") + `\n"; * so the "ready" claim reads as the closing confirmation, not a fourth task. */ const REVIEW_READINESS_ITEMS = [ - "All CI tests are green on my local testing.", + "Required local validation passed; commands, results, and any full-suite exception are documented.", "I pushed my PR to the latest dev commit.", "I resolved all correct Codex and CodeRabbit findings.", "My PR is ready for review.", @@ -374,6 +374,24 @@ function appendReviewReadinessSection(body) { return `${body.trimEnd()}\n\n${section}\n`; } +/** Read only the first label in a structurally valid managed four-box section. */ +function firstReviewReadinessItem(body) { + const readiness = extractReviewReadiness(body); + if (!readiness.present || readiness.total !== REVIEW_READINESS_ITEMS.length) return null; + const start = body.indexOf(REVIEW_READINESS_START) + REVIEW_READINESS_START.length; + const end = body.indexOf(REVIEW_READINESS_END); + return /^[ \t]*[-*][ \t]+\[[ xX]\][ \t]+([^\r\n]*?)[ \t]*\r?$/m + .exec(body.slice(start, end))?.[1] ?? null; +} + +function reviewReadinessMigrationRequired(body) { + return firstReviewReadinessItem(body) === "All CI tests are green on my local testing."; +} + +function reviewReadinessUsesCurrentPolicy(body) { + return firstReviewReadinessItem(body) === REVIEW_READINESS_ITEMS[0]; +} + /** * Remove the bot-managed readiness section from a body. Used so the bot's own * checklist never counts as author-written description substance, and so a @@ -550,6 +568,8 @@ module.exports = { buildReviewReadinessSection, extractReviewReadiness, appendReviewReadinessSection, + reviewReadinessMigrationRequired, + reviewReadinessUsesCurrentPolicy, stripReviewReadinessSection, REVIEW_READINESS_CLAIM_INDEX, uncheckReviewReadinessBoxes, diff --git a/.github/scripts/pr-quality.test.cjs b/.github/scripts/pr-quality.test.cjs index 55c948d65a..8549aefc57 100644 --- a/.github/scripts/pr-quality.test.cjs +++ b/.github/scripts/pr-quality.test.cjs @@ -16,6 +16,8 @@ const { buildReviewReadinessSection, extractReviewReadiness, appendReviewReadinessSection, + reviewReadinessMigrationRequired, + reviewReadinessUsesCurrentPolicy, stripReviewReadinessSection, uncheckReviewReadinessBoxes, REVIEW_READINESS_CLAIM_INDEX, @@ -409,7 +411,7 @@ describe("review readiness checklist", () => { it("treats a reworded but complete section as complete", () => { const reworded = SECTION - .replace("All CI tests are green on my local testing.", "Local suite green.") + .replace("Required local validation passed; commands, results, and any full-suite exception are documented.", "Local suite green.") .replaceAll("- [ ] ", "- [x] "); const result = extractReviewReadiness(reworded); assert.equal(result.present, true); @@ -585,7 +587,7 @@ describe("uncheckReviewReadinessBoxes", () => { "", "## Review readiness checklist", "", - "- [x] All CI tests are green on my local testing.", + "- [x] Required local validation passed; commands, results, and any full-suite exception are documented.", "- [x] I pushed my PR to the latest dev commit.", "- [x] I resolved all correct Codex and CodeRabbit findings.", "- [x] My PR is ready for review.", @@ -596,7 +598,7 @@ describe("uncheckReviewReadinessBoxes", () => { const body = uncheckReviewReadinessBoxes(checkedBody, [ REVIEW_READINESS_CLAIM_INDEX.latest_dev, ]); - assert.ok(body.includes("- [x] All CI tests are green on my local testing.")); + assert.ok(body.includes("- [x] Required local validation passed; commands, results, and any full-suite exception are documented.")); assert.ok(body.includes("- [ ] I pushed my PR to the latest dev commit.")); assert.ok(body.includes("- [x] My PR is ready for review.")); }); @@ -606,7 +608,7 @@ describe("uncheckReviewReadinessBoxes", () => { 0, REVIEW_READINESS_CLAIM_INDEX.latest_dev, ]); - assert.ok(body.includes("- [ ] All CI tests are green on my local testing.")); + assert.ok(body.includes("- [ ] Required local validation passed; commands, results, and any full-suite exception are documented.")); assert.ok(body.includes("- [ ] I pushed my PR to the latest dev commit.")); assert.ok(body.includes("- [x] I resolved all correct Codex and CodeRabbit findings.")); assert.ok(body.includes("- [x] My PR is ready for review.")); @@ -1101,3 +1103,28 @@ describe("comment stripping respects fenced code (regression)", () => { assert.equal(hasScreenshotEvidence(body), false); }); }); + + +describe("managed checklist wording classification", () => { + const oldItem = "All CI tests are green on my local testing."; + const legacy = buildReviewReadinessSection().replace(REVIEW_READINESS_ITEMS[0], oldItem); + for (const mark of [" ", "x", "X"]) { + for (const ending of ["\n", "\r\n"]) { + it(`recognizes old first item with ${JSON.stringify(mark)} and ${JSON.stringify(ending)}`, () => { + const body = legacy.replace(`- [ ] ${oldItem}`, ` * [${mark}] ${oldItem} `).replaceAll("\n", ending); + assert.equal(reviewReadinessMigrationRequired(body), true); + assert.equal(reviewReadinessUsesCurrentPolicy(body), false); + }); + } + } + it("preserves custom later labels and refuses malformed or displaced first items", () => { + assert.equal(reviewReadinessMigrationRequired(legacy.replace(REVIEW_READINESS_ITEMS[1], "Author's branch attestation.")), true); + for (const body of [null, "", oldItem, legacy + legacy, + legacy.replace("", ""), + legacy.replace(oldItem, oldItem + " Extra"), + legacy.replace(oldItem, "Custom").replace(REVIEW_READINESS_ITEMS[1], oldItem), + legacy.replace(`- [ ] ${REVIEW_READINESS_ITEMS[3]}`, ""), + ]) assert.equal(reviewReadinessMigrationRequired(body), false); + assert.equal(reviewReadinessUsesCurrentPolicy(buildReviewReadinessSection()), true); + }); +}); diff --git a/.github/scripts/pr-readiness-reattest.cjs b/.github/scripts/pr-readiness-reattest.cjs new file mode 100644 index 0000000000..59576cfb32 --- /dev/null +++ b/.github/scripts/pr-readiness-reattest.cjs @@ -0,0 +1,273 @@ +"use strict"; + +const { createHash } = require("node:crypto"); + +const SHA40 = /^[0-9a-f]{40}$/i; +const SHA256 = /^[0-9a-f]{64}$/; +const PHASES = new Set(["await-clear", "await-check", "attested"]); +const AWAITING_KEYS = new Set(["version", "headSha", "baseRef", "generation", "phase", "checkpointAt"]); +const ATTESTED_KEYS = new Set([...AWAITING_KEYS, "attestedBodySha256"]); + +/** + * @typedef {{ + * version: 1, + * headSha: string, + * baseRef: string, + * generation: number, + * phase: "await-clear" | "await-check", + * checkpointAt: string | null + * }} AwaitingReattestation + * + * @typedef {{ + * version: 1, + * headSha: string, + * baseRef: string, + * generation: number, + * phase: "attested", + * attestedBodySha256: string, + * checkpointAt: string | null + * }} AttestedReattestation + * + * @typedef {AwaitingReattestation | AttestedReattestation} PendingReattestation + * @typedef {{kind:"absent"} | {kind:"valid", value:PendingReattestation} | {kind:"invalid"}} ParsedPendingReattestation + */ + +/** @param {unknown} value @returns {ParsedPendingReattestation} */ +function parsePendingReattestation(value) { + if (value == null) return { kind: "absent" }; + if (typeof value !== "object" || Array.isArray(value)) return { kind: "invalid" }; + const candidate = /** @type {Record} */ (value); + if ( + candidate.version !== 1 || + typeof candidate.headSha !== "string" || + !SHA40.test(candidate.headSha) || + typeof candidate.baseRef !== "string" || + candidate.baseRef.length === 0 || + !Number.isSafeInteger(candidate.generation) || + candidate.generation <= 0 || + typeof candidate.phase !== "string" || + !PHASES.has(candidate.phase) || + !(candidate.checkpointAt === null || + (typeof candidate.checkpointAt === "string" && isStrictIsoTimestamp(candidate.checkpointAt))) + ) return { kind: "invalid" }; + + if (candidate.phase === "attested") { + if (typeof candidate.attestedBodySha256 !== "string" || !SHA256.test(candidate.attestedBodySha256)) { + return { kind: "invalid" }; + } + } else if (Object.hasOwn(candidate, "attestedBodySha256")) { + return { kind: "invalid" }; + } + + const allowedKeys = candidate.phase === "attested" ? ATTESTED_KEYS : AWAITING_KEYS; + if (Object.keys(candidate).some(key => !allowedKeys.has(key))) return { kind: "invalid" }; + + return { kind: "valid", value: /** @type {PendingReattestation} */ (candidate) }; +} + +/** @param {string} str */ +function bodyDigest(str) { + return createHash("sha256").update(String(str), "utf8").digest("hex"); +} + +function isStrictIsoTimestamp(value) { + if (typeof value !== "string") return false; + if (!/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d{3})?Z$/.test(value)) return false; + const parsed = Date.parse(value); + if (!Number.isFinite(parsed)) return false; + const canonical = new Date(parsed).toISOString(); + return value.includes(".") ? canonical === value : canonical.replace(".000Z", "Z") === value; +} + +function validLiveIdentity(live) { + return Boolean( + live && + typeof live.headSha === "string" && SHA40.test(live.headSha) && + typeof live.baseRef === "string" && live.baseRef.length > 0 && + typeof live.body === "string" && + Number.isSafeInteger(live.authorId) && live.authorId > 0 + ); +} + +function sameIdentity(pending, live) { + return pending.headSha === live.headSha && pending.baseRef === live.baseRef; +} + +function awaitClear(live, generation) { + return { + version: 1, + headSha: live.headSha, + baseRef: live.baseRef, + generation, + phase: "await-clear", + checkpointAt: null, + }; +} + +function nextGeneration(generation) { + return generation < Number.MAX_SAFE_INTEGER ? generation + 1 : 1; +} + +function samePending(left, right) { + return JSON.stringify(left) === JSON.stringify(right); +} + +function qualifyingAuthorBodyEdit({ live, event, checkpointAt }) { + const checkpointMs = Date.parse(checkpointAt); + const eventMs = Date.parse(event?.updatedAt ?? ""); + return Boolean( + event?.name === "pull_request_target" && + event.action === "edited" && + event.senderType === "User" && + Number.isSafeInteger(event.senderId) && event.senderId === live.authorId && + event.headSha === live.headSha && + typeof event.body === "string" && event.body === live.body && + typeof event.previousBody === "string" && event.previousBody !== event.body && + event.updatedAt === live.updatedAt && + Number.isFinite(checkpointMs) && Number.isFinite(eventMs) && + eventMs > checkpointMs + ); +} + +/** + * Advance the durable author re-attestation protocol without writing a PR body. + * A newly seeded/reset episode never consumes the event that caused the reset. + * + * @param {object} input + * @param {unknown} input.pending + * @param {boolean} input.legacy + * @param {boolean} input.current + * @param {{present?:boolean,total?:number,checked?:number,complete?:boolean}} input.readiness + * @param {{headSha:string,baseRef:string,body:string,updatedAt:string,authorId:number}} input.live + * @param {{name?:string,action?:string,senderId?:number,senderType?:string,headSha?:string,body?:string,updatedAt?:string,previousBody?:string}} input.event + * @param {boolean} [input.invalidate] + * @returns {{pending:PendingReattestation|null,canComplete:boolean,changed:boolean,invalidIdentity:boolean}} + */ +function advanceReattestation({ + pending, + legacy, + current, + readiness, + live, + event, + invalidate = false, +}) { + const parsed = parsePendingReattestation(pending); + if (!validLiveIdentity(live)) { + return { + pending: parsed.kind === "valid" ? parsed.value : null, + canComplete: false, + changed: false, + invalidIdentity: true, + }; + } + + const prior = parsed.kind === "valid" ? parsed.value : null; + if (parsed.kind === "invalid") { + return { pending: awaitClear(live, 1), canComplete: false, changed: true, invalidIdentity: false }; + } + + if (prior && !sameIdentity(prior, live)) { + return { + pending: awaitClear(live, nextGeneration(prior.generation)), + canComplete: false, + changed: true, + invalidIdentity: false, + }; + } + + if (legacy || invalidate) { + if (prior?.phase === "await-clear") { + return { pending: prior, canComplete: false, changed: false, invalidIdentity: false }; + } + const next = awaitClear(live, prior ? nextGeneration(prior.generation) : 1); + return { pending: next, canComplete: false, changed: !samePending(prior, next), invalidIdentity: false }; + } + + if (!prior) { + return { pending: null, canComplete: true, changed: false, invalidIdentity: false }; + } + + + // A phase is provisional until the workflow persists it, reads the successful + // comment write's server timestamp, and writes that timestamp into this field. + if (prior.checkpointAt === null) { + return { pending: prior, canComplete: false, changed: false, invalidIdentity: false }; + } + + if (prior.phase === "attested") { + if ( + current && readiness?.present === true && readiness.total === 4 && + readiness.checked === 4 && readiness.complete === true && + prior.attestedBodySha256 === bodyDigest(live.body) + ) { + return { pending: prior, canComplete: true, changed: false, invalidIdentity: false }; + } + const next = awaitClear(live, nextGeneration(prior.generation)); + return { pending: next, canComplete: false, changed: true, invalidIdentity: false }; + } + + if (!current) { + if (prior.phase === "await-clear") { + return { pending: prior, canComplete: false, changed: false, invalidIdentity: false }; + } + return { + pending: awaitClear(live, nextGeneration(prior.generation)), + canComplete: false, + changed: true, + invalidIdentity: false, + }; + } + + if (!qualifyingAuthorBodyEdit({ live, event, checkpointAt: prior.checkpointAt })) { + return { pending: prior, canComplete: false, changed: false, invalidIdentity: false }; + } + + if ( + prior.phase === "await-clear" && current && readiness?.present === true && + readiness.total === 4 && readiness.checked === 0 && readiness.complete === false + ) { + const next = { ...prior, phase: "await-check", checkpointAt: null }; + return { pending: next, canComplete: false, changed: true, invalidIdentity: false }; + } + + if ( + prior.phase === "await-check" && current && readiness?.present === true && + readiness.total === 4 && readiness.checked === 4 && readiness.complete === true + ) { + const next = { + ...prior, + phase: "attested", + attestedBodySha256: bodyDigest(live.body), + checkpointAt: null, + }; + return { pending: next, canComplete: false, changed: true, invalidIdentity: false }; + } + + return { pending: prior, canComplete: false, changed: false, invalidIdentity: false }; +} + +/** + * Whether a saved re-attestation positively authorizes readiness for the live + * PR: a finalized attestation of this exact head, base, and body. A readable + * state that is merely unchanged from an earlier phase is not evidence. + * + * @param {unknown} saved + * @param {{headSha:string,baseRef:string,body:string,authorId:number}} live + */ +function savedAttestationAuthorizes(saved, live) { + const parsed = parsePendingReattestation(saved); + if (parsed.kind !== "valid" || !validLiveIdentity(live)) return false; + const value = parsed.value; + return value.phase === "attested" && + typeof value.checkpointAt === "string" && + sameIdentity(value, live) && + value.attestedBodySha256 === bodyDigest(live.body); +} + +module.exports = { + advanceReattestation, + bodyDigest, + parsePendingReattestation, + savedAttestationAuthorizes, +}; diff --git a/.github/workflows/enforce-pr-target.yml b/.github/workflows/enforce-pr-target.yml index 6743730206..693d01f2a5 100644 --- a/.github/workflows/enforce-pr-target.yml +++ b/.github/workflows/enforce-pr-target.yml @@ -186,6 +186,9 @@ jobs: isChangedFileListTruncated, extractReviewReadiness, appendReviewReadinessSection, + reviewReadinessMigrationRequired, + reviewReadinessUsesCurrentPolicy, + REVIEW_READINESS_ITEMS, stripReviewReadinessSection, uncheckReviewReadinessBoxes, REVIEW_READINESS_CLAIM_INDEX, @@ -204,6 +207,8 @@ jobs: ); const { parseGateState, + advanceReattestation, + savedAttestationAuthorizes, gateStateMarker, parseState, parseReadinessState, @@ -341,7 +346,8 @@ jobs: legacyReadinessState ); - let gateState = storedGateState ?? migratedGateState; + let gateState = storedGateState && typeof storedGateState === "object" && !Array.isArray(storedGateState) + ? storedGateState : migratedGateState; /** * Maintainers from `MAINTAINERS.md` on the trusted default branch @@ -418,13 +424,15 @@ jobs: await migrateLegacyCommentsIfNeeded(); return; } - await github.rest.issues.updateComment({ + const updated = await github.rest.issues.updateComment({ owner, repo, comment_id: gateCommentId, body }); - gateComment.body = body; + // Replace the listed comment instead of mutating it, so a later + // authoritative readback observes the server, not this run's copy. + gateComment = { ...gateComment, body, updated_at: updated.data.updated_at }; await migrateLegacyCommentsIfNeeded(); return; } @@ -435,7 +443,7 @@ jobs: body }); gateCommentId = created.data.id; - gateComment = { id: gateCommentId, body }; + gateComment = { id: gateCommentId, body, updated_at: created.data.updated_at }; await migrateLegacyCommentsIfNeeded(); } @@ -527,6 +535,118 @@ jobs: ); } + let reattestation = null; + let preserveAuthorBody = false; + const initiallyLegacy = reviewReadinessMigrationRequired(pr.body); + const unreadablePendingState = Boolean(gateComment?.body?.includes("opencodex-pr-gate-state:") && + (!storedGateState || typeof storedGateState !== "object" || Array.isArray(storedGateState))); + + function observeReattestation(snapshot, invalidate = false, allowEvent = true) { + const pending = (unreadablePendingState || (initiallyLegacy && gateState.pendingReattestation == null)) && !preserveAuthorBody + ? { invalid: true } : gateState.pendingReattestation; + const result = advanceReattestation({ + pending, + legacy: reviewReadinessMigrationRequired(snapshot.body), + current: reviewReadinessUsesCurrentPolicy(snapshot.body), + readiness: extractReviewReadiness(snapshot.body), + live: { headSha: snapshot.head?.sha, baseRef: snapshot.base?.ref, + body: snapshot.body, updatedAt: snapshot.updated_at, authorId: snapshot.user?.id }, + event: allowEvent ? { name: context.eventName, action: context.payload.action, + senderId: context.payload.sender?.id, senderType: context.payload.sender?.type, + headSha: context.payload.pull_request?.head?.sha, body: context.payload.pull_request?.body, + updatedAt: context.payload.pull_request?.updated_at, + previousBody: context.payload.changes?.body?.from } : {}, + invalidate + }); + preserveAuthorBody ||= pending != null || result.pending != null; + gateState.pendingReattestation = result.pending; + reattestation = result; + return result; + } + + async function persistReattestationCheckpoint(state, options) { + await upsertGateComment(state, options); + if (state.pendingReattestation?.checkpointAt !== null) return; + const checkpointAt = gateComment?.updated_at; + if (typeof checkpointAt !== "string" || !Number.isFinite(Date.parse(checkpointAt))) { + core.setFailed("The re-attestation checkpoint has no authoritative server timestamp."); + return; + } + const finalized = { ...state.pendingReattestation, checkpointAt }; + await upsertGateComment({ ...state, pendingReattestation: finalized }, options); + gateState.pendingReattestation = finalized; + } + + async function retainReattestationDraft(snapshot) { + core.setFailed("Current-head author re-attestation is pending; ordinary quality evaluation resumes after the saved checkpoint."); + if (reattestation?.invalidIdentity) { + core.setFailed("Cannot bind re-attestation to a verified current PR head."); + return; + } + const pending = gateState.pendingReattestation; + const snapshotReadiness = extractReviewReadiness(snapshot.body); + const clearStep = "Wait for the bot to acknowledge the cleared checklist before validating and ticking the boxes again."; + const action = pending?.phase === "await-check" + ? "The cleared checklist has been recorded. Validate this head, tick all four boxes and save the PR description." + : reviewReadinessUsesCurrentPolicy(snapshot.body) && snapshotReadiness.checked > 0 + ? `The first managed item already uses the current wording, but boxes ticked before this notice cannot carry over. Clear all four boxes and save. ${clearStep}` + : `Change the first managed item to ${inlineCode(REVIEW_READINESS_ITEMS[0])}, clear all four boxes and save. ${clearStep}`; + const state = { ...gateState, active: true, maintainersPinged: false, + autoDraftedByBot: !snapshot.draft || gateState.autoDraftedByBot }; + const options = { + status: "DRAFT", statusReason: "author re-attestation is required for the current head.", + actions: [action, "Only a new body edit by the PR author after this notice can advance the checkpoint. If edits share a checkpoint timestamp, make another body edit and save later."], + readiness: snapshotReadiness, checklistRequired: true, + notices: [`Current head: ${inlineCode(snapshot.head.sha)}. Existing PR text and checkbox marks were preserved.`] + }; + await persistReattestationCheckpoint(state, options); + if ((snapshot.labels ?? []).some(label => label.name === REVIEW_READY_LABEL)) { + try { + await github.rest.issues.removeLabel({ owner, repo, issue_number: pull_number, name: REVIEW_READY_LABEL }); + } catch (error) { + core.setFailed("Could not remove the stale review-ready label while re-attestation is pending."); + } + } + if (!snapshot.draft) { + try { await convertToDraft(); } + catch (error) { + state.autoDraftedByBot = false; + state.pendingReattestation = gateState.pendingReattestation; + await upsertGateComment(state, { + ...options, notices: [...options.notices, "Automatic draft conversion failed. Please retain draft state manually until re-attestation is complete."] + }); + core.setFailed("Could not retain draft state during author re-attestation."); + } + } + } + + if (!authorHasPushPermission(authorPermission) && + (reviewReadinessMigrationRequired(pr.body) || gateState.pendingReattestation != null || unreadablePendingState)) { + const { data: freshPr } = await github.rest.pulls.get({ owner, repo, pull_number }); + if (freshPr.node_id !== pr.node_id || freshPr.user?.id !== pr.user?.id) { + core.setFailed("PR identity changed before re-attestation."); + return; + } + Object.assign(pr, freshPr); + const result = observeReattestation(pr); + if (result.invalidIdentity || result.pending?.phase !== "attested") { + await retainReattestationDraft(pr); + return; + } + // Persist the new proof before any ready side effect. A failed write + // cannot be treated as a saved re-attestation checkpoint. + await persistReattestationCheckpoint({ ...gateState, active: true }, { + status: "DRAFT", statusReason: "Current-head re-attestation recorded; checking remaining requirements.", + actions: [], readiness: extractReviewReadiness(pr.body), checklistRequired: true, notices: [] + }); + if (!observeReattestation(pr, false, false).canComplete) { + core.setFailed("Re-attestation finalization is incomplete; readiness was not advanced."); + return; + } + } else if (authorHasPushPermission(authorPermission)) { + gateState.pendingReattestation = null; + } + let behindMain = 0; let behindBase = 0; let aheadMain = 0; @@ -815,7 +935,7 @@ jobs: ? "" : pr.head.sha); const completionHeadSha = - gateState.completedAtHeadSha ?? null; + reattestation?.canComplete ? pr.head.sha : (gateState.completedAtHeadSha ?? null); const headDrifted = completionIsStale({ checklistRequired, checklistComplete, @@ -837,6 +957,11 @@ jobs: repo, pull_number }); + if (preserveAuthorBody || reviewReadinessMigrationRequired(freshPr.body)) { + observeReattestation(freshPr, true, false); + await retainReattestationDraft(freshPr); + return; + } const freshReadiness = extractReviewReadiness( freshPr.body ?? "" ); @@ -848,7 +973,8 @@ jobs: ...defaultGateState(), active: gateState.active, autoDraftedByBot: gateState.autoDraftedByBot, - titlePrefixedByBot: gateState.titlePrefixedByBot + titlePrefixedByBot: gateState.titlePrefixedByBot, + pendingReattestation: gateState.pendingReattestation ?? null }; headDriftNotice = buildStaleNotice({ completionHeadSha, @@ -991,6 +1117,11 @@ jobs: repo, pull_number }); + if (preserveAuthorBody || reviewReadinessMigrationRequired(freshPr.body)) { + observeReattestation(freshPr, true, false); + await retainReattestationDraft(freshPr); + return; + } const freshReadiness = extractReviewReadiness( freshPr.body ?? "" ); @@ -998,7 +1129,8 @@ jobs: ...defaultGateState(), active: gateState.active, autoDraftedByBot: gateState.autoDraftedByBot, - titlePrefixedByBot: gateState.titlePrefixedByBot + titlePrefixedByBot: gateState.titlePrefixedByBot, + pendingReattestation: gateState.pendingReattestation ?? null }; claimNotice = [ ...(claimViolations.includes("review_findings") @@ -1093,6 +1225,40 @@ jobs: return actions; } + if (checklistRequired && checklistComplete && failures.length === 0) { + const expectedPending = JSON.stringify(gateState.pendingReattestation); + try { + if (preserveAuthorBody) { + const { data: finalComment } = await github.rest.issues.getComment({ owner, repo, comment_id: gateCommentId }); + const finalState = parseGateState(finalComment.body); + // Promotion needs positive evidence: the saved comment must hold a + // finalized attestation of this exact head, base, and body. + if (finalComment.user?.login !== "github-actions[bot]" || + JSON.stringify(finalState?.pendingReattestation) !== expectedPending || + !savedAttestationAuthorizes(finalState?.pendingReattestation, { + headSha: pr.head.sha, baseRef: pr.base.ref, body: pr.body, authorId: pr.user?.id })) { + core.setFailed("Saved re-attestation changed before readiness; no ready action was taken."); + return; + } + } + const { data: finalPr } = await github.rest.pulls.get({ owner, repo, pull_number }); + if (reviewReadinessMigrationRequired(finalPr.body)) { + observeReattestation(finalPr, true, false); + await retainReattestationDraft(finalPr); + return; + } + if (finalPr.node_id !== pr.node_id || finalPr.head?.sha !== pr.head.sha || + finalPr.base?.ref !== pr.base.ref || finalPr.body !== pr.body || + finalPr.user?.id !== pr.user?.id || finalPr.state !== "open") { + core.setFailed("PR changed before readiness; no ready action was taken."); + return; + } + } catch (error) { + core.setFailed("Could not refresh the current PR and saved re-attestation before readiness."); + return; + } + } + // The `review-ready` label marks the ready moment for humans and // bots. It is not a CodeRabbit auto-review filter: a positive // `labels:` entry in `.coderabbit.yaml` would restrict ALL reviews @@ -1217,7 +1383,7 @@ jobs: "This pull request is being kept as a draft automatically. Once every issue above is resolved, it will be marked ready for review again.", ...(checklistRequired && !checklistComplete ? [ - `@${pr.user.login} Tick the boxes once your local CI is green, your branch is on the latest ${inlineCode(DEFAULT_BASE)} commit, and every correct Codex and CodeRabbit finding is resolved.` + `@${pr.user.login} Tick the boxes once required local validation has passed with commands, results, and any full-suite exception documented, your branch is on the latest ${inlineCode(DEFAULT_BASE)} commit, and every correct Codex and CodeRabbit finding is resolved.` ] : []) ]); @@ -1242,7 +1408,7 @@ jobs: "This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.", ...(checklistRequired && !checklistComplete ? [ - `@${pr.user.login} Tick the boxes once your local CI is green, your branch is on the latest ${inlineCode(DEFAULT_BASE)} commit, and every correct Codex and CodeRabbit finding is resolved.` + `@${pr.user.login} Tick the boxes once required local validation has passed with commands, results, and any full-suite exception documented, your branch is on the latest ${inlineCode(DEFAULT_BASE)} commit, and every correct Codex and CodeRabbit finding is resolved.` ] : []) ]); @@ -1373,6 +1539,7 @@ jobs: readyState.maintainersPinged = true; notified = true; } + if (!readyConversionFailed) readyState.pendingReattestation = null; readyState.completedAtHeadSha = pr.head.sha; readyState.version = 1; diff --git a/.github/workflows/issue-quality-tests.yml b/.github/workflows/issue-quality-tests.yml index 11bbba12b8..fd064ce101 100644 --- a/.github/workflows/issue-quality-tests.yml +++ b/.github/workflows/issue-quality-tests.yml @@ -12,6 +12,7 @@ on: - ".github/scripts/pr-quality-messages.cjs" - ".github/scripts/pr-quality-messages.test.cjs" - ".github/scripts/pr-quality-state.cjs" + - ".github/scripts/pr-readiness-reattest.cjs" - ".github/scripts/pr-quality-state.test.cjs" - ".github/scripts/pr-labeler.cjs" - ".github/scripts/pr-labeler.test.cjs" @@ -47,6 +48,7 @@ on: - ".github/scripts/pr-quality-messages.cjs" - ".github/scripts/pr-quality-messages.test.cjs" - ".github/scripts/pr-quality-state.cjs" + - ".github/scripts/pr-readiness-reattest.cjs" - ".github/scripts/pr-quality-state.test.cjs" - ".github/scripts/pr-labeler.cjs" - ".github/scripts/pr-labeler.test.cjs" diff --git a/AGENTS.md b/AGENTS.md index 5fc447e7c3..d57f4f7659 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -196,7 +196,7 @@ it binds you regardless of which mechanism is within reach. bun install bun run typecheck # bun x tsc --noEmit (strict) bun run test:changed # import-graph tests against the resolved `dev` merge base -bun run test # full tests/ suite (PR-ready / explicit ask only) +bun run test # full tests/ suite (default before review) bun run lint:gui # GUI eslint bun run privacy:scan # credential/privacy scan used by CI bun run structure:check # structure/ doc-map, ownership, and invariant-binding gate @@ -217,23 +217,29 @@ bun run skill:surface:check # what CI asserts also if the hand-written pages name a command the registry does not have. That second check is not hypothetical: it caught a documented `ocx request-history` that never existed. -During implementation, use the smallest focused checks that directly cover the -changed subsystem. Prefer `bun test tests//.test.ts` for a known -file, `bun test tests/` for one subsystem, or -`bun run test:changed` when the touch set is broader than one file. Do **not** -run repository-wide `bun run test` or a bare `bun test` with no file arguments -for a scoped change by default. `bun run test:changed` follows Bun's parsed module graph: it -selects test files that import changed modules, but it cannot see dependencies -expressed through subprocesses, source files read as data, or golden/derived -files. Run the relevant focused tests explicitly for those paths; if no reliable -focused set covers them, the full suite is required even for a scoped change. -That indirect-dependency case is the explicit exception to the scoped-change -default. The full suite is ~850 files, so otherwise reserve it for a failed or -ambiguous focused result, an explicit user request, or the PR-ready gate below. - -Before creating or updating a non-trivial PR as review-ready, or before -approving such a PR, run `bun run typecheck` and `bun run test`. CI runs these -on Linux, Windows, and macOS. +Run the test suite for a change; `bun run test` is the default before a +non-trivial PR is marked review-ready or approved. During implementation, use +focused files or `bun run test:changed` for faster feedback. + +If a full local run is disproportionately expensive for the task or available +resources, including contention across concurrent worktrees, run at least the +focused regression tests that exercise the changed behavior. This is a scope +exception, not permission to skip testing or ignore a failing test. Record why +the full run was impractical, the exact commands and results, and the coverage +left to CI in the PR's Verification section. Never describe an unrun suite as +passing. Run `bun run typecheck` before review readiness as well. + +`bun run test:changed` follows Bun's parsed module graph, so it cannot discover +dependencies expressed through subprocesses, source files read as data, or +golden/derived files. Run those relevant regression files explicitly. If a +focused set cannot reliably cover the change, keep the PR in draft until the +broader validation is available. + +After pushing, inspect the required CI for the current PR head. Missing, +awaiting-approval, skipped, cancelled, or older-head results are not passing +evidence. Required checks must actually complete successfully before merge. +The repository does not install an automatic pre-push validation hook; +`bun run prepush` remains available as an explicit comprehensive check. Do not rerun passing checks on unchanged code merely for additional confidence. @@ -377,8 +383,9 @@ empty, thin, or malformed descriptions; PRs whose title or description mentions `gui` must include a screenshot of the UI change in the description. Contributor PRs (authors without repository push permission) open in draft and stay there until a four-box review-readiness checklist in the description is -complete: local CI green, branch on the latest `dev` commit, all correct Codex -and CodeRabbit findings fixed, and the ready-for-review confirmation. When all +complete: required local validation passed with its scope documented, branch +on the latest `dev` commit, all correct Codex and CodeRabbit findings fixed, +and the ready-for-review confirmation. When all four boxes are ticked the gate marks the PR ready and notifies the maintainers listed in `MAINTAINERS.md` (excluding the author). Completion is bound to the exact commit the PR head pointed at: if new commits are pushed afterwards, the @@ -387,7 +394,7 @@ and asks the author to test and tick the boxes again against the latest code. Before a completion is accepted, the gate verifies the checklist claims it can check itself: the branch must be on the latest `dev` commit or at most 10 commits behind it, and Codex/CodeRabbit findings must be resolved. The -local-CI box is an author attestation only — fork contributors cannot start +local-validation box is an author attestation only — fork contributors cannot start repository CI; a maintainer has to — so the gate never disproves it; a new push still resets every box. A disproved claim unticks the matching box and keeps the PR a draft. @@ -400,7 +407,7 @@ explicitly integrate through a PR without another maintainer approval, including their own PR, under the policy in `MAINTAINERS.md`. Record the decision and exact-head CI evidence; keep outstanding maintainer objections and security review separate. The bypass is PR-only, so a direct push to `dev` remains rejected regardless of -`--no-verify`. Contributor review and `main`/`preview` rules remain unchanged. +local hook settings. Contributor review and `main`/`preview` rules remain unchanged. [`MAINTAINERS.md`](./MAINTAINERS.md) is authoritative for review and merge policy (approvals, CI requirements, security review, promotion). This file @@ -430,7 +437,8 @@ reviewers (Codex, CodeRabbit). - **Tests:** behavior changes in `src/` need a focused regression test near the existing tests for that subsystem. During implementation, run the relevant focused files and use `bun run test:changed` for import-connected coverage as - described above; the full suite is the PR-ready gate. + described above. Full-suite validation is the default before review readiness; + the documented resource exception still requires focused regression tests. - **Docs sync:** user-facing behavior changes should update `docs-site/` (and keep translated locales from contradicting the English source). - **Privacy:** `bun run privacy:scan` must stay green; never introduce logging diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ea36e08eb4..9b0ecbc6e2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,21 +56,22 @@ A ready-for-review PR is the author's claim that the change is complete, underst stated. A closed PR can be reopened once the stated reason is resolved, or replaced with a clean one. -## Pre-push hook +## Local validation and hooks -After cloning, run once to install a local pre-push hook that runs the typecheck, -unit-test, privacy-scan, and (when `gui/` changed) GUI eslint and React Doctor -portions of the CI gate: +Run `bun run test` before review readiness. If the full local suite is too costly +for the task or available resources, run at least focused regression tests for +the changed behavior. Document the reason, commands, results, and remaining +coverage in the PR. Follow [AGENTS.md](./AGENTS.md#commands) for the complete +validation policy; required CI must pass on the current PR head before merge. +`bun run prepush` remains an optional comprehensive local check. ```sh bun run setup:hooks ``` -This installs a `pre-push` hook (into the hooks dir git reports, so worktrees and -`core.hooksPath` work) that runs `bun run prepush` — `typecheck`, -`lint:gui:if-changed`, `test`, `privacy:scan`, and `doctor:gui:if-changed` — -before every `git push`. Both `lint:gui:if-changed` and `doctor:gui:if-changed` -run their check only when the push touches `gui/`. -The same checks run on ubuntu-latest, macos-latest, and windows-latest in CI (CI -additionally builds the GUI and smoke-tests the CLI). Skip in an emergency with -`git push --no-verify`. +This installs the `post-merge` hook, which rebuilds the packaged dashboard when a +merge changes its source. It also removes the unmodified, retired repository +pre-push hook from Git's resolved hooks directory, including linked worktrees +and `core.hooksPath` setups. Custom pre-push hooks are preserved. Validation no +longer runs automatically on every push; existing contributors should rerun the +setup command once to migrate their hooks. diff --git a/MAINTAINERS.md b/MAINTAINERS.md index 4d04764956..5c05f60ad4 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -36,7 +36,8 @@ when a maintainer steps down. mentions `gui` must include a screenshot of the UI change in the description. Contributor PRs (authors without repository push permission) open in draft and stay there until a four-box review-readiness checklist in the - description is complete: local CI green, branch on the latest `dev` commit, + description is complete: required local validation passed with its scope documented, + branch on the latest `dev` commit, all correct Codex and CodeRabbit findings fixed, and the ready-for-review confirmation. When all four boxes are ticked the gate marks the PR ready and notifies the maintainers listed in `MAINTAINERS.md` (excluding the author). @@ -47,8 +48,11 @@ when a maintainer steps down. Before a completion is accepted, the gate verifies the checklist claims it can check itself: the branch must be on the latest `dev` commit or at most 10 commits behind it, and Codex/CodeRabbit findings must be resolved. - The local-CI box is an author attestation only — fork contributors cannot - start repository CI; a maintainer has to — so the gate never disproves it; + The local-validation box follows the full-suite default and documented resource + exception in [AGENTS.md](./AGENTS.md#commands); focused regression tests remain + mandatory under that exception. It is an author attestation only — fork + contributors cannot start repository CI; a maintainer has to — so the gate + never disproves it; a new push still resets every box. A disproved claim unticks the matching box and keeps the PR a draft. Authors with repository push permission skip the ancestry heuristic only. As diff --git a/docs-site/src/content/docs/contributing.md b/docs-site/src/content/docs/contributing.md index e6f9844c1f..947cedfb27 100644 --- a/docs-site/src/content/docs/contributing.md +++ b/docs-site/src/content/docs/contributing.md @@ -12,14 +12,17 @@ Bun runtime for users, but this checkout's scripts run through your local Bun in git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # install post-merge; retire the managed pre-push hook bun run dev:proxy # proxy API in dev mode bun run dev:gui # dashboard dev server (another terminal) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # full suite (default) ``` +`bun run setup:hooks` installs only `post-merge` and removes an unmodified retired managed +`pre-push` hook, preserving custom hooks. A pre-push hook is no longer required. +`bun run prepush` remains an optional manual check. + `bun run dev` remains an alias for `bun run dev:proxy`. The dashboard dev server is `bun run dev:gui`; the packaged dashboard at `GET /` is produced by `bun run build:gui` (`gui/dist`). @@ -31,13 +34,21 @@ scripts so local commands match CI: ```bash bun run typecheck # strict TypeScript check bun run test:changed # import-graph tests against the resolved dev merge base -bun run test # complete tests/ suite (PR-ready / explicit ask) +bun run test # full suite (default) bun test tests/routing/router.test.ts # focused test file bun run build:gui # Vite GUI build + package preparation bun run privacy:scan # credential/privacy scan used by CI bun run prepare:package # refresh package launchers/assets ``` +Run `bun run test` by default. If a full run is disproportionately expensive for the task size, +available machine resources, or concurrent worktrees, you must at least run focused regression tests +that exercise the changed behavior, such as `bun test tests//.test.ts`. Explain the +reason for narrowing the run and report the exact commands, results, and untested scope. +`bun run test:changed` can supplement this coverage, but it cannot detect every indirect dependency. +Neither relying only on CI nor skipping local testing is a blanket exemption. Before merge, all +required CI checks must pass for the exact current PR head. + `test:changed` selects the first comparison ref that exists, in order: `upstream/dev`, `origin/dev`, then local `dev`. It reports that ref and the exact `git merge-base HEAD ` commit, then passes the merge-base SHA to Bun. @@ -54,8 +65,7 @@ and `tests/test-layout.test.ts` enforces it, so a new test goes into its domain an entry in the map (the tooling test tells you which one is missing). `tests/helpers/` holds shared fixtures and `tests/helpers/repo-root.ts` is how a test reaches repository files; `tests/e2e-style/` holds broader native-parity scenarios. Keep a focused regression near the -existing tests for the subsystem you change (`bun test tests/` runs one subsystem); run -the full suite for shared routing, adapters, config, or server behavior. +existing tests for the subsystem you change (`bun test tests/` runs one subsystem). The docs site you're reading lives in `docs-site/` (Astro + Starlight): @@ -250,6 +260,6 @@ startup path must not import the manifest catalog or activate Compatibility Lab. ## Verify before you claim done -Run the narrowest command that proves your change — `bun run typecheck` for types, a focused -`bun test tests//.test.ts` or runtime probe for behavior, then the broader gates appropriate to -the affected surface. opencodex favors small, verifiable commits over large batches. +Follow the testing policy above and run `bun run typecheck` for type changes, plus the checks +required for the affected surface. Report the commands, results, and remaining untested scope; +only claim the validation you actually completed. diff --git a/docs-site/src/content/docs/contributing/pr-quality.md b/docs-site/src/content/docs/contributing/pr-quality.md index 27bf69f555..bac390abde 100644 --- a/docs-site/src/content/docs/contributing/pr-quality.md +++ b/docs-site/src/content/docs/contributing/pr-quality.md @@ -51,8 +51,8 @@ tells you exactly what to change: self-waive the screenshot requirement. Contributor PRs (authors without repository push permission) open in draft and stay there until a four-box review-readiness checklist in the - description is complete: local CI green, the branch on the latest `dev` - commit, all correct Codex and CodeRabbit findings fixed, and the + description is complete: required local validation passed with its scope + documented, the branch on the latest `dev` commit, all correct Codex and CodeRabbit findings fixed, and the ready-for-review confirmation. Once every box is ticked the check marks the PR ready for review and notifies the maintainers listed in `MAINTAINERS.md` (excluding the author). The gate's status and "what to do" live in a single @@ -67,8 +67,9 @@ tells you exactly what to change: can check itself: the branch must be on the latest `dev` commit or at most 10 commits behind it, and every Codex and CodeRabbit review thread authored by a review bot on the current head must be resolved (unresolved threads - from other authors do not block). The local-CI box is an author attestation - only — fork contributors cannot start repository CI; a maintainer has to — + from other authors do not block). The local-validation box follows the [test-scope policy](/contributing/#build-and-test-commands): + run the full suite by default; when it is too costly, run focused regressions + and document the exception. It is an author attestation only — fork contributors cannot start repository CI; a maintainer has to — so the gate never disproves it; a new push still resets every box. CodeRabbit findings that fall outside the diff range and are reported only in a review body on the current head add to the unresolved count while a bot review @@ -131,3 +132,14 @@ A PR that stalls with unresolved review feedback may be closed, with the reason stated plainly. Closure is not a verdict on the contributor: reopen it once the stated reason is resolved, or replace it with a clean one. Ask if the reason is not clear. + +## Updating an older readiness checklist + +If the gate reports that your checklist still uses the retired local-CI wording, it preserves +your description and asks you to update the first item. Change that item to the wording in +the bot notice, clear all four boxes and save. Wait for the bot to acknowledge the cleared +checklist, validate the displayed head, then tick all four boxes and save again. Changing +only the wording while leaving four ticks does not count as a new attestation. A push or +retarget invalidates the checkpoint. The gate stays red and the PR stays draft until this +sequence and the ordinary quality checks complete. If your save shares the checkpoint's +timestamp, make another body edit and save later; editing only the title does not count. diff --git a/docs-site/src/content/docs/fr/contributing.md b/docs-site/src/content/docs/fr/contributing.md index 345d31359f..d7a5e9296a 100644 --- a/docs-site/src/content/docs/fr/contributing.md +++ b/docs-site/src/content/docs/fr/contributing.md @@ -12,14 +12,17 @@ runtime Bun aux utilisateurs, mais les scripts de ce dépôt utilisent votre ins git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # installer post-merge et retirer l’ancien pre-push géré bun run dev:proxy # proxy API in dev mode bun run dev:gui # dashboard dev server (another terminal) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # suite complète (par défaut) ``` +`bun run setup:hooks` installe uniquement `post-merge` et supprime l’ancien hook `pre-push` +géré s’il n’a pas été modifié, tout en préservant les hooks personnalisés. Le hook `pre-push` +n’est plus obligatoire. `bun run prepush` reste une vérification manuelle facultative. + `bun run dev` reste un alias pour `bun run dev:proxy`. Le serveur de développement du tableau de bord est `bun run dev:gui` ; le tableau de bord packagé en `GET /` est produit par `bun run build:gui` (`gui/dist`). @@ -31,17 +34,25 @@ distincte. Utilisez les scripts enregistrés afin que les commandes locales corr ```bash bun run typecheck # strict TypeScript check bun run test:changed # import-graph tests against the resolved dev merge base -bun run test # complete tests/ suite (PR-ready / explicit ask) +bun run test # suite complète (par défaut) bun test tests/routing/router.test.ts # focused test file bun run build:gui # Vite GUI build + package preparation bun run privacy:scan # credential/privacy scan used by CI bun run prepare:package # refresh package launchers/assets ``` +Exécutez `bun run test` par défaut. Si une exécution complète est disproportionnée par rapport +à la taille de la tâche, aux ressources de la machine ou aux worktrees utilisés en parallèle, vous +devez au minimum exécuter des tests de régression ciblés qui exercent le comportement modifié, par +exemple `bun test tests//.test.ts`. Expliquez ce choix et indiquez les commandes +exactes, leurs résultats et le périmètre non testé. `bun run test:changed` peut compléter cette +couverture, mais ne détecte pas toutes les dépendances indirectes. Se reposer uniquement sur la CI +ou omettre les tests locaux ne constitue pas une exemption générale. Avant la fusion, tous les +contrôles CI obligatoires doivent réussir sur le commit exact de la tête actuelle de la PR. + Les tests Bun vivent dans des répertoires par domaine calqués sur `src/` (`tests//`), la carte étant `scripts/test-layout/layout.json`. `tests/helpers/` contient les fixtures partagées et `tests/e2e-style/` des scénarios plus larges de parité native. Placez une régression ciblée près -des tests existants du sous-système modifié. Exécutez la suite complète pour le routage partagé, les adaptateurs, -la configuration ou le comportement du serveur. +des tests existants du sous-système modifié. Le site de documentation que vous lisez se trouve dans `docs-site/` (Astro + Starlight) : @@ -224,6 +235,6 @@ la fabrique depuis `src/index.ts` lorsqu’elle appartient à l’API publique d ## Vérifiez avant de déclarer que c'est fait -Exécutez la commande la plus étroite qui prouve votre changement — `bun run typecheck` pour les types, un -`bun test tests/.test.ts` ou une sonde d'exécution pour le comportement, puis les portes plus larges appropriées à -la surface affectée. opencodex privilégie les petits commits vérifiables plutôt que les gros lots. +Suivez la politique de test ci-dessus et exécutez `bun run typecheck` pour les changements de types, +ainsi que les vérifications requises pour la zone concernée. Indiquez les commandes, les résultats +et le périmètre non testé ; ne revendiquez que les validations réellement effectuées. diff --git a/docs-site/src/content/docs/fr/contributing/pr-quality.md b/docs-site/src/content/docs/fr/contributing/pr-quality.md index 448432cbc1..0711cb3b0e 100644 --- a/docs-site/src/content/docs/fr/contributing/pr-quality.md +++ b/docs-site/src/content/docs/fr/contributing/pr-quality.md @@ -44,8 +44,8 @@ Trois contrôles déterministes précèdent la revue humaine. Chaque message d déclenchent plus le contrôle privilégié. Un contributeur ne peut pas lever lui-même cette exigence. Les PR de contributeurs sans droit de push sur le dépôt s’ouvrent en brouillon et le restent jusqu’à ce que - les quatre cases de préparation à la revue soient cochées dans la description : CI locale verte, branche sur - le dernier commit de `dev`, tous les constats valides de Codex et CodeRabbit corrigés, et confirmation de + les quatre cases de préparation à la revue soient cochées dans la description : validation locale requise réussie + (commandes, résultats et toute exception à la suite complète documentés), branche sur le dernier commit de `dev`, tous les constats valides de Codex et CodeRabbit corrigés, et confirmation de disponibilité pour la revue. Lorsque les quatre cases sont cochées, le contrôle marque la PR comme prête et avertit les responsables répertoriés dans `MAINTAINERS.md`, à l’exclusion de l’auteur. L’état du contrôle et les actions attendues figurent dans un unique commentaire consolidé, réécrit à chaque exécution. @@ -58,7 +58,7 @@ Trois contrôles déterministes précèdent la revue humaine. Chaque message d Avant d’accepter la liste, le contrôle vérifie les affirmations qu’il peut lui-même confirmer : la branche doit être sur le dernier commit de `dev`, ou au plus 10 commits derrière, et tous les fils de revue Codex et CodeRabbit créés par un robot sur la tête actuelle doivent être résolus. Les fils non résolus d’autres auteurs - ne bloquent pas. La case de CI locale est uniquement une attestation de l’auteur : les contributeurs depuis un + ne bloquent pas. La case de validation locale requise est uniquement une attestation de l’auteur : les contributeurs depuis un fork ne peuvent pas démarrer la CI du dépôt, seul un responsable le peut. Le contrôle ne contredit donc jamais cette case, mais tout nouveau push réinitialise toutes les cases. @@ -76,7 +76,8 @@ Trois contrôles déterministes précèdent la revue humaine. Chaque message d - **Hygiène.** Les changements de comportement exigent un test. Les nouvelles suppressions de règles de lint ou de types, les tests ciblés ou ignorés, les blocs catch vides, la modification de sorties générées et celle d’un lockfile sans son manifeste nécessitent chacun un label d’approbation explicite. Une modification limitée - à un commentaire dans un fichier source ne change pas le comportement et n’exige aucun test. + à un commentaire dans un fichier source ne change pas le comportement et n’exige aucun nouveau test + de régression. La [politique de test locale](/fr/contributing/) reste applicable. - **CI multiplateforme.** Pour les changements concernés, la suite est fragmentée sous Linux et exécutée intégralement sous macOS pour chaque pull request. La voie Windows principale ne s’exécute actuellement que @@ -117,3 +118,15 @@ seules surfaces soumises à cette règle. Toutes les autres restent ouvertes. Une PR bloquée par des remarques de revue non résolues peut être fermée, avec une raison clairement indiquée. La fermeture n’est pas un jugement sur le contributeur : rouvrez la PR lorsque la raison donnée est résolue, ou remplacez-la par une nouvelle PR propre. Demandez des précisions si la raison n’est pas claire. + +## Mettre à jour une ancienne liste de préparation + +Si le contrôle signale l’ancienne formulation sur la CI locale, il conserve votre description. +Remplacez le premier élément par le texte indiqué dans le commentaire du bot, décochez les +quatre cases et enregistrez. Attendez que le bot confirme cette étape, validez le commit +indiqué, puis cochez les quatre cases et enregistrez de nouveau. Modifier uniquement le +texte en conservant les quatre coches ne renouvelle pas l’attestation. Un push ou un +changement de branche cible invalide cette étape. Le contrôle reste en échec et la PR en +brouillon jusqu’à la fin de cette procédure et des contrôles habituels. Si l’enregistrement +partage l’horodatage de l’étape, modifiez de nouveau le corps et enregistrez plus tard ; +modifier seulement le titre ne suffit pas. diff --git a/docs-site/src/content/docs/ja/contributing.md b/docs-site/src/content/docs/ja/contributing.md index 115d182ef6..af2413067d 100644 --- a/docs-site/src/content/docs/ja/contributing.md +++ b/docs-site/src/content/docs/ja/contributing.md @@ -9,14 +9,17 @@ description: opencodex の開発環境、構成、規約、プロバイダーと git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # post-merge の導入と旧管理対象 pre-push の削除 bun run dev:proxy # 開発モードのプロキシ API bun run dev:gui # ダッシュボード dev サーバー(別ターミナル) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # 全テストスイート(既定) ``` +`bun run setup:hooks` は `post-merge` だけを導入し、変更されていない旧管理対象の `pre-push` +フックを削除します。カスタムフックは保持します。`pre-push` フックは必須ではなくなりました。 +`bun run prepush` は任意の手動チェックとして引き続き利用できます。 + `bun run dev` は引き続き `bun run dev:proxy` のエイリアスとして動作します。ダッシュボード dev サーバーは `bun run dev:gui` で、`GET /` で提供するパッケージダッシュボードは `bun run build:gui` でビルドして `gui/dist` に作成します。 @@ -28,6 +31,7 @@ bun run test # complete suite (PR-ready / explicit ask) ```bash bun run typecheck # 厳密な TypeScript 検査 +bun run test:changed # 解決した dev マージベースに対する import graph テスト bun run test # tests/ の全体スイート bun test tests/routing/router.test.ts # 特定テストファイル bun run build:gui # Vite GUI ビルド + パッケージ準備 @@ -35,10 +39,17 @@ bun run privacy:scan # CI で使う資格情報/個人情報検査 bun run prepare:package # パッケージランチャー/asset 更新 ``` +既定では `bun run test` で全テストスイートを実行してください。作業規模、マシンのリソース、 +同時使用中のワークツリーに対して全体実行の負担が過大な場合でも、変更した動作を実際に検証する +回帰テストを最低限実行する必要があります。例は `bun test tests//.test.ts` です。 +範囲を絞った理由、正確なコマンド、結果、未テストの範囲を明記してください。 +`bun run test:changed` は補完に使えますが、すべての間接依存関係を検出するものではありません。 +CI だけに任せたり、ローカルテストを一律に省略したりする例外はありません。マージ前には、 +現在の PR ヘッドの正確なコミットですべての必須 CI チェックが成功している必要があります。 + テストは `src/` を写したドメインディレクトリ(`tests//`)に置かれた Bun テストで、対応表は `scripts/test-layout/layout.json` です。共有 fixture は `tests/helpers/`、範囲の広いネイティブ等価性シナリオは `tests/e2e-style/` にあります。変更した -サブシステムの既存テストの近くに集中した回帰テストを追加してください。共有ルーティング、アダプター、設定、サーバー -動作を触った場合は全体スイートも実行します。 +サブシステムの既存テストの近くに集中した回帰テストを追加してください。 いま読んでいるドキュメントサイトは `docs-site/` にあります(Astro + Starlight)。 @@ -170,6 +181,5 @@ manifest catalog を import したり、Compatibility Lab を有効化したり ## 完了を主張する前に検証 -変更を証明する最も狭いコマンドから実行してください。型は `bun run typecheck`、動作は集中した -`bun test tests/.test.ts` またはランタイム probe で確認した後、影響範囲に応じた広い gate を -実行します。opencodex は大きな batch より小さく検証可能な commit を好みます。 +上記のテスト方針に従い、型の変更には `bun run typecheck` を含め、影響範囲に必要な検証を +実行してください。コマンド、結果、未テストの範囲を報告し、実際に完了した検証だけを主張してください。 diff --git a/docs-site/src/content/docs/ko/contributing.md b/docs-site/src/content/docs/ko/contributing.md index f6eee2bc16..cf6821ec72 100644 --- a/docs-site/src/content/docs/ko/contributing.md +++ b/docs-site/src/content/docs/ko/contributing.md @@ -9,14 +9,17 @@ description: opencodex 개발 환경, 구조, 컨벤션, 프로바이더와 어 git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # post-merge 설치 및 기존 관리형 pre-push 제거 bun run dev:proxy # 개발 모드 프록시 API bun run dev:gui # 대시보드 dev 서버(다른 터미널) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # 전체 테스트 스위트 (기본) ``` +`bun run setup:hooks`는 `post-merge`만 설치하고, 수정되지 않은 기존 관리형 `pre-push` 훅을 +제거합니다. 사용자 정의 훅은 보존합니다. `pre-push` 훅은 더 이상 필수가 아니며, +`bun run prepush`는 선택적으로 직접 실행할 수 있습니다. + `bun run dev`는 계속 `bun run dev:proxy`의 별칭으로 동작합니다. 대시보드 dev 서버는 `bun run dev:gui`이며, `GET /`에서 제공하는 패키지 대시보드는 `bun run build:gui`로 빌드해 `gui/dist`에 만듭니다. @@ -28,6 +31,7 @@ bun run test # complete suite (PR-ready / explicit ask) ```bash bun run typecheck # 엄격한 TypeScript 검사 +bun run test:changed # dev merge base 기준 import graph 테스트 bun run test # tests/ 전체 스위트 bun test tests/routing/router.test.ts # 특정 테스트 파일 bun run build:gui # Vite GUI 빌드 + 패키지 준비 @@ -35,10 +39,17 @@ bun run privacy:scan # CI에서 쓰는 자격 증명/개인정보 bun run prepare:package # 패키지 런처/asset 갱신 ``` +기본적으로 `bun run test`로 전체 테스트 스위트를 실행하세요. 작업 규모, 머신 자원 또는 동시에 +사용 중인 워크트리 때문에 전체 실행 비용이 지나치게 크다면, 최소한 변경한 동작을 실제로 검증하는 +집중 회귀 테스트를 실행해야 합니다. 예를 들어 `bun test tests//.test.ts`를 사용할 수 +있습니다. 범위를 줄인 이유와 정확한 실행 명령, 결과, 테스트하지 않은 범위를 명시하세요. +`bun run test:changed`는 보완 수단이며 모든 간접 의존성을 찾지는 못합니다. CI에만 맡기거나 +로컬 테스트를 생략하는 일괄 면제는 없습니다. 병합 전에는 현재 PR 헤드의 정확한 커밋에서 +모든 필수 CI 검사가 통과해야 합니다. + 테스트는 `src/`를 따라 나눈 도메인 디렉터리(`tests//`)에 놓인 Bun 테스트이며, 지도는 `scripts/test-layout/layout.json`입니다. 공용 fixture는 `tests/helpers/`, 범위가 넓은 네이티브 동등성 시나리오는 `tests/e2e-style/`에 있습니다. 바꾼 -subsystem의 기존 테스트 근처에 집중된 회귀 테스트를 추가하세요. 공용 라우팅, 어댑터, 설정, 서버 -동작을 건드렸다면 전체 스위트도 실행합니다. +subsystem의 기존 테스트 근처에 집중된 회귀 테스트를 추가하세요. 지금 읽고 있는 문서 사이트는 `docs-site/`에 있습니다(Astro + Starlight). @@ -168,6 +179,5 @@ catalog를 import하거나 Compatibility Lab을 활성화해서는 안 됩니다 ## 완료를 주장하기 전에 검증하기 -변경을 증명하는 가장 좁은 명령부터 실행하세요. 타입은 `bun run typecheck`, 동작은 집중된 -`bun test tests/.test.ts` 또는 런타임 probe로 확인한 뒤 영향 범위에 맞는 넓은 gate를 -실행합니다. opencodex는 큰 batch보다 작고 검증 가능한 commit을 선호합니다. +위 테스트 정책을 따르고, 타입 변경에는 `bun run typecheck`를 포함해 영향 범위에 필요한 검사를 +실행하세요. 실행 명령, 결과, 테스트하지 않은 범위를 보고하고 실제로 완료한 검증만 주장하세요. diff --git a/docs-site/src/content/docs/ru/contributing.md b/docs-site/src/content/docs/ru/contributing.md index a1857f6821..3671966569 100644 --- a/docs-site/src/content/docs/ru/contributing.md +++ b/docs-site/src/content/docs/ru/contributing.md @@ -9,14 +9,17 @@ description: Разработка opencodex — настройка окруже git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # установить post-merge и удалить прежний управляемый pre-push bun run dev:proxy # прокси-API в режиме разработки bun run dev:gui # dev-сервер дашборда (другой терминал) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # полный набор тестов (по умолчанию) ``` +`bun run setup:hooks` устанавливает только `post-merge` и удаляет прежний управляемый хук +`pre-push`, если он не был изменён. Пользовательские хуки сохраняются. Хук `pre-push` больше +не обязателен; `bun run prepush` остаётся необязательной ручной проверкой. + `bun run dev` остаётся псевдонимом для `bun run dev:proxy`. Dev-сервер дашборда — `bun run dev:gui`; упакованный дашборд, доступный по `GET /`, собирается командой `bun run build:gui` (`gui/dist`). @@ -27,6 +30,7 @@ bun run test # complete suite (PR-ready / explicit ask) ```bash bun run typecheck # строгая проверка TypeScript +bun run test:changed # тесты графа импортов относительно найденного merge-base dev bun run test # полный набор tests/ bun test tests/routing/router.test.ts # отдельный тестовый файл bun run build:gui # сборка GUI на Vite + подготовка пакета @@ -34,10 +38,18 @@ bun run privacy:scan # проверка учётных данных bun run prepare:package # обновление лаунчеров/ресурсов пакета ``` +По умолчанию запускайте `bun run test`. Если полный прогон несоразмерно затратен с учётом +размера задачи, ресурсов машины или одновременно используемых рабочих деревьев, обязательно +выполните хотя бы целевые регрессионные тесты, проверяющие изменённое поведение, например +`bun test tests//.test.ts`. Объясните причину сокращения прогона и укажите точные +команды, результаты и непроверенную область. `bun run test:changed` дополняет проверку, но не +обнаруживает все косвенные зависимости. Нельзя устанавливать общее исключение, позволяющее +полагаться только на CI или пропускать локальные тесты. Перед слиянием все обязательные проверки +CI должны успешно завершиться для точного текущего коммита HEAD pull request. + Bun-тесты лежат в доменных каталогах, повторяющих `src/` (`tests//`); карта — `scripts/test-layout/layout.json`. В `tests/helpers/` лежат общие fixtures, а в `tests/e2e-style/` — более широкие сценарии нативного паритета. Добавляйте сфокусированный -регрессионный тест рядом с существующими тестами изменяемой подсистемы; если затронуты общая -маршрутизация, адаптеры, конфигурация или поведение сервера, запускайте полный набор. +регрессионный тест рядом с существующими тестами изменяемой подсистемы. Сайт документации, который вы сейчас читаете, находится в `docs-site/` (Astro + Starlight): @@ -173,7 +185,6 @@ startup path не должны импортировать каталог ман ## Проверяйте, прежде чем объявлять работу завершённой -Запускайте самую узкую команду, которая доказывает ваше изменение: `bun run typecheck` для типов, -сфокусированный `bun test tests/.test.ts` или runtime-проверку для поведения, а затем более -широкие проверки, соответствующие затронутой области. opencodex предпочитает небольшие проверяемые -коммиты крупным пачкам изменений. +Следуйте политике тестирования выше и выполняйте `bun run typecheck` при изменении типов, +а также проверки для затронутой области. Указывайте команды, результаты и непроверенную область; +заявляйте только о реально выполненной проверке. diff --git a/docs-site/src/content/docs/tr/contributing.md b/docs-site/src/content/docs/tr/contributing.md index cd00f95262..0174209b26 100644 --- a/docs-site/src/content/docs/tr/contributing.md +++ b/docs-site/src/content/docs/tr/contributing.md @@ -14,14 +14,17 @@ aracının bulunması gerekir. Yayınlanan npm paketi kullanıcılar için kendi git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # post-merge kur ve eski yönetilen pre-push kancasını kaldır bun run dev:proxy # geliştirme modunda proxy API bun run dev:gui # kontrol paneli geliştirme sunucusu (başka bir terminalde) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # tam test paketi (varsayılan) ``` +`bun run setup:hooks` yalnızca `post-merge` kancasını kurar ve değiştirilmemiş eski yönetilen +`pre-push` kancasını kaldırır; özel kancaları korur. `pre-push` kancası artık zorunlu değildir. +`bun run prepush` isteğe bağlı bir manuel denetim olarak kullanılabilir. + `bun run dev`, `bun run dev:proxy` komutunun bir takma adıdır. Kontrol paneli geliştirme sunucusu `bun run dev:gui` ile çalışır; `GET /` adresindeki paketlenmiş kontrol paneli ise `bun run build:gui` (`gui/dist`) tarafından @@ -34,6 +37,7 @@ Yerel komutların CI ile eşleşmesi için depodaki betikleri kullanın: ```bash bun run typecheck # katı TypeScript denetimi +bun run test:changed # çözümlenen dev merge-base için import grafiği testleri bun run test # tests/ paketinin tamamı bun test tests/routing/router.test.ts # odaklanmış test dosyası bun run build:gui # Vite GUI derlemesi + paket hazırlığı @@ -41,12 +45,19 @@ bun run privacy:scan # CI tarafından kullanılan kimlik/gizlilik t bun run prepare:package # paket başlatıcılarını ve varlıklarını yenileme ``` +Varsayılan olarak `bun run test` çalıştırın. Tam çalıştırma görevin boyutu, makine kaynakları +veya eşzamanlı kullanılan çalışma ağaçları nedeniyle orantısız derecede maliyetliyse, en azından +değişen davranışı gerçekten sınayan odaklanmış regresyon testlerini çalıştırmanız gerekir; örneğin +`bun test tests//.test.ts`. Kapsamı daraltma nedenini, tam komutları, sonuçları ve +test edilmeyen kapsamı açıklayın. `bun run test:changed` kapsamı destekleyebilir ancak tüm dolaylı +bağımlılıkları bulamaz. Yalnızca CI sonucuna güvenmek veya yerel testleri atlamak için genel bir +muafiyet yoktur. Birleştirmeden önce tüm zorunlu CI denetimleri PR’ın mevcut başındaki tam commit +için başarılı olmalıdır. + Bun testleri `src/` yapısını yansıtan alan dizinlerinde (`tests//`) bulunur; harita `scripts/test-layout/layout.json` dosyasıdır. `tests/helpers/` paylaşılan test ortamlarını (fixtures) ve `tests/e2e-style/` daha geniş yerel parite senaryolarını içerir. Değiştirdiğiniz alt sistemin mevcut testlerinin -yakınında odaklanmış bir regresyon testi bulundurun; paylaşılan yönlendirme, -adaptörler, yapılandırma veya sunucu davranışları için test paketinin tamamını -çalıştırın. +yakınında odaklanmış bir regresyon testi bulundurun. Okumakta olduğunuz dokümantasyon sitesi `docs-site/` (Astro + Starlight) dizinindedir: @@ -263,7 +274,6 @@ fabrikayı `src/index.ts` dosyasından dışa aktarın. ## Bittiğini iddia etmeden önce doğrulayın -Değişikliğinizi kanıtlayan en dar komutu çalıştırın — tipler için `bun run -typecheck`, davranış için odaklanmış bir `bun test tests/.test.ts` veya -çalışma zamanı probu, ardından etkilenen yüzeye uygun daha geniş kapılar. -opencodex büyük partiler yerine küçük, doğrulanabilir commit'leri tercih eder. +Yukarıdaki test politikasını izleyin; tip değişiklikleri için `bun run typecheck` ve etkilenen +alanın gerektirdiği denetimleri çalıştırın. Komutları, sonuçları ve test edilmeyen kapsamı +bildirin; yalnızca gerçekten tamamlanan doğrulamaları belirtin. diff --git a/docs-site/src/content/docs/tr/contributing/pr-quality.md b/docs-site/src/content/docs/tr/contributing/pr-quality.md index 3248aa7a2a..16f747bc49 100644 --- a/docs-site/src/content/docs/tr/contributing/pr-quality.md +++ b/docs-site/src/content/docs/tr/contributing/pr-quality.md @@ -55,7 +55,8 @@ tam olarak neyi değiştirmeniz gerektiğini söyler: edemez. Katkıda bulunan PR'ları (depo yazma izni olmayan yazarlar) taslak olarak açılır ve açıklamadaki dört kutulu incelemeye hazırlık kontrol listesi tamamlanana -kadar orada kalır: yerel CI yeşil, dal en son `dev` commit'inde, tüm doğru Codex +kadar orada kalır: gerekli yerel doğrulama başarılı +(komutlar, sonuçlar ve tam test paketi istisnaları belgelenmiş), dal en son `dev` commit'inde, tüm doğru Codex ve CodeRabbit bulguları düzeltildi ve incelemeye hazır onayı. Her kutu işaretlendikten sonra kontrol, PR'ı incelemeye hazır olarak işaretler ve `MAINTAINERS.md` dosyasında listelenen bakımcıları bilgilendirir (yazar hariç). @@ -71,7 +72,7 @@ Bir tamamlama kabul edilmeden önce kapı, kontrol listesinin kendisinin kontrol edebileceği iddiaları doğrular: dal en son `dev` commit'inde veya en fazla 10 commit gerisinde olmalı ve geçerli head üzerinde bir inceleme botu tarafından yazılan her Codex ve CodeRabbit inceleme konusu çözülmelidir (diğer yazarların -çözülmemiş konuları engellemez). Yerel CI kutusu yalnızca bir yazar beyanıdır — +çözülmemiş konuları engellemez). Gerekli yerel doğrulama kutusu yalnızca bir yazar beyanıdır — fork katkıda bulunanları depo CI'ını başlatamaz; bir bakımcının başlatması gerekir — bu nedenle kapı bunu asla çürütmez; yeni bir push yine de her kutuyu sıfırlar. Fark aralığının dışına düşen ve yalnızca geçerli head üzerindeki bir @@ -93,8 +94,8 @@ PR-head kodu yürütülmez. lint veya tip bastırmaları, odaklanmış veya atlanmış testler, boş catch blokları, düzenlenen üretilmiş çıktılar ve manifestosu olmadan değiştirilen bir kilit dosyası (lockfile) açık bir onay etiketine ihtiyaç duyar. Bir kaynak - dosyadaki yalnızca yorum değişikliği bir davranış değişikliği değildir ve test - gerektirmez. + dosyadaki yalnızca yorum değişikliği bir davranış değişikliği değildir ve yeni + bir regresyon testi gerektirmez. [Yerel test politikası](/tr/contributing/) yine geçerlidir. - **Çapraz platform CI.** Test paketi her çekme isteği için Linux'ta parçalı (sharded) ve macOS'ta tam olarak çalışır. Windows, dağıtım sınırında çalışır — `main` veya `preview` dalına yükseltmede — bu nedenle yavaş veya kararsız bir @@ -142,3 +143,13 @@ değildir: belirtilen neden çözüldükten sonra yeniden açın veya temiz bir tanesiyle değiştirin. Neden açık değilse sorun. + +## Eski bir inceleme hazırlığı listesini güncelleme + +Denetim eski yerel CI ifadesini bulursa PR açıklamasını değiştirmez. İlk maddeyi botun +belirttiği metinle değiştirin, dört kutunun işaretini kaldırın ve kaydedin. Bot bu adımı +onaylayana kadar bekleyin; gösterilen commit’i doğrulayın, dört kutuyu işaretleyin ve yeniden +kaydedin. Yalnızca metni değiştirip dört işareti korumak yeni doğrulama sayılmaz. Yeni bir +push veya hedef dal değişikliği bu kaydı geçersiz kılar. Bu işlem ve normal kalite kontrolleri +tamamlanana kadar denetim başarısız, PR taslak kalır. Kayıt zamanı kontrol noktasıyla aynıysa +açıklama gövdesini tekrar düzenleyip daha sonra kaydedin; yalnızca başlığı değiştirmek yetmez. diff --git a/docs-site/src/content/docs/zh-cn/contributing.md b/docs-site/src/content/docs/zh-cn/contributing.md index a25ea4fcb1..12f87c05c1 100644 --- a/docs-site/src/content/docs/zh-cn/contributing.md +++ b/docs-site/src/content/docs/zh-cn/contributing.md @@ -9,14 +9,16 @@ description: opencodex 的开发环境、结构、约定,以及添加 provider git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # 安装 post-merge 并移除旧的托管 pre-push bun run dev:proxy # 开发模式代理 API bun run dev:gui # 仪表盘 dev 服务器(另一个终端) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # 完整测试套件(默认) ``` +`bun run setup:hooks` 仅安装 `post-merge`,并移除未经修改的旧版托管 `pre-push` 钩子, +保留自定义钩子。`pre-push` 钩子不再是必需项;`bun run prepush` 仍可作为可选的手动检查。 + `bun run dev` 继续作为 `bun run dev:proxy` 的别名。仪表盘 dev 服务器使用 `bun run dev:gui`; `GET /` 提供的打包仪表盘由 `bun run build:gui` 构建到 `gui/dist`。 @@ -27,6 +29,7 @@ bun run test # complete suite (PR-ready / explicit ask) ```bash bun run typecheck # 严格 TypeScript 检查 +bun run test:changed # 针对解析出的 dev merge-base 的导入图测试 bun run test # 完整 tests/ suite bun test tests/routing/router.test.ts # 聚焦单个测试文件 bun run build:gui # Vite GUI 构建 + package 准备 @@ -34,9 +37,15 @@ bun run privacy:scan # CI 使用的 credential/privacy 扫描 bun run prepare:package # 刷新 package launcher/asset ``` +默认运行 `bun run test` 执行完整测试套件。如果相对于任务规模、机器资源或并行使用的工作树, +完整运行的成本过高,仍必须至少运行实际验证变更行为的针对性回归测试,例如 +`bun test tests//.test.ts`。说明缩小范围的原因,并报告准确的命令、结果和未测试范围。 +`bun run test:changed` 可以补充覆盖,但无法发现所有间接依赖。不存在仅依赖 CI 或完全跳过本地测试 +的一概豁免。合并前,所有必需的 CI 检查必须在当前 PR 头部的确切提交上通过。 + 测试是按 `src/` 划分的领域目录(`tests//`)下的 Bun test,映射表在 `scripts/test-layout/layout.json`。`tests/helpers/` 存放共享 fixture, `tests/e2e-style/` 存放范围更广的原生一致性场景。请在对应 subsystem 的现有测试附近加入聚焦的 -回归测试;若改动涉及共享 routing、adapter、config 或 server 行为,还应运行完整 suite。 +回归测试。 你正在阅读的文档站点位于 `docs-site/`(Astro + Starlight): @@ -159,6 +168,5 @@ package API,还要从 `src/index.ts` export。 ## 在声称完成前先验证 -先运行能证明改动的最小命令:类型检查用 `bun run typecheck`,行为检查用聚焦的 -`bun test tests/.test.ts` 或 runtime probe,然后再执行适合影响范围的更宽 gate。 -opencodex 倾向于小而可验证的 commit,而不是大批量改动。 +遵循上述测试政策,并针对类型变更运行 `bun run typecheck`,以及受影响范围所需的其他检查。 +报告命令、结果和未测试范围,只声明实际完成的验证。 diff --git a/docs-site/src/content/docs/zh-tw/contributing.md b/docs-site/src/content/docs/zh-tw/contributing.md index 86931c1ff6..0ae23c2329 100644 --- a/docs-site/src/content/docs/zh-tw/contributing.md +++ b/docs-site/src/content/docs/zh-tw/contributing.md @@ -9,14 +9,16 @@ description: opencodex 的開發環境、結構、約定,以及新增 provider git clone https://github.com/lidge-jun/opencodex.git cd opencodex bun install +bun run setup:hooks # 安裝 post-merge 並移除舊的受管理 pre-push bun run dev:proxy # 開發模式代理 API bun run dev:gui # 儀表板 dev 伺服器(另一個終端) bun run typecheck # bun x tsc --noEmit -bun run test:changed # routine import-graph test selection -bun test tests/routing/router.test.ts # routine focused test -bun run test # complete suite (PR-ready / explicit ask) +bun run test # 完整測試套件(預設) ``` +`bun run setup:hooks` 僅安裝 `post-merge`,並移除未經修改的舊版受管理 `pre-push` 掛鉤, +保留自訂掛鉤。`pre-push` 掛鉤不再是必要項目;`bun run prepush` 仍可作為選用的手動檢查。 + `bun run dev` 繼續作為 `bun run dev:proxy` 的別名。儀表板 dev 伺服器使用 `bun run dev:gui`; `GET /` 提供的打包儀表板由 `bun run build:gui` 建置到 `gui/dist`。 @@ -27,6 +29,7 @@ bun run test # complete suite (PR-ready / explicit ask) ```bash bun run typecheck # 嚴格 TypeScript 檢查 +bun run test:changed # 針對解析出的 dev merge-base 的匯入圖測試 bun run test # 完整 tests/ suite bun test tests/routing/router.test.ts # 聚焦單個測試檔案 bun run build:gui # Vite GUI 建置 + package 準備 @@ -34,9 +37,15 @@ bun run privacy:scan # CI 使用的 credential/privacy 掃描 bun run prepare:package # 重新整理 package launcher/asset ``` +預設執行 `bun run test` 跑完整測試套件。如果相對於工作規模、機器資源或同時使用的工作樹, +完整執行的成本過高,仍必須至少執行實際驗證變更行為的針對性迴歸測試,例如 +`bun test tests//.test.ts`。說明縮小範圍的原因,並報告確切的命令、結果和未測試範圍。 +`bun run test:changed` 可以補充涵蓋範圍,但無法找出所有間接相依性。不存在僅依賴 CI 或完全略過 +本機測試的一概豁免。合併前,所有必要的 CI 檢查必須在目前 PR 頂端的確切提交上通過。 + 測試是按 `src/` 劃分的領域目錄(`tests//`)下的 Bun test,對應表在 `scripts/test-layout/layout.json`。`tests/helpers/` 存放共享 fixture, `tests/e2e-style/` 存放範圍更廣的原生一致性場景。請在對應 subsystem 的現有測試附近加入聚焦的 -迴歸測試;若改動涉及共享 routing、adapter、config 或 server 行為,還應執行完整 suite。 +迴歸測試。 你正在閱讀的文件站點位於 `docs-site/`(Astro + Starlight): @@ -189,6 +198,5 @@ package API,還要從 `src/index.ts` export。 ## 在聲稱完成前先驗證 -先執行能證明改動的最小命令:型別檢查用 `bun run typecheck`,行為檢查用聚焦的 -`bun test tests/.test.ts` 或 runtime probe,然後再執行適合影響範圍的更寬 gate。 -opencodex 傾向於小而可驗證的 commit,而不是大批次改動。 +遵循上述測試政策,並針對型別變更執行 `bun run typecheck`,以及受影響範圍所需的其他檢查。 +報告命令、結果和未測試範圍,只聲明實際完成的驗證。 diff --git a/docs-site/src/content/docs/zh-tw/contributing/pr-quality.md b/docs-site/src/content/docs/zh-tw/contributing/pr-quality.md index 00f20be8ed..cf05c4ea34 100644 --- a/docs-site/src/content/docs/zh-tw/contributing/pr-quality.md +++ b/docs-site/src/content/docs/zh-tw/contributing/pr-quality.md @@ -22,11 +22,11 @@ description: OpenCodex pull request 的審查就緒門檻、貢獻者責任、 有三個決定性的檢查會在人工作業之前執行,每個失敗訊息都會確切告訴你該改什麼: - **PR 品質(`enforce-target`)。** Pull request 必須以 `dev` 為目標,並帶有真正的描述:變更內容與原因的 **Summary**,加上 **Test plan**(或同等實質內容)。當 diff 更動 `gui/` 下的檔案,或 GitHub 對大型 diff 回傳不完整的變更檔清單時,描述必須包含 UI 變更的截圖;檢查會讓 PR 維持 draft 並留言,直到截圖出現。不完整的檔案清單會保守地視為 GUI 變更。維護者可以針對 `gui/` 變更、GUI 路徑分類誤判、或不完整檔案清單的誤判,加上 `gui-screenshot-waived` label 來豁免截圖要求;新增或移除該 label 會立即重新評估 gate。舊式維護者留言(例如「no gui changes」)在下次 PR 事件時仍會為相容性而辨識,但留言本身不再觸發這個特權 PR gate。貢獻者不能自行豁免截圖要求。 - 沒有 repository push 權限的貢獻者 PR 會以 draft 開啟,並維持 draft 直到描述中的四個格子的 review-ready 檢查清單完成:本機 CI 通過、分支位於最新 `dev` commit、所有正確的 Codex 與 CodeRabbit 發現都已修正、以及 ready-for-review 確認。當每個格子都勾選後,檢查會把 PR 標記為可審查,並通知 `MAINTAINERS.md` 中列出的維護者(不含作者)。gate 的狀態與「該做什麼」集中在單一 bot 留言中,每次執行都會重寫,所以只需看一個地方。完成綁定在 PR head 所指的確切 commit:如果之後又推出新 commit,gate 會把 PR 移回 draft、重設檢查清單與維護者通知,並要求你針對最新程式碼再次測試並勾選。重新定位到 `dev` 會自動清除錯誤分支訊息,並被 gate 記住;draft 會一直持續到檢查清單完成。 - 在接受完成之前,gate 會驗證它能自行檢查的檢查清單聲明:分支必須位於最新 `dev` commit 或落後最多 10 個 commit,而且目前 head 上所有由 review bot 撰寫的 Codex 與 CodeRabbit review thread 都必須已解決(其他作者未解決的 thread 不會阻擋)。本機 CI 的格子只是作者的 attestation——fork 貢獻者無法啟動 repository CI,必須由維護者啟動——所以 gate 永遠不會反駁它;新的 push 仍會重設每個格子。落在 diff 範圍之外、且只在目前 head 的 review body 中回報的 CodeRabbit 發現,在 bot review thread 開啟期間會計入未解決數;解決所有 bot thread 即可清除該格子。被反駁的聲明會取消勾選對應的格子,並讓 PR 維持 draft。當檢查清單完成且所有 gate 都綠燈時,gate 會加上 `review-ready` label,作為就緒時刻的可見狀態標記。 + 沒有 repository push 權限的貢獻者 PR 會以 draft 開啟,並維持 draft 直到描述中的四個格子的 review-ready 檢查清單完成:必要的本機驗證通過,並記錄命令、結果及任何完整套件例外、分支位於最新 `dev` commit、所有正確的 Codex 與 CodeRabbit 發現都已修正、以及 ready-for-review 確認。當每個格子都勾選後,檢查會把 PR 標記為可審查,並通知 `MAINTAINERS.md` 中列出的維護者(不含作者)。gate 的狀態與「該做什麼」集中在單一 bot 留言中,每次執行都會重寫,所以只需看一個地方。完成綁定在 PR head 所指的確切 commit:如果之後又推出新 commit,gate 會把 PR 移回 draft、重設檢查清單與維護者通知,並要求你針對最新程式碼再次測試並勾選。重新定位到 `dev` 會自動清除錯誤分支訊息,並被 gate 記住;draft 會一直持續到檢查清單完成。 + 在接受完成之前,gate 會驗證它能自行檢查的檢查清單聲明:分支必須位於最新 `dev` commit 或落後最多 10 個 commit,而且目前 head 上所有由 review bot 撰寫的 Codex 與 CodeRabbit review thread 都必須已解決(其他作者未解決的 thread 不會阻擋)。必要本機驗證的格子只是作者的 attestation——fork 貢獻者無法啟動 repository CI,必須由維護者啟動——所以 gate 永遠不會反駁它;新的 push 仍會重設每個格子。落在 diff 範圍之外、且只在目前 head 的 review body 中回報的 CodeRabbit 發現,在 bot review thread 開啟期間會計入未解決數;解決所有 bot thread 即可清除該格子。被反駁的聲明會取消勾選對應的格子,並讓 PR 維持 draft。當檢查清單完成且所有 gate 都綠燈時,gate 會加上 `review-ready` label,作為就緒時刻的可見狀態標記。 CodeRabbit 的狀態留言編輯不會觸發 PR gate。CodeRabbit 成功的 `CodeRabbit` commit status 會透過 `status` 事件喚醒受信任的預設分支 gate。gate 將該 status SHA 對應到確切一個目前 head 仍相符的 open PR,然後在變更檢查清單、label、留言或 draft 狀態之前,重新讀取即時的 review thread 與 review body。模糊或過時的 SHA 關聯會被忽略,且不會以 gate 的具寫入權限 token 執行任何 PR head 程式碼。 -- **Hygiene。** 行為變更需要測試;新增 lint 或 type suppression、聚焦或跳過的測試、空的 catch 區塊、編輯產生的輸出,以及未隨 manifest 一起變更的 lockfile,每項都需要明確的核准 label。僅對原始檔做留言層級的變更不算行為變更,也不需要測試。 +- **Hygiene。** 行為變更需要測試;新增 lint 或 type suppression、聚焦或跳過的測試、空的 catch 區塊、編輯產生的輸出,以及未隨 manifest 一起變更的 lockfile,每項都需要明確的核准 label。僅對原始檔做留言層級的變更不算行為變更,也不需要新增迴歸測試。[本機測試政策](/zh-tw/contributing/) 仍然適用。 - **跨平台 CI。** 每個 pull request 的測試套件在 Linux 上分片執行,並在 macOS 上完整執行。Windows 在釋出邊界執行——即提升到 `main` 或 `preview` 時——所以慢速或不穩定的 Windows runner 不能決定你的 pull request 何時變綠。 這對**每個** pull request 都執行,無論其 base 分支為何——包括 base 是另一個 open PR head 的 stacked child。由 `paths:` filter,而非 base 分支,決定 jobs 是否執行:只碰 docs 或 `devlog/` 的 PR 不會佇列任何 job。 @@ -45,3 +45,12 @@ CodeRabbit 會 review 每個 PR,其發現僅供參考。它說對的就照做 ## 當 pull request 被關閉時 停滯且帶著未解決 review 回饋的 PR 可能被關閉,並會清楚陳述原因。關閉不是對貢獻者的判決:一旦陳述的原因解決,就重新開啟它,或用乾淨的 PR 取代。若原因不清楚,請詢問。 + +## 更新舊版審查準備清單 + +若檢查指出清單仍使用舊版「本機 CI 通過」文字,機器人會保留您的 PR 說明。 +請將第一項改成機器人留言指定的文字,取消全部四個勾選並儲存。等待機器人確認 +已記錄此步驟後,驗證留言顯示的 commit,再勾選四項並重新儲存。只改文字而保留 +全部勾選,不算重新確認。新的 push 或目標分支變更會使記錄失效。完成此流程與 +一般品質檢查前,檢查保持失敗,PR 保持草稿。如果儲存時間與檢查點相同,請稍後 +再次修改說明內文並儲存;只修改標題不算。 diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index da4c2d0a16..858db0ff14 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -23,5 +23,5 @@ This file applies to `scripts/` and inherits the repository-wide rules in `/AGEN - Run focused tests or probes for the changed script. - Run `bun run typecheck`. - Run `bun run privacy:scan` when the script handles configuration, credentials, requests, logs, or account data. -- Run `bun run prepush` for release, packaging, dependency, or cross-platform tooling changes. +- Follow the root validation policy: run the suite by default; if a full run is too costly, run at least focused regression tests and document the reason and remaining coverage. `bun run prepush` is available as an explicit comprehensive check. - Report any platform-specific validation that was not executed. diff --git a/scripts/build-gui-if-changed.ts b/scripts/build-gui-if-changed.ts index c48badcfa7..cfa083ddaa 100644 --- a/scripts/build-gui-if-changed.ts +++ b/scripts/build-gui-if-changed.ts @@ -1,6 +1,6 @@ /** * Rebuild the packaged GUI when a merge or pull brought `gui/` changes. - * Used by the `post-merge` git hook. Skip with: git pull --no-verify + * Used by the `post-merge` git hook. * * Why this exists: `ocx` serves `gui/dist`, which is generated output and * therefore gitignored. A fast-forward advances `gui/src` but leaves `gui/dist` diff --git a/scripts/ci/sample-macos-stall.sh b/scripts/ci/sample-macos-stall.sh new file mode 100644 index 0000000000..e6357f5e14 --- /dev/null +++ b/scripts/ci/sample-macos-stall.sh @@ -0,0 +1,152 @@ +#!/usr/bin/env bash +# Observe a silent suite; never signal it, retry it, or change its exit status. +set -u +owner=${1:?owning shell pid required} +suite_log=${2:?suite log required} +case "$owner" in ''|*[!0-9]*) exit 64 ;; esac +[ "$(uname -s)" = Darwin ] || exit 0 +owner_started=$(ps -p "$owner" -o lstart= 2>/dev/null) || exit 0 +[ -n "$owner_started" ] || exit 0 +observer_child= +stop_requested=0 +sample_path= +sample_output_path= +sample_redacted_path= +group_open=0 +remove_sample_files() { + cleanup_failed=0 + [ -z "$sample_path" ] || [ ! -f "$sample_path" ] || rm "$sample_path" 2>/dev/null || cleanup_failed=1 + [ -z "$sample_output_path" ] || [ ! -f "$sample_output_path" ] || rm "$sample_output_path" 2>/dev/null || cleanup_failed=1 + [ -z "$sample_redacted_path" ] || [ ! -f "$sample_redacted_path" ] || rm "$sample_redacted_path" 2>/dev/null || cleanup_failed=1 + sample_path= + sample_output_path= + sample_redacted_path= + [ "$cleanup_failed" -eq 0 ] || echo "::warning::macOS diagnostic temporary-file cleanup failed" +} +stop_observer() { + # Only the observer's current sleep/sample child is owned here; never the suite. + if [ -n "$observer_child" ] && jobs -pr | grep -qx "$observer_child"; then + # The shell's still-running child job is the ownership handle. Unlike comm, + # that handle does not change when the forked child execs sleep or sample. + kill "$observer_child" 2>/dev/null || true + wait "$observer_child" 2>/dev/null || true + fi + remove_sample_files + [ "$group_open" -eq 0 ] || echo "::endgroup::" + exit 0 +} +# A trap only records intent: cleanup occurs after the child job PID is captured, +# never in the spawn/assignment gap or with a reaped PID left over from an old child. +trap 'stop_requested=1' TERM INT +run_observer_child() { + local observer_output + observer_output=$1 + shift + if [ -n "$observer_output" ]; then + { exec "$@" > "$observer_output" 2>&1; } 2>/dev/null & + else + "$@" & + fi + observer_child=$! + [ "$stop_requested" -eq 0 ] || stop_observer + wait "$observer_child" || true + [ "$stop_requested" -eq 0 ] || stop_observer + observer_child= +} +redact_diagnostic_paths() { + REDACT_WORKSPACE=${GITHUB_WORKSPACE:-} REDACT_HOME=${HOME:-} awk ' + function replace_literal(value, needle, replacement, position, output) { + if (needle == "") return value + output = "" + while ((position = index(value, needle)) != 0) { + output = output substr(value, 1, position - 1) replacement + value = substr(value, position + length(needle)) + } + return output value + } + { + workspace = ENVIRON["REDACT_WORKSPACE"] + home = ENVIRON["REDACT_HOME"] + if (length(workspace) >= length(home)) { + line = replace_literal($0, workspace, "${GITHUB_WORKSPACE}") + line = replace_literal(line, home, "${HOME}") + } else { + line = replace_literal($0, home, "${HOME}") + line = replace_literal(line, workspace, "${GITHUB_WORKSPACE}") + } + print line + } + ' +} +last_bytes=$(wc -c < "$suite_log") || exit 0 +quiet=0 +while kill -0 "$owner" 2>/dev/null; do + run_observer_child "" sleep 15 + [ "$stop_requested" -eq 0 ] || stop_observer + [ "$(ps -p "$owner" -o lstart= 2>/dev/null)" = "$owner_started" ] || exit 0 + [ -f "$suite_log" ] || continue + bytes=$(wc -c < "$suite_log") || exit 0 + if [ "$bytes" != "$last_bytes" ]; then + last_bytes=$bytes + quiet=0 + continue + fi + quiet=$((quiet + 15)) + [ "$quiet" -ge 60 ] || continue + + # Keep comm internal for conservative Bun ownership matching. Diagnostic output + # emits basename-only identities so executable paths never enter the CI log. + processes=$(ps -axo pid=,ppid=,comm=) || exit 0 + candidates=$(printf '%s\n' "$processes" | awk -v owner="$owner" '$2 == owner && $NF ~ /(^|\/)bun$/ { print $1 }') + count=$(printf '%s\n' "$candidates" | awk 'NF { n++ } END { print n+0 }') + if [ "$count" -ne 1 ]; then + echo "::warning::macOS suite silent for ${quiet}s; direct Bun owner ambiguous (${count} candidates); no sampling" + exit 0 + fi + suite_pid=$candidates + suite_started=$(ps -p "$suite_pid" -o lstart= 2>/dev/null) || exit 0 + [ -n "$suite_started" ] || exit 0 + echo "::group::macOS silent-suite diagnostics (${quiet}s without output)" + group_open=1 + printf '%s\n' "$processes" | awk -v root="$suite_pid" ' + { + pid[NR]=$1; parent[NR]=$2 + command[NR]=$0 + sub(/^[[:space:]]*[0-9]+[[:space:]]+[0-9]+[[:space:]]+/, "", command[NR]) + } + END { + owned[root]=1 + for (pass=0; pass<16; pass++) for (i=1; i<=NR; i++) if (owned[parent[i]]) owned[pid[i]]=1 + for (i=1; i<=NR; i++) if (owned[pid[i]]) { + count=split(command[i], parts, "/") + print pid[i], parent[i], parts[count] + } + }' + # Request one three-second read-only sample while the suite is still stuck. + # A successful later run cannot replace this evidence. + if [ "$(ps -p "$suite_pid" -o ppid= 2>/dev/null | tr -d ' ')" = "$owner" ] && + [ "$(ps -p "$suite_pid" -o lstart= 2>/dev/null)" = "$suite_started" ]; then + sample_path="${suite_log}.sample" + sample_output_path="${suite_log}.sample-output" + sample_redacted_path="${suite_log}.sample-redacted" + run_observer_child "$sample_output_path" sample "$suite_pid" 3 -file "$sample_path" + if { + set -e + [ ! -f "$sample_output_path" ] || printf '%s\n' 'sample command output:' + [ ! -f "$sample_output_path" ] || sed -n '1,$p' "$sample_output_path" + [ ! -f "$sample_path" ] || printf '%s\n' 'sample report:' + [ ! -f "$sample_path" ] || sed -n '1,$p' "$sample_path" + } 2>/dev/null | redact_diagnostic_paths 2>/dev/null > "$sample_redacted_path"; then + if ! head -c 262144 "$sample_redacted_path" 2>/dev/null; then + echo "::warning::macOS diagnostic capped emission failed" + fi + printf '\n' + else + echo "::warning::macOS diagnostic redaction failed; sample output omitted" + fi + remove_sample_files + fi + echo "::endgroup::" + group_open=0 + exit 0 +done diff --git a/scripts/doctor-gui-if-changed.ts b/scripts/doctor-gui-if-changed.ts index b341e0eab2..dbef9c2876 100644 --- a/scripts/doctor-gui-if-changed.ts +++ b/scripts/doctor-gui-if-changed.ts @@ -1,6 +1,6 @@ /** * Run React Doctor in gui/ when this push includes gui/ changes. - * Used by `bun run prepush`. Skip with: git push --no-verify + * Used by `bun run prepush`. * * Gating by contract (doctor.config.json blocking: "warning"): findings fail * the push. An unavailable engine (offline npx fetch, missing binary) still diff --git a/scripts/lint-gui-if-changed.ts b/scripts/lint-gui-if-changed.ts index 133938a738..fd504489ac 100644 --- a/scripts/lint-gui-if-changed.ts +++ b/scripts/lint-gui-if-changed.ts @@ -1,8 +1,8 @@ /** * Run GUI Oxlint when this push includes gui/ changes. - * Used by `bun run prepush`. Skip with: git push --no-verify + * Used by `bun run prepush`. * - * Mirrors `scripts/doctor-gui-if-changed.ts` so the local pre-push gate and + * Mirrors `scripts/doctor-gui-if-changed.ts` so explicit local validation and * the CI `gates` job agree: GUI lint runs only when the push actually touches * `gui/`. Unlike doctor there is no engine to fetch, so lint findings always * fail the push — there is no infra-degradation path to soft-skip on. diff --git a/scripts/pre-push.sh b/scripts/pre-push.sh deleted file mode 100644 index fa04e570e0..0000000000 --- a/scripts/pre-push.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env sh -# Pre-push hook shim. The actual command list lives in package.json ("prepush"). -# Installed by: bun run setup:hooks -set -e -exec bun run prepush diff --git a/scripts/setup-hooks.ts b/scripts/setup-hooks.ts index c632004dba..57bc8c0262 100644 --- a/scripts/setup-hooks.ts +++ b/scripts/setup-hooks.ts @@ -2,18 +2,16 @@ * Sets up the git hooks for local development. * Run once after cloning: bun run setup:hooks * - * - `pre-push` runs `bun run prepush` (typecheck + tests + privacy scan + GUI - * eslint and React Doctor when `gui/` changed) — the local portion of the CI - * gate. + * - Retires the unmodified repository-managed `pre-push` hook. Validation is + * run explicitly; custom hooks are preserved. * - `post-merge` runs `bun run postmerge`, which rebuilds the packaged GUI when * a merge or pull brought `gui/` changes. `gui/dist` is generated and * gitignored, so a fast-forward advances the source while the dashboard keeps * serving the previously built bundle. - * - * To skip in an emergency: git push --no-verify / git pull --no-verify */ import { execFileSync } from "node:child_process"; -import { existsSync, copyFileSync, mkdirSync, chmodSync, readFileSync, renameSync } from "node:fs"; +import { createHash } from "node:crypto"; +import { existsSync, copyFileSync, mkdirSync, chmodSync, readFileSync, renameSync, lstatSync, unlinkSync } from "node:fs"; import { join, resolve } from "node:path"; const repoRoot = resolve(import.meta.dirname, ".."); @@ -72,15 +70,25 @@ function installHook(name: string, source: string, summary: string): void { console.log(`${name} hook installed at ${dest}. ${summary}`); } -installHook( - "pre-push", - "pre-push.sh", - "Runs typecheck + tests + privacy scan (+ GUI eslint and React Doctor when gui/ changed) before every push.", -); +// Match the exact retired shim (normalizing checkout line endings), never a +// name or a partial marker: a user may have added other work to their hook. +const retiredPrePushSha256 = "2aa6b5f84ab989954d2ccc1a8680d63ad934034778e0ee99c277f8873fd40508"; +const prePushPath = join(hooksDir, "pre-push"); +const prePushStat = lstatSync(prePushPath, { throwIfNoEntry: false }); +if (prePushStat?.isFile()) { + const content = readFileSync(prePushPath, "utf8").replace(/\r\n/g, "\n"); + if (createHash("sha256").update(content).digest("hex") === retiredPrePushSha256) { + unlinkSync(prePushPath); + console.log("Removed the retired repository-managed pre-push hook."); + } else { + console.log("Preserved custom pre-push hook."); + } +} + installHook( "post-merge", "post-merge.sh", "Rebuilds the packaged GUI when a merge or pull brought gui/ changes.", ); -console.log("Skip in an emergency with: git push --no-verify / git pull --no-verify"); +console.log("Run validation explicitly before review; see AGENTS.md for test scope."); diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 236b287f65..f5d260f28a 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -748,6 +748,9 @@ "digitalocean-scaleway-provider.test.ts": "providers", "docs-429-failover-claims.test.ts": "ci-workflows", "docs-bun-source-requirement.test.ts": "ci-workflows", + "setup-hooks.test.ts": "ci-workflows", + "pr-readiness-reattest.test.ts": "ci-workflows", + "exhaustive-deps-suppression.test.ts": "ci-workflows", "docs-provider-billing-claims.test.ts": "ci-workflows", "docs-provider-preset-counts.test.ts": "ci-workflows", "docs-readme-translation-parity.test.ts": "ci-workflows", diff --git a/structure/ops/docs-and-release.md b/structure/ops/docs-and-release.md index dcd15fe1ba..5150826204 100644 --- a/structure/ops/docs-and-release.md +++ b/structure/ops/docs-and-release.md @@ -1,5 +1,13 @@ # Docs And Release +macOS shards and control use the shared fresh-process batch runner described below. +`scripts/ci/sample-macos-stall.sh` remains a standalone diagnostic helper with isolated +observer regression coverage; it is not wired into those bounded batch steps. It samples +only a single identified direct Bun child after silence and cleans up only its own +diagnostic children. Process inventories emit executable basenames; command stdout/stderr +and stack reports redact literal home/workspace prefixes before capped emission. The +catalog picker fixture retains CI-only phase boundaries. + Native steering follows [the shared WebSocket contract](../transports/streaming-health.md#experimental-native-mid-turn-steering); this surface's defaults remain unchanged. Catalog HTTP acquisition follows the [proxy-routing contract](../catalog.md#remote-catalog-http-proxy-routing). @@ -29,6 +37,26 @@ The account reference documents the [Orca source-owned import](../codex-home.md# Its local-only command is declared in `src/cli/capabilities.ts`, and the generated skill surface lists its required source/registry paths and preview/apply flags. +Local validation follows [the contributor test policy](../../AGENTS.md#commands): run the +suite by default, with a documented resource exception requiring focused regression tests. +`scripts/setup-hooks.ts` installs the post-merge hook and retires only an exact match for +the old managed pre-push shim; custom hooks are preserved. Required current-head CI and +security review remain merge requirements. + +The gate preserves legacy checklist bodies and asks the author to update the first item, +clear all four boxes and save, then wait for the bot to record that checkpoint before +validating the displayed head and ticking all four boxes again. Wording-only edits do not +re-attest. The existing bot-comment state stores a versioned pending phase, real head/base, +generation, the phase publication's server timestamp and, only after rechecking, a body digest. Invalid stored state restarts the +clearing phase; a different live head/base invalidates the checkpoint. Only an author body +edit whose live snapshot agrees and whose server timestamp is later than the stored phase checkpoint +can advance it. A new phase is first persisted without a timestamp and then finalized with +the first write's server time; unfinished finalization cannot advance readiness. Hygiene +updates to the outer comment do not move this fence. Equal-second saves require a later +body edit. While re-attestation is pending, the quality check fails explicitly and defers +ordinary quality evaluation; it does not report a green gate. Before ready, the gate re-reads the PR and persisted attestation. These reads do +not make GitHub's later ready mutation atomic with concurrent edits or pushes. + ## Public docs The provider configuration reference and provider guide own the public Google tool-schema policy: diff --git a/tests/ci-workflows/ci-workflows.test.ts b/tests/ci-workflows/ci-workflows.test.ts index dccd181479..e61940f66d 100644 --- a/tests/ci-workflows/ci-workflows.test.ts +++ b/tests/ci-workflows/ci-workflows.test.ts @@ -1589,7 +1589,9 @@ describe("GitHub Actions hardening", () => { // Hygiene reassessment reads the changed-file list; not a write. name !== "github.rest.pulls.listFiles" && // Carry attribution reads the branch's commit messages; not a write. - name !== "github.rest.pulls.listCommits", + name !== "github.rest.pulls.listCommits" && + // Pre-ready re-attestation readback re-reads the saved gate comment; not a write. + name !== "github.rest.issues.getComment", ); expect([...new Set(restWrites)].sort()).toEqual([ "github.rest.issues.addLabels", @@ -1671,7 +1673,7 @@ describe("GitHub Actions hardening", () => { const CHECKLIST_START = ""; const CHECKLIST_END = ""; const CHECKLIST_ITEMS = [ - "All CI tests are green on my local testing.", + "Required local validation passed; commands, results, and any full-suite exception are documented.", "I pushed my PR to the latest dev commit.", "I resolved all correct Codex and CodeRabbit findings.", "My PR is ready for review.", @@ -1797,7 +1799,7 @@ describe("GitHub Actions hardening", () => { const [injected] = callsTo(result, "pulls.update") as [{ body: string }]; expect(injected.body).toContain(CHECKLIST_START); expect(injected.body).toContain(CHECKLIST_END); - expect(injected.body).toContain("- [ ] All CI tests are green on my local testing."); + expect(injected.body).toContain("- [ ] Required local validation passed; commands, results, and any full-suite exception are documented."); expect(injected.body).toContain("- [ ] My PR is ready for review."); const [draft] = callsTo(result, "graphql") as [{ query: string }]; @@ -1856,6 +1858,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -1957,6 +1960,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2005,7 +2009,7 @@ describe("GitHub Actions hardening", () => { ])); const [resetBody] = callsTo(result, "pulls.update") as [{ body: string }]; expect(resetBody.body).toContain(CHECKLIST_START); - expect(resetBody.body).toContain("- [ ] All CI tests are green on my local testing."); + expect(resetBody.body).toContain("- [ ] Required local validation passed; commands, results, and any full-suite exception are documented."); expect(resetBody.body).toContain("- [ ] My PR is ready for review."); expect(resetBody.body).not.toContain("- [x]"); @@ -2049,6 +2053,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "issues.createComment", "issues.deleteComment", @@ -2200,6 +2205,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2272,7 +2278,7 @@ describe("GitHub Actions hardening", () => { ])); const [bodyUpdate] = callsTo(result, "pulls.update") as [{ body: string }]; // Only the latest-dev box is unticked; local CI stays checked. - expect(bodyUpdate.body).toContain("- [x] All CI tests are green on my local testing."); + expect(bodyUpdate.body).toContain("- [x] Required local validation passed; commands, results, and any full-suite exception are documented."); expect(bodyUpdate.body).toContain("- [ ] I pushed my PR to the latest dev commit."); expect(bodyUpdate.body).toContain("- [x] My PR is ready for review."); const drafts = callsTo(result, "graphql") as [{ query: string }]; @@ -2303,6 +2309,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2366,7 +2373,7 @@ describe("GitHub Actions hardening", () => { ])); const [bodyUpdate] = callsTo(result, "pulls.update") as [{ body: string }]; // Only the findings box is unticked; CI and latest-dev stay checked. - expect(bodyUpdate.body).toContain("- [x] All CI tests are green on my local testing."); + expect(bodyUpdate.body).toContain("- [x] Required local validation passed; commands, results, and any full-suite exception are documented."); expect(bodyUpdate.body).toContain("- [x] I pushed my PR to the latest dev commit."); expect(bodyUpdate.body).toContain("- [ ] I resolved all correct Codex and CodeRabbit findings."); expect(bodyUpdate.body).toContain("- [x] My PR is ready for review."); @@ -2422,6 +2429,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2463,6 +2471,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2526,6 +2535,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2554,6 +2564,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -2909,6 +2920,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -3703,6 +3715,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -3760,6 +3773,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "pulls.update", "graphql", @@ -4060,6 +4074,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "pulls.update", "graphql", @@ -4210,6 +4225,7 @@ describe("GitHub Actions hardening", () => { "graphql", "pulls.listReviews", "pulls.listReviews", + "pulls.get", "issues.addLabels", "pulls.update", "graphql", @@ -4331,6 +4347,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -4423,6 +4440,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(restored)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "pulls.update", "graphql", @@ -4481,6 +4499,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(loose)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "pulls.update", "graphql", @@ -4505,6 +4524,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(falsy)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "graphql", "issues.createComment", @@ -4677,6 +4697,7 @@ describe("GitHub Actions hardening", () => { expect(methodsOf(result)).toEqual(readsAllowedBase([ "graphql", "pulls.listReviews", + "pulls.get", "issues.addLabels", "pulls.update", "graphql", @@ -5423,91 +5444,6 @@ describe("lint-gui-if-changed", () => { }); }); -describe("gui exhaustive-deps suppression stays scoped and effective", () => { - // `bun run doctor:gui` exited 1 on dev for one deliberate exception at - // gui/src/pages/Models.tsx, and doctor:gui runs inside `prepush`, so every - // gui-touching push needed --no-verify. Two config edits fixed it, and each has a - // failure mode that is silent rather than loud, which is what these assertions cover. - - test("the oxlint override carries its own react plugin, or it resolves to nothing", async () => { - const oxlintrc = JSON.parse(await readText("gui/.oxlintrc.json")) as { - overrides?: Array<{ files?: string[]; rules?: Record; plugins?: string[] }>; - }; - const overrides = oxlintrc.overrides ?? []; - const scoped = overrides.filter(entry => (entry.files ?? []).includes("src/pages/Models.tsx")); - - expect(scoped).toHaveLength(1); - const override = scoped[0]!; - - // Rule id must match the style the rest of this config uses ("react/..."). The - // eslint-style "react-hooks/..." id silently matches nothing here. - expect(override.rules?.["react/exhaustive-deps"]).toBe("off"); - expect(override.rules).not.toHaveProperty("react-hooks/exhaustive-deps"); - - // Without a per-override plugins key the override is inert: the rule stays on and - // the warning comes back. This is the assertion that catches a well-meaning cleanup - // that deletes a key looking redundant next to the top-level plugin list. - expect(override.plugins).toContain("react"); - - // Narrow by construction: the override turns off exactly one rule. rules-of-hooks and - // react-compiler must keep firing in that file, and a probe confirmed they do. - expect(Object.keys(override.rules ?? {})).toEqual(["react/exhaustive-deps"]); - }); - - test("react-doctor scopes the ignore to one file instead of going blind everywhere", async () => { - const config = JSON.parse(await readText("gui/doctor.config.json")) as { - blocking?: string; - ignore?: { overrides?: Array<{ files?: string[]; rules?: string[] }> }; - rules?: Record; - }; - - // A global rules entry was tried first and rejected: it silenced the rule repo-wide, - // proven by injecting a missing-dep violation into Startup.tsx and watching doctor - // report "No issues". ignore.overrides keeps that violation failing. - expect(config.rules).not.toHaveProperty("react-doctor/exhaustive-deps"); - expect(config.rules).not.toHaveProperty("react-hooks/exhaustive-deps"); - - const overrides = config.ignore?.overrides ?? []; - const scoped = overrides.filter(entry => (entry.files ?? []).includes("src/pages/Models.tsx")); - expect(scoped).toHaveLength(1); - expect(scoped[0]!.rules).toContain("react-hooks/exhaustive-deps"); - - // Every ignore override must name at least one file. An empty or missing files list - // would apply the ignore to the whole scan, which is the failure this pair guards. - for (const entry of overrides) { - expect((entry.files ?? []).length).toBeGreaterThan(0); - expect((entry.rules ?? []).length).toBeGreaterThan(0); - } - - // blocking must stay at warning; flipping it to error would hide the next finding - // instead of this one. scripts/doctor-gui-if-changed.ts documents that contract. - expect(config.blocking).toBe("warning"); - }); - - test("the effect keeps the in-file record of why the dep array stays short", async () => { - const models = await readText("gui/src/pages/Models.tsx"); - const effectEnd = models.indexOf("}, [catalogActive, loadShadowCall, loadV2]);"); - expect(effectEnd).toBeGreaterThan(-1); - - // The reasoning has to sit on the effect, not in a commit message. Read the comment - // block immediately above the dep array rather than the whole file, or this passes on - // any incidental mention elsewhere. - const preceding = models.slice(0, effectEnd).split(/\r?\n/).slice(-8).join("\n"); - expect(preceding).toContain("PreserveManualMemo"); - expect(preceding).toContain("five react-compiler"); - - // Both suppressions are config-side, so the note must point at the two files a reader - // would otherwise have to find by grep. - expect(preceding).toContain("gui/.oxlintrc.json"); - expect(preceding).toContain("gui/doctor.config.json"); - - // An in-file react-doctor disable was tried and removed: doctor passes without it, and - // react/react-compiler penalises a component merely for carrying suppressions. If one - // reappears, the config route has been misunderstood. - expect(models).not.toContain("react-doctor-disable-next-line"); - }); -}); - interface PublicationStep { name: string; id?: string; if?: string; run?: string; env?: Record } async function publicationSteps(): Promise { diff --git a/tests/ci-workflows/exhaustive-deps-suppression.test.ts b/tests/ci-workflows/exhaustive-deps-suppression.test.ts new file mode 100644 index 0000000000..86b3a58b65 --- /dev/null +++ b/tests/ci-workflows/exhaustive-deps-suppression.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, test } from "bun:test"; +import { pathToFileURL } from "node:url"; +import { repoRoot } from "../helpers/repo-root"; + +// Moved byte for byte from ci-workflows.test.ts to keep that file under its size cap. +const root = pathToFileURL(repoRoot() + "/"); + +async function readText(path: string): Promise { + return await Bun.file(new URL(path, root)).text(); +} + +describe("gui exhaustive-deps suppression stays scoped and effective", () => { + // `bun run doctor:gui` exited 1 on dev for one deliberate exception at + // gui/src/pages/Models.tsx, blocking explicit comprehensive validation. + // Two config edits fixed it, and each has a + // failure mode that is silent rather than loud, which is what these assertions cover. + + test("the oxlint override carries its own react plugin, or it resolves to nothing", async () => { + const oxlintrc = JSON.parse(await readText("gui/.oxlintrc.json")) as { + overrides?: Array<{ files?: string[]; rules?: Record; plugins?: string[] }>; + }; + const overrides = oxlintrc.overrides ?? []; + const scoped = overrides.filter(entry => (entry.files ?? []).includes("src/pages/Models.tsx")); + + expect(scoped).toHaveLength(1); + const override = scoped[0]!; + + // Rule id must match the style the rest of this config uses ("react/..."). The + // eslint-style "react-hooks/..." id silently matches nothing here. + expect(override.rules?.["react/exhaustive-deps"]).toBe("off"); + expect(override.rules).not.toHaveProperty("react-hooks/exhaustive-deps"); + + // Without a per-override plugins key the override is inert: the rule stays on and + // the warning comes back. This is the assertion that catches a well-meaning cleanup + // that deletes a key looking redundant next to the top-level plugin list. + expect(override.plugins).toContain("react"); + + // Narrow by construction: the override turns off exactly one rule. rules-of-hooks and + // react-compiler must keep firing in that file, and a probe confirmed they do. + expect(Object.keys(override.rules ?? {})).toEqual(["react/exhaustive-deps"]); + }); + + test("react-doctor scopes the ignore to one file instead of going blind everywhere", async () => { + const config = JSON.parse(await readText("gui/doctor.config.json")) as { + blocking?: string; + ignore?: { overrides?: Array<{ files?: string[]; rules?: string[] }> }; + rules?: Record; + }; + + // A global rules entry was tried first and rejected: it silenced the rule repo-wide, + // proven by injecting a missing-dep violation into Startup.tsx and watching doctor + // report "No issues". ignore.overrides keeps that violation failing. + expect(config.rules).not.toHaveProperty("react-doctor/exhaustive-deps"); + expect(config.rules).not.toHaveProperty("react-hooks/exhaustive-deps"); + + const overrides = config.ignore?.overrides ?? []; + const scoped = overrides.filter(entry => (entry.files ?? []).includes("src/pages/Models.tsx")); + expect(scoped).toHaveLength(1); + expect(scoped[0]!.rules).toContain("react-hooks/exhaustive-deps"); + + // Every ignore override must name at least one file. An empty or missing files list + // would apply the ignore to the whole scan, which is the failure this pair guards. + for (const entry of overrides) { + expect((entry.files ?? []).length).toBeGreaterThan(0); + expect((entry.rules ?? []).length).toBeGreaterThan(0); + } + + // blocking must stay at warning; flipping it to error would hide the next finding + // instead of this one. scripts/doctor-gui-if-changed.ts documents that contract. + expect(config.blocking).toBe("warning"); + }); + + test("the effect keeps the in-file record of why the dep array stays short", async () => { + const models = await readText("gui/src/pages/Models.tsx"); + const effectEnd = models.indexOf("}, [catalogActive, loadShadowCall, loadV2]);"); + expect(effectEnd).toBeGreaterThan(-1); + + // The reasoning has to sit on the effect, not in a commit message. Read the comment + // block immediately above the dep array rather than the whole file, or this passes on + // any incidental mention elsewhere. + const preceding = models.slice(0, effectEnd).split(/\r?\n/).slice(-8).join("\n"); + expect(preceding).toContain("PreserveManualMemo"); + expect(preceding).toContain("five react-compiler"); + + // Both suppressions are config-side, so the note must point at the two files a reader + // would otherwise have to find by grep. + expect(preceding).toContain("gui/.oxlintrc.json"); + expect(preceding).toContain("gui/doctor.config.json"); + + // An in-file react-doctor disable was tried and removed: doctor passes without it, and + // react/react-compiler penalises a component merely for carrying suppressions. If one + // reappears, the config route has been misunderstood. + expect(models).not.toContain("react-doctor-disable-next-line"); + }); +}); diff --git a/tests/ci-workflows/macos-serial-lanes.test.ts b/tests/ci-workflows/macos-serial-lanes.test.ts index 7c331c682f..8047ff7350 100644 --- a/tests/ci-workflows/macos-serial-lanes.test.ts +++ b/tests/ci-workflows/macos-serial-lanes.test.ts @@ -107,6 +107,8 @@ function createFixture(directory: string, options: FixtureOptions): void { // block, so a copy here would let the block and the classifier drift apart unnoticed, which is // the exact failure mode that collapsing four inline signature lists into one file removed. mkdirSync(join(directory, "scripts", "ci"), { recursive: true }); + copyFileSync(repoPath("scripts", "ci", "sample-macos-stall.sh"), + join(directory, "scripts", "ci", "sample-macos-stall.sh")); copyFileSync(repoPath("scripts", "ci", "bun-crash-signatures.sh"), join(directory, "scripts", "ci", "bun-crash-signatures.sh")); copyFileSync(repoPath("scripts", "ci", "run-bun-test-batches.sh"), @@ -137,9 +139,9 @@ function spawnErrorCode(error: unknown): string { return typeof code === "string" && /^[A-Z0-9_]{1,64}$/.test(code) ? code : "SPAWN_ERROR"; } -function runShell(directory: string, shard: number): Promise<{ status: number | null; output: string }> { +function runShell(directory: string, shard: number, scriptOverride?: string): Promise<{ status: number | null; output: string }> { // Use the runner's native /bin/bash (Bash 3 on macOS), never a shell mock. - const command = macosTestBlock(shard); + const command = scriptOverride ?? macosTestBlock(shard); return new Promise((resolve, reject) => { let child: ChildProcessByStdio; try { @@ -275,6 +277,20 @@ function expectBatchArguments(call: Invocation): void { } describe.skipIf(process.platform === "win32")("macOS bounded shard shell ownership", () => { + test("stall observer samples only an identified silent suite without signaling it", async () => { + const directory = mkdtempSync(join(tmpdir(), "ocx macos' observer-")); + try { + createFixture(directory, {}); + const fixture = repoPath("tests", "fixtures", "macos-stall-observer.sh"); + const result = await runShell(directory, 1, + `bash ${shellQuote(fixture)} "$PWD/probe" "$PWD/scripts/ci/sample-macos-stall.sh"`); + expect(result.status, result.output).toBe(0); + for (const scenario of ["silent", "absent", "ambiguous", "progress", "stop"]) { + expect(result.output).toContain(`PASS ${scenario}`); + } + } finally { removeTreeWithRetry(directory); } + }, SPAWN_BUDGET_MS); + test("both actual workflow shards own every file exactly once", async () => { const runs = [await runShard(1), await runShard(2)]; for (const [index, run] of runs.entries()) { diff --git a/tests/ci-workflows/pr-readiness-reattest.test.ts b/tests/ci-workflows/pr-readiness-reattest.test.ts new file mode 100644 index 0000000000..e7afad46d6 --- /dev/null +++ b/tests/ci-workflows/pr-readiness-reattest.test.ts @@ -0,0 +1,290 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { repoPath } from "../helpers/repo-root"; +import { callsTo, runEnforcePrTarget, type Comment } from "../helpers/enforce-pr-target-harness"; + +const HEAD = "3f1c0de0a6a4d0a3f9a1b2c3d4e5f60718293a4b"; +const NEXT = "b".repeat(40); +const OLD = "All CI tests are green on my local testing."; +const CURRENT = "Required local validation passed; commands, results, and any full-suite exception are documented."; +const START = ""; +const END = ""; +const T0 = "2026-09-22T00:00:00Z"; +const T1 = "2026-09-22T00:00:02Z"; +const T2 = "2026-09-22T00:00:04Z"; +const T3 = "2026-09-22T00:00:06Z"; +const T4 = "2026-09-22T00:00:08Z"; +const T5 = "2026-09-22T00:00:10Z"; +const workflow = Bun.YAML.parse(readFileSync(repoPath(".github/workflows/enforce-pr-target.yml"), "utf8")) as { + jobs: Record }>; +}; +const script = workflow.jobs["enforce-target"]!.steps.find(step => step.name === "Enforce PR target, ancestry, and description")!.with!.script!; + +function body(label = CURRENT, checks = 4): string { + return ["## Summary", "", "Update managed validation wording while preserving the author's text and explicitly requiring a fresh attestation.", "", + "## Verification", "", "Hosted workflow regression coverage will exercise the managed checklist state transitions.", "", START, + ...[label, "I pushed my PR to the latest dev commit.", "I resolved all correct Codex and CodeRabbit findings.", "My PR is ready for review."] + .map((text, index) => `- [${index < checks ? "x" : " "}] ${text}`), END].join("\n"); +} +type Result = Awaited>; +function saved(result: Result, updated_at: string): Comment { + const writes = result.calls.filter(call => call.method === "issues.updateComment" || call.method === "issues.createComment"); + const last = writes.at(-1)?.args as { body?: string; comment_id?: number } | undefined; + expect(last?.body).toContain(""); + return { id: last?.comment_id ?? 99, user: { login: "github-actions[bot]" }, body: last!.body!, updated_at }; +} +function pending(comment: Comment) { + const json = comment.body!.match(//)![1]!; + return JSON.parse(json).pendingReattestation; +} +function promotions(result: Result) { + return (callsTo(result, "graphql") as Array<{ query: string }>).filter(call => call.query.includes("markPullRequestReadyForReview")); +} +function bodyWrites(result: Result) { + return (callsTo(result, "pulls.update") as Array<{ body?: string }>).filter(call => call.body !== undefined); +} +async function initialize() { + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD }, updated_at: T0 }, + eventAction: "synchronize", commentUpdatedAt: T1, commentUpdatedAts: [T1, T2], + }); + expect(promotions(result)).toEqual([]); + expect(bodyWrites(result)).toEqual([]); + const comment = saved(result, T2); + expect(pending(comment)).toMatchObject({ headSha: HEAD, phase: "await-clear" }); + return comment; +} +async function clear(comment: Comment) { + const result = await runEnforcePrTarget(script, { + pr: { body: body(CURRENT, 0), draft: true, head: { sha: HEAD }, updated_at: T2 }, + eventAction: "edited", previousBody: body(OLD), comments: [comment], commentUpdatedAt: T3, commentUpdatedAts: [T3, T4], + }); + expect(bodyWrites(result)).toEqual([]); + expect(promotions(result)).toEqual([]); + const next = saved(result, T4); + expect(pending(next)).toMatchObject({ headSha: HEAD, phase: "await-check" }); + return next; +} + +describe("author-applied policy migration with durable re-attestation", () => { + test("legacy complete -> pending -> wording-only stays pending -> clear -> later retick", async () => { + const first = await initialize(); + const wording = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD }, updated_at: T2 }, + eventAction: "edited", previousBody: body(OLD), comments: [first], commentUpdatedAt: T3, + }); + expect(promotions(wording)).toEqual([]); + expect(bodyWrites(wording)).toEqual([]); + expect(pending(saved(wording, T3)).phase).toBe("await-clear"); + const cleared = await clear(first); + const retick = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD }, updated_at: T4 }, + eventAction: "edited", previousBody: body(CURRENT, 0), comments: [cleared], commentUpdatedAt: T5, + }); + expect(bodyWrites(retick)).toEqual([]); + expect(promotions(retick)).toHaveLength(1); + const calls = retick.calls.map(call => call.method); + expect(calls.indexOf("issues.updateComment")).toBeLessThan(calls.indexOf("issues.getComment")); + const checkpoint = (callsTo(retick, "issues.updateComment") as Array<{ body: string }>)[0]!; + expect(pending({ id: 99, body: checkpoint.body })).toMatchObject({ phase: "attested", headSha: HEAD }); + expect(pending(saved(retick, T5))).toBeNull(); + }); + + test("identical pending replay does not duplicate notices or mutate author content", async () => { + const first = await initialize(); + const replay = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD }, updated_at: T0 }, + eventAction: "synchronize", comments: [first], commentUpdatedAt: T3, + }); + for (const name of ["issues.createComment", "issues.updateComment", "issues.addLabels", "issues.removeLabel", "pulls.update"]) { + expect(callsTo(replay, name)).toEqual([]); + } + expect(promotions(replay)).toEqual([]); + }); + + for (const options of [ + { previousBody: undefined, updated_at: T2 }, + { previousBody: body(OLD), updated_at: T1 }, + ]) { + test(`title-only/equal-time edit cannot arm recheck: ${JSON.stringify(options)}`, async () => { + const first = await initialize(); + const result = await runEnforcePrTarget(script, { + pr: { body: body(CURRENT, 0), draft: true, head: { sha: HEAD }, updated_at: options.updated_at }, + eventAction: "edited", previousBody: options.previousBody, comments: [first], commentUpdatedAt: T3, + }); + expect(pending(saved(result, T3)).phase).toBe("await-clear"); + expect(promotions(result)).toEqual([]); + }); + } + + test("another head invalidates the clear checkpoint instead of inheriting its ticks", async () => { + const cleared = await clear(await initialize()); + const result = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: NEXT }, updated_at: T4 }, + eventAction: "edited", previousBody: body(CURRENT, 0), comments: [cleared], commentUpdatedAt: T5, + }); + expect(pending(saved(result, T5))).toMatchObject({ headSha: NEXT, phase: "await-clear", generation: 2 }); + expect(promotions(result)).toEqual([]); + expect(bodyWrites(result)).toEqual([]); + }); + + test("a failed current-head claim invalidates proof without unchecking the author body", async () => { + const cleared = await clear(await initialize()); + const result = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD }, updated_at: T4 }, + eventAction: "edited", previousBody: body(CURRENT, 0), comments: [cleared], commentUpdatedAt: T5, + compareByBasehead: { [`dev...${HEAD}`]: { ahead_by: 0, behind_by: 11 } }, + }); + expect(pending(saved(result, T5)).phase).toBe("await-clear"); + expect(bodyWrites(result)).toEqual([]); + expect(promotions(result)).toEqual([]); + }); + + test("fresh legacy classification prevents an otherwise scheduled reset writer", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD } }, eventAction: "synchronize", + pullSnapshots: [{}, {}, {}, { body: body(OLD) }], commentUpdatedAt: T1, + }); + expect(bodyWrites(result)).toEqual([]); + expect(pending(saved(result, T1)).phase).toBe("await-clear"); + expect(promotions(result)).toEqual([]); + }); + + test("a moved head on the final read yields no ready or completion side effects", async () => { + const cleared = await clear(await initialize()); + const result = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD }, updated_at: T4 }, + eventAction: "edited", previousBody: body(CURRENT, 0), comments: [cleared], commentUpdatedAt: T5, + pullSnapshots: [{}, {}, {}, {}, { head: { sha: NEXT } }], + }); + expect(promotions(result)).toEqual([]); + expect(callsTo(result, "issues.addLabels")).toEqual([]); + expect(bodyWrites(result)).toEqual([]); + expect(result.warnings.some(w => w.includes("no ready action was taken"))).toBe(true); + expect(pending(saved(result, T5)).phase).toBe("attested"); + }); +}); + + +describe("reattest mutation failures and authoritative readback", () => { + test("failed draft conversion releases persisted ownership", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: false, head: { sha: HEAD }, updated_at: T0 }, + failGraphqlOn: ["convertPullRequestToDraft"], commentUpdatedAt: T1, + }); + expect(saved(result, T1).body).toContain('"autoDraftedByBot":false'); + expect(result.warnings.some(w => w.includes("Could not retain draft state"))).toBe(true); + }); + + test("failed ready-label removal fails pending evaluation", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD }, updated_at: T0 }, + labels: ["review-ready"], failOn: ["issues.removeLabel"], commentUpdatedAt: T1, + }); + expect(result.warnings.some(w => w.startsWith("setFailed:") && w.includes("stale review-ready label"))).toBe(true); + expect(promotions(result)).toEqual([]); + }); + + test("pending wrong-base evaluation cannot report a successful quality gate", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, base: { ref: "main" }, head: { sha: HEAD } }, commentUpdatedAt: T1, + }); + expect(result.warnings.some(w => w.startsWith("setFailed:") && w.includes("re-attestation is pending"))).toBe(true); + expect(bodyWrites(result)).toEqual([]); + }); + + test("permission lookup failure retains contributor preservation", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD } }, failPermissionLookup: true, commentUpdatedAt: T1, + }); + expect(pending(saved(result, T1)).phase).toBe("await-clear"); + expect(bodyWrites(result)).toEqual([]); + }); + + test("a hygiene update does not move the persisted phase fence", async () => { + const first = await initialize(); + first.updated_at = T3; + const result = await runEnforcePrTarget(script, { + pr: { body: body(CURRENT, 0), draft: true, head: { sha: HEAD }, updated_at: T2 }, + eventAction: "edited", previousBody: body(OLD), comments: [first], commentUpdatedAt: T4, + }); + expect(pending(saved(result, T4)).phase).toBe("await-check"); + }); + + test("unfinished checkpoint is finalized but cannot consume the same author edit", async () => { + const first = await initialize(); + first.body = first.body!.replace(/"checkpointAt":"[^"]+"/, '"checkpointAt":null'); + first.updated_at = T1; + const result = await runEnforcePrTarget(script, { + pr: { body: body(CURRENT, 0), draft: true, head: { sha: HEAD }, updated_at: T2 }, + eventAction: "edited", previousBody: body(OLD), comments: [first], commentUpdatedAt: T3, + }); + const writes = callsTo(result, "issues.updateComment") as Array<{ body: string }>; + expect(writes).toHaveLength(2); + expect(writes[0]!.body).toContain("0/4"); + expect(pending({ id: 99, body: writes[0]!.body }).checkpointAt).toBeNull(); + expect(pending(saved(result, T3))).toMatchObject({ phase: "await-clear", checkpointAt: T3 }); + expect(promotions(result)).toEqual([]); + }); + + test("unchanged provisional replay uses the already persisted server fence", async () => { + const first = await initialize(); + first.body = first.body!.replace(/"checkpointAt":"[^"]+"/, '"checkpointAt":null'); + first.updated_at = T1; + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD }, updated_at: T0 }, + eventAction: "synchronize", comments: [first], commentUpdatedAt: T3, + }); + expect(callsTo(result, "issues.updateComment")).toHaveLength(1); + expect(pending(saved(result, T3))).toMatchObject({ phase: "await-clear", checkpointAt: T1 }); + expect(promotions(result)).toEqual([]); + }); + + test("phase fence uses the first publication time rather than finalization time", async () => { + const first = await initialize(); + expect(pending(first)).toMatchObject({ checkpointAt: T1, phase: "await-clear" }); + expect(first.updated_at).toBe(T2); + }); + + test("missing server time leaves provisional state unable to progress", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD }, updated_at: T0 }, + }); + expect(pending(saved(result, T1)).checkpointAt).toBeNull(); + expect(result.warnings.some(w => w.includes("no authoritative server timestamp"))).toBe(true); + expect(promotions(result)).toEqual([]); + }); + + test("failed finalization rejects the run before any ready mutation", async () => { + await expect(runEnforcePrTarget(script, { + pr: { body: body(OLD), draft: true, head: { sha: HEAD }, updated_at: T0 }, + commentUpdatedAts: [T1, T2], failCommentWrite: 2, + })).rejects.toThrow(); + }); + + for (const malformed of [false, true]) { + test(`late saved-state mismatch cannot promote (${malformed})`, async () => { + const cleared = await clear(await initialize()); + const finalComment = malformed ? { ...cleared, body: '' } : cleared; + const result = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD }, updated_at: T4 }, + eventAction: "edited", previousBody: body(CURRENT, 0), comments: [cleared], commentUpdatedAt: T5, finalComment, + }); + expect(promotions(result)).toEqual([]); + expect(callsTo(result, "issues.addLabels")).toEqual([]); + expect(result.warnings.some(w => w.includes("no ready action"))).toBe(true); + }); + } +}); + + +test("a current checklist that becomes legacy at final read never advances readiness", async () => { + const result = await runEnforcePrTarget(script, { + pr: { body: body(), draft: true, head: { sha: HEAD }, updated_at: T4 }, + pullSnapshots: [{}, {}, {}, { body: body(OLD) }], commentUpdatedAt: T5, + }); + expect(promotions(result)).toEqual([]); + expect(callsTo(result, "issues.addLabels")).toEqual([]); + expect(bodyWrites(result)).toEqual([]); + expect(pending(saved(result, T5)).phase).toBe("await-clear"); +}); diff --git a/tests/ci-workflows/setup-hooks.test.ts b/tests/ci-workflows/setup-hooks.test.ts new file mode 100644 index 0000000000..50f2af6029 --- /dev/null +++ b/tests/ci-workflows/setup-hooks.test.ts @@ -0,0 +1,152 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { execFileSync } from "node:child_process"; +import { copyFileSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { isAbsolute, join, relative } from "node:path"; +import { repoPath } from "../helpers/repo-root"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +// Independent fixture for the retired, formerly shipped hook, not the remover's hash. +const legacyHook = [ + "#!/usr/bin/env sh", + '# Pre-push hook shim. The actual command list lives in package.json ("prepush").', + "# Installed by: bun run setup:hooks", + "set -e", + "exec bun run prepush", + "", +].join("\n"); +const roots: string[] = []; +afterEach(() => { for (const root of roots.splice(0)) removeTreeWithRetry(root); }); + +function gitEnv(root: string): NodeJS.ProcessEnv { + // The test preload retains the real global Git config. Never let it redirect + // fixture commands or hook writes into the developer's own checkout. + const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_"))); + return { ...env, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: join(root, ".fixture-gitconfig") }; +} + +function git(root: string, ...args: string[]): string { + return execFileSync("git", args, { cwd: root, env: gitEnv(root), encoding: "utf8", stdio: "pipe" }).trim(); +} + +function fixture(): string { + const root = realpathSync(mkdtempSync(join(tmpdir(), "ocx-hook-setup-"))); + roots.push(root); + git(root, "init", "--quiet"); + mkdirSync(join(root, "scripts")); + for (const name of ["setup-hooks.ts", "post-merge.sh"]) { + copyFileSync(repoPath("scripts", name), join(root, "scripts", name)); + } + return root; +} + +function setup(root: string): string { + return execFileSync(process.execPath, [join(root, "scripts/setup-hooks.ts")], { + cwd: root, env: gitEnv(root), encoding: "utf8", timeout: 10_000, stdio: "pipe", + }); +} + +function hooks(root: string): string { + const path = git(root, "rev-parse", "--path-format=absolute", "--git-path", "hooks"); + // A linked worktree resolves to its parent fixture's shared hooks directory. + expect(roots.some(fixtureRoot => { + const rel = relative(fixtureRoot, path); + return rel !== ".." && !rel.startsWith("../") && !rel.startsWith("..\\") && !isAbsolute(rel); + })).toBe(true); + return path; +} + +describe("local hook setup", () => { + test("fixture harness isolates inherited global hooks and Git directory overrides", () => { + const external = fixture(); + const externalHook = join(hooks(external), "pre-push"); + writeFileSync(externalHook, "user-owned hook\n"); + const globalConfig = join(external, "global-config"); + git(external, "config", "--file", globalConfig, "core.hooksPath", hooks(external)); + const savedGlobal = process.env.GIT_CONFIG_GLOBAL; + const savedDir = process.env.GIT_DIR; + try { + process.env.GIT_CONFIG_GLOBAL = globalConfig; + process.env.GIT_DIR = join(external, ".git"); + const root = fixture(); + setup(root); + expect(existsSync(join(hooks(root), "pre-push"))).toBe(false); + expect(readFileSync(externalHook, "utf8")).toBe("user-owned hook\n"); + expect(existsSync(join(hooks(external), "post-merge"))).toBe(false); + } finally { + if (savedGlobal === undefined) delete process.env.GIT_CONFIG_GLOBAL; + else process.env.GIT_CONFIG_GLOBAL = savedGlobal; + if (savedDir === undefined) delete process.env.GIT_DIR; + else process.env.GIT_DIR = savedDir; + } + }); + + test("fresh setup installs only post-merge and is idempotent", () => { + const root = fixture(); + setup(root); + const hookDir = hooks(root); + expect(existsSync(join(hookDir, "pre-push"))).toBe(false); + expect(readFileSync(join(hookDir, "post-merge"), "utf8")) + .toBe(readFileSync(repoPath("scripts/post-merge.sh"), "utf8")); + setup(root); + expect(readdirSync(hookDir).filter(name => name.startsWith("post-merge.backup-"))).toEqual([]); + }); + + for (const ending of ["\n", "\r\n"]) { + test(`retires the shipped hook with ${JSON.stringify(ending)} line endings`, () => { + const root = fixture(); + writeFileSync(join(hooks(root), "pre-push"), legacyHook.replace(/\n/g, ending)); + setup(root); + expect(existsSync(join(hooks(root), "pre-push"))).toBe(false); + expect(existsSync(join(hooks(root), "post-merge"))).toBe(true); + }); + } + + test("preserves custom hooks even when they contain the old shim", () => { + const root = fixture(); + const custom = legacyHook + "echo custom validation\n"; + writeFileSync(join(hooks(root), "pre-push"), custom); + setup(root); + expect(readFileSync(join(hooks(root), "pre-push"), "utf8")).toBe(custom); + }); + + test("uses a configured hooks directory without touching the default one", () => { + const root = fixture(); + const original = hooks(root); + writeFileSync(join(original, "pre-push"), legacyHook); + const customDir = join(root, "custom hooks"); + mkdirSync(customDir); + writeFileSync(join(customDir, "pre-push"), legacyHook); + git(root, "config", "core.hooksPath", customDir); + setup(root); + expect(existsSync(join(customDir, "pre-push"))).toBe(false); + expect(existsSync(join(customDir, "post-merge"))).toBe(true); + expect(readFileSync(join(original, "pre-push"), "utf8")).toBe(legacyHook); + }); + + test("linked worktrees migrate the Git-resolved shared hooks directory", () => { + const root = fixture(); + git(root, "add", "scripts"); + git(root, "-c", "user.name=Fixture", "-c", `user.email=${["fixture", "example.invalid"].join("@")}`, + "-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "fixture"); + const linked = join(root, "linked"); + git(root, "worktree", "add", "--detach", linked); + const shared = hooks(root); + writeFileSync(join(shared, "pre-push"), legacyHook); + setup(linked); + expect(hooks(linked)).toBe(shared); + expect(existsSync(join(shared, "pre-push"))).toBe(false); + expect(existsSync(join(shared, "post-merge"))).toBe(true); + }); + + test.skipIf(process.platform === "win32")("preserves symlinked pre-push hooks", () => { + const root = fixture(); + const target = join(root, "user-hook"); + writeFileSync(target, legacyHook); + const hook = join(hooks(root), "pre-push"); + symlinkSync(target, hook); + setup(root); + expect(lstatSync(hook).isSymbolicLink()).toBe(true); + expect(readFileSync(target, "utf8")).toBe(legacyHook); + }); +}); diff --git a/tests/ci-workflows/zz-pr-coderabbit-readiness-revalidation.test.ts b/tests/ci-workflows/zz-pr-coderabbit-readiness-revalidation.test.ts index cc2e2cfb27..101bd9f687 100644 --- a/tests/ci-workflows/zz-pr-coderabbit-readiness-revalidation.test.ts +++ b/tests/ci-workflows/zz-pr-coderabbit-readiness-revalidation.test.ts @@ -31,7 +31,7 @@ const GATE_MARKER = ""; const CHECKLIST_START = ""; const CHECKLIST_END = ""; const CHECKLIST_ITEMS = [ - "All CI tests are green on my local testing.", + "Required local validation passed; commands, results, and any full-suite exception are documented.", "I pushed my PR to the latest dev commit.", "I resolved all correct Codex and CodeRabbit findings.", "My PR is ready for review.", @@ -281,3 +281,14 @@ describe("workflow comment-spam hardening", () => { expect(docs).toContain("CodeRabbit status-comment edits do not trigger the PR gate"); }); }); + +for (const draft of [false, true]) { + test(`invalid contributor PR gets scoped validation guidance (draft=${draft})`, async () => { + const result = await runEnforcePrTarget(await readGateScript(), { + pr: { base: { ref: "main" }, draft, body: "Incomplete description" }, + }); + const body = gateBodyFrom(result); + expect(body).toContain("required local validation has passed with commands, results, and any full-suite exception documented"); + expect(body).not.toContain("local CI is green"); + }); +} diff --git a/tests/cli/cli-connect-readiness.test.ts b/tests/cli/cli-connect-readiness.test.ts index f534ed5b9b..607dfe5fdd 100644 --- a/tests/cli/cli-connect-readiness.test.ts +++ b/tests/cli/cli-connect-readiness.test.ts @@ -13,7 +13,7 @@ import { beforeAll, describe, expect, spyOn, test } from "bun:test"; import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; -import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { chmodSync, copyFileSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { COLD_SPAWN_WARMUP_HOOK_BUDGET_MS, warmColdSpawn } from "../helpers/cold-spawn-warmup"; @@ -55,6 +55,10 @@ type ProbeResult = { newerVersion?: string; selectionUnchanged: boolean; failures: RuntimeProbeFailure[]; + installedIsolation?: Record<"isolated" | "inherited", { + sentinelCalls: Array<{ args: string[]; completed: boolean }>; + lowerCalls: string[]; + }>; }; }; @@ -104,6 +108,8 @@ function runStatusProbe(options: { preferred?: "valid" | "failed" | "missing"; persisted?: boolean; fullDiagnostics?: boolean; + /** Windows-only negative control; always a test-owned external install root. */ + externalInstalledRoot?: string; /** "connect" drives `ocx connect status`; "status" drives the general `ocx status` collector. */ surface?: "connect" | "status"; /** @@ -158,6 +164,8 @@ function runStatusProbe(options: { runtimeEnv.PATH = [selectedDir, lowerDir].join(delimiter); runtimeEnv.HOME = opencodexHome; runtimeEnv.USERPROFILE = opencodexHome; + // Installed Windows runtimes are discovered outside PATH under LOCALAPPDATA. + runtimeEnv.LOCALAPPDATA = join(opencodexHome, "local-app-data"); runtimeEnv.FIXTURE_RUNTIME_DIRS = JSON.stringify({ selected: selectedDir, lower: lowerDir, rejected: rejectedDir }); runtimeEnv.FIXTURE_FULL_DIAGNOSTICS = options.fullDiagnostics ? "1" : "0"; if (options.persisted) writeFileSync(join(opencodexHome, "codex-runtime.json"), JSON.stringify({ @@ -259,6 +267,32 @@ function runStatusProbe(options: { } runtime = { beforeDiagnostics, afterDiagnostics: calls(), diagnosticsCached, newerVersion, selectionUnchanged: selectionBefore === readOptional(selectionPath), failures }; + const externalRoot = process.env.FIXTURE_EXTERNAL_INSTALLED_ROOT; + if (externalRoot) { + const { execFileSync } = require("node:child_process"); + const sentinel = join(externalRoot, "OpenAI", "Codex", "bin", "fixture-installed", "codex.exe"); + const observeInstalled = env => { + const sentinelCalls = []; + const lowerBefore = calls().lower.length; + resolveCodexRuntime({ + env, + execFileSync: (file, args, options) => { + // Record attempts too: a failed external launch still violates isolation. + const call = file === sentinel ? { args: [...args], completed: false } : null; + if (call) sentinelCalls.push(call); + const output = execFileSync(file, args, options); + if (call) call.completed = true; + return output; + }, + }); + return { sentinelCalls, lowerCalls: calls().lower.slice(lowerBefore) }; + }; + // Explicit deps make both observations cold without altering the ordinary cache oracle. + runtime.installedIsolation = { + isolated: observeInstalled({ ...process.env }), + inherited: observeInstalled({ ...process.env, LOCALAPPDATA: externalRoot }), + }; + } } console.log(JSON.stringify({ lines: captured, commandCode, status, runtime, exitCode, errors, catalogUnchanged })); })(); @@ -282,6 +316,7 @@ function runStatusProbe(options: { OPENCODEX_CLAUDE_DESKTOP_CONFIG_DIR: join(opencodexHome, "desktop"), FIXTURE_LADDER: JSON.stringify(options.ladder), FIXTURE_SURFACE: options.surface ?? "connect", + FIXTURE_EXTERNAL_INSTALLED_ROOT: options.externalInstalledRoot ?? "", ...runtimeEnv, }, }); @@ -423,6 +458,30 @@ describe("connected-client runtime probe scope", () => { }); }, COLD_SPAWN_WARMUP_HOOK_BUDGET_MS); + test.skipIf(process.platform !== "win32")("isolates inherited Windows installs without disabling full discovery", () => { + const externalRoot = mkdtempSync(join(tmpdir(), "ocx-readiness-external-")); + try { + const installed = join(externalRoot, "OpenAI", "Codex", "bin", "fixture-installed"); + mkdirSync(installed, { recursive: true }); + // A real PE executable: Bun answers --version and is harmless as an unselected candidate. + copyFileSync(process.execPath, join(installed, "codex.exe")); + const probe = runStatusProbe({ connected: true, ladder: "observed", externalInstalledRoot: externalRoot }); + expect(probe.status.readiness).toBe("ready"); + expect(probe.runtime?.beforeDiagnostics.selected).toEqual([ + "--version", "debug models --bundled", "debug models --bundled", + ]); + expect(probe.runtime?.beforeDiagnostics.lower).toEqual([]); + expect(probe.runtime?.selectionUnchanged).toBe(true); + expect(probe.runtime?.installedIsolation).toEqual({ + isolated: { sentinelCalls: [], lowerCalls: ["--version"] }, + // Removing only the environment isolation must execute the external sentinel. + inherited: { sentinelCalls: [{ args: ["--version"], completed: true }], lowerCalls: ["--version"] }, + }); + } finally { + removeTreeWithRetry(externalRoot); + } + }, SPAWN_BUDGET_MS); + test("observes only the selected runtime and leaves full diagnostics available", () => { const probe = runStatusProbe({ connected: true, ladder: "observed", fullDiagnostics: true }); diff --git a/tests/codex-integration/catalog-full-picker-order.test.ts b/tests/codex-integration/catalog-full-picker-order.test.ts index 272db59408..1360d72648 100644 --- a/tests/codex-integration/catalog-full-picker-order.test.ts +++ b/tests/codex-integration/catalog-full-picker-order.test.ts @@ -182,6 +182,10 @@ describe("picker ordering through production catalog writers", () => { let catalogPath: string; let fetchCalls: number; let runtimeCommand: string; + let fixtureCase = 0; + const phase = (name: string) => { + if (process.env.CI) process.stderr.write(`[catalog-picker-fixture] case=${fixtureCase} phase=${name}\n`); + }; // Same executable-fixture protocol as codex-convergence-account-selectors.test.ts: // a forced resolver refresh must receive the same version and catalog as a warm read. @@ -216,6 +220,8 @@ describe("picker ordering through production catalog writers", () => { } beforeEach(() => { + fixtureCase += 1; + phase("setup:start"); previousEnv = envKeys.map(key => process.env[key]); previousFetch = globalThis.fetch; root = realpathSync.native(mkdtempSync(join(tmpdir(), "ocx-picker-writers-"))); @@ -236,21 +242,27 @@ describe("picker ordering through production catalog writers", () => { runtimeCommand = createRuntimeFixture(catalog); process.env.CODEX_CLI_PATH = runtimeCommand; // Resolve the real fixture executable before admission captures runtime provenance. + phase("setup:catalog-probe:start"); expect(loadBundledCodexCatalog()?.models?.[0]?.slug).toBe("gpt-5.5"); + phase("setup:catalog-probe:end"); assertRuntimeIdentity(); + phase("setup:identity:end"); writeFileSync(catalogPath, JSON.stringify(catalog)); fetchCalls = 0; globalThis.fetch = (async () => { fetchCalls += 1; throw new Error("catalog writer fixture must not make a network request"); }) as typeof fetch; + phase("setup:end"); }); afterEach(() => { + phase("cleanup:database:start"); try { const database = resolveCodexCatalogSerializationDatabasePath(resolveEffectiveUserIdentity(), codexHome); for (const suffix of ["", "-journal", "-wal", "-shm"]) rmSync(`${database}${suffix}`, { force: true }); } finally { + phase("cleanup:restore:start"); globalThis.fetch = previousFetch; envKeys.forEach((key, index) => { const value = previousEnv[index]; @@ -260,7 +272,9 @@ describe("picker ordering through production catalog writers", () => { resetCatalogRuntimeStateForTests(); resetCodexRuntimeResolveCacheForTests(); resetCodexModelEntitlementCacheForTests(); + phase("cleanup:remove:start"); removeTreeWithRetry(root); + phase("cleanup:end"); } }); @@ -285,7 +299,9 @@ describe("picker ordering through production catalog writers", () => { } async function writeCatalog(writer: "convergence" | "retained", next: OcxConfig, degraded = false): Promise { + phase(`${writer}:identity:start`); assertRuntimeIdentity(); + phase(`${writer}:config:start`); const requestedRoster = [...next.subagentModels!]; saveConfig(next); const saved = loadConfig(); @@ -298,15 +314,18 @@ describe("picker ordering through production catalog writers", () => { markModelsFetchFailure("opencode-go"); } if (writer === "convergence") { + phase("convergence:write:start"); const result = await convergeCodexCatalog(captureCatalogAdmissionSnapshot(next), { action: "converge", scope: "catalog", reason: "management-mutation", mode: "explicit", deadlineMs: 5_000, }); expect(result.catalogRefresh).toMatchObject({ status: "committed", degraded }); } else { + phase("retained:write:start"); const result = await syncCatalogModels(next); expect(result.path).toBe(catalogPath); expect(result.skippedReason).toBeUndefined(); } + phase(`${writer}:write:end`); assertRuntimeIdentity(); expect(fetchCalls).toBe(0); return (JSON.parse(readFileSync(catalogPath, "utf8")) as RawCatalog).models ?? []; diff --git a/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts b/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts index eb7e213420..b69dbd97f6 100644 --- a/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts +++ b/tests/codex-integration/codex-reset-credit-auto-redeem.test.ts @@ -278,6 +278,8 @@ describe("reset-credit auto-redeemer runtime (#822)", () => { test("two processes reserve one durable id before either consume settles", async () => { const journalFile = join(dir, "j.json"); const moduleUrl = pathToFileURL(repoPath("src/codex/reset-credit-auto-redeem.ts")).href; + const aclUrl = pathToFileURL(repoPath("src/lib/windows-secret-acl.ts")).href; + const principalUrl = pathToFileURL(repoPath("src/lib/windows-user-principal.ts")).href; const deadline = performance.now() + 25_000; const markerPath = (name: string) => join(dir, name + ".json"); const publish = (name: string) => { @@ -291,6 +293,16 @@ describe("reset-credit auto-redeemer runtime (#822)", () => { import { existsSync, writeFileSync, renameSync } from "node:fs"; import { join } from "node:path"; import { createResetCreditAutoRedeemer } from ${JSON.stringify(moduleUrl)}; + import { setIcaclsRunnerForTests } from ${JSON.stringify(aclUrl)}; + import { setSyntheticWindowsPrincipalForTests } from ${JSON.stringify(principalUrl)}; + // This case proves cross-process SQLite reservation and durable publication, + // not host ACL tools. Their 30s budget exceeds this fixture's 20s deadline. + // Keep real file/SQLite I/O; isolate only unrelated OS helper processes in + // these disposable children. Production hardening remains unchanged. + if (process.platform === "win32") { + setSyntheticWindowsPrincipalForTests("*S-1-5-21-1-2-3-1001"); + setIcaclsRunnerForTests(() => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); + } const home = ${JSON.stringify(dir)}; const worker = ${JSON.stringify(worker)}; const deadline = performance.now() + 20_000; @@ -335,6 +347,7 @@ describe("reset-credit auto-redeemer runtime (#822)", () => { while (true) { if (performance.now() >= deadline) throw new Error("reservation contention deadline exceeded"); scheduledMs = null; + publish(worker + "-tick", {}); outcome = await redeemer.tick(); if (outcome.kind === "dispatched") break; const contention = outcome.kind === "error" && ( @@ -384,7 +397,9 @@ describe("reset-credit auto-redeemer runtime (#822)", () => { const children: ReturnType[] = []; const released = new Set(); const diagnostics = () => children.map(({ worker, child, output }) => - `${worker} pid=${child.pid} exit=${child.exitCode}\nstdout: ${output.stdout}\nstderr: ${output.stderr}`).join("\n"); + `${worker} pid=${child.pid} exit=${child.exitCode} phases=${JSON.stringify( + Object.fromEntries(["ready", "tick", "consume", "result"].map(phase => [phase, existsSync(markerPath(worker + "-" + phase))])), + )}\nstdout: ${output.stdout}\nstderr: ${output.stderr}`).join("\n"); const waitUntil = async (label: string, ready: () => boolean) => { while (true) { for (const { worker, child } of children) { diff --git a/tests/fixtures/macos-stall-observer.sh b/tests/fixtures/macos-stall-observer.sh new file mode 100644 index 0000000000..7c82762a2e --- /dev/null +++ b/tests/fixtures/macos-stall-observer.sh @@ -0,0 +1,175 @@ +#!/usr/bin/env bash +set -euo pipefail +task_root=${1:?fixture directory required} +observer_script=${2:?observer script required} +mkdir -p "$task_root/bin" +cat > "$task_root/bin/uname" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' Darwin +EOF +cat > "$task_root/bin/sleep" <<'EOF' +#!/usr/bin/env bash +if [ "${MODE:-}" = stop ]; then + printf '%s' "$$" > "$CHILD_FILE" + exec /bin/sleep 15 +fi +if [ "${MODE:-}" = progress ]; then printf '.' >> "$SUITE_LOG"; fi +/bin/sleep 0.02 +EOF +cat > "$task_root/bin/ps" <<'EOF' +#!/usr/bin/env bash +if [ "$1" = -axo ]; then + if [ "$MODE" = absent ]; then exit 0; fi + printf '%s %s %s/bin/bun\n' "$CHILD" "$OWNER" "$GITHUB_WORKSPACE" + printf '888888 %s %s/helpers/worker\n' "$CHILD" "$HOME" + if [ "$MODE" = ambiguous ]; then printf '999999 %s /usr/local/bin/bun\n' "$OWNER"; fi +elif [ "$4" = ppid= ]; then + printf '%s\n' "$OWNER" +else + n=0 + [ ! -f "$COUNTER" ] || n=$(cat "$COUNTER") + n=$((n+1)); printf '%s' "$n" > "$COUNTER" + if [ "$MODE" = progress ] && [ "$n" -gt 8 ]; then printf changed; else printf stable; fi +fi +EOF +cat > "$task_root/bin/sample" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$1" >> "$SAMPLED" +if [ "${MODE:-}" = stop-sample ]; then + printf '%s' "$$" > "$SAMPLE_CHILD_FILE" + printf 'partial report %s\n' "$GITHUB_WORKSPACE" > "$4" + exec /bin/sleep 15 +fi +printf 'stdout workspace %s/source/file.ts symbol_name + 42\n' "$GITHUB_WORKSPACE" +printf 'stderr home %s/Library/cache symbol_error + 84\n' "$HOME" >&2 +{ + printf 'report workspace %s/build/object.o UUID ABCD symbol_report + 126\n' "$GITHUB_WORKSPACE" + printf 'report home %s/.cache/object.o\n' "$HOME" + if [ "${MODE:-}" = oversize ]; then + prefix_file="$SUITE_LOG.prefix" + : > "$prefix_file" + # The observer emits these exact sections before this marker. Derive padding + # from that content instead of relying on a fixed approximation. + printf '%s\n' 'sample command output:' >> "$prefix_file" + printf 'stdout workspace %s/source/file.ts symbol_name + 42\n' '${GITHUB_WORKSPACE}' >> "$prefix_file" + printf 'stderr home %s/Library/cache symbol_error + 84\n' '${HOME}' >> "$prefix_file" + printf '%s\n' 'sample report:' >> "$prefix_file" + printf 'report workspace %s/build/object.o UUID ABCD symbol_report + 126\n' '${GITHUB_WORKSPACE}' >> "$prefix_file" + printf 'report home %s/.cache/object.o\n' '${HOME}' >> "$prefix_file" + prefix_bytes=$(wc -c < "$prefix_file") + padding=$((262144 - prefix_bytes - 8)) + test "$padding" -gt 0 + awk -v count="$padding" 'BEGIN { for (i=0; i "$4" +EOF +chmod +x "$task_root/bin/"* +export PATH="$task_root/bin:$PATH" +export OWNER=$$ CHILD=$$ +export HOME="$task_root/home [literal].*" +export GITHUB_WORKSPACE="$HOME/work space & source" +mkdir -p "$GITHUB_WORKSPACE" +for MODE in silent absent ambiguous progress; do + export MODE SUITE_LOG="$task_root/$MODE.log" COUNTER="$task_root/$MODE.counter" SAMPLED="$task_root/$MODE.sampled" + printf start > "$SUITE_LOG" + bash "$observer_script" "$OWNER" "$SUITE_LOG" > "$task_root/$MODE.out" 2>&1 + if [ "$MODE" = silent ]; then + test "$(cat "$SAMPLED")" = "$CHILD" + grep -q 'sample command output:' "$task_root/$MODE.out" + grep -q 'sample report:' "$task_root/$MODE.out" + grep -q '${GITHUB_WORKSPACE}/source/file.ts symbol_name + 42' "$task_root/$MODE.out" + grep -q '${HOME}/Library/cache symbol_error + 84' "$task_root/$MODE.out" + grep -q 'UUID ABCD symbol_report + 126' "$task_root/$MODE.out" + grep -q "^$CHILD $OWNER bun$" "$task_root/$MODE.out" + grep -q '^888888 .* worker$' "$task_root/$MODE.out" + ! grep -Fq "$GITHUB_WORKSPACE" "$task_root/$MODE.out" + ! grep -Fq "$HOME" "$task_root/$MODE.out" + test ! -e "$SUITE_LOG.sample" + test ! -e "$SUITE_LOG.sample-output" + test ! -e "$SUITE_LOG.sample-redacted" + else + test ! -e "$SAMPLED" + fi + kill -0 "$OWNER" + printf 'PASS %s\n' "$MODE" +done + +# Empty workspace must never become an empty-string replacement; HOME still redacts. +export MODE=silent GITHUB_WORKSPACE= SUITE_LOG="$task_root/empty.log" COUNTER="$task_root/empty.counter" SAMPLED="$task_root/empty.sampled" +printf start > "$SUITE_LOG" +bash "$observer_script" "$OWNER" "$SUITE_LOG" > "$task_root/empty.out" 2>&1 +grep -q '${HOME}/Library/cache symbol_error + 84' "$task_root/empty.out" +! grep -q '${GITHUB_WORKSPACE}' "$task_root/empty.out" +printf 'PASS empty-prefix\n' + +# The longer known prefix is redacted first when HOME is nested below workspace. +export GITHUB_WORKSPACE="$task_root/reverse root" +export HOME="$GITHUB_WORKSPACE/private home [literal].*" +export MODE=silent SUITE_LOG="$task_root/reverse.log" COUNTER="$task_root/reverse.counter" SAMPLED="$task_root/reverse.sampled" +mkdir -p "$HOME" +printf start > "$SUITE_LOG" +bash "$observer_script" "$OWNER" "$SUITE_LOG" > "$task_root/reverse.out" 2>&1 +grep -q '${GITHUB_WORKSPACE}/source/file.ts symbol_name + 42' "$task_root/reverse.out" +grep -q '${HOME}/Library/cache symbol_error + 84' "$task_root/reverse.out" +! grep -Fq "$GITHUB_WORKSPACE" "$task_root/reverse.out" +! grep -Fq "$HOME" "$task_root/reverse.out" +printf 'PASS reverse-nesting\n' + +# The cap applies after stdout, stderr and report are combined and redacted. +export HOME="$task_root/home [literal].*" GITHUB_WORKSPACE="$task_root/home [literal].*/work space & source" +export MODE=oversize SUITE_LOG="$task_root/oversize.log" COUNTER="$task_root/oversize.counter" SAMPLED="$task_root/oversize.sampled" +printf start > "$SUITE_LOG" +bash "$observer_script" "$OWNER" "$SUITE_LOG" > "$task_root/oversize.out" 2>&1 +test "$(wc -c < "$task_root/oversize.out")" -le 262300 +! grep -Fq "$GITHUB_WORKSPACE" "$task_root/oversize.out" +! grep -Fq "$HOME" "$task_root/oversize.out" +grep -q '\${GITHUB$' "$task_root/oversize.out" +! grep -q 'private-boundary-tail' "$task_root/oversize.out" +printf 'PASS capped-redaction\n' + +# TERM during a live diagnostic sleep must reap that child immediately without +# touching the suite/owner. This uses the real shell job table, not fake ps. +export MODE=stop SUITE_LOG="$task_root/stop.log" COUNTER="$task_root/stop.counter" CHILD_FILE="$task_root/stop.child" +printf start > "$SUITE_LOG" +bash "$observer_script" "$OWNER" "$SUITE_LOG" > "$task_root/stop.out" 2>&1 & +watcher=$! +for attempt in $(seq 1 200); do + [ ! -f "$CHILD_FILE" ] || break + /bin/sleep 0.01 +done +test -f "$CHILD_FILE" +diagnostic_child=$(cat "$CHILD_FILE") +kill -TERM "$watcher" +wait "$watcher" +! kill -0 "$diagnostic_child" 2>/dev/null +kill -0 "$OWNER" +test ! -e "$SUITE_LOG.sample" +test ! -e "$SUITE_LOG.sample-output" +test ! -e "$SUITE_LOG.sample-redacted" +printf 'PASS stop\n' + +# TERM during the real exec sleep inside sample must still close the Actions +# group on observer stdout and remove both observer-owned diagnostic files. +export MODE=stop-sample SUITE_LOG="$task_root/stop-sample.log" COUNTER="$task_root/stop-sample.counter" \ + SAMPLED="$task_root/stop-sample.sampled" SAMPLE_CHILD_FILE="$task_root/stop-sample.child" +printf start > "$SUITE_LOG" +bash "$observer_script" "$OWNER" "$SUITE_LOG" > "$task_root/stop-sample.out" 2>&1 & +watcher=$! +for attempt in $(seq 1 400); do + [ ! -f "$SAMPLE_CHILD_FILE" ] || break + /bin/sleep 0.01 +done +test -f "$SAMPLE_CHILD_FILE" +diagnostic_child=$(cat "$SAMPLE_CHILD_FILE") +kill -TERM "$watcher" +wait "$watcher" +! kill -0 "$diagnostic_child" 2>/dev/null +kill -0 "$OWNER" +grep -q '^::group::macOS silent-suite diagnostics' "$task_root/stop-sample.out" +grep -q '^::endgroup::$' "$task_root/stop-sample.out" +test ! -e "$SUITE_LOG.sample" +test ! -e "$SUITE_LOG.sample-output" +test ! -e "$SUITE_LOG.sample-redacted" +printf 'PASS stop-sample\n' diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index f4bb72d4eb..d612e32353 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -576,6 +576,9 @@ "digitalocean-scaleway-provider.test.ts": "providers", "docs-429-failover-claims.test.ts": "ci-workflows", "docs-bun-source-requirement.test.ts": "ci-workflows", + "setup-hooks.test.ts": "ci-workflows", + "pr-readiness-reattest.test.ts": "ci-workflows", + "exhaustive-deps-suppression.test.ts": "ci-workflows", "docs-provider-billing-claims.test.ts": "ci-workflows", "docs-provider-preset-counts.test.ts": "ci-workflows", "docs-readme-translation-parity.test.ts": "ci-workflows", diff --git a/tests/helpers/enforce-pr-target-harness.ts b/tests/helpers/enforce-pr-target-harness.ts index 59236b47ab..3657cbc56d 100644 --- a/tests/helpers/enforce-pr-target-harness.ts +++ b/tests/helpers/enforce-pr-target-harness.ts @@ -59,6 +59,8 @@ export type PullRequestState = { body?: string; draft?: boolean; base?: { ref: string }; + head?: { sha: string }; + updated_at?: string; user?: { login: string }; /** `pulls.get` changed_files; omit to default to listed file count in harness. */ changed_files?: number; @@ -68,6 +70,7 @@ export type Comment = { id: number; user?: { login: string }; body?: string; + updated_at?: string; /** GitHub's per-comment association; used for the GUI-screenshot waiver. */ author_association?: string; }; @@ -83,6 +86,18 @@ export type IssueEvent = { export type RunOptions = { /** The PR as `pulls.get` will report it — the live, authoritative state. */ pr: PullRequestState; + /** Per-read authoritative changes, independent of webhook payload snapshots. */ + pullSnapshots?: PullRequestState[]; + /** Previous body carried by a real body-edited webhook. */ + previousBody?: string; + /** Server time on successful bot comment writes. */ + commentUpdatedAt?: string; + /** Distinct server times on each successful comment write. */ + commentUpdatedAts?: string[]; + /** One-based comment-write attempt to fail after an earlier checkpoint succeeds. */ + failCommentWrite?: number; + /** Late authoritative comment override for pre-ready revalidation. */ + finalComment?: Comment; /** * What the webhook delivered, if it differs from `pr`. * @@ -780,9 +795,29 @@ export async function runEnforcePrTarget( return { ahead_by: 0, behind_by: 0 }; } + let pullReadIndex = 0; + let snapshotPr = structuredClone(pr); + let currentGateComment = structuredClone(pages.flat().find(comment => + comment.user?.login === "github-actions[bot]" && comment.body?.includes(""))); + let commentWriteIndex = 0; + const savedComment = (args: unknown, id: number) => { + const index = commentWriteIndex++; + return { id, body: String((args as { body?: string }).body ?? ""), + user: { login: "github-actions[bot]" }, + updated_at: options.commentUpdatedAts?.[index] ?? options.commentUpdatedAt }; + }; + const rest = { pulls: { - get: (args: unknown) => respond("pulls.get", args, pr), + get: (args: unknown) => { + if (!options.pullSnapshots) return respond("pulls.get", args, pr); + const delta = options.pullSnapshots[pullReadIndex++] ?? {}; + snapshotPr = { ...snapshotPr, ...delta, + base: { ...snapshotPr.base, ...delta.base }, + head: { ...snapshotPr.head, ...delta.head }, + user: { ...snapshotPr.user, ...delta.user } }; + return respond("pulls.get", args, structuredClone(snapshotPr)); + }, update: (args: unknown) => respond("pulls.update", args, { ...pr }), // Page-specific open-PR fixtures; missing pages are empty so paginate ends. list: (args: unknown) => { @@ -814,8 +849,21 @@ export async function runEnforcePrTarget( const page = Number((args as { page?: number })?.page ?? 1); return respond("issues.listEvents", args, issueEventPages[page - 1] ?? []); }, - createComment: (args: unknown) => respond("issues.createComment", args, { id: 99 }), - updateComment: (args: unknown) => respond("issues.updateComment", args, { id: 7 }), + getComment: (args: unknown) => respond("issues.getComment", args, options.finalComment ?? currentGateComment), + createComment: async (args: unknown) => { + const saved = savedComment(args, 99); + if (options.failCommentWrite === commentWriteIndex) throw octokitError("issues.createComment", 500); + const response = await respond("issues.createComment", args, saved); + currentGateComment = saved; + return response; + }, + updateComment: async (args: unknown) => { + const saved = savedComment(args, Number((args as { comment_id?: number }).comment_id ?? 7)); + if (options.failCommentWrite === commentWriteIndex) throw octokitError("issues.updateComment", 500); + const response = await respond("issues.updateComment", args, saved); + currentGateComment = saved; + return response; + }, deleteComment: (args: unknown) => respond("issues.deleteComment", args, {}), addLabels: (args: unknown) => respond("issues.addLabels", args, {}), removeLabel: (args: unknown) => respond("issues.removeLabel", args, {}), @@ -1004,6 +1052,7 @@ export async function runEnforcePrTarget( */ payload = { action: options.eventAction ?? (options.eventName === "issue_comment" ? "created" : "opened"), + ...(options.previousBody === undefined ? {} : { changes: { body: { from: options.previousBody } } }), number: eventPr.number, // An issue comment on a PR is delivered with `issue` + `comment`, never // `pull_request`. The gate resolves the PR number from whichever object