ci(advisor): add credential-free repair stage for eligible findings
- Dominant language
- TypeScript
- Stars
- 22.5k
- Forks
- 3.1k
- Avg merge
- 1d 43m
- Merged PRs (30d)
- 718
Description
## Problem Statement
The PR Review Advisor can collect nine specialist reviews and identify concrete blockers, but resolving those blockers still requires a contributor or maintainer to interpret the combined feedback, edit the branch, and start another review cycle.
A repair agent must not receive GitHub write authority. The pull-request checkout, comments, changed files, and repository instructions are untrusted inputs; giving an agent a GitHub token, `gh`, or a general network path would let prompt-injected code cross directly into branch authority.
NemoClaw already has the pieces needed for a safer design:
- **PR Review Advisor** loads executable automation from the trusted workflow revision, binds analysis to immutable PR SHAs, separates GitHub context collection from model execution, disables PR-provided agent customization, removes worktree symlinks, and keeps the model-bearing jobs read-only.
- **PR merge-conflict fixer** lets Pi edit a credential-free OpenShell checkout, exports a patch, reconstructs the result in trusted code, requires a GitHub-verified commit, and updates the branch atomically with `beforeOid` and `force: false`.
- **Post-merge docs authoring** passes an approved patch by same-run artifact ID, binds approval to the exact patch SHA-256 and source commit, limits paths, modes, file count, and bytes, validates that tests do not mutate the candidate, and stops automation when maintainers take ownership.
Review-finding repair is broader than conflict resolution and acts on a contributor-owned branch. It therefore needs an explicit repairability and ownership gate, a schema-bound candidate receipt, stronger candidate validation, and an exact-new-head CI contract.
This issue complements #9774. That issue owns stable finding identity, convergence, and the human handoff for repeated no-progress roots; it intentionally does not edit contributor branches.
## Desired Behavior
After the Advisor completes for an eligible same-repository PR head, a trusted workflow should be able to produce one bounded repair attempt:
1. Bind the attempt to the exact reviewed PR head SHA, base SHA, Advisor run ID and attempt, trusted workflow SHA, and open finding IDs.
2. Select only findings that policy classifies as safe for automated source, test, or documentation repair.
3. Confirm that the PR is open, non-draft, targets `main`, is same-repository, has explicitly opted in, and still permits the intended maintainer automation.
4. Give Pi the complete specialist artifacts, current PR context, accepted product scope, and exact checkout in an offline OpenShell sandbox.
5. Let Pi edit only the disposable checkout and return a patch plus a structured proposal receipt. Pi must not commit, push, call GitHub, run tests, or make product decisions.
6. Recreate and validate the candidate in a trusted job that has no repository write permission, model credential, or other secret.
7. Upload only the validated patch and trusted validation receipt as a short-lived same-run artifact, addressed by the artifact ID returned by the validator.
8. If every candidate gate passes and the live Git and ownership state is unchanged, let a small deterministic publisher create one GitHub-verified commit and atomically advance the PR branch without force.
9. Validate the resulting commit with the repository's real required gates without starting another repair attempt for the bot-created head.
If selection, resolution, validation, or publication fails, the PR branch must remain unchanged. The workflow should retain a bounded, redacted diagnostic receipt for a maintainer.
## Workflow and Privilege Domains
Use separate jobs with default `permissions: {}` and the smallest job-level permissions:
1. **Select and collect** — read-only GitHub access. Load trusted code at `github.workflow_sha`, verify the live PR, collect the canonical Advisor ledger and all nine specialist artifacts, make the deterministic repairability decision, and emit an immutable context bundle.
2. **Repair** — artifact-read permission only. Fetch the exact PR data without executing its actions, hooks, submodules, LFS filters, package scripts, or agent configuration. Configure inference on the trusted host, then run Pi inside credential-free OpenShell. Emit an untrusted patch and proposal receipt.
3. **Validate** — read-only, secret-free, disposable job. Reconstruct the, apply the patch, enforce path and object policy, run trusted test selection and security checks, verify tests did not mutate the candidate unexpectedly, and emit a trusted validation receipt plus patch.
4. **Publish** — the only job with `contents: write`. Download the validated artifact by same-run artifact ID, revalidate its structure and hashes, recreate the exact candidate tree, recheck the live PR and ownership state, create the verified commit through the Git Database API, and atomically update the ref.
5. **Validate generated head** — run actual required validation for the new commit SHA through the maintainer-approved dispatch or equivalent mechanism. It must not invoke the repair stage again.
No job that has a model credential may have repository write permission. No job that executes PR-controlled code may have a model credential, repository write permission, checkout credential, or other secret. The publisher must never receive the Pi sandbox or execute PR-controlled commands.
## V1 Trust and Authority Contract
### Advisor and selection
Trusted code records:
- repository, PR number, author, head repository, head ref, reviewed head SHA, and base SHA;
- Advisor workflow SHA, run ID, run attempt, and immutable specialist artifact identities;
- the canonical open-finding ledger and current review-thread state;
- the accepted issue or maintainer decision that establishes product scope;
- the opt-in and branch-ownership state;
- a deterministic repairability decision and reason for every open finding.
V1 must skip drafts, forks, PRs without explicit opt-in, PRs whose branch ownership cannot be safely established, and findings that require product direction, maintainer judgment, author attestation, DCO or commit-verification repair, dependency or credential access, external-system mutation, or ambiguous intent. A security finding is not automatically repairable merely because it includes a suggested fix.
The attempt is one-shot. Pi does not decide which findings are in scope. Exactly two Pi turns run inside the same disposable sandbox: the first edits the selected paths, and the second reviews and finalizes that candidate. A failed or incomplete final proposal does not start a third turn or another attempt. The turns share only ephemeral in-attempt context; no model memory or transcript crosses attempts.
### Trusted checkout preparation
Follow the Advisor boundary rather than using a normal PR checkout in a privileged job:
- execute workflow helpers only from `github.workflow_sha`;
- bind base and head to full immutable SHAs;
- use `persist-credentials: false`, with LFS and submodules disabled;
- fetch PR content only as data and never execute PR-provided actions, package lifecycle scripts, hooks, filters, skills, extensions, prompts, or configuration;
- remove or reject symlinks before uploading the workspace to OpenShell;
- pin actions, runtime packages, OpenShell, and the Pi image by reviewed version or digest.
### Credential-free OpenShell resolver
Run one pinned two-turn Pi conversation in a pinned OpenShell sandbox image. The sandbox receives four immutable inputs: trusted workflow code, the exact checkout, the Advisor/context bundle, and the repair instruction. Pi receives no PR, branch, commit, workflow, run, attempt, artifact, or receipt identity. Only a disposable working tree and output directory are writable, and the conversation is destroyed with the sandbox.
Pi may use repository read, search, and edit/write tools. Unlike the merge-conflict fixer, V1 must not expose Pi's `bash` or any general process-execution tool; trusted host code performs Git operations and patch export. Pi does not run tests. The sandbox has no GitHub token, upstream model credential, `gh` authentication, package-registry credential, or external egress; model access is provided only through `inference.local`. PR-provided skills, extensions, prompts, hooks, context files, sessions, and agent configuration are disabled.
The resolver emits:
- one bounded binary patch against the recorded head;
- the finding IDs it attempted to address;
- changed paths and a concise change summary;
- any finding it could not safely address;
- no commit, ref update, test claim, or GitHub write.
The resolver output is untrusted. Its receipt does not authorize publication.
### Artifact and receipt contract
Use two artifact boundaries:
- The repair job uploads the untrusted patch and proposal receipt for the validator.
- The validator uploads a new artifact containing the validated patch and a trusted validation receipt. The publisher downloads this artifact by the upload step's artifact ID, with no repository or run override.
Artifacts must be regular non-symlink files in a bounded directory, enforce maximum file and byte counts, use `if-no-files-found: error`, and have short retention. The trusted validation receipt must use a committed schema and include at least:
- schema version, repository, PR number, head ref, source head SHA, and base SHA;
- Advisor workflow SHA, run ID, run attempt, and finding IDs;
- patch SHA-256, candidate tree SHA, changed paths, object types, and modes;
- selected validation commands, exit results, and a digest proving validation did not mutate the approved candidate;
- product-scope and opt-in decision identity;
- final outcome and deterministic skip/failure reason.
The publisher must recompute the patch hash and candidate tree and reject any mismatch. Artifact names, summaries, or model-authored claims are not authority.
### Trusted candidate validation
A separate job with no write permission or secrets must reconstruct the recorded head in a clean checkout and apply the patch itself. It must reject at least:
- malformed, oversized, empty-without-explanation, or non-applicable patches;
- excessive path or byte counts;
- binary, symlink, submodule, mode, rename, or unsafe-path changes outside the approved V1 class;
- changes to `.github/**`, `.agents/**`, `CODEOWNERS`, repository-control files, ownership files, security policy, agent instructions, or other denied surfaces unless maintainers explicitly approve a narrower exception;
- secrets, credential material, generated artifacts, or unexplained dependency/lockfile changes;
- a patch whose changed paths or finding receipt disagree with the reconstructed tree;
- an outdated head, base, Advisor run, finding ledger, product-scope decision, opt-in, or ownership state;
- formatting, lint, type, architecture, targeted-test, security, or risk-plan failures selected by trusted repository code.
Validation runs outside Pi's sandbox. The validator, not Pi, chooses and executes tests. Because tests execute PR-controlled code, the validation job must be disposable, use `persist-credentials: false`, receive no secrets or model credential, and expose no repository write authority. It must hash the candidate before and after validation and fail if validation changes the approved tree unexpectedly.
### Deterministic publication
Only the publisher receives `contents: write`. It must not execute PR code or use model output as commands. It reconstructs the candidate from the recorded head and validated patch, requires the same patch SHA-256 and candidate tree SHA, and rechecks the live PR/base/finding/opt-in/ownership state at the write boundary.
The publisher creates a one-parent commit through the Git Database API with the recorded PR head as its parent. It omits custom signature data and requires GitHub to report `verification.verified` before updating the ref. The atomic ref update must use:
- `beforeOid`: the recorded reviewed head SHA;
- `afterOid`: the verified repair commit SHA;
- `force`: `false`.
The sandbox never receives this token. A human push, changed base, changed metadata/ownership state, rejected verification, tree mismatch, or failed atomic update leaves the branch unchanged. The workflow must not claim ownership of the contributor branch beyond this single compare-and-swap update.
## CI and Loop Prevention
Using the repository `GITHUB_TOKEN` for publication prevents a bot push from recursively starting ordinary `push` or `pull_request` workflows. That is useful for loop prevention, but it must not become a required-check bypass: existing results belong to the old head, while merge authority must evaluate the new repair commit.
Before implementation, maintainers must choose and test the exact-new-head validation mechanism. The expected contract is:
- one repair attempt for the tuple of reviewed head SHA, Advisor run/attempt, and stable finding root;
- no Advisor-repair trigger for the generated commit, even under manual reruns or later event delivery;
- an explicit validation-only dispatch or equivalent trusted mechanism for the generated commit SHA;
- no synthetic success under the names of checks that did not actually run;
- all normal merge, DCO, GitHub verification, CodeQL, automated-review, and required-CI policy remains in force;
- a later human-authored head may become eligible only after a fresh Advisor run and a new opt-in decision.
Reuse the docs workflow's structural loop prevention where possible: a bounded ownership/attempt state and triggers that exclude the automation's own output. Do not rely only on `GITHUB_TOKEN` event suppression.
The workflow may report the repair result in the existing Advisor surface or a bounded workflow artifact, but it must not create an independent unbounded comment loop.
## Acceptance Criteria
- [ ] A non-draft, explicitly opted-in, same-repository PR with an eligible Advisor blocker produces at most one repair attempt for the recorded head/run/root.
- [ ] Product-scope, maintainer-decision, DCO, commit-verification, credential, external-mutation, ambiguous, and unsupported-path findings are skipped with a deterministic reason.
- [ ] Every job declares minimal permissions, and a workflow-boundary test fails if model credentials and repository write authority can coexist.
- [ ] Trusted executable code comes only from the pinned workflow revision; PR content is fetched only as data with credentials, LFS, submodules, hooks, lifecycle scripts, and agent customization disabled.
- [ ] The OpenShell sandbox can edit its disposable checkout but cannot read a GitHub or upstream model credential, reach an external endpoint, run a general shell/process tool, run tests, commit, or push.
- [ ] The resolver emits only a bounded patch and structured untrusted proposal receipt.
- [ ] Trusted validation independently reconstructs the candidate and rejects unsupported paths/types, secrets, stale state, malformed output, failed checks, or validation-time mutation.
- [ ] The publisher downloads only the validator's same-run artifact ID and validates regular-file, size, schema, source-SHA, finding-set, patch-hash, and candidate-tree contracts.
- [ ] The validated candidate tree is identical to the tree published by the trusted job.
- [ ] The repair commit is GitHub-verified, has exactly the recorded PR head as its parent, and contains no custom signature material.
- [ ] A changed PR head, base, finding ledger, product-scope decision, opt-in, or ownership state prevents publication.
- [ ] The ref update uses `beforeOid`, `force: false`, and never force-pushes.
- [ ] The generated head receives actual exact-SHA validation and cannot inherit or fabricate required-check success from the prior head.
- [ ] The generated head does not start another automated repair attempt; #9774's convergence and human-handoff contract remains authoritative.
- [ ] Fork PRs and drafts are skipped before sandbox creation and model inference in V1.
- [ ] Every success, failure, and cancellation path deletes the sandbox and preserves only a bounded, redacted diagnostic receipt.
- [ ] Tests cover malicious PR instructions, artifact symlinks/path escapes, patch/path/type/size limits, stale-head/base/finding/ownership races, candidate-tree and patch-hash mismatch, verification failure, atomic-update failure, validation mutation, exact-SHA CI dispatch, and loop prevention.
- [ ] A staging-repository test proves the verified commit, compare-and-swap ref update, exact-new-head checks, and no-repair-loop behavior end to end.
## Constraints and Non-goals
- V1 is maintainer automation, not a supported contributor-facing repair service.
- V1 does not give Pi repository write access or any GitHub credential.
- V1 does not let Pi run tests, arbitrary shell commands, network requests, commits, or pushes.
- V1 does not fix fork PRs, drafts, merge conflicts, product-scope decisions, contributor attestations, or external-system failures.
- V1 does not approve or merge a PR, waive a required gate, or treat Advisor or repair output as merge authority.
- V1 does not introduce a general iterative or cross-attempt memory loop; one bounded two-turn attempt either validates and publishes or stops.
- V1 does not require a second model reviewer. The existing Advisor and exact-new-head validation remain authoritative; a future independent repair-review phase requires its own product decision.
- V1 reuses the existing Advisor context and finding identity plus conflict-fixer and post-merge-docs primitives rather than creating a second review ledger, artifact service, or branch-update mechanism.
## Product Decisions Required Before Implementation
- Workflow owner and lifecycle.
- Approved model/provider and pinned Pi/OpenShell artifacts.
- Exact opt-in and ownership signal, including whether `maintainer_can_modify` is required.
- Exact finding classes and path/file types eligible for V1 repair.
- Patch, path-count, runtime, artifact-retention, and model-cost limits.
- The exact-new-head CI/required-check mechanism after `GITHUB_TOKEN` publication.
- Whether success/failure is reported only in workflow artifacts or also in the Advisor PR comment.
- Retention and redaction policy for patches, prompts, specialist artifacts, and repair receipts.
## Related Work
- #7542 and #7556 — credential-free Pi conflict resolution with deterministic GitHub publication.
- #7600 — PR Review Advisor's trusted-workflow and OpenShell credential boundary.
- #9234 and `.github/workflows/post-merge-docs.yaml` — exact-patch review receipts, same-run artifact publication, bounded paths, and automation-to-maintainer ownership transfer.
- #9774 — stable finding identity, convergence, and human handoff for unproductive review-and-fix loops.
## Checklist
- [x] I searched existing issues and this is not a duplicate.
- [x] I described the problem and desired behavior.
Category: Feature
## Repository-Wide Workflow Audit (2026-09-01)
This design was checked against every current non-E2E workflow on `origin/main` at `2b7ae7fed4dd1e0aa0363ef488e643c3c9d424c3`, with deeper inspection of the Advisor, conflict fixer, post-merge docs, PR CI, governance, package-verification, release reconciliation, hosted-runner recovery, documentation publication, and image-attestation paths. E2E files and E2E runs were intentionally left untouched during this audit.
### Additional reusable patterns
- **Treat the trigger as a trust boundary.** If repair begins from `workflow_run`, accept only the exact PR Review Advisor workflow identity and path, the NemoClaw repository, the expected upstream event, a successful completed run, and the intended run attempt. Fetch the upstream run and PR again through trusted code and reject mismatched repository, workflow, event, head SHA, base SHA, run ID, attempt, or artifact set before model setup. The hosted-runner recovery workflow demonstrates why a privileged downstream workflow must validate every source-run field rather than trust the event name alone.
- **Use the correct trusted-code identity for each event.** `pull_request_target` and `workflow_run` controllers execute helpers from `github.workflow_sha` or the recorded base SHA. Push-based controllers use the exact triggering `github.sha`. Never execute a helper from the PR head or a mutable branch name in a secret-bearing, model-bearing, or write-capable job.
- **Cancel stale thinking, never cancel an in-flight write.** Discovery, Advisor analysis, and repair preparation may use per-PR concurrency with `cancel-in-progress: true`. Validation and publication must be non-cancelable once promotion begins and must still use compare-and-swap at the write boundary. A newer event may make the old attempt fail stale, but it must not interrupt a writer between object creation and ref reconciliation.
- **Make promotion content-addressed.** Follow the image-attestation pattern: the untrusted repair produces a candidate, trusted validation produces the authoritative candidate-tree and patch digests, and the publisher receives only those validated identities. Validate the caller identity and every typed input before granting the publishing job write authority. Job outputs, artifact names, summaries, and model receipts are routing hints, not authority.
- **Use artifact IDs, not artifact-name discovery.** Pass the upload step's same-run artifact ID directly to the next trusted phase. Bind the receipt to `run_id`, `run_attempt`, workflow SHA, source head, base, finding ledger, patch digest, and candidate tree. Reject repository overrides, run overrides, multiple matches, expired artifacts, unexpected files, symlinks, non-regular files, and files that change while being read.
- **Make Git hermetic in trusted reconstruction.** Clone without hardlinks; detach at the exact SHA; set `GIT_CONFIG_GLOBAL=/dev/null`, `GIT_CONFIG_NOSYSTEM=1`, `GIT_LFS_SKIP_SMUDGE=1`, `GIT_TERMINAL_PROMPT=0`; disable hooks and LFS filters explicitly; and never allow user/system Git configuration to influence patch application, tree construction, or validation.
- **Keep privileged checkouts sparse.** A job that needs a package credential, model credential, or write token should check out only the base-controlled helpers and policy files it needs, with credentials, LFS, and submodules disabled. The OpenShell SDK packaging workflow is the model for downloading or verifying a privileged dependency without executing PR content.
- **Separate evaluation from evidence publication.** Specialist jobs use `fail-fast: false` so every Advisor opinion is collected, but selection must require the complete expected artifact set. A repair must not begin from a partial Advisor result. Likewise, a missing or rejected candidate artifact is a clean no-op: explain the failure and leave the branch unchanged.
- **Preserve independent governance gates.** DCO, commit verification, CodeQL, CodeRabbit, required CI, product scope, and maintainer decisions remain separate authorities. Repair may consume their findings but may not rewrite their status, manufacture a check result, or classify an old-head result as valid for the generated commit.
- **Attach real validation to the generated SHA.** Current PR CI is triggered by `pull_request`; a branch update made with the repository `GITHUB_TOKEN` will not safely provide the ordinary new-head lifecycle by itself. V1 therefore needs a dedicated validation-only entry point that accepts and revalidates one full commit SHA, runs the trusted required-check graph, and publishes genuine check runs attached to that SHA. The aggregate `checks` result must represent its actual dependencies, not a comment, artifact receipt, or synthetic success.
- **Wait for GitHub's verification state.** Commit or tag verification can be eventually consistent. After creating the repair commit, perform a small bounded re-read loop and require `verification.verified == true` before the atomic ref update. Timeout or an unexpected verification reason leaves the PR ref unchanged.
- **Make ownership head-scoped.** The post-merge docs workflow stops when a draft becomes ready for review; the equivalent here is that opt-in applies only to the recorded PR head and finding ledger. Any human push, base change, draft transition, opt-out, finding-set change, or maintainer takeover invalidates the attempt. Automation never assumes continuing ownership of a contributor branch.
- **Reconcile deterministically without rerunning the model.** Scheduled or manually dispatched reconciliation may repair a dropped event, verify whether a validated commit was published, or emit a bounded diagnostic. It must not start another Pi turn for the same head/run/finding root. The release-target workflows demonstrate the combination of non-canceling serialization, idempotent re-read, and later reconciliation.
- **Keep cleanup fail-safe and diagnostics bounded.** Sandbox deletion runs on every outcome. Cleanup failure must not replace the primary repair failure. Persist only length-bounded, control-character-sanitized diagnostics and short-lived artifacts; never retain tokens, full environment dumps, or uncontrolled model transcripts.
- **Pin every executable dependency.** Pin actions by commit SHA, Pi and OpenShell images by digest, runtime packages by reviewed version, and install from trusted lockfiles with lifecycle scripts disabled. Any pin update goes through normal dependency review.
- **Use protected environments only for external publication.** Workflows that publish outside GitHub use named environments and keep the external credential in the final publication step. The repair publisher should not receive an upstream model key or external publishing secret; if maintainers want a rollout approval, a protected environment can gate the deterministic publisher without moving Pi into that privilege domain.
### Additional implementation requirements from the audit
- [ ] The repair trigger validates the exact upstream Advisor workflow identity, event, conclusion, repository, PR, run ID, run attempt, workflow SHA, head SHA, base SHA, and complete nine-artifact set before sandbox creation.
- [ ] Analysis/repair concurrency cancels stale work per PR, while validation/publication is serialized, non-cancelable, and still protected by final live-state compare-and-swap.
- [ ] All privileged jobs use sparse trusted checkouts and hermetic Git settings; workflow tests reject mutable trusted refs, enabled checkout credentials, hooks, LFS filters, submodules, hardlinks, and user/system Git configuration.
- [ ] The publisher consumes the validator's exact same-run artifact ID and typed receipt, not an artifact-name search or model-authored output.
- [ ] GitHub verification is re-read with a bounded retry before ref publication; a timeout or non-verified result leaves the branch unchanged.
- [ ] A dedicated validation-only workflow attaches real required checks to the exact generated commit without invoking Advisor repair again.
- [ ] Reconciliation can recover dropped deterministic state but cannot invoke Pi twice for the same reviewed head, Advisor attempt, and stable finding root.
- [ ] Any human or metadata ownership transition invalidates the head-scoped opt-in and prevents publication.
- [ ] Missing, partial, rejected, stale, or ambiguous artifacts produce a bounded diagnostic and no branch mutation.
- [ ] Workflow-boundary tests prove that model credentials, PR-controlled execution, repository write authority, and external publication credentials never coexist in one job.
Contributor guide
Assessment
This issue has not been assessed yet.