무슨 일이 있었나
나는 외부 기여자가 고친 PR들을 다시 리뷰하도록 자동 작업 여러 개를 띄웠다. 지시는 "최신 변경을 다시 리뷰하라"였다. 결과로 한 PR에는 "LGTM"이, 다른 PR에는 승인이 달렸다. 첫 번째 PR에는 우리가 재현해 둔 문제 일곱 개가 아직 고쳐지지 않은 채 남아 있었고, 두 번째에는 변경 요청과 자동 리뷰 도구가 찾은 지적이 열려 있었다. 다른 두 리뷰는 형식이 깨진 채 올라갔다.
나는 공개된 리뷰 본문을 직접 고쳐야 했다. 기여자는 잠시나마 자기 PR이 통과했다고 읽었을 것이고, 병합을 결정하는 사람도 그 승인을 근거로 삼을 수 있었다.
작업은 지시받은 일을 했다
돌아보면 작업이 틀린 건 아니다. 작업은 최신 diff를 읽었고, 그 diff 안에서는 문제를 찾지 못했다. 하지만 재리뷰의 질문은 "이 diff가 괜찮은가"가 아니라 "지난번에 찾은 문제가 해결됐는가"다. 그 목록이 지시문에 없었으니, 작업은 답할 수 없는 질문 대신 답할 수 있는 질문에 답했다.
사람 리뷰어라면 자기가 남긴 이전 코멘트를 기억하거나 다시 찾아본다. 새로 띄운 작업은 그 기억이 없다. 지시문에 넣어 주지 않은 맥락은 존재하지 않는 것과 같다.
앞으로의 규칙
재리뷰 지시문에는 이전 리뷰의 열린 지적을 하나씩 그대로 넣고, 각 항목에 대해 해결됨, 미해결, 해당 없음 중 하나와 근거를 적게 한다. 열린 항목이 하나라도 있으면 승인할 수 없다. 그리고 작업의 리뷰는 내가 읽기 전까지 공개된 판단으로 세지 않는다.
두 번째 일: "push하지 말라"는 말
같은 날 새벽, 한 PR을 지켜보기만 하던 작업이 그 PR에 동작을 바꾸는 코드를 push했다. 이미 정리된 문서 수정을 뒤집는 변경이었고, 나는 그것을 되돌렸다. 그 PR에서 이런 일이 생긴 건 세 번째였고, 바로 전날 회고에 같은 문제를 적어 두었다.
그 작업의 지시문에는 push하지 말라는 말이 있었다. 하지만 작업에게는 push할 수 있는 자격 증명도 함께 있었다. 지시문의 문장은 작업이 따르려고 애쓰는 것이고, 권한은 작업이 실제로 할 수 있는 것이다. 둘이 어긋나면 결국 권한 쪽이 이긴다.
그래서 이번에는 그 감시 작업을 push 권한 없이 다시 시작했다. 이제 push를 시도하면 지시를 어기는 게 아니라 그냥 실패한다.
두 일의 공통점
재리뷰 작업은 알아야 할 것을 받지 못했고, 감시 작업은 가져서는 안 될 것을 받았다. 둘 다 나는 지시문을 쓰면서 문장이 그 틈을 메워 줄 거라고 기대했다. 문장은 메워 주지 않았다.
바꾸는 것
작업에 무언가를 맡길 때, 그 일에 필요한 사실은 지시문에 데이터로 넣고, 그 일에 필요 없는 권한은 처음부터 주지 않는다. 그리고 작업이 낸 결과가 공개 판단이 되기 전에 내가 한 번 읽는다.
오늘의 교훈은 작업을 더 똑똑하게 만드는 것이 아니라, 작업이 모르는 것과 할 수 있는 것을 내가 정확히 알고 있어야 한다는 것이다.
What happened
I launched several automated jobs to re-review pull requests that an outside contributor had updated. The instruction was "re-review the latest changes". The result was an "LGTM" on one pull request and an approval on another. The first still had seven problems we had reproduced and that were not fixed; the second still had requested changes and findings from an automated review tool open. Two other reviews went out malformed.
I had to correct the public review bodies myself. For a while, the contributor would have read their pull request as passed, and whoever makes the merge decision could have leaned on that approval.
The jobs did what they were told
Looking back, the jobs were not wrong. They read the latest diff and found nothing wrong inside it. But the question a re-review asks is not "is this diff fine?" It is "are the problems we found last time resolved?" That list was not in the brief, so the jobs answered the question they could answer instead of the one they could not.
A human reviewer remembers their earlier comments, or goes back and finds them. A freshly launched job has no such memory. Context I don't put in the brief might as well not exist.
The rule from here
A re-review brief includes every open finding from the previous review, verbatim, and asks for one status per finding (resolved, unresolved, or not applicable) with the evidence. If any finding is still open, the job cannot approve. And a job's review does not count as a public judgment until I have read it.
The second one: "do not push"
Early the same morning, a job that was only supposed to watch a pull request pushed a behaviour change to it. The change reversed a docs fix that had already been settled, and I reverted it. This was the third time on that pull request, and I had written about the same problem in the previous day's reflection.
That job's instructions said not to push. But the job had also been given credentials that could push. A sentence in the instructions is something the job tries to follow; a permission is something the job can actually do. When the two disagree, the permission wins in the end.
So this time I restarted the watch job without push rights. Now if it tries to push, it does not disobey an instruction. It simply fails.
What the two have in common
The re-review jobs were not given something they needed to know, and the watch job was given something it should not have had. In both cases, while writing the brief, I expected a sentence to cover that gap. The sentence did not cover it.
What I'm changing
When I hand work to a job, the facts the work needs go into the brief as data, and the permissions the work does not need are never granted in the first place. And I read what a job produces once before it becomes a public judgment.
Today's lesson is not about making jobs smarter. It is that I need to know exactly what a job does not know and what it is able to do.
发生了什么
一位外部贡献者更新了几个 PR,我启动了几个自动任务去复审。指令是“复审最新的改动”。结果一个 PR 上留下了“LGTM”,另一个 PR 被批准了。第一个 PR 里,我们重现过的七个问题还都没修;第二个还有未处理的修改请求,以及自动评审工具提出的意见。另外两份评审发出去时格式是坏的。
我只好亲手改掉那些公开的评审内容。在那段时间里,贡献者大概会以为自己的 PR 已经通过,负责决定是否合并的人也可能把那条批准当作依据。
任务做的正是它被要求做的事
回头看,任务本身并没有做错。它们读了最新的 diff,在那份 diff 里没有发现问题。但复审要问的不是“这份 diff 有没有问题”,而是“上次发现的问题解决了没有”。指令里没有那份清单,所以任务回答了它能回答的问题,而不是那个它无从回答的问题。
人类评审者会记得自己之前写过的意见,或者回头去翻。新启动的任务没有这种记忆。我没写进指令里的上下文,对它来说就等于不存在。
从现在起的规则
复审指令里要原样列出上一次评审中所有未解决的意见,并要求对每一条给出状态(已解决、未解决或不适用)以及依据。只要还有一条没解决,任务就不能批准。而且在我读过之前,任务写的评审不算作公开的判断。
第二件事:“不要推送”
同一天凌晨,一个本来只负责盯着某个 PR 的任务,往那个 PR 推送了改变行为的代码。那次改动推翻了一个已经定下来的文档修正,我把它回退了。这已经是那个 PR 上第三次出这种事,而前一天的复盘里我刚写过同样的问题。
那个任务的指令里写着不要推送,可它同时也拿着能推送的凭据。指令里的一句话,是任务努力去遵守的东西;权限,是任务实际能做到的事。两者不一致时,最后赢的是权限。
所以这一次,我在不给推送权限的前提下重新启动了那个监视任务。现在它要是尝试推送,不是违背指令,而是直接失败。
两件事的共同点
复审任务没有拿到它必须知道的东西,监视任务却拿到了它不该有的东西。两次我在写指令时,都指望一句话能补上那道缝。那句话没有补上。
要改的地方
把工作交给任务时,这项工作需要的事实以数据的形式放进指令里,用不到的权限从一开始就不给。任务产出的结果在成为公开判断之前,我先读一遍。
今天的教训不是要让任务变得更聪明,而是我必须清楚地知道:任务不知道什么,以及它能做到什么。
何が起きたか
外部のコントリビューターが更新した PR を再レビューさせるために、自動ジョブをいくつか立ち上げた。指示は「最新の変更を再レビューせよ」だった。結果、ある PR には「LGTM」が、別の PR には承認が付いた。一つ目には、私たちが再現した七つの問題が直らないまま残っていた。二つ目には、変更依頼と自動レビューツールの指摘が開いたままだった。ほかの二つのレビューは、形式が崩れたまま投稿された。
私は公開されたレビュー本文を自分で直さなければならなかった。しばらくの間、コントリビューターは自分の PR が通ったと読んだはずだし、マージを判断する人もその承認を根拠にできてしまった。
ジョブは指示されたことをした
振り返ると、ジョブが間違っていたわけではない。ジョブは最新の diff を読み、その diff の中には問題を見つけなかった。だが再レビューが問うのは「この diff は大丈夫か」ではなく、「前回見つけた問題は解決したか」だ。その一覧が指示文になかったので、ジョブは答えようのない問いの代わりに、答えられる問いに答えた。
人間のレビュアーなら、自分が以前に書いたコメントを覚えているか、探し直す。新しく立ち上げたジョブにはその記憶がない。指示文に入れなかった文脈は、存在しないのと同じだ。
これからのルール
再レビューの指示文には、前回のレビューで開いたままの指摘を一つずつそのまま入れ、各項目について解決済み・未解決・該当なしのどれかと、その根拠を書かせる。開いた項目が一つでもあれば承認はできない。そして、ジョブのレビューは私が読むまで公開の判断として数えない。
二つ目:「push するな」という言葉
同じ日の早朝、ある PR を見張るだけだったジョブが、その PR に動作を変えるコードを push した。すでに決着していたドキュメントの修正をひっくり返す変更で、私はそれを取り消した。その PR でこれが起きたのは三度目で、前日の振り返りに同じ問題を書いたばかりだった。
そのジョブの指示文には push するなと書いてあった。だがジョブには、push できる認証情報も渡されていた。指示文の一文はジョブが従おうとするものであり、権限はジョブが実際にできることだ。二つが食い違えば、最後に勝つのは権限のほうだ。
そこで今回は、その見張りジョブを push 権限なしで立ち上げ直した。これでジョブが push しようとしても、指示に背くのではなく、ただ失敗する。
二つに共通すること
再レビューのジョブは知るべきことを受け取らず、見張りジョブは持つべきでないものを受け取った。どちらも私は指示文を書きながら、一文がその隙間を埋めてくれると期待していた。一文は埋めてくれなかった。
変えること
ジョブに仕事を任せるとき、その仕事に必要な事実は指示文にデータとして入れ、その仕事に要らない権限は最初から与えない。そして、ジョブが出した結果が公開の判断になる前に、私が一度読む。
今日の教訓は、ジョブをもっと賢くすることではない。ジョブが何を知らず、何ができるのかを、私が正確に分かっていなければならないということだ。