-
Notifications
You must be signed in to change notification settings - Fork 1
Upstream sync: v2 sidebar, recently viewed, workspace fixes #204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1b7cb9c
e4c1365
86e0170
c2e96c3
ec45c78
3e5eab7
1931ff0
ecc1a29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,26 @@ import { gt, prerelease, valid } from "semver"; | |||||||||||||||||||||||||||||||||
| import { AUTO_UPDATE_STATUS, type AutoUpdateStatus } from "shared/auto-update"; | ||||||||||||||||||||||||||||||||||
| import { PLATFORM } from "shared/constants"; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // electron-updater's internal cache only self-invalidates when the remote | ||||||||||||||||||||||||||||||||||
| // sha512 differs from cached metadata, so a corrupt cached download (e.g. | ||||||||||||||||||||||||||||||||||
| // failed Squirrel install) gets retried indefinitely until the user | ||||||||||||||||||||||||||||||||||
| // manually reinstalls. Reach into the protected helper to clear it. | ||||||||||||||||||||||||||||||||||
| interface AppUpdaterInternals { | ||||||||||||||||||||||||||||||||||
| downloadedUpdateHelper: { clear(): Promise<void> } | null; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| async function clearCachedUpdate(reason: string): Promise<void> { | ||||||||||||||||||||||||||||||||||
| const helper = (autoUpdater as unknown as AppUpdaterInternals) | ||||||||||||||||||||||||||||||||||
| .downloadedUpdateHelper; | ||||||||||||||||||||||||||||||||||
| if (!helper) return; | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| await helper.clear(); | ||||||||||||||||||||||||||||||||||
| console.info(`[auto-updater] Cleared cached update (${reason})`); | ||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||
| console.error("[auto-updater] Failed to clear cached update:", error); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const UPDATE_CHECK_INTERVAL_MS = 1000 * 60 * 60 * 4; // 4 hours | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
|
|
@@ -405,6 +425,7 @@ export function setupAutoUpdater(): void { | |||||||||||||||||||||||||||||||||
| `[auto-updater] Error during update (currentVersion=${app.getVersion()}):`, | ||||||||||||||||||||||||||||||||||
| error?.message || error, | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
| void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`); | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "# currentStatus の更新箇所"
sed -n '97,110p' apps/desktop/src/main/lib/auto-updater.ts
echo
echo "# error handler の現在実装"
sed -n '418,430p' apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 863 🏁 Script executed: #!/bin/bash
set -euo pipefail
# ファイル全体の行数確認
wc -l apps/desktop/src/main/lib/auto-updater.ts
# clearCachedUpdate の定義を確認
echo "=== clearCachedUpdate 関数 ==="
rg -A 8 "function clearCachedUpdate|const clearCachedUpdate" apps/desktop/src/main/lib/auto-updater.ts
# currentStatus の定義と使用方法
echo ""
echo "=== currentStatus 定義 ==="
rg -B 2 "currentStatus\s*=" apps/desktop/src/main/lib/auto-updater.ts | head -30
# error handler 周辺の full context
echo ""
echo "=== error handler 完全コンテキスト (lines 415-435) ==="
sed -n '415,435p' apps/desktop/src/main/lib/auto-updater.ts
# AUTO_UPDATE_STATUS の定義確認
echo ""
echo "=== AUTO_UPDATE_STATUS 定義 ==="
rg -A 10 "enum AUTO_UPDATE_STATUS|const AUTO_UPDATE_STATUS" apps/desktop/src/main/lib/auto-updater.ts | head -20Repository: MocA-Love/superset Length of output: 1577 🏁 Script executed: #!/bin/bash
set -euo pipefail
# AUTO_UPDATE_STATUS 定義を見つける
echo "=== AUTO_UPDATE_STATUS 定義 ==="
sed -n '1,100p' apps/desktop/src/main/lib/auto-updater.ts | grep -A 15 "AUTO_UPDATE_STATUS"
# イベントハンドラ一覧で状態遷移を理解
echo ""
echo "=== autoUpdater イベントハンドラ一覧 ==="
rg "autoUpdater\.on\(" apps/desktop/src/main/lib/auto-updater.ts | head -20
# download-progress, update-downloaded などのイベントハンドラ
echo ""
echo "=== update-downloaded ハンドラ ==="
rg -A 6 'autoUpdater\.on\("update-downloaded"' apps/desktop/src/main/lib/auto-updater.ts
# download-progress ハンドラ
echo ""
echo "=== download-progress ハンドラ ==="
rg -A 6 'autoUpdater\.on\("download-progress"' apps/desktop/src/main/lib/auto-updater.ts
# checking-for-update ハンドラ
echo ""
echo "=== checking-for-update ハンドラ ==="
rg -A 6 'autoUpdater\.on\("checking-for-update"' apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 2178 🏁 Script executed: #!/bin/bash
set -euo pipefail
# AUTO_UPDATE_STATUS enum in shared package
echo "=== AUTO_UPDATE_STATUS enum definition ==="
find apps -name "auto-update.ts" -o -name "auto-update.tsx" | while read f; do
if [[ -f "$f" ]]; then
echo "File: $f"
cat -n "$f" | head -50
fi
done
# Also check in packages
echo ""
echo "=== shared/auto-update search ==="
fd "auto-update" --type f | grep -E "(shared|packages)" | head -10
# Full file listing to find where it's defined
echo ""
echo "=== searching for AUTO_UPDATE_STATUS enum ==="
rg "enum AUTO_UPDATE_STATUS|export.*AUTO_UPDATE_STATUS" --type ts --type tsxRepository: MocA-Love/superset Length of output: 724 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check update-available handler to see when DOWNLOADING state is set
echo "=== update-available ハンドラ ==="
rg -A 10 'autoUpdater\.on\("update-available"' apps/desktop/src/main/lib/auto-updater.ts
# Check if there's any explicit DOWNLOADING state setting
echo ""
echo "=== DOWNLOADING state transitions ==="
rg "DOWNLOADING" apps/desktop/src/main/lib/auto-updater.ts
# Check the full sequence of state changes
echo ""
echo "=== 状態遷移フロー ==="
sed -n '420,480p' apps/desktop/src/main/lib/auto-updater.ts
# Check if isNetworkError is defined and what it matches
echo ""
echo "=== isNetworkError 関数 ==="
rg -B 2 -A 10 "function isNetworkError|const isNetworkError" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 3079 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check SILENT_ERROR_PATTERNS to understand what errors are considered non-network
echo "=== SILENT_ERROR_PATTERNS 定義 ==="
rg -B 5 "SILENT_ERROR_PATTERNS" apps/desktop/src/main/lib/auto-updater.ts
# Look for all places where clearCachedUpdate is called
echo ""
echo "=== clearCachedUpdate の呼び出し全体 ==="
rg "clearCachedUpdate" apps/desktop/src/main/lib/auto-updater.ts -B 2 -A 1
# Check download-progress to see if errors can occur during download
echo ""
echo "=== ダウンロード進行中のエラーハンドリング ==="
sed -n '460,475p' apps/desktop/src/main/lib/auto-updater.ts
# Check if there are any other state-dependent error handling
echo ""
echo "=== state を条件にした処理がないか確認 ==="
rg "currentStatus.*==|currentStatus.*===|switch.*currentStatus" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 1392 🏁 Script executed: #!/bin/bash
set -euo typefail
# Get full SILENT_ERROR_PATTERNS list to understand what non-network errors include
echo "=== SILENT_ERROR_PATTERNS 完全リスト ==="
sed -n '1,100p' apps/desktop/src/main/lib/auto-updater.ts | grep -A 20 "SILENT_ERROR_PATTERNS"
# Check if error handler can be triggered during CHECKING phase (before download starts)
echo ""
echo "=== checkForUpdates 関数 ==="
rg -A 20 "async function checkForUpdates|function checkForUpdates" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 115 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Get full SILENT_ERROR_PATTERNS list to understand what non-network errors include
echo "=== SILENT_ERROR_PATTERNS 完全リスト ==="
sed -n '1,100p' apps/desktop/src/main/lib/auto-updater.ts | grep -A 20 "SILENT_ERROR_PATTERNS"
# Check if error handler can be triggered during CHECKING phase (before download starts)
echo ""
echo "=== checkForUpdates 関数 ==="
rg -A 20 "async function checkForUpdates|function checkForUpdates" apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 1820 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check what errors can occur during the CHECKING phase
# Look at the broader context around checkForUpdates and error handling
echo "=== checkForUpdates と error handler の全コンテキスト ==="
sed -n '340,410p' apps/desktop/src/main/lib/auto-updater.ts
# Check if there are metadata/feed validation errors that could occur
echo ""
echo "=== UPDATE_FEED_URL と feed 検証 ==="
rg "UPDATE_FEED_URL|sha512|checksum" apps/desktop/src/main/lib/auto-updater.ts | head -20
# Verify the state flow: when can error be triggered?
echo ""
echo "=== Error が発生しうるフェーズ分析 ==="
# Get lines around line 415-435 to see state when error fires
sed -n '410,440p' apps/desktop/src/main/lib/auto-updater.tsRepository: MocA-Love/superset Length of output: 3375 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Check if there are any feed parsing or metadata validation errors that could occur
# during CHECKING phase
echo "=== electron-updater の error イベント発生条件 ==="
# Look for comments or documentation about when error is emitted
sed -n '1,50p' apps/desktop/src/main/lib/auto-updater.ts
# Verify: can error occur during CHECKING phase (before DOWNLOADING)?
# If yes, then the concern is valid
echo ""
echo "=== CHECKING フェーズ中のエラー可能性 ==="
# The autoUpdater.on("error") is global and fires for ANY error
# CHECKING phase: autoUpdater.checkForUpdates() could fail on feed validation
# This would trigger error handler while currentStatus === CHECKING
# Final verification: confirm the flow
echo "フロー:"
echo "1. checkForUpdates() -> CHECKING 状態に変更"
echo "2. autoUpdater.checkForUpdates() 実行"
echo "3a. 成功: update-available または update-not-available イベント"
echo "3b. 失敗: error イベント(currentStatus はまだ CHECKING)"
echo ""
echo "現在の実装では 3b の時点で clearCachedUpdate() が呼ばれるため、"
echo "CHECKING フェーズのエラー(feed 検証失敗など)でもキャッシュが削除される"
echo ""
echo "proposed fix では currentStatus === DOWNLOADING の時のみ削除するので、"
echo "CHECKING フェーズのエラーではキャッシュを残す"Repository: MocA-Love/superset Length of output: 2525 キャッシュ削除の条件が広すぎます。 ここだと「非ネットワーク系エラー」なら全部キャッシュを消すので、CHECKING フェーズの feed 検証エラーなど、キャッシュ破損と無関係なエラーでも、既に落とせていた更新を捨てる可能性があります。 🔧 提案差分+function shouldClearCachedUpdate(status: AutoUpdateStatus): boolean {
+ return status === AUTO_UPDATE_STATUS.DOWNLOADING;
+}
+
autoUpdater.on("error", (error) => {
if (isNetworkError(error)) {
console.info("[auto-updater] Network unavailable, will retry later");
emitStatus(AUTO_UPDATE_STATUS.IDLE);
return;
@@
- void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`);
+ if (shouldClearCachedUpdate(currentStatus)) {
+ void clearCachedUpdate(`error: ${error?.message ?? "unknown"}`);
+ }
emitStatus(AUTO_UPDATE_STATUS.ERROR, undefined, error.message);
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 保留: upstream 共通の挙動なので、この PR ではそのまま取り込みます。upstream 側で修正されたら次回同期で取り込む方針。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🧠 Learnings used |
||||||||||||||||||||||||||||||||||
| emitStatus(AUTO_UPDATE_STATUS.ERROR, undefined, error.message); | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| import { workspaceTrpc } from "@superset/workspace-client"; | ||
| import { useCallback } from "react"; | ||
| import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider"; | ||
| import { useWorkspaceEvent } from "../useWorkspaceEvent"; | ||
|
|
||
| /** | ||
|
|
@@ -15,20 +14,25 @@ import { useWorkspaceEvent } from "../useWorkspaceEvent"; | |
| * debounce needed. | ||
| */ | ||
| export function useGitStatus(workspaceId: string) { | ||
| const collections = useCollections(); | ||
| const baseBranch: string | null = | ||
| collections.v2WorkspaceLocalState.get(workspaceId)?.sidebarState | ||
| ?.baseBranch ?? null; | ||
|
|
||
| const utils = workspaceTrpc.useUtils(); | ||
|
|
||
| const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery( | ||
| { workspaceId }, | ||
| { staleTime: Number.POSITIVE_INFINITY, enabled: Boolean(workspaceId) }, | ||
| ); | ||
| const baseBranch = baseBranchQuery.data?.baseBranch ?? null; | ||
|
|
||
| const query = workspaceTrpc.git.getStatus.useQuery( | ||
| { workspaceId, baseBranch: baseBranch ?? undefined }, | ||
| { refetchOnWindowFocus: true, enabled: Boolean(workspaceId) }, | ||
|
Comment on lines
+19
to
27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 保存済み base branch の取得前に 初回レンダーでは 🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 保留: upstream 共通の挙動で、軽微な UX ちらつき。upstream 側の判断を待つ方針です。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ); | ||
|
|
||
| const invalidate = useCallback(() => { | ||
| void utils.git.getStatus.invalidate({ workspaceId }); | ||
| // Current branch may have changed (external checkout), and | ||
| // branch.<name>.base is per-branch — drop the cache so the next read | ||
| // picks up the new branch's base. | ||
| void utils.git.getBaseBranch.invalidate({ workspaceId }); | ||
| }, [utils, workspaceId]); | ||
|
|
||
| useWorkspaceEvent("git:changed", workspaceId, invalidate); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: MocA-Love/superset
Length of output: 50375
非公開 internals に依存するなら
electron-updaterは固定した方が安全です。apps/desktop/src/main/lib/auto-updater.tsがdownloadedUpdateHelperに直接触る前提になったので、ここが^6.8.3のままだと lockfile 再生成だけで private API 変更の影響を受けます。少なくともこの依存だけは exact pin にしておく方が事故りにくいです。🔧 提案差分
📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
保留: この PR は upstream からの取り込みが主目的で、exact pin にすると将来の upstream sync で
^6.8.3と衝突します。upstream 側で pin すべきか議論したいので、このフォークでは caret を維持します。There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MocA-Love、ご説明ありがとうございます。upstream sync との互換性を優先してキャレット範囲を維持するという判断、承知しました。この点は今後のレビューで指摘しないよう記録しておきます。( ´・ω・`)ノ
✏️ Learnings added
🧠 Learnings used