From b4bff18f75c35fc66d4581f9a26dd9c982201f99 Mon Sep 17 00:00:00 2001 From: leyoonafr Date: Sat, 25 Jul 2026 16:43:45 +0800 Subject: [PATCH 1/2] fix issue loop control plane recovery --- .github/workflows/issue-dev-loop-evidence.yml | 6 +- loops/issue-dev-loop/LOOP.md | 4 +- loops/issue-dev-loop/SKILL.md | 4 +- .../references/github-operations.md | 2 +- loops/issue-dev-loop/review/REVIEW.md | 2 +- .../issue-dev-loop/review/reviewer-prompt.md | 2 +- .../scripts/github-command-gate.mjs | 8 +- .../scripts/lib/github-identity.mjs | 105 ++++------ .../scripts/lib/review-publication.mjs | 164 ++++++++++++++++ .../issue-dev-loop/scripts/lib/run-store.mjs | 98 ++++++++-- .../issue-dev-loop/scripts/lib/validation.mjs | 8 + .../issue-dev-loop/scripts/publish-review.mjs | 49 +++++ .../templates/implementation-brief.md | 2 + loops/issue-dev-loop/templates/pr-body.md | 6 + .../tests/github-identity-routing.test.mjs | 184 +++++++++++------- loops/issue-dev-loop/tests/runtime.test.mjs | 124 +++++++++++- scripts/verify-docs-ui.mjs | 4 +- tests/docs-cutover.test.ts | 7 +- 18 files changed, 616 insertions(+), 163 deletions(-) create mode 100644 loops/issue-dev-loop/scripts/lib/review-publication.mjs create mode 100644 loops/issue-dev-loop/scripts/publish-review.mjs diff --git a/.github/workflows/issue-dev-loop-evidence.yml b/.github/workflows/issue-dev-loop-evidence.yml index cdb51587..6b62c6ee 100644 --- a/.github/workflows/issue-dev-loop-evidence.yml +++ b/.github/workflows/issue-dev-loop-evidence.yml @@ -148,7 +148,8 @@ jobs: docker run --rm --network none \ --mount "type=volume,src=${candidate_volume},dst=/work" \ "${image}" \ - pnpm verify 2>&1 | tee "${RUNNER_TEMP}/issue-dev-evidence/pnpm-verify.log" + sh -ceu 'git config --global --add safe.directory /work; pnpm verify' \ + 2>&1 | tee "${RUNNER_TEMP}/issue-dev-evidence/pnpm-verify.log" candidate_exit_code="${PIPESTATUS[0]}" set -e baseline_status=blocked @@ -158,7 +159,8 @@ jobs: docker run --rm --network none \ --mount "type=volume,src=${baseline_volume},dst=/work" \ "${image}" \ - pnpm test 2>&1 | tee "${RUNNER_TEMP}/issue-dev-evidence/owner-merged-baseline-tests.log" + sh -ceu 'git config --global --add safe.directory /work; pnpm test' \ + 2>&1 | tee "${RUNNER_TEMP}/issue-dev-evidence/owner-merged-baseline-tests.log" baseline_exit_code="${PIPESTATUS[0]}" set -e if [[ "${baseline_exit_code}" == "0" ]]; then diff --git a/loops/issue-dev-loop/LOOP.md b/loops/issue-dev-loop/LOOP.md index e2449717..dd91b1e6 100644 --- a/loops/issue-dev-loop/LOOP.md +++ b/loops/issue-dev-loop/LOOP.md @@ -83,11 +83,11 @@ The recorded implementation commit is the product-code boundary. Later commits m ### 5. Verify before publication -Run relevant checks and `pnpm verify`. For UI behavior, capture before/after screenshots under `screen-shots/` at meaningful desktop and mobile viewports and include interaction or accessibility evidence where applicable. Bind `before` to the frozen base and `after` to the latest `$implement` commit; never put the containing commit's not-yet-known hash inside its own files. Commit sanitized screenshots and run metadata to the issue branch. The low-privilege `pull_request` evidence workflow checks out the exact candidate head, resolves its frozen run base, proves that base remains an ancestor of live `dev`, and separately checks out the immutable owner-merged base without persisted credentials. It installs candidate and baseline dependencies with lifecycle scripts disabled into distinct Docker volumes. Candidate `pnpm verify` and the actual frozen owner-merged baseline `pnpm test` run in separate no-network containers that receive no GitHub token and cannot mount any host checkout. Before accepting the artifact, the installed control plane independently rejects any exact-head change to the workflow or trusted control/verification plane. The manifest binds candidate head, workflow-run SHA, frozen owner-merged base SHA, and the live PR base SHA that selected the workflow. +Run relevant checks and `pnpm verify`. For UI behavior, capture before/after screenshots under `screen-shots/` at meaningful desktop and mobile viewports and include interaction or accessibility evidence where applicable. Bind `before` to the frozen base and `after` to the latest `$implement` commit; never put the containing commit's not-yet-known hash inside its own files. Commit sanitized screenshots and run metadata to the issue branch. The Draft PR body must render at least one representative before/after pair with Markdown image syntax, using `raw.githubusercontent.com` URLs pinned to the exact recorded head; links to an index or manifest alone are not UI evidence. The low-privilege `pull_request` evidence workflow checks out the exact candidate head, resolves its frozen run base, proves that base remains an ancestor of live `dev`, and separately checks out the immutable owner-merged base without persisted credentials. It installs candidate and baseline dependencies with lifecycle scripts disabled into distinct Docker volumes. Candidate `pnpm verify` and the actual frozen owner-merged baseline `pnpm test` run in separate no-network containers that receive no GitHub token and cannot mount any host checkout. Before accepting the artifact, the installed control plane independently rejects any exact-head change to the workflow or trusted control/verification plane. The manifest binds candidate head, workflow-run SHA, frozen owner-merged base SHA, and the live PR base SHA that selected the workflow. ### 6. Create the draft PR -Push the issue branch and create a draft PR targeting `dev`. Immediately bind it to the run with `record-pr`; later evidence and review gates accept only that PR and head. The PR must include the issue, run ID, base/head SHAs, risk, changes, evidence links, screenshots, known limitations, and explicit owner-only merge language. In `Verification`, bind every manifest check to its own result using the exact line `- \`\`: passed (exit code 0)`; a summary-level pass statement is not sufficient. +Push the issue branch and create a draft PR targeting `dev`. Immediately bind it to the run with `record-pr`; later evidence and review gates accept only that PR and head. The PR must include the issue, run ID, base/head SHAs, risk, changes, evidence links, rendered before/after screenshots for UI work, known limitations, and explicit owner-only merge language. In `Verification`, bind every manifest check to its own result using the exact line `- \`\`: passed (exit code 0)`; a summary-level pass statement is not sufficient. ### 7. Independent review diff --git a/loops/issue-dev-loop/SKILL.md b/loops/issue-dev-loop/SKILL.md index b96d4dc6..dcecc252 100644 --- a/loops/issue-dev-loop/SKILL.md +++ b/loops/issue-dev-loop/SKILL.md @@ -37,13 +37,13 @@ Require `$implement` to write a unique `logs/runs//implementation-result ## Publish a draft PR -Push only the issue branch and create a **draft** PR targeting `dev` using `templates/pr-body.md`; preserve its machine-readable run marker and owner-only merge statement. Bind it immediately with `loopctl.mjs record-pr --run-id --pr-url --head-sha `. Include the issue, risk assessment, test results, evidence manifest, screenshots, known limitations, run ID, and exact head SHA. Never target `main` from an issue branch. +Push only the issue branch and create a **draft** PR targeting `dev` using `templates/pr-body.md`; preserve its machine-readable run marker and owner-only merge statement. Bind it immediately with `loopctl.mjs record-pr --run-id --pr-url --head-sha `. Include the issue, risk assessment, test results, evidence manifest, screenshots, known limitations, run ID, and exact head SHA. For UI work, render at least one representative before/after pair directly in the PR with Markdown images whose `raw.githubusercontent.com` URLs are pinned to that exact head; an index or manifest link alone is insufficient. Never target `main` from an issue branch. ## Run independent review After the draft PR exists, spawn the project agent `echo_ui_pr_reviewer` with a fresh context. Give it only the issue snapshot, acceptance criteria, repository instructions, base SHA, head SHA, diff, CI results, and evidence manifest. Do not give it executor conversation history or rationale. -Publish every round through the installed wrapper with role `reviewer`; the executor identity may not author it. Every PR write must include `--repo codeacme17/echo-ui` (or use the full configured PR URL). Use `gh pr review --comment --body ` for a body-only review and include the exact run/cycle/round/head marker; the gate rejects body files, skipped rounds, duplicates, and publications outside the next durable cycle. When file/line findings require inline comments, use only the reviewer's gated `POST repos/codeacme17/echo-ui/pulls//reviews` API shape with `event=COMMENT`, the exact durable head, and validated inline fields. Post findings verbatim as one review plus inline comments. Each finding needs a run-wide stable ID, severity, confidence, evidence, expected resolution, and the published GitHub inline comment ID or `null` for a body-only finding. Record the GitHub review URL for each round. Accepted repairs must start after that round was submitted, and executor replies must be posted through the automation wrapper after the corresponding `$implement` invocation finishes. One executor response comment may resolve only one finding. Follow `review/REVIEW.md` and `review/response-policy.md`. +Publish every round through the installed wrapper with role `reviewer`; the executor identity may not author it. Every PR write must include `--repo codeacme17/echo-ui` (or use the full configured PR URL). Use `gh pr review --comment --body ` for a body-only review and include the exact run/cycle/round/head marker; the gate rejects body files, skipped rounds, duplicates, and publications outside the next durable cycle. When file/line findings require inline comments, use the installed `publish-review.mjs` command through the reviewer wrapper. Give it the recorded PR number and head, cycle and round, inline body text, and one canonical JSON object per comment; the trusted publisher validates every field and submits one exact-head `COMMENT` review. Do not construct nested `gh api` fields directly. Post findings verbatim as one review plus inline comments. Each finding needs a run-wide stable ID, severity, confidence, evidence, expected resolution, and the published GitHub inline comment ID or `null` for a body-only finding. Record the GitHub review URL for each round. Accepted repairs must start after that round was submitted, and executor replies must be posted through the automation wrapper after the corresponding `$implement` invocation finishes. One executor response comment may resolve only one finding. Follow `review/REVIEW.md` and `review/response-policy.md`. The executor must classify every finding as `accepted`, `rejected`, `needs-human`, `stale`, or `already-fixed`: diff --git a/loops/issue-dev-loop/references/github-operations.md b/loops/issue-dev-loop/references/github-operations.md index 21691c24..15e58eb1 100644 --- a/loops/issue-dev-loop/references/github-operations.md +++ b/loops/issue-dev-loop/references/github-operations.md @@ -18,7 +18,7 @@ Publish reviewer output only as a non-approving comment review: "$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity" --loop-root "$ECHO_UI_LOOP_TARGET_ROOT" reviewer -- gh pr review --repo codeacme17/echo-ui --comment --body ``` -The shell launcher removes Node preload hooks before starting the router. The router then builds a strict child environment, rejects reviewer pushes and every reviewer mutation except a comment-only review on the recorded PR, and rejects approvals, change requests, merges, executor-authored reviews, GraphQL, and administration APIs. Reviewer publication requires the next run/cycle/round marker; the gate paginates existing reviews and rejects duplicates, skipped rounds, body files, or an omitted reviewer publication. A reviewer API write is permitted only for one exact-head `event=COMMENT` review with validated inline fields. Authenticated Git disables worktree hooks, fsmonitor, external diff, textconv, proxy environment variables, repository-local HTTP/proxy/cookie/header/SSL/URL rewrites, and the `ext` transport. Remote Git is restricted to exact origin push/fetch/issue-branch discovery shapes and executes against a canonical configured-repository HTTPS URL. Automation API mutations are limited to the current issue's exact claim label, the current issue/PR or journal comments, and current-PR review-comment replies. PR bodies and comments must be inline; body files, API input files, and typed `@file` expansion are rejected. PR creation requires the durably authorized issue or published evolve request branch, explicit `--repo codeacme17/echo-ui`, `--base dev`, and `--draft`; later PR writes require the same explicit repository (or full configured PR URL), the exact durable checkpoint, the recorded live PR branch/base/head, and the phase-specific Draft/evidence/review gates. `pr edit` is limited to title/body updates and requesting `codeacme17`; non-`--undo` `pr ready` is rejected because only the owner may mark a Draft Ready. Issue pushes derive `codex/issue-` from the durable checkpoint, require a clean exact local issue branch whose post-`$implement` commits contain only the current run's evidence paths, and reject local branch overrides; evolve pushes require the automation-authored request publication. Every other push shape is rejected. +The shell launcher removes Node preload hooks before starting the router. The router then builds a strict child environment, rejects reviewer pushes and every reviewer mutation except a comment-only review on the recorded PR, and rejects approvals, change requests, merges, executor-authored reviews, GraphQL, and administration APIs. Reviewer publication requires the next run/cycle/round marker; the gate paginates existing reviews and rejects duplicates, skipped rounds, body files, or an omitted reviewer publication. Body-only reviews use the validated `gh pr review --comment` shape. Inline reviews must use the installed `publish-review.mjs`; it accepts only canonical comment JSON, validates the durable exact head and finding markers, and is the sole component allowed to stream a canonical `event=COMMENT` payload to GitHub. Authenticated Git disables worktree hooks, fsmonitor, external diff, textconv, proxy environment variables, repository-local HTTP/proxy/cookie/header/SSL/URL rewrites, and the `ext` transport. Remote Git is restricted to exact origin push/fetch/issue-branch discovery shapes and executes against a canonical configured-repository HTTPS URL. Automation API mutations are limited to the current issue's exact claim label, the current issue/PR or journal comments, and current-PR review-comment replies. PR bodies and comments must be inline; body files, arbitrary API input files, and typed `@file` expansion are rejected. PR creation requires the durably authorized issue or published evolve request branch, explicit `--repo codeacme17/echo-ui`, `--base dev`, and `--draft`; later PR writes require the same explicit repository (or full configured PR URL), the exact durable checkpoint, the recorded live PR branch/base/head, and the phase-specific Draft/evidence/review gates. `pr edit` is limited to title/body updates and requesting `codeacme17`; non-`--undo` `pr ready` is rejected because only the owner may mark a Draft Ready. Issue pushes derive `codex/issue-` from the durable checkpoint, require a clean exact local issue branch whose post-`$implement` commits contain only the current run's evidence paths, and reject local branch overrides; evolve pushes require the automation-authored request publication. Every other push shape is rejected. ## Selection and claim diff --git a/loops/issue-dev-loop/review/REVIEW.md b/loops/issue-dev-loop/review/REVIEW.md index 33fa9355..da0f8888 100644 --- a/loops/issue-dev-loop/review/REVIEW.md +++ b/loops/issue-dev-loop/review/REVIEW.md @@ -24,7 +24,7 @@ Do not emit formatter noise, personal style preferences, speculative concerns, u ## Publication -The fresh reviewer publishes each round verbatim through `reviewerGitHubLogin` as one non-approving GitHub review, adding `` for deduplication and `` to the round body. A body-only round uses the gated `gh pr review --comment --body ` command; body files are rejected so the identity gate can validate the marker before publication. Findings with a concrete file and line must be posted in one gated `POST repos//pulls//reviews` call whose event is exactly `COMMENT`, commit ID is the durable recorded head, and inline path/line/side/body fields carry the finding markers; arbitrary input files and every other mutating API shape are rejected. Cross-cutting findings remain in the review body. The reviewer identity must differ from the executor and owner. +The fresh reviewer publishes each round verbatim through `reviewerGitHubLogin` as one non-approving GitHub review, adding `` for deduplication and `` to the round body. A body-only round uses the gated `gh pr review --comment --body ` command; body files are rejected so the identity gate can validate the marker before publication. Findings with a concrete file and line must be posted with the installed `publish-review.mjs` through the reviewer wrapper. Pass `--loop-root`, `--pr`, the durable `--head`, `--cycle`, `--round`, the inline `--body`, and one `--comment` canonical JSON object for each finding. Each comment object contains only `path`, `line`, `side`, and `body`; the trusted publisher validates exact-head state, markers, paths, lines, sides, ordering, and uniqueness before submitting one `COMMENT` review with a canonical JSON payload. Do not construct nested `gh api` fields or pass arbitrary input files. Cross-cutting findings remain in the review body. The reviewer identity must differ from the executor and owner. Before the final round is published, write the complete cycle result with a temporary/unassigned final `reviewUrl` and run the installed `loopctl review-digest --result `. Put its returned `` marker in the final body. After GitHub assigns the review URL, replace the temporary value and rerun `review-digest`; it must be unchanged. This canonical publication digest replaces only review URLs with a fixed placeholder and therefore avoids a URL↔digest cycle. The later recorded full-file digest still protects the final URLs and every other byte. The reviewer must not downgrade severity or omit findings. diff --git a/loops/issue-dev-loop/review/reviewer-prompt.md b/loops/issue-dev-loop/review/reviewer-prompt.md index 57d8704d..22e0da5c 100644 --- a/loops/issue-dev-loop/review/reviewer-prompt.md +++ b/loops/issue-dev-loop/review/reviewer-prompt.md @@ -1 +1 @@ -Review the Echo UI PR diff between `` and `` as an independent, fresh-context reviewer for review cycle ``. Use `$code-review`. Read only the supplied issue snapshot, acceptance criteria, repository instructions, diff, CI results, and evidence manifest. Follow `loops/issue-dev-loop/review/REVIEW.md`. Do not edit files or rely on executor conversation history. Publish a body-only non-approving review through `"$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity" --loop-root "$ECHO_UI_LOOP_TARGET_ROOT" reviewer -- gh pr review --repo codeacme17/echo-ui --comment --body ` and include the exact run/cycle/round/head marker. Before a final round, use the installed `loopctl review-digest` procedure so the marker is computed before GitHub assigns the final review URL. If file/line findings require inline comments, use the same installed launcher with only the strict `gh api repos/codeacme17/echo-ui/pulls//reviews --method POST` COMMENT-review field shape documented by `REVIEW.md`. Never use the repository launcher for credentials, invoke the `.mjs` router directly, run raw `gh`, use `--body-file`, use `--input`, call any other mutating API, approve/request changes/merge, or alter global authentication. Return structured findings or `PASS` and the unique review URL. +Review the Echo UI PR diff between `` and `` as an independent, fresh-context reviewer for review cycle ``. Use `$code-review`. Read only the supplied issue snapshot, acceptance criteria, repository instructions, diff, CI results, and evidence manifest. Follow `loops/issue-dev-loop/review/REVIEW.md`. Do not edit files or rely on executor conversation history. Publish a body-only non-approving review through `"$ECHO_UI_LOOP_CONTROL_PLANE/scripts/with-github-identity" --loop-root "$ECHO_UI_LOOP_TARGET_ROOT" reviewer -- gh pr review --repo codeacme17/echo-ui --comment --body ` and include the exact run/cycle/round/head marker. Before a final round, use the installed `loopctl review-digest` procedure so the marker is computed before GitHub assigns the final review URL. If file/line findings require inline comments, use the same installed launcher to run `node "$ECHO_UI_LOOP_CONTROL_PLANE/scripts/publish-review.mjs"` with the exact arguments documented by `REVIEW.md`; provide each comment as canonical JSON. Never use the repository launcher for credentials, invoke the `.mjs` router directly, run raw `gh`, construct nested API fields, use `--body-file`, call any other mutating API, approve/request changes/merge, or alter global authentication. Return structured findings or `PASS` and the unique review URL. diff --git a/loops/issue-dev-loop/scripts/github-command-gate.mjs b/loops/issue-dev-loop/scripts/github-command-gate.mjs index c3e49a3e..9276117a 100755 --- a/loops/issue-dev-loop/scripts/github-command-gate.mjs +++ b/loops/issue-dev-loop/scripts/github-command-gate.mjs @@ -29,11 +29,17 @@ async function main() { if (!['git', 'gh'].includes(executableName)) { throw new Error(`unsupported authenticated tool: ${tool}`) } + if ( + tool === 'credential' && + (args.length !== 1 || !['get', 'store', 'erase'].includes(args[0])) + ) { + throw new Error('Git credential helper requires one supported operation') + } const trustedControlPlane = await loadTrustedControlPlane() const executable = trustedControlPlane.executables[executableName] const executableArgs = tool === 'credential' - ? ['auth', 'git-credential'] + ? ['auth', 'git-credential', ...args] : tool === 'git' ? hardenedGitArguments(args, { expectedRepository: authorization?.expectedRepository, diff --git a/loops/issue-dev-loop/scripts/lib/github-identity.mjs b/loops/issue-dev-loop/scripts/lib/github-identity.mjs index df4ba88d..d10001ab 100644 --- a/loops/issue-dev-loop/scripts/lib/github-identity.mjs +++ b/loops/issue-dev-loop/scripts/lib/github-identity.mjs @@ -21,6 +21,10 @@ import { verifyLatestDurableCheckpoint, } from './checkpoint-proof.mjs' import { verifyPublishedEvolveRequest } from './evolve.mjs' +import { + parseReviewPublisherArguments, + reviewPublisherSyntheticGitHubArguments, +} from './review-publication.mjs' import { readEvents } from './run-store.mjs' const execFileAsync = promisify(execFile) @@ -896,72 +900,6 @@ function apiField(field) { : { name: field.slice(0, separator), value: field.slice(separator + 1) } } -function reviewerInlineReviewAllowed(request, authorization, expectedRepository) { - const match = request.endpoint?.match(/^repos\/([^/]+\/[^/]+)\/pulls\/(\d+)\/reviews$/) - if ( - request.method !== 'POST' || - request.usesInput || - request.usesFileExpansion || - !match || - !repositoryInScope(match[1], expectedRepository) || - Number(match[2]) !== authorization?.issue?.prNumber || - !authorization?.issue?.runId || - !/^[0-9a-f]{40}$/i.test(authorization.issue.headSha ?? '') - ) { - return false - } - const fields = request.fields.map(apiField) - const values = (name) => fields.filter((field) => field.name === name).map((field) => field.value) - const body = values('body') - const commitIds = values('commit_id') - const events = values('event') - const publication = parseReviewPublication(body[0], authorization) - if ( - body.length !== 1 || - commitIds.length !== 1 || - events.length !== 1 || - commitIds[0] !== authorization.issue.headSha || - events[0] !== 'COMMENT' || - !publication - ) { - return false - } - const permittedNames = new Set([ - 'body', - 'commit_id', - 'event', - 'comments[][path]', - 'comments[][line]', - 'comments[][side]', - 'comments[][body]', - ]) - if (fields.some((field) => !permittedNames.has(field.name))) return false - const paths = values('comments[][path]') - const lines = values('comments[][line]') - const sides = values('comments[][side]') - const comments = values('comments[][body]') - if ( - paths.length < 1 || - paths.length > 50 || - lines.length !== paths.length || - sides.length !== paths.length || - comments.length !== paths.length - ) { - return false - } - return paths.every( - (filePath, index) => - filePath.length > 0 && - !path.isAbsolute(filePath) && - !filePath.split(/[\\/]/).includes('..') && - /^[1-9][0-9]*$/.test(lines[index]) && - ['LEFT', 'RIGHT'].includes(sides[index]) && - comments[index].includes( - `/gi, + ), + ] + if ( + matches.length !== 1 || + matches[0][1] !== authorization?.issue?.runId || + matches[0][4].toLowerCase() !== authorization?.issue?.headSha?.toLowerCase() + ) { + throw new Error('review body must contain one exact run, cycle, round, and head marker') + } + return { + cycle: Number(matches[0][2]), + round: Number(matches[0][3]), + } +} + +function parseComment(source, { authorization, cycle, round }) { + let comment + try { + comment = JSON.parse(source) + } catch { + throw new Error('each --comment must contain one JSON object') + } + const keys = Object.keys(comment).sort() + if ( + JSON.stringify(keys) !== JSON.stringify(['body', 'line', 'path', 'side']) || + typeof comment.path !== 'string' || + comment.path.length === 0 || + path.posix.isAbsolute(comment.path) || + comment.path.includes('\\') || + comment.path.split('/').includes('..') || + !Number.isInteger(comment.line) || + comment.line < 1 || + !['LEFT', 'RIGHT'].includes(comment.side) || + typeof comment.body !== 'string' || + !comment.body.includes( + ``, + ), + )?.[1] ?? null, + ) + if (findingIds.some((findingId) => !findingId) || new Set(findingIds).size !== findingIds.length) { + throw new Error('trusted review publisher requires unique stable finding markers') + } + return { + endpoint: `repos/${authorization.expectedRepository}/pulls/${prNumber}/reviews`, + payload: { + commit_id: headSha, + event: 'COMMENT', + body, + comments, + }, + publication: { cycle, round, headSha }, + } +} + +export function reviewPublisherSyntheticGitHubArguments(request) { + const args = [ + 'api', + request.endpoint, + '--method', + 'POST', + '-f', + `commit_id=${request.payload.commit_id}`, + '-f', + 'event=COMMENT', + '-f', + `body=${request.payload.body}`, + ] + for (const comment of request.payload.comments) { + args.push( + '-F', + `comments[][path]=${comment.path}`, + '-F', + `comments[][line]=${comment.line}`, + '-F', + `comments[][side]=${comment.side}`, + '-f', + `comments[][body]=${comment.body}`, + ) + } + return args +} diff --git a/loops/issue-dev-loop/scripts/lib/run-store.mjs b/loops/issue-dev-loop/scripts/lib/run-store.mjs index 87050fb7..2a83bedc 100644 --- a/loops/issue-dev-loop/scripts/lib/run-store.mjs +++ b/loops/issue-dev-loop/scripts/lib/run-store.mjs @@ -283,10 +283,13 @@ const REQUIRED_BRIEF_SECTIONS = [ function briefSection(source, heading) { const escaped = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') - return ( - source.match(new RegExp(`## ${escaped}[ \\t]*\\r?\\n([\\s\\S]*?)(?=\\n## |$)`))?.[1]?.trim() ?? - '' - ) + const matches = [...source.matchAll(new RegExp(`^## ${escaped}[ \\t]*\\r?$`, 'gm'))] + const selected = matches.at(-1) + if (!selected || selected.index === undefined) return '' + const sectionStart = selected.index + selected[0].length + const remainder = source.slice(sectionStart).replace(/^\r?\n/, '') + const nextSection = remainder.search(/^## [^\r\n]+[ \t]*\r?$/m) + return (nextSection === -1 ? remainder : remainder.slice(0, nextSection)).trim() } function withoutHtmlComments(source) { @@ -314,23 +317,31 @@ function visibleMarkdownLines(source) { } function parseFrozenBrief(source) { + const contractMarker = '' + const markerIndex = source.lastIndexOf(contractMarker) + const contractSource = + markerIndex === -1 ? source : source.slice(markerIndex + contractMarker.length) const sections = Object.fromEntries( - REQUIRED_BRIEF_SECTIONS.map((heading) => [heading, briefSection(source, heading)]), + REQUIRED_BRIEF_SECTIONS.map((heading) => [heading, briefSection(contractSource, heading)]), ) for (const [heading, contents] of Object.entries(sections)) { if (!contents || contents.includes('`) && requiredVisibleMetadata.every((fragment) => visiblePullRequestBody.includes(fragment)) && @@ -1058,6 +1127,11 @@ export async function transitionRun({ !/\bpending\b/i.test(`${verificationSection}\n${evidenceSection}\n${reviewSection}`) && (!run.uiEvidenceRequired || (screenshotPaths.length > 0 && + embeddedScreenshots.before.size > 0 && + embeddedScreenshots.after.size > 0 && + [...embeddedScreenshots.before, ...embeddedScreenshots.after].every( + (screenshotPath) => screenshotPaths.includes(screenshotPath), + ) && screenshotPaths.every((screenshotPath) => visiblePullRequestBody.includes(screenshotPath), ))) diff --git a/loops/issue-dev-loop/scripts/lib/validation.mjs b/loops/issue-dev-loop/scripts/lib/validation.mjs index 588cb585..a4ad0d13 100644 --- a/loops/issue-dev-loop/scripts/lib/validation.mjs +++ b/loops/issue-dev-loop/scripts/lib/validation.mjs @@ -88,8 +88,10 @@ export async function validateLoop({ 'scripts/lib/active-journal.mjs', 'scripts/lib/github.mjs', 'scripts/lib/github-identity.mjs', + 'scripts/lib/review-publication.mjs', 'scripts/lib/trusted-control-plane.mjs', 'scripts/github-command-gate.mjs', + 'scripts/publish-review.mjs', 'scripts/identity-bin/gh', 'scripts/identity-bin/git', 'scripts/lib/issue-claim.mjs', @@ -223,6 +225,12 @@ export async function validateLoop({ (evidenceWorkflowSource.match(/docker run --rm --network none/g)?.length ?? 0) < 2 || !evidenceWorkflowSource.includes('src=${GITHUB_WORKSPACE}/trusted,dst=/source,readonly') || !evidenceWorkflowSource.includes('pnpm test') || + !evidenceWorkflowSource.includes( + 'git config --global --add safe.directory /work; pnpm verify', + ) || + !evidenceWorkflowSource.includes( + 'git config --global --add safe.directory /work; pnpm test', + ) || !evidenceWorkflowSource.includes( '--trusted-workflow-sha "${{ steps.run.outputs.base_sha }}"', ) || diff --git a/loops/issue-dev-loop/scripts/publish-review.mjs b/loops/issue-dev-loop/scripts/publish-review.mjs new file mode 100644 index 00000000..bb932ca9 --- /dev/null +++ b/loops/issue-dev-loop/scripts/publish-review.mjs @@ -0,0 +1,49 @@ +#!/usr/bin/env node + +import { spawn } from 'node:child_process' + +import { parseReviewPublisherArguments } from './lib/review-publication.mjs' +import { loadTrustedControlPlane } from './lib/trusted-control-plane.mjs' + +async function main() { + if (process.env.ECHO_UI_LOOP_GITHUB_ROLE !== 'reviewer') { + throw new Error('trusted review publisher requires the reviewer identity') + } + let authorization + try { + authorization = JSON.parse(process.env.ECHO_UI_LOOP_AUTHORIZATION) + } catch { + throw new Error('trusted review publisher has invalid authorization context') + } + const request = parseReviewPublisherArguments(process.argv.slice(2), { authorization }) + const trustedControlPlane = await loadTrustedControlPlane() + const child = spawn( + trustedControlPlane.executables.gh, + ['api', request.endpoint, '--method', 'POST', '--input', '-'], + { + env: process.env, + stdio: ['pipe', 'inherit', 'inherit'], + shell: false, + }, + ) + child.stdin.end(`${JSON.stringify(request.payload)}\n`) + await new Promise((resolve, reject) => { + child.once('error', reject) + child.once('exit', (code, signal) => { + if (signal) { + reject(new Error(`trusted review publisher terminated by signal ${signal}`)) + return + } + if (code !== 0) { + reject(new Error(`trusted review publisher exited with status ${code ?? 1}`)) + return + } + resolve() + }) + }) +} + +main().catch((error) => { + process.stderr.write(`${error.message}\n`) + process.exitCode = 1 +}) diff --git a/loops/issue-dev-loop/templates/implementation-brief.md b/loops/issue-dev-loop/templates/implementation-brief.md index 60589745..a2c159a2 100644 --- a/loops/issue-dev-loop/templates/implementation-brief.md +++ b/loops/issue-dev-loop/templates/implementation-brief.md @@ -12,6 +12,8 @@ {{ISSUE_BODY}} + + ## Acceptance criteria diff --git a/loops/issue-dev-loop/templates/pr-body.md b/loops/issue-dev-loop/templates/pr-body.md index 8eb039be..bf9beaff 100644 --- a/loops/issue-dev-loop/templates/pr-body.md +++ b/loops/issue-dev-loop/templates/pr-body.md @@ -19,6 +19,12 @@ Closes #{{ISSUE_NUMBER}} ## Evidence + + ## Independent review ## Known limitations diff --git a/loops/issue-dev-loop/tests/github-identity-routing.test.mjs b/loops/issue-dev-loop/tests/github-identity-routing.test.mjs index e6074e02..b4e646ed 100644 --- a/loops/issue-dev-loop/tests/github-identity-routing.test.mjs +++ b/loops/issue-dev-loop/tests/github-identity-routing.test.mjs @@ -462,6 +462,24 @@ if [ "$1 $2 $3 $4" != "api user --jq .login" ]; then echo "comment review published" exit 0 fi + if [ "$1" = "api" ] && [ "$2" = "repos/example/repo/pulls/106/reviews" ] && [ "$3 $4 $5" = "--method POST --input" ] && [ "$6" = "-" ]; then + ${JSON.stringify(process.execPath)} -e ' + let source = "" + process.stdin.setEncoding("utf8") + process.stdin.on("data", (chunk) => { source += chunk }) + process.stdin.on("end", () => { + const payload = JSON.parse(source) + process.stdout.write(JSON.stringify({ + id: 401, + state: "COMMENTED", + commit_id: payload.commit_id, + body: payload.body, + comments: payload.comments + })) + }) + ' + exit 0 + fi ${JSON.stringify(process.execPath)} -e 'process.stdout.write(JSON.stringify({args: process.argv.slice(1)}))' -- "$@" exit 0 fi @@ -511,6 +529,11 @@ if [ "$1 $2" = "diff --name-status" ]; then fi exit 0 fi +if [ -f ${JSON.stringify(path.join(parent, 'exercise-credential-helper'))} ]; then + helper=\${GIT_CONFIG_VALUE_1#\\!} + printf 'protocol=https\\nhost=github.com\\n\\n' | /bin/sh -c "$helper get" + exit $? +fi ${JSON.stringify(process.execPath)} -e 'process.stdout.write(JSON.stringify({args: process.argv.slice(1), config: process.env.GH_CONFIG_DIR, hasGhToken: Boolean(process.env.GH_TOKEN || process.env.GITHUB_TOKEN), gitConfig: Array.from({length: Number(process.env.GIT_CONFIG_COUNT)}, (_, index) => [process.env[\`GIT_CONFIG_KEY_\${index}\`], process.env[\`GIT_CONFIG_VALUE_\${index}\`]]).flat()}))' -- "$@" `, 'utf8', @@ -824,6 +847,31 @@ test('automation git command clears global helpers and injects the selected gh c ]) }) +test('selected GitHub credential helper forwards the Git credential operation', async () => { + const fixture = await createFixture() + await writeFile( + path.join(path.dirname(fixture.loopRoot), 'exercise-credential-helper'), + 'enabled\n', + 'utf8', + ) + const { stdout } = await execFileAsync( + process.execPath, + [ + routerPath, + '--loop-root', + fixture.loopRoot, + 'automation', + '--', + 'git', + 'push', + 'origin', + 'codex/issue-123', + ], + { env: fixture.env }, + ) + assert.deepEqual(JSON.parse(stdout).args, ['auth', 'git-credential', 'get']) +}) + test('automation push rejects dirty or unrecorded post-implement content', async () => { for (const marker of ['dirty-git', 'unsafe-trailing-diff']) { const fixture = await createFixture() @@ -1293,84 +1341,84 @@ test('reviewer adjudication COMMENT is exact-head, finding-bound, and non-cyclic await assert.rejects(publish(), /already published/) }) -test('reviewer may publish exact-head inline comments only as a COMMENT review API request', async () => { +test('reviewer cannot bypass the trusted publisher with a direct inline review API request', async () => { + const fixture = await createFixture() + await assert.rejects( + execFileAsync( + process.execPath, + [ + routerPath, + '--loop-root', + fixture.loopRoot, + 'reviewer', + '--', + 'gh', + 'api', + 'repos/example/repo/pulls/106/reviews', + '--method', + 'POST', + '-f', + `commit_id=${'b'.repeat(40)}`, + '-f', + 'event=COMMENT', + '-f', + `body=`, + '-F', + 'comments[][path]=src/fixture.ts', + '-F', + 'comments[][line]=10', + '-F', + 'comments[][side]=RIGHT', + '-f', + 'comments[][body]= Finding', + ], + { env: fixture.env }, + ), + /GitHub action is prohibited for the reviewer role/, + ) +}) + +test('reviewer may publish one exact-head COMMENT review through the trusted JSON publisher', async () => { const fixture = await createFixture() + const publisherPath = path.join(path.dirname(fixture.routerPath), 'publish-review.mjs') + const headSha = 'b'.repeat(40) + const marker = `` + const finding = { + path: 'src/fixture.ts', + line: 10, + side: 'RIGHT', + body: ' Finding', + } const { stdout } = await execFileAsync( - process.execPath, + routerLauncherPath, [ - routerPath, '--loop-root', fixture.loopRoot, 'reviewer', '--', - 'gh', - 'api', - 'repos/example/repo/pulls/106/reviews', - '--method', - 'POST', - '-f', - `commit_id=${'b'.repeat(40)}`, - '-f', - 'event=COMMENT', - '-f', - `body=`, - '-F', - 'comments[][path]=src/fixture.ts', - '-F', - 'comments[][line]=10', - '-F', - 'comments[][side]=RIGHT', - '-f', - 'comments[][body]= Finding', + 'node', + publisherPath, + '--loop-root', + fixture.loopRoot, + '--pr', + '106', + '--head', + headSha, + '--cycle', + '1', + '--round', + '1', + '--body', + marker, + '--comment', + JSON.stringify(finding), ], { env: fixture.env }, ) - assert.match(stdout, /pulls\/106\/reviews/) - - for (const unsafe of [ - ['-f', `commit_id=${'b'.repeat(40)}`, '-f', 'event=APPROVE', '-f', 'body=unsafe'], - ['-f', `commit_id=${'d'.repeat(40)}`, '-f', 'event=COMMENT', '-f', 'body=wrong head'], - ['--input', '/tmp/unvalidated-review.json'], - [ - '--hostname', - 'example.invalid', - '-f', - `commit_id=${'b'.repeat(40)}`, - '-f', - 'event=COMMENT', - '-f', - `body=`, - '-F', - 'comments[][path]=src/fixture.ts', - '-F', - 'comments[][line]=10', - '-F', - 'comments[][side]=RIGHT', - '-f', - 'comments[][body]= Finding', - ], - ]) { - await assert.rejects( - execFileAsync( - process.execPath, - [ - routerPath, - '--loop-root', - fixture.loopRoot, - 'reviewer', - '--', - 'gh', - 'api', - 'repos/example/repo/pulls/106/reviews', - '--method', - 'POST', - ...unsafe, - ], - { env: fixture.env }, - ), - /GitHub action is prohibited for the reviewer role/, - ) - } + const published = JSON.parse(stdout) + assert.equal(published.state, 'COMMENTED') + assert.equal(published.commit_id, headSha) + assert.deepEqual(published.comments, [finding]) }) test('automation PR creation is bound to the active branch, dev, and Draft', async () => { diff --git a/loops/issue-dev-loop/tests/runtime.test.mjs b/loops/issue-dev-loop/tests/runtime.test.mjs index 5b5acd23..48411324 100644 --- a/loops/issue-dev-loop/tests/runtime.test.mjs +++ b/loops/issue-dev-loop/tests/runtime.test.mjs @@ -459,7 +459,17 @@ async function publishFixtureCheckpoint({ loopRoot, runId }) { return { ...prepared, commentUrl } } -function pullRequestFixture(run, headSha, { draft = true, merged = false } = {}) { +function pullRequestFixture( + run, + headSha, + { draft = true, embeddedScreenshots = false, merged = false } = {}, +) { + const screenshotEvidence = embeddedScreenshots + ? [ + `![Before](https://raw.githubusercontent.com/codeacme17/echo-ui/${headSha}/loops/issue-dev-loop/screen-shots/${run.runId}/before/fixture.png)`, + `![After](https://raw.githubusercontent.com/codeacme17/echo-ui/${headSha}/loops/issue-dev-loop/screen-shots/${run.runId}/after/fixture.png)`, + ] + : [] return { state: merged ? 'closed' : 'open', draft, @@ -486,6 +496,7 @@ function pullRequestFixture(run, headSha, { draft = true, merged = false } = {}) '- `pnpm test (owner-merged baseline tests)`: passed (exit code 0)', '## Evidence', 'Exact-head workflow evidence is attached or pending for this draft.', + ...screenshotEvidence, '## Independent review', 'Fresh-context review is attached or pending for this draft.', '## Known limitations', @@ -501,6 +512,7 @@ async function recordFixturePr({ headSha, number = 200, uiEvidenceRequired = false, + embeddedScreenshots = uiEvidenceRequired, }) { const briefPath = path.join(loopRoot, 'handoffs', run.runId, 'implementation-brief.md') await publishFixtureCheckpoint({ loopRoot, runId: run.runId }) @@ -574,7 +586,7 @@ async function recordFixturePr({ githubApi: async (endpoint) => endpoint.includes('/compare/') ? { status: 'ahead', base_commit: { sha: implementationCommit } } - : pullRequestFixture(run, headSha), + : pullRequestFixture(run, headSha, { embeddedScreenshots }), trailingPathValidator: async () => {}, }) await publishFixtureCheckpoint({ loopRoot, runId: run.runId }) @@ -1436,6 +1448,114 @@ test('frozen brief rejects empty scope, TDD, checks, evidence, risk, and stop se ) }) +test('frozen brief rejects level-three headings that impersonate contract sections', async () => { + const { loopRoot } = await createFixture() + const { run } = await startFixtureRun({ + loopRoot, + issueNumber: 148, + issueTitle: 'Reject nested frozen contract', + issueUrl: 'https://github.com/codeacme17/echo-ui/issues/148', + entropy: 'brief148', + }) + await publishFixtureCheckpoint({ loopRoot, runId: run.runId }) + const briefPath = path.join(loopRoot, 'handoffs', run.runId, 'implementation-brief.md') + await writeFile( + briefPath, + [ + '# Implementation brief', + '', + '- UI evidence required: yes', + '', + '## Issue snapshot', + '', + ...[ + 'Acceptance criteria', + 'In scope', + 'Out of scope', + 'Pre-agreed TDD seams', + 'Required targeted checks', + 'Required UI evidence', + 'Risks and owner-confirmation boundaries', + 'Stop conditions', + ].flatMap((heading) => [ + `## ${heading}`, + heading === 'Required targeted checks' + ? '- `pnpm test -- issue-controlled-decoy`' + : 'Issue-controlled text that must not satisfy the frozen contract.', + '', + ]), + '', + '', + '### Acceptance criteria', + 'A deterministic acceptance criterion with sufficient detail.', + '', + '### In scope', + 'The requested behavior.', + '', + '### Out of scope', + 'Unrelated behavior.', + '', + '### Pre-agreed TDD seams', + 'The public control boundary.', + '', + '### Required targeted checks', + '- `pnpm test -- target`', + '- `pnpm verify`', + '', + '### Required UI evidence', + 'Paired screenshots.', + '', + '### Risks and owner-confirmation boundaries', + 'No authority changes.', + '', + '### Stop conditions', + 'Stop after committing.', + ].join('\n'), + 'utf8', + ) + await assert.rejects( + freezeBrief({ loopRoot, runId: run.runId }), + /concrete Acceptance criteria section/, + ) +}) + +test('UI draft PR requires embedded before and after screenshots pinned to its exact head', async () => { + const { loopRoot } = await createFixture() + const { run } = await startFixtureRun({ + loopRoot, + issueNumber: 149, + issueTitle: 'Show UI evidence in the PR', + issueUrl: 'https://github.com/codeacme17/echo-ui/issues/149', + entropy: 'images149', + }) + await assert.rejects( + recordFixturePr({ + loopRoot, + run, + headSha: 'e'.repeat(40), + number: 304, + uiEvidenceRequired: true, + embeddedScreenshots: false, + }), + /embedded before and after screenshots pinned to the exact PR head/, + ) +}) + +test('evidence workflow marks volume checkouts safe before Git-backed verification', async () => { + const workflow = await readFile( + path.resolve(repositoryLoopRoot, '..', '..', '.github', 'workflows', 'issue-dev-loop-evidence.yml'), + 'utf8', + ) + assert.match( + workflow, + /git config --global --add safe\.directory \/work; pnpm verify/, + ) + assert.match( + workflow, + /git config --global --add safe\.directory \/work; pnpm test/, + ) +}) + test('automation identity cannot overlap the repository owner', async () => { const { loopRoot, channelRoot } = await createFixture() const channelPath = path.join(channelRoot, 'channel.json') diff --git a/scripts/verify-docs-ui.mjs b/scripts/verify-docs-ui.mjs index b44f9ff0..8eef39bc 100644 --- a/scripts/verify-docs-ui.mjs +++ b/scripts/verify-docs-ui.mjs @@ -90,6 +90,7 @@ const readContract = (page) => inlineCode: measure('article main p code'), installPackage: measure('article main aside[aria-label="Installation"]'), islandMobileMenu: measure('.island-mobile-menu'), + legacyVerifierCount: document.querySelectorAll('[data-legacy-verifier]').length, logo: measure('.echo-docs-logo'), navLabels: [ ...document.querySelectorAll('.nextra-navbar > nav > .nextra-scrollbar > :is(a, button)'), @@ -175,7 +176,7 @@ const assertContentContract = (contract, colorScheme, lang = 'en') => { assert.equal(contract.demoSurface.justifyContent, 'center') assert.equal(contract.demoSurface.padding, '20px') } - assert.equal(contract.footer?.display, 'none') + assert.equal(contract.legacyVerifierCount, 0) assert.equal(contract.headerExternalLinks, 0) assert.ok(contract.headerThemeSwitch) assert.equal(contract.installPackage, null) @@ -187,7 +188,6 @@ const assertContentContract = (contract, colorScheme, lang = 'en') => { assert.equal(contract.sidebarActive?.backgroundColor, 'rgba(0, 0, 0, 0)') assert.equal(contract.sidebarActive?.color, 'rgb(253, 170, 4)') assert.equal(contract.sidebarActive?.fontWeight, '400') - assert.equal(contract.sidebarFooter?.display, 'none') assert.deepEqual( contract.sidebarSeparators, lang === 'zh' ? ['可控组件', '可视化', '容器'] : ['Controller', 'Visualization', 'Container'], diff --git a/tests/docs-cutover.test.ts b/tests/docs-cutover.test.ts index efbdb1bf..b4976c52 100644 --- a/tests/docs-cutover.test.ts +++ b/tests/docs-cutover.test.ts @@ -16,12 +16,14 @@ const readManifest = async (path: string) => describe('production documentation cutover', () => { it('uses Nextra as the only documentation toolchain in normal installs and builds', async () => { - const [manifest, exampleManifest, docsManifest, workspace, readme] = await Promise.all([ + const [manifest, exampleManifest, docsManifest, workspace, readme, docsUiVerifier] = + await Promise.all([ readManifest('package.json'), readManifest('example/package.json'), readManifest('docs/package.json'), readFile(resolve(repositoryRoot, 'pnpm-workspace.yaml'), 'utf8'), readFile(resolve(repositoryRoot, 'README.md'), 'utf8'), + readFile(resolve(repositoryRoot, 'scripts/verify-docs-ui.mjs'), 'utf8'), ]) const trackedIslandDocs = execFileSync('git', ['ls-files', 'docs/.island'], { cwd: repositoryRoot, @@ -68,6 +70,9 @@ describe('production documentation cutover', () => { expect(readme).toContain('pnpm test:docs') expect(readme).not.toMatch(/docs:nextra|IslandJS/i) expect(trackedIslandDocs).toBe('') + expect(docsUiVerifier).toContain("document.querySelectorAll('[data-legacy-verifier]').length") + expect(docsUiVerifier).not.toContain("contract.footer?.display, 'none'") + expect(docsUiVerifier).not.toContain("contract.sidebarFooter?.display, 'none'") expect( execFileSync('git', ['ls-files', 'scripts/verify-island-style-parity.mjs'], { From 62b9d60382c07490ac7804b8a57e2fd5469ea273 Mon Sep 17 00:00:00 2001 From: leyoonafr Date: Sat, 25 Jul 2026 20:40:06 +0800 Subject: [PATCH 2/2] chore: retrigger bootstrap CI after Actions incident