Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/skills/verify-clerk-android/references/remote.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Each checkout has a random id, and its sessions carry it. When the checkout hold

`attach` always tries `gh pr edit --attach` itself first, on every kind of machine, and hands off only when that cannot work. The hand-off sends the evidence of a remote run to the session's runner. It sends the files that the flags of `attach` chose from the sealed run directory, with a manifest that names the run, the pull request, the device, the commit, and each file's size and SHA-256. With `--before`, the manifest also names the before run, its commit, and its counts, and the hand-off carries one file of the before run for each file of the run. Those files come from the before run's directory in this checkout, whichever session made that run. When that directory is gone, as it is in a new sandbox, `attach` fails with `USAGE` before it sends anything, and its fix is to attach without `--before` and name the before run in the PR. The files travel over the session's tunnel, with the session's token, in pieces of 1 MiB. The session job keeps them in one directory and uploads that directory as the artifact `verify-evidence` when the job ends. The artifact holds nothing else of the session.

`.github/workflows/verify-attach.yml` then runs, from the default branch. It downloads the artifact, checks every file against the manifest and against its own limits, and puts the block in the description with `gh pr edit --attach`. It holds a file of the before run to the same checks as a file of the run. It writes the description twice, as `attach` does: the first write uploads the files as a list, and the second lays them out in the tables that `SKILL.md` describes under Evidence. For the second write it reads the addresses of the uploads from the block, and takes only addresses of GitHub uploads. If the pull request or the block changed between the two writes, it leaves the list and says so in its run. A rerun of that job uploads the files again and lays them out. The tables take about twice the characters of the list, so in a description near GitHub's limit of 65,536 characters the second write can fail after the first one succeeded. The job then fails, and the list stays until the description is shorter and the job runs again. It never runs anything from the branch or from the artifact. The workflow did not run the tests, so the line it writes says that the session reported the result, links the session's run in the Actions tab, and names the account that started that run, as GitHub records it. With a before run, the line says that the session reports that run, and it links the commit of that run so that a reader can open it.
`.github/workflows/verify-attach.yml` then runs, from the default branch. It downloads the artifact, checks every file against the manifest and against its own limits, and puts the block in the description with `gh issue edit --attach`, which, unlike `gh pr edit`, does not read the review requests of the pull request. A team that is requested as a reviewer, as CODEOWNERS requests one, is a review request that the evidence token cannot read, so `gh pr edit` would fail there. It holds a file of the before run to the same checks as a file of the run. It writes the description twice, as `attach` does: the first write uploads the files as a list, and the second lays them out in the tables that `SKILL.md` describes under Evidence. For the second write it reads the addresses of the uploads from the block, and takes only addresses of GitHub uploads. If the pull request or the block changed between the two writes, it leaves the list and says so in its run. A rerun of that job uploads the files again and lays them out. The tables take about twice the characters of the list, so in a description near GitHub's limit of 65,536 characters the second write can fail after the first one succeeded. The job then fails, and the list stays until the description is shorter and the job runs again. It never runs anything from the branch or from the artifact. The workflow did not run the tests, so the line it writes says that the session reported the result, links the session's run in the Actions tab, and names the account that started that run, as GitHub records it. With a before run, the line says that the session reports that run, and it links the commit of that run so that a reader can open it.

The repository needs this set up once:

Expand Down
8 changes: 4 additions & 4 deletions .github/scripts/verify-attach.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ function mediaTables(files, hasBefore, uploaded) {
);
}

// `gh pr edit --attach` rewrites only a markdown reference to a file, so the files go up as a list first, and the tables are written from the addresses GitHub gave them.
// `gh issue edit --attach` rewrites only a markdown reference to a file, so the files go up as a list first, and the tables are written from the addresses GitHub gave them.
function evidenceBlock(manifest, session, onDisk = file => file.name, uploaded = null) {
const { start, end } = evidenceMarkers(manifest.platform);
const { before } = manifest;
Expand Down Expand Up @@ -881,15 +881,15 @@ function publishStep({

const bodyFile = path.join(work, 'body.md');
fs.writeFileSync(bodyFile, next.body, { mode: 0o600 });
const editBody = ['pr', 'edit', String(manifest.pr), '--repo', repo, '--body-file', bodyFile];
const editBody = ['issue', 'edit', String(manifest.pr), '--repo', repo, '--body-file', bodyFile];
const args = [...editBody, ...next.files.flatMap(file => ['--attach', `./${onDisk(file)}`])];
if (dryRun) {
say('plain', `would run, in ${filesDir}:\ngh ${args.map(quoted).join(' ')}`);
return 'dry run';
}
const edited = gh(args, { cwd: filesDir });
if (edited.status !== 0) {
throw new Failed(`gh pr edit failed for pull request #${manifest.pr}\n${printable(edited.stderr)}`);
throw new Failed(`gh issue edit failed for pull request #${manifest.pr}\n${printable(edited.stderr)}`);
}
if (next.files.length === 0) {
say(
Expand All @@ -914,7 +914,7 @@ function publishStep({
const arranged = gh(editBody, { cwd: filesDir });
if (arranged.status !== 0) {
throw new Failed(
`gh pr edit failed to lay out the files in pull request #${manifest.pr}\n${printable(arranged.stderr)}`,
`gh issue edit failed to lay out the files in pull request #${manifest.pr}\n${printable(arranged.stderr)}`,
);
}
say(
Expand Down
46 changes: 32 additions & 14 deletions .github/scripts/verify-attach.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1305,14 +1305,14 @@ describe('the step that edits the description', () => {
'api repos/clerk/clerk-ios/pulls/669',
'api repos/clerk/clerk-ios/pulls/669/commits?per_page=100&page=1',
'api repos/clerk/clerk-ios/pulls/669',
'pr edit',
'issue edit',
'api repos/clerk/clerk-ios/pulls/669',
'pr edit',
'issue edit',
],
);
const edit = fake.state.calls[3];
assert.deepEqual(edit.args, [
'pr',
'issue',
'edit',
'669',
'--repo',
Expand All @@ -1338,6 +1338,22 @@ describe('the step that edits the description', () => {
]);
});

it('publishes when a team is requested as a reviewer, which gh pr edit cannot read with the evidence token', async () => {
const env = await fetched();
const fake = fakeGh();
const teamRequested = {
gh: (args, options) =>
args[0] === 'pr'
? {
status: 1,
stderr:
'GraphQL: Resource not accessible by personal access token (repository.pullRequest.reviewRequests.nodes.0.requestedReviewer)\n',
}
: fake.gh(args, options),
};
assert.equal(publish(env, teamRequested).outcome, 'published');
});

it('replaces the block of an earlier run and keeps the text around it', async () => {
const env = await fetched();
const earlier = blockOf(manifestFor(MEDIA, { run: 'r20261007-010101-aaaa' }));
Expand Down Expand Up @@ -1392,7 +1408,7 @@ describe('the step that edits the description', () => {
assert.equal(outcome, 'dry run');
assert.deepEqual(fake.state.calls, []);
assert.deepEqual(said, [
`plain: would run, in ${path.join(env.WORK, 'files')}:\ngh pr edit 669 --repo clerk/clerk-ios --body-file ${path.join(env.WORK, 'body.md')} --attach ./evidence-n0nce-1.mp4 --attach ./evidence-n0nce-2.png`,
`plain: would run, in ${path.join(env.WORK, 'files')}:\ngh issue edit 669 --repo clerk/clerk-ios --body-file ${path.join(env.WORK, 'body.md')} --attach ./evidence-n0nce-1.mp4 --attach ./evidence-n0nce-2.png`,
]);
assert.equal(
fs.readFileSync(path.join(env.WORK, 'body.md'), 'utf8'),
Expand All @@ -1413,7 +1429,7 @@ describe('the step that edits the description', () => {
return lines.map(line => line.replace(/-[0-9a-f]{16}-/g, '-<nonce>-').replaceAll(env.WORK, '<work>'));
};
assert.deepEqual(await dry('ahead'), [
'would run, in <work>/files:\ngh pr edit 669 --repo clerk/clerk-ios --body-file <work>/body.md --attach ./evidence-<nonce>-1.mp4 --attach ./evidence-<nonce>-2.png',
'would run, in <work>/files:\ngh issue edit 669 --repo clerk/clerk-ios --body-file <work>/body.md --attach ./evidence-<nonce>-1.mp4 --attach ./evidence-<nonce>-2.png',
]);
assert.deepEqual(await dry('diverged'), [
'::notice title=verify-attach::Pull request #669 is not the pull request of run 37731397352 (the commit the session started on is not one of its commits or a commit of its base branch), so nothing is published.',
Expand Down Expand Up @@ -1639,7 +1655,7 @@ describe('the step that edits the description', () => {
const { outcome, said } = publish(await fetched(), fake);
assert.equal(outcome, 'listed');
assert.equal(fake.state.laidOutBody, null);
assert.equal(fake.state.calls.filter(call => call.args[0] === 'pr').length, 1);
assert.equal(fake.state.calls.filter(call => call.args[0] === 'issue').length, 1);
assert.deepEqual(said, [
'notice: Put verify run r20261008-052713-253e in the description of pull request #669 as a list of 2 files. The pull request changed before they could be laid out. A rerun of this job uploads them again and lays them out.',
]);
Expand Down Expand Up @@ -1682,11 +1698,13 @@ describe('the step that edits the description', () => {
const fake = fakeGh();
const failing = {
gh: (args, options) =>
args[0] === 'pr' && !args.includes('--attach') ? { status: 1, stderr: 'HTTP 502\n' } : fake.gh(args, options),
args[0] === 'issue' && !args.includes('--attach')
? { status: 1, stderr: 'HTTP 502\n' }
: fake.gh(args, options),
};
assert.throws(
() => publish(env, failing),
error => error.message === 'gh pr edit failed to lay out the files in pull request #669\ngh: HTTP 502',
error => error.message === 'gh issue edit failed to lay out the files in pull request #669\ngh: HTTP 502',
);
});

Expand Down Expand Up @@ -1970,7 +1988,7 @@ describe('the step that edits the description', () => {
const { outcome, said } = publish(await fetched(), fake);
assert.equal(outcome, 'refused');
assert.equal(
fake.state.calls.some(call => call.args[0] === 'pr'),
fake.state.calls.some(call => call.args[0] === 'issue'),
false,
);
assert.deepEqual(said, [
Expand Down Expand Up @@ -2005,7 +2023,7 @@ describe('the step that edits the description', () => {
const rerun = fakeGh({ ...PULL, body: list });
assert.equal(publish(await fetched(), rerun).outcome, 'published');
assert.deepEqual(
rerun.state.calls.filter(call => call.args[0] === 'pr').map(call => call.args.includes('--attach')),
rerun.state.calls.filter(call => call.args[0] === 'issue').map(call => call.args.includes('--attach')),
[true, false],
);
assert.equal(rerun.state.pull.body, `${PULL.body}\r\n${laidOut(SAID, ...AFTER_ONLY).replaceAll('\n', '\r\n')}`);
Expand Down Expand Up @@ -2079,24 +2097,24 @@ describe('the step that edits the description', () => {
const fake = fakeGh();
const failing = {
gh: (args, options) =>
args[0] === 'pr'
args[0] === 'issue'
? { status: 1, stderr: '::error::from gh\nunsupported authentication type\n' }
: fake.gh(args, options),
};
assert.throws(
() => publish(env, failing),
error =>
error.message ===
'gh pr edit failed for pull request #669\ngh: ::error::from gh\ngh: unsupported authentication type',
'gh issue edit failed for pull request #669\ngh: ::error::from gh\ngh: unsupported authentication type',
);
});
});

describe('what the workflow can be told by the script', () => {
it('writes each message as one workflow command, whatever the text holds', () => {
assert.equal(
workflowCommand('error', 'gh pr edit failed\ngh: ::add-mask::x\r\n100%'),
'::error title=verify-attach::gh pr edit failed%0Agh: ::add-mask::x%0D%0A100%25',
workflowCommand('error', 'gh issue edit failed\ngh: ::add-mask::x\r\n100%'),
'::error title=verify-attach::gh issue edit failed%0Agh: ::add-mask::x%0D%0A100%25',
);
});

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/verify-attach.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
RUN_HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }}
run: node .github/scripts/verify-attach.mjs fetch

- name: Install a gh whose pr edit has --attach
- name: Install a gh whose issue edit has --attach
if: steps.evidence.outputs.found == 'true'
env:
GH_VERSION: "2.102.0"
Expand Down
Loading