~bigbes/sr-ht-spec

ref: 3d811988f9960cf9057e09f30b59ceb71a7623c2 sr-ht-spec/service/comment_test.go -rw-r--r-- 11.3 KiB
3d811988 — Eugene Blikh service: wrap the merge and reject failures with culpa 9 days ago
                                                                                
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
package service

import (
	"context"
	"errors"
	"testing"

	"sourcecraft.dev/bigbes/sr-ht-spec/core"
)

// autoMergeSetup opens a proposal that does not auto-merge yet — there is no
// policy at the time it is opened — and then installs one that covers it. The
// agent's next revision is therefore the first moment auto-merge can fire,
// which is exactly the window an open review thread has to hold shut.
func autoMergeSetup(t *testing.T) (*Service, context.Context, *Space, ProposeResult, string) {
	t.Helper()
	svc, _ := newTestService(t)
	ctx := context.Background()
	sp, err := svc.CreateSpace(ctx, fxSpace)
	if err != nil {
		t.Fatalf("CreateSpace: %v", err)
	}
	base, err := sp.Repo.ApprovedHead(ctx)
	if err != nil {
		t.Fatalf("ApprovedHead: %v", err)
	}

	res, err := svc.Propose(ctx, ProposeRequest{
		Space:     fxSpace,
		Principal: agentPrincipal(),
		Title:     "firehose note",
		IfMatch:   base.String(),
		Message:   "add notes/a.md",
		Writes:    []DocumentWrite{{Path: "notes/a.md", Content: mdDoc("N-1", "A", "first body")}},
	})
	if err != nil {
		t.Fatalf("Propose: %v", err)
	}
	if res.Merged {
		t.Fatalf("proposal auto-merged with no policy installed")
	}

	// Widen the policy so the proposal's paths now qualify.
	commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
		".spec.yml": []byte("review:\n  auto_merge: [notes/**]\n"),
	})
	return svc, ctx, sp, res, base.String()
}

// revise is the agent's next push onto an open proposal, which is where
// auto-merge is re-evaluated.
func revise(t *testing.T, svc *Service, ctx context.Context, res ProposeResult, base, body string) ProposeResult {
	t.Helper()
	out, err := svc.Propose(ctx, ProposeRequest{
		Space:      fxSpace,
		Principal:  agentPrincipal(),
		ProposalID: res.Proposal.ID,
		IfMatch:    base,
		Message:    "revise notes/a.md",
		Writes:     []DocumentWrite{{Path: "notes/a.md", Content: mdDoc("N-1", "A", body)}},
	})
	if err != nil {
		t.Fatalf("revise: %v", err)
	}
	return out
}

func commentOn(t *testing.T, svc *Service, ctx context.Context, proposalID int) Thread {
	t.Helper()
	th, err := svc.CommentOn(ctx, CommentRequest{
		Principal:  ownerPrincipal(),
		Space:      fxSpace,
		ProposalID: proposalID,
		DocPath:    "notes/a.md",
		Anchor: core.CommentAnchor{
			DocID: "N-1", HeadingPath: nil, Index: 0, BlockHash: "whatever", Side: core.SideNew,
		},
		Body: "This contradicts SPEC-0003.",
	})
	if err != nil {
		t.Fatalf("CommentOn: %v", err)
	}
	return th
}

// The gate. Without a comment the agent's revision lands under policy; with an
// open thread it must not, because the owner has engaged with this proposal and
// it must not slip past them unattended on the agent's next push.
func TestOpenThreadSuppressesPolicyAutoMerge(t *testing.T) {
	svc, ctx, _, res, base := autoMergeSetup(t)

	commentOn(t, svc, ctx, res.Proposal.ID)

	revised := revise(t, svc, ctx, res, base, "second body")
	if revised.Merged {
		t.Fatal("policy auto-merged a proposal with an open review thread")
	}
	if revised.Proposal.State != core.StateOpen {
		t.Errorf("state = %s, want open", revised.Proposal.State)
	}
}

// The control for the test above: the same revision, with nothing to hold it
// back, does land. Without this, a broken auto-merge would make the gate test
// pass for the wrong reason.
func TestPolicyAutoMergeStillFiresWithoutAThread(t *testing.T) {
	svc, ctx, _, res, base := autoMergeSetup(t)

	revised := revise(t, svc, ctx, res, base, "second body")
	if !revised.Merged {
		t.Fatal("policy did not auto-merge an uncommented proposal; the gate test above proves nothing")
	}
	if revised.Proposal.Approval != core.ApprovalPolicy {
		t.Errorf("approval = %q, want policy", revised.Proposal.Approval)
	}
}

// Resolving the thread lifts the gate: the owner has had their say, so the
// proposal returns to the firehose rather than needing a manual click.
func TestResolvingTheThreadRestoresAutoMerge(t *testing.T) {
	svc, ctx, _, res, base := autoMergeSetup(t)

	th := commentOn(t, svc, ctx, res.Proposal.ID)
	if err := svc.ResolveThread(ctx, ownerPrincipal(), th.Root.ID, true); err != nil {
		t.Fatalf("ResolveThread: %v", err)
	}

	revised := revise(t, svc, ctx, res, base, "second body")
	if !revised.Merged {
		t.Fatal("a resolved thread still suppressed auto-merge")
	}
}

// An agent reply does not lift the gate. The agent answering a critique is not
// the owner accepting the answer, and if it were, the gate would be under the
// control of the thing it exists to hold back.
func TestAgentReplyDoesNotLiftTheGate(t *testing.T) {
	svc, ctx, _, res, base := autoMergeSetup(t)

	th := commentOn(t, svc, ctx, res.Proposal.ID)
	if _, err := svc.ReplyTo(ctx, agentPrincipal(), th.Root.ID, "Fixed in this revision."); err != nil {
		t.Fatalf("ReplyTo: %v", err)
	}

	revised := revise(t, svc, ctx, res, base, "second body")
	if revised.Merged {
		t.Fatal("an agent reply cleared the auto-merge gate")
	}
}

// The gate is on policy auto-merge only. A comment nobody got round to
// resolving must not be able to wedge a proposal shut — the owner clicking
// approve is the judgement the thread was asking for.
func TestManualApproveIgnoresOpenThreads(t *testing.T) {
	svc, ctx, _, res, _ := autoMergeSetup(t)

	commentOn(t, svc, ctx, res.Proposal.ID)

	merged, err := svc.MergeHuman(ctx, fxSpace, res.Proposal.ID)
	if err != nil {
		t.Fatalf("MergeHuman with an open thread: %v", err)
	}
	if merged.State != core.StateMerged {
		t.Errorf("state = %s, want merged", merged.State)
	}
	if merged.Approval != core.ApprovalHuman {
		t.Errorf("approval = %q, want human", merged.Approval)
	}
}

// Who may do what. An agent may join a conversation but may neither start one
// nor declare it finished: both would hand it control of the gate.
func TestThreadAuthority(t *testing.T) {
	svc, ctx, _, res, _ := autoMergeSetup(t)

	if _, err := svc.CommentOn(ctx, CommentRequest{
		Principal: agentPrincipal(), Space: fxSpace, ProposalID: res.Proposal.ID,
		DocPath: "notes/a.md", Anchor: core.CommentAnchor{DocID: "N-1", Side: core.SideNew},
		Body: "self-review",
	}); !errors.Is(err, ErrForbidden) {
		t.Errorf("agent opening a thread = %v, want ErrForbidden", err)
	}

	th := commentOn(t, svc, ctx, res.Proposal.ID)

	reply, err := svc.ReplyTo(ctx, agentPrincipal(), th.Root.ID, "Acknowledged.")
	if err != nil {
		t.Fatalf("agent reply: %v", err)
	}
	if !reply.Agent {
		t.Error("agent reply did not come back marked as an agent's")
	}

	if err := svc.ResolveThread(ctx, agentPrincipal(), th.Root.ID, true); !errors.Is(err, ErrForbidden) {
		t.Errorf("agent resolving a thread = %v, want ErrForbidden", err)
	}

	threads, err := svc.Threads(ctx, ownerPrincipal(), res.Proposal.ID)
	if err != nil {
		t.Fatalf("Threads: %v", err)
	}
	if len(threads) != 1 {
		t.Fatalf("Threads returned %d threads, want 1 (replies belong to their root)", len(threads))
	}
	if len(threads[0].Replies) != 1 || !threads[0].Replies[0].Agent {
		t.Errorf("replies = %+v, want one agent reply", threads[0].Replies)
	}
	if !threads[0].Open() {
		t.Error("thread reports closed; the agent must not have been able to resolve it")
	}
}

// --- anchoring against a revision (no database) ---

const anchorDoc = `# Storage

The first paragraph of the storage section.

The second paragraph, which will be edited.

## Trade-offs

A trade-off paragraph.
`

func anchorFor(t *testing.T, src string, ordinal int) core.CommentAnchor {
	t.Helper()
	a, err := AnchorOf("SPEC-0007", []byte(src), ordinal, core.SideNew)
	if err != nil {
		t.Fatalf("AnchorOf(%d): %v", ordinal, err)
	}
	return a
}

// AnchorOf and AnchorBlocks must number blocks identically — one builds an
// anchor, the other resolves it, and a disagreement would put every comment on
// the wrong block of its own section.
func TestAnchorOfAgreesWithResolution(t *testing.T) {
	src := []byte(anchorDoc)
	blocks := anchorBlocksOf(src)

	for ordinal := range blocks {
		a := anchorFor(t, anchorDoc, ordinal)
		got, state := core.ResolveAnchor(a, blocks)
		if got != ordinal || state != core.AnchorExact {
			t.Errorf("block %d: resolved to (%d, %s), want (%d, %s)",
				ordinal, got, state, ordinal, core.AnchorExact)
		}
	}

	if _, err := AnchorOf("SPEC-0007", src, len(blocks), core.SideNew); !errors.Is(err, ErrInvalid) {
		t.Errorf("AnchorOf past the end = %v, want ErrInvalid", err)
	}
}

// The three outcomes, against a revision the agent has since pushed: untouched
// text stays anchored, edited text keeps its comment and says it was edited,
// and text that is gone is reported outdated rather than moved onto a
// neighbouring paragraph.
func TestAnchorThreadsReportsFitAgainstARevision(t *testing.T) {
	const revised = `# Storage

The first paragraph of the storage section.

The second paragraph, completely rewritten in the agent's revision.

## Trade-offs

A trade-off paragraph.
`
	untouched := anchorFor(t, anchorDoc, 1) // "The first paragraph..."
	edited := anchorFor(t, anchorDoc, 2)    // "The second paragraph, which will be edited."

	// An anchor to a block that the revision drops entirely.
	const withExtra = anchorDoc + "\nA paragraph that the revision removes.\n"
	removed := anchorFor(t, withExtra, 5)

	threads := []Thread{
		{DocPath: "specs/a.md", Anchor: untouched},
		{DocPath: "specs/a.md", Anchor: edited},
		{DocPath: "specs/a.md", Anchor: removed},
		{DocPath: "specs/gone.md", Anchor: untouched},
	}
	docs := []ProposalDoc{{Path: "specs/a.md", Base: []byte(anchorDoc), Proposed: []byte(revised)}}

	got := AnchorThreads(threads, docs)
	want := []core.AnchorState{
		core.AnchorExact,
		core.AnchorEdited,
		core.AnchorOutdated,
		core.AnchorOutdated, // its document is not among the proposal's changes
	}
	for i := range want {
		if got[i].State != want[i] {
			t.Errorf("thread %d: state = %s, want %s", i, got[i].State, want[i])
		}
	}
	if got[0].Block < 0 || got[1].Block < 0 {
		t.Errorf("a resolved anchor must name a block: %d, %d", got[0].Block, got[1].Block)
	}
	if got[2].Block != -1 || got[3].Block != -1 {
		t.Errorf("an outdated anchor must name no block: %d, %d", got[2].Block, got[3].Block)
	}
	// The input must not be mutated: a caller rendering two revisions would
	// otherwise see the first one's answers on the second.
	if threads[0].State != "" || threads[0].Block != 0 {
		t.Error("AnchorThreads mutated its input")
	}
}

// A thread whose document the proposal no longer changes — the agent reverted
// it — is outdated, not dropped. A comment that silently vanished would look
// like one that was never made.
func TestRevertedDocumentOutdatesItsThreadsRatherThanLosingThem(t *testing.T) {
	threads := []Thread{{DocPath: "specs/a.md", Anchor: anchorFor(t, anchorDoc, 1)}}

	got := AnchorThreads(threads, nil)
	if len(got) != 1 {
		t.Fatalf("AnchorThreads returned %d threads, want 1 kept", len(got))
	}
	if got[0].State != core.AnchorOutdated || got[0].Block != -1 {
		t.Errorf("state/block = %s/%d, want outdated/-1", got[0].State, got[0].Block)
	}
}

// A comment on the old side of a deleted block reads the base, not the proposed
// text — there is no proposed text for a block the change removes.
func TestOldSideAnchorsAgainstTheBase(t *testing.T) {
	a := anchorFor(t, anchorDoc, 2)
	a.Side = core.SideOld

	got := AnchorThreads([]Thread{{DocPath: "specs/a.md", Anchor: a}},
		[]ProposalDoc{{Path: "specs/a.md", Base: []byte(anchorDoc), Proposed: []byte("# Storage\n")}})

	if got[0].State != core.AnchorExact {
		t.Errorf("state = %s, want %s: an old-side anchor resolves against the base",
			got[0].State, core.AnchorExact)
	}
}