diff --git a/docs/operate/private-repositories.md b/docs/operate/private-repositories.md index fb33e971..e4df485e 100644 --- a/docs/operate/private-repositories.md +++ b/docs/operate/private-repositories.md @@ -87,24 +87,32 @@ Every other channel follows the same four rules: - **Which issue:** found by the marker label, or by its exact title when someone removed the label. Either way it must be one of the action's own reports: the body carries the report heading. A same-titled issue you opened by hand is left alone, unless its body pastes a report verbatim. - **Whose issue:** the creator does not matter, so a rotated PAT reuses the issue instead of opening a second one. With several reports, an open one wins, then the newest. - **Each run:** the body is replaced; the issue opens when the target fails or drifts and closes when it is healthy. +- **A long report is cut to fit:** GitHub keeps at most 65,536 characters in an issue body. A longer report keeps every section's status and leaves lines out: from the transcript first, then from the sections with the most detail. It opens by saying how many lines were left out and marks each cut in place, while the `artifact` channel delivers the whole report. - **What the log says:** every write is announced with the placeholder, never the slug: `report: updated issue #7 in private repository #1` (or `created`), and `report: created label "settings-as-code-report" in private repository #1` when the delivery creates the marker label. A write that landed before a later step failed is announced the same way, ahead of the warning. - **Permission:** the PAT needs `"Issues"` (read and write) on every target repository, on top of the section permissions. +The first delivery creates the marker label, `settings-as-code-report`. Each run reads it once per target, before that target's settings are checked or applied. Where the settings declare `labels` without naming the marker, the answer decides what that section does with it: + +| The read answers | The marker is | Check | Apply | +| ------------------------------ | ------------- | ------------------------------------------------------ | ----------------------------------------------- | +| 404: no such label | left out | says nothing about it | does not create it | +| 200: the label exists | declared | compares it like any declared label | keeps it | +| anything else: the read failed | declared | reports `drift` where the repository has no such label | creates the label where the repository has none | + +A failed read counts as a label that exists, so no label is deleted unread. A settings file that names the marker itself is outside the table: its own entry manages the label, and a rename away from the marker's name is dropped unless the read answers 404. + `private-report: issue-on-failure` is the quiet variant of `issue`. When a target fails (or drifts in check mode) it behaves identically: the reused, marker-labelled report issue is written and opened. -On a healthy run it only looks up an open report issue: +On a healthy run it reads the marker label, then looks for an open report issue carrying it: - **One is open,** left over from an earlier failure: it is updated with the healthy report and closed. -- **None is open:** nothing is written, no issue, no label, zero notification noise, and the log says so: `report: nothing to deliver for private repository #1`. - -A repository that never fails never sees an issue, at the cost that healthy runs' reports are not delivered anywhere. Choose `issue` or `artifact` when you need the report mirror on every run. +- **None is open:** the delivery writes nothing, no issue, no label, zero notification noise, and the log says so: `report: nothing to deliver for private repository #1`. -Two caveats on the quiet variant: +A repository that never fails never sees an issue, and no delivery creates the marker label there. The cost is that healthy runs' reports are not delivered anywhere. Choose `issue` or `artifact` when you need the report mirror on every run. -- **A declared `labels` section still gets the marker.** The `settings-as-code-report` marker label is injected into it, so the apply creates the label even on healthy repositories (a label, not a notification). -- **A removed marker label hides an open issue.** If someone removes the label while a report issue is open, the healthy path cannot find the issue, so it stays open until the next failing run recreates the label and reclaims it. +One caveat on the quiet variant: a removed marker label hides an open issue. If someone removes the label while a report issue is open, the healthy path cannot find the issue, so it stays open until the next failing run recreates the label and reclaims it. -Everything else matches `issue`: the same delivery gate, the same Issues grant (the healthy lookup is a read; the open and close are writes), and delivery still runs in `mode: check`. +Everything else matches `issue`: the same delivery gate, the same Issues grant (with no open issue, a healthy run's delivery only reads; the open and the close are writes), and delivery still runs in `mode: check`. `private-report: artifact` concatenates the report for every proven-private target into one document, encrypts it to an age recipient, and uploads it as the workflow artifact `settings-as-code-private-report` (file `private-report.md.age`). Use it when the people who need the report cannot be given repository access, since the archive travels with the run rather than living in the target repo. diff --git a/src/flows/deliver.ts b/src/flows/deliver.ts index adfcf444..116063ef 100644 --- a/src/flows/deliver.ts +++ b/src/flows/deliver.ts @@ -21,7 +21,6 @@ import { describeProblem, type Problem } from "../problem.js"; import type { ArtifactUploader } from "../report/artifact-report.js"; import { type ClosedOutcome, - isIssueChannel, openReportChannel, type PrivateReportChannel, type RedactedDetail, @@ -137,8 +136,9 @@ export interface OpenedTarget { export interface Delivery { /** - * `work` is told whether to inject the issue channel's marker label. On close, a redacted target proven private or - * internal gets its report, an unproven one the withheld notice, and its public line is closed-value only. + * `work` is told whether to inject the issue channel's marker label, which the channel answers for a target it will + * report on. On close, a redacted target proven private or internal gets its report, an unproven one the withheld + * notice, and its public line is closed-value only. */ target( opened: OpenedTarget, @@ -192,7 +192,9 @@ export async function withDelivery( reports !== null && exposure.kind === "redacted" && isPrivateVisibility(exposure.visibility); const delivery: Delivery = { async target({ repo, channel, exposure }, work) { - const outcome = await work(deliverable(exposure) && isIssueChannel(cfg.privateReport)); + const declaresMarker = + reports !== null && deliverable(exposure) && (await reports.declaresMarker(repo)); + const outcome = await work(declaresMarker); return closeTarget(io, channel, outcome, async (detail) => { if (reports === null) { return; diff --git a/src/report/composer.ts b/src/report/composer.ts index 60c74210..1cfa0d55 100644 --- a/src/report/composer.ts +++ b/src/report/composer.ts @@ -1,6 +1,7 @@ /** - * One markdown document per redacted target, rendered identically for both channels. Pure, and structurally typed on - * purpose, so it depends on no action-layer types. + * One markdown document per redacted target, rendered identically for both channels, except that a channel whose + * document has a length limit gets its lines cut to fit. Pure, and structurally typed on purpose, so it depends on no + * action-layer types. */ import type { AnnotationLevel } from "../io.js"; @@ -29,6 +30,9 @@ export interface ReportInput { transcript: TranscriptLine[]; } +/** Joins the lines of one detail cell: a table row is a single line, so the cell breaks its lines in HTML. */ +const BREAK = "
"; + /** A fence longer than any backtick run inside the content, so a transcript line can never terminate the block early. */ function fenceFor(content: string): string { const longest = content.match(/`+/g)?.reduce((max, run) => Math.max(max, run.length), 0) ?? 0; @@ -42,12 +46,34 @@ function transcriptLine(entry: TranscriptLine): string { /** The report's first line. src/report/issue-report.ts reads it back as the proof that an issue body is one of these reports. */ export const REPORT_HEADING = "# settings-as-code private report:"; -export function composeReport(input: ReportInput): string { +/** How many lines of each list a document carries: one count per section's detail, in outcome order, and the transcript's. */ +interface Kept { + detail: number[]; + transcript: number; +} + +/** Stands where a list was cut, in a detail cell and under the transcript alike. */ +function leftOutMark(count: number): string { + return `(lines left out: ${count})`; +} + +/** The document carrying `kept` lines of each list. A list cut short ends in its mark, and any cut is announced under the heading. */ +function render(input: ReportInput, kept: Kept, limit: number): string { + const total = input.outcomes.reduce((sum, o) => sum + o.detail.length, input.transcript.length); + const carried = kept.detail.reduce((sum, count) => sum + count, kept.transcript); const lines: string[] = [ `${REPORT_HEADING} ${input.target}`, "", "Full, unredacted report for this target. The public run redacts it; this document is its private mirror.", "", + ]; + if (carried < total) { + lines.push( + `**Cut to fit ${limit} characters.** Lines left out: ${total - carried} of ${total}; each cut is marked where it was made.`, + "", + ); + } + lines.push( renderTable("| | |\n|---|---|", [ ["Target", markdownCell(input.target)], ["Admin repository", markdownCell(input.adminRepo)], @@ -59,18 +85,22 @@ export function composeReport(input: ReportInput): string { "", "## Sections", "", - ]; + ); if (input.outcomes.length === 0) { lines.push("No sections ran for this target."); } else { lines.push( renderTable( "| Section | Status | Detail |\n|---|---|---|", - input.outcomes.map((outcome) => [ - markdownCell(outcome.key), - markdownCell(outcome.status), - outcome.detail.map(markdownCell).join("
"), - ]), + input.outcomes.map((outcome, row) => { + const shown = outcome.detail.slice(0, kept.detail[row]).map(markdownCell); + const cut = outcome.detail.length - shown.length; + return [ + markdownCell(outcome.key), + markdownCell(outcome.status), + (cut > 0 ? [...shown, leftOutMark(cut)] : shown).join(BREAK), + ]; + }), ), ); } @@ -78,10 +108,75 @@ export function composeReport(input: ReportInput): string { if (input.transcript.length === 0) { lines.push("No output was captured for this target."); } else { - const body = input.transcript.map(transcriptLine).join("\n"); - const fence = fenceFor(body); - lines.push(fence, body, fence); + const shown = input.transcript.slice(0, kept.transcript).map(transcriptLine); + if (shown.length > 0) { + const body = shown.join("\n"); + const fence = fenceFor(body); + lines.push(fence, body, fence); + } + if (shown.length < input.transcript.length) { + lines.push( + ...(shown.length > 0 ? [""] : []), + leftOutMark(input.transcript.length - shown.length), + ); + } } lines.push(""); return lines.join("\n"); } + +/** + * The lines the document carries under `limit`: each section's detail first, one line per section in turn, then the + * transcript from its start, each list ending at its first line that does not fit. These lines never push the + * document past the limit. The rest (heading, run table, section statuses, marks) is never cut, so a limit below it + * is not met. + */ +function fit(input: ReportInput, limit: number): Kept { + const kept: Kept = { detail: input.outcomes.map(() => 0), transcript: 0 }; + // What a first transcript line adds beyond itself: the two fences, their line ends, and the blank line above the mark. + const fences = 2 * fenceFor(input.transcript.map(transcriptLine).join("\n")).length + 3; + let room = limit - render(input, kept, limit).length - fences; + const take = (cost: number): boolean => { + if (cost > room) { + return false; + } + room -= cost; + return true; + }; + const open = new Set(input.outcomes.keys()); + for (let depth = 0; open.size > 0; depth++) { + for (const row of open) { + const line = input.outcomes[row]?.detail[depth]; + if (line !== undefined && take(markdownCell(line).length + BREAK.length)) { + kept.detail[row] = depth + 1; + } else { + open.delete(row); + } + } + } + for (const entry of input.transcript) { + if (!take(transcriptLine(entry).length + 1)) { + break; + } + kept.transcript += 1; + } + return kept; +} + +function everyLine(input: ReportInput): Kept { + return { + detail: input.outcomes.map((outcome) => outcome.detail.length), + transcript: input.transcript.length, + }; +} + +/** The whole report, whatever its length. */ +export function composeReport(input: ReportInput): string { + return render(input, everyLine(input), Number.POSITIVE_INFINITY); +} + +/** The report for a channel whose document holds `limit` characters: over it, lines are left out and the document counts them. */ +export function composeReportWithin(input: ReportInput, limit: number): string { + const whole = render(input, everyLine(input), limit); + return whole.length <= limit ? whole : render(input, fit(input, limit), limit); +} diff --git a/src/report/delivery.ts b/src/report/delivery.ts index 4a545495..8663836a 100644 --- a/src/report/delivery.ts +++ b/src/report/delivery.ts @@ -20,13 +20,15 @@ import { revealPrivate } from "../private-open.js"; import { describeProblem } from "../problem.js"; import type { SectionKey, SettingsFile } from "../schema.js"; import { type ArtifactUploader, deliverArtifactReport } from "./artifact-report.js"; -import { composeReport } from "./composer.js"; +import { composeReport, composeReportWithin, type ReportInput } from "./composer.js"; import { deliverIssueReport, + ISSUE_BODY_LIMIT, type IssueReportMode, injectMarkerLabel, type LandedWrites, MARKER_LABEL, + mayHoldMarkerLabel, } from "./issue-report.js"; /** The channels, in the words of the `private-report` input; the type below says what each delivers. */ @@ -90,9 +92,10 @@ export interface ReportRunMeta { } /** - * `on` is false when the channel is off or the target is not redacted. The notice is returned rather than emitted, so - * the caller can route it through the target's capturing sink. An injected document is a new document, so it earns - * its brand where every document does: through validateSettingsDoc, the brand's one mint. + * `on` is what the target's channel answered (ReportChannel.declaresMarker): false when no issue channel reports on + * the target, or when the target holds no marker label to keep. The notice is returned rather than emitted, so the + * caller can route it through the target's capturing sink. An injected document is a new document, so it earns its + * brand where every document does: through validateSettingsDoc, the brand's one mint. */ export function applyMarkerInjection( settings: ValidatedSettings, @@ -168,7 +171,16 @@ interface ReportTarget { * content) and never changes any target's result. */ export interface ReportChannel { + /** + * Whether the target's settings must declare the marker label (applyMarkerInjection), asked before the target's + * work. Only the issue channel has a label to keep, and it says no for a target it reads as holding none. + */ + declaresMarker(repo: RepoRef | null): Promise; + + /** Delivers one target's report once the target's work is done. */ deliver(target: ReportTarget): Promise; + + /** Ends the run's deliveries; a channel that gathers its reports sends them here. */ flush(): Promise; } @@ -202,9 +214,9 @@ export function openReportChannel( } /** The seal opens in full here: the readers are the target repository's own, or hold the artifact's decryption key. */ -function composeTargetReport(meta: ReportRunMeta, target: ReportTarget): string { +function reportOf(meta: ReportRunMeta, target: ReportTarget): ReportInput { const { slug, outcomes, transcript } = revealPrivate(target.detail); - return composeReport({ + return { target: slug, adminRepo: meta.adminRepo, runUrl: meta.runUrl, @@ -213,7 +225,7 @@ function composeTargetReport(meta: ReportRunMeta, target: ReportTarget): string timestamp: meta.timestamp, outcomes: outcomes.map((o) => ({ key: o.key, status: o.status, detail: o.detail })), transcript, - }); + }; } function issueChannel( @@ -223,6 +235,8 @@ function issueChannel( io: Io, ): ReportChannel { return { + // An unparseable slug names no repository to read, and no settings are applied to it either. + declaresMarker: async (repo) => repo !== null && (await mayHoldMarkerLabel(api, repo)), async deliver(target) { if (target.repo === null) { // The issue channel posts INTO the target repository, and an unparseable slug names none; the loss must not be silent. @@ -232,7 +246,7 @@ function issueChannel( ); return; } - const body = composeTargetReport(meta, target); + const body = composeReportWithin(reportOf(meta, target), ISSUE_BODY_LIMIT); const delivery = await deliverIssueReport( api, target.repo, @@ -279,8 +293,9 @@ function artifactChannel( ): ReportChannel { const reports: string[] = []; return { + declaresMarker: async () => false, async deliver(target) { - reports.push(`\n\n${composeTargetReport(meta, target)}`); + reports.push(`\n\n${composeReport(reportOf(meta, target))}`); }, async flush() { if (reports.length === 0) { diff --git a/src/report/issue-report.ts b/src/report/issue-report.ts index c605dde7..d183b955 100644 --- a/src/report/issue-report.ts +++ b/src/report/issue-report.ts @@ -56,8 +56,15 @@ export const ISSUE_REPORT_ENDPOINTS = { route: "POST /repos/{owner}/{repo}/labels", statuses: { 201: "marker label created", 422: "the marker label already exists" }, }, + readLabel: { + route: "GET /repos/{owner}/{repo}/labels/{name}", + statuses: { 200: "the target holds the marker label", 404: "the target holds no marker label" }, + }, } as const satisfies Record; +/** GitHub's cap on an issue body, from the 422 a longer one earns every time ("body is too long (maximum is 65536 characters)"); its REST reference states none. A string's length in UTF-16 units is never below that count. */ +export const ISSUE_BODY_LIMIT = 65536; + export type IssueReportMode = "always" | "on-failure"; /** @@ -433,6 +440,25 @@ export async function deliverIssueReport( } } +/** + * False only when the target answers 404 for the marker label. Declared there, the label would be created by the + * labels section, and a check would call a repository no report ever touched drifted. Any other answer, a failed read + * included, declares it without printing anything, so no label is deleted unread and the delivery that follows + * reports a failure that persists. + */ +export async function mayHoldMarkerLabel(api: GitHubClient, repo: RepoRef): Promise { + try { + const label = await api.tryRequest( + "GET", + expand(ISSUE_REPORT_ENDPOINTS.readLabel, { repo }, { name: MARKER_LABEL }), + ); + return !("error" in label && label.error.status === 404); + } catch { + // A client that throws breaks the GitHubClient contract; its message embeds the request path (the private slug), so nothing from it may escape. + return true; + } +} + /** * A declared `labels` section under the delete policy would DELETE the marker label an earlier run's report delivery * created, so the marker joins the declared set unless an entry already manages it. An entry renaming the marker AWAY diff --git a/test/e2e/fuzz.ts b/test/e2e/fuzz.ts index 538cc3cb..25b83ab1 100644 --- a/test/e2e/fuzz.ts +++ b/test/e2e/fuzz.ts @@ -738,10 +738,10 @@ interface Counterfactual { artifactDir?: string; } -/** What a raw target may still receive, by first path segment: the parse gate stops it before any section call, and the report channel's marker-label ensure-create is POST-only, so a labels GET or PATCH is a section call leaking through. */ +/** What a raw target may still receive, by first path segment: the parse gate stops it before any section call, and the report channel reads the marker label (GET) and ensure-creates it (POST), so a labels PATCH or DELETE is a leaked section call. */ const RAW_TARGET_ALLOWED_BY_HEAD: Record> = { contents: new Set(["GET"]), - labels: new Set(["POST"]), + labels: new Set(["GET", "POST"]), issues: new Set(["GET", "POST", "PATCH"]), }; diff --git a/test/e2e/mock/chaos.ts b/test/e2e/mock/chaos.ts index bcfc6a5b..5e0d7d2c 100644 --- a/test/e2e/mock/chaos.ts +++ b/test/e2e/mock/chaos.ts @@ -16,6 +16,7 @@ import type { Json, MockResponse } from "./support.js"; const CORE_FAULT_KEYS = { "core.discoveryList": "GET /user/repos (multi-repo discovery listing)", "core.contentsGet": "GET /repos/{owner}/{repo}/contents/{path} (settings-file fetch)", + "core.reportLabelRead": `${ISSUE_REPORT_ENDPOINTS.readLabel.route} (report marker-label read)`, "core.reportLabelCreate": `${ISSUE_REPORT_ENDPOINTS.createLabel.route} (report marker-label ensure-create)`, "core.issuesList": `${ISSUE_REPORT_ENDPOINTS.list.route} (report issue lookup)`, "core.issueCreate": `${ISSUE_REPORT_ENDPOINTS.create.route} (report issue create)`, diff --git a/test/e2e/mock/core-paths.ts b/test/e2e/mock/core-paths.ts index 4a1f4370..c00bff38 100644 --- a/test/e2e/mock/core-paths.ts +++ b/test/e2e/mock/core-paths.ts @@ -419,6 +419,39 @@ function serveMarkerLabelCreate( return { response: { status: 201, body: label }, coreKey }; } +/** + * The marker-label read precedes the target's work in both modes, so it is served here, beside the ensure-create, and + * only for the marker name on a delivery target; any other label GET falls through to section matching, where no + * route answers it. + */ +function serveMarkerLabelRead( + request: IssueReportRequest, + run: IssueReportRun, +): IssueReportOutcome | null { + const labelMatch = request.pathname.match(/^\/repos\/([^/]+\/[^/]+)\/labels\/([^/]+)$/); + if ( + !labelMatch || + request.method !== "GET" || + decodeURIComponent(labelMatch[2] ?? "") !== MARKER_LABEL + ) { + return null; + } + const slug = decodeURIComponent(labelMatch[1] ?? ""); + if (!isReportDeliveryTarget(slug, run.scenario, run.multi, run.faults)) { + return null; + } + const coreKey = "core.reportLabelRead" as const; + const admitted = admitIssueRoute(request, run, slug, coreKey); + if (!("state" in admitted)) { + return admitted; + } + const label = findLabel(admitted.state, MARKER_LABEL); + return { + response: label ? ok(label) : { status: 404, body: { message: "Not Found" } }, + coreKey, + }; +} + /** GitHub's default is newest first; the number stands in for created_at, which the seeded issues do not carry. */ function listIssues(state: MockState, query: Record): IssueReportOutcome { if (query.sort !== undefined && query.sort !== "created") { @@ -512,6 +545,7 @@ function serveIssueRoute( /** * Null when the path is not an issue-channel route, so the caller falls through to section matching. + * GET /repos/{o}/{r}/labels/{marker} -> the marker read (Issues: read) * POST /repos/{o}/{r}/labels (marker) -> ensure-create (Issues: write) * .../issues -> the issue routes */ @@ -519,7 +553,11 @@ export function handleIssueReport( request: IssueReportRequest, run: IssueReportRun, ): IssueReportOutcome | null { - return serveMarkerLabelCreate(request, run) ?? serveIssueRoute(request, run); + return ( + serveMarkerLabelRead(request, run) ?? + serveMarkerLabelCreate(request, run) ?? + serveIssueRoute(request, run) + ); } /** Wire attempts the probe can make, from the client's own MAX_RETRIES; once a slug's repository.get has faulted this many times the probe has given up, so the next repository.get is not a probe retry and the exemption expires. */ diff --git a/test/e2e/scenarios/multi-report-issue-on-failure-healthy-quiet.yml b/test/e2e/scenarios/multi-report-issue-on-failure-healthy-quiet.yml index 2cbede74..43af0789 100644 --- a/test/e2e/scenarios/multi-report-issue-on-failure-healthy-quiet.yml +++ b/test/e2e/scenarios/multi-report-issue-on-failure-healthy-quiet.yml @@ -1,9 +1,8 @@ # The issue-on-failure channel on a HEALTHY run: total quiet. svc-quiet is -# private and clean (the declared labels, marker included, match live), so the -# channel performs exactly one read - the open-issue lookup by the marker -# label - finds no open report issue, and writes NOTHING: no marker-label -# ensure-create, no issue create or update, no fallback title scan. A healthy -# repo never sees a report issue at all. +# private and converged and never needed a report, so neither its settings nor +# the repository holds the marker label. The channel reads the marker label +# (404: not declared, the check stays clean) and looks up an open report +# issue, finds none, and writes NOTHING: no label, no issue, no title scan. name: multi-report-issue-on-failure-healthy-quiet settings: {} inputs: @@ -15,9 +14,6 @@ repos: labels: - name: declared color: "00ff00" - - name: settings-as-code-report - color: "0e2a47" - description: "managed by settings-as-code private reporting - do not remove" live_state: repo: private: true @@ -25,9 +21,6 @@ repos: labels: - name: declared color: "00ff00" - - name: settings-as-code-report - color: "0e2a47" - description: "managed by settings-as-code private reporting - do not remove" permissions: issues: write contents: read @@ -41,8 +34,8 @@ expect: - "svc-quiet" stdout_lacks: - "svc-quiet" - # Zero report issues created; the one request the channel made was the - # marker-label lookup. + # Zero report issues created; the channel looked for an open one by the + # marker label. issue_report: slug: e2e-owner/svc-quiet created_count: 0 @@ -56,6 +49,8 @@ expect: # Both create-guard reads (the state=all label lookup and the title scan) share # the open lookup's path; their state=all query is what sets them apart. - "GET /repos/e2e-owner/svc-quiet/issues?state=all" - # The single read: an indexed lookup for an OPEN report issue by the marker. + # The two reads: the marker label, then an indexed lookup for an OPEN report + # issue by the marker. requests_contain: + - "GET /repos/e2e-owner/svc-quiet/labels/settings-as-code-report" - "GET /repos/e2e-owner/svc-quiet/issues?state=open&labels=settings-as-code-report&per_page=100" diff --git a/test/e2e/scenarios/multi-report-issue-on-failure-marker-kept.yml b/test/e2e/scenarios/multi-report-issue-on-failure-marker-kept.yml new file mode 100644 index 00000000..63f456dc --- /dev/null +++ b/test/e2e/scenarios/multi-report-issue-on-failure-marker-kept.yml @@ -0,0 +1,68 @@ +# The other side of the marker label: a repository that already holds it +# keeps it. An earlier report left svc-kept the label and a closed report +# issue, and its settings declare `labels` without the marker, where labels +# deletes what is undeclared. The marker read (200) declares the label for this +# run, so the apply writes no label and the check that follows exits 0 with +# zero writes. +name: multi-report-issue-on-failure-marker-kept +settings: {} +inputs: + mode: apply + private_report: issue-on-failure +repos: + e2e-owner/svc-kept: + settings: + labels: + - name: declared + color: "00ff00" + live_state: + repo: + private: true + visibility: private + labels: + - name: declared + color: "00ff00" + - name: settings-as-code-report + color: "0e2a47" + description: "managed by settings-as-code private reporting - do not remove" + issues: + - number: 7 + title: "[automated] settings-as-code: private settings report" + body: "# settings-as-code private report: e2e-owner/svc-kept\n\nOLD REPORT BODY" + state: closed + labels: + - name: settings-as-code-report + user: + login: e2e-token-user + html_url: "https://github.com/e2e-owner/svc-kept/issues/7" + permissions: + issues: write + contents: read + administration: read +expect: + exit_code: 0 + result: applied + repos_result: + "private repository #1": applied + summary_lacks: + - "svc-kept" + stdout_lacks: + - "svc-kept" + issue_report: + slug: e2e-owner/svc-kept + created_count: 0 + lookup_by_label: true + # No label is written in either direction, and the closed issue is left + # alone. + never: + - "DELETE /repos/e2e-owner/svc-kept/labels" + - "POST /repos/e2e-owner/svc-kept/labels" + - "PATCH /repos/e2e-owner/svc-kept/labels" + - "POST /repos/e2e-owner/svc-kept/issues" + - "PATCH /repos/e2e-owner/svc-kept/issues" + # The read that declares the marker for this run. + requests_contain: + - "GET /repos/e2e-owner/svc-kept/labels/settings-as-code-report" + # The check half: a rerun in check mode against the same repository must + # exit 0 with zero writes. + fixpoint: converges diff --git a/test/flows/deliver.test.ts b/test/flows/deliver.test.ts index 05241030..b4ea4298 100644 --- a/test/flows/deliver.test.ts +++ b/test/flows/deliver.test.ts @@ -145,35 +145,50 @@ describe("engineOutcome", () => { }); describe("withDelivery", () => { - test.each<[PrivateReportChannel, Exposure, boolean]>([ - ["none", PRIVATE, false], - ["issue", PRIVATE, true], - ["issue", { kind: "redacted", visibility: "internal" }, true], - ["issue", { kind: "redacted", visibility: "unknown" }, false], - ["issue", SHOWN, false], - ["issue-on-failure", PRIVATE, true], - ["artifact", PRIVATE, false], - ])("under %s a %j target injects the marker: %p", async (channel, exposure, injects) => { - // The artifact channel opens only with an upload port; nothing is delivered here. - const uploader: ArtifactUploader | undefined = - channel === "artifact" ? { upload: async () => ({ uploaded: true as const }) } : undefined; - await delivered( - cfg(channel, "apply"), - {}, - async (delivery, io) => { - const opened = { - repo: repo("o/priv"), - channel: publicChannel(io, "o/priv", true), - exposure, - }; - await delivery.target(opened, async (injectsMarker) => { - expect(injectsMarker).toBe(injects); - return outcome("clean"); - }); - }, - uploader, - ); - }); + // Who is asked: only an issue channel reads the marker label, and only for a target it will report on. + const markerRead = `GET /repos/o/priv/labels/${MARKER_LABEL}`; + // An unrouted GET is the double's 404: the target holds no marker label, so there is none to declare. + const LABEL = { held: { [markerRead]: { data: { name: MARKER_LABEL } } }, absent: {} }; + const INTERNAL: Exposure = { kind: "redacted", visibility: "internal" }; + const UNKNOWN: Exposure = { kind: "redacted", visibility: "unknown" }; + test.each<[PrivateReportChannel, Exposure, keyof typeof LABEL, boolean, number]>([ + ["none", PRIVATE, "held", false, 0], + ["issue", PRIVATE, "held", true, 1], + ["issue", INTERNAL, "held", true, 1], + ["issue", PRIVATE, "absent", false, 1], + ["issue", UNKNOWN, "held", false, 0], + ["issue", SHOWN, "held", false, 0], + ["issue-on-failure", PRIVATE, "held", true, 1], + ["issue-on-failure", PRIVATE, "absent", false, 1], + ["artifact", PRIVATE, "held", false, 0], + ])( + "under %s a %j target with the marker label %s injects it: %p, after %i read(s)", + async (channel, exposure, label, injects, reads) => { + // The artifact channel opens only with an upload port; nothing is delivered here. + const uploader: ArtifactUploader | undefined = + channel === "artifact" ? { upload: async () => ({ uploaded: true as const }) } : undefined; + const asked: string[] = []; + await delivered( + cfg(channel, "apply"), + LABEL[label], + async (delivery, io, events) => { + const opened = { + repo: repo("o/priv"), + channel: publicChannel(io, "o/priv", true), + exposure, + }; + await delivery.target(opened, async (injectsMarker) => { + // The read is over before the target's work starts, so the work plans against its answer. + asked.push(...events); + expect(injectsMarker).toBe(injects); + return outcome("clean"); + }); + }, + uploader, + ); + expect(asked).toEqual(Array(reads).fill(`api ${markerRead}`)); + }, + ); test("channel none: a redacted target closes sealed and speaks one closed-value line; a shown target closes open and stays silent", async () => { const { api } = await delivered(cfg("none", "apply"), {}, async (delivery, io, events) => { @@ -218,6 +233,7 @@ describe("withDelivery", () => { async () => outcome("drift", [{ key: "labels", status: "drift", detail: ["+ CANARY"] }]), ); expect(events).toEqual([ + `api GET /repos/o/priv/labels/${MARKER_LABEL}`, "api POST /repos/o/priv/labels", `api GET /repos/o/priv/issues?state=all&labels=${MARKER_LABEL}&per_page=100&page=1`, "api PATCH /repos/o/priv/issues/7", diff --git a/test/flows/multi.test.ts b/test/flows/multi.test.ts index 666f16bb..d8699cd3 100644 --- a/test/flows/multi.test.ts +++ b/test/flows/multi.test.ts @@ -5,7 +5,8 @@ import { Decrypter, generateX25519Identity, identityToRecipient } from "age-encr import { DEFAULT_DISCOVERY_FILTERS } from "../../src/discovery/discover.js"; import { SectionSelection } from "../../src/engine/section-selection.js"; import { runMulti } from "../../src/flows/multi.js"; -import type { TargetOutcome } from "../../src/flows/redact.js"; +import { REDACTED_NOTE, type TargetOutcome } from "../../src/flows/redact.js"; +import { DEFAULT_API_VERSION, GitHubApi } from "../../src/github/api.js"; import { isPrivate } from "../../src/private.js"; import { describeProblem, type Problem } from "../../src/problem.js"; import { @@ -14,9 +15,12 @@ import { type ArtifactUploader, } from "../../src/report/artifact-report.js"; import { REPORT_HEADING } from "../../src/report/composer.js"; -import { ISSUE_TITLE, MARKER_LABEL } from "../../src/report/issue-report.js"; +import { ISSUE_TITLE, MARKER_LABEL, MARKER_LABEL_CONFIG } from "../../src/report/issue-report.js"; +import { ADMIN_OWNER, ADMIN_SLUG } from "../e2e/constants.js"; +import { startMockServer } from "../e2e/mock/server.js"; +import { scenario } from "../e2e/mock/server-test-support.js"; import { captureIo } from "../io/capture.js"; -import { MockApi } from "../mock-api.js"; +import { MockApi, type Route } from "../mock-api.js"; import { withTempDir } from "../temp-dir.js"; /** True when the target closed sealed: the redaction decision, read from the brand. */ @@ -570,6 +574,21 @@ describe("runMulti redaction (private-repos: redact)", () => { })); }); +/** Answers an issue write the way GitHub does: 422 to a body over its 65536-character limit, on every attempt. */ +class BodyLimitApi extends MockApi { + override async tryRequest( + ...args: Parameters + ): ReturnType { + const [method, path, payload] = args; + const body = (payload as { body?: unknown } | undefined)?.body; + if (method !== "GET" && path.includes("/issues") && String(body ?? "").length > 65536) { + this.calls.push({ method, path, payload }); + return { error: { status: 422, message: "Validation Failed", body: "" } }; + } + return super.tryRequest(...args); + } +} + describe("runMulti private-report: issue wiring", () => { const listPath = (slug: string) => `GET /repos/${slug}/issues?state=all&labels=${MARKER_LABEL}&per_page=100&page=1`; @@ -584,7 +603,7 @@ describe("runMulti private-report: issue wiring", () => { { data?: unknown; error?: { status: number; message: string; body: string } } > = {}, ) { - return new MockApi({ + return new BodyLimitApi({ "GET /repos/o/priv": { data: { description: "CANARY-live", private: true } }, "GET /repos/o/priv/contents/.github/settings.yml": { data: 'repository:\n description: "CANARY-want"\n', @@ -636,13 +655,14 @@ describe("runMulti private-report: issue wiring", () => { expect(publicText).not.toContain("o/priv"); }); - test("marker label is injected when a labels section is declared, notice lands in the report only", async () => { + test("the marker label of a target that holds it is declared, and the notice lands in the report only", async () => { const api = reportApi({ "GET /repos/o/priv/contents/.github/settings.yml": { data: 'labels:\n - name: bug\n color: "d73a4a"\n', }, - "GET /repos/o/priv/labels?per_page=100&page=1": { data: [] }, - }).allowMutations("POST /repos/o/priv/labels", "PATCH /repos/o/priv/labels/bug"); + [`GET /repos/o/priv/labels/${MARKER_LABEL}`]: { data: MARKER_LABEL_CONFIG }, + "GET /repos/o/priv/labels?per_page=100&page=1": { data: [MARKER_LABEL_CONFIG] }, + }).allowMutations("POST /repos/o/priv/labels"); const { io, annotations } = captureIo(); await runMulti( api, @@ -663,6 +683,48 @@ describe("runMulti private-report: issue wiring", () => { expect(annotations.some((a) => a.includes(MARKER_LABEL))).toBe(false); }); + test("a report over GitHub's issue-body limit is delivered cut to fit, every drift line still in its section", async () => { + // Uncut, the body earns GitHub's 422 on every attempt: the report is never delivered and re-running never helps. + const live = Array.from({ length: 300 }, (_, i) => ({ + name: `area/component-${String(i).padStart(3, "0")}`, + color: "ededed", + description: null, + })); + const page = (n: number) => `GET /repos/o/priv/labels?per_page=100&page=${n}`; + const api = reportApi({ + "GET /repos/o/priv/contents/.github/settings.yml": { data: "labels: []\n" }, + [page(1)]: { data: live.slice(0, 100) }, + [page(2)]: { data: live.slice(100, 200) }, + [page(3)]: { data: live.slice(200) }, + [page(4)]: { data: [] }, + }); + const { io, annotations, logs } = captureIo(); + await runTargets( + api, + cfg({ + reposInput: "o/priv", + mode: "check", + privateRepos: "redact", + privateReport: "issue", + selfSlug: "admin/repo", + }), + io, + ); + expect(annotations.filter((a) => a.includes("could not deliver"))).toEqual([]); + expect(logs).toEqual(["report: updated issue #7 in private repository #1"]); + const patch = api.calls.find( + (c) => c.method === "PATCH" && c.path === "/repos/o/priv/issues/7", + ); + const body = String(((patch?.payload ?? {}) as { body?: unknown }).body ?? ""); + expect(body).toMatch( + /^\*\*Cut to fit 65536 characters\.\*\* Lines left out: \d+ of 600; each cut is marked where it was made\.$/m, + ); + // The transcript repeats the Sections table, so the cut falls there and no label is lost from the report. + for (const label of live) { + expect(body).toContain(`labels[${label.name}]: undeclared`); + } + }); + test("delivery failure warns safely and never changes the target result", async () => { const api = reportApi({ "PATCH /repos/o/priv/issues/7": { @@ -909,13 +971,107 @@ describe("runMulti private-report: issue-on-failure wiring", () => { expect(String(payload.body ?? "")).toContain("o/priv"); }); - test("marker injection still fires; its notice lands in the closing report only", async () => { + // The settings file declares one label, "bug", and the repository holds it; only the marker label varies. + const markerRead = `GET /repos/o/priv/labels/${MARKER_LABEL}`; + const bug = { name: "bug", color: "d73a4a", description: null }; + const unread: Route = { error: { status: 500, message: "Server Error", body: "" } }; + /** What the marker read answers, beside the labels the repository really holds. */ + const MARKER = { + absent: { read: { error: { status: 404, message: "Not Found", body: "" } }, live: [bug] }, + held: { read: { data: MARKER_LABEL_CONFIG }, live: [bug, MARKER_LABEL_CONFIG] }, + unreadHeld: { read: unread, live: [bug, MARKER_LABEL_CONFIG] }, + unreadAbsent: { read: unread, live: [bug] }, + } satisfies Record; + const QUIET = ["log: report: nothing to deliver for private repository #1"]; + const MARKER_POST = `POST /repos/o/priv/labels (${MARKER_LABEL})`; + /** A target that plans nothing: an apply with nothing to change still says "applied", and its proof is the empty write list. */ + const converged = (result: "clean" | "applied") => ({ result, writes: [], said: QUIET }); + test.each< + [ + string, + "check" | "apply", + keyof typeof MARKER, + { result: "clean" | "applied" | "drift"; writes: string[]; said: string[] }, + ] + >([ + ["404", "check", "absent", converged("clean")], + ["404", "apply", "absent", converged("applied")], + ["the label", "check", "held", converged("clean")], + ["the label", "apply", "held", converged("applied")], + ["500 over a label that exists", "check", "unreadHeld", converged("clean")], + ["500 over a label that exists", "apply", "unreadHeld", converged("applied")], + // The price of never deleting a label unread: a failed read declares the marker where the repository has none. + [ + "500 over no label", + "check", + "unreadAbsent", + { + result: "drift", + writes: [MARKER_POST, "POST /repos/o/priv/issues"], + said: [ + `log: report: created label "${MARKER_LABEL}" in private repository #1`, + "log: report: created issue #9 in private repository #1", + `annotate warning: private repository #1: drift - labels. ${REDACTED_NOTE}`, + ], + }, + ], + [ + "500 over no label", + "apply", + "unreadAbsent", + { result: "applied", writes: [MARKER_POST], said: QUIET }, + ], + ])( + "the marker read answers %s, in %s: the declared labels plan and write only what that answer allows", + async (_name, mode, marker, planned) => { + // Declared on a 404, a check calls a repository no report ever touched drifted and an apply creates the label there. + // Left undeclared where the label exists, an earlier report's label is drift in check and a DELETE in apply. + const { read, live } = MARKER[marker]; + const api = new MockApi({ + "GET /repos/o/priv": { data: { private: true } }, + "GET /repos/o/priv/contents/.github/settings.yml": { + data: 'labels:\n - name: bug\n color: "d73a4a"\n', + }, + "GET /repos/o/priv/labels?per_page=100&page=1": { data: live }, + [markerRead]: read, + [openListPath("o/priv")]: { data: [] }, + [allListPath("o/priv")]: { data: [] }, + "GET /repos/o/priv/issues?state=all&sort=created&direction=desc&per_page=100&page=1": { + data: [], + }, + "POST /repos/o/priv/issues": { data: { number: 9 } }, + }).allowMutations("POST /repos/o/priv/labels", "DELETE /repos/o/priv/labels/*"); + const { io, events } = captureIo(); + const targets = await runTargets( + api, + cfg({ + reposInput: "o/priv", + mode, + privateRepos: "redact", + privateReport: "issue-on-failure", + selfSlug: "admin/repo", + }), + io, + ); + expect(targets[0]?.result).toBe(planned.result); + expect( + api.mutations().map((c) => { + const name = (c.payload as { name?: string } | undefined)?.name; + return `${c.method} ${c.path}${name === undefined ? "" : ` (${name})`}`; + }), + ).toEqual(planned.writes); + expect(events).toEqual(["mask: o/priv", ...planned.said]); + }, + ); + + test("the marker label of a target that holds it is declared; its notice lands in the closing report only", async () => { const api = new MockApi({ "GET /repos/o/priv": { data: { private: true } }, "GET /repos/o/priv/contents/.github/settings.yml": { data: 'labels:\n - name: bug\n color: "d73a4a"\n', }, - "GET /repos/o/priv/labels?per_page=100&page=1": { data: [] }, + [markerRead]: { data: MARKER_LABEL_CONFIG }, + "GET /repos/o/priv/labels?per_page=100&page=1": { data: [MARKER_LABEL_CONFIG] }, [openListPath("o/priv")]: { data: [issue7] }, "PATCH /repos/o/priv/issues/7": { data: { number: 7 } }, }).allowMutations("POST /repos/o/priv/labels"); @@ -931,11 +1087,13 @@ describe("runMulti private-report: issue-on-failure wiring", () => { }), io, ); - const created = api - .mutations() - .filter((c) => c.method === "POST" && c.path === "/repos/o/priv/labels") - .map((c) => (c.payload as { name?: string }).name); - expect(created).toContain(MARKER_LABEL); + // The apply's one label write creates "bug"; the marker it declared is left as it is. + expect( + api + .mutations() + .filter((c) => c.path.startsWith("/repos/o/priv/labels")) + .map((c) => [c.method, (c.payload as { name?: string }).name]), + ).toEqual([["POST", "bug"]]); const patch = api.calls.find( (c) => c.method === "PATCH" && c.path === "/repos/o/priv/issues/7", ); @@ -945,6 +1103,62 @@ describe("runMulti private-report: issue-on-failure wiring", () => { }); }); +describe("runMulti private-report: the marker read of a hidden target", () => { + test("its trace line is redacted under debug, like the target's other requests", async () => { + // The read is the first request that names the target after the visibility probe; made before the slug is masked, its trace line would print the private slug. + const slug = `${ADMIN_OWNER}/svc-hidden`; + const declared = [{ name: "declared", color: "00ff00" }]; + const handle = await startMockServer( + scenario({ + inputs: { mode: "check", private_report: "issue-on-failure" }, + repos: { + [slug]: { + settings: { labels: declared }, + live_state: { repo: { private: true, visibility: "private" }, labels: declared }, + permissions: { issues: "write", contents: "read", administration: "read" }, + }, + }, + }), + ); + try { + const trace: string[] = []; + const io = { ...captureIo().io, debug: (line: string) => trace.push(line) }; + const api = new GitHubApi({ + token: "t", + io, + baseUrl: handle.url, + apiVersion: DEFAULT_API_VERSION, + }); + const targets = await runTargets( + api, + cfg({ + reposInput: slug, + adminOwner: ADMIN_OWNER, + mode: "check", + privateRepos: "redact", + privateReport: "issue-on-failure", + selfSlug: ADMIN_SLUG, + }), + io, + ); + expect(targets.map((t) => [t.display, t.result])).toEqual([ + ["private repository #1", "clean"], + ]); + expect(handle.violations).toEqual([]); + // The marker read is the run's one 404, so the one traced 404 is its line. + expect( + handle.requests.filter((r) => r.status === 404).map((r) => `${r.method} ${r.pathname}`), + ).toEqual([`GET /repos/${slug}/labels/${MARKER_LABEL}`]); + expect(trace.filter((line) => line.includes("-> 404"))).toEqual([ + expect.stringMatching(/^GET -> 404 \(\d+ms\)$/), + ]); + expect(trace.join("\n")).not.toContain("svc-hidden"); + } finally { + await handle.stop(); + } + }); +}); + describe("runMulti private-report: artifact wiring", () => { /** A capturing uploader plus the age keypair the ciphertext decrypts with. */ async function artifactHarness() { diff --git a/test/report/composer.test.ts b/test/report/composer.test.ts index 528148ca..5b7e0b06 100644 --- a/test/report/composer.test.ts +++ b/test/report/composer.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { composeReport, type ReportInput } from "../../src/report/composer.js"; +import { composeReport, composeReportWithin, type ReportInput } from "../../src/report/composer.js"; function input(overrides: Partial = {}): ReportInput { return { @@ -93,3 +93,130 @@ describe("composeReport", () => { expect(report).not.toContain("| Section |"); }); }); + +describe("composeReportWithin", () => { + const detail = (section: string, count: number): string[] => + Array.from( + { length: count }, + (_, i) => `${section}[entry-${i}]: undeclared, so apply deletes it`, + ); + /** A drifted section whose transcript repeats its detail, as a check run's does. */ + const drifted = (count: number): ReportInput => { + const lines = detail("labels", count); + return input({ + outcomes: [{ key: "labels", status: "drift", detail: lines }], + transcript: lines.map((line) => ({ line: `drift: ${line}` })), + }); + }; + /** What a cut report says about itself: the counts in its head notice, and the count each mark carries. */ + const leftOut = (report: string) => ({ + head: /^\*\*Cut to fit (\d+) characters\.\*\* Lines left out: (\d+) of (\d+); /m + .exec(report) + ?.slice(1) + .map(Number), + marks: [...report.matchAll(/\(lines left out: (\d+)\)/g)].map((match) => Number(match[1])), + }); + const transcriptOf = (report: string): string => report.slice(report.indexOf("## Transcript")); + + test("a report within the limit is the whole report, with no notice", () => { + const report = drifted(5); + const whole = composeReport(report); + expect(composeReportWithin(report, whole.length)).toBe(whole); + expect(leftOut(whole)).toEqual({ head: undefined, marks: [] }); + }); + + test("one character over, the transcript is cut and every detail line stays; the head and the mark count the cut", () => { + // Uncut, a body over the channel's limit is refused on every run, so the report is never delivered at all. + const report = drifted(40); + const whole = composeReport(report); + const limit = whole.length - 1; + const cut = composeReportWithin(report, limit); + expect(cut.length).toBeLessThanOrEqual(limit); + const row = (text: string) => text.split("\n").find((line) => line.startsWith("| labels |")); + expect(row(cut)).toBe(row(whole) as string); + const { head, marks } = leftOut(cut); + expect(marks).toHaveLength(1); + expect(head).toEqual([limit, marks[0] as number, 80]); + // The transcript keeps its first lines, in order, under a closed fence; the mark stands below it. + const kept = 40 - (marks[0] as number); + expect(kept).toBeGreaterThan(0); + expect(transcriptOf(cut)).toBe( + [ + "## Transcript", + "", + "```", + ...(report.transcript.slice(0, kept).map((entry) => entry.line) as string[]), + "```", + "", + `(lines left out: ${marks[0]})`, + "", + ].join("\n"), + ); + }); + + test("when the detail alone is over the limit, sections take turns: a short section keeps its lines and the long one is cut", () => { + // Spent section by section, a long first section would take the whole limit and a failed section's one line would be the one left out. + const long = detail("labels", 60); + const report = input({ + outcomes: [ + { key: "labels", status: "drift", detail: long }, + { key: "rulesets", status: "failed", detail: ["rulesets: 403 Forbidden"] }, + ], + transcript: long.map((line) => ({ line })), + }); + const limit = Math.floor(composeReport(report).length / 3); + const cut = composeReportWithin(report, limit); + expect(cut.length).toBeLessThanOrEqual(limit); + expect(cut).toContain("| rulesets | failed | rulesets: 403 Forbidden |"); + const { head, marks } = leftOut(cut); + // Two marks: the labels cell's, then the transcript's, which is left out whole and so has no fence. + expect(marks).toHaveLength(2); + expect(marks[1]).toBe(60); + expect(head).toEqual([limit, (marks[0] as number) + 60, 121]); + expect(cut).toContain(`${long[0]}
`); + expect(cut).toContain(`
(lines left out: ${marks[0]}) |`); + expect(transcriptOf(cut)).toBe("## Transcript\n\n(lines left out: 60)\n"); + }); + + test("detail and transcript lines never push the body past the limit; the fixed part is not cut, so a limit below it is not met", () => { + // One unpaid character in the fit is a refused body at some limit, so every limit up to the whole report is tried. + const varied = (prefix: string): string[] => + Array.from({ length: 12 }, (_, i) => `${prefix}${"x".repeat((i * 7) % 23)}`); + const report = input({ + outcomes: [ + { key: "labels", status: "drift", detail: ["a | b", "``` c", "", ...varied("d")] }, + { key: "repository", status: "clean", detail: [] }, + { key: "rulesets", status: "failed", detail: ["e\nf", ...varied("g")] }, + ], + transcript: [ + { line: "```" }, + { level: "warning", line: "" }, + ...varied("h").map((line) => ({ line })), + ], + }); + // The fixed part is the body with all 42 lines left out: the heading, the notice, the run table, each section's + // status, and the marks. Its length moves only with the digits of the limit its notice names. + const fixedPart = (limit: number): number => + composeReportWithin(report, 0).length + String(limit).length - 1; + const cuts = Array.from({ length: composeReport(report).length }, (_, limit) => { + const cut = composeReportWithin(report, limit); + const { head, marks } = leftOut(cut); + return { limit, length: cut.length, head, left: marks.reduce((sum, n) => sum + n, 0) }; + }); + const limitsOf = (found: typeof cuts): number[] => found.map((cut) => cut.limit); + const reachable = cuts.filter((cut) => cut.limit >= fixedPart(cut.limit)); + const below = cuts.filter((cut) => cut.limit < fixedPart(cut.limit)); + + expect(limitsOf(reachable.filter((cut) => cut.length > cut.limit))).toEqual([]); + // Below the fixed part nothing more can go: the body is that part, every line left out, and over the limit. + expect( + limitsOf(below.filter((cut) => cut.length !== fixedPart(cut.limit) || cut.left !== 42)), + ).toEqual([]); + expect( + limitsOf(cuts.filter((cut) => cut.head?.join() !== [cut.limit, cut.left, 42].join())), + ).toEqual([]); + // Not vacuous: both sides of the fixed part were tried, and the reachable side met cuts of more than twenty sizes. + expect(below.length).toBeGreaterThan(500); + expect(new Set(reachable.map((cut) => cut.left)).size).toBeGreaterThan(20); + }); +}); diff --git a/test/report/delivery.test.ts b/test/report/delivery.test.ts index 70a9dede..ae423f31 100644 --- a/test/report/delivery.test.ts +++ b/test/report/delivery.test.ts @@ -270,6 +270,29 @@ describe("the artifact channel", () => { expect(annotations).toEqual([]); }); + test("a report longer than an issue body holds is uploaded whole: only the issue channel cuts", async () => { + // The guide names this channel as the one that delivers the whole report; cut here too, the lines an issue leaves out would exist nowhere. + const { recipient, uploader, uploads, decrypt } = await harness(); + const detail = Array.from({ length: 700 }, (_, i) => `labels[label-${i}]: ${"x".repeat(90)}`); + const channel = openReportChannel( + new MockApi({}), + "artifact", + META, + recipient, + silentIo(), + uploader, + ); + await channel?.deliver({ + ...target("o/a", 1), + detail: sealed("o/a", [{ key: "labels", status: "drift", detail }]), + }); + await channel?.flush(); + const document = await decrypt(uploads[0]?.file.data as Uint8Array); + expect(document.length).toBeGreaterThan(65536); + expect(document).toContain(detail.at(-1) as string); + expect(document).not.toContain("Cut to fit"); + }); + test("flush uploads nothing when no target delivered", async () => { const { recipient, uploader, uploads } = await harness(); await openReportChannel( diff --git a/test/report/issue-report.test.ts b/test/report/issue-report.test.ts index d52396a1..2a4718e9 100644 --- a/test/report/issue-report.test.ts +++ b/test/report/issue-report.test.ts @@ -11,6 +11,7 @@ import { injectMarkerLabel, MARKER_LABEL, MARKER_LABEL_CONFIG, + mayHoldMarkerLabel, } from "../../src/report/issue-report.js"; import type { SettingsFile } from "../../src/schema.js"; import { MockApi, type Route } from "../mock-api.js"; @@ -506,6 +507,33 @@ describe("deliverIssueReport under mode: on-failure", () => { }); }); +describe("mayHoldMarkerLabel", () => { + const READ = `GET /repos/o/private-repo/labels/${MARKER_LABEL}`; + const failed = (status: number): Route => ({ error: { status, message: "CANARY", body: "" } }); + + // Only a 404 may leave the marker undeclared: were a failed read to do it too, an apply would delete a label it never saw. + test.each<[string, Route, boolean]>([ + ["the label (200)", { data: MARKER_LABEL_CONFIG }, true], + ["no such label (404)", failed(404), false], + ["a denial (403)", failed(403), true], + ["a server error (500)", failed(500), true], + ["no HTTP answer", { failed: `${READ}: the transport failed` }, true], + ])("%s answers %p, after one read", async (_name, route, held) => { + const api = new MockApi({ [READ]: route }); + expect(await mayHoldMarkerLabel(api, SLUG)).toBe(held); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([READ]); + }); + + test("a client that throws instead of answering is read like any failed read, and its message never escapes", async () => { + // The thrown message embeds the request path, which names the private slug. + const api = new MockApi({}); + api.tryRequest = async (method, path) => { + throw new Error(`${method} ${path}: connection reset`); + }; + expect(await mayHoldMarkerLabel(api, SLUG)).toBe(true); + }); +}); + describe("injectMarkerLabel", () => { type Outcome = ReturnType["outcome"]; const bug = { name: "bug", color: "d73a4a" };