diff --git a/appview/issues/issues.go b/appview/issues/issues.go --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -329,6 +329,11 @@ func (rp *Issues) CloseIssue(w http.ResponseWriter, r *http.Request) { l := rp.logger.With("handler", "CloseIssue") user := rp.oauth.GetMultiAccountUser(r) + if user == nil { + l.Error("nil user") + rp.pages.Notice(w, "issue-action", "You must be logged in to close this issue.") + return + } f, err := rp.repoResolver.Resolve(r) if err != nil { l.Error("failed to get repo and knot", "err", err) @@ -349,6 +354,12 @@ // TODO: make this more granular if isIssueOwner || isRepoOwner || isCollaborator { + if err := rp.writeIssueStateRecord(r, user.Did, issue.AtUri(), models.StateClosed); err != nil { + l.Error("failed to write issue state record", "err", err) + rp.pages.Notice(w, "issue-action", "Failed to close issue. Try again later.") + return + } + err = db.CloseIssues( rp.db, orm.FilterEq("id", issue.Id), @@ -377,6 +388,11 @@ func (rp *Issues) ReopenIssue(w http.ResponseWriter, r *http.Request) { l := rp.logger.With("handler", "ReopenIssue") user := rp.oauth.GetMultiAccountUser(r) + if user == nil { + l.Error("nil user") + rp.pages.Notice(w, "issue-action", "You must be logged in to reopen this issue.") + return + } f, err := rp.repoResolver.Resolve(r) if err != nil { l.Error("failed to get repo and knot", "err", err) @@ -396,6 +412,12 @@ isIssueOwner := user.Did == issue.Did if isCollaborator || isRepoOwner || isIssueOwner { + if err := rp.writeIssueStateRecord(r, user.Did, issue.AtUri(), models.StateOpen); err != nil { + l.Error("failed to write issue state record", "err", err) + rp.pages.Notice(w, "issue-action", "Failed to reopen issue. Try again later.") + return + } + err := db.ReopenIssues( rp.db, orm.FilterEq("id", issue.Id), @@ -419,6 +441,28 @@ http.Error(w, "forbidden", http.StatusUnauthorized) return } +} + +func (rp *Issues) writeIssueStateRecord(r *http.Request, actorDid string, subject syntax.ATURI, value models.StateValue) error { + client, err := rp.oauth.AuthorizedClient(r) + if err != nil { + return err + } + + record, err := models.AsIssueStateRecord(subject, value, time.Now()) + if err != nil { + return err + } + + _, err = comatproto.RepoPutRecord(r.Context(), client, &comatproto.RepoPutRecord_Input{ + Collection: tangled.RepoIssueStateNSID, + Repo: actorDid, + Rkey: tid.TID(), + Record: &lexutil.LexiconTypeDecoder{ + Val: &record, + }, + }) + return err } func (rp *Issues) RepoIssues(w http.ResponseWriter, r *http.Request) { diff --git a/appview/models/entity_state.go b/appview/models/entity_state.go --- a/appview/models/entity_state.go +++ b/appview/models/entity_state.go @@ -2,8 +2,10 @@ import ( "fmt" + "time" "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/samber/lo" "tangled.org/core/api/tangled" ) @@ -69,4 +71,43 @@ default: return StateRecord{}, fmt.Errorf("unknown pull status variant: %q", record.Status) } +} + +func AsIssueStateRecord(subject syntax.ATURI, value StateValue, createdAt time.Time) (tangled.RepoIssueState, error) { + var variant string + switch value { + case StateOpen: + variant = tangled.RepoIssueStateOpen + case StateClosed: + variant = tangled.RepoIssueStateClosed + default: + return tangled.RepoIssueState{}, fmt.Errorf("invalid issue state: %q", value) + } + return tangled.RepoIssueState{ + Issue: subject.String(), + State: variant, + CreatedAt: createdAt.UTC().Format(syntax.AtprotoDatetimeLayout), + }, nil +} + +func AsPullStatusRecords(subjects []syntax.ATURI, value StateValue, createdAt time.Time) ([]tangled.RepoPullStatus, error) { + var variant string + switch value { + case StateOpen: + variant = tangled.RepoPullStatusOpen + case StateClosed: + variant = tangled.RepoPullStatusClosed + case StateMerged: + variant = tangled.RepoPullStatusMerged + default: + return nil, fmt.Errorf("invalid pull status: %q", value) + } + created := createdAt.UTC().Format(syntax.AtprotoDatetimeLayout) + return lo.Map(subjects, func(subject syntax.ATURI, _ int) tangled.RepoPullStatus { + return tangled.RepoPullStatus{ + Pull: subject.String(), + Status: variant, + CreatedAt: created, + } + }), nil } diff --git a/appview/models/entity_state_test.go b/appview/models/entity_state_test.go --- a/appview/models/entity_state_test.go +++ b/appview/models/entity_state_test.go @@ -78,3 +78,69 @@ t.Fatal("unknown pull status variant must be rejected") } } + +func TestAsIssueStateRecord_RoundTrip(t *testing.T) { + const rkey = "3jzfcijpj2z2a" + subject := syntax.ATURI(testIssueSubject) + created := time.Date(2026, 6, 30, 12, 30, 15, int(123*time.Millisecond), time.UTC) + + for _, value := range []StateValue{StateOpen, StateClosed} { + rec, err := AsIssueStateRecord(subject, value, created) + if err != nil { + t.Fatalf("AsIssueStateRecord(%q): %v", value, err) + } + if rec.Issue != testIssueSubject { + t.Fatalf("Issue = %q, want %q", rec.Issue, testIssueSubject) + } + + back, err := IssueStateFromRecord("did:plc:akshay", rkey, rec) + if err != nil { + t.Fatalf("IssueStateFromRecord after AsIssueStateRecord(%q): %v", value, err) + } + if back.Value != value { + t.Fatalf("round-trip value = %q, want %q", back.Value, value) + } + if back.SortMicros != created.UnixMicro() { + t.Fatalf("round-trip SortMicros = %d, want %d", back.SortMicros, created.UnixMicro()) + } + } + + if _, err := AsIssueStateRecord(subject, StateMerged, created); err == nil { + t.Fatal("merged is not a valid issue state and must be rejected") + } +} + +func TestAsPullStatusRecords_RoundTrip(t *testing.T) { + const rkey = "3jzfcijpj2z2a" + subject := syntax.ATURI("at://did:plc:limpet/sh.tangled.repo.pull/p1") + created := time.Date(2026, 6, 30, 12, 30, 15, int(123*time.Millisecond), time.UTC) + + for _, value := range []StateValue{StateOpen, StateClosed, StateMerged} { + recs, err := AsPullStatusRecords([]syntax.ATURI{subject}, value, created) + if err != nil { + t.Fatalf("AsPullStatusRecords(%q): %v", value, err) + } + if len(recs) != 1 { + t.Fatalf("AsPullStatusRecords(%q) returned %d records, want 1", value, len(recs)) + } + rec := recs[0] + if rec.Pull != subject.String() { + t.Fatalf("Pull = %q, want %q", rec.Pull, subject) + } + + back, err := PullStatusFromRecord("did:plc:akshay", rkey, rec) + if err != nil { + t.Fatalf("PullStatusFromRecord after AsPullStatusRecords(%q): %v", value, err) + } + if back.Value != value { + t.Fatalf("round-trip value = %q, want %q", back.Value, value) + } + if back.SortMicros != created.UnixMicro() { + t.Fatalf("round-trip SortMicros = %d, want %d", back.SortMicros, created.UnixMicro()) + } + } + + if _, err := AsPullStatusRecords([]syntax.ATURI{subject}, "bogus", created); err == nil { + t.Fatal("invalid pull status value must be rejected") + } +} diff --git a/appview/oauth/scopes.go b/appview/oauth/scopes.go --- a/appview/oauth/scopes.go +++ b/appview/oauth/scopes.go @@ -19,8 +19,10 @@ "repo:sh.tangled.repo.collaborator", "repo:sh.tangled.repo.issue", "repo:sh.tangled.repo.issue.comment", + "repo:sh.tangled.repo.issue.state", "repo:sh.tangled.repo.pull", "repo:sh.tangled.repo.pull.comment", + "repo:sh.tangled.repo.pull.status", "repo:sh.tangled.spindle", "repo:sh.tangled.spindle.member", "repo:sh.tangled.string", diff --git a/appview/pulls/lifecycle.go b/appview/pulls/lifecycle.go --- a/appview/pulls/lifecycle.go +++ b/appview/pulls/lifecycle.go @@ -16,9 +16,12 @@ l := s.logger.With("handler", "ClosePull") user := s.oauth.GetMultiAccountUser(r) - if user != nil { - l = l.With("user", user.Did) + if user == nil { + l.Error("nil user") + s.pages.Notice(w, "pull-action-error", "You must be logged in to close this pull.") + return } + l = l.With("user", user.Did) f, err := s.repoResolver.Resolve(r) if err != nil { @@ -29,7 +32,7 @@ pull, ok := r.Context().Value("pull").(*models.Pull) if !ok { l.Error("failed to get pull") - s.pages.Notice(w, "pull-error", "Failed to edit patch. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to close pull. Try again later.") return } l = l.With("pull_id", pull.PullId, "pull_owner", pull.OwnerDid) @@ -42,18 +45,9 @@ isCloseAllowed := isOwner || isCollaborator || isPullAuthor if !isCloseAllowed { l.Error("unauthorized to close pull", "is_owner", isOwner, "is_collaborator", isCollaborator, "is_pull_author", isPullAuthor) - s.pages.Notice(w, "pull-close", "You are unauthorized to close this pull.") + s.pages.Notice(w, "pull-action-error", "You are unauthorized to close this pull.") return } - - // Start a transaction - tx, err := s.db.BeginTx(r.Context(), nil) - if err != nil { - l.Error("failed to start transaction", "err", err) - s.pages.Notice(w, "pull-close", "Failed to close pull.") - return - } - defer tx.Rollback() // if this PR is stacked, then we want to close all PRs above this one on the stack stack := r.Context().Value("stack").(models.Stack) @@ -63,6 +57,21 @@ atUris = append(atUris, p.AtUri()) p.State = models.PullClosed } + + if err := s.writePullStatusRecords(r, user.Did, atUris, models.StateClosed); err != nil { + l.Error("failed to write pull status records", "err", err) + s.pages.Notice(w, "pull-action-error", "Failed to close pull. Try again later.") + return + } + + tx, err := s.db.BeginTx(r.Context(), nil) + if err != nil { + l.Error("failed to start transaction", "err", err) + s.pages.Notice(w, "pull-action-error", "Failed to close pull.") + return + } + defer tx.Rollback() + err = db.ClosePulls( tx, orm.FilterEq("repo_did", string(f.RepoDid)), @@ -70,13 +79,14 @@ ) if err != nil { l.Error("failed to close pulls in database", "err", err, "pulls_to_close", len(pullsToClose)) - s.pages.Notice(w, "pull-close", "Failed to close pull.") + s.pages.Notice(w, "pull-action-error", "Failed to close pull.") + return } // Commit the transaction if err = tx.Commit(); err != nil { l.Error("failed to commit transaction", "err", err) - s.pages.Notice(w, "pull-close", "Failed to close pull.") + s.pages.Notice(w, "pull-action-error", "Failed to close pull.") return } @@ -92,21 +102,24 @@ l := s.logger.With("handler", "ReopenPull") user := s.oauth.GetMultiAccountUser(r) - if user != nil { - l = l.With("user", user.Did) + if user == nil { + l.Error("nil user") + s.pages.Notice(w, "pull-action-error", "You must be logged in to reopen this pull.") + return } + l = l.With("user", user.Did) f, err := s.repoResolver.Resolve(r) if err != nil { l.Error("failed to resolve repo", "err", err) - s.pages.Notice(w, "pull-reopen", "Failed to reopen pull.") + s.pages.Notice(w, "pull-action-error", "Failed to reopen pull.") return } pull, ok := r.Context().Value("pull").(*models.Pull) if !ok { l.Error("failed to get pull") - s.pages.Notice(w, "pull-error", "Failed to edit patch. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to reopen pull. Try again later.") return } l = l.With("pull_id", pull.PullId, "pull_owner", pull.OwnerDid, "state", pull.State) @@ -119,18 +132,9 @@ isCloseAllowed := isOwner || isCollaborator || isPullAuthor if !isCloseAllowed { l.Error("unauthorized to reopen pull", "is_owner", isOwner, "is_collaborator", isCollaborator, "is_pull_author", isPullAuthor) - s.pages.Notice(w, "pull-close", "You are unauthorized to close this pull.") + s.pages.Notice(w, "pull-action-error", "You are unauthorized to reopen this pull.") return } - - // Start a transaction - tx, err := s.db.BeginTx(r.Context(), nil) - if err != nil { - l.Error("failed to start transaction", "err", err) - s.pages.Notice(w, "pull-reopen", "Failed to reopen pull.") - return - } - defer tx.Rollback() // if this PR is stacked, then we want to reopen all PRs above this one on the stack stack := r.Context().Value("stack").(models.Stack) @@ -140,6 +144,21 @@ atUris = append(atUris, p.AtUri()) p.State = models.PullOpen } + + if err := s.writePullStatusRecords(r, user.Did, atUris, models.StateOpen); err != nil { + l.Error("failed to write pull status records", "err", err) + s.pages.Notice(w, "pull-action-error", "Failed to reopen pull. Try again later.") + return + } + + tx, err := s.db.BeginTx(r.Context(), nil) + if err != nil { + l.Error("failed to start transaction", "err", err) + s.pages.Notice(w, "pull-action-error", "Failed to reopen pull.") + return + } + defer tx.Rollback() + err = db.ReopenPulls( tx, orm.FilterEq("repo_did", string(f.RepoDid)), @@ -147,13 +166,14 @@ ) if err != nil { l.Error("failed to reopen pulls in database", "err", err, "pulls_to_reopen", len(pullsToReopen)) - s.pages.Notice(w, "pull-close", "Failed to reopen pull.") + s.pages.Notice(w, "pull-action-error", "Failed to reopen pull.") + return } // Commit the transaction if err = tx.Commit(); err != nil { l.Error("failed to commit transaction", "err", err) - s.pages.Notice(w, "pull-reopen", "Failed to reopen pull.") + s.pages.Notice(w, "pull-action-error", "Failed to reopen pull.") return } diff --git a/appview/pulls/merge.go b/appview/pulls/merge.go --- a/appview/pulls/merge.go +++ b/appview/pulls/merge.go @@ -20,14 +20,17 @@ l := s.logger.With("handler", "MergePull") user := s.oauth.GetMultiAccountUser(r) - if user != nil { - l = l.With("user", user.Did) + if user == nil { + l.Error("nil user") + s.pages.Notice(w, "pull-action-error", "You must be logged in to merge this pull.") + return } + l = l.With("user", user.Did) f, err := s.repoResolver.Resolve(r) if err != nil { l.Error("failed to resolve repo", "err", err) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to merge pull request. Try again later.") return } l = l.With("repo_at", f.RepoAt().String()) @@ -35,7 +38,7 @@ pull, ok := r.Context().Value("pull").(*models.Pull) if !ok { l.Error("failed to get pull") - s.pages.Notice(w, "pull-merge-error", "Failed to merge patch. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to merge patch. Try again later.") return } l = l.With("pull_id", pull.PullId, "target_branch", pull.TargetBranch) @@ -43,7 +46,7 @@ stack, ok := r.Context().Value("stack").(models.Stack) if !ok { l.Error("failed to get stack") - s.pages.Notice(w, "pull-merge-error", "Failed to merge patch. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to merge patch. Try again later.") return } @@ -94,34 +97,39 @@ ) if err != nil { l.Error("failed to connect to knot server", "err", err, "knot", f.Knot) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to merge pull request. Try again later.") return } err = tangled.RepoMerge(r.Context(), client, mergeInput) if xrpcerr := xrpcclient.HandleXrpcErr(err); xrpcerr != nil { s.logger.Error("failed to merge", "xrpcerr", xrpcerr, "err", err) - s.pages.Notice(w, "pull-merge-error", xrpcerr.Error()) + s.pages.Notice(w, "pull-action-error", xrpcerr.Error()) return } - - tx, err := s.db.Begin() - if err != nil { - l.Error("failed to start transaction", "err", err) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") - return - } - defer tx.Rollback() var atUris []syntax.ATURI for _, p := range pullsToMerge { atUris = append(atUris, p.AtUri()) p.State = models.PullMerged } + + if err := s.writePullStatusRecords(r, user.Did, atUris, models.StateMerged); err != nil { + l.Error("failed to write pull status records after merge", "err", err) + } + + tx, err := s.db.Begin() + if err != nil { + l.Error("failed to start transaction", "err", err) + s.pages.Notice(w, "pull-action-error", "Failed to merge pull request. Try again later.") + return + } + defer tx.Rollback() + err = db.MergePulls(tx, orm.FilterEq("repo_did", string(f.RepoDid)), orm.FilterIn("at_uri", atUris)) if err != nil { l.Error("failed to update pull request status in database", "err", err) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to merge pull request. Try again later.") return } @@ -129,7 +137,7 @@ if err != nil { // TODO: this is unsound, we should also revert the merge from the knotserver here l.Error("failed to commit merge transaction", "err", err) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") + s.pages.Notice(w, "pull-action-error", "Failed to merge pull request. Try again later.") return } diff --git a/appview/pulls/resubmit.go b/appview/pulls/resubmit.go --- a/appview/pulls/resubmit.go +++ b/appview/pulls/resubmit.go @@ -400,7 +400,7 @@ newStack, err := s.newStack(r.Context(), repo, userDid, targetBranch, pull.PullSource, formatPatches, blobs, nil, nil) if err != nil { l.Error("failed to create resubmitted stack", "err", err) - s.pages.Notice(w, "pull-merge-error", "Failed to merge pull request. Try again later.") + s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.") return } diff --git a/appview/pulls/state.go b/appview/pulls/state.go new file mode 100644 --- /dev/null +++ b/appview/pulls/state.go @@ -0,0 +1,50 @@ +package pulls + +import ( + "net/http" + "time" + + comatproto "github.com/bluesky-social/indigo/api/atproto" + "github.com/bluesky-social/indigo/atproto/syntax" + lexutil "github.com/bluesky-social/indigo/lex/util" + "github.com/samber/lo" + + "tangled.org/core/api/tangled" + "tangled.org/core/appview/models" + "tangled.org/core/tid" +) + +func (s *Pulls) writePullStatusRecords(r *http.Request, actorDid string, subjects []syntax.ATURI, value models.StateValue) error { + if len(subjects) == 0 { + return nil + } + + client, err := s.oauth.AuthorizedClient(r) + if err != nil { + return err + } + + records, err := models.AsPullStatusRecords(subjects, value, time.Now()) + if err != nil { + return err + } + + writes := lo.Map(records, func(record tangled.RepoPullStatus, _ int) *comatproto.RepoApplyWrites_Input_Writes_Elem { + rkey := tid.TID() + return &comatproto.RepoApplyWrites_Input_Writes_Elem{ + RepoApplyWrites_Create: &comatproto.RepoApplyWrites_Create{ + Collection: tangled.RepoPullStatusNSID, + Rkey: &rkey, + Value: &lexutil.LexiconTypeDecoder{ + Val: &record, + }, + }, + } + }) + + _, err = comatproto.RepoApplyWrites(r.Context(), client, &comatproto.RepoApplyWrites_Input{ + Repo: actorDid, + Writes: writes, + }) + return err +} diff --git a/appview/pages/templates/repo/pulls/pull.html b/appview/pages/templates/repo/pulls/pull.html --- a/appview/pages/templates/repo/pulls/pull.html +++ b/appview/pages/templates/repo/pulls/pull.html @@ -595,6 +595,7 @@ {{ if eq $lastIdx $item.RoundNumber }} {{ block "mergeStatus" $root }} {{ end }}
+
{{ end }}