- commit
- fb3940d0224778bbab72bdf259ce1caca311fcb4
- parent
- a5b9173eb7d5065bf5fef5d53b1533f2af36c062
- author
- Dami <dami.etoile@gmail.com>
| M | docs/ | 29 | ++++ |
| M | docs/ | 6 | +- |
| M | internal/ | 31 | ++-- |
| A | internal/ | 93 | ++++++++++++ |
| A | internal/ | 113 | ++++++++++++++ |
| M | internal/ | 17 | ++ |
| M | internal/ | 17 | ++- |
| M | internal/ | 3 | + |
| M | internal/ | 3 | + |
| M | internal/ | 13 | +- |
| M | internal/ | 20 | ++- |
| A | internal/ | 240 | ++++++++++++++++++++++++++++++ |
| A | internal/ | 122 | +++++++++++++++ |
| M | internal/ | 4 | + |
| M | internal/ | 3 | + |
| M | scripts/ | 29 | ++++ |
@@ -12,6 +12,35 @@ Pull request diffs use commits available in the hosted repository. Missing commi
1212 1313 Events follow [NIP-34](https://github.com/nostr-protocol/nips/blob/master/34.md), with replies using [NIP-22](https://github.com/nostr-protocol/nips/blob/master/22.md). Markdown is supported in issues and pull request descriptions; comments are plain text. Status reflects the newest visible event signed by an authorized author or maintainer.1414 15+## Review comments16+17+A comment under a pull request or patch can point at one line of the diff. Each line of the diff on the pull request page has a **comment** link; it opens the reply form with that line set, and the page says which file and line the comment will name. Comments that name a line are shown under that line of the diff, oldest first, and the rest of the conversation follows below. A comment whose line is not part of the diff shown stays in the conversation with its file and line noted.18+19+A review comment is a kind 1111 event with the NIP-22 tags a comment normally carries plus two tags:20+21+| Tag | Value |22+| --- | --- |23+| `file` | The path of the file in the diff, as Git prints it after `b/`. |24+| `line` | The line number, then the side: `old` for the base version or `new` for the proposed version. |25+26+Example tags for a comment on line 12 of the new version of `src/main.go`:27+28+```json29+[30+ ["a", "30617:<owner pubkey>:tinyrelay"],31+ ["E", "<pull request id>", "", "<author pubkey>"],32+ ["K", "1618"],33+ ["P", "<author pubkey>"],34+ ["e", "<pull request id>", "", "<author pubkey>"],35+ ["k", "1618"],36+ ["p", "<author pubkey>"],37+ ["file", "src/main.go"],38+ ["line", "12", "new"]39+]40+```41+42+The two tags go together, once each. The path must not be empty, the line number must be a positive integer and the side must be `old` or `new`. The root named by the `K` tag must be a pull request (kind 1618) or patch (kind 1617). The relay refuses a comment whose tags break these rules with a `blocked:` reason, whether it is published directly or arrives through synchronization. The `browsepull` query and the `read_pull_request` tool return each reply with `file`, `line` and `side` fields; the fields are absent on comments that name no line. Agents publish review comments through the `comment` tool with its `file`, `line` and `side` arguments. See [MCP](mcp.md).43+1544 The browser tools page exposes the same lists and detail pages to browser agents. See [Browser tools](webmcp.md).1645 1746 ## Synchronization@@ -95,7 +95,7 @@ Write tools, which publish through the same path as `POST /events`:
9595 | `publish_event` | Publish any signed Nostr event. |9696 | `create_issue` | Open a kind 1621 issue on a hosted repository. |9797 | `create_pull_request` | Open a kind 1618 pull request with its commit and clone URL. |98-| `comment` | Reply to an issue, pull request or comment with a kind 1111 event carrying NIP-22 tags. |98+| `comment` | Reply to an issue, pull request or comment with a kind 1111 event carrying NIP-22 tags. `file`, `line` and `side` anchor the comment to one diff line. |9999 | `set_status` | Mark an issue or pull request open, resolved, merged, closed or draft with a kind 1630 to 1633 event. |100100 | `post_message` | Post a kind 9 message in a room, with optional `mentions` as `p` tags. |101101 | `start_thread` | Start a kind 11 thread in a room with an optional title. |@@ -111,6 +111,10 @@ Write tools, which publish through the same path as `POST /events`:
111111 112112 The relay never signs on a caller's behalf. Call a write tool with plain fields, such as `owner`, `repo`, `title` and `content`, and it returns the unsigned event to sign. Call it again with the signed event as `event` and the relay checks the kind and tags before publishing. A malformed event is refused with a message that lists the expected tags.113113 114+### Reviewing a diff line115+116+`comment` takes optional `file`, `line` and `side` arguments when the root is a pull request or patch. `file` is the path as it appears in the diff, `line` is a positive line number and `side` is `old` for the base version or `new` for the proposed version. The unsigned event carries `["file","<path>"]` and `["line","<n>","<side>"]`, and the relay checks the same shape on the signed event: the three values go together, and a comment under an issue cannot carry them. `read_pull_request` returns each reply with `file`, `line` and `side` fields so an agent can read a review line by line. See [Review comments](git-collaboration.md#review-comments).117+114118 ### Asking a person115119 116120 `request_decision` builds an event addressed to one person. Pass `pubkey`, `request` (`approve`, `decide` or `question`) and `content`, then either `room` for a kind 9 message in that room or `root`, `root_kind` and `root_pubkey` for a kind 1111 comment under an issue, pull request or other event. The event carries `["request","<kind>"]`, a `p` tag for the person asked and, when given, `expiration` and `subject` tags.@@ -27,17 +27,17 @@ type collaborationItem struct {
2727 }2828 2929 type collaborationDetail struct {30- Repository map[string]any `json:"repository"`31- Root event.Event `json:"root"`32- Item collaborationItem `json:"item"`33- Replies []event.Event `json:"replies"`34- Updates []event.Event `json:"updates"`35- Statuses []event.Event `json:"statuses"`36- Diff string `json:"diff,omitempty"`37- CanStatus bool `json:"can_status"`38- DiffUnavailable string `json:"diff_unavailable,omitempty"`39- DiffTruncated bool `json:"diff_truncated,omitempty"`40- ReplyCursor string `json:"reply_cursor,omitempty"`30+ Repository map[string]any `json:"repository"`31+ Root event.Event `json:"root"`32+ Item collaborationItem `json:"item"`33+ Replies []collaborationReply `json:"replies"`34+ Updates []event.Event `json:"updates"`35+ Statuses []event.Event `json:"statuses"`36+ Diff string `json:"diff,omitempty"`37+ CanStatus bool `json:"can_status"`38+ DiffUnavailable string `json:"diff_unavailable,omitempty"`39+ DiffTruncated bool `json:"diff_truncated,omitempty"`40+ ReplyCursor string `json:"reply_cursor,omitempty"`4141 }4242 4343 func (t *Tenant) collaborationRepository(ctx context.Context, repo gitrelay.Repository) map[string]any {@@ -196,12 +196,11 @@ func (t *Tenant) browseCollaborationDetail(ctx context.Context, actor string, q
196196 if err != nil {197197 return nil, err198198 }199- detail.Replies = replies200- if len(detail.Replies) > replyLimit {201- detail.Replies = detail.Replies[:replyLimit]202- last := detail.Replies[replyLimit-1]203- detail.ReplyCursor = collaborationCursor(collaborationItemFrom(last))199+ if len(replies) > replyLimit {200+ replies = replies[:replyLimit]201+ detail.ReplyCursor = collaborationCursor(collaborationItemFrom(replies[replyLimit-1]))204202 }203+ detail.Replies = collaborationReplies(replies)205204 if kind == event.KIND_GIT_PR {206205 detail.Updates, err = t.Query(ctx, []event.Filter{{Authors: []string{root.PubKey}, Kinds: []int{1619}, Tags: map[string][]string{"E": {root.ID}, "P": {root.PubKey}}, Limit: intPtr(100)}}, browseSession(t, actor))207206 if err != nil {@@ -0,0 +1,93 @@
1+package daemon2+3+import (4+ "errors"5+ "strconv"6+ "strings"7+8+ "github.com/FelineStateMachine/tinyrelay/internal/event"9+)10+11+// Review comments. A kind 1111 comment under a pull request or patch may name12+// one line of the diff with ["file", "<path>"] and ["line", "<n>", "<side>"],13+// where the side is "old" for the base or "new" for the proposed version. The14+// relay validates the shape whenever the tags are present and browse results15+// carry the anchor as plain fields so clients can place the comment.16+17+const reviewFileMax = 409618+19+// reviewAnchor is the diff line a comment names. A zero value means the20+// comment is not anchored.21+type reviewAnchor struct {22+ File string `json:"file,omitempty"`23+ Line int `json:"line,omitempty"`24+ Side string `json:"side,omitempty"`25+}26+27+// collaborationReply is one comment in a thread with its anchor, when it has28+// one, alongside the event fields.29+type collaborationReply struct {30+ event.Event31+ reviewAnchor32+}33+34+func collaborationReplies(rows []event.Event) []collaborationReply {35+ out := make([]collaborationReply, 0, len(rows))36+ for _, row := range rows {37+ anchor, _, _ := reviewAnchorOf(row)38+ out = append(out, collaborationReply{Event: row, reviewAnchor: anchor})39+ }40+ return out41+}42+43+// reviewAnchorOf reads a comment's anchor. It reports whether the tags are44+// present and returns a blocked reason when they are malformed.45+func reviewAnchorOf(e event.Event) (reviewAnchor, bool, error) {46+ if e.Kind != 1111 {47+ return reviewAnchor{}, false, nil48+ }49+ var files, lines [][]string50+ for _, tag := range e.Tags {51+ if len(tag) == 0 {52+ continue53+ }54+ switch tag[0] {55+ case "file":56+ files = append(files, tag)57+ case "line":58+ lines = append(lines, tag)59+ }60+ }61+ if len(files) == 0 && len(lines) == 0 {62+ return reviewAnchor{}, false, nil63+ }64+ if len(files) != 1 || len(lines) != 1 {65+ return reviewAnchor{}, true, errors.New("blocked: a review comment carries one file tag and one line tag")66+ }67+ file := files[0]68+ if len(file) < 2 || strings.TrimSpace(file[1]) == "" || len(file[1]) > reviewFileMax || strings.ContainsAny(file[1], "\n\r\x00") {69+ return reviewAnchor{}, true, errors.New("blocked: a review comment file tag needs a path")70+ }71+ line := lines[0]72+ if len(line) < 3 {73+ return reviewAnchor{}, true, errors.New("blocked: a review comment line tag needs a line number and a side, old or new")74+ }75+ n, err := strconv.Atoi(line[1])76+ if err != nil || n < 1 || strconv.Itoa(n) != line[1] {77+ return reviewAnchor{}, true, errors.New("blocked: a review comment line must be a positive integer")78+ }79+ if line[2] != "old" && line[2] != "new" {80+ return reviewAnchor{}, true, errors.New("blocked: a review comment side must be old or new")81+ }82+ if kind := event.Tag(e, "K"); kind != "1617" && kind != "1618" {83+ return reviewAnchor{}, true, errors.New("blocked: a review comment anchors to a pull request or patch")84+ }85+ return reviewAnchor{File: file[1], Line: n, Side: line[2]}, true, nil86+}87+88+// validateReviewAnchor is the admission check for comments that carry file89+// and line tags. Comments without them pass.90+func validateReviewAnchor(e event.Event) error {91+ _, _, err := reviewAnchorOf(e)92+ return err93+}@@ -0,0 +1,113 @@
1+package daemon2+3+import (4+ "context"5+ "encoding/json"6+ "strings"7+ "testing"8+ "time"9+10+ "github.com/FelineStateMachine/tinyrelay/internal/event"11+ "github.com/FelineStateMachine/tinyrelay/internal/replication"12+)13+14+func TestReviewAnchorShape(t *testing.T) {15+ base := [][]string{{"a", "30617:" + strings.Repeat("a", 64) + ":test"}, {"E", strings.Repeat("b", 64), "", strings.Repeat("a", 64)}, {"K", "1618"}, {"P", strings.Repeat("a", 64)}}16+ comment := func(kind string, extra ...[]string) event.Event {17+ tags := append([][]string(nil), base...)18+ tags[2] = []string{"K", kind}19+ return event.Event{Kind: 1111, Tags: append(tags, extra...)}20+ }21+ anchor, present, err := reviewAnchorOf(comment("1618", []string{"file", "src/main.go"}, []string{"line", "12", "new"}))22+ if err != nil || !present || anchor != (reviewAnchor{File: "src/main.go", Line: 12, Side: "new"}) {23+ t.Fatalf("valid anchor: %+v %v %v", anchor, present, err)24+ }25+ if _, present, err := reviewAnchorOf(comment("1621")); err != nil || present {26+ t.Fatalf("plain comment: %v %v", present, err)27+ }28+ cases := map[string][][]string{29+ "missing line": {{"file", "a"}},30+ "missing file": {{"line", "1", "new"}},31+ "two files": {{"file", "a"}, {"file", "b"}, {"line", "1", "new"}},32+ "empty path": {{"file", " "}, {"line", "1", "new"}},33+ "zero line": {{"file", "a"}, {"line", "0", "new"}},34+ "padded line": {{"file", "a"}, {"line", "01", "new"}},35+ "word line": {{"file", "a"}, {"line", "one", "new"}},36+ "no side": {{"file", "a"}, {"line", "1"}},37+ "unknown side": {{"file", "a"}, {"line", "1", "left"}},38+ "newline in path": {{"file", "a\nb"}, {"line", "1", "new"}},39+ }40+ for name, extra := range cases {41+ if err := validateReviewAnchor(comment("1618", extra...)); err == nil || !strings.HasPrefix(err.Error(), "blocked: ") {42+ t.Fatalf("%s: %v", name, err)43+ }44+ }45+ if err := validateReviewAnchor(comment("1621", []string{"file", "a"}, []string{"line", "1", "new"})); err == nil || !strings.Contains(err.Error(), "pull request or patch") {46+ t.Fatalf("anchor under an issue: %v", err)47+ }48+ if err := validateReviewAnchor(comment("1617", []string{"file", "a"}, []string{"line", "3", "old"})); err != nil {49+ t.Fatalf("anchor under a patch: %v", err)50+ }51+}52+53+func TestReviewCommentsAreValidatedAndBrowsedWithTheirAnchor(t *testing.T) {54+ ctx := context.Background()55+ _, tenant := testTenant(t)56+ p := tenant.Policy()57+ p.Features.Grasp = true58+ if err := tenant.applyPolicy(ctx, p); err != nil {59+ t.Fatal(err)60+ }61+ owner, _ := event.PublicKey(testOwnerSecret)62+ now := time.Now().Unix()63+ agentTestRepo(t, tenant, "review")64+ coordinate := "30617:" + owner + ":review"65+ pull := signedEvent(t, testOwnerSecret, event.KIND_GIT_PR, now, [][]string{{"a", coordinate}, {"p", owner}, {"subject", "Change"}, {"c", strings.Repeat("1", 40)}, {"clone", "https://example.test/review.git"}}, "Why")66+ if err := publishAs(t, tenant, pull); err != nil {67+ t.Fatalf("pull request rejected: %v", err)68+ }69+ comment := func(createdAt int64, extra ...[]string) event.Event {70+ tags := append([][]string{{"a", coordinate}, {"E", pull.ID, "", owner}, {"K", "1618"}, {"P", owner}, {"e", pull.ID, "", owner}, {"k", "1618"}, {"p", owner}}, extra...)71+ return signedEvent(t, testOwnerSecret, 1111, createdAt, tags, "Review note")72+ }73+ if err := publishAs(t, tenant, comment(now+1, []string{"file", "README"}, []string{"line", "2", "left"})); err == nil || !strings.HasPrefix(err.Error(), "blocked: a review comment") {74+ t.Fatalf("malformed anchor (bad side): %v", err)75+ }76+ if err := publishAs(t, tenant, comment(now+2, []string{"file", "README"}, []string{"line", "2"})); err == nil || !strings.HasPrefix(err.Error(), "blocked: a review comment") {77+ t.Fatalf("malformed anchor (no side): %v", err)78+ }79+ if err := tenant.ingest(ctx, comment(now+3, []string{"file", ""}, []string{"line", "2", "new"}), replication.OriginImport); err == nil || !strings.HasPrefix(err.Error(), "blocked: a review comment") {80+ t.Fatalf("malformed anchor over import: %v", err)81+ }82+ anchored := comment(now+4, []string{"file", "README"}, []string{"line", "2", "new"})83+ if err := publishAs(t, tenant, anchored); err != nil {84+ t.Fatalf("anchored comment rejected: %v", err)85+ }86+ plain := comment(now + 5)87+ if err := publishAs(t, tenant, plain); err != nil {88+ t.Fatalf("plain comment rejected: %v", err)89+ }90+ raw, _ := json.Marshal(map[string]any{"owner": owner, "repo": "review", "event": pull.ID})91+ result, err := tenant.Execute(ctx, "", "browsepull", []json.RawMessage{raw})92+ if err != nil {93+ t.Fatalf("browsepull: %v", err)94+ }95+ encoded, _ := json.Marshal(result)96+ var detail struct {97+ Replies []struct {98+ ID string `json:"id"`99+ File string `json:"file"`100+ Line int `json:"line"`101+ Side string `json:"side"`102+ } `json:"replies"`103+ }104+ if err := json.Unmarshal(encoded, &detail); err != nil || len(detail.Replies) != 2 {105+ t.Fatalf("replies = %s %v", encoded, err)106+ }107+ if detail.Replies[0].ID != plain.ID || detail.Replies[0].File != "" || detail.Replies[0].Line != 0 || detail.Replies[0].Side != "" {108+ t.Fatalf("plain reply carried an anchor: %+v", detail.Replies[0])109+ }110+ if detail.Replies[1].ID != anchored.ID || detail.Replies[1].File != "README" || detail.Replies[1].Line != 2 || detail.Replies[1].Side != "new" {111+ t.Fatalf("anchored reply = %+v", detail.Replies[1])112+ }113+}@@ -178,6 +178,23 @@ func TestMCPWriteToolsBuildValidateAndPublish(t *testing.T) {
178178 if w.Code != http.StatusOK || isError || unsigned["kind"] != float64(1111) || string(tags) != `[["a","30617:`+owner+`:tiny"],["E","`+rootID+`","","`+owner+`"],["K","1621"],["P","`+owner+`"],["e","`+rootID+`","","`+owner+`"],["k","1621"],["p","`+owner+`"]]` {179179 t.Fatalf("comment template: %d %s", w.Code, w.Body.String())180180 }181+ w, response = mcpCall{method: "tools/call", name: "comment", arguments: map[string]any{"owner": owner, "repo": "tiny", "root": rootID, "root_kind": 1621, "root_pubkey": owner, "content": "Here.", "file": "README", "line": 2, "side": "new"}, sign: true}.do(t, app, "/mcp")182+ result, isError = mcpToolResult(t, response)183+ if w.Code != http.StatusOK || !isError || !strings.Contains(result["content"].([]any)[0].(map[string]any)["text"].(string), "pull request or patch") {184+ t.Fatalf("comment anchor under an issue: %d %s", w.Code, w.Body.String())185+ }186+ w, response = mcpCall{method: "tools/call", name: "comment", arguments: map[string]any{"owner": owner, "repo": "tiny", "root": strings.Repeat("d", 64), "root_kind": 1618, "root_pubkey": owner, "content": "Here.", "file": "README", "line": 2, "side": "new"}, sign: true}.do(t, app, "/mcp")187+ result, isError = mcpToolResult(t, response)188+ unsigned, _ = result["structuredContent"].(map[string]any)["unsigned"].(map[string]any)189+ tags, _ = json.Marshal(unsigned["tags"])190+ if w.Code != http.StatusOK || isError || !strings.HasSuffix(string(tags), `["file","README"],["line","2","new"]]`) {191+ t.Fatalf("comment anchor template: %d %s", w.Code, w.Body.String())192+ }193+ w, response = mcpCall{method: "tools/call", name: "comment", arguments: map[string]any{"event": sign(1111, [][]string{{"a", "30617:" + owner + ":tiny"}, {"E", strings.Repeat("d", 64), "", owner}, {"K", "1618"}, {"P", owner}, {"e", strings.Repeat("d", 64), "", owner}, {"k", "1618"}, {"p", owner}, {"file", "README"}, {"line", "0", "new"}}, "Here.")}, sign: true}.do(t, app, "/mcp")194+ result, isError = mcpToolResult(t, response)195+ if w.Code != http.StatusOK || !isError || !strings.Contains(result["content"].([]any)[0].(map[string]any)["text"].(string), "positive integer") {196+ t.Fatalf("comment anchor check: %d %s", w.Code, w.Body.String())197+ }181198 w, response = mcpCall{method: "tools/call", name: "set_status", arguments: map[string]any{"owner": owner, "repo": "tiny", "root": rootID, "root_pubkey": owner, "status": "resolved"}, sign: true}.do(t, app, "/mcp")182199 result, isError = mcpToolResult(t, response)183200 unsigned, _ = result["structuredContent"].(map[string]any)["unsigned"].(map[string]any)@@ -60,7 +60,7 @@ var (
6060 const (6161 mcpIssueShape = `Expected a signed kind 1621 event with tags ["a","30617:<owner>:<repo>"], ["p","<owner>"], ["subject","<title>"] and optional ["t","<label>"] tags, with the Markdown description in content.`6262 mcpPullShape = `Expected a signed kind 1618 event with tags ["a","30617:<owner>:<repo>"], ["p","<owner>"], ["subject","<title>"], ["c","<40-hex commit>"], ["clone","<http or https URL>"], optional ["merge-base","<40-hex commit>"] and optional ["t","<label>"] tags, with the description in content.`63- mcpCommentShape = `Expected a signed kind 1111 event with NIP-22 tags ["a","30617:<owner>:<repo>"], ["E","<root id>","","<root pubkey>"], ["K","<root kind>"], ["P","<root pubkey>"], ["e","<parent id>","","<parent pubkey>"], ["k","<parent kind>"], ["p","<parent pubkey>"], with the comment in content. The parent is the root for a top-level comment.`63+ mcpCommentShape = `Expected a signed kind 1111 event with NIP-22 tags ["a","30617:<owner>:<repo>"], ["E","<root id>","","<root pubkey>"], ["K","<root kind>"], ["P","<root pubkey>"], ["e","<parent id>","","<parent pubkey>"], ["k","<parent kind>"], ["p","<parent pubkey>"], with the comment in content. The parent is the root for a top-level comment. A review comment under a pull request or patch may add ["file","<path>"] and ["line","<n>","old" or "new"] to name one diff line.`6464 mcpStatusShape = `Expected a signed event of kind 1630 (open), 1631 (resolved or merged), 1632 (closed) or 1633 (draft) with tags ["a","30617:<owner>:<repo>"], ["e","<root id>","","root"] and ["p","<root pubkey>"].`6565 mcpMessageShape = `Expected a signed kind 9 event with tag ["h","<room id>"] and optional ["p","<pubkey>"] mentions, with the message in content.`6666 mcpThreadShape = `Expected a signed kind 11 event with tag ["h","<room id>"] and optional ["subject","<title>"], with the opening post in content.`@@ -172,7 +172,7 @@ func (t *Tenant) mcpTools() (*mcp.Registry, error) {
172172 add("publish_event", "Publish a signed Nostr event to this relay. The relay's access rules apply as they do for POST /events.", mcp.Object(map[string]any{"event": mcpEvent}, "event"), mcpPublishes, t.mcpWrite(nil, nil, "Expected a signed Nostr event."))173173 add("create_issue", "Open an issue on a hosted repository. Pass owner, repo, title and content to receive the unsigned kind 1621 event, sign it, then call again with the signed event.", mcp.Object(map[string]any{"event": mcpEvent, "owner": mcpPubKey, "repo": mcpID, "title": mcpText, "content": mcpText, "labels": map[string]any{"type": "array", "items": mcpText}}), mcpPublishes, t.mcpWrite(mcpBuildIssue, mcpCheckIssue, mcpIssueShape))174174 add("create_pull_request", "Open a pull request on a hosted repository. Pass owner, repo, title, commit and clone (plus optional content, merge_base and labels) to receive the unsigned kind 1618 event, sign it, then call again with the signed event.", mcp.Object(map[string]any{"event": mcpEvent, "owner": mcpPubKey, "repo": mcpID, "title": mcpText, "content": mcpText, "commit": mcpCommit, "clone": map[string]any{"type": "string", "description": "HTTP or HTTPS clone URL without credentials."}, "merge_base": mcpCommit, "labels": map[string]any{"type": "array", "items": mcpText}}), mcpPublishes, t.mcpWrite(mcpBuildPull, mcpCheckPull, mcpPullShape))175- add("comment", "Reply to an issue, pull request or comment with a NIP-22 kind 1111 event. Pass owner, repo, root, root_kind, root_pubkey and content (plus parent, parent_kind and parent_pubkey to answer a comment) to receive the unsigned event, sign it, then call again with the signed event.", mcp.Object(map[string]any{"event": mcpEvent, "owner": mcpPubKey, "repo": mcpID, "root": mcpHash, "root_kind": mcpRootKind, "root_pubkey": mcpPubKey, "parent": mcpHash, "parent_kind": map[string]any{"type": "integer", "enum": []int{1111, 1617, 1618, 1621}}, "parent_pubkey": mcpPubKey, "content": mcpText}), mcpPublishes, t.mcpWrite(mcpBuildComment, mcpCheckComment, mcpCommentShape))175+ add("comment", "Reply to an issue, pull request or comment with a NIP-22 kind 1111 event. Pass owner, repo, root, root_kind, root_pubkey and content (plus parent, parent_kind and parent_pubkey to answer a comment) to receive the unsigned event, sign it, then call again with the signed event. To review one line of a pull request or patch diff, add file, line and side (old for the base, new for the change).", mcp.Object(map[string]any{"event": mcpEvent, "owner": mcpPubKey, "repo": mcpID, "root": mcpHash, "root_kind": mcpRootKind, "root_pubkey": mcpPubKey, "parent": mcpHash, "parent_kind": map[string]any{"type": "integer", "enum": []int{1111, 1617, 1618, 1621}}, "parent_pubkey": mcpPubKey, "content": mcpText, "file": map[string]any{"type": "string", "minLength": 1, "description": "Path of the diff file the comment is about."}, "line": map[string]any{"type": "integer", "minimum": 1, "description": "Line number on the chosen side of the diff."}, "side": map[string]any{"type": "string", "enum": []string{"old", "new"}}}), mcpPublishes, t.mcpWrite(mcpBuildComment, mcpCheckComment, mcpCommentShape))176176 add("set_status", "Change the status of an issue or pull request. The author, repository owner and maintainers may do this. Pass owner, repo, root, root_pubkey and status to receive the unsigned event, sign it, then call again with the signed event.", mcp.Object(map[string]any{"event": mcpEvent, "owner": mcpPubKey, "repo": mcpID, "root": mcpHash, "root_pubkey": mcpPubKey, "status": map[string]any{"type": "string", "enum": []string{"open", "resolved", "merged", "closed", "draft"}}}), mcpPublishes, t.mcpWrite(mcpBuildStatus, mcpCheckStatus, mcpStatusShape))177177 add("post_message", "Post a kind 9 chat message in a room. Pass room and content (plus mentions, a list of public keys) to receive the unsigned event, sign it, then call again with the signed event. Posting in an open room joins it.", mcp.Object(map[string]any{"event": mcpEvent, "room": mcpRoomID, "content": mcpText, "mentions": map[string]any{"type": "array", "items": mcpPubKey}}), mcpPublishes, t.mcpWrite(mcpBuildMessage, mcpCheckMessage, mcpMessageShape))178178 add("start_thread", "Start a kind 11 thread in a room. Pass room and content (plus an optional title) to receive the unsigned event, sign it, then call again with the signed event.", mcp.Object(map[string]any{"event": mcpEvent, "room": mcpRoomID, "title": mcpText, "content": mcpText}), mcpPublishes, t.mcpWrite(mcpBuildThread, mcpCheckThread, mcpThreadShape))@@ -338,6 +338,16 @@ func mcpBuildComment(call mcp.Call) (mcpUnsigned, error) {
338338 return mcpUnsigned{}, errors.New("parent needs parent_kind and parent_pubkey")339339 }340340 tags = append(tags, []string{"E", root, "", rootPubKey}, []string{"K", strconv.Itoa(rootKind)}, []string{"P", rootPubKey}, []string{"e", parent, "", parentPubKey}, []string{"k", strconv.Itoa(parentKind)}, []string{"p", parentPubKey})341+ file, line, side := call.String("file"), call.Int("line"), call.String("side")342+ if file != "" || line != 0 || side != "" {343+ if strings.TrimSpace(file) == "" || line < 1 || (side != "old" && side != "new") {344+ return mcpUnsigned{}, errors.New("file, line (a positive integer) and side (old or new) go together")345+ }346+ if rootKind != event.KIND_GIT_PR && rootKind != event.KIND_GIT_PATCH {347+ return mcpUnsigned{}, errors.New("file and line anchor a comment to a pull request or patch diff")348+ }349+ tags = append(tags, []string{"file", file}, []string{"line", strconv.Itoa(line), side})350+ }341351 return mcpUnsigned{Kind: 1111, CreatedAt: time.Now().Unix(), Tags: tags, Content: content}, nil342352 }343353 @@ -420,6 +430,9 @@ func mcpCheckComment(e event.Event) error {
420430 if strings.TrimSpace(e.Content) == "" {421431 return errors.New("content must not be empty")422432 }433+ if err := validateReviewAnchor(e); err != nil {434+ return errors.New(strings.TrimPrefix(err.Error(), "blocked: "))435+ }423436 return nil424437 }425438 @@ -631,6 +631,9 @@ func (t *Tenant) commitImported(ctx context.Context, e event.Event, origin repli
631631 if err := t.validatePrivateRepositoryPlacement(ctx, e); err != nil {632632 return err633633 }634+ if err := validateReviewAnchor(e); err != nil {635+ return err636+ }634637 if t.sites != nil {635638 if err := sites.ValidateManifest(e); err != nil {636639 return err@@ -178,6 +178,9 @@ func (t *Tenant) Publish(ctx context.Context, e event.Event, s relay.Session) (s
178178 if err := t.validatePrivateRepositoryPlacement(ctx, e); err != nil {179179 return "", err180180 }181+ if err := validateReviewAnchor(e); err != nil {182+ return "", err183+ }181184 if err := t.validatePushRegistration(ctx, e, s); err != nil {182185 return "", err183186 }@@ -18,20 +18,23 @@
1818 <label>Description <textarea name="content" required></textarea></label><button>Sign and publish</button>1919 </nostr-compose><noscript><p>JavaScript and a connected signer are required to publish.</p></noscript></details>{{else}}<p><a href="/signin">Sign in</a> to start a conversation.</p>{{end}}2020 </section>21-{{else}}{{$item := asMap (index $data "item")}}<section id="collaboration-detail">21+{{else}}{{$item := asMap (index $data "item")}}{{$diff := reviewDiff .Event .Query .Actor}}{{$anchor := reviewAnchor .Query}}<section id="collaboration-detail">2222 <p><a href="{{if $issues}}{{repoURL .Query "issues" ""}}{{else}}{{repoURL .Query "prs" ""}}{{end}}">{{if $issues}}All issues{{else}}All pull requests{{end}}</a></p>2323 <h1>{{index $item "title"}}</h1>2424 <p>{{index $item "status"}} by {{template "nostrKey" (str (index $item "author"))}} <time datetime="{{datetime (index $item "created_at")}}">{{when (index $item "created_at")}}</time></p>2525 {{with collabLabels $item}}<p>Labels: {{join . ", "}}</p>{{end}}2626 <article id="collaboration-body">{{markdown (str (index $item "content"))}}</article>27-{{if not $issues}}<h2>Changes</h2>{{if index $data "diff_unavailable"}}<p>{{index $data "diff_unavailable"}}. The diff will be available after the required commits have been synchronized.</p>{{else}}{{diffHTML .Event}}{{if index $data "diff_truncated"}}<p>This diff is truncated. Clone the repository to review the complete change.</p>{{end}}{{end}}{{end}}27+{{if not $issues}}<h2>Changes</h2>{{if index $data "diff_unavailable"}}<p>{{index $data "diff_unavailable"}}. The diff will be available after the required commits have been synchronized.</p>{{else}}<diff-view><table><tbody>28+{{range $line := $diff.Lines}}<tr id="D{{$line.Number}}"><td><a href="#D{{$line.Number}}">{{$line.Number}}</a></td><td><code>{{$line.HTML}}</code>{{with $line.CommentURL}} <small><a href="{{.}}">comment</a></small>{{end}}</td></tr>29+{{range $c := $line.Comments}}{{$r := asMap $c}}<tr><td></td><td>{{template "nostrEvent" $c}}{{if $.Actor}}<details><summary>Reply to this comment</summary><nostr-compose kind="1111" coordinate="{{$coordinate}}" root="{{index $item "id"}}" root-pubkey="{{index $item "author"}}" root-kind="{{index $item "kind"}}" parent="{{index $r "id"}}" parent-pubkey="{{index $r "pubkey"}}" parent-kind="1111"><label>Reply <textarea name="content" required></textarea></label><button>Sign and reply</button></nostr-compose></details>{{end}}</td></tr>30+{{end}}{{end}}</tbody>{{if $diff.Truncated}}<tfoot><tr><td colspan="2">Diff preview truncated (256 KiB or 10,000 lines).</td></tr></tfoot>{{end}}</table></diff-view>{{if index $data "diff_truncated"}}<p>This diff is truncated. Clone the repository to review the complete change.</p>{{end}}{{end}}{{end}}2831 <h2>Conversation</h2>29-{{$replies := collabReplies .Event}}{{if $replies}}<ol id="events">30-{{range $reply := $replies}}{{$r := asMap $reply}}<li>{{template "nostrEvent" $reply}}32+{{$replies := $diff.Thread}}{{if $replies}}<ol id="events">33+{{range $reply := $replies}}{{$r := asMap $reply}}<li>{{template "nostrEvent" $reply}}{{with reviewLabel $reply}}<p><small>on {{.}}</small></p>{{end}}3134 {{if $.Actor}}<details><summary>Reply to this comment</summary><nostr-compose kind="1111" coordinate="{{$coordinate}}" root="{{index $item "id"}}" root-pubkey="{{index $item "author"}}" root-kind="{{index $item "kind"}}" parent="{{index $r "id"}}" parent-pubkey="{{index $r "pubkey"}}" parent-kind="1111"><label>Reply <textarea name="content" required></textarea></label><button>Sign and reply</button></nostr-compose></details>{{end}}</li>3235 {{end}}</ol>{{else}}<p>No replies yet.</p>{{end}}3336 {{with index $data "reply_cursor"}}<p id="pages"><a href="{{browsePageURL "/repo" $.Query "cursor" .}}">older replies</a></p>{{end}}34-{{if .Actor}}<h2>Reply</h2><nostr-compose kind="1111" coordinate="{{$coordinate}}" root="{{index $item "id"}}" root-pubkey="{{index $item "author"}}" root-kind="{{index $item "kind"}}"><label>Comment <textarea name="content" required></textarea></label><button>Sign and reply</button></nostr-compose>37+{{if .Actor}}<h2>Reply</h2>{{with $anchor}}<p id="reply-anchor">Commenting on <code>{{.File}}</code> line {{.Line}} ({{.Side}} side). <a href="{{repoURL $.Query "pr" (str (index $item "id"))}}">Comment on the whole change instead</a></p>{{end}}<nostr-compose id="reply-compose" kind="1111" coordinate="{{$coordinate}}" root="{{index $item "id"}}" root-pubkey="{{index $item "author"}}" root-kind="{{index $item "kind"}}">{{with $anchor}}<input type="hidden" name="file" value="{{.File}}"><input type="hidden" name="line" value="{{.Line}}"><input type="hidden" name="side" value="{{.Side}}">{{end}}<label>Comment <textarea name="content" required></textarea></label><button>Sign and reply</button></nostr-compose>3538 {{if index $data "can_status"}}<details id="status-controls"><summary>Change status</summary><nostr-compose kind="status" coordinate="{{$coordinate}}" root="{{index $item "id"}}" root-pubkey="{{index $item "author"}}" root-kind="{{index $item "kind"}}"><label>Status <select name="status"><option value="1630">Reopen</option><option value="1631">{{if $issues}}Resolve{{else}}Mark merged{{end}}</option><option value="1632">Close</option><option value="1633">Draft</option></select></label><label>Comment <textarea name="content"></textarea></label><button>Sign and update</button></nostr-compose></details>{{end}}3639 <noscript><p>JavaScript and a connected signer are required to publish.</p></noscript>{{else}}<p><a href="/signin">Sign in</a> to reply.</p>{{end}}3740 </section>{{end}}{{end}}@@ -27,9 +27,11 @@
2727 // <file-tools hash="…" type="…" size="…">2828 // Shares one stored file: copies its Blossom URI or share link, decrypts2929 // a share link fragment and sends a random-key file as a NIP-17 message.30-// <nostr-compose kind="…" [coordinate="30617:…"] [root="…" root-pubkey="…" root-kind="…"] [parent="…" …]>30+// <nostr-compose kind="…" [coordinate="30617:…"] [root="…" root-pubkey="…" root-kind="…"] [parent="…" …] [file="…" line="…" side="old|new"]>3131 // Signs a NIP-34 issue, pull request, NIP-22 reply or status event and3232 // posts it to the relay. A reply outside a repository omits the coordinate.33+// A reply under a pull request or patch may name one diff line through34+// the file, line and side attributes or hidden inputs of those names.3335 // <wiki-compose name="…" [author="…" coordinate="30818:…" event="…"]>3436 // Signs a NIP-54 article and posts it. Someone other than the author3537 // publishes a fork of the version in view; a "propose" button also signs@@ -349,6 +351,15 @@
349351 // The relay receives the signed event directly, so the same control works350352 // with NIP-07 and the existing NIP-46 bridge.351353 class NostrCompose extends FormElement {354+ connectedCallback() {355+ super.connectedCallback();356+ // A comment link on a diff line opens the page at this compose.357+ if (this.id && globalThis.location?.hash === "#" + this.id) {358+ this.scrollIntoView?.({block: "center"});359+ this.form?.elements.content?.focus?.();360+ }361+ }362+352363 async submit(form) {353364 if (!window.nostr?.signEvent) throw Error("Connect a signer first.");354365 const value = name => form.elements[name]?.value.trim() || "";@@ -370,6 +381,13 @@
370381 const parentKind = this.getAttribute("parent-kind") || rootKind;371382 if (!isHex64(parent) || !isHex64(parentPubkey) || !["1111", rootKind].includes(parentKind)) throw Error("The reply address is invalid.");372383 tags.push(["E", root, "", pubkey], ["K", rootKind], ["P", pubkey], ["e", parent, "", parentPubkey], ["k", parentKind], ["p", parentPubkey]);384+ const anchor = name => value(name) || this.getAttribute(name) || "";385+ const file = anchor("file"), line = anchor("line"), side = anchor("side");386+ if (file || line || side) {387+ if (!["1617", "1618"].includes(rootKind)) throw Error("Line comments belong to a pull request or patch.");388+ if (!file || !/^[1-9]\d*$/.test(line) || !["old", "new"].includes(side)) throw Error("The line reference is incomplete: it needs a file, a line number and a side.");389+ tags.push(["file", file], ["line", line, side]);390+ }373391 } else {374392 tags.push(["e", root, "", "root"], ["p", pubkey]);375393 if (coordinate.split(":")[1] !== pubkey) tags.push(["p", coordinate.split(":")[1]]);@@ -0,0 +1,240 @@
1+package webui2+3+import (4+ "html/template"5+ "net/url"6+ "strconv"7+ "strings"8+)9+10+// Review rendering for the pull request page. The diff is split into rows11+// that know their file and their line number on each side, so a comment12+// carrying file and line tags can be placed under the row it names and every13+// row can offer a link that opens the compose with that line preset.14+15+// diffLine is one row of a unified diff. Old and New are the line numbers on16+// each side of the hunk the row belongs to; a row outside a hunk, or on the17+// side it does not touch, has 0 there.18+type diffLine struct {19+ Number int20+ Text string21+ Wrapper string22+ File string23+ Old int24+ New int25+}26+27+// diffLines parses rendered diff text into rows. Header lines keep the ins28+// and del markers of the plain renderer; only hunk bodies are numbered.29+func diffLines(lines []string) []diffLine {30+ out := make([]diffLine, 0, len(lines))31+ file := ""32+ old, next := 0, 033+ inHunk := false34+ for index, line := range lines {35+ row := diffLine{Number: index + 1, Text: line}36+ switch {37+ case strings.HasPrefix(line, "diff --git "):38+ inHunk, file = false, ""39+ case strings.HasPrefix(line, "@@"):40+ inHunk = true41+ old, next = hunkStarts(line)42+ row.Wrapper = "b"43+ case !inHunk && strings.HasPrefix(line, "+++ "):44+ row.Wrapper = "ins"45+ if path := diffPath(line[4:], "b/"); path != "" {46+ file = path47+ }48+ case !inHunk && strings.HasPrefix(line, "--- "):49+ row.Wrapper = "del"50+ if path := diffPath(line[4:], "a/"); path != "" && file == "" {51+ file = path52+ }53+ case inHunk && strings.HasPrefix(line, "+"):54+ row.Wrapper, row.New = "ins", next55+ next++56+ case inHunk && strings.HasPrefix(line, "-"):57+ row.Wrapper, row.Old = "del", old58+ old++59+ case inHunk && strings.HasPrefix(line, "\\"):60+ // "\ No newline at end of file" belongs to no line.61+ case inHunk:62+ row.Old, row.New = old, next63+ old++64+ next++65+ case strings.HasPrefix(line, "+"):66+ row.Wrapper = "ins"67+ case strings.HasPrefix(line, "-"):68+ row.Wrapper = "del"69+ }70+ row.File = file71+ out = append(out, row)72+ }73+ return out74+}75+76+// hunkStarts reads the old and new starting line numbers of a hunk header77+// such as "@@ -12,7 +12,8 @@".78+func hunkStarts(header string) (int, int) {79+ fields := strings.Fields(header)80+ if len(fields) < 3 {81+ return 0, 082+ }83+ start := func(value, prefix string) int {84+ value = strings.TrimPrefix(value, prefix)85+ if comma := strings.IndexByte(value, ','); comma >= 0 {86+ value = value[:comma]87+ }88+ n, err := strconv.Atoi(value)89+ if err != nil || n < 0 {90+ return 091+ }92+ return n93+ }94+ return start(fields[1], "-"), start(fields[2], "+")95+}96+97+// diffPath reads the path from a "--- a/path" or "+++ b/path" header. Git98+// quotes paths with unusual characters; /dev/null means no file on that side.99+func diffPath(value, prefix string) string {100+ value = strings.TrimSpace(value)101+ if tab := strings.IndexByte(value, '\t'); tab >= 0 {102+ value = value[:tab]103+ }104+ if strings.HasPrefix(value, `"`) {105+ if unquoted, err := strconv.Unquote(value); err == nil {106+ value = unquoted107+ }108+ }109+ if value == "/dev/null" {110+ return ""111+ }112+ return strings.TrimPrefix(value, prefix)113+}114+115+// reviewLine is one diff row ready for the template: its escaped markup,116+// the link that starts a comment on it and the comments already anchored to117+// it, oldest first.118+type reviewLine struct {119+ diffLine120+ HTML template.HTML121+ CommentURL string122+ Comments []any123+}124+125+// reviewDiff is the pull request diff with its anchored comments placed.126+// Thread holds the replies that are not shown inline, so the conversation127+// below the diff lists each comment once.128+type reviewDiff struct {129+ Available bool130+ Truncated bool131+ Lines []reviewLine132+ Thread []any133+}134+135+// reviewDiffView builds the diff for a pull request page. A signed-in136+// viewer gets a comment link on each numbered line. Anchored comments whose137+// line is not in the diff stay in the thread with their anchor shown.138+func reviewDiffView(value any, query url.Values, actor string) reviewDiff {139+ m := valueMap(value)140+ replies := collaborationReplies(value)141+ view := reviewDiff{Thread: replies}142+ diff, _ := m["diff"].(string)143+ if diff == "" {144+ return view145+ }146+ view.Available = true147+ providedTruncated, _ := m["diff_truncated"].(bool)148+ view.Truncated = providedTruncated || len(diff) > sourcePreviewLimit149+ if view.Truncated {150+ diff = boundedContent(diff)151+ }152+ lines, linesTruncated := previewLines(diff)153+ view.Truncated = view.Truncated || linesTruncated154+ rows := diffLines(lines)155+ view.Lines = make([]reviewLine, 0, len(rows))156+ index := make(map[string]int, len(rows))157+ for _, row := range rows {158+ var markup strings.Builder159+ writeWrapped(&markup, row.Wrapper, row.Text)160+ line := reviewLine{diffLine: row, HTML: template.HTML(markup.String())}161+ if row.File != "" {162+ if row.New > 0 {163+ index[reviewKey(row.File, "new", row.New)] = len(view.Lines)164+ }165+ if row.Old > 0 {166+ index[reviewKey(row.File, "old", row.Old)] = len(view.Lines)167+ }168+ if actor != "" && (row.New > 0 || row.Old > 0) {169+ line.CommentURL = reviewCommentURL(query, row)170+ }171+ }172+ view.Lines = append(view.Lines, line)173+ }174+ view.Thread = view.Thread[:0]175+ placed := make(map[int][]any)176+ for _, reply := range replies {177+ r := valueMap(reply)178+ file := plainString(r["file"])179+ lineNumber := int(unixSeconds(r["line"]))180+ side := plainString(r["side"])181+ at, ok := index[reviewKey(file, side, lineNumber)]182+ if file == "" || lineNumber < 1 || !ok {183+ view.Thread = append(view.Thread, reply)184+ continue185+ }186+ placed[at] = append(placed[at], reply)187+ }188+ for at, comments := range placed {189+ // Replies arrive newest first; read a line's comments in order.190+ for left, right := 0, len(comments)-1; left < right; left, right = left+1, right-1 {191+ comments[left], comments[right] = comments[right], comments[left]192+ }193+ view.Lines[at].Comments = comments194+ }195+ return view196+}197+198+func reviewKey(file, side string, line int) string {199+ return side + ":" + strconv.Itoa(line) + ":" + file200+}201+202+// reviewCommentURL opens the pull request page with the compose preset to203+// one line. Added and context lines name the new side; removed lines the old.204+func reviewCommentURL(query url.Values, row diffLine) string {205+ id := query.Get("id")206+ if id == "" {207+ id = query.Get("path")208+ }209+ side, line := "new", row.New210+ if line == 0 {211+ side, line = "old", row.Old212+ }213+ values := url.Values{"file": {row.File}, "line": {strconv.Itoa(line)}, "side": {side}}214+ return repoURL(query, "pr", id) + "&" + values.Encode() + "#reply-compose"215+}216+217+// reviewAnchorQuery reads the line a compose should be preset to from the218+// page query. It returns nil unless every part is valid.219+func reviewAnchorQuery(query url.Values) map[string]any {220+ if repoView(query) != "pr" {221+ return nil222+ }223+ file, side := query.Get("file"), query.Get("side")224+ line, err := strconv.Atoi(query.Get("line"))225+ if strings.TrimSpace(file) == "" || err != nil || line < 1 || (side != "old" && side != "new") {226+ return nil227+ }228+ return map[string]any{"File": file, "Line": line, "Side": side}229+}230+231+// reviewAnchorLabel describes a reply's anchor for the thread, or returns232+// an empty string for an unanchored reply.233+func reviewAnchorLabel(reply any) string {234+ r := valueMap(reply)235+ file := plainString(r["file"])236+ if file == "" {237+ return ""238+ }239+ return file + " line " + plainString(r["line"]) + " (" + plainString(r["side"]) + ")"240+}@@ -0,0 +1,122 @@
1+package webui2+3+import (4+ "context"5+ "encoding/json"6+ "net/http"7+ "net/http/httptest"8+ "net/url"9+ "strings"10+ "testing"11+12+ "github.com/FelineStateMachine/tinyrelay/internal/policy"13+)14+15+const reviewTestDiff = "diff --git a/README b/README\nindex 1111111..2222222 100644\n--- a/README\n+++ b/README\n@@ -1,3 +1,4 @@\n first\n-second\n+two\n+three\n fourth\ndiff --git a/docs/new.md b/docs/new.md\nnew file mode 100644\n--- /dev/null\n+++ b/docs/new.md\n@@ -0,0 +1,2 @@\n+# New\n+body\n\\ No newline at end of file\n"16+17+func TestDiffLinesNumberBothSidesAndTrackFiles(t *testing.T) {18+ lines, _ := previewLines(reviewTestDiff)19+ rows := diffLines(lines)20+ want := map[int]diffLine{21+ 3: {Wrapper: "del", File: "README"},22+ 5: {Wrapper: "b", File: "README"},23+ 6: {File: "README", Old: 1, New: 1},24+ 7: {Wrapper: "del", File: "README", Old: 2},25+ 8: {Wrapper: "ins", File: "README", New: 2},26+ 9: {Wrapper: "ins", File: "README", New: 3},27+ 10: {File: "README", Old: 3, New: 4},28+ 14: {Wrapper: "ins", File: "docs/new.md"},29+ 16: {Wrapper: "ins", File: "docs/new.md", New: 1},30+ 17: {Wrapper: "ins", File: "docs/new.md", New: 2},31+ 18: {File: "docs/new.md"},32+ }33+ for number, expected := range want {34+ got := rows[number-1]35+ expected.Number, expected.Text = number, got.Text36+ if got != expected {37+ t.Fatalf("row %d = %+v, want %+v", number, got, expected)38+ }39+ }40+ if got := rows[11]; got.File != "" || got.Old != 0 || got.New != 0 {41+ t.Fatalf("a new diff header kept the previous file: %+v", got)42+ }43+}44+45+type reviewBackend struct{ *fakeBackend }46+47+func (b *reviewBackend) Query(ctx context.Context, method string, params []json.RawMessage, actor string) (any, error) {48+ b.call = method49+ b.params = params50+ owner := b.policy.Owner51+ root := map[string]any{"id": strings.Repeat("b", 64), "title": "Rename second", "kind": 1618, "author": owner, "content": "Why", "status": "open"}52+ reply := func(id, content string, extra ...[]string) map[string]any {53+ tags := append([][]string{{"a", "30617:" + owner + ":test"}, {"E", strings.Repeat("b", 64), "", owner}, {"K", "1618"}, {"P", owner}}, extra...)54+ return map[string]any{"id": id, "kind": 1111, "pubkey": strings.Repeat("c", 64), "created_at": 1700000000, "content": content, "tags": tags}55+ }56+ anchored := reply(strings.Repeat("1", 64), "Prefer a longer word.", []string{"file", "README"}, []string{"line", "2", "new"})57+ anchored["file"], anchored["line"], anchored["side"] = "README", 2, "new"58+ lost := reply(strings.Repeat("2", 64), "Outside the diff.", []string{"file", "missing.txt"}, []string{"line", "9", "old"})59+ lost["file"], lost["line"], lost["side"] = "missing.txt", 9, "old"60+ plain := reply(strings.Repeat("3", 64), "Looks good overall.")61+ return map[string]any{"item": root, "replies": []any{plain, lost, anchored}, "can_status": true, "diff": reviewTestDiff, "repository": map[string]any{"owner": owner, "clone": []string{"https://git.example/test.git"}}}, nil62+}63+64+func TestPullRequestPagePlacesAnchoredCommentsUnderDiffLines(t *testing.T) {65+ for _, viewer := range []string{"", strings.Repeat("a", 64)} {66+ backend := &reviewBackend{&fakeBackend{policy: policy.Defaults(strings.Repeat("a", 64))}}67+ app, err := New(backend, Options{Actor: func(*http.Request) (string, error) { return viewer, nil }})68+ if err != nil {69+ t.Fatal(err)70+ }71+ query := url.Values{"owner": {backend.policy.Owner}, "repo": {"test"}}72+ link := repoURL(query, "pr", strings.Repeat("b", 64))73+ w := httptest.NewRecorder()74+ app.ServeHTTP(w, httptest.NewRequest(http.MethodGet, link, nil))75+ body := w.Body.String()76+ if w.Code != http.StatusOK || backend.call != "browsepull" {77+ t.Fatalf("viewer %q: %d %s", viewer, w.Code, body)78+ }79+ if strings.Contains(body, "class=") {80+ t.Fatalf("viewer %q: markup carries class attributes", viewer)81+ }82+ // The anchored comment sits in the row after new line 2 of README,83+ // which is rendered diff row 8; the plain and unplaced comments stay84+ // in the thread, the unplaced one with its anchor shown.85+ row := strings.Index(body, `<tr id="D8">`)86+ nextRow := strings.Index(body, `<tr id="D9">`)87+ if row < 0 || nextRow < row || !strings.Contains(body[row:nextRow], "Prefer a longer word.") || !strings.Contains(body[row:nextRow], `<nostr-event id="event-`+strings.Repeat("1", 64)+`">`) {88+ t.Fatalf("viewer %q: anchored comment not placed under its line: %s", viewer, body[row:nextRow])89+ }90+ thread := body[strings.Index(body, `<ol id="events">`):]91+ if strings.Contains(thread, "Prefer a longer word.") || !strings.Contains(thread, "Looks good overall.") || !strings.Contains(thread, "on missing.txt line 9 (old)") {92+ t.Fatalf("viewer %q: thread = %s", viewer, thread)93+ }94+ commentLink := strings.Contains(body, `file=README&line=2&side=new#reply-compose">comment</a>`)95+ removedLink := strings.Contains(body, `file=README&line=2&side=old#reply-compose">comment</a>`)96+ if viewer == "" {97+ if commentLink || strings.Contains(body, "<nostr-compose") {98+ t.Fatal("guest received comment links or a compose")99+ }100+ continue101+ }102+ if !commentLink || !removedLink || !strings.Contains(body, `&file=docs%2Fnew.md&line=1&side=new#reply-compose">comment</a>`) {103+ t.Fatalf("comment links missing: %s", body)104+ }105+ if strings.Contains(body, `<input type="hidden" name="file"`) {106+ t.Fatal("compose was preset without a query anchor")107+ }108+ w = httptest.NewRecorder()109+ app.ServeHTTP(w, httptest.NewRequest(http.MethodGet, link+"&file=README&line=2&side=new", nil))110+ body = w.Body.String()111+ for _, want := range []string{`<nostr-compose id="reply-compose" kind="1111"`, `<input type="hidden" name="file" value="README">`, `<input type="hidden" name="line" value="2">`, `<input type="hidden" name="side" value="new">`, `<p id="reply-anchor">Commenting on <code>README</code> line 2 (new side).`} {112+ if !strings.Contains(body, want) {113+ t.Fatalf("preset compose missing %s: %s", want, body)114+ }115+ }116+ w = httptest.NewRecorder()117+ app.ServeHTTP(w, httptest.NewRequest(http.MethodGet, link+"&file=README&line=0&side=left", nil))118+ if strings.Contains(w.Body.String(), `name="file"`) {119+ t.Fatal("an invalid query anchor preset the compose")120+ }121+ }122+}@@ -214,6 +214,10 @@ source-view b{font-weight:600}source-view i{font-style:normal;color:var(--muted)
214214 source-view tfoot td,diff-view tfoot td{white-space:normal;color:var(--muted);font-family:var(--sans)}215215 diff-view ins,diff-view del{text-decoration:none}diff-view tr:has(ins){background:var(--add)}diff-view tr:has(del){background:var(--del)}diff-view b{color:var(--muted);font-weight:600}216216 diff-view caption{text-align:left;font:13px var(--mono);padding:8px 12px;border-bottom:1px solid var(--line);caption-side:top}217+/* Review comments: the link that starts one on a diff line, and the comments placed under that line. */218+diff-view td>small{margin-left:12px;font:11px var(--sans)}diff-view td>small a{color:var(--muted);text-decoration:none}diff-view tr:hover td>small a,diff-view td>small a:focus{color:var(--fg);text-decoration:underline}219+diff-view td>nostr-event{white-space:normal;max-width:720px;border:0;padding:6px 0 4px}diff-view td>details{white-space:normal;font:12px var(--mono);margin:0 0 8px}diff-view td>details>summary{cursor:pointer;color:var(--muted)}diff-view td>details nostr-compose{border-bottom:0;padding:6px 0 0}220+#reply-anchor{font:13px var(--sans);color:var(--muted)}#events li>p{margin:-6px 0 8px;font:12px var(--mono);color:var(--muted)}217221 218222 /* Wiki: the page list, one article, the merge request bar and the editor. */219223 #wiki-list #rows th{white-space:nowrap}#wiki-list #rows td:first-child{width:auto;font:14px var(--sans)}#wiki-list #rows td:nth-child(3),#wiki-list #rows td:nth-child(4){width:110px;font:12px var(--mono);color:var(--muted)}@@ -250,6 +250,9 @@ func parseTemplates() (*template.Template, error) {
250250 "collabItems": collaborationItems,251251 "collabReplies": collaborationReplies,252252 "collabLabels": collaborationLabels,253+ "reviewDiff": reviewDiffView,254+ "reviewAnchor": reviewAnchorQuery,255+ "reviewLabel": reviewAnchorLabel,253256 "agentState": agentState,254257 "agentSince": agentSince,255258 "agentScope": agentScope,@@ -47,6 +47,35 @@ test("nested NIP-22 replies preserve the root and address the immediate parent",
4747 assert.deepEqual(s.sent[0].event.tags, [["a", coordinate], ["E", root, "", author], ["K", "1621"], ["P", author], ["e", parent, "", parentAuthor], ["k", "1111"], ["p", parentAuthor]]);4848 });4949 50+test("review comments carry file and line tags from hidden inputs or attributes", async () => {51+ const pullAttrs = {coordinate, root, "root-pubkey": author, "root-kind": "1618"};52+ const fromInputs = setup({...pullAttrs, kind: "1111"}, {content: "Prefer two", file: "README", line: "2", side: "new"});53+ await fromInputs.component.submit(fromInputs.form);54+ assert.deepEqual(fromInputs.sent[0].event.tags.slice(-2), [["file", "README"], ["line", "2", "new"]]);55+ const fromAttributes = setup({...pullAttrs, kind: "1111", file: "src/app.js", line: "14", side: "old"}, {content: "Keep this"});56+ await fromAttributes.component.submit(fromAttributes.form);57+ assert.deepEqual(fromAttributes.sent[0].event.tags.slice(-2), [["file", "src/app.js"], ["line", "14", "old"]]);58+ const plain = setup({...pullAttrs, kind: "1111"}, {content: "General note"});59+ await plain.component.submit(plain.form);60+ assert.equal(plain.sent[0].event.tags.some(tag => tag[0] === "file" || tag[0] === "line"), false);61+});62+63+test("incomplete or misplaced line references are rejected before signing", async () => {64+ const pullAttrs = {coordinate, root, "root-pubkey": author, "root-kind": "1618"};65+ for (const [attrs, values] of [66+ [{...pullAttrs, kind: "1111"}, {content: "x", file: "README", line: "0", side: "new"}],67+ [{...pullAttrs, kind: "1111"}, {content: "x", file: "README", line: "2", side: "left"}],68+ [{...pullAttrs, kind: "1111"}, {content: "x", line: "2", side: "new"}],69+ [{...pullAttrs, kind: "1111"}, {content: "x", file: "README"}],70+ [{...rootAttrs, kind: "1111"}, {content: "x", file: "README", line: "2", side: "new"}]71+ ]) {72+ const s = setup(attrs, values);73+ await assert.rejects(s.component.submit(s.form), /line|pull request/);74+ assert.equal(s.signed.length, 0);75+ assert.equal(s.sent.length, 0);76+ }77+});78+5079 test("status selector signs a NIP-34 root marker with an optional empty body", async () => {5180 const s = setup({...rootAttrs, kind: "status"}, {status: "1632", content: ""});5281 await s.component.submit(s.form);