Merge approval and documentReview into one approval type

.gitignore

@@ -52,6 +52,9 @@ server/publish.zip
 server/teams-app/manifest.json
 server/teams-app/*.zip
 
+# Visual Studio user-local project settings
+*.csproj.user
+
 # Enterprise proposal analysis (proprietary)
 docs/proposal-v4/
 

core/mcp/modules/NotificationClient.psm1

@@ -265,8 +265,8 @@ function Send-TaskNotification {
     Optional notification settings. If not provided, reads from config.
 
     .PARAMETER Type
-    PRD §4.6 question type — singleChoice (default) | approval | documentReview |
-    freeText | priorityRanking. Drives card rendering and response parsing.
+    PRD sec. 4.6 question type — singleChoice (default) | approval | freeText |
+    priorityRanking. Drives card rendering and response parsing.
 
     .PARAMETER DeliverableSummary
     Optional 1-3 line summary shown in channel notifications (PRD §5.2).
@@ -802,7 +802,7 @@ function ConvertTo-TypedResponse {
     Hashtable. Keys (only set when applicable):
       answer_type        - the type string echoed back
       answer             - resolved string for singleChoice (key) / freeText (string)
-      approval_decision  - approval / documentReview decision value
+      approval_decision  - approval decision value
       comment            - free-text comment
       ranked_items       - array for priorityRanking
       attachment_refs    - array of @{ name; size_bytes; storage_ref; description }
@@ -860,10 +860,6 @@ function ConvertTo-TypedResponse {
             if ($decision) { $out['approval_decision'] = $decision }
             if ($comment)  { $out['comment']           = $comment  }
         }
-        'documentReview' {
-            if ($decision) { $out['approval_decision'] = $decision }
-            if ($comment)  { $out['comment']           = $comment  }
-        }
         'priorityRanking' {
             if ($rankedItems) {
                 # Server emits RankedItem[] = [{ optionId: Guid, rank: int }]

core/mcp/tools/task-answer-question/metadata.yaml

@@ -11,11 +11,12 @@ inputSchema:
       description: "The answer - either an option key (A, B, C, D, E) or a custom freeform answer. Required for singleChoice/freeText types."
     type:
       type: string
-      enum: [singleChoice, approval, documentReview, freeText, priorityRanking]
+      enum: [singleChoice, approval, freeText, priorityRanking]
       description: "Question type. Determines which fields are required/validated."
     decision:
       type: string
-      description: "Required for approval/documentReview. Allowed values vary per type (PRD Section 4.1)."
+      enum: [approved, rejected]
+      description: "Required for approval. Allowed values: approved, rejected."
     comment:
       type: string
       description: "Required when decision=rejected on an approval question."

core/mcp/tools/task-answer-question/script.ps1

@@ -43,10 +43,9 @@ function Invoke-TaskAnswerQuestion {
 
     # Type-specific validation (PRD §4.1, §4.6)
     $validDecisions = @{
-        approval       = @('approved', 'rejected', 'abstained')
-        documentReview = @('approved', 'changes_requested', 'comment_only')
+        approval = @('approved', 'rejected')
     }
-    $validTypes = @('singleChoice', 'approval', 'documentReview', 'freeText', 'priorityRanking')
+    $validTypes = @('singleChoice', 'approval', 'freeText', 'priorityRanking')
     if ($questionType -and $questionType -notin $validTypes) {
         throw "Invalid 'type' value '$questionType'. Allowed: $($validTypes -join ', ')"
     }
@@ -58,8 +57,8 @@ function Invoke-TaskAnswerQuestion {
         if ($decision -notin $validDecisions[$questionType]) {
             throw "Invalid 'decision' value '$decision' for type '$questionType'. Allowed: $($validDecisions[$questionType] -join ', ')"
         }
-        if ($decision -in @('rejected', 'changes_requested') -and -not $comment) {
-            throw "'comment' is required when decision='$decision'"
+        if ($decision -eq 'rejected' -and -not $comment) {
+            throw "'comment' is required when decision='rejected'"
         }
     } elseif ($questionType -eq 'priorityRanking') {
         if (-not $rankedItems -or @($rankedItems).Count -eq 0) {
@@ -78,19 +77,19 @@ function Invoke-TaskAnswerQuestion {
     # 'type' is omitted — legacy callers default to singleChoice for this check
     # so decision/comment/ranked_items can't sneak in alongside a plain answer.
     $effectiveType = if ($questionType) { $questionType } else { 'singleChoice' }
-    if ($decision -and $effectiveType -notin @('approval', 'documentReview')) {
-        throw "'decision' is only valid for type 'approval' or 'documentReview', got type='$effectiveType'"
+    if ($decision -and $effectiveType -ne 'approval') {
+        throw "'decision' is only valid for type 'approval', got type='$effectiveType'"
     }
-    if ($comment -and $effectiveType -notin @('approval', 'documentReview')) {
-        throw "'comment' is only valid for type 'approval' or 'documentReview', got type='$effectiveType'"
+    if ($comment -and $effectiveType -ne 'approval') {
+        throw "'comment' is only valid for type 'approval', got type='$effectiveType'"
     }
     if ($rankedItems -and $effectiveType -ne 'priorityRanking') {
         throw "'ranked_items' is only valid for type 'priorityRanking', got type='$effectiveType'"
     }
     # Reject 'answer' when caller passes it alongside a typed payload — would
     # produce inconsistent resolvedEntry (e.g., answer='A' + approval_decision='approved').
     if ($answer -and $effectiveType -notin @('singleChoice', 'freeText')) {
-        throw "'answer' is only valid for type 'singleChoice' or 'freeText', got type='$effectiveType'. Use 'decision' (approval/documentReview) or 'ranked_items' (priorityRanking)."
+        throw "'answer' is only valid for type 'singleChoice' or 'freeText', got type='$effectiveType'. Use 'decision' (approval) or 'ranked_items' (priorityRanking)."
     }
 
     # Synthesize an answer string for non-question types so downstream

core/mcp/tools/task-mark-needs-input/metadata.yaml

@@ -114,7 +114,7 @@ inputSchema:
         - sub_tasks
     type:
       type: string
-      enum: [singleChoice, approval, documentReview, freeText, priorityRanking]
+      enum: [singleChoice, approval, freeText, priorityRanking]
       default: singleChoice
       description: "Question type (PRD Section 4.6). Drives card rendering and response parsing."
     deliverable_summary:

core/mcp/tools/task-mark-needs-input/script.ps1

@@ -20,7 +20,7 @@ function Invoke-TaskMarkNeedsInput {
     if (-not $question -and -not $questionsArg -and -not $splitProposal) { throw "Either 'questions' array, 'question' object, or 'split_proposal' is required" }
     if (($question -or $questionsArg) -and $splitProposal) { throw "Cannot provide both questions and split_proposal - use one at a time" }
 
-    $validTypes = @('singleChoice', 'approval', 'documentReview', 'freeText', 'priorityRanking')
+    $validTypes = @('singleChoice', 'approval', 'freeText', 'priorityRanking')
     if ($questionType -notin $validTypes) {
         throw "Invalid 'type' value '$questionType'. Allowed: $($validTypes -join ', ')"
     }

core/ui/static/modules/actions.js

@@ -306,18 +306,40 @@ function renderQuestionItem(item) {
     }
 
     if (questionType === 'approval') {
+        const attachments = Array.isArray(question.attachments) ? question.attachments
+            : Array.isArray(question.attachmentList) ? question.attachmentList
+            : [];
+        const hasAttachments = attachments.length > 0;
+        const attachmentListHtml = hasAttachments ? `
+                <div class="approval-attachments">
+                    <div class="approval-attachments-label">Confirm the attachments you reviewed:</div>
+                    <ul class="approval-attachment-list">
+                        ${attachments.map(att => {
+                            const id = att.attachmentId || att.id || att.attachment_id || '';
+                            const name = att.name || 'attachment';
+                            return `
+                                <li class="approval-attachment-item">
+                                    <label>
+                                        <input type="checkbox" class="approval-reviewed-attachment" data-attachment-id="${escapeAttr(id)}" />
+                                        <span class="approval-attachment-name">${escapeHtml(name)}</span>
+                                    </label>
+                                </li>
+                            `;
+                        }).join('')}
+                    </ul>
+                </div>` : '';
         return `
-        <div class="action-item" data-task-id="${escapeHtml(item.task_id)}" data-type="question" data-question-type="approval">
+        <div class="action-item" data-task-id="${escapeHtml(item.task_id)}" data-type="question" data-question-type="approval" data-has-attachments="${hasAttachments ? 'true' : 'false'}">
             <div class="action-item-header">
                 <span class="action-item-type question">Approval</span>
                 <span class="action-item-task">${escapeHtml(item.task_name)}</span>
             </div>
             <div class="action-item-body">
                 <div class="action-question-text">${escapeHtml(question.question || 'No question text')}</div>
                 ${question.context ? `<div class="action-question-context">${escapeHtml(question.context)}</div>` : ''}
+                ${attachmentListHtml}
                 <div class="approval-buttons">
                     <button class="ctrl-btn approval-decision" data-decision="approved">Approve</button>
-                    <button class="ctrl-btn approval-decision" data-decision="abstained">Abstain</button>
                     <button class="ctrl-btn approval-decision danger" data-decision="rejected">Reject</button>
                 </div>
                 <div class="approval-comment-section" style="display:none;">
@@ -868,19 +890,32 @@ function attachActionHandlers(container) {
                 return;
             }
 
+            let reviewedAttachmentIds = null;
+            if (actionItem.dataset.hasAttachments === 'true') {
+                reviewedAttachmentIds = Array.from(
+                    actionItem.querySelectorAll('.approval-reviewed-attachment:checked')
+                ).map(cb => cb.dataset.attachmentId).filter(Boolean);
+                if (reviewedAttachmentIds.length === 0) {
+                    showToast('Please confirm you reviewed at least one attachment', 'warning');
+                    return;
+                }
+            }
+
             btn.disabled = true;
             btn.textContent = 'Submitting...';
 
             try {
+                const body = {
+                    task_id: taskId,
+                    answer: decision,
+                    decision: decision,
+                    comment: comment || null
+                };
+                if (reviewedAttachmentIds) body.reviewed_attachment_ids = reviewedAttachmentIds;
                 const response = await fetch(`${API_BASE}/api/task/answer`, {
                     method: 'POST',
                     headers: { 'Content-Type': 'application/json' },
-                    body: JSON.stringify({
-                        task_id: taskId,
-                        answer: decision,
-                        decision: decision,
-                        comment: comment || null
-                    })
+                    body: JSON.stringify(body)
                 });
 
                 if (!response.ok) {

server/docs/MOTHERSHIP-E2E-SETUP.md

@@ -1,6 +1,6 @@
 # Mothership E2E Test Setup (Playwright + Azurite)
 
-How to run the Mothership web UI end-to-end tests locally. Tests cover the magic-link respond flow for all six question types: `singleChoice`, `multiChoice`, `approval`, `documentReview`, `freeText`, `priorityRanking`.
+How to run the Mothership web UI end-to-end tests locally. Tests cover the magic-link respond flow for all question types: `singleChoice`, `multiChoice`, `approval`, `freeText`, `priorityRanking`.
 
 ---
 

server/src/Dotbot.Server/Models/ApprovalDecisions.cs

@@ -2,23 +2,15 @@ namespace Dotbot.Server.Models;
 
 public static class ApprovalDecisions
 {
-    public const string Approve = "approve";
-    public const string Reject = "reject";
-    public const string Abstain = "abstain";
+    public const string Approved = "approved";
+    public const string Rejected = "rejected";
 
-    public const string RequestChanges = "request-changes";
-    public const string CommentOnly = "comment-only";
-
-    public static readonly string[] ApprovalAllowed = [Approve, Reject, Abstain];
-    public static readonly string[] DocumentReviewAllowed = [Approve, RequestChanges, CommentOnly];
+    public static readonly string[] ApprovalAllowed = [Approved, Rejected];
 
     public static string Label(string decision) => decision switch
     {
-        Approve => "Approve",
-        Reject => "Reject",
-        Abstain => "Abstain",
-        RequestChanges => "Request Changes",
-        CommentOnly => "Comment Only",
+        Approved => "Approve",
+        Rejected => "Reject",
         _ => decision,
     };
 }

server/src/Dotbot.Server/Models/QuestionTypes.cs

@@ -5,7 +5,6 @@ public static class QuestionTypes
     public const string SingleChoice = "singleChoice";
     public const string MultiChoice = "multiChoice";
     public const string Approval = "approval";
-    public const string DocumentReview = "documentReview";
     public const string FreeText = "freeText";
     public const string PriorityRanking = "priorityRanking";
 
@@ -14,7 +13,6 @@ public static class QuestionTypes
         SingleChoice,
         MultiChoice,
         Approval,
-        DocumentReview,
         FreeText,
         PriorityRanking,
     ];

server/src/Dotbot.Server/Pages/Respond.cshtml

@@ -52,23 +52,6 @@
             @switch (Model.Template.Type)
             {
                 case QuestionTypes.Approval:
-                    @Html.Raw(ApprovalRadio(ApprovalDecisions.Approve, "Approve", "A"))
-                    @Html.Raw(ApprovalRadio(ApprovalDecisions.Reject, "Reject", "R", requireComment: true))
-                    @Html.Raw(ApprovalRadio(ApprovalDecisions.Abstain, "Abstain", "X"))
-                    <div class="mt-1">
-                        <label for="comment" class="label-text">Comment (required when rejecting):</label>
-                        <textarea name="comment" id="comment" class="free-text"></textarea>
-                    </div>
-                    break;
-
-                case QuestionTypes.FreeText:
-                    <div class="mt-1">
-                        <label for="freeText" class="label-text">Your response:</label>
-                        <textarea name="freeText" id="freeText" class="free-text" required></textarea>
-                    </div>
-                    break;
-
-                case QuestionTypes.DocumentReview:
                     @if (Model.Template.Attachments is { Count: > 0 } docs)
                     {
                         <div class="mt-1">
@@ -104,15 +87,21 @@
                             </ul>
                         </div>
                     }
-                    @Html.Raw(ApprovalRadio(ApprovalDecisions.Approve, "Approve", "AP"))
-                    @Html.Raw(ApprovalRadio(ApprovalDecisions.RequestChanges, "Request Changes", "RC", requireComment: true))
-                    @Html.Raw(ApprovalRadio(ApprovalDecisions.CommentOnly, "Comment Only", "CO"))
+                    @Html.Raw(ApprovalRadio(ApprovalDecisions.Approved, "Approve", "A"))
+                    @Html.Raw(ApprovalRadio(ApprovalDecisions.Rejected, "Reject", "R", requireComment: true))
                     <div class="mt-1">
-                        <label for="comment" class="label-text">Feedback (required for "Request Changes"):</label>
+                        <label for="comment" class="label-text">Comment (required when rejecting):</label>
                         <textarea name="comment" id="comment" class="free-text"></textarea>
                     </div>
                     break;
 
+                case QuestionTypes.FreeText:
+                    <div class="mt-1">
+                        <label for="freeText" class="label-text">Your response:</label>
+                        <textarea name="freeText" id="freeText" class="free-text" required></textarea>
+                    </div>
+                    break;
+
                 case QuestionTypes.PriorityRanking:
                     <p class="label-text">Drag to reorder. Position 1 = highest priority.</p>
                     <ol id="rank-list" class="rank-list">
@@ -182,7 +171,7 @@
             if (!form) return;
             const type = form.dataset.questionType;
 
-            // ── Approval / DocumentReview: comment required when destructive option selected ──
+            // ── Approval: comment required when destructive option selected ──
             (function wireApproval() {
                 const radios = form.querySelectorAll('input[type="radio"][name="approvalDecision"]');
                 if (radios.length === 0) return;
@@ -202,9 +191,9 @@
                 });
             })();
 
-            // ── DocumentReview: at least one attachment confirmation ──
+            // ── Approval with attachments: at least one attachment confirmation ──
             (function wireDocReview() {
-                if (type !== 'documentReview') return;
+                if (type !== 'approval') return;
                 const checkboxes = form.querySelectorAll('input[type="checkbox"][name="reviewedAttachmentIds"]');
                 if (checkboxes.length === 0) return;
                 form.addEventListener('submit', function (e) {

server/src/Dotbot.Server/Services/Delivery/NotificationSummaryBuilder.cs

@@ -29,8 +29,8 @@ public NotificationSummary Build(
                 ? template.Project.ProjectId
                 : template.Project.Name,
             // Legacy singleChoice templates carry no DeliverableSummary; providers render
-            // Title + Context as before. Validator enforces non-empty for approval /
-            // documentReview, so this stays null only when intentional (PRD §8 line 651).
+            // Title + Context as before. Validator enforces non-empty for approval with
+            // attachments, so this stays null only when intentional.
             DeliverableSummary = template.DeliverableSummary,
             Context = template.Context,
             Attachments = template.Attachments?

server/src/Dotbot.Server/Validation/QuestionTemplateValidator.cs

@@ -19,7 +19,7 @@ public QuestionTemplateValidator(IOptions<QuestionTemplateValidationSettings> se
             CheckQuestionId,
             CheckProjectId,
             CheckType,
-            CheckDeliverableSummary,
+            // CheckDeliverableSummary,
             CheckOptionUniqueness,
             CheckAttachments,
             CheckReferenceLinks,
@@ -48,11 +48,16 @@ private IEnumerable<string> CheckType(QuestionTemplate t)
             yield return $"Unknown type '{t.Type}'. Allowed types: {string.Join(", ", QuestionTypes.AllowedTypes)}";
     }
 
+    // TODO: unskip when outpost supports preparing summary of attachments
     private IEnumerable<string> CheckDeliverableSummary(QuestionTemplate t)
     {
-        if ((t.Type == QuestionTypes.Approval || t.Type == QuestionTypes.DocumentReview)
+        // Required only when an approval question carries attachments — that is the
+        // doc-review case where a reviewer needs a 1-3 line summary of what they're
+        // approving. Plain approvals (no doc) don't need it.
+        if (t.Type == QuestionTypes.Approval
+            && t.Attachments is { Count: > 0 }
             && string.IsNullOrWhiteSpace(t.DeliverableSummary))
-            yield return $"deliverableSummary is required when type is '{t.Type}'";
+            yield return "deliverableSummary is required when type is 'approval' and attachments are present";
     }
 
     private IEnumerable<string> CheckOptionUniqueness(QuestionTemplate t)