Merge pull request 'Review saved pull request diffs and draft feedback offline' (#356) from timmy/355-offline-review-drafts into main

README.md

@@ -224,13 +224,20 @@ Users can explicitly enable **Keep My Work available offline**. Each healthy liv
 refresh then stores a seven-day, versioned snapshot containing only the signed-in
 user identity and queue-card metadata for issues, pull requests, unread updates,
 and pagination totals. For each opened Today issue or pull request, Stackchain additionally retains an allowlisted detail record with its body and
-newest 20 comments. A previously opened unread update is retained by notification
+newest 20 comments. Requested-review records also retain the head SHA, CI state,
+newest 20 prior reviews, and at most 50 sanitized file previews with 400 diff lines
+per file. A previously opened unread update is retained by notification
 identity with allowlisted subject context and its newest 20 conversation messages.
-This account-bound cache is limited to ten records across all detail kinds; diffs,
-credentials, repository catalogs, events, and complete API responses are excluded.
+This account-bound cache is limited to ten records across all detail kinds; credentials,
+repository catalogs, events, raw patches, and complete API responses are excluded.
 A cold offline launch labels the saved time. Cached Today details open in the existing
-phone sheet, where comments can enter the account-bound durable outbox; planning,
-assignment, review, merge, and close controls remain disabled until reconnection.
+phone sheet, where comments can enter the account-bound durable outbox. For issue and
+non-review pull details, planning, assignment, review, merge, and close controls remain disabled
+until reconnection. Cached
+requested reviews open in the existing review sheet so file progress, notes, summary,
+decision, and inline-comment drafts remain usable under the saved head SHA. Review
+submission is never queued: reconnect reloads the current head before enabling submit;
+a changed head starts clean SHA-scoped progress while retaining the prior-head draft.
 Cached unread updates use the same phone conversation sheet and replies enter the
 account-bound durable outbox, while mark read, ownership, deferral, and older-message loading remain disabled until reconnection.
 Cards without a saved detail explain that reconnection is required. **Clear offline

frontend/dashboard.js

@@ -105,6 +105,7 @@
   let hasContextSnapshot = false;
   let selectedReview = null;
   let reviewTrigger = null;
+  let offlineReview = false;
   let selectedUpdate = null;
   let updateTrigger = null;
   let selectedIssue = null;
@@ -245,7 +246,8 @@
     retry.hidden = status.failed === 0;
   }
   const offlineToday = createOfflineToday({
-    loadDetail: item => item.kind === 'pull' ? pullController.load(item) : issueController.load(item),
+    loadDetail: item => item.is_review ? reviewController.load(item) :
+      item.kind === 'pull' ? pullController.load(item) : issueController.load(item),
     loadSavedDetail: (login, item) => offlineWorkStore.loadDetail(login, item),
     saveDetail: (login, item, detail) => offlineWorkStore.saveDetail(login, item, detail),
     onStatus: renderOfflineTodayStatus,
@@ -521,7 +523,10 @@
       if (item.kind === 'issue') {
         issueTrigger = trigger;
         openIssueSheet(item, trigger, savedDetail);
-      } else if (item.kind === 'pull' && !item.is_review) {
+      } else if (item.is_review) {
+        reviewTrigger = trigger;
+        openReviewSheet(item, trigger, savedDetail);
+      } else if (item.kind === 'pull') {
         pullTrigger = trigger;
         openPullSheet(item, trigger, savedDetail);
       } else if (savedUpdate) {
@@ -1769,9 +1774,10 @@
     qs('#review-inline-body').focus();
   }
 
-  async function openReviewSheet(item, trigger) {
+  async function openReviewSheet(item, trigger, cachedDetail = null) {
     selectedReview = item;
     reviewTrigger = trigger;
+    offlineReview = Boolean(cachedDetail);
     qs('#review-sheet').classList.add('open');
     qs('#review-sheet-key').textContent = item.key;
     qs('#review-sheet-title').textContent = item.title;
@@ -1793,13 +1799,14 @@
     qs('#review-submit-status').textContent = '';
     qs('#continue-review-to-merge').hidden = true;
     qs('#submit-review').disabled = true;
+    qs('#submit-review').textContent = offlineReview ? 'Reconnect to validate & submit' : 'Submit review';
     closeInlineComposer();
     draft = null;
     reviewFiles = [];
     selectedReviewHead = '';
     qs('#close-review-sheet').focus();
     try {
-      const detail = await reviewController.load(selectedReview);
+      const detail = cachedDetail || await reviewController.load(selectedReview);
       if (selectedReview !== item) return;
       qs('#review-sheet-body').innerHTML = renderMarkdown(detail.body || 'No description provided.');
       qs('#review-ci-state').textContent = 'CI ' + (detail.ci_state || 'unknown');
@@ -1856,8 +1863,10 @@
         '<div class="review-history"><strong>' + escapeHtml(review.user?.login || 'Reviewer') + '</strong> · ' +
         escapeHtml(review.state || 'commented') + (review.body ? '<div class="small markdown-content">' + renderMarkdown(review.body) + '</div>' : '') + '</div>'
       ).join('') : '<div>No prior reviews.</div>';
-      qs('#review-sheet-status').textContent = 'Ready to review · by ' + (detail.author || 'unknown author');
-      qs('#submit-review').disabled = false;
+      qs('#review-sheet-status').textContent = offlineReview ?
+        'Offline review · saved ' + fmt(detail.saved_at) + ' · draft feedback stays on this device.' :
+        'Ready to review · by ' + (detail.author || 'unknown author');
+      qs('#submit-review').disabled = offlineReview;
     } catch (error) {
       if (selectedReview !== item) return;
       qs('#review-sheet-status').textContent = error.message + ' Retry here or use Open in Gitea.';
@@ -1873,6 +1882,7 @@
     }
     qs('#review-sheet').classList.remove('open');
     selectedReview = null;
+    offlineReview = false;
     progress = null;
     draft = null;
     reviewFiles = [];
@@ -3135,7 +3145,9 @@
     offlineStatus.hidden = true;
     setOfflineWorkMode(false);
     setStatus('Reconnecting…');
-    contextPoller.refresh({ force: true });
+    contextPoller.refresh({ force: true }).then(() => {
+      if (selectedReview && offlineReview) openReviewSheet(selectedReview, reviewTrigger);
+    });
   }
   keepWorkOffline.addEventListener('change', () => {
     offlineWorkStore.setEnabled(keepWorkOffline.checked);

frontend/offline-work.js

@@ -19,9 +19,13 @@
   ];
   const DETAIL_FIELDS = [
     'title', 'body', 'state', 'labels', 'assignees', 'author', 'url', 'due_date', 'milestone',
-    'id', 'repository', 'subject_type', 'subject_body', 'source_updated_at',
+    'id', 'repository', 'subject_type', 'subject_body', 'source_updated_at', 'head_sha', 'ci_state',
   ];
   const COMMENT_FIELDS = ['id', 'author', 'body', 'created_at', 'updated_at', 'url'];
+  const REVIEW_FILE_FIELDS = [
+    'filename', 'status', 'additions', 'deletions', 'diff_available', 'diff_binary', 'diff_truncated',
+  ];
+  const REVIEW_FIELDS = ['id', 'state', 'body', 'submitted_at'];
 
   function pick(source, fields) {
     const output = {};
@@ -53,7 +57,8 @@
         const notificationId = Number(item?.notification_id || 0);
         return Number.isInteger(notificationId) && notificationId > 0 ? 'update:' + notificationId : '';
       }
-      const kind = item?.kind === 'pull' ? 'pull' : item?.kind === 'issue' ? 'issue' : '';
+      const kind = item?.kind === 'review' ? 'review' :
+        item?.kind === 'pull' ? 'pull' : item?.kind === 'issue' ? 'issue' : '';
       const repository = String(item?.repository || '');
       const number = Number(item?.number || 0);
       return kind && repository && number > 0 ? [kind, repository, number].join(':') : '';
@@ -74,19 +79,30 @@
       login = String(login || '').trim();
       if (!enabled() || !login || !key || !detail) return false;
       const conversation = detail.conversation || {};
+      const data = {
+        ...pick(detail, DETAIL_FIELDS),
+        conversation: {
+          comments: (conversation.comments || []).slice(-20).map(comment => pick(comment, COMMENT_FIELDS)),
+          page: Number(conversation.page || 1),
+          older_page: conversation.older_page ?? null,
+          total: Number(conversation.total || 0),
+        },
+      };
+      if (item?.is_review || item?.kind === 'review') {
+        data.files = (detail.files || []).slice(0, 50).map(file => ({
+          ...pick(file, REVIEW_FILE_FIELDS),
+          diff_lines: (file?.diff_lines || []).slice(0, 400).map(line => String(line)),
+        }));
+        data.reviews = (detail.reviews || []).slice(-20).map(review => ({
+          ...pick(review, REVIEW_FIELDS),
+          user: pick(review?.user, ['login']),
+        }));
+      }
       const record = {
         key,
         user_login: login,
         saved_at: now().toISOString(),
-        data: {
-          ...pick(detail, DETAIL_FIELDS),
-          conversation: {
-            comments: (conversation.comments || []).slice(-20).map(comment => pick(comment, COMMENT_FIELDS)),
-            page: Number(conversation.page || 1),
-            older_page: conversation.older_page ?? null,
-            total: Number(conversation.total || 0),
-          },
-        },
+        data,
       };
       const records = readDetails().filter(candidate =>
         !(candidate?.key === key && candidate?.user_login === login)

frontend/service-worker.js

@@ -1,6 +1,6 @@
 const BASE = new URL('./', self.location.href).pathname;
 importScripts(BASE + 'static/background-issue-sync.js');
-const CACHE = 'stackchain-dashboard-shell-v43';
+const CACHE = 'stackchain-dashboard-shell-v44';
 const OUTAGE_STATUSES = new Set([500, 502, 503, 504]);
 const NAVIGATION_TIMEOUT_MS = self.__STACKCHAIN_NAVIGATION_TIMEOUT_MS || 4000;
 const SHELL = [

tests/test_markdown_renderer.py

@@ -137,4 +137,4 @@ def test_markdown_work_bodies_are_mobile_safe_block_containers():
     assert ".markdown-content { min-width:0; max-width:100%; overflow-wrap:anywhere;" in css
     assert ".markdown-content pre { max-width:100%; overflow-x:auto;" in css
     assert ".markdown-content a { min-height:44px;" in css
-    assert "stackchain-dashboard-shell-v43" in worker
+    assert "stackchain-dashboard-shell-v44" in worker

tests/test_mobile_composer_integration.py

@@ -35,4 +35,4 @@ def test_offline_shell_contains_every_local_dashboard_runtime_asset():
     shell_assets = set(re.findall(r"BASE \+ '([^']+)'", worker.split("async function sessionCsrf", 1)[0]))
 
     assert local_assets <= shell_assets, f"Offline shell is missing: {sorted(local_assets - shell_assets)}"
-    assert "stackchain-dashboard-shell-v43" in worker
+    assert "stackchain-dashboard-shell-v44" in worker

tests/test_offline_today.py

@@ -111,6 +111,32 @@ process.stdout.write(JSON.stringify({calls, status}));
     assert result["status"] == {"total": 2, "ready": 2, "failed": 0, "pending": 0}
 
 
+def test_requested_reviews_are_warmed_as_distinct_today_items():
+    result = run_scenario("""
+const loaded = [];
+const saved = [];
+const review = {
+  kind:'pull', is_review:true, repository:'stackchain/dashboard', number:12,
+  updated_at:'2026-08-08T12:00:00Z',
+};
+const warmer = createOfflineToday({
+  loadDetail: async item => {
+    loaded.push(item.is_review ? 'review' : item.kind);
+    return {head_sha:'abc123', files:[{filename:'src/app.js'}]};
+  },
+  loadSavedDetail: () => null,
+  saveDetail: (_login, item, detail) => saved.push({item, detail}),
+});
+const status = await warmer.warm('timmy', [review]);
+process.stdout.write(JSON.stringify({loaded, saved, status}));
+""")
+
+    assert result["loaded"] == ["review"]
+    assert result["saved"][0]["item"]["is_review"] is True
+    assert result["saved"][0]["detail"]["head_sha"] == "abc123"
+    assert result["status"] == {"total": 1, "ready": 1, "failed": 0, "pending": 0}
+
+
 @pytest.mark.anyio
 async def test_dashboard_warms_today_after_fresh_snapshot_and_exposes_mobile_retry():
     html = await dashboard()

tests/test_offline_work.py

@@ -154,6 +154,48 @@ process.stdout.write(JSON.stringify({
     assert "private diff" not in result["raw"]
 
 
+def test_requested_review_cache_is_sha_scoped_allowlisted_and_bounded():
+    result = run_scenario("""
+const store = createOfflineWorkStore({storage, now:() => new Date('2026-08-07T12:00:00Z')});
+store.setEnabled(true);
+const review = {kind:'pull', repository:'stackchain/dashboard', number:12, is_review:true};
+store.saveDetail('timmy', review, {
+  title:'Review offline work', body:'Review description', author:'alexander', head_sha:'abc123',
+  ci_state:'success', token:'top-secret', source_updated_at:'2026-08-07T11:00:00Z',
+  files:Array.from({length:55}, (_, fileIndex) => ({
+    filename:'src/file-' + fileIndex + '.js', status:'modified', additions:2, deletions:1,
+    diff_available:true, diff_binary:false, diff_truncated:false, patch:'must-not-persist',
+    diff_lines:Array.from({length:405}, (_, lineIndex) => '+' + fileIndex + ':' + lineIndex),
+  })),
+  reviews:Array.from({length:25}, (_, index) => ({
+    id:index + 1, state:'COMMENT', body:'Review ' + (index + 1),
+    submitted_at:'2026-08-07T10:00:00Z', user:{login:'reviewer-' + index, email:'private@example.test'},
+    token:'review-secret',
+  })),
+});
+const loaded = store.loadDetail('timmy', review);
+process.stdout.write(JSON.stringify({
+  loaded,
+  wrongUser:store.loadDetail('alexander', review),
+  raw:[...values.values()].join(' '),
+}));
+""")
+
+    assert result["wrongUser"] is None
+    assert result["loaded"]["head_sha"] == "abc123"
+    assert result["loaded"]["ci_state"] == "success"
+    assert len(result["loaded"]["files"]) == 50
+    assert len(result["loaded"]["files"][0]["diff_lines"]) == 400
+    assert result["loaded"]["files"][0]["filename"] == "src/file-0.js"
+    assert len(result["loaded"]["reviews"]) == 20
+    assert result["loaded"]["reviews"][0]["body"] == "Review 6"
+    assert result["loaded"]["reviews"][0]["user"] == {"login": "reviewer-5"}
+    assert "top-secret" not in result["raw"]
+    assert "must-not-persist" not in result["raw"]
+    assert "private@example.test" not in result["raw"]
+    assert "review-secret" not in result["raw"]
+
+
 @pytest.mark.anyio
 async def test_dashboard_offers_private_offline_work_controls_and_read_only_hydration():
     html = await dashboard()
@@ -219,3 +261,17 @@ async def test_saved_unread_update_opens_offline_with_queued_reply_and_read_cont
     assert "qs('#load-older-update-comments').disabled = offline;" in html
     assert "document.querySelectorAll('[data-notification-id], [data-later-preset], [data-today-add]')" in html
     assert "replies and read acknowledgements queue for sync" in html
+
+
+@pytest.mark.anyio
+async def test_saved_requested_review_opens_offline_for_sha_scoped_drafting_only():
+    html = await dashboard()
+
+    assert "item.is_review ? reviewController.load(item)" in html
+    assert "openReviewSheet(item, trigger, savedDetail)" in html
+    assert "async function openReviewSheet(item, trigger, cachedDetail = null)" in html
+    assert "cachedDetail || await reviewController.load(selectedReview)" in html
+    assert "Offline review · saved " in html
+    assert "Reconnect to validate & submit" in html
+    assert "qs('#submit-review').disabled = offlineReview;" in html
+    assert "if (selectedReview && offlineReview) openReviewSheet(selectedReview, reviewTrigger);" in html

tests/test_service_worker.py

@@ -108,7 +108,7 @@ async function dispatchNotificationClick(route) {{
 def test_navigation_deadline_ships_in_a_new_shell_cache():
     source = WORKER.read_text()
 
-    assert "stackchain-dashboard-shell-v43" in source
+    assert "stackchain-dashboard-shell-v44" in source
     assert "BASE + 'static/dashboard.css'" in source
     assert "BASE + 'static/dashboard.js'" in source
     assert "BASE + 'static/install-app.js'" in source
@@ -117,14 +117,14 @@ def test_navigation_deadline_ships_in_a_new_shell_cache():
 def test_mobile_search_viewport_ships_in_a_new_offline_shell():
     source = WORKER.read_text()
 
-    assert "stackchain-dashboard-shell-v43" in source
+    assert "stackchain-dashboard-shell-v44" in source
     assert "BASE + 'static/mobile-search-viewport.js'" in source
 
 
 def test_update_ownership_flow_ships_atomically_in_a_new_offline_shell():
     source = WORKER.read_text()
 
-    assert "stackchain-dashboard-shell-v43" in source
+    assert "stackchain-dashboard-shell-v44" in source
     assert "BASE + 'static/update-ownership.js'" in source