Milestone 4b: pull request reviews and line comments (contract between workers)

The global rules from docs/m1-interfaces.md, docs/m3-interfaces.md and docs/m4-interfaces.md apply:

• No JS and no inline styles.
• Full hex OIDs only, validated paths, and user values after --end-of-options.
• No unwrap in non-test code.
• Tests use tempfile::TempDir.
• Never put cargo target dirs or big files in /tmp.

Concepts

• A review belongs to a PR (issue_id) and has an author. Its state is one of:
  • pending: a draft, visible only to its author, with at most one per author per PR.
  • commented, approved or changes_requested: submitted.
  • dismissed: a Write holder dismissed an approval or a change request, giving a reason.
• A review comment is a comment on one line of the PR diff, identified by:
  • path: the new path; for a deleted file, the old path;
  • side: new for added and context lines, old for deleted lines;
  • line: the line number on that side;
  • commit_oid: the head OID when it was written. It belongs to a review. A comment whose reply_to is set is a reply in the root comment's thread; replies are submitted immediately, through an auto-created commented review without a body. A root carries resolved.
• Outdated: after the head moves, a root comment is outdated when the blob at path differs between its commit_oid and the new head (this is coarse on purpose; line remapping comes later). Outdated threads still show in the conversation, with an "Outdated" badge, and no longer show on the Files tab.
• diff_hunk stores the last up to 4 diff lines ending at the commented line, as plain text with a +, - or space prefix, captured at creation. The conversation shows it as the thread's context without running git again.
• PR authors can't approve or request changes on their own PR; they can only comment. Anyone who can Read the base repo and is signed in can review. A review by a Read user counts in the summary, but merge rules don't enforce anything yet.

Schema (migration 0008_reviews.sql)

CREATE TABLE pull_reviews (
    id TEXT PRIMARY KEY, issue_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE,
    author_id TEXT NOT NULL REFERENCES users(id),
    state TEXT NOT NULL,                 -- pending | commented | approved | changes_requested | dismissed
    body TEXT NOT NULL DEFAULT '',
    commit_oid TEXT NOT NULL,            -- head at submit (or at creation while pending)
    dismissed_reason TEXT,
    created_at TEXT NOT NULL, submitted_at TEXT);
CREATE INDEX pull_reviews_issue ON pull_reviews(issue_id, submitted_at);
CREATE TABLE review_comments (
    id TEXT PRIMARY KEY, issue_id TEXT NOT NULL REFERENCES issues(id) ON DELETE CASCADE,
    review_id TEXT NOT NULL REFERENCES pull_reviews(id) ON DELETE CASCADE,
    author_id TEXT NOT NULL REFERENCES users(id),
    path TEXT NOT NULL, side TEXT NOT NULL, line INTEGER NOT NULL,
    commit_oid TEXT NOT NULL, diff_hunk TEXT NOT NULL DEFAULT '',
    reply_to TEXT REFERENCES review_comments(id) ON DELETE CASCADE,
    body TEXT NOT NULL, outdated INTEGER NOT NULL DEFAULT 0, resolved INTEGER NOT NULL DEFAULT 0,
    resolved_by TEXT REFERENCES users(id),
    created_at TEXT NOT NULL, updated_at TEXT NOT NULL);
CREATE INDEX review_comments_issue ON review_comments(issue_id, path, created_at);

Issue events gain the kinds review_dismissed (data {"review","reason"}), thread_resolved and thread_unresolved.

forge-core (src/reviews.rs model plus store/reviews.rs, an impl Store block)

pub enum ReviewState { Pending, Commented, Approved, ChangesRequested, Dismissed }   // as_str / FromStr
pub enum Side { Old, New }                                                          // "old" | "new"
pub struct Review { pub id: Id, pub issue_id: Id, pub author_id: Id, pub author_name: String, pub state: ReviewState,
    pub body: String, pub commit_oid: String, pub dismissed_reason: Option<String>, pub created_at: String, pub submitted_at: Option<String> }
pub struct ReviewComment { pub id: Id, pub review_id: Id, pub author_id: Id, pub author_name: String, pub path: String, pub side: Side,
    pub line: u32, pub commit_oid: String, pub diff_hunk: String, pub reply_to: Option<Id>, pub body: String,
    pub outdated: bool, pub resolved: bool, pub created_at: String, pub updated_at: String, pub pending: bool /* review is pending */ }
pub struct Thread { pub root: ReviewComment, pub replies: Vec<ReviewComment> }

// store/reviews.rs
pub async fn pending_review(&self, issue_id: &str, author_id: &str) -> Result<Option<Review>>;
pub async fn add_review_comment(&self, issue_id: &str, author_id: &str, pending: bool, path: &str, side: Side, line: u32,
    commit_oid: &str, diff_hunk: &str, body: &str) -> Result<ReviewComment>;
    // pending=true: put it in the author's pending review, creating that review if needed;
    // pending=false: create a submitted `commented` review holding just this comment
pub async fn reply_review_comment(&self, issue_id: &str, author_id: &str, root_id: &str, body: &str) -> Result<ReviewComment>;
    // root must be a submitted root comment of this PR; copies path/side/line/commit_oid
pub async fn submit_review(&self, issue_id: &str, author_id: &str, state: ReviewState /* not Pending/Dismissed */,
    body: &str, head_oid: &str) -> Result<Review>;
    // turns the pending review into `state` (or creates one with no comments); Commented with an empty body
    // and no comments is Invalid; the PR author's Approved or ChangesRequested is Forbidden
pub async fn discard_pending_review(&self, issue_id: &str, author_id: &str) -> Result<()>;
pub async fn dismiss_review(&self, review_id: &str, actor_id: &str, reason: &str) -> Result<()>;  // approved or changes_requested only; adds the event
pub async fn list_reviews(&self, issue_id: &str) -> Result<Vec<Review>>;                         // submitted ones, oldest first
pub async fn review_threads(&self, issue_id: &str, viewer_id: Option<&str>) -> Result<Vec<Thread>>;
    // submitted comments plus the viewer's own pending ones; replies oldest first
pub async fn review_comment(&self, id: &str) -> Result<Option<ReviewComment>>;
pub async fn edit_review_comment(&self, id: &str, body: &str) -> Result<()>;   // records edit_history target_kind 'review_comment'
pub async fn delete_review_comment(&self, id: &str) -> Result<()>;             // deleting a root deletes its replies
pub async fn set_thread_resolved(&self, root_id: &str, actor_id: &str, resolved: bool) -> Result<()>; // adds an event
pub async fn mark_outdated(&self, issue_id: &str, ids: &[Id]) -> Result<()>;
pub async fn review_summary(&self, issue_id: &str) -> Result<ReviewSummary>;
    // per reviewer, their latest non-commented, non-dismissed submitted state
pub struct ReviewSummary { pub approved_by: Vec<String>, pub changes_requested_by: Vec<String> }

Bodies use the same limits as issue comments. author_name comes from a join on users.

forge-git (added to src/pulls.rs)

pub async fn blob_at(env: &GitEnv, repo: &Path, commit: &str, path: &str) -> Result<Option<String>>; // `rev-parse <commit>:<path>` gives the blob oid; None if absent. Validate path: no NUL, not absolute, no `..` segments

Outdated marking (forge-web pulls_sync.rs, extending the head_updated path)

After update_pull_head, for each submitted root comment that isn't outdated, compare blob_at(old commit_oid, path) with blob_at(new_head, path). Mark it outdated if they differ (cache by (commit_oid, path)).

Web

• Files tab /pulls/{n}/files:
  • Line anchors. Each diff line gets an id L-<file_index>-<side>-<line> and a small "+" link to ?comment=<file_index>:<side>:<line>#L-…. Use file_index (the file's position in the diff), not the path, to keep URLs short. The server maps it back to the path and re-validates it against the current diff.
  • Inline comment form. When comment= matches a line, render a form right under that line. It POSTs /pulls/{n}/review_comments with csrf, file_index, side, line, head_oid (rejected if stale: "the pull request changed, reload"), body, and two buttons: action=single ("Add single comment") or action=review ("Start a review" / "Add review comment" if one is pending).
  • Threads. Existing, non-outdated threads on the current diff render under their line: comments (markdown-rendered like issue comments), a reply form, a Resolve/Unresolve button (PR author or Write), and edit/delete for the author.
  • Review bar at the top for signed-in viewers: "Finish your review" with a pending comment count, a body textarea, radio comment/approve/request_changes (approve and request changes hidden for the PR author), Submit and Discard. Posts to /pulls/{n}/reviews or /pulls/{n}/reviews/discard.
  • The diff_section component gains an optional slot. Implement it as an optional &HashMap<(usize, Side, u32), Vec<...>> of rendered thread data plus an optional open-form target; keep commit.rs and compare.rs calls working, passing none.
• Conversation timeline: submitted reviews appear in created order:
  • Avatar/name with the verb ("approved these changes" / "requested changes" / "reviewed"), the body, then its threads, each showing the diff_hunk as a small code block, the path:line, an "Outdated" badge, comments and replies, and a "Resolved" collapsed marker using <details>.
  • Dismissed reviews show struck-through with the reason.
• Merge box: a summary line: "Approved by a, b", "Changes requested by c". Write holders see a "Dismiss" form for each approve or request-changes review, with a reason.
• Routes (all POST with CSRF, all redirecting 303 back to the referring tab with an anchor):
  • /pulls/{n}/review_comments
  • /pulls/{n}/review_comments/{id}/reply
  • /pulls/{n}/review_comments/{id}/edit (GET form + POST)
  • /pulls/{n}/review_comments/{id}/delete
  • /pulls/{n}/review_comments/{id}/resolve (with resolved=1|0)
  • /pulls/{n}/reviews, /pulls/{n}/reviews/discard, /pulls/{n}/reviews/{id}/dismiss
• Permissions:
  • Read and signed in to comment or review.
  • Author of the comment to edit or delete; Triage+ may also delete.
  • PR author or Write to resolve; Write to dismiss.
  • Archived repo: no writes. Merged or closed PR: comments and replies still allowed, new reviews not.