Skip to content

feat(pr-pipeline): run the project's app-control skill in pr-verify after the deterministic tests #8651

Closed
12 of 12 issues completed

Description

@kyle-sexton

Refs: #7580
Refs: #7555
Decision: #7555 (comment)
Blocked by: #8816 (adds the app-control plugin and /app-control:drive, whose report.json this step reads).

Agent Brief

Type: Feature
Summary: After the deterministic tests pass, the verifier runs the project's own app-control skill against the PR head and folds what it finds into its verdict. Stack-neutral: the project names its skill; Docker is used only if that skill uses it.

Problem: The verifier judges a PR from its diff and text; nothing drives the running app the way a user would. The line that asked for this ("agent browser checks run after the deterministic tests, with no write token while PR code runs") sat in Phase 9, which closed with it unbuilt.

Current behavior:

  • .github/workflows/pr-verify.yml:3-27: runs on workflow_run after pr-require-checks completes; "No pull request code runs here"; job tokens can only read (:66-68, :384-386).
  • No app-control plugin exists yet; Phase 13 creates /app-control:drive and the generated control-<app> skill shape.

Desired behavior:

  • A setting in docs/conventions/pr-pipeline.yaml names the project's app-control skill. Unset: the step is skipped and the verdict says no app check ran.
  • When set: a separate job, after the deterministic checks pass, checks out the PR head and runs /app-control:drive --unattended, which finds the project's control skill; the verdict job judges drive's report.json (drive itself never judges; D54). It holds no write token and no secret beyond the model credential the run needs, on a fresh GitHub-hosted runner, under ADR 0049's hardening.
  • Its findings go to the verdict job as data, framed as untrusted (docs/conventions/untrusted-content/README.md); a blocking finding makes the verdict FAIL.
  • Nothing in the workflow names a stack. Docker appears only if the project's skill starts it.

Key interfaces:

Done when:

  • The diff adds the setting, the new job and the verdict input, and the job's permissions: grant no write scope (read from the diff).
  • With the setting unset, the verdict notes that no app check ran (a workflow.test.mjs case). CI's test-node passes on the PR (a PR check; this run does not run it).
  • CI's check-plugins passes on the PR (a PR check; this run does not run it).
  • Security & Cost's pass on the final head is recorded on the PR (.github/workflows changes; owner rule 2026-10-20).

Out of scope:

Origin:

  • Verifier agents that run the app and click around like a user, then fix and repeat: hers (Lauren, stream ~53:10), adapted.
  • Docker app stack plus browser checks in CI: the owner's (2026-10-16); moved to Phase 13 and made stack-neutral: ours, confirmed by the owner 2026-10-20.

Review: security (runs PR code in CI).

Do not run with: other pr-verify.yml changes.

Size Large, Complexity High, Uncertainty Knowable.


Slice of #7580 (Phase 13), filed from the owner's 2026-10-20 disposition round.

Activity

  1. added
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    on Oct 20, 2026
  2. kyle-sexton commented on Oct 30, 2026

    @kyle-sexton
    MemberAuthor

    Phase text for S7 (the pr-verify capture job), from the evidence-verification plan (Brief: #11364 (comment)). Slices are sub-issues of this issue.

    Satisfies: AC1, AC2, AC5, AC9, AC10
    Size Large. Depends on: S2a (#11388), S4a (#11386), S4b (#11390), S6 (#11385), and owner answers to U1 and U2 (USER-RESERVED). Files: .github/workflows/pr-verify.yml, .github/scripts/pr-verify/prompt.md, workflow.test.mjs. Design: T8, sequence flow 2.

    • Probe step first (U1/U2 evidence): after setup-runner-isolation, a head-code step tries to read the runtime token and the credential from a hook; the job fails closed if either is readable.
    • Job capture: same if as .github/workflows/pr-verify.yml:73-77 (fork heads never run), gates, read-only token, .base plugins, --trusted-root .base, /testing:run-e2e unattended app=<merge.verify-capture.app> base=<base-sha>; uploads publishable files as pr-capture-<pr>-<sha> with retention-days; skip with note when the key is unset.
    • Gates first: the capture job runs the kill switch, resolver, trusted-trigger and agree steps as its first steps (check-intent's copies stay), so it captures nothing on a run that judges nothing (.github/workflows/pr-verify.yml:102-175). timeout-minutes: 30; the outer session runs through ci-workflows' run-claude-cli with its own role, a tool allowlist and the drive maxEntryPoints cap (S4b (feat(harness-ops): mask control-claude-code captures and launch changed plugins #11390)); a capture timeout leaves capture: blocked and never stops the judge.
    • Judge jobs: only check-intent gains needs: capture with !cancelled(); check-intent-subscription and check-intent-fallback keep their success chain (.github/workflows/pr-verify.yml:434-445) and download the same run's artifact by id. All three download into pr-verify-data/capture/; the prompt frames it untrusted and says a missing or blocked capture is never a FAIL reason, while a trusted-text manifest line Evidence loop: 3/3 cap reached, last NEEDS WORK is a FAIL (AC7's park path then runs).
    • Last step, after the probe passes on a real PR: set merge.verify-capture.app: claude-code and show one in-repo PR with a judged capture (AC1). Comment block at .github/workflows/pr-verify.yml:33-36 updated (reviewed change: jobs now take an artifact).
    • Relates to pr-merge: per-PR live smoke of changed skills before unattended merge (owner decision 16) #7529 (Claude Code UI leg of the per-PR smoke).
    • Sanity Check: node --test .github/scripts/pr-verify/workflow.test.mjs exits 0 with cases: capture job permissions contain no write; the three judge jobs carry identical capture-download steps; capture if includes the head-repository check; no job-level always() in pr-verify.yml; only check-intent needs: capture; a probe step exists, runs after setup-runner-isolation and before any head code, and has no continue-on-error; actionlint passes.

    Depends on

    🤖 Generated with Claude Code

    https://claude.ai/code/session_01UJUn7KoThXoYQoxfvdCVi7

  3. 1 remaining item

  4. kyle-sexton commented on Oct 30, 2026

    @kyle-sexton
    MemberAuthor

    Owner decision 2026-10-30, Q11: the capture job stays in pr-verify as its own job (no write token; only its run directory reaches the judge). No verdict-forgery probe and no artifact-name filter: the only actors are the owner's own agents, and the producer and reviewer are separate agents. Q10: it uses pr-verify's existing credential legs. Remaining dependencies: #11388, #11386, #11390, #11385. Brief and decisions: #11364

  5. kyle-sexton commented on Oct 31, 2026

    @kyle-sexton
    MemberAuthor

    Decisions 2026-10-31 (audit of Q10-Q12, resolved from the owner's standing direction s1, s5, s10):

    • The capture job sets CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 so hooks and Bash in the driven session get no credentials (https://code.claude.com/docs/en/env-vars). No separate OS user and no /proc defense: reading another process's environment on purpose needs a malicious actor, which the threat model excludes.
    • No process-group kill before upload (a guard against our own actors).
    • Evidence waits between capture and review in workflow artifacts with a retention period (as pr-verify's verdict does), never on a branch.
  6. added
    host-runNeeds an unattended run on a host; the host runner takes it, not a person or the issue lane.
    on Nov 1, 2026
  7. kyle-sexton commented on Nov 1, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded.

    The capture job is built as the 2026-10-30 S7 phase text on this issue describes. S7 replaces two parts of the body: the job runs /testing:run-e2e unattended app=<merge.verify-capture.app>, not /app-control:drive --unattended, and a missing or blocked capture is never a FAIL reason (the body said a blocking finding fails the verdict); the trusted manifest line Evidence loop: 3/3 cap reached, last NEEDS WORK is a FAIL. Q10 is the owner's 2026-10-30 answer ("we already have pr-verify", #11364 issuecomment-6536099521): the job uses pr-verify's existing credential legs. Q11 was decided on 2026-10-31 from the owner's standing direction, not in the owner's words (#11364 issuecomment-6547731014, which supersedes issuecomment-6547464663): the capture job may hold that credential, and the PR's code runs in a child process whose environment and readable files hold no token. The 2026-10-31 comment here adds CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1, no process-group kill before upload, and evidence in workflow artifacts with a retention period; it drops a separate OS user and a /proc defense, not S7's probe step, so the probe step stays. S6 (#11385) already added the merge.verify-capture key, with app unset recording capture: no-app. At origin/main b9ae6bbaed this edits the pr-verify workflow, which the lanes App cannot push (.github/actions/resolve-config/patch-rules.json:75-76). The body's Security & Cost criterion stays: no later decision on this issue removes it. Of the S7 dependencies, #11388 and #11385 are closed; #11386 and #11390 are open, so the build waits on them. Agents decided this on evidence under standing direction §1.

    Acceptance criteria:

    • feat(harness-ops): let control-claude-code run unattended on sessions it launches #11386 and feat(harness-ops): mask control-claude-code captures and launch changed plugins #11390 are closed by merged pull requests before the build starts.
    • With merge.verify-capture.app unset, the capture job is skipped and the verdict's evidence records capture: no-app (a .github/scripts/pr-verify/workflow.test.mjs case).
    • A probe step runs after setup-runner-isolation and before any head code, has no continue-on-error, and fails the job if a hook can read the runtime token or the credential.
    • The step that starts the app under test passes it no credential variable, and a test asserts that the app process's environment holds none of the pr-verify credential names.
    • The capture job's permissions grant no write scope, its if keeps fork heads out, and only check-intent gains needs: capture.
    • The capture job's environment sets CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1.
    • Capture evidence reaches the judge jobs only as a workflow artifact with a retention period, framed as untrusted. A missing or blocked capture is never a FAIL reason, and the trusted line Evidence loop: 3/3 cap reached, last NEEDS WORK is a FAIL.
    • node --test .github/scripts/pr-verify/workflow.test.mjs exits 0, and CI's test-node and check-plugins pass on the PR.
    • Security & Cost's pass on the final head is recorded on the PR (owner rule 2026-10-20, for a workflow change).

    Route: builder (Automation Lanes).

  8. kyle-sexton commented on Nov 6, 2026

    @kyle-sexton
    MemberAuthor

    Security & Cost pass on the capture, main 39a20d000f: PASS, conditional, with one MEDIUM finding being fixed. It was a fresh-context review of .github/workflows/pr-verify-capture.yml, .github/scripts/pr-verify/ and ci-workflows run-claude-cli/lane.cjs, against the threat model in standing direction §3 and §10 (the owner's decisions are recorded on #11364).

    # Severity Finding Disposition
    1 MEDIUM PR head code can run outside the sibling sandbox, as a Bash child of the outer leg that holds the real key. The capture prompt (pr-verify-capture.yml:925-935) sets no route limit on /testing:run-e2e, and three of its routes run head code: the non-UI route (run-e2e/SKILL.md:73), own driving (:95-96) and a committed suite (:166). Only the env scrub protects the key there. Separately, UNVERIFIED: head code could write a user-scope proxy or CA setting that the managed settings do not pin. No route to the key was found. Being fixed in this session: the prompt allows the drive route only and records any other route blocked, and the managed settings pin NODE_EXTRA_CA_CERTS and the proxy variables. No exposure today, because with the key out of credit every capture records blocked before head code runs.
    2 LOW After the drive, head code on that route could rewrite .base composite actions that run later, and so read the job's read-only GITHUB_TOKEN. lane.cjs:726-776 already binds _work/_actions read-only. If GitHub did not enforce cache-mode: read, this would become cache poisoning (HIGH). Closed by finding 1's fix and by #13710 (Bash narrowed). cache-mode: read enforcement is being checked against the GitHub docs.
    3 LOW Head code can forge evidence.capture, but nothing gates on it, and the judges read the capture as untrusted data. None now.

    Checked and clean:

    • Fork heads: never captured (:105, :1242, find-capture.mjs:70-76).
    • Key proxy: headers come from an allowlist, x-api-key is replaced, a per-job token cap applies, and the key never reaches its log.
    • Checkouts: .git is removed before the checkout post steps; persist-credentials: false.
    • Cost: standard runners on a public repository, retention of at most 30 days, and no larger-runner or Azure route.

    The box stays unticked until the finding-1 fix merges and cache-mode: read is confirmed.

  9. kyle-sexton commented on Nov 6, 2026

    @kyle-sexton
    MemberAuthor

    Done-when graded: all 13 boxes ticked (4 in the body, 9 in the 2026-11-01 decision), against main after PR 13770. Several rows are met by the design change the owner chose on #12232 (Q1): the capture runs in its own workflow, pr-verify-capture.yml, and pr-verify's judges download its artifact. #12232's closing table records the same mapping.

    Box Evidence
    Setting, job and verdict input; no write scope merge.verify-capture in docs/conventions/pr-pipeline.yaml; the capture workflow; the evidence block in write-verdict.mjs:86-158; test "the capture workflow and its jobs can write nothing"
    Unset app → no app check ran, plus a test pr-verify-capture.yml:176-179; workflow.test.mjs:1494; write-verdict.test.mjs:102 (no-app)
    test-node and check-plugins pass on the PR Passed on #13053, #13536 and #13712
    #11386 and #11390 closed by merged PRs #12257 and #12374
    Probe after isolation, before head code, fails closed "Check the probe" fails the job before the head checkout (:853-897). The probe step itself keeps continue-on-error so a credit-stop handoff ends green (owner fallback decision on #12232).
    The app's start step passes no credential, plus a test Started in the sibling sandbox under an env -i allowlist; sibling-init.sh probes for every pr-verify credential name and fails closed (#12232 row 2)
    No write permissions, no fork heads, only check-intent needs: capture Met by the design change: with capture in its own workflow, no job needs it. Fork heads: :105 and :1242, and test workflow.test.mjs:1352.
    CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 Set in the capture job's managed settings
    Artifact with retention, framed untrusted; blocked is never a FAIL; the 3/3 line is a FAIL pr-capture-* retention 1-30 days; prompt.md:85-87 and :128-131
    node --test workflow.test.mjs exits 0 76/76 at #12232 close, and green on every capture PR since
    Security & Cost pass on the final head Comment 6691237700. PR 13770 has since closed its MEDIUM finding with a tool rule: the driver legs hold only a client key, and their Bash is limited to the driver and read-only git (pr-verify-capture.yml:1123-1137). GitHub enforces cache-mode: read: "a job cannot restore or save caches beyond the mode it is granted" (workflow syntax).

    Still open: #11364, this issue's last open sub-issue. It waits on #13757's three PRs (13762, 13765, 13774) and on one live capture when PR_VERIFY_CLAUDE_API_KEY has credit. This issue closes after #11364 does (R233).

  10. removed
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    on Nov 7, 2026
  11. melodic-automation-lanes commented on Nov 7, 2026

    @melodic-automation-lanes
    Contributor

    Every sub-issue of this split is closed as completed by a listed human or a merge (#11364, #11384, #11385, #11386, #11387, #11388, #11389, #11390, #11391, #11392, #11393, #12232), so the lanes App closes it as completed.

  12. removed
    host-runNeeds an unattended run on a host; the host runner takes it, not a person or the issue lane.
    on Nov 7, 2026
  13. kyle-sexton commented on Nov 7, 2026

    @kyle-sexton
    MemberAuthor

    Closing record. All 13 done-when items are met: 4 in the body and 9 in issuecomment-6552343414. The S&C grade of 2026-11-06 found the pr-verify app-control run built.

    This issue stayed open only so its child, #11364, could close first. #11364 is now closed: a UI-change PR was captured locally, judged against its acceptance criteria by a fresh verifier, and its redacted evidence posted (#13958). #7580 closes next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Fields

    Work status

    confirmed

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions