coverage — ~bigbes/sr-ht-spec — 3cb1c03d — mcpsrv/comment.go

Coverage
93.5% 43/46 statements
Δ
—
Blob
b8ee287
Uncovered L154-L155L158-L159L192-L193
1 package mcpsrv
2
3 import (
4 "context"
5 "fmt"
6 "strings"
7 "time"
8
9 "sourcecraft.dev/bigbes/sr-ht-spec/authn"
10 "sourcecraft.dev/bigbes/sr-ht-spec/service"
11 )
12
13 // Commenter is everything spec_comment may reach, and the list is short on
14 // purpose: read the threads, read the revision they are anchored against, and
15 // append a reply.
16 //
17 // What it leaves out is the load-bearing part. *service.Service also has
18 // CommentOn and ResolveThread, and both are owner-only there; naming them here
19 // would be harmless today and a regression the first time somebody relaxed the
20 // service-side check. An unresolved thread suppresses policy auto-merge, so an
21 // agent that could open or resolve one would hold the gate that exists to hold
22 // its own output back. The handler is written against this interface rather
23 // than against Writer so that "the comment tool cannot resolve a thread" is a
24 // property of the types, not of the handler remembering not to.
25 type Commenter interface {
26 Threads(ctx context.Context, p authn.Principal, proposalID int) ([]service.Thread, error)
27 ReplyTo(ctx context.Context, p authn.Principal, threadID int, body string) (service.Comment, error)
28 GetProposal(ctx context.Context, id int) (service.Proposal, error)
29 ProposalDiff(ctx context.Context, p service.Proposal) ([]service.ProposalDoc, error)
30 }
31
32 type commentInput struct {
33 Proposal int `json:"proposal" jsonschema:"the proposal whose review threads to read, or whose thread to reply to — the id spec_propose returned"`
34 // Thread and Body together are the reply; omitting both lists.
35 Thread int `json:"thread,omitempty" jsonschema:"reply to this thread, given as the thread id from a listing. Omit both this and body to list the proposal's threads instead."`
36 Body string `json:"body,omitempty" jsonschema:"the reply text, required when thread is given"`
37 }
38
39 // commentAuthor is one message: a thread's root or a reply. Times are formatted
40 // strings rather than time.Time because the tool's schema is derived from this
41 // struct by reflection, and a time.Time would reach the agent as an object with
42 // no fields.
43 type commentAuthor struct {
44 ID int `json:"id"`
45 // Thread is the id of the thread this message belongs to. On a root it is
46 // its own id, which is the value to send back as the thread argument.
47 Thread int `json:"thread"`
48 Author string `json:"author"`
49 // Agent reports whether an agent wrote this, so a reply of one's own is
50 // distinguishable from the owner's critique without parsing the name.
51 Agent bool `json:"agent"`
52 Body string `json:"body"`
53 Created string `json:"created"`
54 }
55
56 type commentThread struct {
57 commentAuthor
58 // Document is the path of the document the thread is on, and DocID the
59 // archive key its anchor was written against — the anchor survives a rename,
60 // so the two can disagree.
61 Document string `json:"document"`
62 DocID string `json:"doc_id,omitempty"`
63 // Heading is the enclosing headings of the commented block, outermost
64 // first. With Document it is how to find the block being talked about.
65 Heading []string `json:"heading,omitempty"`
66 // Side is which revision of the block was commented on: "new" for the
67 // proposed text, "old" for a block the proposal deletes.
68 Side string `json:"side,omitempty"`
69 // State is how well the anchor still fits the CURRENT proposal branch:
70 // "anchored" (the block is there verbatim), "edited" (the block is at that
71 // position but its text has changed since the comment) or "outdated" (the
72 // anchor lost its block). Acting on an outdated critique is the failure this
73 // field exists to prevent.
74 State string `json:"state"`
75 // Block is the block's index in the document, or -1 when the anchor is
76 // outdated and points at nothing.
77 Block int `json:"block"`
78 // Open reports whether the thread still awaits the owner. Only the owner can
79 // close one; a reply never does.
80 Open bool `json:"open"`
81 Replies []commentAuthor `json:"replies,omitempty"`
82 }
83
84 type commentOutput struct {
85 Proposal int `json:"proposal"`
86 // Threads is set when listing, and empty when the proposal has no review
87 // threads at all — which is a proposal nobody has commented on, not an
88 // error.
89 Threads []commentThread `json:"threads,omitempty"`
90 // Reply is set when replying, and carries the stored reply as it was
91 // attributed: an agent's reply comes back under its own agent identity.
92 Reply *commentAuthor `json:"reply,omitempty"`
93 }
94
95 16 func commentHandler(ctx context.Context, c Commenter, in commentInput) (commentOutput, error) {
96 16 // Both modes of this tool begin by reading the review conversation — the
97 16 // reply path lists the threads before it can answer one — so it is a read
98 16 // surface and carries the read grant. The grant vocabulary has no separate
99 16 // action for commenting, and inventing one here would put a word in the
100 16 // instance's dictionary that no token was ever minted with.
101 16 if err := requireRead(ctx); err != nil {
102 1 return commentOutput{}, err
103 1 }
104 15 if in.Proposal <= 0 {
105 1 return commentOutput{}, fmt.Errorf("proposal must name a proposal id")
106 1 }
107 // The mode is chosen on whether body was sent at all, not on whether it has
108 // anything in it: a whitespace body is a caller that meant to reply and
109 // botched it, and quietly listing instead would look like the reply landed.
110 14 body := strings.TrimSpace(in.Body)
111 14 switch {
112 1 case in.Thread > 0 && body == "":
113 1 return commentOutput{}, fmt.Errorf("a reply to thread %d needs a body", in.Thread)
114 1 case in.Thread <= 0 && in.Body != "":
115 1 return commentOutput{}, fmt.Errorf("body needs the thread it answers; pass thread, " +
116 1 "or omit body to list this proposal's threads")
117 }
118
119 // The principal is the one the resolver middleware put on this request.
120 // service.Threads and service.ReplyTo apply the ACL — this layer forwards
121 // the caller rather than deciding anything, so the read plane has one policy.
122 12 principal := authn.PrincipalFromContext(ctx)
123 12
124 12 // Threads runs before anything touches git, in both modes. It is one query,
125 12 // it is where the ACL is enforced, and a proposal with no threads needs no
126 12 // revision read at all.
127 12 threads, err := c.Threads(ctx, principal, in.Proposal)
128 12 if err != nil {
129 2 return commentOutput{}, err
130 2 }
131
132 10 if in.Thread > 0 {
133 2 return replyToThread(ctx, c, principal, threads, in.Proposal, in.Thread, body)
134 2 }
135 8 return listThreads(ctx, c, threads, in.Proposal)
136 }
137
138 // listThreads resolves every anchor against the proposal branch as it stands
139 // now and reports the fit.
140 //
141 // The anchoring is not optional decoration. A thread's stored anchor says where
142 // the comment was written, and the branch has moved since — often because this
143 // very agent revised it. An agent asking "what should I fix" that is not told
144 // the critique no longer describes any block will go and fix the wrong
145 // paragraph, so the state travels with every thread.
146 8 func listThreads(ctx context.Context, c Commenter, threads []service.Thread, proposalID int) (commentOutput, error) {
147 8 out := commentOutput{Proposal: proposalID}
148 8 if len(threads) == 0 {
149 1 return out, nil
150 1 }
151
152 7 p, err := c.GetProposal(ctx, proposalID)
153 7 if err != nil {
154 0 return commentOutput{}, err
155 0 }
156 7 docs, err := c.ProposalDiff(ctx, p)
157 7 if err != nil {
158 0 return commentOutput{}, err
159 0 }
160
161 7 out.Threads = make([]commentThread, 0, len(threads))
162 19 for _, t := range service.AnchorThreads(threads, docs) {
163 19 out.Threads = append(out.Threads, threadEntry(t))
164 19 }
165 7 return out, nil
166 }
167
168 // replyToThread appends the reply, after checking the thread is one of this
169 // proposal's.
170 //
171 // A thread id is a global integer, so a mistyped one names a real thread on
172 // somebody else's proposal, and service.ReplyTo would accept it: the id is all
173 // it needs. Requiring the proposal and checking membership here turns that
174 // typo into an error instead of a reply that lands out of sight of the agent
175 // that wrote it. The threads were already read for the ACL check, so it costs
176 // nothing.
177 2 func replyToThread(ctx context.Context, c Commenter, p authn.Principal, threads []service.Thread, proposalID, threadID int, body string) (commentOutput, error) {
178 2 found := false
179 5 for _, t := range threads {
180 5 if t.Root.ID == threadID {
181 1 found = true
182 1 break
183 }
184 }
185 2 if !found {
186 1 return commentOutput{}, fmt.Errorf("proposal %d has no review thread %d; "+
187 1 "call spec_comment with only proposal to list its threads", proposalID, threadID)
188 1 }
189
190 1 reply, err := c.ReplyTo(ctx, p, threadID, body)
191 1 if err != nil {
192 0 return commentOutput{}, err
193 0 }
194 1 entry := authorEntry(reply)
195 1 return commentOutput{Proposal: proposalID, Reply: &entry}, nil
196 }
197
198 19 func threadEntry(t service.Thread) commentThread {
199 19 e := commentThread{
200 19 commentAuthor: authorEntry(t.Root),
201 19 Document: t.DocPath,
202 19 DocID: t.Anchor.DocID,
203 19 Heading: t.Anchor.HeadingPath,
204 19 Side: string(t.Anchor.Side),
205 19 State: string(t.State),
206 19 Block: t.Block,
207 19 Open: t.Open(),
208 19 }
209 19 // A root's thread id is its own id: that is the value spec_comment takes
210 19 // back as the thread argument.
211 19 e.Thread = t.Root.ID
212 19 for _, r := range t.Replies {
213 7 e.Replies = append(e.Replies, authorEntry(r))
214 7 }
215 19 return e
216 }
217
218 27 func authorEntry(c service.Comment) commentAuthor {
219 27 return commentAuthor{
220 27 ID: c.ID,
221 27 Thread: c.ParentID,
222 27 Author: c.Author,
223 27 Agent: c.Agent,
224 27 Body: c.Body,
225 27 Created: c.Created.UTC().Format(time.RFC3339),
226 27 }
227 27 }