fix(auto-update): treat only explicit semver pins as user-pinned#2006
fix(auto-update): treat only explicit semver pins as user-pinned#2006code-yeongyu merged 7 commits intodevfrom
Conversation
|
@cubic-dev-ai please review this PR. |
@code-yeongyu I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai please re-review |
@code-yeongyu I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 5/5
- Low-severity, localized dead code in
src/hooks/auto-update-checker/hook/background-update-check.tsreduces clarity but is unlikely to affect runtime behavior, so risk is minimal. - This PR looks safe to merge given the issue is limited to an unreachable
if (pluginInfo.isPinned)branch with no functional impact noted. - Pay close attention to
src/hooks/auto-update-checker/hook/background-update-check.ts- unreachableif (pluginInfo.isPinned)block could be removed or refactored for clarity.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/auto-update-checker/hook/background-update-check.ts">
<violation number="1" location="src/hooks/auto-update-checker/hook/background-update-check.ts:63">
P3: Dead code: The `if (pluginInfo.isPinned)` check at the end of this function is unreachable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| if (pluginInfo.isPinned) { | ||
| await showUpdateAvailableToast(ctx, latestVersion, getToastMessage) | ||
| await showUpdateAvailableToast(ctx, latestVersion, () => getPinnedVersionToastMessage(latestVersion)) |
There was a problem hiding this comment.
P3: Dead code: The if (pluginInfo.isPinned) check at the end of this function is unreachable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/auto-update-checker/hook/background-update-check.ts, line 63:
<comment>Dead code: The `if (pluginInfo.isPinned)` check at the end of this function is unreachable.</comment>
<file context>
@@ -56,7 +60,7 @@ export async function runBackgroundUpdateCheck(
if (pluginInfo.isPinned) {
- await showUpdateAvailableToast(ctx, latestVersion, getToastMessage)
+ await showUpdateAvailableToast(ctx, latestVersion, () => getPinnedVersionToastMessage(latestVersion))
log(`[auto-update-checker] User-pinned version detected (${pluginInfo.entry}), skipping auto-update. Notification only.`)
return
</file context>
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 3/5
- The regex in
src/cli/config-manager/plugin-name-with-version.tsis too strict (trailing dot, no hyphens), so valid versions like3.0.0-betaor3.0.0-rc-1won’t match, which can break prerelease plugin handling. - Given the concrete version-parsing regression risk, the change carries some user-facing impact and warrants a cautious merge.
- Pay close attention to
src/cli/config-manager/plugin-name-with-version.ts- tighten version parsing without excluding valid prerelease formats.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/cli/config-manager/plugin-name-with-version.ts">
<violation number="1" location="src/cli/config-manager/plugin-name-with-version.ts:7">
P1: The regex requires a trailing dot and disallows hyphens, failing to match valid prerelease channels like `3.0.0-beta` or `3.0.0-rc-1`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Fixes #1920 Installer-written exact versions (e.g., oh-my-opencode@3.5.2) were incorrectly treated as user-pinned, blocking auto-updates for all installer users. Fix isPinned to only block auto-update when pinnedVersion is an explicit semver string (user's intent). Channel tags (latest, beta, next) and bare package name all allow auto-update. Fix installer fallback to return bare PACKAGE_NAME for stable versions and PACKAGE_NAME@{channel} for prerelease versions, preserving channel tracking.
code-yeongyu
force-pushed
the
fix/1920-auto-update-pinned
branch
from
e3fb515 to
f260d15
Compare
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/anthropic-context-window-limit-recovery/executor.test.ts">
<violation number="1" location="src/hooks/anthropic-context-window-limit-recovery/executor.test.ts:305">
P2: Missing `finally` block for teardown: `fakeTimeouts.restore()` will be skipped if assertions fail, leaking mocked global timers to other tests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
src/hooks/anthropic-context-window-limit-recovery/executor.test.ts
Outdated
Show resolved
Hide resolved
code-yeongyu
force-pushed
the
fix/1920-auto-update-pinned
branch
from
5c1b655 to
8938b63
Compare
- executor.test.ts: capture globalThis.setTimeout/clearTimeout at module level - events.test.ts: replace dynamic await import with static top-level import
Replace exact call count assertions with delta-based checks: - capture errorSpy.mock.calls.length before processing events - slice to only check calls made during this test's execution - use try/finally to guarantee mockRestore() even on assertion failure This prevents test pollution from cross-file spy leakage in CI batch runs.
recovery-hook.test.ts uses mock.module() at top level which patches the executor module in the shared bun module cache. When run in the same batch as executor.test.ts, executeCompact becomes the mocked no-op version, causing all lock management tests to fail. Move it to the isolated step (each file gets its own bun process) and enumerate the remaining anthropic-context-window-limit-recovery test files explicitly to avoid including recovery-hook.test.ts in the batch.
|
@cubic-dev-ai The previous review was on a now-reverted commit. The current HEAD already has |
@code-yeongyu I have started the AI code review. It will take a few minutes to complete. |
|
@cubic-dev-ai Please re-review the current HEAD. The |
@code-yeongyu I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 13 files
Confidence score: 4/5
- This PR appears safe to merge overall, but there is a moderate consistency risk due to differing
isPinnedlogic betweensrc/hooks/auto-update-checker/checker/plugin-entry.tsand existing logic insrc/cli/doctor/checks/system-plugin.ts(dist-tags likebetatreated as pinned). - Impact is likely limited to version-pinning checks rather than core functionality, which keeps the risk moderate rather than high.
- Pay close attention to
src/hooks/auto-update-checker/checker/plugin-entry.tsandsrc/cli/doctor/checks/system-plugin.ts- alignisPinnedbehavior to avoid inconsistent results.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/auto-update-checker/checker/plugin-entry.ts">
<violation number="1" location="src/hooks/auto-update-checker/checker/plugin-entry.ts:14">
P2: This introduces inconsistent logic across the codebase. `src/cli/doctor/checks/system-plugin.ts` still uses the old logic for `isPinned` (treating dist-tags like `beta` as pinned). Consider centralizing this version parsing logic to ensure CLI doctor and auto-updater agree.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| configPath: string | ||
| } | ||
|
|
||
| function isExplicitVersionPin(pinnedVersion: string): boolean { |
There was a problem hiding this comment.
P2: This introduces inconsistent logic across the codebase. src/cli/doctor/checks/system-plugin.ts still uses the old logic for isPinned (treating dist-tags like beta as pinned). Consider centralizing this version parsing logic to ensure CLI doctor and auto-updater agree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/auto-update-checker/checker/plugin-entry.ts, line 14:
<comment>This introduces inconsistent logic across the codebase. `src/cli/doctor/checks/system-plugin.ts` still uses the old logic for `isPinned` (treating dist-tags like `beta` as pinned). Consider centralizing this version parsing logic to ensure CLI doctor and auto-updater agree.</comment>
<file context>
@@ -11,6 +11,10 @@ export interface PluginEntryInfo {
configPath: string
}
+function isExplicitVersionPin(pinnedVersion: string): boolean {
+ return /^\d+\.\d+\.\d+/.test(pinnedVersion)
+}
</file context>
|
@cubic-dev-ai Please re-review. The |
@code-yeongyu I have started the AI code review. It will take a few minutes to complete. |
…tags as not pinned
|
@cubic-dev-ai Please re-review. Fixed the |
@code-yeongyu I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 14 files
Confidence score: 4/5
- This PR looks safe to merge; the only issues are broken links in
README.zh-cn.md, which are low impact and documentation-only. - All three issues point to non-existent docs paths, so users following those links will hit dead ends but core functionality is unaffected.
- Pay close attention to
README.zh-cn.md- fix or update the brokendocs/manifesto.md,docs/guide/orchestration.md, anddocs/reference/features.mdlinks.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.zh-cn.md">
<violation number="1" location="README.zh-cn.md:264">
P2: Broken link: `docs/reference/features.md` does not exist.</violation>
<violation number="2" location="README.zh-cn.md:268">
P2: Broken link: `docs/guide/orchestration.md` does not exist.</violation>
<violation number="3" location="README.zh-cn.md:304">
P2: Broken link: `docs/manifesto.md` does not exist.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| - **生产力功能**：Ralph Loop、Todo Enforcer、Comment Checker、Think Mode 等 | ||
|
|
||
| ## 配置 | ||
| **想知道做这个插件的哲学理念吗？** 阅读 [Ultrawork 宣言](docs/manifesto.md)。 |
There was a problem hiding this comment.
P2: Broken link: docs/manifesto.md does not exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.zh-cn.md, line 304:
<comment>Broken link: `docs/manifesto.md` does not exist.</comment>
<file context>
@@ -1,392 +1,347 @@
-- **生产力功能**：Ralph Loop、Todo Enforcer、Comment Checker、Think Mode 等
-
-## 配置
+**想知道做这个插件的哲学理念吗？** 阅读 [Ultrawork 宣言](docs/manifesto.md)。
-个性鲜明，但可以根据个人喜好调整。
</file context>
|
|
||
| --- | ||
|
|
||
| > **第一次用 oh-my-opencode？** 阅读 **[概述](docs/guide/overview.md)** 了解你拥有哪些功能，或查看 **[编排指南](docs/guide/orchestration.md)** 了解 Agent 如何协作。 |
There was a problem hiding this comment.
P2: Broken link: docs/guide/orchestration.md does not exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.zh-cn.md, line 268:
<comment>Broken link: `docs/guide/orchestration.md` does not exist.</comment>
<file context>
@@ -1,392 +1,347 @@
+
+---
+
+> **第一次用 oh-my-opencode？** 阅读 **[概述](docs/guide/overview.md)** 了解你拥有哪些功能，或查看 **[编排指南](docs/guide/orchestration.md)** 了解 Agent 如何协作。
+
+## 如何卸载 (Uninstallation)
</file context>
|
|
||
| 想加你自己的？放进 `.opencode/skills/*/SKILL.md` 或者 `~/.config/opencode/skills/*/SKILL.md` 就行。 | ||
|
|
||
| **想看所有的硬核功能说明吗？** 点击查看 **[详细特性文档 (Features)](docs/reference/features.md)** ，深入了解 Agent 架构、Hook 流水线、核心工具链和所有的内置 MCP 等等。 |
There was a problem hiding this comment.
P2: Broken link: docs/reference/features.md does not exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.zh-cn.md, line 264:
<comment>Broken link: `docs/reference/features.md` does not exist.</comment>
<file context>
@@ -1,392 +1,347 @@
+
+想加你自己的？放进 `.opencode/skills/*/SKILL.md` 或者 `~/.config/opencode/skills/*/SKILL.md` 就行。
+
+**想看所有的硬核功能说明吗？** 点击查看 **[详细特性文档 (Features)](docs/reference/features.md)** ，深入了解 Agent 架构、Hook 流水线、核心工具链和所有的内置 MCP 等等。
+
+---
</file context>
1 participant
Summary
isPinnednow returnstrueonly for explicit semver pins (for example,3.5.2), not dist-tags or bare package nameoh-my-opencode, prerelease ->oh-my-opencode@{channel}Problem
The installer writes exact versions like
oh-my-opencode@3.5.2intoopencode.json. The auto-updater treated any non-latestvalue as user-pinned, so installer users were blocked from auto-updates and shown a misleading restart message.Fix
findPluginEntrylatest,beta,next) and bare package name are treated as auto-updatable channelsoh-my-opencodeoh-my-opencode@{channel}(for example,@beta)Update available: X.Y.Z (version pinned, update manually)findPluginEntrytests for bare package,@latest,@beta, and explicit semver pinAddresses cubic concern on #1985
PR #1985 dropped prerelease channel information on fallback. This change preserves prerelease channel tracking by writing
oh-my-opencode@{channel}when the current version is prerelease.Verification
bun test src/hooks/auto-update-checker/bun test src/cli/config-manager/bun run buildbun testSummary by cubic
Treats only exact semver versions (e.g., 3.5.2) as pinned to restore auto‑updates for installer users, preserves prerelease channel tracking, and aligns the doctor output. Also clarifies the pinned-version toast and stabilizes CI by isolating a flaky recovery test.
Written for commit 8f37d7f. Summary will update on new commits.