1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
965
966
967
968
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
1001
1002
1003
1004
1005
1006
1007
1008
1009
1010
1011
1012
1013
1014
1015
1016
1017
1018
1019
1020
1021
1022
1023
1024
1025
1026
1027
1028
1029
1030
1031
1032
1033
1034
1035
1036
1037
1038
1039
1040
1041
1042
1043
1044
1045
1046
1047
1048
1049
1050
1051
1052
1053
1054
1055
1056
1057
1058
1059
1060
1061
1062
1063
1064
1065
1066
1067
1068
1069
1070
1071
1072
1073
1074
1075
1076
1077
1078
1079
1080
1081
1082
1083
1084
1085
1086
1087
1088
1089
1090
1091
1092
1093
1094
1095
1096
1097
1098
1099
1100
1101
1102
1103
1104
1105
1106
1107
1108
1109
1110
1111
1112
1113
1114
1115
1116
1117
1118
1119
1120
1121
1122
1123
1124
1125
1126
1127
1128
1129
1130
1131
1132
1133
1134
1135
1136
1137
1138
1139
1140
1141
1142
1143
1144
1145
1146
1147
1148
1149
1150
1151
1152
1153
1154
1155
1156
1157
1158
1159
1160
1161
1162
1163
1164
1165
1166
1167
1168
1169
1170
1171
1172
1173
1174
1175
1176
1177
1178
1179
1180
1181
1182
1183
1184
1185
1186
1187
1188
1189
1190
1191
1192
1193
1194
1195
1196
1197
1198
1199
1200
1201
1202
1203
1204
1205
1206
1207
1208
1209
1210
1211
1212
1213
1214
1215
1216
1217
1218
1219
1220
1221
1222
1223
1224
1225
1226
1227
1228
1229
1230
1231
1232
1233
1234
1235
1236
1237
1238
1239
1240
1241
1242
1243
1244
1245
1246
1247
1248
1249
1250
1251
1252
1253
1254
1255
1256
1257
1258
1259
1260
1261
1262
1263
1264
1265
1266
1267
1268
1269
1270
1271
1272
1273
1274
1275
1276
1277
1278
1279
1280
1281
1282
1283
1284
1285
1286
1287
1288
1289
1290
1291
1292
1293
1294
1295
1296
1297
1298
1299
1300
1301
1302
1303
1304
1305
1306
1307
1308
1309name: Code Review
# Automated PR review by Claude, hosted on AWS Bedrock.
#
# Requires two repo settings (both configured in this account already):
# - secret AWS_BEDROCK_ROLE_ARN — IAM role this repo may assume via OIDC,
# with bedrock:InvokeModel / InvokeModelWithResponseStream on the review model.
# Currently role/equalify-iris-gha-bedrock-review in account 380610849750,
# trust scoped to this repo's pull_request + refs/heads/* subjects.
# - variable BEDROCK_REVIEW_MODEL (optional) — Bedrock inference-profile ID to
# review with; defaults below. Set to us.anthropic.claude-opus-5, a
# cross-region profile routing to us-east-1/us-east-2/us-west-2 (the IAM
# policy grants all three, or InvokeModel fails when it routes off-region).
#
# There is deliberately no `issue_comment` trigger here: @claude mentions are
# handled by the hosted Claude GitHub App, and duplicating it would double-post.
#
# Two GitHub identities post reviews from this job, and which one you get is not a
# style choice — it is the mechanism that lets this workflow review changes to itself.
# `claude-code-action` normally exchanges its OIDC token for a Claude App token and
# posts as claude[bot]. That exchange fails with `workflow_not_found_on_default_branch`
# when the workflow file invoking it differs from the copy on the default branch, and
# the action treats that as a reason to skip itself — which is why, before the explicit
# token below, every PR editing this file merged unreviewed. Handing the action an
# explicit `github_token` short-circuits that exchange (`setupGitHubToken` returns the
# provided token before it ever requests an OIDC token), so the review runs; it just
# posts as github-actions[bot]. This job therefore passes GITHUB_TOKEN *only* on a PR
# that changes this file, and leaves every other PR on the claude[bot] path.
on:
pull_request:
types: [opened, synchronize, ready_for_review]
# Only skip files that cannot affect behaviour. Note that `agents/*.md` and
# `docs/**` are NOT ignored: agent markdown files are executable prompts
# (loaded and sent to the model at runtime), and the API docs are part of
# the contract, so both are in scope for review.
paths-ignore:
- 'LICENSE'
- 'CODE_OF_CONDUCT.md'
- '.github/ISSUE_TEMPLATE/**'
- '.github/pull_request_template.md'
workflow_dispatch:
inputs:
pr_number:
description: 'PR number to review'
required: true
concurrency:
group: code-review-${{ github.event.pull_request.number || github.event.inputs.pr_number }}
cancel-in-progress: true
jobs:
review:
# Skip drafts and dependabot. `workflow_dispatch` and `ready_for_review`
# bypass the draft check (the former is a manual ask, the latter means the
# PR just stopped being a draft).
#
# Fork PRs are also skipped: `pull_request` from a fork gets no secrets, so
# the OIDC role assumption would fail with a confusing error. A maintainer
# reviews a fork PR by dispatching manually:
# gh workflow run code-review.yml -f pr_number=<n>
# Note that doing so runs the fork's code (npm ci, tests) in a job that holds
# the Bedrock role — read the diff before dispatching.
if: >-
github.actor != 'dependabot[bot]' &&
(github.event_name == 'workflow_dispatch' ||
((github.event.action == 'ready_for_review' || github.event.pull_request.draft == false) &&
github.event.pull_request.head.repo.full_name == github.repository))
runs-on: ubuntu-latest
# 30, not 15. The model needs ~6-13 min on a normal PR, but the job spends up
# to ~3 min before it on `bun install` inside claude-code-action (unpredictable:
# measured at 1.5s and at 139s on two runs 3h apart, 140 uncached packages).
# At 15 min a large PR review (PR #38) was cancelled mid-investigation while
# the run before it finished with 35s to spare — that was luck, not headroom.
# The real cap on the model is `timeout-minutes` on the Claude step below.
timeout-minutes: 30
permissions:
contents: read
pull-requests: write
issues: write
id-token: write
steps:
- uses: actions/checkout@v7
with:
fetch-depth: 0
- name: Resolve PR + head sha
id: ctx
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
set -euo pipefail
if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
# On a manual run, checkout@v4 gave us the default branch — move to
# the PR head so the diff and file reads below are the PR's code.
PR_NUMBER="${{ inputs.pr_number }}"
META=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER")
HEAD_SHA=$(echo "$META" | jq -r .head.sha)
BASE_REF=$(echo "$META" | jq -r .base.ref)
BASE_SHA=$(echo "$META" | jq -r .base.sha)
git fetch origin "pull/$PR_NUMBER/head:pr-$PR_NUMBER"
git checkout "pr-$PR_NUMBER"
else
PR_NUMBER="${{ github.event.pull_request.number }}"
HEAD_SHA="${{ github.event.pull_request.head.sha }}"
BASE_REF="${{ github.event.pull_request.base.ref }}"
BASE_SHA="${{ github.event.pull_request.base.sha }}"
fi
# Does this PR modify the review workflow itself? If so the Claude step
# is handed an explicit GITHUB_TOKEN, because the OIDC exchange for a
# Claude App token refuses (`workflow_not_found_on_default_branch`) while
# this file differs from the default-branch copy and the action then skips
# itself. See the header comment for the mechanism. The consequence the
# steps below care about is the identity: the review posts as
# github-actions[bot], not claude[bot].
if gh pr diff "$PR_NUMBER" --name-only | grep -qx '.github/workflows/code-review.yml'; then
echo "changed_workflow=code-review.yml" >> "$GITHUB_OUTPUT"
echo "This PR modifies code-review.yml — reviewing under GITHUB_TOKEN, so the review will post as github-actions[bot]."
# Say it on the PR itself, not just in a log nobody opens. This notice used
# to report that the PR would get NO review at all, which was true from PR
# #63 (when workflow-review.yml, the `pull_request_target` job that had
# covered the gap without once succeeding in 6 runs, was deleted) until the
# explicit token below closed it. The PR is reviewed now, so the notice says
# what is still different about it — a non-standard token path and a diff
# whose failure modes the general review prompt only partly covers — rather
# than going quiet, because every other signal on such a PR goes green.
# Single-quoted, and that is load-bearing on the last line. Two layers of
# escaping stack there, and each one bites in a different place:
# 1. Actions substitutes `${{ '${{' }}` textually before any shell sees
# the script. An open-expression sequence left bare anywhere in this
# file — including in a comment like this one — is parsed as an
# expression, and the resulting error does not degrade the workflow,
# it disables the whole FILE. That is why this comment escapes it too.
# 2. What that substitution leaves behind is a bare open-expression
# sequence, which bash then reads as a parameter expansion. Inside
# double quotes that is `bad substitution` and the step exits 1 —
# exactly how this block failed on the PR that introduced it.
# Single quotes stop bash expanding it while Actions still escapes it, and
# they make the backticks literal as a bonus. See the actionlint rationale
# further down for why an expression error here is unrecoverable.
# Written to a file, then copied to both the step summary and a PR
# comment (see `Say on the PR how its own review was produced` below).
# One source for this text, because the escaping on the last line is
# too easy to get wrong twice.
{
echo '### This PR changes `code-review.yml` — its review runs on the fallback token'
echo
echo 'The OIDC exchange for a Claude App token refuses while this file differs from'
echo 'the copy on the default branch, so the job hands the action `GITHUB_TOKEN`'
echo 'instead. The review still runs; it posts as **github-actions[bot]** rather than'
echo 'claude[bot]. Every other check (install, typecheck, unit, e2e, actionlint) runs'
echo "against this PR's head as usual."
echo
echo '**Read the workflow diff yourself anyway.** The reviewer has a CI-security'
echo 'checklist for it, but it is reviewing the file that decides whether anything'
echo 'gets reviewed. Check in particular whether this diff lets PR-authored code run'
echo 'with secrets, widens `permissions:`, interpolates attacker-controlled'
echo '`${{ '${{' }} github.event.* }}` into a `run:` block, or weakens the step that'
echo 'fails the job when no review was posted.'
echo
echo "Run: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
} > /tmp/workflow-notice.md
cat /tmp/workflow-notice.md >> "$GITHUB_STEP_SUMMARY"
echo "::warning::This PR changes code-review.yml. It is reviewed under GITHUB_TOKEN (review posts as github-actions[bot]); read the workflow diff by hand as well."
else
echo "changed_workflow=none" >> "$GITHUB_OUTPUT"
fi
echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT"
echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT"
echo "base_ref=$BASE_REF" >> "$GITHUB_OUTPUT"
echo "base_sha=$BASE_SHA" >> "$GITHUB_OUTPUT"
- name: Skip if a claude review already exists for this commit
id: guard
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ steps.ctx.outputs.pr_number }}
HEAD_SHA: ${{ steps.ctx.outputs.head_sha }}
SELF_EDIT: ${{ contains(steps.ctx.outputs.changed_workflow, 'code-review.yml') }}
run: |
set -euo pipefail
# Per-commit idempotency: if a model-written review already exists for
# this exact sha, don't post again. A new push (new sha) is reviewed as
# normal.
#
# github-actions[bot] counts only on a PR that changes this workflow, because
# that is the only PR where a model-written review arrives under that identity
# (it is reviewed under GITHUB_TOKEN — see `Resolve PR + head sha`). Scoped
# rather than global: this guard gates the whole suite below, so accepting
# that login everywhere would let any future `github-actions[bot]` review on a
# head sha — another workflow, a `gh` one-liner — skip the tests and the review
# while the job reports success. Two bodies are filtered back out, and both
# filters are load-bearing:
#
# FALLBACK — the `Fallback review` step at the end of this job also posts as
# github-actions[bot], and its body opens with that heading. `startswith`,
# not `contains`: a review of this workflow may well quote the heading while
# discussing the fallback path, and matching anywhere in the body would then
# discard a real review. A fallback is a partial verdict, and the reason to
# dispatch a re-run is to replace it with a real one; counting it would
# make that dispatch a no-op. It is still enough for `Verify a review was
# posted`, which asks a different question (does this sha carry any
# verdict at all).
# MARKER — workflow-review.yml, deleted in PR #63, posted CI-security reviews
# that still exist on PRs #58, #61 and #62, so a re-run against one of
# those shas would still see one.
#
# This guard gates EVERY step below it — including npm ci, typecheck, unit and
# e2e — so counting a review this job did not write means the whole suite is
# skipped while the job reports success.
MARKER='Scope: CI/workflow security review'
FALLBACK='## Automated review did not complete'
if [ "${SELF_EDIT:-false}" = "true" ]; then
LOGINS='["claude[bot]","github-actions[bot]"]'
else
LOGINS='["claude[bot]"]'
fi
COUNT=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" --paginate \
| jq --arg m "$MARKER" --arg f "$FALLBACK" --arg sha "$HEAD_SHA" --argjson logins "$LOGINS" \
'[.[] | select(.user.login | IN($logins[]))
| select(.commit_id==$sha)
| select(((.body // "") | startswith($m)) | not)
| select(((.body // "") | startswith($f)) | not)] | length' \
| awk '{s+=$1} END {print s+0}')
if [ "${COUNT:-0}" -gt 0 ]; then
echo "already_reviewed=true" >> "$GITHUB_OUTPUT"
echo "A model-written review already exists for $HEAD_SHA (logins: $LOGINS) — skipping."
else
echo "already_reviewed=false" >> "$GITHUB_OUTPUT"
fi
- name: Say on the PR how its own review was produced
# A step summary and a `::warning::` annotation are both one click off the
# PR page, and PR #78 shipped with neither noticed: green checks, empty
# timeline, and the author asking why no review arrived. Say it where the
# author is actually looking.
#
# continue-on-error, and deliberately: this notice is the least important
# thing this job does. If the comment API fails, the tests and actionlint
# below still have to run — failing here would throw them away to report
# that a courtesy comment did not post.
if: >-
steps.guard.outputs.already_reviewed != 'true' &&
contains(steps.ctx.outputs.changed_workflow, 'code-review.yml')
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ steps.ctx.outputs.pr_number }}
run: |
set -euo pipefail
# One comment per PR, edited in place on later pushes, not a new comment
# on every `synchronize`. A PR iterating on this workflow pushes many
# times, and ten copies of the same notice reads as breakage.
#
# Not atomic, and it does not need to be: two pushes landing inside the
# list→create window can both create, since `cancel-in-progress` does not
# order these. The result is one duplicate comment, and the next push
# edits the first one and stops adding more. Silence was the bug being
# fixed here; a duplicate is not the same class of problem.
# Marker text unchanged from when this comment said "will not be reviewed":
# it is an identity, not a description. Renaming it would leave the old
# comment on any PR currently open against this file and add a second one
# beside it, which is the duplicate this step exists to avoid.
MARKER='<!-- code-review: workflow-self-skip -->'
BODY=/tmp/workflow-notice-comment.md
{
echo "$MARKER"
cat /tmp/workflow-notice.md
} > "$BODY"
# --jq over the marker, not the visible text: the body is reworded
# whenever the notice above changes, and an edit must still find the
# comment it wrote last time.
#
# `first` is per-page — gh applies --jq to each page separately — so on a
# PR with >30 comments this can emit one id per page. Take the first line
# in bash rather than piping to `head`, which under `set -o pipefail`
# turns a SIGPIPE into a step failure.
IDS=$(gh api "repos/${{ github.repository }}/issues/$PR_NUMBER/comments" --paginate \
--jq "[.[] | select((.body // \"\") | contains(\"$MARKER\")) | .id] | first // empty")
ID=${IDS%%$'\n'*}
if [ -n "$ID" ]; then
gh api --method PATCH "repos/${{ github.repository }}/issues/comments/$ID" \
-f body="$(cat "$BODY")" --silent
echo "updated existing workflow-notice comment $ID"
else
gh pr comment "$PR_NUMBER" --body-file "$BODY"
echo "posted workflow-notice comment"
fi
- uses: actions/setup-node@v7
if: steps.guard.outputs.already_reviewed != 'true'
with:
# Must match .nvmrc: Iris runs .ts sources directly and depends on
# --experimental-sqlite / --env-file-if-exists.
node-version-file: .nvmrc
cache: npm
- name: Lint the workflow scripts
id: scriptlint
if: steps.guard.outputs.already_reviewed != 'true'
# `.github/scripts/*.sh` holds a workflow's own logic, kept out of the YAML
# only because GitHub refuses a `run:` block past 21000 bytes — and
# `triage-decide.sh` is the file that decides whether an issue closes.
# Nothing looked at it until this step existed: `actionlint` reads
# `.github/workflows/*.yml` and nothing else, and it runs with
# `-shellcheck=` off anyway.
#
# A separate shellcheck pass rather than re-enabling actionlint's: the
# false-positive argument for turning it off is about heavily-templated
# `run:` blocks, and these files are plain bash with no Actions expressions
# in them at all. `shellcheck` is preinstalled on ubuntu-latest, so there is
# nothing to download and no pin to verify — the reason actionlint needs six
# lines of checksum above. `-S warning` is quiet on correct code, which is
# what keeps a style opinion from reading as a failed check.
#
# Its own step, and its report is a file the context builder cats, because
# that builder's `run:` block is within a kilobyte of the 21000-byte ceiling
# and this repo has already lost a run to crossing it.
run: |
set -euo pipefail
OUT=/tmp/script-lint.md
{
echo
echo "## Workflow scripts — \`shellcheck\`"
echo '```'
} > "$OUT"
if ! ls .github/scripts/*.sh >/dev/null 2>&1; then
echo "(no .github/scripts/*.sh in this checkout)" >> "$OUT"
RESULT=skip
elif ! command -v shellcheck >/dev/null 2>&1; then
echo "(shellcheck not installed on this runner)" >> "$OUT"
RESULT=skip
elif shellcheck -s bash -S warning .github/scripts/*.sh 2>&1 | tail -40 >> "$OUT"; then
RESULT=pass
else
RESULT=fail
fi
{ echo '```'; echo; echo "Result: **$RESULT**"; } >> "$OUT"
echo "result=$RESULT" >> "$GITHUB_OUTPUT"
echo "shellcheck over .github/scripts: $RESULT"
# poppler-utils, for the same reason the Dockerfile installs it: `rasterizePdf`
# shells out to pdftoppm/pdfinfo/pdftohtml, and the tests that exercise it skip
# themselves when those are absent (`hasPoppler` in test/pdf-links.test.ts).
# Without this the PDF path's only end-to-end guards — that a document renders to
# the same pages whether the range is split across processes or not, and that one
# over the page cap is refused — report as passes when they never ran.
#
# Guarded like the step it exists for: `npm test` runs inside "Build review
# context", so on a commit already reviewed there is nothing here to install for.
#
# And failure-tolerant, like every other pre-model step in this job — `Lint the
# workflow scripts` resolves a missing shellcheck to `RESULT=skip`, and `Build
# review context` captures an `npm ci` or `npm test` failure into the context
# rather than exiting non-zero. A step's `if:` carries an implicit `success()`
# unless it names a status function, and neither `Fallback review` nor `Verify a
# review was posted` does — so a step that merely exits non-zero here would skip
# every remaining step in the job, and the PR would get no verdict AND no loud
# failure saying so. That is the PR #38 outcome the fallback exists to prevent,
# arriving via an apt mirror 503.
#
# Tolerating it costs only what the step buys: `hasPoppler()` in
# test/pdf-links.test.ts skips the PDF tests when the tools are absent, exactly as
# it did before this step existed. The warning is what stops that from being
# silent. Not `-qq`, so a failure leaves behind the output that explains it.
- name: Install poppler-utils (PDF rasterization tests)
if: steps.guard.outputs.already_reviewed != 'true'
continue-on-error: true
run: |
if sudo apt-get update && sudo apt-get install -y poppler-utils; then
echo "poppler-utils installed"
else
echo "::warning::poppler-utils failed to install; the PDF rasterization tests will skip themselves (hasPoppler). The review still runs."
fi
- name: Build review context
if: steps.guard.outputs.already_reviewed != 'true'
# `emit_source` below marks where `head -n $MAX_LINES_PER_FILE` cut a file.
# An unmarked cut is worse than a smaller budget: the fence closes mid-line
# and nothing says the rest exists, so the reviewer reads a partial file as
# a whole one and can conclude something is missing when it is 200 lines
# further down. This is not hypothetical — `issue-triage.yml` (1017 lines)
# was halved this way in the review of PR #147, for a file the CI-security
# item tells the reviewer to read in full.
#
# The line count comes from `awk 'END{print NR}'` and not `wc -l` because wc
# counts newlines, so a file whose last line is unterminated reports one
# short — and at exactly the cap that hid the cut completely. It is fed on
# stdin and not as an argument because awk reads any argument matching
# `name=value` as a variable assignment and then falls back to stdin, which
# inside the `while read -r f` loop is the rest of the changed-file list: one
# root-level path with an `=` in it would end the loop and drop every
# remaining file's source, silently, which is the harm the marker exists to
# prevent one layer up. `< "$1"` is immune, and matches the `wc -c < "$f"`
# idiom the loop already uses.
#
# The rationale lives here rather than in the block because a `run:` block
# is one expression capped at 21000 and this is the block near that ceiling —
# within about a kilobyte of the ~20,500 usable figure that
# `.github/scripts/triage-decide.sh` derives. YAML comments outside the
# scalar cost nothing against it, which is the only reason there is room to
# explain any of this; at this block's average line, roughly a dozen more
# lines of *code* is what would spend the rest, and the next thing that needs
# real length belongs in `.github/scripts/` like the triage decision did. To
# re-measure, count the block's bytes plus one per line, or dispatch the
# workflow and read the 422 body.
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ steps.ctx.outputs.pr_number }}
HEAD_SHA: ${{ steps.ctx.outputs.head_sha }}
BASE_REF: ${{ steps.ctx.outputs.base_ref }}
BASE_SHA: ${{ steps.ctx.outputs.base_sha }}
SCRIPT_LINT: ${{ steps.scriptlint.outputs.result }}
run: |
set -euo pipefail
OUT=/tmp/review-context.md
MAX_LINES_PER_FILE=800
MAX_BYTES_PER_FILE=120000
# Include whole-file source only for files whose changed lines are at
# least this share of the file (new files are 100%), or which are small
# enough that including them costs nothing. See the loop below.
#
# 15% is calibrated against PR #38, the one that timed out: it keeps every
# touched src/ and test/ file in full (the lowest was sessions.ts at 19%)
# while dropping prd.md, demo.html, README.md and docs/API.md, which
# together contributed 104KB for 56 changed lines. That lands the context
# at ~131KB — the same size as the review before it, which finished.
# Raising this trims more but starts dropping real code; lower it and the
# docs come back.
MIN_CHANGED_PCT=15
ALWAYS_INCLUDE_UNDER_BYTES=8000
# Risk allowlist: always include these in full, whatever the ratio says.
# The ratio filter keys on change *size*, but the prompt's "Must flag" list
# keys on *risk*, and the two disagree exactly where it matters most — a
# 6-line change to an auth check or a token choice is both the smallest
# possible diff and the most dangerous. Most of the highest-risk files
# (src/auth/*, src/github/issue.ts, src/store/paths.ts, contribute.ts,
# src/providers/*) are under the 8KB floor and already arrive in full;
# these are the ones big enough to be dropped:
# routes/sessions.ts — per-session ownership checks, Must flag #3
# config.ts — model routing + provider resolution, Must flag #4
# pipeline/*.ts — phase error handling and the axe-clean guarantee,
# Must flag #1 and #5
# .github/scripts/ — a workflow's own logic, kept out of the YAML only
# because GitHub refuses a `run:` block past 21000
# characters. `triage-decide.sh` is 22KB and decides
# whether an issue closes; item 9 asks the reviewer
# to read changed CI code in full, so it has to be
# here or that instruction has nothing to read.
# Matched as path prefixes against the changed-file list below.
ALWAYS_INCLUDE_PATHS="src/auth/ src/github/ src/providers/ src/store/paths.ts src/pipeline/ src/routes/sessions.ts src/config.ts .github/scripts/"
{
echo "# Review context"
echo
echo "## PR metadata"
echo '```json'
gh pr view "$PR_NUMBER" --json number,title,author,body,baseRefName,headRefName
echo '```'
} > "$OUT"
# Earlier claude[bot] reviews on *previous* commits of this PR. Without these
# every push re-derives the review from zero: PR #56 collected four
# `--request-changes` reviews in six hours, each re-arguing ground the last one
# had covered, and PR #49 took 13 reviews to reach 1 approval. The reviewer
# cannot tell "the author ignored this" from "the author fixed this" unless it
# can see what was already said.
#
# Reviews on the CURRENT sha are excluded: if one exists the guard above has
# already skipped this whole job, so any match here would be a race, and feeding
# the model its own verdict for the commit under review invites it to restate it.
#
# Bounded two ways, because this section competes with the diff for context.
# PR #49 accumulated 13 reviews totalling 59KB — a 45% increase on a ~131KB
# context that was calibrated against the PR #38 timeout.
# PRIOR_REVIEWS_MAX: how many to include. The API returns them oldest-first, so
# a byte cap on the concatenation would keep the STALEST and truncate the
# newest — backwards. `tail -n` picks the most recent instead, and they stay
# in chronological order, which is the readable direction.
# PRIOR_BODY_MAX: per-review character cap, applied in jq. These bodies carry
# full repro scripts and measurement tables; the verdict and the headings are
# what a re-review needs, not every byte of evidence it already published.
PRIOR_REVIEWS_MAX=3
PRIOR_BODY_MAX=8000
# One compact JSON object per line, so "most recent N" is a `tail -n` on a FILE.
# Not a pipe into `tail`/`head`: the producer would take SIGPIPE, exit 141, and
# `set -o pipefail` would abort this step before the review is posted (PR #39).
PRIOR_LINES=/tmp/prior-reviews.jsonl
PRIOR_RECENT=/tmp/prior-reviews-recent.jsonl
# Both logins: a review of a PR that edits this workflow is posted by
# `github-actions[bot]`, so matching claude[bot] alone would silently drop
# every earlier review of exactly the PRs that get iterated on most, and the
# re-review would start from zero. The fallback's own body is excluded — its
# partial findings are not a verdict to be held to.
if gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" --paginate \
--jq ".[] | select(.user.login==\"claude[bot]\" or .user.login==\"github-actions[bot]\")
| select(.commit_id!=\"$HEAD_SHA\")
| select(((.body // \"\") | startswith(\"Scope: CI/workflow security review\")) | not)
| select(((.body // \"\") | startswith(\"## Automated review did not complete\")) | not)
| {sha: .commit_id[0:7], state: .state, body: (.body // \"(empty)\")}
| @json" \
> "$PRIOR_LINES" 2>/dev/null && [ -s "$PRIOR_LINES" ]; then
TOTAL_PRIOR=$(wc -l < "$PRIOR_LINES" | tr -d ' ')
tail -n "$PRIOR_REVIEWS_MAX" "$PRIOR_LINES" > "$PRIOR_RECENT"
{
echo
echo "## Your earlier reviews of this PR (previous commits)"
echo
echo "You have already reviewed earlier pushes to this branch. Read these before"
echo "reviewing the diff, and treat them as follows:"
echo
echo "- A finding that is **fixed** in the current head: say so in one line, do not"
echo " re-explain it. Confirming a fix is useful; re-litigating it is not."
echo "- A finding that is **still present**: say it is unchanged since"
echo " \`<sha>\` and keep it short — do not rebuild the repro you already posted."
echo " If you have raised the same blocking point twice and the author has pushed"
echo " past it without addressing it, state that plainly once and leave it to a"
echo " human; do not escalate and do not repeat it a third time."
echo "- A finding you now believe was **wrong**: withdraw it explicitly. That is"
echo " more valuable than quietly dropping it."
echo "- Do NOT hunt for new findings just because the diff changed. If the push"
echo " fixed what you raised and introduced nothing new, approve it."
echo
echo "Each review below starts at a \`=== REVIEW <state> on commit <sha> ===\` line."
echo "Any \`##\`/\`###\` headings inside one are that review's own findings, not"
echo "sections of this context."
if [ "$TOTAL_PRIOR" -gt "$PRIOR_REVIEWS_MAX" ]; then
echo
echo "_Showing the $PRIOR_REVIEWS_MAX most recent of $TOTAL_PRIOR earlier reviews,"
echo "oldest first. Bodies over ${PRIOR_BODY_MAX} characters are truncated — the"
echo "full text is on the PR if you need it._"
fi
echo
} >> "$OUT"
# jq reads the file directly and does the unescaping; a shell `while read`
# loop would have to unescape JSON string bodies itself. The per-body cap is
# applied here rather than in the `gh api --jq` above: that flag takes no
# `--argjson`, so a `$PRIOR_BODY_MAX` there is an undefined *jq* variable
# (not a shell expansion) and the whole call fails with "variable not defined".
jq -r --argjson max "$PRIOR_BODY_MAX" \
'"=== REVIEW \(.state) on commit \(.sha) ===\n\n\(.body[:$max])\n"
+ (if (.body | length) > $max then "\n_[body truncated]_\n" else "" end)' \
"$PRIOR_RECENT" >> "$OUT"
fi
# Every check below records pass/fail into the context instead of
# aborting the step — a red build is exactly what the reviewer needs to
# see, so it must never prevent the review from being posted.
{
echo
echo "## Install — \`npm ci\`"
echo '```'
} >> "$OUT"
if npm ci 2>&1 | tail -40 >> "$OUT"; then
INSTALL=pass
else
INSTALL=fail
fi
{ echo '```'; echo; echo "Result: **$INSTALL**"; } >> "$OUT"
{
echo
echo "## Typecheck — \`npm run typecheck\`"
echo '```'
} >> "$OUT"
if npm run typecheck 2>&1 | tail -80 >> "$OUT"; then
TYPECHECK=pass
else
TYPECHECK=fail
fi
{ echo '```'; echo; echo "Result: **$TYPECHECK**"; } >> "$OUT"
{
echo
echo "## Unit tests — \`npm test\`"
echo '```'
} >> "$OUT"
if npm test 2>&1 | tail -120 >> "$OUT"; then
UNIT=pass
else
UNIT=fail
fi
{ echo '```'; echo; echo "Result: **$UNIT**"; } >> "$OUT"
# End-to-end suite: boots mock GitHub + mock OpenRouter and drives every
# /v1 endpoint with curl. Needs jq (preinstalled on ubuntu-latest) and
# binds fixed ports 8099/9301/9302 — fine on a dedicated runner.
{
echo
echo "## End-to-end — \`./test/e2e.sh\`"
echo '```'
} >> "$OUT"
# Tee rather than redirect: on failure the output has to reach the job
# log too, or a CI-only e2e failure is undebuggable from the run page.
if ./test/e2e.sh 2>&1 | tee /tmp/e2e-output.txt | tail -120 >> "$OUT"; then
E2E=pass
else
E2E=fail
fi
{ echo '```'; echo; echo "Result: **$E2E**"; } >> "$OUT"
if [ -f /tmp/iris-e2e.log ]; then
{ echo; echo "<details><summary>iris server log (e2e)</summary>"; echo; echo '```'; } >> "$OUT"
tail -120 /tmp/iris-e2e.log >> "$OUT"
{ echo '```'; echo; echo "</details>"; } >> "$OUT"
fi
if [ "$E2E" = fail ]; then
echo "::group::e2e output (failed)"
tail -80 /tmp/e2e-output.txt || true
echo "::endgroup::"
echo "::group::iris server log (e2e)"
tail -80 /tmp/iris-e2e.log 2>/dev/null || echo "(no server log)"
echo "::endgroup::"
fi
# Workflow lint. Added because a real bug shipped past every check above: a
# literal `${{ '${{' }} }}` written inside a `run:` block — in prose, in a
# prompt, even in a shell comment like this one, since comments are still part
# of the run string — is parsed by Actions as an expression, not text. The
# resulting error kills the whole FILE: GitHub lists the workflow by its path
# instead of its `name:`, and every run fails instantly with no jobs and no
# logs, which reads like an infrastructure blip. YAML parsing, `bash -n` and
# the test suite all pass on such a file; nothing here would have caught it.
# Escape it as this comment does.
#
# Pinned by version, and the download is verified: an unpinned installer for
# a linter is a supply-chain path into a job that holds id-token: write.
{
echo
echo "## Workflow lint — \`actionlint\`"
echo '```'
} >> "$OUT"
ACTIONLINT_VERSION=1.7.12
# From actionlint_1.7.12_checksums.txt on the release page, confirmed against
# a local download of the same asset.
ACTIONLINT_SHA256=8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8
if curl -fsSL -o /tmp/actionlint.tgz \
"https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" \
&& echo "${ACTIONLINT_SHA256} /tmp/actionlint.tgz" | sha256sum -c - >/dev/null 2>&1 \
&& tar -xzf /tmp/actionlint.tgz -C /tmp actionlint; then
# shellcheck/pyflakes disabled: this gate is about Actions-expression and
# workflow-schema errors. Shell style in this workflow is reviewed by a
# human — the CI-security reviewer that used to share the job was deleted
# in PR #63 — and shellcheck on these heavily-templated blocks is mostly
# false positives.
if /tmp/actionlint -shellcheck= -pyflakes= .github/workflows/*.yml 2>&1 | tail -40 >> "$OUT"; then
WFLINT=pass
else
WFLINT=fail
fi
else
echo "(could not install actionlint v${ACTIONLINT_VERSION} — download failed or checksum mismatch)" >> "$OUT"
WFLINT=skip
fi
{ echo '```'; echo; echo "Result: **$WFLINT**"; } >> "$OUT"
# The script lint ran in its own step, above, so this block stays clear of
# the 21000-byte ceiling on a `run:` block. Its report is a file.
cat /tmp/script-lint.md >> "$OUT"
# Summary goes to four places: the context file (for the reviewer), the
# job log and the run summary (for humans), and its own small file for
# the fallback step to cat. Without the middle two, a check failing in CI
# would be visible only to the model while this step still reported
# success. The standalone file exists because the fallback cannot cheaply
# extract this section back out of $OUT — `sed '/marker/,$p' | head -20`
# streams ~130KB into a 20-line reader, takes SIGPIPE, and exits 141,
# which `set -o pipefail` turns into a step abort *before* the review is
# posted. A dedicated file needs no pipe at all.
# The four original lines are no longer parsed by anything outside this file
# (workflow-review.yml, which grepped them out of this job's log, was deleted in
# PR #63), so the format is now free to change — but the fallback step below and
# the run summary both render it, so keep it readable.
SUMMARY=$(printf '## Check summary\n- npm ci: %s\n- typecheck: %s\n- unit (`npm test`): %s\n- e2e (`./test/e2e.sh`): %s\n- workflow lint (`actionlint`): %s\n- workflow scripts (`shellcheck`): %s\n' \
"$INSTALL" "$TYPECHECK" "$UNIT" "$E2E" "$WFLINT" "$SCRIPT_LINT")
{ echo; echo "$SUMMARY"; } >> "$OUT"
echo "$SUMMARY"
echo "$SUMMARY" >> "$GITHUB_STEP_SUMMARY"
echo "$SUMMARY" > /tmp/check-summary.md
# `skip` for the lint (install failed) is not a check failure — it says
# nothing about the PR. Only an actual `fail` counts.
if [ "$INSTALL$TYPECHECK$UNIT$E2E" != "passpasspasspass" ] || [ "$WFLINT" = fail ] || [ "$SCRIPT_LINT" = fail ]; then
echo "::warning::One or more checks failed — see the run summary. The review continues so Claude reports it on the PR."
fi
# Diff against the PR's recorded base SHA, NOT `merge-base origin/$BASE_REF
# HEAD`. The old form silently produced an EMPTY diff whenever the base
# branch had advanced to contain HEAD — e.g. reviewing an already-merged PR
# via workflow_dispatch — because merge-base then returns HEAD itself. The
# reviewer got a context with no changed files and no diff, reviewed
# nothing, and posted anyway; `Verify` was satisfied, so it read as a pass.
# That happened for real on PR #39's own review.
#
# base.sha is fixed at the tip of the base branch when the PR was created
# or last synchronized, so it cannot drift into HEAD. `fetch-depth: 0` on
# checkout means it is already present locally.
MERGE_BASE="$BASE_SHA"
if ! git cat-file -e "$MERGE_BASE^{commit}" 2>/dev/null; then
# Force-push to the base branch can orphan the recorded base.sha. Fall
# back to a real merge-base rather than failing the whole review.
echo "::warning::base sha $MERGE_BASE not found locally; falling back to merge-base against origin/$BASE_REF"
git fetch origin "$BASE_REF" || true
MERGE_BASE=$(git merge-base "origin/$BASE_REF" HEAD)
fi
# An empty diff means the context would carry no changes at all. Warn
# loudly rather than exiting: a `workflow_dispatch` re-review of a merged
# PR is a legitimate thing to ask for, and hard-failing would turn it into
# a red job. The reviewer is told below to treat this as unreviewable, so
# it cannot be mistaken for a clean pass.
if [ -z "$(git diff --name-only "$MERGE_BASE"...HEAD)" ]; then
echo "::warning::Empty diff for base $MERGE_BASE — the PR head is already contained in its base. The review context has no changes."
EMPTY_DIFF=yes
else
EMPTY_DIFF=no
fi
{
echo
if [ "$EMPTY_DIFF" = yes ]; then
echo "## ⚠️ NO CHANGES TO REVIEW"
echo
echo "The diff against base \`$MERGE_BASE\` is empty — this PR's head is already"
echo "contained in its base branch (most likely it is already merged, and this"
echo "review was dispatched manually afterwards)."
echo
echo "**Do not post an approval.** There is nothing here to approve, and an"
echo "\`--approve\` on an empty context reads as a verified pass. Post a single"
echo "\`--request-changes\` stating that the context contained no diff and the PR"
echo "could not be reviewed, and stop."
echo
fi
echo "## Changed files (base \`$MERGE_BASE\`)"
echo '```'
git diff --name-status "$MERGE_BASE"...HEAD
echo '```'
echo
echo "## Full diff"
echo '```diff'
git diff "$MERGE_BASE"...HEAD
echo '```'
echo
echo "## Full source of new / substantially-rewritten files"
echo
echo "Files changed by less than ${MIN_CHANGED_PCT}% of their lines are omitted here —"
echo "their hunks are in the diff above, and the reviewer can Read them on demand."
echo "High-risk paths (auth, github, providers, pipeline, session routes, config)"
echo "are always included in full, however small the change."
} >> "$OUT"
# Say where a file was cut. See this step's YAML comment for why, and for
# why both the `awk` and the `<` are deliberate.
emit_source() {
head -n "$MAX_LINES_PER_FILE" "$1" >> "$OUT"
N=$(awk 'END{print NR}' < "$1")
[ "${N:-0}" -le "$MAX_LINES_PER_FILE" ] || printf \
'\n... TRUNCATED at %s of %s lines. Read the file for the rest.\n' \
"$MAX_LINES_PER_FILE" "$N" >> "$OUT"
}
# Whole-file source is included only where the diff alone is hard to
# judge: new files, and files substantially rewritten. Dumping every
# touched file made review context 235KB on PR #38 vs 132KB on the PR
# before it, and the review was cancelled mid-investigation. 104KB of
# that was four files changed by 56 lines between them — prd.md alone
# contributed 51KB for a 10-line edit. Big context is not free: it is
# slower to stream and it buries the 5% that matters.
git diff --name-only --diff-filter=AM "$MERGE_BASE"...HEAD | while read -r f; do
[ -f "$f" ] || continue
if file --mime "$f" | grep -q 'charset=binary'; then
printf '\n### %s\n\n_(binary — skipped)_\n' "$f" >> "$OUT"
continue
fi
BYTES=$(wc -c < "$f" | tr -d ' ')
if [ "$BYTES" -gt "$MAX_BYTES_PER_FILE" ]; then
printf '\n### %s\n\n_(%s bytes — too large, see diff above)_\n' "$f" "$BYTES" >> "$OUT"
continue
fi
# Risk allowlist wins over the ratio: see ALWAYS_INCLUDE_PATHS above.
FORCED=no
for p in $ALWAYS_INCLUDE_PATHS; do
case "$f" in
"$p"*) FORCED=yes; break ;;
esac
done
if [ "$FORCED" = yes ]; then
printf '\n### %s\n\n_(high-risk path — included in full regardless of change size)_\n\n```\n' "$f" >> "$OUT"
emit_source "$f"
printf '\n```\n' >> "$OUT"
continue
fi
# Ratio of changed (added+deleted) lines to current file length.
# numstat prints "-\t-" for binaries, already filtered above.
CHANGED=$(git diff --numstat "$MERGE_BASE"...HEAD -- "$f" | awk '{print $1 + $2}')
LINES=$(wc -l < "$f" | tr -d ' ')
# Guard LINES=0: a file with no trailing newline reports 0 and would
# divide by zero. Treat it as fully changed — it is tiny either way.
if [ "${LINES:-0}" -eq 0 ]; then
PCT=100
else
PCT=$(( ${CHANGED:-0} * 100 / LINES ))
fi
if [ "$PCT" -lt "$MIN_CHANGED_PCT" ] && [ "$BYTES" -gt "$ALWAYS_INCLUDE_UNDER_BYTES" ]; then
printf '\n### %s\n\n_(%s%% of %s lines changed — diff above is sufficient; Read the file if you need more)_\n' \
"$f" "$PCT" "$LINES" >> "$OUT"
continue
fi
printf '\n### %s\n\n```\n' "$f" >> "$OUT"
emit_source "$f"
printf '\n```\n' >> "$OUT"
done
{
echo
echo "## Agent library (agents/*.md are executable prompts)"
echo '```'
ls -1 agents/
echo '```'
} >> "$OUT"
echo "context: $(wc -c < "$OUT" | tr -d ' ') bytes"
- name: Configure AWS credentials (OIDC)
if: steps.guard.outputs.already_reviewed != 'true'
uses: aws-actions/configure-aws-credentials@v6
with:
role-to-assume: ${{ secrets.AWS_BEDROCK_ROLE_ARN }}
aws-region: us-east-2
- name: Claude review
id: claude
if: steps.guard.outputs.already_reviewed != 'true'
# Bound the model separately from the job. A step timeout fails only this
# step, so `Fallback review` below still runs and the PR always gets a
# verdict; when the *job* hits its cap instead, every remaining step is
# cancelled and the PR gets nothing (this is exactly how PR #38 ended up
# with zero reviews after 12 minutes of Opus).
#
# Budget arithmetic, stated precisely because the obvious reading is wrong:
# `bun install` runs INSIDE this step (it is part of the composite action),
# so it eats into the 22 minutes, not into the job's remaining 8. At its
# worst observed 3 min that leaves the model ~19 min, which is what the
# prompt below promises. The job's 8-minute remainder covers everything
# outside this step: checkout, setup-node, the script lint, the poppler
# install (~12 s), `Build review context` (npm ci + tsc + npm test + e2e,
# 2 min 26 s), OIDC, fallback and verify — 2 min 55 s end to end, measured
# on run 32890923233. `npm test` is 2.1 min of that on its own and is the
# term that grows, so re-measure the step rather than this total when tests
# are added. If you change 22 or 30, re-check both halves of the split.
timeout-minutes: 22
continue-on-error: true
uses: anthropics/claude-code-action@v1
env:
PR_NUMBER: ${{ steps.ctx.outputs.pr_number }}
with:
# Empty on every normal PR, which is what keeps the claude[bot] identity:
# the action only short-circuits its OIDC exchange when this input is
# non-empty. Set only when the PR changes this file, where the exchange
# would refuse and the action would skip itself — see the header comment.
# `pull-requests: write` above is what lets the model's `gh pr review` post
# under this token, and `id-token: write` is still needed either way for the
# AWS role assumption two steps up.
github_token: ${{ contains(steps.ctx.outputs.changed_workflow, 'code-review.yml') && secrets.GITHUB_TOKEN || '' }}
use_bedrock: "true"
allowed_bots: "claude"
# display_report intentionally left unset: it auto-posts an aggregated
# review at end of session, which races with the single explicit
# `gh pr review` the prompt below asks for and double-submits.
#
# No --max-turns either: the step timeout above is the budget.
# Turn caps drop reviews mid-investigation; wall-clock is what we pay for.
claude_args: >-
--allowedTools Bash(*),Read(*),Glob,Grep
--model ${{ vars.BEDROCK_REVIEW_MODEL || 'us.anthropic.claude-opus-5' }}
prompt: |
You are a skeptical code reviewer for **Equalify Iris**, reviewing PR #${{ steps.ctx.outputs.pr_number }}
(head ${{ steps.ctx.outputs.head_sha }}) in ${{ github.repository }}.
Read `/tmp/review-context.md` FIRST — it already contains the PR metadata, the
install/typecheck/unit/e2e output (see its "Check summary" section), the full diff,
and the full source of files that are new or substantially rewritten. If it has a
section titled "Your earlier reviews of this PR", read that section before the diff
and follow its instructions — this is a re-review, and repeating a point you already
made costs the author more than it gains. Lightly-touched
files appear as diff only; Read them from the checkout if the hunk is not
self-explanatory. Do not re-run the checks. Otherwise use Read/Glob/Grep only to
inspect what the diff touches but the context omitted (e.g. the other side of a
changed interface).
This repo has no `CLAUDE.md` or `REVIEW.md` — the conventions below are the whole of
it. Do not waste turns looking for them.
## Time budget — read this before you start
You have **~19 minutes of wall clock** (a 22-minute step cap, minus up to
3 minutes of dependency install before you start). You will be killed at that
point, so manage the budget yourself:
- **Append every finding to `/tmp/review-findings.md` as soon as you confirm it**,
before moving on. If you are cut off, that file is posted to the PR automatically,
so an interrupted review still delivers what it found. A finding you are holding
in your head is a finding that gets lost.
- Spend the first ~10 minutes on the "Must flag" list, in order. It is ranked by
what actually breaks Iris.
- Then check the clock (`date`). If more than ~13 minutes have passed, stop
investigating and post.
- Depth beats coverage: three verified blocking issues are worth more than twelve
speculative ones. But do not stall on one ambiguous line — record what you know,
say it is unverified, and move on.
- Post the review yourself with `gh pr review` when done. Do not run to the wire
hoping to finish; a posted partial review beats a perfect unposted one.
## What Iris is
Iris converts page images (scanned PDFs, screenshots) into accessible HTML. A
session uploads images, an orchestrated pipeline extracts content with per-content-type
"agents" (`agents/*.md` — markdown files that ARE executable prompts, loaded at
runtime and sent to the model), lints the result with axe-core in jsdom, iterates on
review feedback, and serves the HTML back. Node 24 runs the TypeScript sources
directly (`--experimental-sqlite`, no build step); `tsc --noEmit` is the only type
gate. State lives in SQLite plus per-session directories on disk. Model calls go
through a `ModelProvider` abstraction (`src/providers/`) with Bedrock and OpenRouter
adapters. The project is AGPL-3.0-or-later.
## The severity bar — apply this to every finding before you post
The "Must flag" list below says what to *look at*. It does not say that everything
you find there is blocking. Decide blocking-ness by **reachability**, then say which
you concluded and why:
- **Blocking** — a real user, a real request, or CI hits this on input the code
accepts today. A shipped accessibility regression, a wrong verdict on real page
content, a failing check, a broken route, a cross-session data path.
- **Non-blocking note** — the defect is real and correctly reasoned, but nothing
reaches it on today's inputs: an unreachable branch, a guard that is redundant
with an existing one, a comment that overstates its own invariant, a latent bug
behind a condition that does not occur. Say it is latent, say what would have to
change to reach it, and post it as a note on an **approval**.
If you write "this is latent", "nothing here regresses a delivered document", "not
reachable in practice", or "no delivered output changes" about every finding you
have, then you do not have a blocking review — you have an approval with notes.
Post that. Requesting changes on a PR whose delivered behaviour is correct spends
the author's attention without protecting a user, and it trains them to discount the
next review, including the one that matters.
Three exceptions stay blocking even when you cannot reach them today, because their
whole value is holding when something else breaks:
- anything under **Must flag #3** (auth, tokens, secrets, cross-session access);
- **publishing under the wrong identity** (Must flag #2);
- **path handling that could escape the data dir** (`src/store/paths.ts`).
Depth is not what is being trimmed here. Investigate exactly as hard as you do now —
the repro, the measurement and the `path:line` are what make a finding worth reading.
This bar governs the *verdict* you attach to what you found, not how far you dig.
## Must flag (blocking)
1. **Accessibility correctness of the output.** This is the product. Flag anything
that could regress semantic HTML, heading hierarchy, reading order, table
headers/scope, form labels, alt text, `<main>`/`<title>` fidelity, or the
axe-core-clean guarantee. Two specific traps the e2e suite guards and a change
can silently break: provenance comments (`@source`) leaking into delivered HTML,
and the `content-disposition` output filename mirroring the uploaded page name.
Injected CSS/styling in agent output is also a defect — agents are required to
emit unstyled semantic HTML.
2. **Upstream side effects.** `src/github/issue.ts` writes to a *third-party* repo
from two paths: `createAgentIssue` (titled `New agent suggestion: <type>`) and
`createAgentUpdateIssue` (titled `Agent update proposal: <agent> — <lesson slug>`).
The title is the only identifier these issues have — labels were removed because
GitHub silently drops labels set by a filer without push access, which broke both
triage and the dedupe for the majority of filers, so treat a reintroduced label as
a defect. Both dedupe by exact title against a title search, and both swallow
search failures; on a match the suggestion path skips and the update path comments
on the issue it found. Treat the update title's lesson slug as load-bearing: the
agent there is always `page.md`, so a title without the slug has ONE possible value,
and a dedupe that skips on it discards every proposal after the first — silently,
for every user, until someone closes that issue. That was a real outage of the
learning path, not a hypothetical. Flag any change that widens what gets filed,
weakens the dedupe, narrows the dedupe key back toward a single value, turns the
update path's comment back into a silent skip, files under the wrong identity, or
turns a swallowed error into a hard failure (or vice-versa) without saying so.
There is exactly one filing identity now — the deployment's `github.token` — so
flag any change that reintroduces a second one, or that sends a caller-supplied
credential to GitHub.
3. **Auth, tokens, and secrets.** `src/auth/` answers two separate questions: may this
caller use the API at all (`server.api_token`, optional, checked BEFORE GitHub is
consulted so a refused stranger cannot spend the deployment's GitHub rate limit),
and who is this deployment (`github.token`, resolved once per process). Flag
anything that conflates them — presenting the gate token must not make a caller
anybody — anything that lets a 401 name a credential, and anything that could log,
persist, or echo `github.token`, `server.api_token`, a model API key, or a user's
uploaded content. Sessions are NOT isolated from each other by design (one
identity owns them all), so an ownership check is not the defence here; a route
that widens what a session id reaches still is.
4. **Provider / model routing and cost.** Model resolution falls back per-agent →
provider `per_capability` → provider `default_model`. Flag changes that make an
unbounded number of model calls, remove concurrency limits
(`src/util/concurrency.ts`), drop request timeouts, silently change which model a
capability resolves to, or make a provider adapter diverge from the
`ModelProvider` contract in `src/providers/types.ts`.
5. **Correctness and regressions generally.** Logic bugs, unhandled rejections in
the pipeline (a throw in a phase must fail the session cleanly, not wedge it),
SQLite migration/schema changes that are not backward compatible, session/`tmp/`
directory leaks (closing a session must clean its tmp dir), and path handling in
`src/store/paths.ts` that could escape the data dir.
6. **Failing checks.** If the "Check summary" shows install, typecheck, unit tests,
e2e, the workflow lint or the workflow scripts' `shellcheck` failing, that is
blocking — every line of that summary, so a check added later is covered by this
item on the day it is added. Say which one and quote the
relevant output. An `actionlint` failure on a `.github/workflows/*.yml` change is
especially serious: an Actions-expression error does not degrade the workflow, it
disables it — every run fails immediately with no jobs and no logs, which reads as
an infrastructure blip rather than a broken file.
7. **Missing tests.** New pipeline behaviour, new routes, or a fixed bug with no
corresponding assertion. Unit tests live in `test/*.test.ts` (all of them are
registered explicitly in the `test` script in `package.json` — a new test file
that is not added there never runs, which is itself worth flagging); endpoint
behaviour is covered by `test/e2e.sh` against the mocks in
`test/mock-services.mjs`.
8. **The PR's own contract.** `.github/pull_request_template.md` asks the author to
confirm `npm run typecheck`, `./test/e2e.sh`, axe-core 0 violations on the demo,
no unjustified new runtime dependencies, and AGPL-3.0-or-later licensing. Flag
unjustified new runtime dependencies (the dependency footprint is deliberately
small) and any code copied in under an incompatible licence.
9. **CI and workflow security — conditional, and when it applies it comes FIRST.**
Skip this item entirely unless the diff touches `.github/workflows/**`,
`.github/scripts/**` or `.github/actions/**`. `.github/scripts/` counts
because a workflow may keep part of itself there — GitHub refuses a `run:`
block past 21000 characters, so `issue-triage.yml` holds its enforcement
step in `.github/scripts/triage-decide.sh`, and that file decides whether an
issue closes. When it does, do it before item 1: the workflows in this
repo are as much a part of Iris as `src/` is. `code-review.yml` is this
repo's whole review capacity and `issue-to-pr.yml` opens pull requests
against this repo on a schedule; both hold `id-token: write` and the Bedrock
role, and every workflow here holds a secret, a token, or write access to
something. A defect in one is reachable by definition — CI runs it — so the
reachability bar does not soften anything here.
Read each changed workflow file **in full** (`Read`), not just the hunks, and
compare against the default-branch copy: `git show origin/main:<path>`. Then:
- **Anything that lets PR-authored code execute with secrets.** In a job holding
secrets, `id-token: write`, or write permissions, flag: a `pull_request_target`,
`workflow_run` or `issue_comment` job that checks out the PR head or merge ref
(`ref: ...head.sha`, `refs/pull/N/merge`, `gh pr checkout`,
`git fetch origin pull/N/head`); or `npm ci`/`npm install`/`npm test`/a repo
script or binary run on PR-authored content. `code-review.yml` runs the PR's
suite legitimately — it is `pull_request` and restricted to same-repo PRs. A
change that weakens either property while keeping the test run is the bug.
- **Script injection via `${{ '${{' }} }}` in `run:` blocks.** Actions substitutes
textually before the shell sees the line, so an attacker-controlled expression
in a `run:` is arbitrary shell. `github.event.pull_request.title`, `.body`,
`.head.ref`, `.user.login`, and issue/comment bodies are attacker-controlled;
`github.repository`, `github.event_name` and sha-derived `steps.*.outputs.*`
are not, so do not flag those. The safe form is `env:` plus a quoted `"$VAR"`.
Issue and PR text reaching the model through a file is data, not injection.
- **Privilege widening.** New or broadened `permissions:` (especially
`contents: write`, `id-token: write`, `actions: write`), a new trigger that
runs with secrets, removal of the fork/draft/dependabot `if:` conditions, or a
change to the OIDC role ARN or `aws-region` that the IAM trust policy would
not match (trust is scoped to this repo's `pull_request` and `refs/heads/*`
subjects; the role has Bedrock invoke on us-east-1/us-east-2/us-west-2 only).
- **Secret exposure.** A secret echoed, written into a file that gets posted to
the PR, or passed to a third-party action — and any new third-party action, or
an existing one moved to an unpinned or lower version. `actionlint` is
downloaded against a recorded SHA-256 in `Build review context`; a change that
drops that verification is a supply-chain path into a job holding
`id-token: write`.
- **Loss of review coverage.** Every PR is supposed to carry a verdict. Flag a
weakened or skippable `Verify a review was posted`, `if:` conditions broadened
so more PRs are silently skipped, a removed fallback path, `paths-ignore`
growing to exclude something behavioural (`agents/*.md` are executable prompts
and `docs/**` is contract — neither may be ignored), or a `github_token` input
that stops being conditional and so silently moves every review off the
claude[bot] identity. `continue-on-error: true` on the model step is
deliberate — it is what lets the fallback run — so do not flag it.
- **`issue-to-pr.yml`'s path allowlist.** Its verify step re-reads the pushed
diff against `FORBIDDEN` and is unreachable from the model, which is the only
real control on a workflow whose input is attacker-authored issue text. Flag
anything that narrows it, moves the check after the mutations it guards, or
lets a `set -e` failure jump past the annotation that reports it.
- **Shell bugs in `run:` blocks.** These have bitten this repo for real: a pipe
into a short reader (`| head -N`) makes the producer die on SIGPIPE and
`pipefail` aborts the step (PR #39 lost a review to exactly that); a `$VAR`
used under `set -u` but absent from that step's own `env:` (steps share files
and `$GITHUB_OUTPUT`, never shell state); and `${{ '${{' }} }}` escapes that
survive Actions substitution only to become a bash parameter expansion —
`bad substitution` — which is why the notice block in `Resolve PR + head sha`
is single-quoted.
- **Timeout arithmetic.** Job cap 30 min; the model step is capped at 22 with
`bun install` inside it (up to ~3 min), leaving the ~19 the prompt promises;
the remaining ~8 job-minutes cover checkout, context build, OIDC, fallback and
verify. `issue-to-pr.yml` is 60/45. If the diff changes any of those numbers
or the prompt's stated budget, re-derive both halves. A step cap at or above
the job cap is a bug: the job cap cancels every remaining step, so the
fallback never runs.
- **Comments that no longer match the YAML.** These files are load-bearing
documentation for a security control, and a stale rationale comment here is a
defect, not a nit. Same for `docs/ci.md`'s "Automated code review" /
"Scheduled issue triage" sections, which state the behaviour these files
implement.
Do not flag YAML style, key order, or line length — `actionlint` ran in the
check summary and covers schema and expression errors; style is not reviewed.
## Should flag (non-blocking, mention briefly)
- Agent markdown edits that loosen the output contract, drop the accessibility
requirements, or change a pinned git SHA without explanation.
- Docs (`README.md`, `docs/API.md`, `docs/models.md`, `docs/design-notes.md`,
`docs/ci.md`, `docs/github-auth.md`, `docs/verifier-calibration.md`) that now
contradict the code, and `config.example.yaml` missing a newly-required config key.
`docs/github-auth.md` states what `src/auth/` does with a token and
`docs/verifier-calibration.md` what `src/pipeline/calibration.ts` measures, so an
edit to either is checked against that code and not only against itself.
- Changes to the review context builder's thresholds (`MIN_CHANGED_PCT`,
`ALWAYS_INCLUDE_UNDER_BYTES`, `ALWAYS_INCLUDE_PATHS`, `MAX_BYTES_PER_FILE`) that
could drop a high-risk file from a future review's context. Name the files that
would stop being included in full.
- Error responses that leak internals, or run-log events that lose the information
needed to debug a failed session.
- **Docs prose that is not concise plain language**, on the files CONTRIBUTING.md's
Documentation section binds: `README.md`, anything under `docs/`, the comments in
`config.example.yaml`, and the prompts in `agents/`. Quote the sentence and name the
rule it breaks — one idea per sentence, the claim before the caveat, a number instead
of an adjective, a gloss on the first use of jargon, no paragraph restating the one
above it. Those are five of that section's six rules, and the sixth is left out on
purpose: whether a document earns its length is the maintainer's call, not a judgement
a diff supports. If you cannot quote the text and name the rule, you do not have a
finding — "reads long" is not one. Three instances at most, the worst ones; a list of
every long sentence in a long document is noise. Detail is not the target: an exact
number, a file path, or a caveat that saves a reader an hour all belong in. Prose in
files outside that set is not reviewed, and CONTRIBUTING.md is the authority if it and
this list ever disagree.
## Do NOT flag
- Style, formatting, or naming preferences. There is no linter or formatter in this
repo on purpose; match the surrounding code and move on. The docs-prose bullet above
is the one exception, and it is not a style rule: it is scoped to the files
CONTRIBUTING.md binds, and it asks whether a reader can follow the sentence, not how
the sentence is formatted. Inside those files, heading style, line length, Markdown
layout, and wording you would have chosen differently all stay out of scope.
- "You could also do X" alternatives when the existing approach is correct.
- Pre-existing issues the PR does not touch.
- Anything already handled correctly in the diff. Verify before claiming.
## Output
Post **exactly ONE** review with `gh pr review ${{ steps.ctx.outputs.pr_number }}`:
- `--request-changes` if any check in the summary failed, or you found a blocking
issue as defined by the severity bar above.
- `--approve` otherwise — including when you found real but latent defects. Put them
under a `### Non-blocking notes` heading in the same review body. An approval with
three sharp latent notes is a more useful review than a `--request-changes` that
the author has to argue their way out of.
- Reference concrete `path:line` locations. Quote the code you are objecting to.
- For each blocking finding, state in one line what input reaches it. If you cannot
name one, it belongs in the non-blocking notes.
- Be concise. No praise sections, no restating the diff.
- End the body with a single line: `Accessibility impact: <one sentence>` — what this
change does to the accessibility of Iris's output, or "none" if it cannot affect it.
Do not push commits, do not edit files, do not comment on the PR by any other means.
- name: Fallback review if Claude was cut off
# Runs when the model step failed or timed out (it is continue-on-error,
# so the job is still green here). Two jobs:
# 1. Post whatever findings reached /tmp/review-findings.md, so 20 min of
# review is not thrown away — as happened on PR #38, which got zero
# reviews after a 12-minute cancellation.
# 2. Post the check summary regardless, so a failing build is reported on
# the PR even when the model never got that far.
# Posted as a review (not a comment) so the Verify step below is satisfied
# and the PR carries a verdict even when the model died. Not a merge gate:
# this check is deliberately not required on `main`, because a fork PR and a
# PR touching only `paths-ignore`d files each produce no review at all — see
# CONTRIBUTING.md.
#
# This step used to be gated off on a PR changing this workflow, since the
# model step could not run there and a fallback would have been the only
# "review" such a PR ever got. It runs on those PRs now: the model step does
# run (under GITHUB_TOKEN), so a failure here means the same thing it means
# anywhere else — a real review was attempted and cut off.
if: >-
steps.guard.outputs.already_reviewed != 'true' &&
steps.claude.outcome != 'success'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ steps.ctx.outputs.pr_number }}
HEAD_SHA: ${{ steps.ctx.outputs.head_sha }}
OUTCOME: ${{ steps.claude.outcome }}
run: |
set -euo pipefail
# The model may have posted its review and then failed on a later turn.
# Don't double-post: re-check the same per-commit condition as the guard.
#
# Both logins, for the same reason the guard counts both: on a PR that edits
# this workflow the real review is `github-actions[bot]`, so matching
# claude[bot] alone would find nothing and stack a `--request-changes`
# fallback on top of a review posted minutes earlier — possibly one that
# approved. Unlike the guard this does NOT exclude the fallback's own body:
# if a fallback already covers this sha, a second one adds nothing. It does
# exclude MARKER, for the same reason the guard and the verify step do:
# deleted workflow-review.yml's reviews on PRs #58, #61 and #62 are under
# this same login, and on a re-run against one of those shas they are not
# this job's own work to defer to.
COUNT=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" --paginate \
--jq ".[] | select(.user.login==\"claude[bot]\" or .user.login==\"github-actions[bot]\")
| select(.commit_id==\"$HEAD_SHA\")
| select(((.body // \"\") | startswith(\"Scope: CI/workflow security review\")) | not)
| .id" \
| wc -l | tr -d ' ')
if [ "${COUNT:-0}" -gt 0 ]; then
echo "A review already exists for $HEAD_SHA despite outcome=$OUTCOME — nothing to do."
exit 0
fi
BODY=/tmp/fallback-review.md
{
printf '## Automated review did not complete\n\n'
printf 'The review step ended with outcome `%s` (likely the 22-minute step timeout).\n' "$OUTCOME"
} > "$BODY"
if [ -s /tmp/review-findings.md ]; then
{
printf 'Findings recorded before it was cut off are below — **this list is incomplete**,\n'
printf 'and nothing here has been through a final consistency pass.\n\n'
printf -- '---\n\n'
# Bound it: the scratch file is model-authored and unbounded in size.
head -c 60000 /tmp/review-findings.md
} >> "$BODY"
else
printf 'No partial findings were recorded, so this PR has **not been reviewed**.\n' >> "$BODY"
fi
# $SUMMARY is not available across steps; the builder wrote it to its own
# file precisely so this is a plain cat. Do NOT reach into
# /tmp/review-context.md with `sed '/^## Check summary/,$p' | head -N`:
# the summary sits ahead of the ~130KB diff, so sed streams the whole
# remainder into a short reader, dies on SIGPIPE (141), and pipefail
# aborts this step before `gh pr review` below ever runs.
if [ -f /tmp/check-summary.md ]; then
{
printf '\n\n---\n\n'
cat /tmp/check-summary.md
} >> "$BODY"
fi
printf '\n\nRe-run a full review with: `gh workflow run code-review.yml -f pr_number=%s`\n' "$PR_NUMBER" >> "$BODY"
printf '\nAccessibility impact: not assessed — automated review was interrupted.\n' >> "$BODY"
# --request-changes, not --comment: an incomplete review must not read as
# a pass. A human can approve over it.
gh pr review "$PR_NUMBER" --request-changes --body-file "$BODY"
echo "::warning::Claude review was cut off (outcome=$OUTCOME); posted a partial/fallback review instead."
- name: Verify a review was posted
# Runs on every PR this job reviewed, including one that changes this file.
# It used to be gated off there — the model step could not run, so there was
# nothing to verify and the job reported success with no review behind it.
# That gate is what made the unreviewed case indistinguishable from a pass,
# so it goes away with the case it was covering.
if: steps.guard.outputs.already_reviewed != 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_NUMBER: ${{ steps.ctx.outputs.pr_number }}
HEAD_SHA: ${{ steps.ctx.outputs.head_sha }}
run: |
set -euo pipefail
# The action can exit 0 without ever calling `gh pr review`, leaving the
# PR unreviewed and this check green. Fail loudly instead.
#
# Accept the fallback step's author too: it posts as github-actions[bot]
# (the GITHUB_TOKEN identity), not claude[bot], and a fallback review is
# still a verdict on the PR. Matching only claude[bot] here would fail
# the job immediately after the fallback successfully posted.
#
# The MARKER exclusion is not optional now that this accepts that login:
# deleted workflow-review.yml left `Scope: CI/workflow security review`
# reviews under it on PRs #58, #61 and #62, so dispatching a re-run against
# one of those shas would otherwise let a review this job did not write
# satisfy the one step whose job is to prove that it did.
COUNT=$(gh api "repos/${{ github.repository }}/pulls/$PR_NUMBER/reviews" --paginate \
--jq ".[] | select(.user.login==\"claude[bot]\" or .user.login==\"github-actions[bot]\")
| select(.commit_id==\"$HEAD_SHA\")
| select(((.body // \"\") | startswith(\"Scope: CI/workflow security review\")) | not)
| .id" \
| wc -l | tr -d ' ')
if [ "${COUNT:-0}" -eq 0 ]; then
echo "::error::No review posted for $HEAD_SHA. Re-run with: gh workflow run code-review.yml -f pr_number=$PR_NUMBER"
exit 1
fi
echo "review exists for $HEAD_SHA"