diff --git a/README.md b/README.md index e5eb79a..456cb2f 100644 --- a/README.md +++ b/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 diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 2824ce1..14fac6f 100644 --- a/frontend/dashboard.js +++ b/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 @@ '
' + escapeHtml(review.user?.login || 'Reviewer') + ' · ' + escapeHtml(review.state || 'commented') + (review.body ? '
' + renderMarkdown(review.body) + '
' : '') + '
' ).join('') : '
No prior reviews.
'; - 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); diff --git a/frontend/offline-work.js b/frontend/offline-work.js index c200286..58a345e 100644 --- a/frontend/offline-work.js +++ b/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) diff --git a/frontend/service-worker.js b/frontend/service-worker.js index 4f61297..3117078 100644 --- a/frontend/service-worker.js +++ b/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 = [ diff --git a/tests/test_markdown_renderer.py b/tests/test_markdown_renderer.py index 50689e6..e82bd7b 100644 --- a/tests/test_markdown_renderer.py +++ b/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 diff --git a/tests/test_mobile_composer_integration.py b/tests/test_mobile_composer_integration.py index 199abbe..3be6439 100644 --- a/tests/test_mobile_composer_integration.py +++ b/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 diff --git a/tests/test_offline_today.py b/tests/test_offline_today.py index c9ce67a..25145af 100644 --- a/tests/test_offline_today.py +++ b/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() diff --git a/tests/test_offline_work.py b/tests/test_offline_work.py index 13b29a8..c059f71 100644 --- a/tests/test_offline_work.py +++ b/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 diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index 4d2451a..9967312 100644 --- a/tests/test_service_worker.py +++ b/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