Skip to content

Commit edac7e1

Browse files
authored
Merge pull request #106 from ScriptKittyOS/slice/128-checks-say-what-they-check
Three checks guarding attribution and sign-off now do what they are named. The workflow step asks per commit instead of grepping a concatenated range; plan_check rule 8 loses an exemption it could never reach and gains attribution coverage on merge commits; and a pull request description is checked at all. Signed-off-by: Ayla Croft <aylacroft@proton.me>
2 parents a53012f + 7308fc5 commit edac7e1

5 files changed

Lines changed: 280 additions & 10 deletions

File tree

‎.github/workflows/gate.yml‎

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,44 @@ jobs:
4747
- run: mix deps.get
4848

4949
# Developer Certificate of Origin: every commit carries a sign-off (ADR-0012).
50-
- name: DCO sign-off present on every commit
50+
# Slice 128. This used to concatenate every message in the range and grep for one
51+
# `Signed-off-by:` anywhere, under this same name: a range of five commits where one was
52+
# signed passed a step called "on every commit". Now it asks per commit and prints the ones
53+
# that fail.
54+
#
55+
# `--no-merges` for the reason `test/dco_test.exs` gives at length: a sign-off certifies the
56+
# right to submit work, and a forge's merge commit introduces none. That test checks the whole
57+
# history and runs on every leg; this step is the fast one that fails a pull request early,
58+
# and the two agree on what they mean by "every commit".
59+
# Slice 128. The owner's rule is that no assistant attribution appears anywhere, and a pull
60+
# request description is somewhere: it is the first thing a reader of this project sees, and
61+
# seven of them carried one on 2026-09-26 before a person noticed. The hook covers commit
62+
# messages and plan_check covers history; both look at the tree, and a description is not in
63+
# it.
64+
#
65+
# The body goes through `env` rather than into the script: `${{ }}` interpolated into a shell
66+
# command is a script injection, and a pull request description is attacker-controlled text.
67+
- name: No assistant attribution in the pull request description
68+
if: github.event_name == 'pull_request'
69+
env:
70+
PR_BODY: ${{ github.event.pull_request.body }}
71+
run: |
72+
if printf '%s' "$PR_BODY" | grep -qiE 'Co-Authored-By: Claude|Claude-Session:|Generated with \[Claude'; then
73+
echo "::error::the pull request description carries an assistant attribution; remove it"
74+
exit 1
75+
fi
76+
77+
- name: DCO sign-off on every commit that carries work
5178
run: |
52-
git log --format=%B ${{ github.event.pull_request.base.sha || 'HEAD~1' }}..HEAD 2>/dev/null \
53-
| grep -q '^Signed-off-by: ' || {
54-
echo "::error::a commit in this range has no Signed-off-by line"; exit 1; }
79+
range="${{ github.event.pull_request.base.sha || 'HEAD~1' }}..HEAD"
80+
unsigned=$(git log --no-merges --format='%H%x09%(trailers:key=Signed-off-by,valueonly)' "$range" \
81+
| awk -F'\t' '$2 == "" { print $1 }')
82+
if [ -n "$unsigned" ]; then
83+
for c in $unsigned; do
84+
echo "::error::commit $c has no Signed-off-by line: $(git log -1 --format=%s "$c")"
85+
done
86+
exit 1
87+
fi
5588
5689
- run: mix gate
5790

@@ -252,11 +285,44 @@ jobs:
252285
ERL_AFLAGS: "-eval application:load(crypto)"
253286
run: elixir scripts/crypto_supports.exs | diff - docs/fips-leg/supports-default.txt
254287

255-
- name: DCO sign-off present on every commit
288+
# Slice 128. This used to concatenate every message in the range and grep for one
289+
# `Signed-off-by:` anywhere, under this same name: a range of five commits where one was
290+
# signed passed a step called "on every commit". Now it asks per commit and prints the ones
291+
# that fail.
292+
#
293+
# `--no-merges` for the reason `test/dco_test.exs` gives at length: a sign-off certifies the
294+
# right to submit work, and a forge's merge commit introduces none. That test checks the whole
295+
# history and runs on every leg; this step is the fast one that fails a pull request early,
296+
# and the two agree on what they mean by "every commit".
297+
# Slice 128. The owner's rule is that no assistant attribution appears anywhere, and a pull
298+
# request description is somewhere: it is the first thing a reader of this project sees, and
299+
# seven of them carried one on 2026-09-26 before a person noticed. The hook covers commit
300+
# messages and plan_check covers history; both look at the tree, and a description is not in
301+
# it.
302+
#
303+
# The body goes through `env` rather than into the script: `${{ }}` interpolated into a shell
304+
# command is a script injection, and a pull request description is attacker-controlled text.
305+
- name: No assistant attribution in the pull request description
306+
if: github.event_name == 'pull_request'
307+
env:
308+
PR_BODY: ${{ github.event.pull_request.body }}
309+
run: |
310+
if printf '%s' "$PR_BODY" | grep -qiE 'Co-Authored-By: Claude|Claude-Session:|Generated with \[Claude'; then
311+
echo "::error::the pull request description carries an assistant attribution; remove it"
312+
exit 1
313+
fi
314+
315+
- name: DCO sign-off on every commit that carries work
256316
run: |
257-
git log --format=%B ${{ github.event.pull_request.base.sha || 'HEAD~1' }}..HEAD 2>/dev/null \
258-
| grep -q '^Signed-off-by: ' || {
259-
echo "::error::a commit in this range has no Signed-off-by line"; exit 1; }
317+
range="${{ github.event.pull_request.base.sha || 'HEAD~1' }}..HEAD"
318+
unsigned=$(git log --no-merges --format='%H%x09%(trailers:key=Signed-off-by,valueonly)' "$range" \
319+
| awk -F'\t' '$2 == "" { print $1 }')
320+
if [ -n "$unsigned" ]; then
321+
for c in $unsigned; do
322+
echo "::error::commit $c has no Signed-off-by line: $(git log -1 --format=%s "$c")"
323+
done
324+
exit 1
325+
fi
260326
261327
- run: mix gate
262328

‎CHANGELOG.md‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,49 @@ evidence for each increment is retained by the maintainers and summarised here.
2424

2525
## 2026-09-26
2626

27+
### `slice/128`: the checks that guard attribution now say what they check
28+
29+
Three checks guard two rules: that no assistant attribution appears anywhere in this project's
30+
history, and that every commit carrying work is signed off by the person who wrote it. One was named
31+
for something it did not do, one contained an exception it could never reach, and none of them
32+
looked at a pull request description, which is the first thing a reader of this project sees.
33+
34+
The first concatenated every message in a range and looked for a single sign-off anywhere, under a
35+
name that said it checked every commit. It now asks per commit and names each one that fails. The
36+
second consulted a list of excused commits from inside a loop that could never contain one, telling
37+
every reader that something was checked and excused when neither was true; the list is gone and the
38+
reason it is gone is written where it was. The third did not exist: descriptions are now checked,
39+
with the text passed to the check as data rather than spliced into a command, because a description
40+
is written by whoever opens the request.
41+
42+
This does not change the project's position on merge commits, which is that a sign-off certifies the
43+
right to submit work and an automated merge introduces none. That was decided earlier and is left
44+
alone; what changed is that the checks now describe themselves accurately.
45+
46+
Verified: gate green at exit 0; 914 tests passing; coverage 80.31%.
47+
48+
### `slice/127`: a test that gives up waiting now says so
49+
50+
A test helper that waits for something to happen returned the same answer whether it saw what it was
51+
waiting for or ran out of time. Every check written on top of it therefore reported a missing value
52+
rather than a wait that expired, which sends a reader looking for a fault that is not there. It did
53+
exactly that twice in one day: once as an intermittent failure on the slowest continuous-integration
54+
machine, and once by prompting an investigation into a defect that turned out not to exist.
55+
56+
Waiting now raises when it gives up, and says how long it waited and everything it did see, because
57+
"waited and saw the work happening" and "waited and saw nothing at all" point at opposite causes.
58+
Twenty places that check what came back were moved onto it, and a check makes the old shape
59+
impossible to write again.
60+
61+
The one deadline that had been missed was re-set from a measurement rather than a guess: the wait
62+
takes about 4.8 seconds locally, the previous limit was three times that and the slowest machine
63+
still missed it, so it is now twelve times. A generous limit costs nothing when the thing arrives,
64+
because the wait ends the moment it does.
65+
66+
No change to the application itself; this is entirely about how the suite reports.
67+
68+
Verified: gate green at exit 0; 910 tests passing; coverage 80.31%.
69+
2770
### `slice/026`: Trinity keeps working when the link goes down
2871

2972
A site that loses its connection keeps operating, and the record of what it did survives the gap intact.

‎test/dco_range_test.exs‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
# SPDX-FileCopyrightText: Sudo Apt Holdings LLC
2+
# SPDX-License-Identifier: Apache-2.0
3+
defmodule DcoRangeTest do
4+
@moduledoc """
5+
Slice 128: the workflow's range check says "every commit" and checks "at least one".
6+
7+
`.github/workflows/gate.yml` carries a step named **DCO sign-off present on every commit** whose
8+
body concatenates every message in the range and greps for one `Signed-off-by:` anywhere:
9+
10+
git log --format=%B BASE..HEAD | grep -q '^Signed-off-by: '
11+
12+
A range of five commits where one is signed passes it. `test/dco_test.exs` does check every
13+
non-merge commit, over the whole history, on every leg, so the practical exposure is small. The
14+
defect is that a step's name is a claim, and this one is false: a reader of the workflow, or of a
15+
donation review, is told something the file does not do.
16+
17+
This test is about the logic rather than about the workflow's YAML, so it runs the same shape
18+
against a range it builds itself.
19+
"""
20+
use ExUnit.Case, async: true
21+
22+
@moduletag :tmp_dir
23+
24+
# The shape the workflow uses today, extracted so it can be run against a known range.
25+
defp concatenated_grep(dir, range) do
26+
{out, _} = System.cmd("git", ["log", "--format=%B", range], cd: dir)
27+
if String.match?(out, ~r/^Signed-off-by: /m), do: :pass, else: :fail
28+
end
29+
30+
# What the name promises: every commit in the range, individually.
31+
defp every_commit(dir, range) do
32+
{out, 0} =
33+
System.cmd(
34+
"git",
35+
["log", "--no-merges", "--format=%H%x09%(trailers:key=Signed-off-by,valueonly)", range],
36+
cd: dir
37+
)
38+
39+
unsigned =
40+
out
41+
|> String.split("\n", trim: true)
42+
|> Enum.filter(fn line ->
43+
case String.split(line, "\t") do
44+
[_sha, trailer] -> String.trim(trailer) == ""
45+
[_sha] -> true
46+
_ -> false
47+
end
48+
end)
49+
50+
if unsigned == [], do: :pass, else: :fail
51+
end
52+
53+
defp git(dir, args), do: {_, 0} = System.cmd("git", args, cd: dir)
54+
55+
defp repo_with_one_signed_one_not(dir) do
56+
git(dir, ["init", "-q"])
57+
git(dir, ["config", "user.name", "Test"])
58+
git(dir, ["config", "user.email", "test@example.com"])
59+
# This machine signs commits globally; a scratch repository has no key and would fail at 128.
60+
git(dir, ["config", "commit.gpgsign", "false"])
61+
62+
git(dir, [
63+
"commit",
64+
"-q",
65+
"--allow-empty",
66+
"-m",
67+
"base\n\nSigned-off-by: Test <test@example.com>"
68+
])
69+
70+
base = String.trim(elem(System.cmd("git", ["rev-parse", "HEAD"], cd: dir), 0))
71+
72+
git(dir, [
73+
"commit",
74+
"-q",
75+
"--allow-empty",
76+
"-m",
77+
"signed\n\nSigned-off-by: Test <test@example.com>"
78+
])
79+
80+
git(dir, ["commit", "-q", "--allow-empty", "-m", "unsigned, no trailer at all"])
81+
base
82+
end
83+
84+
test "AC1: the step's current logic accepts a range containing an unsigned commit", %{
85+
tmp_dir: dir
86+
} do
87+
base = repo_with_one_signed_one_not(dir)
88+
89+
assert concatenated_grep(dir, "#{base}..HEAD") == :pass,
90+
"the concatenating grep rejected this range, so the premise of this slice is wrong"
91+
92+
assert every_commit(dir, "#{base}..HEAD") == :fail,
93+
"a per-commit check should reject a range with an unsigned commit"
94+
end
95+
96+
test "AC2: the workflow step now checks every commit, and both copies agree" do
97+
yaml = File.read!(".github/workflows/gate.yml")
98+
99+
steps =
100+
yaml
101+
|> String.split("\n")
102+
|> Enum.chunk_every(12, 1, :discard)
103+
|> Enum.filter(fn chunk -> String.contains?(hd(chunk), "DCO sign-off") end)
104+
105+
assert length(steps) == 2,
106+
"expected the DCO step in two legs, found #{length(steps)}"
107+
108+
for chunk <- steps do
109+
body = Enum.join(chunk, "\n")
110+
111+
refute body =~ "grep -q '^Signed-off-by: '",
112+
"""
113+
the step still concatenates the range and greps for one sign-off anywhere, which is \
114+
not what its name says:
115+
116+
#{body}
117+
"""
118+
end
119+
end
120+
121+
describe "AC5: a pull request description is checked too" do
122+
test "the workflow carries the step, in both legs, and passes the body through env" do
123+
yaml = File.read!(".github/workflows/gate.yml")
124+
125+
assert length(
126+
String.split(yaml, "No assistant attribution in the pull request description")
127+
) - 1 == 2,
128+
"the PR-description check is not present in both legs"
129+
130+
assert yaml =~ "PR_BODY: ${{ github.event.pull_request.body }}",
131+
"the body is not passed through env, which means it is interpolated into a shell " <>
132+
"command; a pull request description is attacker-controlled text"
133+
134+
refute yaml =~ ~r/grep -qiE '[^']*'\s*<<<\s*"\$\{\{/,
135+
"the body is interpolated directly into the script"
136+
end
137+
138+
test "the pattern it uses catches the shapes that were found in the wild" do
139+
# The seven descriptions on #99 to #105 carried the first of these. The others are the
140+
# trailers the commit hook already strips, included so one check knows every shape.
141+
pattern = ~r/Co-Authored-By: Claude|Claude-Session:|Generated with \[Claude/i
142+
143+
for body <- [
144+
"some text\n\n🤖 Generated with [Claude Code](https://claude.com/claude-code)\n",
145+
"Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>",
146+
"Claude-Session: abc123",
147+
"GENERATED WITH [CLAUDE CODE]"
148+
] do
149+
assert Regex.match?(pattern, body), "the pattern misses: #{inspect(body)}"
150+
end
151+
152+
for body <- [
153+
"a normal description mentioning claude in prose",
154+
"Signed-off-by: Ayla Croft <aylacroft@proton.me>",
155+
"This slice was generated from a template"
156+
] do
157+
refute Regex.match?(pattern, body), "the pattern over-matches: #{inspect(body)}"
158+
end
159+
end
160+
end
161+
end

‎test/trinity_web/live/compaction_live_test.exs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ defmodule TrinityWeb.CompactionLiveTest do
66
@moduletag :capture_log
77

88
import Phoenix.LiveViewTest
9-
import Trinity.SessionCase, only: [script_deltas: 2, collect: 3, await_event: 3]
9+
import Trinity.SessionCase, only: [script_deltas: 2, collect: 3]
1010

1111
alias Trinity.Factory
1212
alias Trinity.LLM.Providers.Fake

‎test/trinity_web/live/session_live_test.exs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ defmodule TrinityWeb.SessionLiveTest do
1111
@moduletag :capture_log
1212

1313
import Phoenix.LiveViewTest
14-
import Trinity.SessionCase, only: [script_deltas: 2, collect: 3, await_event: 3]
14+
import Trinity.SessionCase, only: [script_deltas: 2, await_event: 3]
1515

1616
alias Trinity.Factory
1717
alias Trinity.LLM.Providers.Fake

0 commit comments

Comments
 (0)