diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 10134dd750..1c1ef2bd76 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -100,15 +100,16 @@ limits, security/privacy/containment implications, and meaningful proof gaps. -- The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. -Draft PRs may leave them unchecked. After further changes, clear the final checkbox -until the owner reviews the updated diff and refreshes affected validation. +Draft PRs may leave them unchecked. These declarations record initial readiness +when opening the PR for review; later pushes do not require a new human sign-off. +Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. -- [ ] I declare the current revision ready for maintainer review. +- [ ] I declare this PR ready for initial maintainer review. ## Before merge diff --git a/.github/workflows/pr-readiness.yml b/.github/workflows/pr-readiness.yml new file mode 100644 index 0000000000..84e2d26eb6 --- /dev/null +++ b/.github/workflows/pr-readiness.yml @@ -0,0 +1,51 @@ +name: PR readiness + +on: + pull_request_target: + branches: [main, "release/*"] + types: + [ + opened, + edited, + synchronize, + reopened, + ready_for_review, + converted_to_draft, + ] + workflow_dispatch: + inputs: + pull_request_number: + description: Open PR to recheck + required: true + type: string + +permissions: {} + +concurrency: + group: pr-readiness-${{ github.event.pull_request.number || inputs.pull_request_number }} + cancel-in-progress: true + +jobs: + readiness: + name: Validate author declaration + runs-on: ubuntu-24.04 + timeout-minutes: 3 + permissions: + contents: read + pull-requests: read + statuses: write + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - name: Check initial readiness + uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9 + with: + script: | + const { pathToFileURL } = require("node:url"); + const entry = pathToFileURL( + `${process.env.GITHUB_WORKSPACE}/scripts/ci/pr-readiness.mjs`, + ); + const { runReadinessCheck } = await import(entry.href); + await runReadinessCheck({ github, context, core }); diff --git a/AGENTS.md b/AGENTS.md index 418ee003c1..6e031b9340 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -94,7 +94,7 @@ required to open an issue. Follow [human ownership and readiness](CONTRIBUTING.md#pull-request-ownership-and-readiness) when preparing a PR. AI-assisted contributions are welcome; a human must review and own the final diff, check the validation evidence and its limitations, and -explicitly declare the current revision ready for maintainer review. +explicitly declare the PR ready for initial maintainer review. Keep the PR body concise and current, with the concrete problem, observable outcome, approach, and evidence. Report only checks that actually ran as executed; @@ -103,11 +103,11 @@ Address actionable automated review findings and explain disagreements. Leave the template's **Author readiness** declarations unchecked for the human owner to complete. Never attest to human review on their behalf. Keep the PR in -draft until the human declares the current diff ready. After further changes, -clear the ready-for-review checkbox and refresh affected evidence so the human -can review and renew the declaration. CI and automated review results support -review; they do not establish human readiness, maintainer scope approval, or -merge permission. +draft until the human declares the PR initially ready. Later pushes do not +invalidate that declaration or require another human sign-off. Keep the PR body +and affected validation current as work changes. The readiness check validates +the filled declaration; CI and automated review results do not establish that +human review occurred, maintainer scope approval, or merge permission. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f067f7e4c..10cede32b3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -67,7 +67,7 @@ Before requesting maintainer review, the human owner must: passing. Follow the [verification requirements](#development-feedback-and-pr-verification) for required checks and real-provider coverage. - Complete the **Author readiness** declaration in the PR template, explicitly - stating that the current revision is ready for review. + stating that the PR is ready for initial review. Keep the PR description concise and current. Explain the problem, observable outcome, approach, and useful evidence; link long logs or put them in optional @@ -78,10 +78,23 @@ approval or permission to merge. Draft PRs may request early feedback with open questions and incomplete checks. Leave the readiness declaration unchecked until the human owner has completed -it. After further changes, clear the ready-for-review checkbox until the owner -reviews the updated diff, refreshes affected validation and the PR description, -and renews the declaration. AI agents must leave human declarations to the human -owner and must not attest to a human review on their behalf. +it. The declaration records initial readiness; later pushes do not invalidate +it or require another human sign-off. Keep the PR description and affected +validation current as work changes. AI agents must leave human declarations to +the human owner and must not attest to a human review on their behalf. + +The `PR readiness` check reports pending for drafts and requires a named human +owner and all three template declarations for non-drafts. It checks that the +declaration is filled, not whether the code or evidence is correct. Existing PRs +can copy the **Author readiness** section from the current template. +Open PRs must have distinct head commits because GitHub shares commit statuses. + +Maintainers enable merge enforcement after the workflow is merged: run +`gh workflow run pr-readiness.yml -f pull_request_number=NUMBER` for existing +open PRs, then add `PR readiness` from GitHub Actions to the branch's required +status checks alongside its existing checks. Deploy the workflow to each +protected target branch before requiring it there. The workflow reads PR +metadata using trusted default-branch code and writes only a commit status. Maintainers may return a PR to the author or close it when the problem is unsupported, validation is missing, or the owner cannot explain the change. diff --git a/docs/adb-device-analysis.md b/docs/adb-device-analysis.md index 8c94df25f7..44b5e13347 100644 --- a/docs/adb-device-analysis.md +++ b/docs/adb-device-analysis.md @@ -26,6 +26,9 @@ Readiness is `rea inspect-adb-client`: it runs `adb version` and reports the resolved binary, its version, and where it was selected from. A missing binary reports `capability_unavailable` with the `REA_ADB_PATH` remediation rather than attempting an installation. +The `binary_session` inventory uses the same bounded client-version probe; it +does not query devices, start the adb server, or establish that a requested +serial is reachable. Device state is checked by each device operation. ## Answer an analyst question diff --git a/docs/apktool-resource-analysis.md b/docs/apktool-resource-analysis.md index f29d7948ce..447d6ccfc5 100644 --- a/docs/apktool-resource-analysis.md +++ b/docs/apktool-resource-analysis.md @@ -21,6 +21,8 @@ Readiness is `rea inspect-apktool-client`: it runs `apktool --version` and reports the resolved launcher, its selection source, and its version. A missing launcher reports `capability_unavailable` with the `REA_APKTOOL_COMMAND` remediation rather than attempting an installation. +`binary_session` uses the same host probe for tool availability; it does not +validate a particular APK or guarantee that the selected input can be decoded. On Windows, a `.bat` or `.cmd` launcher (such as Chocolatey's `apktool.bat`) runs through an explicit `cmd.exe /d /s /c` call. That call has no AutoRun, diff --git a/docs/installation.md b/docs/installation.md index 9672455df7..b84d6033a7 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -639,12 +639,18 @@ memory has no original file offset. For FAT Mach-O, offsets refer to the origina container file. Results retain Hopper's image-relative offset, the observed slice base, and the source executable path. Loader selection reads the Mach-O container header, including FAT files with a -single architecture. For FAT64, REA validates the architecture table and selected -Mach-O header, prepares a private thin image, and loads it with Hopper's native -Mach-O loader. It checks the entire source's SHA-256 while copying the slice; -source identity and reported offsets still refer to the original container. -An ambiguous architecture subtype requires an explicitly extracted thin image; -REA does not guess. Configured loader arguments remain explicit overrides. +single architecture. For FAT32 and FAT64 containers, REA validates the +architecture table and selected Mach-O header, prepares a private thin image, +and loads it with Hopper's native Mach-O loader instead of Hopper's implicit +FAT slice choice. For arm64 targets, REA ranks subtype bases 0 (plain arm64) and +2 (arm64e), ignoring capability bits for ranking while retaining the raw subtype +for slice identity. These slices' function-entry encodings decode under +`--aarch64`, ahead of Apple's arm64e.x1 variant (base subtype 12), which is +selected only when it is the arm64 family's only candidate. REA checks the +entire source's SHA-256 while copying the slice; source identity and reported +offsets still refer to the original container. Any other ambiguous architecture +subtype requires an explicitly extracted thin image; REA does not guess. +Configured loader arguments remain explicit overrides. FAT64 is distinct from the CPU architecture: FAT32 containers can contain 64-bit executable images. This preparation avoids the Raw Binary loader dialog observed with native FAT64 loading on Hopper 6.1.0-demo. diff --git a/docs/process-capture.md b/docs/process-capture.md index 3482dc18d8..2bf0d06da7 100644 --- a/docs/process-capture.md +++ b/docs/process-capture.md @@ -383,13 +383,24 @@ exit comparison like `settlement.elapsed_ms`. Every capture requires `truncation_details`, with separate accounting for: - `raw_terminal`: original UTF-8 PTY chunk bytes and observed/retained chunk - counts. A chunk that does not fit `limits.output_bytes` is omitted whole; - a later smaller chunk can still fit. + counts. A chunk that does not fit either its remaining original-text budget + or normalized-text budget is omitted whole; a later smaller chunk can still + fit. A normalized-text omission also omits its original chunk, so raw and + normalized coverage can both be incomplete for that event. +- `normalized_terminal`: normalized comparison-text bytes retained from raw + chunks. This is optional when reading historical captures. A raw-fitting + chunk is omitted if an ordered normalization stage would exceed the remaining + `output_bytes` budget before the final normalized size is known. Its final + size is unknown, not assumed to equal the budget. The capture records the + omitted-frame count and a limitation; later smaller chunks remain eligible. - `rendered_terminal`: cumulative UTF-8 serialized-state and visible-line bytes, plus observed/retained frame counts. This is independent of the raw chunk budget and does not count JSON encoding. Repeated screen snapshots can exhaust it even when all raw output fits. Rendered states use retained PTY - input, so raw omissions also limit rendered coverage. + input, so raw omissions also limit rendered coverage. When bounded + normalization omits a rendered frame before its final size is known, + `unknown_size_frames` reports that count; `observed_bytes` includes only + rendered frame sizes that were determined exactly. - `filesystem_before` and `filesystem_after`: file-count/depth limits, enumeration failures, whole-file hash budget and bytes successfully hashed. Directory names are read incrementally within the remaining entry capacity. diff --git a/scripts/ci/plan.mjs b/scripts/ci/plan.mjs index 4b7d315cfc..0d17d7bbd6 100644 --- a/scripts/ci/plan.mjs +++ b/scripts/ci/plan.mjs @@ -10,6 +10,7 @@ const HOST_TEST_OWNERS = ["apple", "package", "sqlite"]; // workflows' release, publication and self-hosted runner safeguards. const GUARDED_WORKFLOWS = new Set([ ".github/workflows/pr-title.yml", + ".github/workflows/pr-readiness.yml", ".github/workflows/ci-full-pr.yml", ".github/workflows/release.yml", ".github/workflows/real-ghidra-windows.yml", diff --git a/scripts/ci/pr-readiness.d.mts b/scripts/ci/pr-readiness.d.mts new file mode 100644 index 0000000000..5ae731dc08 --- /dev/null +++ b/scripts/ci/pr-readiness.d.mts @@ -0,0 +1,63 @@ +export interface ReadinessPullRequest { + readonly number: number; + readonly state: "open" | "closed"; + readonly draft: boolean; + readonly body: string | null; + readonly head: { readonly sha: string }; + readonly base: { readonly ref: string }; +} +export interface ReadinessResult { + readonly state: "success" | "failure" | "pending"; + readonly description: string; +} +export interface ReadinessGitHub { + readonly rest: { + readonly pulls: { + get(input: { + owner: string; + repo: string; + pull_number: number; + }): Promise<{ data: ReadinessPullRequest }>; + }; + readonly repos: { + createCommitStatus( + input: ReadinessResult & { + owner: string; + repo: string; + sha: string; + context: string; + target_url: string; + }, + ): Promise; + readonly listPullRequestsAssociatedWithCommit: unknown; + }; + }; + paginate( + method: unknown, + input: { + owner: string; + repo: string; + commit_sha: string; + per_page: number; + }, + ): Promise< + readonly Pick[] + >; +} +export function evaluateReadiness( + pull: Pick, + template: string, +): ReadinessResult; +export function runReadinessCheck(input: { + github: ReadinessGitHub; + context: { + repo: { owner: string; repo: string }; + payload: { + pull_request?: { number: number }; + inputs?: { pull_request_number?: string }; + }; + serverUrl: string; + runId: number; + }; + core: { info(message: string): void; setFailed(message: string): void }; +}): Promise; diff --git a/scripts/ci/pr-readiness.mjs b/scripts/ci/pr-readiness.mjs new file mode 100644 index 0000000000..f78f709dd5 --- /dev/null +++ b/scripts/ci/pr-readiness.mjs @@ -0,0 +1,173 @@ +import { readFile } from "node:fs/promises"; + +const result = (state, description) => ({ state, description }); + +function readinessLines(markdown) { + const lines = markdown.split(/\r?\n/u); + const visible = []; + let fence; + let comment = false; + for (const source of lines) { + if (fence) { + if ( + new RegExp( + `^ {0,3}${fence.character}{${fence.length},}\\s*$`, + "u", + ).test(source) + ) + fence = undefined; + continue; + } + const opening = !comment && /^ {0,3}(`{3,}|~{3,})/u.exec(source); + if (opening) { + fence = { character: opening[1][0], length: opening[1].length }; + continue; + } + let line = ""; + let offset = 0; + while (offset < source.length) { + const marker = comment ? "-->" : "", "Human owner: @contributor") + .replaceAll("- [ ]", "- [x]"); +const makePull = ( + changes: Partial = {}, +): ReadinessPullRequest => ({ + number: 42, + state: "open", + draft: false, + body: readyBody, + head: { sha: "a".repeat(40) }, + base: { ref: "main" }, + ...changes, +}); + +function fixture(changes: Partial = {}) { + const pull = makePull(changes); + const get = vi.fn(async () => ({ data: pull })); + const createCommitStatus = vi.fn( + async ( + _status: Parameters< + ReadinessGitHub["rest"]["repos"]["createCommitStatus"] + >[0], + ) => ({}), + ); + const paginate = vi.fn(async () => [pull]); + const github = { + rest: { + pulls: { get }, + repos: { + createCommitStatus, + listPullRequestsAssociatedWithCommit: vi.fn(), + }, + }, + paginate, + }; + const context = { + repo: { owner: "morluto", repo: "rea" }, + payload: { pull_request: { number: 42 } }, + serverUrl: "https://github.com", + runId: 123, + }; + const core = { info: vi.fn(), setFailed: vi.fn() }; + return { github, context, core, get, createCommitStatus, paginate, pull }; +} + +describe("initial author declaration", () => { + it("accepts the filled current template and uppercase checkmarks", () => { + expect(evaluateReadiness(makePull(), template).state).toBe("success"); + expect( + evaluateReadiness( + makePull({ body: readyBody.replaceAll("[x]", "[X]") }), + template, + ).state, + ).toBe("success"); + expect( + evaluateReadiness( + makePull({ body: readyBody.replaceAll("\n", "\r\n") }), + template, + ).state, + ).toBe("success"); + }); + + it("allows drafts without a body or declarations", () => { + expect( + evaluateReadiness(makePull({ draft: true, body: null }), template).state, + ).toBe("pending"); + }); + + it.each([0, 1, 2])("requires declaration %i to be checked", (index) => { + let position = 0; + const body = readyBody.replaceAll("- [x]", () => + position++ === index ? "- [ ]" : "- [x]", + ); + expect(evaluateReadiness(makePull({ body }), template).state).toBe( + "failure", + ); + }); + + it.each([ + null, + readyBody.replace("Human owner: @contributor", "Human owner:"), + readyBody.replace("Human owner: @contributor", "Human owner: @one @two"), + readyBody.replace( + "Human owner: @contributor", + "Human owner: @automation[bot]", + ), + readyBody + "\nHuman owner: @other\n", + readyBody.replace("## Author readiness", "## Other"), + readyBody + readyBody, + ``, + `~~~markdown\n${readyBody}\n~~~`, + `\`\`\`markdown\n${readyBody}\n\`\`\``, + readyBody + .split("\n") + .map((line) => `> ${line}`) + .join("\n"), + readyBody.replace( + "- [x] I declare this PR ready for initial maintainer review.", + "", + ), + readyBody + + "\n- [x] I declare this PR ready for initial maintainer review.\n", + ])( + "rejects missing, hidden, ambiguous, or incomplete declarations (%#)", + (body) => { + expect(evaluateReadiness(makePull({ body }), template).state).toBe( + "failure", + ); + }, + ); + + it("ignores checked examples in comments and fenced blocks before the actual declaration", () => { + for (const example of [ + `\n`, + `~~~markdown\n${readyBody}\n~~~\n`, + `\`\`\`markdown\n${readyBody}\n\`\`\`\n`, + ]) + expect( + evaluateReadiness(makePull({ body: example + readyBody }), template) + .state, + ).toBe("success"); + }); + + it("does not treat an HTML comment inside a code example as hiding the real declaration", () => { + const body = + "~~~html\n