From f0591515fd2b3af01c05f22aae5417c3b56f65af Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 07:02:24 +0000 Subject: [PATCH 01/13] fix(skills): preserve SKILL.md content when renaming Rename skills by moving the skill directory and updating frontmatter name instead of recreate-with-stub-description, which wiped the body and supporting files. Co-authored-by: Serhii Dziupin --- .../sections/skills/SkillsSidebar.tsx | 32 ++------ .../ui/src/lib/i18n/messages/de.settings.ts | 1 + .../ui/src/lib/i18n/messages/en.settings.ts | 1 + .../ui/src/lib/i18n/messages/es.settings.ts | 1 + .../ui/src/lib/i18n/messages/fr.settings.ts | 1 + .../ui/src/lib/i18n/messages/ja.settings.ts | 1 + .../ui/src/lib/i18n/messages/ko.settings.ts | 1 + .../ui/src/lib/i18n/messages/pl.settings.ts | 1 + .../src/lib/i18n/messages/pt-BR.settings.ts | 1 + .../ui/src/lib/i18n/messages/uk.settings.ts | 1 + .../src/lib/i18n/messages/zh-CN.settings.ts | 1 + .../src/lib/i18n/messages/zh-TW.settings.ts | 1 + packages/ui/src/stores/useSkillsStore.ts | 45 +++++++++++ packages/vscode/src/bridge-config-runtime.ts | 19 +++++ packages/vscode/src/opencodeConfig.ts | 58 +++++++++++++- .../web/server/lib/opencode/DOCUMENTATION.md | 1 + .../lib/opencode/feature-routes-runtime.js | 3 +- .../web/server/lib/opencode/skill-routes.js | 17 ++++ packages/web/server/lib/opencode/skills.js | 78 ++++++++++++++++++- .../web/server/lib/opencode/skills.test.js | 70 ++++++++++++++++- 20 files changed, 301 insertions(+), 33 deletions(-) diff --git a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx index 0b13c6e0..5060624e 100644 --- a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx +++ b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx @@ -49,16 +49,16 @@ export const SkillsSidebar: React.FC = ({ onItemSelect }) => skills, setSelectedSkill, setSkillDraft, - createSkill, deleteSkill, + renameSkill, getSkillDetail, } = useSkillsStore(useShallow((s) => ({ selectedSkillName: s.selectedSkillName, skills: s.skills, setSelectedSkill: s.setSelectedSkill, setSkillDraft: s.setSkillDraft, - createSkill: s.createSkill, deleteSkill: s.deleteSkill, + renameSkill: s.renameSkill, getSkillDetail: s.getSkillDetail, }))); @@ -169,31 +169,11 @@ export const SkillsSidebar: React.FC = ({ onItemSelect }) => return; } - // Get full detail to copy - const detail = await getSkillDetail(renameDialogSkill.name); - if (!detail) { - toast.error(t('settings.skills.sidebar.toast.renameLoadFailed')); - setRenameDialogSkill(null); - return; - } - - // Create new skill with new name - const success = await createSkill({ - name: sanitizedName, - description: 'Renamed skill', // Will need proper description - scope: renameDialogSkill.scope, - source: renameDialogSkill.source, - }); - + // Rename in place on disk so SKILL.md body and supporting files are preserved. + const success = await renameSkill(renameDialogSkill.name, sanitizedName); if (success) { - // Delete old skill - const deleteSuccess = await deleteSkill(renameDialogSkill.name); - if (deleteSuccess) { - toast.success(`Skill renamed to "${sanitizedName}"`); - setSelectedSkill(sanitizedName); - } else { - toast.error(t('settings.skills.sidebar.toast.removeOldAfterRenameFailed')); - } + toast.success(t('settings.skills.sidebar.toast.skillRenamed', { name: sanitizedName })); + setSelectedSkill(sanitizedName); } else { toast.error(t('settings.skills.sidebar.toast.renameFailed')); } diff --git a/packages/ui/src/lib/i18n/messages/de.settings.ts b/packages/ui/src/lib/i18n/messages/de.settings.ts index 8bc4fe64..4f9ea716 100644 --- a/packages/ui/src/lib/i18n/messages/de.settings.ts +++ b/packages/ui/src/lib/i18n/messages/de.settings.ts @@ -669,6 +669,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameLoadFailed': 'Skill-Details konnten nicht geladen werden', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Alter Skill konnte nach Umbenennung nicht entfernt werden', 'settings.skills.sidebar.toast.renameFailed': 'Skill konnte nicht umbenannt werden', + 'settings.skills.sidebar.toast.skillRenamed': 'Skill umbenannt in "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Skill löschen', 'settings.skills.sidebar.deleteDialog.description': 'Möchten Sie den Skill "{name}" wirklich löschen?', 'settings.skills.sidebar.renameDialog.title': 'Skill umbenennen', diff --git a/packages/ui/src/lib/i18n/messages/en.settings.ts b/packages/ui/src/lib/i18n/messages/en.settings.ts index ece90e3f..210f7f3b 100644 --- a/packages/ui/src/lib/i18n/messages/en.settings.ts +++ b/packages/ui/src/lib/i18n/messages/en.settings.ts @@ -721,6 +721,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameLoadFailed': 'Failed to load skill details', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Failed to remove old skill after rename', 'settings.skills.sidebar.toast.renameFailed': 'Failed to rename skill', + 'settings.skills.sidebar.toast.skillRenamed': 'Skill renamed to "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Delete Skill', 'settings.skills.sidebar.deleteDialog.description': 'Are you sure you want to delete skill "{name}"?', 'settings.skills.sidebar.renameDialog.title': 'Rename Skill', diff --git a/packages/ui/src/lib/i18n/messages/es.settings.ts b/packages/ui/src/lib/i18n/messages/es.settings.ts index 657a936c..59b253e5 100644 --- a/packages/ui/src/lib/i18n/messages/es.settings.ts +++ b/packages/ui/src/lib/i18n/messages/es.settings.ts @@ -688,6 +688,7 @@ export const settingsDict = { "settings.skills.sidebar.toast.renameLoadFailed": "No se pudo cargar la información de la habilidad", "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "No se pudo eliminar la habilidad antigua después del cambio de nombre", "settings.skills.sidebar.toast.renameFailed": "No se pudo cambiar el nombre de la habilidad", + "settings.skills.sidebar.toast.skillRenamed": "Habilidad renombrada a \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Eliminar habilidad", "settings.skills.sidebar.deleteDialog.description": "¿Estás seguro de que quieres eliminar la habilidad \"{name}\"?", "settings.skills.sidebar.renameDialog.title": "Cambiar nombre habilidad", diff --git a/packages/ui/src/lib/i18n/messages/fr.settings.ts b/packages/ui/src/lib/i18n/messages/fr.settings.ts index 050f7904..aa8ef8e0 100644 --- a/packages/ui/src/lib/i18n/messages/fr.settings.ts +++ b/packages/ui/src/lib/i18n/messages/fr.settings.ts @@ -609,6 +609,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameLoadFailed': 'Échec du chargement des détails du skill', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Échec de la suppression de l\'ancien skill après le renommage', 'settings.skills.sidebar.toast.renameFailed': 'Échec du renommage du skill', + 'settings.skills.sidebar.toast.skillRenamed': 'Skill renommé en "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Supprimer le skill', 'settings.skills.sidebar.deleteDialog.description': 'Êtes-vous sûr de vouloir supprimer le skill « {name} » ?', 'settings.skills.sidebar.renameDialog.title': 'Renommer le skill', diff --git a/packages/ui/src/lib/i18n/messages/ja.settings.ts b/packages/ui/src/lib/i18n/messages/ja.settings.ts index be7fe2b3..8e2e97de 100644 --- a/packages/ui/src/lib/i18n/messages/ja.settings.ts +++ b/packages/ui/src/lib/i18n/messages/ja.settings.ts @@ -721,6 +721,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameLoadFailed': 'Skill 詳細の読み込みに失敗しました', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '名前変更後に古い Skill の削除に失敗しました', 'settings.skills.sidebar.toast.renameFailed': 'Skill の名前変更に失敗しました', + 'settings.skills.sidebar.toast.skillRenamed': 'Skill の名前を「{name}」に変更しました', 'settings.skills.sidebar.deleteDialog.title': 'Skill を削除', 'settings.skills.sidebar.deleteDialog.description': 'Skill "{name}" を削除してもよろしいですか？', 'settings.skills.sidebar.renameDialog.title': 'Skill の名前変更', diff --git a/packages/ui/src/lib/i18n/messages/ko.settings.ts b/packages/ui/src/lib/i18n/messages/ko.settings.ts index accc0d40..6bb43de5 100644 --- a/packages/ui/src/lib/i18n/messages/ko.settings.ts +++ b/packages/ui/src/lib/i18n/messages/ko.settings.ts @@ -688,6 +688,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameLoadFailed': '스킬 세부 정보를 로드하지 못했습니다', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '이름 변경 후 이전 스킬을 제거하지 못했습니다', 'settings.skills.sidebar.toast.renameFailed': '스킬 이름을 변경하지 못했습니다', + 'settings.skills.sidebar.toast.skillRenamed': '스킬 이름이 "{name}"(으)로 변경되었습니다', 'settings.skills.sidebar.deleteDialog.title': '스킬 삭제', 'settings.skills.sidebar.deleteDialog.description': '스킬 "{name}"을 삭제하시겠습니까?', 'settings.skills.sidebar.renameDialog.title': '스킬 이름 변경', diff --git a/packages/ui/src/lib/i18n/messages/pl.settings.ts b/packages/ui/src/lib/i18n/messages/pl.settings.ts index 318072d8..27077db4 100644 --- a/packages/ui/src/lib/i18n/messages/pl.settings.ts +++ b/packages/ui/src/lib/i18n/messages/pl.settings.ts @@ -1867,6 +1867,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameFailed': 'Nie udało się zmienić nazwy umiejętności', 'settings.skills.sidebar.toast.renameLoadFailed': 'Nie udało się załadować szczegółów umiejętności', 'settings.skills.sidebar.toast.skillDeleted': 'Umiejętność „{name}” została usunięta', + 'settings.skills.sidebar.toast.skillRenamed': 'Zmieniono nazwę umiejętności na „{name}”', 'settings.skills.sidebar.total': 'Suma: {count}', 'settings.usage.pace.prediction': 'Prognoza: {prediction}', 'settings.usage.pace.predictionLabel': 'Prognoza: ', diff --git a/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts b/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts index a931c456..55273e8a 100644 --- a/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts +++ b/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts @@ -688,6 +688,7 @@ export const settingsDict = { "settings.skills.sidebar.toast.renameLoadFailed": "Não foi possível carregar as informações da habilidade", "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "Não foi possível excluir a habilidade antiga depois da renomeação", "settings.skills.sidebar.toast.renameFailed": "Não foi possível renomear da habilidade", + "settings.skills.sidebar.toast.skillRenamed": "Habilidade renomeada para \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Excluir habilidade", "settings.skills.sidebar.deleteDialog.description": "Tem certeza de que deseja excluir a habilidade \"{name}\"?", "settings.skills.sidebar.renameDialog.title": "Renomear habilidade", diff --git a/packages/ui/src/lib/i18n/messages/uk.settings.ts b/packages/ui/src/lib/i18n/messages/uk.settings.ts index 65655407..a32a8c01 100644 --- a/packages/ui/src/lib/i18n/messages/uk.settings.ts +++ b/packages/ui/src/lib/i18n/messages/uk.settings.ts @@ -688,6 +688,7 @@ export const settingsDict = { "settings.skills.sidebar.toast.renameLoadFailed": "Не вдалося завантажити деталі навичок", "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "Не вдалося видалити стару навичку після перейменування", "settings.skills.sidebar.toast.renameFailed": "Не вдалося перейменувати навичку", + "settings.skills.sidebar.toast.skillRenamed": "Навичку перейменовано на \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Видалити навичку", "settings.skills.sidebar.deleteDialog.description": "Ви впевнені, що бажаєте видалити навичку «{name}»?", "settings.skills.sidebar.renameDialog.title": "Перейменувати навичку", diff --git a/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts b/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts index 3650a25a..dc2c0b36 100644 --- a/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts +++ b/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts @@ -688,6 +688,7 @@ export const settingsDict = { 'settings.skills.sidebar.toast.renameLoadFailed': '加载技能详情失败', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '重命名后移除旧技能失败', 'settings.skills.sidebar.toast.renameFailed': '重命名技能失败', + 'settings.skills.sidebar.toast.skillRenamed': '技能已重命名为“{name}”', 'settings.skills.sidebar.deleteDialog.title': '删除技能', 'settings.skills.sidebar.deleteDialog.description': '确定要删除技能“{name}”吗？', 'settings.skills.sidebar.renameDialog.title': '重命名技能', diff --git a/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts b/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts index c027b3ad..8d35e718 100644 --- a/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts +++ b/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts @@ -685,6 +685,7 @@ 'settings.skills.sidebar.toast.renameLoadFailed': '載入 skill 詳情失敗', 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '重新命名後移除舊 skill 失敗', 'settings.skills.sidebar.toast.renameFailed': '重新命名 skill 失敗', + 'settings.skills.sidebar.toast.skillRenamed': 'skill 已重新命名為「{name}」', 'settings.skills.sidebar.deleteDialog.title': '刪除 Skill', 'settings.skills.sidebar.deleteDialog.description': '確定要刪除 skill「{name}」嗎？', 'settings.skills.sidebar.renameDialog.title': '重新命名 Skill', diff --git a/packages/ui/src/stores/useSkillsStore.ts b/packages/ui/src/stores/useSkillsStore.ts index 2ffe69dd..9acbde46 100644 --- a/packages/ui/src/stores/useSkillsStore.ts +++ b/packages/ui/src/stores/useSkillsStore.ts @@ -143,6 +143,7 @@ interface SkillsStore { getSkillDetail: (name: string) => Promise; createSkill: (config: SkillConfig) => Promise; updateSkill: (name: string, config: Partial) => Promise; + renameSkill: (name: string, newName: string) => Promise; deleteSkill: (name: string) => Promise; getSkillByName: (name: string) => DiscoveredSkill | undefined; @@ -382,6 +383,50 @@ export const useSkillsStore = create()( } }, + renameSkill: async (name: string, newName: string) => { + startConfigUpdate("Renaming skill..."); + let requiresReload = false; + try { + const currentDirectory = getCurrentDirectory(); + const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + + const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(name)}${queryParams}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ renameTo: newName }), + }); + + const payload = await response.json().catch(() => null); + if (!response.ok) { + const message = payload?.error || 'Failed to rename skill'; + throw new Error(message); + } + + const needsReload = payload?.requiresReload ?? false; + invalidateSkillsLoadCache(currentDirectory); + if (needsReload) { + requiresReload = true; + await refreshSkillsAfterOpenCodeRestart({ + message: payload?.message, + delayMs: payload?.reloadDelayMs, + }); + return true; + } + + const loaded = await get().loadSkills(); + if (loaded) { + emitConfigChange("skills", { source: CONFIG_EVENT_SOURCE }); + } + return loaded; + } catch { + return false; + } finally { + if (!requiresReload) { + finishConfigUpdate(); + } + } + }, + deleteSkill: async (name: string) => { startConfigUpdate("Deleting skill..."); let requiresReload = false; diff --git a/packages/vscode/src/bridge-config-runtime.ts b/packages/vscode/src/bridge-config-runtime.ts index 361f2a1a..569900e5 100644 --- a/packages/vscode/src/bridge-config-runtime.ts +++ b/packages/vscode/src/bridge-config-runtime.ts @@ -25,6 +25,7 @@ import { createSkill, updateSkill, deleteSkill, + renameSkill, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, @@ -693,6 +694,24 @@ export async function handleConfigBridgeMessage( } if (normalizedMethod === 'PATCH') { + if (typeof body?.renameTo === 'string') { + const newName = body.renameTo.trim(); + renameSkill(skillName, newName, workingDirectory); + await ctx?.manager?.restart(); + return { + id, + type, + success: true, + data: { + success: true, + name: newName, + requiresReload: true, + message: `Skill renamed to ${newName} successfully. Reloading interface…`, + reloadDelayMs: deps.clientReloadDelayMs, + }, + }; + } + updateSkill(skillName, (body || {}) as Record, workingDirectory); await ctx?.manager?.restart(); return { diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index 79a7455d..c46da9eb 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -2761,7 +2761,7 @@ export const updateSkill = (skillName: string, updates: Record, let mdModified = false; for (const [field, value] of Object.entries(updates || {})) { - if (field === 'scope') continue; + if (field === 'scope' || field === 'source' || field === 'targetPath' || field === 'renameTo') continue; if (field === 'instructions') { const normalizedValue = typeof value === 'string' ? value : value == null ? '' : String(value); @@ -2833,3 +2833,59 @@ export const deleteSkill = (skillName: string, workingDirectory?: string): void throw new Error(`Skill "${skillName}" not found`); } }; + +export const renameSkill = (oldName: string, newName: string, workingDirectory?: string): void => { + ensureSkillDirs(); + validateSkillName(newName); + + if (oldName === newName) { + return; + } + + const existing = getSkillScope(oldName, workingDirectory); + if (!existing.path) { + throw new Error(`Skill "${oldName}" not found`); + } + if (existing.path === BUILT_IN_SKILL_LOCATION || !fs.existsSync(existing.path)) { + throw new Error(`Skill "${oldName}" cannot be renamed`); + } + if (path.basename(existing.path) !== 'SKILL.md') { + throw new Error(`Skill "${oldName}" target must be a SKILL.md file`); + } + + const conflict = getSkillScope(newName, workingDirectory); + if (conflict.path) { + throw new Error(`Skill ${newName} already exists at ${conflict.path}`); + } + + const oldDir = path.dirname(existing.path); + const newDir = path.join(path.dirname(oldDir), newName); + const directoriesDiffer = path.resolve(oldDir) !== path.resolve(newDir); + + if (directoriesDiffer && fs.existsSync(newDir)) { + throw new Error(`Skill directory already exists at ${newDir}`); + } + + if (directoriesDiffer) { + fs.renameSync(oldDir, newDir); + } + + const newPath = path.join(newDir, 'SKILL.md'); + try { + const mdData = parseMdFile(newPath); + mdData.frontmatter = { + ...mdData.frontmatter, + name: newName, + }; + writeMdFile(newPath, mdData.frontmatter, mdData.body); + } catch (error) { + if (directoriesDiffer && fs.existsSync(newDir) && !fs.existsSync(oldDir)) { + try { + fs.renameSync(newDir, oldDir); + } catch { + // Best-effort rollback; surface the original write failure. + } + } + throw error; + } +}; diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index c57ecb8b..02088195 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -351,6 +351,7 @@ an authoritative loopback callback URL even when OpenChamber binds port `0`. ## Public exports (skill-routes.js) - `registerSkillRoutes(app, dependencies)`: registers skills-related routes: - Skills config CRUD and metadata under `/api/config/skills*` + - Skill rename via `PATCH /api/config/skills/:name` with `{ renameTo }` (directory rename preserves `SKILL.md` body and supporting files) - Skills catalog listing/source pagination, scan, and install routes - Supporting skill file read/write/delete routes diff --git a/packages/web/server/lib/opencode/feature-routes-runtime.js b/packages/web/server/lib/opencode/feature-routes-runtime.js index d127515d..dd3726d7 100644 --- a/packages/web/server/lib/opencode/feature-routes-runtime.js +++ b/packages/web/server/lib/opencode/feature-routes-runtime.js @@ -38,7 +38,7 @@ import { decodePluginId, } from './plugins.js'; import { SKILL_DIR, SKILL_SCOPE, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile } from './shared.js'; -import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill } from './skills.js'; +import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill, renameSkill } from './skills.js'; import { getCuratedSkillsSources } from '../skills-catalog/curated-sources.js'; import { getCacheKey, getCachedScan, setCachedScan } from '../skills-catalog/cache.js'; import { isClawdHubSource, parseSkillRepoSource } from '../skills-catalog/source.js'; @@ -256,6 +256,7 @@ export const createFeatureRoutesRuntime = (dependencies) => { createSkill, updateSkill, deleteSkill, + renameSkill, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, diff --git a/packages/web/server/lib/opencode/skill-routes.js b/packages/web/server/lib/opencode/skill-routes.js index b66680a5..314e6b99 100644 --- a/packages/web/server/lib/opencode/skill-routes.js +++ b/packages/web/server/lib/opencode/skill-routes.js @@ -21,6 +21,7 @@ export const registerSkillRoutes = (app, dependencies) => { createSkill, updateSkill, deleteSkill, + renameSkill, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, @@ -611,6 +612,22 @@ export const registerSkillRoutes = (app, dependencies) => { return res.status(400).json({ error }); } + if (typeof updates?.renameTo === 'string') { + const newName = updates.renameTo.trim(); + console.log(`[Server] Renaming skill: ${skillName} -> ${newName}`); + console.log('[Server] Working directory:', directory); + renameSkill(skillName, newName, directory); + await refreshOpenCodeAfterConfigChange('skill rename'); + + return res.json({ + success: true, + name: newName, + requiresReload: true, + message: `Skill renamed to ${newName} successfully. Reloading interface…`, + reloadDelayMs: clientReloadDelayMs, + }); + } + console.log(`[Server] Updating skill: ${skillName}`); console.log('[Server] Working directory:', directory); diff --git a/packages/web/server/lib/opencode/skills.js b/packages/web/server/lib/opencode/skills.js index 91027b52..5f1a53d2 100644 --- a/packages/web/server/lib/opencode/skills.js +++ b/packages/web/server/lib/opencode/skills.js @@ -412,12 +412,22 @@ function getSkillSources(skillName, workingDirectory, discoveredSkill = null) { return sources; } -function createSkill(skillName, config, workingDirectory, scope) { - ensureDirs(); +function isValidSkillName(skillName) { + return typeof skillName === 'string' + && skillName.length > 0 + && skillName.length <= 64 + && /^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$/.test(skillName); +} - if (!/^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$/.test(skillName) || skillName.length > 64) { +function assertValidSkillName(skillName) { + if (!isValidSkillName(skillName)) { throw new Error(`Invalid skill name "${skillName}". Must be 1-64 lowercase alphanumeric characters with hyphens, cannot start or end with hyphen.`); } +} + +function createSkill(skillName, config, workingDirectory, scope) { + ensureDirs(); + assertValidSkillName(skillName); const existing = getSkillScope(skillName, workingDirectory); if (existing.path) { @@ -505,7 +515,7 @@ function updateSkill(skillName, updates, workingDirectory, targetPath = null) { let mdModified = false; for (const [field, value] of Object.entries(updates)) { - if (field === 'scope' || field === 'source' || field === 'targetPath') { + if (field === 'scope' || field === 'source' || field === 'targetPath' || field === 'renameTo') { continue; } @@ -592,6 +602,65 @@ function deleteSkill(skillName, workingDirectory) { } } +function renameSkill(oldName, newName, workingDirectory) { + ensureDirs(); + assertValidSkillName(newName); + + if (oldName === newName) { + return; + } + + const existing = getSkillScope(oldName, workingDirectory); + if (!existing.path) { + throw new Error(`Skill "${oldName}" not found`); + } + if (existing.path === BUILT_IN_SKILL_LOCATION || !fs.existsSync(existing.path)) { + throw new Error(`Skill "${oldName}" cannot be renamed`); + } + if (path.basename(existing.path) !== 'SKILL.md') { + throw new Error(`Skill "${oldName}" target must be a SKILL.md file`); + } + + const conflict = getSkillScope(newName, workingDirectory); + if (conflict.path) { + throw new Error(`Skill ${newName} already exists at ${conflict.path}`); + } + + const oldDir = path.dirname(existing.path); + const newDir = path.join(path.dirname(oldDir), newName); + const directoriesDiffer = path.resolve(oldDir) !== path.resolve(newDir); + + if (directoriesDiffer && fs.existsSync(newDir)) { + throw new Error(`Skill directory already exists at ${newDir}`); + } + + // Rename the skill directory in place so supporting files and SKILL.md body are preserved. + if (directoriesDiffer) { + fs.renameSync(oldDir, newDir); + } + + const newPath = path.join(newDir, 'SKILL.md'); + try { + const mdData = parseMdFile(newPath); + mdData.frontmatter = { + ...mdData.frontmatter, + name: newName, + }; + writeMdFile(newPath, mdData.frontmatter, mdData.body); + } catch (error) { + if (directoriesDiffer && fs.existsSync(newDir) && !fs.existsSync(oldDir)) { + try { + fs.renameSync(newDir, oldDir); + } catch (rollbackError) { + console.error(`Failed to rollback skill rename from ${newDir} to ${oldDir}:`, rollbackError); + } + } + throw error; + } + + console.log(`Renamed skill: ${oldName} -> ${newName} (path: ${newPath})`); +} + export { getSkillSources, discoverSkills, @@ -599,4 +668,5 @@ export { createSkill, updateSkill, deleteSkill, + renameSkill, }; diff --git a/packages/web/server/lib/opencode/skills.test.js b/packages/web/server/lib/opencode/skills.test.js index 95f9c722..bda9ea97 100644 --- a/packages/web/server/lib/opencode/skills.test.js +++ b/packages/web/server/lib/opencode/skills.test.js @@ -1,8 +1,9 @@ import { describe, expect, it } from 'vitest'; +import fs from 'fs'; import fsPromises from 'fs/promises'; import os from 'os'; import path from 'path'; -import { getSkillSources, mergeDiscoveredSkills } from './skills.js'; +import { getSkillSources, mergeDiscoveredSkills, renameSkill } from './skills.js'; describe('skills', () => { it('merges locally discovered skills missing from OpenCode live discovery', () => { @@ -110,4 +111,71 @@ describe('skills', () => { await fsPromises.rm(tempRoot, { recursive: true, force: true }); } }); + + it('renames a skill directory while preserving SKILL.md body and supporting files', async () => { + const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-rename-')); + const projectRoot = path.join(tempRoot, 'project'); + const skillDir = path.join(projectRoot, '.opencode', 'skills', 'original-skill'); + const skillPath = path.join(skillDir, 'SKILL.md'); + const supportPath = path.join(skillDir, 'notes.md'); + const body = [ + '# Original Skill', + '', + 'Preserve this non-trivial body across rename.', + '', + '## Details', + '', + '- step one', + '- step two', + ].join('\n'); + + try { + await fsPromises.mkdir(skillDir, { recursive: true }); + await fsPromises.writeFile( + skillPath, + [ + '---', + 'name: original-skill', + 'description: Original skill description', + 'license: MIT', + '---', + '', + body, + '', + ].join('\n'), + 'utf8', + ); + await fsPromises.writeFile(supportPath, 'supporting file contents\n', 'utf8'); + + renameSkill('original-skill', 'renamed-skill', projectRoot); + + const renamedDir = path.join(projectRoot, '.opencode', 'skills', 'renamed-skill'); + const renamedPath = path.join(renamedDir, 'SKILL.md'); + const renamedSupportPath = path.join(renamedDir, 'notes.md'); + + expect(fs.existsSync(skillDir)).toBe(false); + expect(fs.existsSync(renamedPath)).toBe(true); + expect(fs.existsSync(renamedSupportPath)).toBe(true); + + const sources = getSkillSources('renamed-skill', projectRoot, { + name: 'renamed-skill', + path: renamedPath, + scope: 'project', + source: 'opencode', + description: 'fallback', + }); + + expect(sources.md.exists).toBe(true); + expect(sources.md.name).toBe('renamed-skill'); + expect(sources.md.description).toBe('Original skill description'); + expect(sources.md.instructions).toBe(body); + expect(await fsPromises.readFile(renamedSupportPath, 'utf8')).toBe('supporting file contents\n'); + + const raw = await fsPromises.readFile(renamedPath, 'utf8'); + expect(raw).toContain('license: MIT'); + expect(raw).not.toContain('Renamed skill'); + } finally { + await fsPromises.rm(tempRoot, { recursive: true, force: true }); + } + }); }); From bfea13ef1d8a67e7bb417d1377513a2d6c093209 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 08:54:29 +0000 Subject: [PATCH 02/13] fix(skills): harden rename to managed roots and cover failures Restrict in-place skill rename to managed skill directories, require frontmatter name to match before moving, roll back/reject with tests, hide rename in the UI for unmanaged paths, and drop unused toast keys. Co-authored-by: Serhii Dziupin --- .../sections/skills/SkillsSidebar.tsx | 19 ++- .../sections/skills/skillLocations.ts | 19 +++ .../ui/src/lib/i18n/messages/de.settings.ts | 2 - .../ui/src/lib/i18n/messages/en.settings.ts | 2 - .../ui/src/lib/i18n/messages/es.settings.ts | 2 - .../ui/src/lib/i18n/messages/fr.settings.ts | 2 - .../ui/src/lib/i18n/messages/ja.settings.ts | 2 - .../ui/src/lib/i18n/messages/ko.settings.ts | 2 - .../ui/src/lib/i18n/messages/pl.settings.ts | 2 - .../src/lib/i18n/messages/pt-BR.settings.ts | 2 - .../ui/src/lib/i18n/messages/uk.settings.ts | 2 - .../src/lib/i18n/messages/zh-CN.settings.ts | 2 - .../src/lib/i18n/messages/zh-TW.settings.ts | 2 - packages/vscode/src/opencodeConfig.ts | 62 ++++++++ .../web/server/lib/opencode/DOCUMENTATION.md | 2 +- packages/web/server/lib/opencode/skills.js | 65 ++++++++ .../web/server/lib/opencode/skills.test.js | 143 ++++++++++++++++++ 17 files changed, 303 insertions(+), 29 deletions(-) diff --git a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx index 5060624e..98646486 100644 --- a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx +++ b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx @@ -27,6 +27,7 @@ import { SidebarGroup } from '@/components/sections/shared/SidebarGroup'; import { Icon } from "@/components/icon/Icon"; import { useI18n } from '@/lib/i18n'; import { SETTINGS_PANEL_TITLE_CLASS } from '@/components/sections/shared/SettingsSection'; +import { isManagedSkillFilesystemPath } from '@/components/sections/skills/skillLocations'; interface SkillsSidebarProps { onItemSelect?: () => void; @@ -35,6 +36,9 @@ interface SkillsSidebarProps { const BUILT_IN_SKILL_LOCATION = ''; const isBuiltInSkill = (skill: DiscoveredSkill | null | undefined): boolean => skill?.path === BUILT_IN_SKILL_LOCATION; +const isRenamableSkill = (skill: DiscoveredSkill | null | undefined): boolean => ( + !!skill && !isBuiltInSkill(skill) && isManagedSkillFilesystemPath(skill.path) +); export const SkillsSidebar: React.FC = ({ onItemSelect }) => { const { t } = useI18n(); @@ -140,14 +144,14 @@ export const SkillsSidebar: React.FC = ({ onItemSelect }) => }; const handleOpenRenameDialog = (skill: DiscoveredSkill) => { - if (isBuiltInSkill(skill)) return; + if (!isRenamableSkill(skill)) return; setRenameNewName(skill.name); setRenameDialogSkill(skill); }; const handleRenameSkill = async () => { if (!renameDialogSkill) return; - if (isBuiltInSkill(renameDialogSkill)) { + if (!isRenamableSkill(renameDialogSkill)) { setRenameDialogSkill(null); return; } @@ -443,13 +447,16 @@ const SkillListItem: React.FC = ({ : t('settings.skills.sidebar.badge.opencode'); const badgeClassName = 'typography-micro text-muted-foreground bg-[var(--surface-muted)] px-1 rounded flex-shrink-0 leading-none pb-px border border-[var(--interactive-border)]/50'; const isBuiltIn = isBuiltInSkill(skill); + const canRename = isRenamableSkill(skill); const [isContextMenuOpen, setIsContextMenuOpen] = React.useState(false); const renderMenuItems = (Item: React.ElementType) => ( <> - { e.stopPropagation(); onRename(); }}> - - {t('settings.common.actions.rename')} - + {canRename ? ( + { e.stopPropagation(); onRename(); }}> + + {t('settings.common.actions.rename')} + + ) : null} { e.stopPropagation(); onDuplicate(); }}> {t('settings.common.actions.duplicate')} diff --git a/packages/ui/src/components/sections/skills/skillLocations.ts b/packages/ui/src/components/sections/skills/skillLocations.ts index a0009a20..cc07fd5e 100644 --- a/packages/ui/src/components/sections/skills/skillLocations.ts +++ b/packages/ui/src/components/sections/skills/skillLocations.ts @@ -57,3 +57,22 @@ export function locationPartsFrom(value: SkillLocationValue): { scope: SkillScop } return { scope: match.scope, source: match.source }; } + +/** True when a discovered skill path is under a managed skill root that rename/delete may mutate. */ +export function isManagedSkillFilesystemPath(skillPath: string | null | undefined): boolean { + if (!skillPath || skillPath === '') return false; + const normalized = skillPath.replace(/\\/g, '/'); + if ( + normalized.includes('/.cache/opencode/skills/') + || normalized.includes('/Caches/opencode/skills/') + || normalized.includes('/Library/Caches/opencode/skills/') + ) { + return false; + } + return ( + /\/\.opencode\/skills?\//.test(normalized) + || /\/\.claude\/skills\//.test(normalized) + || /\/\.agents\/skills\//.test(normalized) + || /\/\.config\/opencode\/skills?\//.test(normalized) + ); +} diff --git a/packages/ui/src/lib/i18n/messages/de.settings.ts b/packages/ui/src/lib/i18n/messages/de.settings.ts index 4f9ea716..d35dbb81 100644 --- a/packages/ui/src/lib/i18n/messages/de.settings.ts +++ b/packages/ui/src/lib/i18n/messages/de.settings.ts @@ -666,8 +666,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" erfolgreich gelöscht', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Skill konnte nicht gelöscht werden', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Skill-Details für Duplizierung konnten nicht geladen werden', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Skill-Details konnten nicht geladen werden', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Alter Skill konnte nach Umbenennung nicht entfernt werden', 'settings.skills.sidebar.toast.renameFailed': 'Skill konnte nicht umbenannt werden', 'settings.skills.sidebar.toast.skillRenamed': 'Skill umbenannt in "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Skill löschen', diff --git a/packages/ui/src/lib/i18n/messages/en.settings.ts b/packages/ui/src/lib/i18n/messages/en.settings.ts index 210f7f3b..fa36d723 100644 --- a/packages/ui/src/lib/i18n/messages/en.settings.ts +++ b/packages/ui/src/lib/i18n/messages/en.settings.ts @@ -718,8 +718,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" deleted successfully', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Failed to delete skill', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Failed to load skill details for duplication', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Failed to load skill details', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Failed to remove old skill after rename', 'settings.skills.sidebar.toast.renameFailed': 'Failed to rename skill', 'settings.skills.sidebar.toast.skillRenamed': 'Skill renamed to "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Delete Skill', diff --git a/packages/ui/src/lib/i18n/messages/es.settings.ts b/packages/ui/src/lib/i18n/messages/es.settings.ts index 59b253e5..8ed3af1a 100644 --- a/packages/ui/src/lib/i18n/messages/es.settings.ts +++ b/packages/ui/src/lib/i18n/messages/es.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { "settings.skills.sidebar.toast.skillDeleted": "Habilidad \"{name}\" eliminada con éxito", "settings.skills.sidebar.toast.deleteSkillFailed": "No se pudo eliminar la habilidad", "settings.skills.sidebar.toast.duplicateLoadFailed": "No se pudo cargar la información de la habilidad para duplicarla", - "settings.skills.sidebar.toast.renameLoadFailed": "No se pudo cargar la información de la habilidad", - "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "No se pudo eliminar la habilidad antigua después del cambio de nombre", "settings.skills.sidebar.toast.renameFailed": "No se pudo cambiar el nombre de la habilidad", "settings.skills.sidebar.toast.skillRenamed": "Habilidad renombrada a \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Eliminar habilidad", diff --git a/packages/ui/src/lib/i18n/messages/fr.settings.ts b/packages/ui/src/lib/i18n/messages/fr.settings.ts index aa8ef8e0..afb4cf1b 100644 --- a/packages/ui/src/lib/i18n/messages/fr.settings.ts +++ b/packages/ui/src/lib/i18n/messages/fr.settings.ts @@ -606,8 +606,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" supprimé avec succès', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Échec de la suppression du skill', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Échec du chargement des détails du skill pour la duplication', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Échec du chargement des détails du skill', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Échec de la suppression de l\'ancien skill après le renommage', 'settings.skills.sidebar.toast.renameFailed': 'Échec du renommage du skill', 'settings.skills.sidebar.toast.skillRenamed': 'Skill renommé en "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Supprimer le skill', diff --git a/packages/ui/src/lib/i18n/messages/ja.settings.ts b/packages/ui/src/lib/i18n/messages/ja.settings.ts index 8e2e97de..b820e176 100644 --- a/packages/ui/src/lib/i18n/messages/ja.settings.ts +++ b/packages/ui/src/lib/i18n/messages/ja.settings.ts @@ -718,8 +718,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" を削除しました', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Skill の削除に失敗しました', 'settings.skills.sidebar.toast.duplicateLoadFailed': '複製用の Skill 詳細の読み込みに失敗しました', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Skill 詳細の読み込みに失敗しました', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '名前変更後に古い Skill の削除に失敗しました', 'settings.skills.sidebar.toast.renameFailed': 'Skill の名前変更に失敗しました', 'settings.skills.sidebar.toast.skillRenamed': 'Skill の名前を「{name}」に変更しました', 'settings.skills.sidebar.deleteDialog.title': 'Skill を削除', diff --git a/packages/ui/src/lib/i18n/messages/ko.settings.ts b/packages/ui/src/lib/i18n/messages/ko.settings.ts index 6bb43de5..80bde6b0 100644 --- a/packages/ui/src/lib/i18n/messages/ko.settings.ts +++ b/packages/ui/src/lib/i18n/messages/ko.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': '스킬 "{name}"을 삭제했습니다', 'settings.skills.sidebar.toast.deleteSkillFailed': '스킬을 삭제하지 못했습니다', 'settings.skills.sidebar.toast.duplicateLoadFailed': '복제를 위한 스킬 세부 정보를 로드하지 못했습니다', - 'settings.skills.sidebar.toast.renameLoadFailed': '스킬 세부 정보를 로드하지 못했습니다', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '이름 변경 후 이전 스킬을 제거하지 못했습니다', 'settings.skills.sidebar.toast.renameFailed': '스킬 이름을 변경하지 못했습니다', 'settings.skills.sidebar.toast.skillRenamed': '스킬 이름이 "{name}"(으)로 변경되었습니다', 'settings.skills.sidebar.deleteDialog.title': '스킬 삭제', diff --git a/packages/ui/src/lib/i18n/messages/pl.settings.ts b/packages/ui/src/lib/i18n/messages/pl.settings.ts index 27077db4..eb524ec0 100644 --- a/packages/ui/src/lib/i18n/messages/pl.settings.ts +++ b/packages/ui/src/lib/i18n/messages/pl.settings.ts @@ -1863,9 +1863,7 @@ export const settingsDict = { 'settings.skills.sidebar.title': 'Umiejętności', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Nie udało się usunąć umiejętności', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Nie udało się załadować szczegółów umiejętności do duplikacji', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Nie udało się usunąć starej umiejętności po zmianie nazwy', 'settings.skills.sidebar.toast.renameFailed': 'Nie udało się zmienić nazwy umiejętności', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Nie udało się załadować szczegółów umiejętności', 'settings.skills.sidebar.toast.skillDeleted': 'Umiejętność „{name}” została usunięta', 'settings.skills.sidebar.toast.skillRenamed': 'Zmieniono nazwę umiejętności na „{name}”', 'settings.skills.sidebar.total': 'Suma: {count}', diff --git a/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts b/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts index 55273e8a..d83e4afb 100644 --- a/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts +++ b/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { "settings.skills.sidebar.toast.skillDeleted": "Habilidade \"{name}\" excluída com sucesso", "settings.skills.sidebar.toast.deleteSkillFailed": "Não foi possível excluir a habilidade", "settings.skills.sidebar.toast.duplicateLoadFailed": "Não foi possível carregar as informações da habilidade para duplicá-la", - "settings.skills.sidebar.toast.renameLoadFailed": "Não foi possível carregar as informações da habilidade", - "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "Não foi possível excluir a habilidade antiga depois da renomeação", "settings.skills.sidebar.toast.renameFailed": "Não foi possível renomear da habilidade", "settings.skills.sidebar.toast.skillRenamed": "Habilidade renomeada para \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Excluir habilidade", diff --git a/packages/ui/src/lib/i18n/messages/uk.settings.ts b/packages/ui/src/lib/i18n/messages/uk.settings.ts index a32a8c01..0a64e6e3 100644 --- a/packages/ui/src/lib/i18n/messages/uk.settings.ts +++ b/packages/ui/src/lib/i18n/messages/uk.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { "settings.skills.sidebar.toast.skillDeleted": "Навичку \"{name}\" успішно видалено", "settings.skills.sidebar.toast.deleteSkillFailed": "Не вдалося видалити навичку", "settings.skills.sidebar.toast.duplicateLoadFailed": "Не вдалося завантажити деталі навичок для дублювання", - "settings.skills.sidebar.toast.renameLoadFailed": "Не вдалося завантажити деталі навичок", - "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "Не вдалося видалити стару навичку після перейменування", "settings.skills.sidebar.toast.renameFailed": "Не вдалося перейменувати навичку", "settings.skills.sidebar.toast.skillRenamed": "Навичку перейменовано на \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Видалити навичку", diff --git a/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts b/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts index dc2c0b36..f906328d 100644 --- a/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts +++ b/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': '技能“{name}”已删除', 'settings.skills.sidebar.toast.deleteSkillFailed': '删除技能失败', 'settings.skills.sidebar.toast.duplicateLoadFailed': '加载技能详情以复制失败', - 'settings.skills.sidebar.toast.renameLoadFailed': '加载技能详情失败', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '重命名后移除旧技能失败', 'settings.skills.sidebar.toast.renameFailed': '重命名技能失败', 'settings.skills.sidebar.toast.skillRenamed': '技能已重命名为“{name}”', 'settings.skills.sidebar.deleteDialog.title': '删除技能', diff --git a/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts b/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts index 8d35e718..29563dee 100644 --- a/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts +++ b/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts @@ -682,8 +682,6 @@ 'settings.skills.sidebar.toast.skillDeleted': 'skill「{name}」已刪除', 'settings.skills.sidebar.toast.deleteSkillFailed': '刪除 skill 失敗', 'settings.skills.sidebar.toast.duplicateLoadFailed': '複製 skill 的詳細資訊載入失敗', - 'settings.skills.sidebar.toast.renameLoadFailed': '載入 skill 詳情失敗', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '重新命名後移除舊 skill 失敗', 'settings.skills.sidebar.toast.renameFailed': '重新命名 skill 失敗', 'settings.skills.sidebar.toast.skillRenamed': 'skill 已重新命名為「{name}」', 'settings.skills.sidebar.deleteDialog.title': '刪除 Skill', diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index c46da9eb..1ecce46e 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -2834,6 +2834,57 @@ export const deleteSkill = (skillName: string, workingDirectory?: string): void } }; +const isPathInside = (candidatePath: string, parentPath: string): boolean => { + const resolvedCandidate = path.resolve(candidatePath); + const resolvedParent = path.resolve(parentPath); + return resolvedCandidate === resolvedParent + || resolvedCandidate.startsWith(`${resolvedParent}${path.sep}`); +}; + +const getManagedSkillRoots = (workingDirectory?: string): string[] => { + const roots: string[] = []; + const pushRoot = (dir?: string | null) => { + if (!dir) return; + const resolved = path.resolve(dir); + if (!roots.includes(resolved)) { + roots.push(resolved); + } + }; + + pushRoot(SKILL_DIR); + pushRoot(path.join(OPENCODE_CONFIG_DIR, 'skill')); + pushRoot(path.join(os.homedir(), '.opencode', 'skills')); + pushRoot(path.join(os.homedir(), '.opencode', 'skill')); + pushRoot(path.join(os.homedir(), '.claude', 'skills')); + pushRoot(path.join(os.homedir(), '.agents', 'skills')); + + const customConfigDir = process.env.OPENCODE_CONFIG_DIR + ? path.resolve(process.env.OPENCODE_CONFIG_DIR) + : null; + pushRoot(customConfigDir ? path.join(customConfigDir, 'skills') : null); + pushRoot(customConfigDir ? path.join(customConfigDir, 'skill') : null); + + if (workingDirectory) { + const worktreeRoot = findWorktreeRoot(workingDirectory) || path.resolve(workingDirectory); + for (const ancestor of getAncestors(workingDirectory, worktreeRoot)) { + pushRoot(path.join(ancestor, '.opencode', 'skills')); + pushRoot(path.join(ancestor, '.opencode', 'skill')); + pushRoot(path.join(ancestor, '.claude', 'skills')); + pushRoot(path.join(ancestor, '.agents', 'skills')); + } + } + + return roots; +}; + +const isManagedSkillPath = (skillMdPath: string, workingDirectory?: string): boolean => { + if (!skillMdPath || skillMdPath === BUILT_IN_SKILL_LOCATION) { + return false; + } + const skillDir = path.dirname(path.resolve(skillMdPath)); + return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root)); +}; + export const renameSkill = (oldName: string, newName: string, workingDirectory?: string): void => { ensureSkillDirs(); validateSkillName(newName); @@ -2852,6 +2903,17 @@ export const renameSkill = (oldName: string, newName: string, workingDirectory?: if (path.basename(existing.path) !== 'SKILL.md') { throw new Error(`Skill "${oldName}" target must be a SKILL.md file`); } + if (!isManagedSkillPath(existing.path, workingDirectory)) { + throw new Error(`Skill "${oldName}" is outside managed skill directories and cannot be renamed`); + } + + const mdDataBeforeMove = parseMdFile(existing.path); + const frontmatterName = typeof mdDataBeforeMove.frontmatter?.name === 'string' + ? mdDataBeforeMove.frontmatter.name + : oldName; + if (frontmatterName !== oldName) { + throw new Error(`Skill "${oldName}" does not match ${existing.path}`); + } const conflict = getSkillScope(newName, workingDirectory); if (conflict.path) { diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index 02088195..1f21f955 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -351,7 +351,7 @@ an authoritative loopback callback URL even when OpenChamber binds port `0`. ## Public exports (skill-routes.js) - `registerSkillRoutes(app, dependencies)`: registers skills-related routes: - Skills config CRUD and metadata under `/api/config/skills*` - - Skill rename via `PATCH /api/config/skills/:name` with `{ renameTo }` (directory rename preserves `SKILL.md` body and supporting files) + - Skill rename via `PATCH /api/config/skills/:name` with `{ renameTo }` (directory rename preserves `SKILL.md` body and supporting files; restricted to managed skill roots under `.opencode/skills|skill`, `.claude/skills`, and `.agents/skills`) - Skills catalog listing/source pagination, scan, and install routes - Supporting skill file read/write/delete routes diff --git a/packages/web/server/lib/opencode/skills.js b/packages/web/server/lib/opencode/skills.js index 5f1a53d2..a0594ad3 100644 --- a/packages/web/server/lib/opencode/skills.js +++ b/packages/web/server/lib/opencode/skills.js @@ -602,6 +602,60 @@ function deleteSkill(skillName, workingDirectory) { } } +function isPathInside(candidatePath, parentPath) { + if (!candidatePath || !parentPath) return false; + const resolvedCandidate = path.resolve(candidatePath); + const resolvedParent = path.resolve(parentPath); + return resolvedCandidate === resolvedParent + || resolvedCandidate.startsWith(`${resolvedParent}${path.sep}`); +} + +function getManagedSkillRoots(workingDirectory) { + const roots = []; + const pushRoot = (dir) => { + if (!dir) return; + const resolved = path.resolve(dir); + if (!roots.includes(resolved)) { + roots.push(resolved); + } + }; + + pushRoot(SKILL_DIR); + pushRoot(path.join(OPENCODE_CONFIG_DIR, 'skill')); + pushRoot(path.join(os.homedir(), '.opencode', 'skills')); + pushRoot(path.join(os.homedir(), '.opencode', 'skill')); + pushRoot(path.join(os.homedir(), '.claude', 'skills')); + pushRoot(path.join(os.homedir(), '.agents', 'skills')); + + const customConfigDir = process.env.OPENCODE_CONFIG_DIR + ? path.resolve(process.env.OPENCODE_CONFIG_DIR) + : null; + if (customConfigDir) { + pushRoot(path.join(customConfigDir, 'skills')); + pushRoot(path.join(customConfigDir, 'skill')); + } + + if (workingDirectory) { + const worktreeRoot = findWorktreeRoot(workingDirectory) || path.resolve(workingDirectory); + for (const ancestor of getAncestors(workingDirectory, worktreeRoot)) { + pushRoot(path.join(ancestor, '.opencode', 'skills')); + pushRoot(path.join(ancestor, '.opencode', 'skill')); + pushRoot(path.join(ancestor, '.claude', 'skills')); + pushRoot(path.join(ancestor, '.agents', 'skills')); + } + } + + return roots; +} + +function isManagedSkillPath(skillMdPath, workingDirectory) { + if (!skillMdPath || skillMdPath === BUILT_IN_SKILL_LOCATION) { + return false; + } + const skillDir = path.dirname(path.resolve(skillMdPath)); + return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root)); +} + function renameSkill(oldName, newName, workingDirectory) { ensureDirs(); assertValidSkillName(newName); @@ -620,6 +674,17 @@ function renameSkill(oldName, newName, workingDirectory) { if (path.basename(existing.path) !== 'SKILL.md') { throw new Error(`Skill "${oldName}" target must be a SKILL.md file`); } + if (!isManagedSkillPath(existing.path, workingDirectory)) { + throw new Error(`Skill "${oldName}" is outside managed skill directories and cannot be renamed`); + } + + const mdDataBeforeMove = parseMdFile(existing.path); + const frontmatterName = typeof mdDataBeforeMove.frontmatter?.name === 'string' + ? mdDataBeforeMove.frontmatter.name + : oldName; + if (frontmatterName !== oldName) { + throw new Error(`Skill "${oldName}" does not match ${existing.path}`); + } const conflict = getSkillScope(newName, workingDirectory); if (conflict.path) { diff --git a/packages/web/server/lib/opencode/skills.test.js b/packages/web/server/lib/opencode/skills.test.js index bda9ea97..6fec177e 100644 --- a/packages/web/server/lib/opencode/skills.test.js +++ b/packages/web/server/lib/opencode/skills.test.js @@ -178,4 +178,147 @@ describe('skills', () => { await fsPromises.rm(tempRoot, { recursive: true, force: true }); } }); + + it('rolls back the directory rename when frontmatter write fails', async () => { + const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-rename-rollback-')); + const projectRoot = path.join(tempRoot, 'project'); + const skillDir = path.join(projectRoot, '.opencode', 'skills', 'rollback-skill'); + const skillPath = path.join(skillDir, 'SKILL.md'); + const body = '# Rollback body\n\nMust remain in the original directory.'; + + try { + await fsPromises.mkdir(skillDir, { recursive: true }); + await fsPromises.writeFile( + skillPath, + [ + '---', + 'name: rollback-skill', + 'description: Rollback skill', + '---', + '', + body, + '', + ].join('\n'), + 'utf8', + ); + await fsPromises.chmod(skillPath, 0o444); + + expect(() => renameSkill('rollback-skill', 'rollback-skill-renamed', projectRoot)).toThrow(); + + expect(fs.existsSync(skillDir)).toBe(true); + expect(fs.existsSync(path.join(projectRoot, '.opencode', 'skills', 'rollback-skill-renamed'))).toBe(false); + expect(await fsPromises.readFile(skillPath, 'utf8')).toContain(body); + } finally { + try { + await fsPromises.chmod(skillPath, 0o644); + } catch { + // Best-effort cleanup when the file was rolled back under a different mode. + } + await fsPromises.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it('rejects invalid names, missing skills, conflicts, unmanaged paths, and frontmatter mismatches', async () => { + const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-rename-reject-')); + const projectRoot = path.join(tempRoot, 'project'); + const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-skill'); + const conflictDir = path.join(projectRoot, '.opencode', 'skills', 'taken-name'); + const mismatchDir = path.join(projectRoot, '.opencode', 'skills', 'folder-name'); + const unmanagedDir = path.join(projectRoot, 'custom-skills', 'unmanaged-skill'); + const cacheStamp = `oc-rename-${Date.now()}`; + const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-skill'); + + try { + await fsPromises.mkdir(managedDir, { recursive: true }); + await fsPromises.writeFile( + path.join(managedDir, 'SKILL.md'), + [ + '---', + 'name: managed-skill', + 'description: Managed', + '---', + '', + 'Managed body', + '', + ].join('\n'), + 'utf8', + ); + + await fsPromises.mkdir(conflictDir, { recursive: true }); + await fsPromises.writeFile( + path.join(conflictDir, 'SKILL.md'), + [ + '---', + 'name: taken-name', + 'description: Taken', + '---', + '', + 'Taken body', + '', + ].join('\n'), + 'utf8', + ); + + await fsPromises.mkdir(mismatchDir, { recursive: true }); + await fsPromises.writeFile( + path.join(mismatchDir, 'SKILL.md'), + [ + '---', + 'name: frontmatter-name', + 'description: Mismatch', + '---', + '', + 'Mismatch body', + '', + ].join('\n'), + 'utf8', + ); + + await fsPromises.mkdir(unmanagedDir, { recursive: true }); + await fsPromises.writeFile( + path.join(unmanagedDir, 'SKILL.md'), + [ + '---', + 'name: unmanaged-skill', + 'description: Unmanaged', + '---', + '', + 'Unmanaged body', + '', + ].join('\n'), + 'utf8', + ); + + await fsPromises.mkdir(cacheDir, { recursive: true }); + await fsPromises.writeFile( + path.join(cacheDir, 'SKILL.md'), + [ + '---', + 'name: cache-skill', + 'description: Cache skill', + '---', + '', + 'Cache body', + '', + ].join('\n'), + 'utf8', + ); + + expect(() => renameSkill('managed-skill', 'Invalid_Name', projectRoot)).toThrow(/Invalid skill name/); + expect(() => renameSkill('missing-skill', 'new-skill', projectRoot)).toThrow(/not found/); + expect(() => renameSkill('managed-skill', 'taken-name', projectRoot)).toThrow(/already exists/); + expect(() => renameSkill('folder-name', 'renamed-mismatch', projectRoot)).toThrow(/does not match/); + expect(() => renameSkill('cache-skill', 'cache-skill-renamed', projectRoot)).toThrow(/managed skill directories/); + + expect(fs.existsSync(managedDir)).toBe(true); + expect(fs.existsSync(cacheDir)).toBe(true); + expect(fs.existsSync(path.join(projectRoot, '.opencode', 'skills', 'renamed-mismatch'))).toBe(false); + } finally { + await fsPromises.rm(tempRoot, { recursive: true, force: true }); + await fsPromises.rm(path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp), { + recursive: true, + force: true, + }); + } + }); }); From be38fb8cf4b2139a1cc1ba70f752f279942017c1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 08:54:39 +0000 Subject: [PATCH 03/13] fix(desktop): strip AppImage ARGV0 before child shells (#2588) AppImage exports ARGV0 into the process environment. zsh treats that as argv[0] for every external command, which broke Python venv detection in the integrated terminal and managed OpenCode sessions. Clear ARGV0 in Electron before login-shell probing, refuse to re-apply it from shell snapshots, and strip it from terminal PTY and managed OpenCode launch environments. Co-authored-by: Serhii Dziupin --- packages/electron/README.md | 2 ++ packages/electron/main.mjs | 7 +++- packages/web/server/lib/inherited-env.js | 23 +++++++++++++ packages/web/server/lib/inherited-env.test.js | 29 ++++++++++++++++ .../web/server/lib/opencode/DOCUMENTATION.md | 4 ++- .../web/server/lib/opencode/env-runtime.js | 5 ++- .../server/lib/opencode/env-runtime.test.js | 22 ++++++++++++ packages/web/server/lib/opencode/lifecycle.js | 5 +-- .../web/server/lib/opencode/lifecycle.test.js | 34 +++++++++++++++++++ .../web/server/lib/terminal/DOCUMENTATION.md | 1 + packages/web/server/lib/terminal/runtime.js | 3 ++ .../web/server/lib/terminal/runtime.test.js | 18 ++++++++++ 12 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 packages/web/server/lib/inherited-env.js create mode 100644 packages/web/server/lib/inherited-env.test.js diff --git a/packages/electron/README.md b/packages/electron/README.md index 2e052833..45722457 100644 --- a/packages/electron/README.md +++ b/packages/electron/README.md @@ -85,6 +85,8 @@ After packaging, run `bun run --cwd packages/electron verify:linux-appimage`. Th Running a packaged Linux AppImage requires FUSE (`libfuse.so.2`, typically `libfuse2` / `libfuse2t64` on Debian/Ubuntu). Without FUSE, start with `APPIMAGE_EXTRACT_AND_RUN=1`. Keep the AppImage on a writable path so in-app updates can replace it. +Desktop clears AppImage `ARGV0` from `process.env` before probing the login shell and starting the in-process server. Leaving it set makes zsh rewrite argv[0] for integrated-terminal and managed-OpenCode child commands to the AppImage path. + Linux updates are supported only when the packaged app is running from a writable AppImage. Update checks, downloads, and installation report an actionable error when `APPIMAGE` is missing, invalid, or read-only; a missing release feed (`latest-linux.yml` 404 before the first Linux publish) is treated as “no update available”. macOS and Windows updater behavior is unchanged. Release builds keep `latest-linux.yml` (x64) and `latest-linux-arm64.yml` separate and validate each manifest against its AppImage before upload. Linux AppImages download full updates (no `.blockmap` differential channel yet). ### Updater End-to-End Fixture diff --git a/packages/electron/main.mjs b/packages/electron/main.mjs index d82fbb3b..46722691 100644 --- a/packages/electron/main.mjs +++ b/packages/electron/main.mjs @@ -1374,11 +1374,16 @@ const loadShellEnv = () => { // Merge the user's login-shell env (PATH, etc.) into this process before we import { pathLooksUserConfigured, mergePathValues } from '@openchamber/web/server/lib/opencode/path-utils.js'; +import { stripAppImageArgv0Leak } from '@openchamber/web/server/lib/inherited-env.js'; // import/start the server in-process. The server and its children (opencode // CLI, git, etc.) inherit process.env directly now — there is no sidecar // subprocess to hand a custom env to. const inheritUserShellEnv = () => { + // Clear before probing/merging so login-shell snapshots and children never + // inherit the AppImage path as argv[0] via zsh's ARGV0 parameter (#2588). + stripAppImageArgv0Leak(process.env); + const shellEnv = loadShellEnv(); if (!shellEnv) return; @@ -1388,7 +1393,7 @@ const inheritUserShellEnv = () => { const currentPathLooksUserConfigured = pathLooksUserConfigured(currentPath, homeDir, delimiter); for (const [key, value] of Object.entries(shellEnv)) { - if (key === 'PATH') continue; + if (key === 'PATH' || key === 'ARGV0') continue; if (typeof process.env[key] === 'undefined') { process.env[key] = value; } diff --git a/packages/web/server/lib/inherited-env.js b/packages/web/server/lib/inherited-env.js new file mode 100644 index 00000000..9cae7f27 --- /dev/null +++ b/packages/web/server/lib/inherited-env.js @@ -0,0 +1,23 @@ +/** + * Sanitize environment objects inherited by user-facing child processes. + * + * Linux AppImage runtimes export `ARGV0` as the AppImage path before launching + * the packaged app. zsh treats an exported `ARGV0` as the argv[0] for every + * external command it spawns, which corrupts Python venv detection and any + * other program that reads argv[0]/$0 while leaving `/proc/self/exe` correct. + * + * See openchamber/openchamber#2588 and pingdotgg/t3code#2509. + */ + +/** + * Remove AppImage `ARGV0` from a mutable env object (or `process.env`). + * @param {NodeJS.ProcessEnv | Record | null | undefined} env + * @returns {typeof env} + */ +export function stripAppImageArgv0Leak(env) { + if (!env || typeof env !== 'object') return env; + if (Object.prototype.hasOwnProperty.call(env, 'ARGV0')) { + delete env.ARGV0; + } + return env; +} diff --git a/packages/web/server/lib/inherited-env.test.js b/packages/web/server/lib/inherited-env.test.js new file mode 100644 index 00000000..e7193190 --- /dev/null +++ b/packages/web/server/lib/inherited-env.test.js @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest'; +import { stripAppImageArgv0Leak } from './inherited-env.js'; + +describe('stripAppImageArgv0Leak', () => { + it('removes ARGV0 from a child env object', () => { + const env = { + PATH: '/usr/bin', + ARGV0: '/path/to/OpenChamber-1.17.2-linux-x86_64.AppImage', + SHELL: '/bin/zsh', + }; + + expect(stripAppImageArgv0Leak(env)).toBe(env); + expect(env).toEqual({ + PATH: '/usr/bin', + SHELL: '/bin/zsh', + }); + }); + + it('is a no-op when ARGV0 is absent', () => { + const env = { PATH: '/usr/bin', SHELL: '/bin/bash' }; + stripAppImageArgv0Leak(env); + expect(env).toEqual({ PATH: '/usr/bin', SHELL: '/bin/bash' }); + }); + + it('tolerates nullish env values', () => { + expect(stripAppImageArgv0Leak(null)).toBeNull(); + expect(stripAppImageArgv0Leak(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index c57ecb8b..bb9fcdfc 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -120,7 +120,9 @@ The runtime maintains active-session count incrementally from idempotent activit Managed OpenCode launch also merges the environment returned by the agent-tool runtime. PATH and `OPENCODE_SERVER_PASSWORD` remain lifecycle-owned and cannot be replaced by injected values. External OpenCode processes receive no -OpenChamber tool injection. +OpenChamber tool injection. Managed launch env strips AppImage `ARGV0` before +spawn so zsh-backed OpenCode tools do not rewrite child argv[0] to the AppImage +path (#2588). Set `OPENCHAMBER_STARTUP_PERF=1` to emit bounded startup phase records for server listen, managed OpenCode preparation/readiness, and proxy readiness holds. Every OpenCode bootstrap emits one terminal `opencode.bootstrap.ready` or `opencode.bootstrap.error` event, including reused and external server paths. Records contain controlled phase/outcome/route labels and timing values only; they never contain request URLs, runtime keys, directories, session IDs, credentials, or content. diff --git a/packages/web/server/lib/opencode/env-runtime.js b/packages/web/server/lib/opencode/env-runtime.js index 0bd65b50..5ed52956 100644 --- a/packages/web/server/lib/opencode/env-runtime.js +++ b/packages/web/server/lib/opencode/env-runtime.js @@ -232,7 +232,7 @@ export const createOpenCodeEnvRuntime = (deps) => { return; } - const skipKeys = new Set(['PWD', 'OLDPWD', 'SHLVL', '_']); + const skipKeys = new Set(['PWD', 'OLDPWD', 'SHLVL', '_', 'ARGV0']); for (const [key, value] of Object.entries(snapshot)) { if (skipKeys.has(key)) { continue; @@ -244,6 +244,9 @@ export const createOpenCodeEnvRuntime = (deps) => { process.env[key] = value; } + // AppImage ARGV0 must never remain on process.env (zsh rewrites argv[0]; #2588). + delete process.env.ARGV0; + const currentPath = process.env.PATH || ''; const shellPath = snapshot.PATH || ''; if (!shellPath) { diff --git a/packages/web/server/lib/opencode/env-runtime.test.js b/packages/web/server/lib/opencode/env-runtime.test.js index 52ce6908..96386db3 100644 --- a/packages/web/server/lib/opencode/env-runtime.test.js +++ b/packages/web/server/lib/opencode/env-runtime.test.js @@ -131,6 +131,28 @@ describe('OpenCode env runtime', () => { expect(process.env.PATH).toBe(defaultDir); }); + it('clears AppImage ARGV0 when applying a login-shell env snapshot', () => { + const previousArgv0 = process.env.ARGV0; + process.env.ARGV0 = '/path/to/OpenChamber.AppImage'; + delete process.env.OPENCHAMBER_ARGV0_TEST_MARKER; + const { runtime, state } = createRuntime({}); + state.cachedLoginShellEnvSnapshot = { + PATH: '/usr/bin', + ARGV0: '/leaked/from/shell.AppImage', + OPENCHAMBER_ARGV0_TEST_MARKER: '1', + }; + + try { + runtime.applyLoginShellEnvSnapshot(); + expect(process.env.ARGV0).toBeUndefined(); + expect(process.env.OPENCHAMBER_ARGV0_TEST_MARKER).toBe('1'); + } finally { + delete process.env.OPENCHAMBER_ARGV0_TEST_MARKER; + if (previousArgv0 === undefined) delete process.env.ARGV0; + else process.env.ARGV0 = previousArgv0; + } + }); + it('throws a specific error for a missing configured OpenCode binary in strict mode', async () => { const { runtime } = createRuntime({ opencodeBinary: '/missing/opencode' }); diff --git a/packages/web/server/lib/opencode/lifecycle.js b/packages/web/server/lib/opencode/lifecycle.js index 046a710e..77cb3f4d 100644 --- a/packages/web/server/lib/opencode/lifecycle.js +++ b/packages/web/server/lib/opencode/lifecycle.js @@ -1,5 +1,6 @@ import { spawn, spawnSync } from 'node:child_process'; import net from 'node:net'; +import { stripAppImageArgv0Leak } from '../inherited-env.js'; import { registerManagedProcess, unregisterManagedProcess, reapOrphanedProcesses } from './managed-process-registry.js'; import { recordStartupPerformance } from './startup-performance.js'; @@ -518,13 +519,13 @@ export const createOpenCodeLifecycleRuntime = (deps) => { timeout: 30000, cwd: state.openCodeWorkingDirectory, shellEnvKeysCount: Object.keys(shellEnv).length, - env: { + env: stripAppImageArgv0Leak({ ...shellEnv, ...process.env, ...managedOpenCodeEnv, PATH: envPath, OPENCODE_SERVER_PASSWORD: openCodePassword, - }, + }), }); if (!serverInstance || !serverInstance.url) { diff --git a/packages/web/server/lib/opencode/lifecycle.test.js b/packages/web/server/lib/opencode/lifecycle.test.js index aadb5483..1a1c1b21 100644 --- a/packages/web/server/lib/opencode/lifecycle.test.js +++ b/packages/web/server/lib/opencode/lifecycle.test.js @@ -285,6 +285,40 @@ describe('OpenCode lifecycle', () => { await server.close(); }); + it('strips AppImage ARGV0 from managed OpenCode launch env', async () => { + delete process.env.OPENCODE_BINARY; + const previousArgv0 = process.env.ARGV0; + process.env.ARGV0 = '/path/to/OpenChamber/OpenChamber-1.17.2-linux-x86_64.AppImage'; + const child = createMockChild(); + spawnMock.mockImplementationOnce(() => { + queueMicrotask(() => { + child.stdout.emit('data', 'opencode server listening on http://127.0.0.1:45678\n'); + }); + return child; + }); + + try { + const runtime = createRuntime({ + getManagedOpenCodeShellEnvSnapshot: vi.fn(() => ({ + PATH: '/home/user/.bun/bin:/usr/local/bin:/usr/bin', + ARGV0: '/leaked/from/shell/snapshot.AppImage', + SHELL_ONLY: 'yes', + })), + }); + const server = await runtime.startOpenCode(); + const [, , options] = spawnMock.mock.calls[0]; + + expect(options.env).not.toHaveProperty('ARGV0'); + expect(options.env.SHELL_ONLY).toBe('yes'); + expect(options.env.PATH).toBe('/home/user/.bun/bin:/usr/local/bin:/usr/bin'); + + await server.close(); + } finally { + if (previousArgv0 === undefined) delete process.env.ARGV0; + else process.env.ARGV0 = previousArgv0; + } + }); + it('adds managed OpenChamber tool environment without allowing it to replace launch invariants', async () => { const child = createMockChild(); spawnMock.mockImplementationOnce(() => { diff --git a/packages/web/server/lib/terminal/DOCUMENTATION.md b/packages/web/server/lib/terminal/DOCUMENTATION.md index ffc27411..d662c060 100644 --- a/packages/web/server/lib/terminal/DOCUMENTATION.md +++ b/packages/web/server/lib/terminal/DOCUMENTATION.md @@ -24,6 +24,7 @@ HTTP remains the authenticated command plane for create, resize, appearance upda - Concurrent creates for one ID are single-flight only when working directory and shell preference match. Existing IDs cannot be reused for another working directory. - Dimensions are bounded to 1-1000 columns and 1-500 rows; input is capped at 64 KiB. - PTY children explicitly clear `NODE_CHANNEL_FD`; daemon IPC descriptors are host-private and invalid after PTY descriptor cleanup. +- PTY children also strip AppImage `ARGV0` (and other host-private shell vars such as `ELECTRON_RUN_AS_NODE`, `BASH_ENV`, `ENV`, `BASH_XTRACEFD`). An exported `ARGV0` makes zsh rewrite argv[0] for every external command, which breaks Python venv detection and other argv[0]/$0 consumers while leaving `/proc/self/exe` correct. - `GET /api/terminal/shells` reports shell IDs available on the active server using the same augmented PATH provided to spawned PTYs, plus whether each executable has a supported login-mode argument. `auto` preserves environment/platform fallback order; an explicit unavailable shell fails creation instead of silently running a different shell. Login mode is opt-in and uses only built-in arguments for known shells. Preference changes affect new sessions and explicit restarts, not running PTYs. - PTY data and exit callbacks enter one FIFO queue. Stale callbacks from replaced processes are ignored. - Scrollback is retained on the server and capped at 512 KiB with UTF-8-safe trimming. Device-status, device-attribute, cursor-position reply, and color-query exchanges are removed from replay history with incomplete control sequences carried across PTY chunks; live output remains byte-for-byte unchanged. diff --git a/packages/web/server/lib/terminal/runtime.js b/packages/web/server/lib/terminal/runtime.js index f717cb7a..3d036ff0 100644 --- a/packages/web/server/lib/terminal/runtime.js +++ b/packages/web/server/lib/terminal/runtime.js @@ -10,6 +10,7 @@ import { import { sanitizeTerminalHistoryChunk } from './history.js'; import { consumeTerminalThemeQueries, terminalThemeModeReport } from './theme-response.js'; import { createTerminalShellResolver, getTerminalShellLoginArgs, normalizeTerminalShell } from './shells.js'; +import { stripAppImageArgv0Leak } from '../inherited-env.js'; const MAX_SESSIONS = 20; const MAX_HISTORY_BYTES = 512 * 1024; @@ -66,6 +67,8 @@ export function createTerminalRuntime({ // required because bun-pty also inherits Bun's native process environment. env.NODE_CHANNEL_FD = ''; delete env.BASH_XTRACEFD; delete env.BASH_ENV; delete env.ENV; delete env.ELECTRON_RUN_AS_NODE; + // AppImage exports ARGV0; zsh would otherwise rewrite argv[0] for every command (#2588). + stripAppImageArgv0Leak(env); const options = { name: 'xterm-256color', cwd, cols, rows, env, ...(process.platform === 'win32' ? { useConpty: true } : {}) }; return { process: provider.spawn(executable, args, options), backend: provider.backend, shell: resolvedShell.id, loginShell }; } catch (error) { lastError = error; } diff --git a/packages/web/server/lib/terminal/runtime.test.js b/packages/web/server/lib/terminal/runtime.test.js index 4bc21420..4a39225f 100644 --- a/packages/web/server/lib/terminal/runtime.test.js +++ b/packages/web/server/lib/terminal/runtime.test.js @@ -154,6 +154,8 @@ describe('terminal runtime', () => { expect(harness.processes[0].options.cwd).toBe('/repo'); expect(harness.processes[0].options.env.COLORFGBG).toBe('0;15'); expect(harness.processes[0].options.env.NODE_CHANNEL_FD).toBe(''); + expect(harness.processes[0].options.env).not.toHaveProperty('ARGV0'); + expect(harness.processes[0].options.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE'); harness.processes[0].emitData('\u001b[?2031h\u001b]10;?\u0007\u001b]11;?\u0007'); expect(harness.processes[0].writes).toEqual(['\u001b]10;rgb:1b1b/1b1b/1b1b\u001b\\', '\u001b]11;rgb:fafa/f8f8/f0f0\u001b\\']); @@ -173,6 +175,22 @@ describe('terminal runtime', () => { } finally { await harness.runtime.shutdown(); } }); + it('strips AppImage ARGV0 from PTY child environments', async () => { + const previousArgv0 = process.env.ARGV0; + process.env.ARGV0 = '/path/to/OpenChamber/OpenChamber-1.17.2-linux-x86_64.AppImage'; + const harness = createHarness(); + try { + const response = createResponse(); + await harness.routes.post.get('/api/terminal/create')({ body: { sessionId: 'term-argv0', cwd: '/repo', cols: 80, rows: 24 } }, response); + expect(response.statusCode).toBe(200); + expect(harness.processes[0].options.env).not.toHaveProperty('ARGV0'); + } finally { + if (previousArgv0 === undefined) delete process.env.ARGV0; + else process.env.ARGV0 = previousArgv0; + await harness.runtime.shutdown(); + } + }); + it('lists available shells and uses the selected shell for create and restart', async () => { const executables = new Set(['/bin/zsh', '/bin/bash', '/bin/sh']); const harness = createHarness({ From 5defd1af75ca218d9a28006884d0701fe6d9c703 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 09:12:35 +0000 Subject: [PATCH 04/13] fix(terminal): drop native ARGV0 for bun-pty via env -u bun-pty merges the OS environ into PTY children, so deleting ARGV0 from the JS env object alone left the AppImage path in the shell. Wrap Linux PTY spawns with env -u ARGV0, clear native ARGV0 under Bun via libc unsetenv, and always clear process.env even when no login-shell snapshot exists. Co-authored-by: Serhii Dziupin --- packages/electron/main.mjs | 4 +- packages/web/server/lib/inherited-env.js | 51 +++++++++++++++++++ packages/web/server/lib/inherited-env.test.js | 38 +++++++++++++- .../web/server/lib/opencode/env-runtime.js | 8 +-- .../server/lib/opencode/env-runtime.test.js | 15 ++++++ .../web/server/lib/terminal/DOCUMENTATION.md | 2 +- packages/web/server/lib/terminal/runtime.js | 6 ++- .../web/server/lib/terminal/runtime.test.js | 27 ++++++++-- 8 files changed, 138 insertions(+), 13 deletions(-) diff --git a/packages/electron/main.mjs b/packages/electron/main.mjs index 46722691..ca7a493e 100644 --- a/packages/electron/main.mjs +++ b/packages/electron/main.mjs @@ -1374,7 +1374,7 @@ const loadShellEnv = () => { // Merge the user's login-shell env (PATH, etc.) into this process before we import { pathLooksUserConfigured, mergePathValues } from '@openchamber/web/server/lib/opencode/path-utils.js'; -import { stripAppImageArgv0Leak } from '@openchamber/web/server/lib/inherited-env.js'; +import { clearAppImageArgv0FromProcessEnv } from '@openchamber/web/server/lib/inherited-env.js'; // import/start the server in-process. The server and its children (opencode // CLI, git, etc.) inherit process.env directly now — there is no sidecar @@ -1382,7 +1382,7 @@ import { stripAppImageArgv0Leak } from '@openchamber/web/server/lib/inherited-en const inheritUserShellEnv = () => { // Clear before probing/merging so login-shell snapshots and children never // inherit the AppImage path as argv[0] via zsh's ARGV0 parameter (#2588). - stripAppImageArgv0Leak(process.env); + clearAppImageArgv0FromProcessEnv(); const shellEnv = loadShellEnv(); if (!shellEnv) return; diff --git a/packages/web/server/lib/inherited-env.js b/packages/web/server/lib/inherited-env.js index 9cae7f27..a75de0ff 100644 --- a/packages/web/server/lib/inherited-env.js +++ b/packages/web/server/lib/inherited-env.js @@ -9,6 +9,11 @@ * See openchamber/openchamber#2588 and pingdotgg/t3code#2509. */ +import { createRequire } from 'node:module'; +import { existsSync } from 'node:fs'; + +const LINUX_ENV_BINARIES = ['/usr/bin/env', '/bin/env']; + /** * Remove AppImage `ARGV0` from a mutable env object (or `process.env`). * @param {NodeJS.ProcessEnv | Record | null | undefined} env @@ -21,3 +26,49 @@ export function stripAppImageArgv0Leak(env) { } return env; } + +/** + * Clear AppImage `ARGV0` from this process. + * + * Bun keeps a native environ that `bun-pty` inherits even after + * `delete process.env.ARGV0`. On Linux under Bun we also call libc `unsetenv`. + */ +export function clearAppImageArgv0FromProcessEnv() { + delete process.env.ARGV0; + if (process.platform !== 'linux' || typeof Bun === 'undefined') return; + try { + const require = createRequire(import.meta.url); + const { dlopen } = require('bun:ffi'); + const libc = dlopen('libc.so.6', { + unsetenv: { args: ['cstring'], returns: 'i32' }, + }); + libc.symbols.unsetenv(Buffer.from('ARGV0\0')); + } catch { + // Node/Electron and environments without bun:ffi rely on explicit child envs. + } +} + +/** + * Resolve a Linux PTY launch that drops native `ARGV0` before the shell starts. + * + * `bun-pty` merges the OS environ into the child, so deleting `ARGV0` from the + * JS env object alone is not enough. Wrapping with `env -u ARGV0` unsets it + * before execing the real shell. No-op on non-Linux platforms. + * + * @param {string} executable + * @param {string[]} args + * @returns {{ executable: string, args: string[] }} + */ +export function resolveLinuxPtyLaunch(executable, args = []) { + if (process.platform !== 'linux') { + return { executable, args }; + } + const envBinary = LINUX_ENV_BINARIES.find((candidate) => existsSync(candidate)); + if (!envBinary) { + return { executable, args }; + } + return { + executable: envBinary, + args: ['-u', 'ARGV0', executable, ...args], + }; +} diff --git a/packages/web/server/lib/inherited-env.test.js b/packages/web/server/lib/inherited-env.test.js index e7193190..8348d449 100644 --- a/packages/web/server/lib/inherited-env.test.js +++ b/packages/web/server/lib/inherited-env.test.js @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest'; -import { stripAppImageArgv0Leak } from './inherited-env.js'; +import { + clearAppImageArgv0FromProcessEnv, + resolveLinuxPtyLaunch, + stripAppImageArgv0Leak, +} from './inherited-env.js'; describe('stripAppImageArgv0Leak', () => { it('removes ARGV0 from a child env object', () => { @@ -27,3 +31,35 @@ describe('stripAppImageArgv0Leak', () => { expect(stripAppImageArgv0Leak(undefined)).toBeUndefined(); }); }); + +describe('clearAppImageArgv0FromProcessEnv', () => { + it('removes ARGV0 from process.env', () => { + const previous = process.env.ARGV0; + process.env.ARGV0 = '/path/to/OpenChamber.AppImage'; + try { + clearAppImageArgv0FromProcessEnv(); + expect(process.env.ARGV0).toBeUndefined(); + } finally { + if (previous === undefined) delete process.env.ARGV0; + else process.env.ARGV0 = previous; + } + }); +}); + +describe('resolveLinuxPtyLaunch', () => { + it('wraps the shell with env -u ARGV0 on Linux', () => { + if (process.platform !== 'linux') return; + expect(resolveLinuxPtyLaunch('/bin/zsh', ['-l'])).toEqual({ + executable: expect.stringMatching(/\/env$/), + args: ['-u', 'ARGV0', '/bin/zsh', '-l'], + }); + }); + + it('leaves non-Linux launches unchanged', () => { + if (process.platform === 'linux') return; + expect(resolveLinuxPtyLaunch('/bin/zsh', ['-l'])).toEqual({ + executable: '/bin/zsh', + args: ['-l'], + }); + }); +}); diff --git a/packages/web/server/lib/opencode/env-runtime.js b/packages/web/server/lib/opencode/env-runtime.js index 5ed52956..23950d63 100644 --- a/packages/web/server/lib/opencode/env-runtime.js +++ b/packages/web/server/lib/opencode/env-runtime.js @@ -2,6 +2,7 @@ import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { clearAppImageArgv0FromProcessEnv } from '../inherited-env.js'; import { mergePathValues } from './path-utils.js'; export const createOpenCodeEnvRuntime = (deps) => { @@ -227,6 +228,10 @@ export const createOpenCodeEnvRuntime = (deps) => { }; const applyLoginShellEnvSnapshot = () => { + // Always clear AppImage ARGV0, even when no login-shell snapshot is available. + // Otherwise a leaked process.env.ARGV0 survives into later child spawns (#2588). + clearAppImageArgv0FromProcessEnv(); + const snapshot = getLoginShellEnvSnapshot(); if (!snapshot) { return; @@ -244,9 +249,6 @@ export const createOpenCodeEnvRuntime = (deps) => { process.env[key] = value; } - // AppImage ARGV0 must never remain on process.env (zsh rewrites argv[0]; #2588). - delete process.env.ARGV0; - const currentPath = process.env.PATH || ''; const shellPath = snapshot.PATH || ''; if (!shellPath) { diff --git a/packages/web/server/lib/opencode/env-runtime.test.js b/packages/web/server/lib/opencode/env-runtime.test.js index 96386db3..7f51f40f 100644 --- a/packages/web/server/lib/opencode/env-runtime.test.js +++ b/packages/web/server/lib/opencode/env-runtime.test.js @@ -153,6 +153,21 @@ describe('OpenCode env runtime', () => { } }); + it('clears AppImage ARGV0 even when no login-shell snapshot is available', () => { + const previousArgv0 = process.env.ARGV0; + process.env.ARGV0 = '/path/to/OpenChamber.AppImage'; + const { runtime, state } = createRuntime({}); + state.cachedLoginShellEnvSnapshot = null; + + try { + runtime.applyLoginShellEnvSnapshot(); + expect(process.env.ARGV0).toBeUndefined(); + } finally { + if (previousArgv0 === undefined) delete process.env.ARGV0; + else process.env.ARGV0 = previousArgv0; + } + }); + it('throws a specific error for a missing configured OpenCode binary in strict mode', async () => { const { runtime } = createRuntime({ opencodeBinary: '/missing/opencode' }); diff --git a/packages/web/server/lib/terminal/DOCUMENTATION.md b/packages/web/server/lib/terminal/DOCUMENTATION.md index d662c060..40196998 100644 --- a/packages/web/server/lib/terminal/DOCUMENTATION.md +++ b/packages/web/server/lib/terminal/DOCUMENTATION.md @@ -24,7 +24,7 @@ HTTP remains the authenticated command plane for create, resize, appearance upda - Concurrent creates for one ID are single-flight only when working directory and shell preference match. Existing IDs cannot be reused for another working directory. - Dimensions are bounded to 1-1000 columns and 1-500 rows; input is capped at 64 KiB. - PTY children explicitly clear `NODE_CHANNEL_FD`; daemon IPC descriptors are host-private and invalid after PTY descriptor cleanup. -- PTY children also strip AppImage `ARGV0` (and other host-private shell vars such as `ELECTRON_RUN_AS_NODE`, `BASH_ENV`, `ENV`, `BASH_XTRACEFD`). An exported `ARGV0` makes zsh rewrite argv[0] for every external command, which breaks Python venv detection and other argv[0]/$0 consumers while leaving `/proc/self/exe` correct. +- PTY children also strip AppImage `ARGV0` (and other host-private shell vars such as `ELECTRON_RUN_AS_NODE`, `BASH_ENV`, `ENV`, `BASH_XTRACEFD`). An exported `ARGV0` makes zsh rewrite argv[0] for every external command, which breaks Python venv detection and other argv[0]/$0 consumers while leaving `/proc/self/exe` correct. On Linux, PTY spawn is wrapped with `env -u ARGV0` because `bun-pty` merges the native OS environ and would otherwise reintroduce `ARGV0` after a JS-only delete. - `GET /api/terminal/shells` reports shell IDs available on the active server using the same augmented PATH provided to spawned PTYs, plus whether each executable has a supported login-mode argument. `auto` preserves environment/platform fallback order; an explicit unavailable shell fails creation instead of silently running a different shell. Login mode is opt-in and uses only built-in arguments for known shells. Preference changes affect new sessions and explicit restarts, not running PTYs. - PTY data and exit callbacks enter one FIFO queue. Stale callbacks from replaced processes are ignored. - Scrollback is retained on the server and capped at 512 KiB with UTF-8-safe trimming. Device-status, device-attribute, cursor-position reply, and color-query exchanges are removed from replay history with incomplete control sequences carried across PTY chunks; live output remains byte-for-byte unchanged. diff --git a/packages/web/server/lib/terminal/runtime.js b/packages/web/server/lib/terminal/runtime.js index 3d036ff0..9aa328d4 100644 --- a/packages/web/server/lib/terminal/runtime.js +++ b/packages/web/server/lib/terminal/runtime.js @@ -10,7 +10,7 @@ import { import { sanitizeTerminalHistoryChunk } from './history.js'; import { consumeTerminalThemeQueries, terminalThemeModeReport } from './theme-response.js'; import { createTerminalShellResolver, getTerminalShellLoginArgs, normalizeTerminalShell } from './shells.js'; -import { stripAppImageArgv0Leak } from '../inherited-env.js'; +import { stripAppImageArgv0Leak, resolveLinuxPtyLaunch } from '../inherited-env.js'; const MAX_SESSIONS = 20; const MAX_HISTORY_BYTES = 512 * 1024; @@ -68,9 +68,11 @@ export function createTerminalRuntime({ env.NODE_CHANNEL_FD = ''; delete env.BASH_XTRACEFD; delete env.BASH_ENV; delete env.ENV; delete env.ELECTRON_RUN_AS_NODE; // AppImage exports ARGV0; zsh would otherwise rewrite argv[0] for every command (#2588). + // bun-pty also merges the native OS environ, so wrap with `env -u ARGV0` on Linux. stripAppImageArgv0Leak(env); + const launch = resolveLinuxPtyLaunch(executable, args); const options = { name: 'xterm-256color', cwd, cols, rows, env, ...(process.platform === 'win32' ? { useConpty: true } : {}) }; - return { process: provider.spawn(executable, args, options), backend: provider.backend, shell: resolvedShell.id, loginShell }; + return { process: provider.spawn(launch.executable, launch.args, options), backend: provider.backend, shell: resolvedShell.id, loginShell }; } catch (error) { lastError = error; } } throw lastError ?? new Error('No executable shell found'); diff --git a/packages/web/server/lib/terminal/runtime.test.js b/packages/web/server/lib/terminal/runtime.test.js index 4a39225f..f0e2b622 100644 --- a/packages/web/server/lib/terminal/runtime.test.js +++ b/packages/web/server/lib/terminal/runtime.test.js @@ -156,6 +156,10 @@ describe('terminal runtime', () => { expect(harness.processes[0].options.env.NODE_CHANNEL_FD).toBe(''); expect(harness.processes[0].options.env).not.toHaveProperty('ARGV0'); expect(harness.processes[0].options.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE'); + if (process.platform === 'linux') { + expect(harness.processes[0].shell).toMatch(/\/env$/); + expect(harness.processes[0].args.slice(0, 3)).toEqual(['-u', 'ARGV0', expect.any(String)]); + } harness.processes[0].emitData('\u001b[?2031h\u001b]10;?\u0007\u001b]11;?\u0007'); expect(harness.processes[0].writes).toEqual(['\u001b]10;rgb:1b1b/1b1b/1b1b\u001b\\', '\u001b]11;rgb:fafa/f8f8/f0f0\u001b\\']); @@ -184,6 +188,11 @@ describe('terminal runtime', () => { await harness.routes.post.get('/api/terminal/create')({ body: { sessionId: 'term-argv0', cwd: '/repo', cols: 80, rows: 24 } }, response); expect(response.statusCode).toBe(200); expect(harness.processes[0].options.env).not.toHaveProperty('ARGV0'); + if (process.platform === 'linux') { + expect(harness.processes[0].shell).toMatch(/\/env$/); + expect(harness.processes[0].args[0]).toBe('-u'); + expect(harness.processes[0].args[1]).toBe('ARGV0'); + } } finally { if (previousArgv0 === undefined) delete process.env.ARGV0; else process.env.ARGV0 = previousArgv0; @@ -216,14 +225,24 @@ describe('terminal runtime', () => { const created = createResponse(); await harness.routes.post.get('/api/terminal/create')({ body: { sessionId: 'term-shell', cwd: '/repo', shell: 'zsh', loginShell: true } }, created); expect(created.statusCode).toBe(200); - expect(harness.processes[0].shell).toBe('/bin/zsh'); - expect(harness.processes[0].args).toEqual(['-l']); + if (process.platform === 'linux') { + expect(harness.processes[0].shell).toMatch(/\/env$/); + expect(harness.processes[0].args).toEqual(['-u', 'ARGV0', '/bin/zsh', '-l']); + } else { + expect(harness.processes[0].shell).toBe('/bin/zsh'); + expect(harness.processes[0].args).toEqual(['-l']); + } const restarted = createResponse(); await harness.routes.post.get('/api/terminal/:sessionId/restart')({ params: { sessionId: 'term-shell' }, body: { shell: 'bash', loginShell: true } }, restarted); expect(restarted.statusCode).toBe(200); - expect(harness.processes[1].shell).toBe('/bin/bash'); - expect(harness.processes[1].args).toEqual(['-l']); + if (process.platform === 'linux') { + expect(harness.processes[1].shell).toMatch(/\/env$/); + expect(harness.processes[1].args).toEqual(['-u', 'ARGV0', '/bin/bash', '-l']); + } else { + expect(harness.processes[1].shell).toBe('/bin/bash'); + expect(harness.processes[1].args).toEqual(['-l']); + } } finally { await harness.runtime.shutdown(); } }); From 20fc675af0e0fb445933a908be531adaa823fb74 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 09:25:25 +0000 Subject: [PATCH 05/13] fix(skills): drive UI rename gating from server renamable flag Expose authoritative renamable on skill list responses using the same managed-root policy as renameSkill, drop the divergent UI path heuristic, and remove an unused rejection-test fixture. Co-authored-by: Serhii Dziupin --- .../sections/skills/SkillsSidebar.tsx | 3 +-- .../sections/skills/skillLocations.ts | 19 ------------------- packages/ui/src/stores/useSkillsStore.ts | 4 ++++ packages/vscode/src/bridge-config-runtime.ts | 17 ++++++++++++++++- packages/vscode/src/opencodeConfig.ts | 2 ++ .../web/server/lib/opencode/DOCUMENTATION.md | 1 + .../lib/opencode/feature-routes-runtime.js | 3 ++- .../web/server/lib/opencode/skill-routes.js | 9 ++++++++- packages/web/server/lib/opencode/skills.js | 1 + .../web/server/lib/opencode/skills.test.js | 16 ---------------- 10 files changed, 35 insertions(+), 40 deletions(-) diff --git a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx index 98646486..f440b9a5 100644 --- a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx +++ b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx @@ -27,7 +27,6 @@ import { SidebarGroup } from '@/components/sections/shared/SidebarGroup'; import { Icon } from "@/components/icon/Icon"; import { useI18n } from '@/lib/i18n'; import { SETTINGS_PANEL_TITLE_CLASS } from '@/components/sections/shared/SettingsSection'; -import { isManagedSkillFilesystemPath } from '@/components/sections/skills/skillLocations'; interface SkillsSidebarProps { onItemSelect?: () => void; @@ -37,7 +36,7 @@ const BUILT_IN_SKILL_LOCATION = ''; const isBuiltInSkill = (skill: DiscoveredSkill | null | undefined): boolean => skill?.path === BUILT_IN_SKILL_LOCATION; const isRenamableSkill = (skill: DiscoveredSkill | null | undefined): boolean => ( - !!skill && !isBuiltInSkill(skill) && isManagedSkillFilesystemPath(skill.path) + !!skill && !isBuiltInSkill(skill) && skill.renamable === true ); export const SkillsSidebar: React.FC = ({ onItemSelect }) => { diff --git a/packages/ui/src/components/sections/skills/skillLocations.ts b/packages/ui/src/components/sections/skills/skillLocations.ts index cc07fd5e..a0009a20 100644 --- a/packages/ui/src/components/sections/skills/skillLocations.ts +++ b/packages/ui/src/components/sections/skills/skillLocations.ts @@ -57,22 +57,3 @@ export function locationPartsFrom(value: SkillLocationValue): { scope: SkillScop } return { scope: match.scope, source: match.source }; } - -/** True when a discovered skill path is under a managed skill root that rename/delete may mutate. */ -export function isManagedSkillFilesystemPath(skillPath: string | null | undefined): boolean { - if (!skillPath || skillPath === '') return false; - const normalized = skillPath.replace(/\\/g, '/'); - if ( - normalized.includes('/.cache/opencode/skills/') - || normalized.includes('/Caches/opencode/skills/') - || normalized.includes('/Library/Caches/opencode/skills/') - ) { - return false; - } - return ( - /\/\.opencode\/skills?\//.test(normalized) - || /\/\.claude\/skills\//.test(normalized) - || /\/\.agents\/skills\//.test(normalized) - || /\/\.config\/opencode\/skills?\//.test(normalized) - ); -} diff --git a/packages/ui/src/stores/useSkillsStore.ts b/packages/ui/src/stores/useSkillsStore.ts index 9acbde46..76f7b9c4 100644 --- a/packages/ui/src/stores/useSkillsStore.ts +++ b/packages/ui/src/stores/useSkillsStore.ts @@ -70,6 +70,8 @@ export interface DiscoveredSkill { description?: string; /** Domain folder parsed from file path, e.g. "automation-ai", "lark-ecosystem" */ group?: string; + /** Authoritative server flag: skill lives under a managed root and can be renamed in place. */ + renamable?: boolean; } /** Parse the domain group folder from a skill file path. @@ -93,6 +95,7 @@ interface RawSkillResponse { path: string; scope?: SkillScope; source?: SkillSource; + renamable?: boolean; sources?: { md?: { description?: string; @@ -237,6 +240,7 @@ export const useSkillsStore = create()( source: s.source ?? 'opencode', description: s.sources?.md?.description || '', group: parseSkillGroup(s.path), + renamable: s.renamable === true, })); set({ skills: configSkills, isLoading: false }); diff --git a/packages/vscode/src/bridge-config-runtime.ts b/packages/vscode/src/bridge-config-runtime.ts index 569900e5..0ec36bee 100644 --- a/packages/vscode/src/bridge-config-runtime.ts +++ b/packages/vscode/src/bridge-config-runtime.ts @@ -26,6 +26,7 @@ import { updateSkill, deleteSkill, renameSkill, + isManagedSkillPath, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, @@ -653,7 +654,21 @@ export async function handleConfigBridgeMessage( if (!name && normalizedMethod === 'GET') { const skills = await resolveDiscoveredSkills(deps, ctx, workingDirectory); - return { id, type, success: true, data: { skills } }; + return { + id, + type, + success: true, + data: { + skills: skills.map((skill) => ({ + ...skill, + renamable: Boolean( + skill.path + && skill.path !== '' + && isManagedSkillPath(skill.path, workingDirectory) + ), + })), + }, + }; } const skillName = typeof name === 'string' ? name.trim() : ''; diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index 1ecce46e..0bf94c32 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -2885,6 +2885,8 @@ const isManagedSkillPath = (skillMdPath: string, workingDirectory?: string): boo return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root)); }; +export { isManagedSkillPath }; + export const renameSkill = (oldName: string, newName: string, workingDirectory?: string): void => { ensureSkillDirs(); validateSkillName(newName); diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index 1f21f955..9e4a6f3b 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -352,6 +352,7 @@ an authoritative loopback callback URL even when OpenChamber binds port `0`. - `registerSkillRoutes(app, dependencies)`: registers skills-related routes: - Skills config CRUD and metadata under `/api/config/skills*` - Skill rename via `PATCH /api/config/skills/:name` with `{ renameTo }` (directory rename preserves `SKILL.md` body and supporting files; restricted to managed skill roots under `.opencode/skills|skill`, `.claude/skills`, and `.agents/skills`) + - Skill list responses include authoritative `renamable` derived from the same managed-root policy used by rename - Skills catalog listing/source pagination, scan, and install routes - Supporting skill file read/write/delete routes diff --git a/packages/web/server/lib/opencode/feature-routes-runtime.js b/packages/web/server/lib/opencode/feature-routes-runtime.js index dd3726d7..69efc12b 100644 --- a/packages/web/server/lib/opencode/feature-routes-runtime.js +++ b/packages/web/server/lib/opencode/feature-routes-runtime.js @@ -38,7 +38,7 @@ import { decodePluginId, } from './plugins.js'; import { SKILL_DIR, SKILL_SCOPE, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile } from './shared.js'; -import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill, renameSkill } from './skills.js'; +import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill, renameSkill, isManagedSkillPath } from './skills.js'; import { getCuratedSkillsSources } from '../skills-catalog/curated-sources.js'; import { getCacheKey, getCachedScan, setCachedScan } from '../skills-catalog/cache.js'; import { isClawdHubSource, parseSkillRepoSource } from '../skills-catalog/source.js'; @@ -257,6 +257,7 @@ export const createFeatureRoutesRuntime = (dependencies) => { updateSkill, deleteSkill, renameSkill, + isManagedSkillPath, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, diff --git a/packages/web/server/lib/opencode/skill-routes.js b/packages/web/server/lib/opencode/skill-routes.js index 314e6b99..db6893ce 100644 --- a/packages/web/server/lib/opencode/skill-routes.js +++ b/packages/web/server/lib/opencode/skill-routes.js @@ -22,6 +22,7 @@ export const registerSkillRoutes = (app, dependencies) => { updateSkill, deleteSkill, renameSkill, + isManagedSkillPath, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, @@ -213,9 +214,15 @@ export const registerSkillRoutes = (app, dependencies) => { const enrichedSkills = skills.map((skill) => { const sources = getSkillSources(skill.name, directory, skill); + const skillPath = typeof skill.path === 'string' ? skill.path : null; return { ...skill, - sources + sources, + renamable: Boolean( + skillPath + && skillPath !== '' + && isManagedSkillPath(skillPath, directory) + ), }; }); diff --git a/packages/web/server/lib/opencode/skills.js b/packages/web/server/lib/opencode/skills.js index a0594ad3..9ee24d6b 100644 --- a/packages/web/server/lib/opencode/skills.js +++ b/packages/web/server/lib/opencode/skills.js @@ -734,4 +734,5 @@ export { updateSkill, deleteSkill, renameSkill, + isManagedSkillPath, }; diff --git a/packages/web/server/lib/opencode/skills.test.js b/packages/web/server/lib/opencode/skills.test.js index 6fec177e..d19bf3d3 100644 --- a/packages/web/server/lib/opencode/skills.test.js +++ b/packages/web/server/lib/opencode/skills.test.js @@ -224,7 +224,6 @@ describe('skills', () => { const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-skill'); const conflictDir = path.join(projectRoot, '.opencode', 'skills', 'taken-name'); const mismatchDir = path.join(projectRoot, '.opencode', 'skills', 'folder-name'); - const unmanagedDir = path.join(projectRoot, 'custom-skills', 'unmanaged-skill'); const cacheStamp = `oc-rename-${Date.now()}`; const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-skill'); @@ -274,21 +273,6 @@ describe('skills', () => { 'utf8', ); - await fsPromises.mkdir(unmanagedDir, { recursive: true }); - await fsPromises.writeFile( - path.join(unmanagedDir, 'SKILL.md'), - [ - '---', - 'name: unmanaged-skill', - 'description: Unmanaged', - '---', - '', - 'Unmanaged body', - '', - ].join('\n'), - 'utf8', - ); - await fsPromises.mkdir(cacheDir, { recursive: true }); await fsPromises.writeFile( path.join(cacheDir, 'SKILL.md'), From 0d24d0a167cdd9b10be98a79240be4f54054c796 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 11:05:27 +0000 Subject: [PATCH 06/13] fix(skills): repair renameSkill directory resolution after merge Use getRequestDirectory and x-opencode-directory like the other skill mutations, and pin renamable list/store mapping with focused tests. Co-authored-by: Serhii Dziupin --- packages/ui/src/stores/useSkillsStore.test.ts | 85 +++++++++++++++++++ packages/ui/src/stores/useSkillsStore.ts | 11 ++- .../server/lib/opencode/skill-routes.test.js | 62 ++++++++++++++ 3 files changed, 154 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/stores/useSkillsStore.test.ts b/packages/ui/src/stores/useSkillsStore.test.ts index f75fb711..3c569d98 100644 --- a/packages/ui/src/stores/useSkillsStore.test.ts +++ b/packages/ui/src/stores/useSkillsStore.test.ts @@ -98,9 +98,94 @@ describe('useSkillsStore directory resolution', () => { source: 'agents', description: 'Repository local', group: undefined, + renamable: false, }]); }); + test('loadSkills maps authoritative renamable from the list response', async () => { + runtimeFetchImpl = async () => new Response(JSON.stringify({ + skills: [ + { + name: 'managed-skill', + path: `${activeProjectPath}/.opencode/skills/managed-skill/SKILL.md`, + scope: 'project', + source: 'opencode', + renamable: true, + sources: { md: { description: 'Managed' } }, + }, + { + name: 'cache-skill', + path: '/home/ubuntu/.cache/opencode/skills/hash/cache-skill/SKILL.md', + scope: 'user', + source: 'opencode', + renamable: false, + sources: { md: { description: 'Cache' } }, + }, + ], + }), { + headers: { 'Content-Type': 'application/json' }, + }); + + expect(await useSkillsStore.getState().loadSkills()).toBe(true); + expect(useSkillsStore.getState().skills).toEqual([ + { + name: 'managed-skill', + path: `${activeProjectPath}/.opencode/skills/managed-skill/SKILL.md`, + scope: 'project', + source: 'opencode', + description: 'Managed', + group: undefined, + renamable: true, + }, + { + name: 'cache-skill', + path: '/home/ubuntu/.cache/opencode/skills/hash/cache-skill/SKILL.md', + scope: 'user', + source: 'opencode', + description: 'Cache', + group: 'hash', + renamable: false, + }, + ]); + }); + + test('renameSkill uses getRequestDirectory query and x-opencode-directory header', async () => { + runtimeFetchImpl = async (_url, init) => { + if (init?.method === 'PATCH') { + return new Response(JSON.stringify({ + success: true, + requiresReload: false, + }), { + headers: { 'Content-Type': 'application/json' }, + }); + } + + return new Response(JSON.stringify({ + skills: [{ + name: 'new-skill', + path: `${activeProjectPath}/.opencode/skills/new-skill/SKILL.md`, + scope: 'project', + source: 'opencode', + renamable: true, + sources: { md: { description: 'Renamed' } }, + }], + }), { + headers: { 'Content-Type': 'application/json' }, + }); + }; + + const renamed = await useSkillsStore.getState().renameSkill('old-skill', 'new-skill'); + expect(renamed).toBe(true); + + const renameCall = runtimeFetchCalls.find((call) => String(call.url).includes('/api/config/skills/old-skill')); + expect(renameCall).toBeTruthy(); + expect(renameCall?.url).toContain(`directory=${encodeURIComponent(activeProjectPath)}`); + + const headers = new Headers(renameCall?.headers); + expect(headers.get('content-type')).toBe('application/json'); + expect(headers.get('x-opencode-directory')).toBe(activeProjectPath); + }); + test('invalidateSkillsLoadCache() with no argument clears the active-project cache key used by loadSkills', async () => { expect(await useSkillsStore.getState().loadSkills()).toBe(true); expect(runtimeFetchCalls.length).toBe(1); diff --git a/packages/ui/src/stores/useSkillsStore.ts b/packages/ui/src/stores/useSkillsStore.ts index 2614a8df..2157db21 100644 --- a/packages/ui/src/stores/useSkillsStore.ts +++ b/packages/ui/src/stores/useSkillsStore.ts @@ -408,12 +408,15 @@ export const useSkillsStore = create()( startConfigUpdate("Renaming skill..."); let requiresReload = false; try { - const currentDirectory = getCurrentDirectory(); - const queryParams = currentDirectory ? `?directory=${encodeURIComponent(currentDirectory)}` : ''; + const directory = getRequestDirectory(); + const queryParams = directory ? `?directory=${encodeURIComponent(directory)}` : ''; const response = await runtimeFetch(`/api/config/skills/${encodeURIComponent(name)}${queryParams}`, { method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, + headers: { + 'Content-Type': 'application/json', + ...(directory ? { 'x-opencode-directory': directory } : {}), + }, body: JSON.stringify({ renameTo: newName }), }); @@ -424,7 +427,7 @@ export const useSkillsStore = create()( } const needsReload = payload?.requiresReload ?? false; - invalidateSkillsLoadCache(currentDirectory); + invalidateSkillsLoadCache(directory); if (needsReload) { requiresReload = true; await refreshSkillsAfterOpenCodeRestart({ diff --git a/packages/web/server/lib/opencode/skill-routes.test.js b/packages/web/server/lib/opencode/skill-routes.test.js index c83f7f84..3ba8526e 100644 --- a/packages/web/server/lib/opencode/skill-routes.test.js +++ b/packages/web/server/lib/opencode/skill-routes.test.js @@ -9,7 +9,9 @@ import { deleteSkill, discoverSkills, getSkillSources, + isManagedSkillPath, mergeDiscoveredSkills, + renameSkill, updateSkill, } from './skills.js'; import { @@ -58,6 +60,8 @@ const startSkillsApp = ({ projectRoot }) => { createSkill, updateSkill, deleteSkill, + renameSkill, + isManagedSkillPath, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile, @@ -154,4 +158,62 @@ describe('skill-routes directory soft fallback', () => { const payload = await listResponse.json(); expect(payload.skills.map((skill) => skill.name)).toContain('manual-repo-skill'); }); + + it('marks managed-root skills renamable and cache skills not renamable', async () => { + projectRoot = createTempProject(); + const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-list-skill'); + fs.mkdirSync(managedDir, { recursive: true }); + fs.writeFileSync( + path.join(managedDir, 'SKILL.md'), + [ + '---', + 'name: managed-list-skill', + 'description: Managed list skill', + '---', + '', + 'Managed body', + '', + ].join('\n'), + 'utf8', + ); + + const cacheStamp = `oc-skill-routes-${Date.now()}`; + const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-list-skill'); + fs.mkdirSync(cacheDir, { recursive: true }); + fs.writeFileSync( + path.join(cacheDir, 'SKILL.md'), + [ + '---', + 'name: cache-list-skill', + 'description: Cache list skill', + '---', + '', + 'Cache body', + '', + ].join('\n'), + 'utf8', + ); + + try { + appHandle = startSkillsApp({ projectRoot }); + const listResponse = await fetch( + `${appHandle.baseUrl}/api/config/skills?directory=${encodeURIComponent(projectRoot)}`, + ); + expect(listResponse.status).toBe(200); + const payload = await listResponse.json(); + + const managed = payload.skills.find((entry) => entry.name === 'managed-list-skill'); + const cached = payload.skills.find((entry) => entry.name === 'cache-list-skill'); + + expect(managed).toBeTruthy(); + expect(managed.renamable).toBe(true); + expect(cached).toBeTruthy(); + expect(cached.renamable).toBe(false); + } finally { + fs.rmSync(path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp), { + recursive: true, + force: true, + }); + } + }); }); From 2dd3bbfe8e4dc507a7f8fc4887b7ab8c1225d093 Mon Sep 17 00:00:00 2001 From: Howon Lee Date: Mon, 3 Aug 2026 20:46:32 +0900 Subject: [PATCH 07/13] feat: add DeepSeek quota provider --- packages/ui/src/lib/quota/providers/index.ts | 1 + packages/ui/src/types/quota.ts | 1 + packages/vscode/src/quotaProviders.test.ts | 70 +++++++++ packages/vscode/src/quotaProviders.ts | 112 +++++++++++++ .../web/server/lib/quota/DOCUMENTATION.md | 1 + packages/web/server/lib/quota/index.js | 1 + .../server/lib/quota/providers/deepseek.js | 116 ++++++++++++++ .../lib/quota/providers/deepseek.test.js | 147 ++++++++++++++++++ .../web/server/lib/quota/providers/index.js | 8 + 9 files changed, 457 insertions(+) create mode 100644 packages/web/server/lib/quota/providers/deepseek.js create mode 100644 packages/web/server/lib/quota/providers/deepseek.test.js diff --git a/packages/ui/src/lib/quota/providers/index.ts b/packages/ui/src/lib/quota/providers/index.ts index 7320ba06..5d1798c3 100644 --- a/packages/ui/src/lib/quota/providers/index.ts +++ b/packages/ui/src/lib/quota/providers/index.ts @@ -22,5 +22,6 @@ export const QUOTA_PROVIDERS: QuotaProviderMeta[] = [ { id: 'wafer', name: 'Wafer.ai' }, { id: 'opencode-go', name: 'OpenCode Go' }, { id: 'crof', name: 'CrofAI' }, + { id: 'deepseek', name: 'DeepSeek' }, { id: 'neuralwatt', name: 'NeuralWatt' }, ]; diff --git a/packages/ui/src/types/quota.ts b/packages/ui/src/types/quota.ts index 6e5e11a4..365f588d 100644 --- a/packages/ui/src/types/quota.ts +++ b/packages/ui/src/types/quota.ts @@ -17,6 +17,7 @@ export type QuotaProviderId = | 'wafer' | 'opencode-go' | 'crof' + | 'deepseek' | 'neuralwatt'; export interface UsageWindow { diff --git a/packages/vscode/src/quotaProviders.test.ts b/packages/vscode/src/quotaProviders.test.ts index 9f7b466b..7d00df14 100644 --- a/packages/vscode/src/quotaProviders.test.ts +++ b/packages/vscode/src/quotaProviders.test.ts @@ -11,6 +11,7 @@ const AUTH = JSON.stringify({ crof: { key: 'test-token' }, neuralwatt: { key: 'test-token' }, 'zai-coding-plan': { key: 'test-token' }, + deepseek: { key: 'test-token' }, }); ((fs as unknown) as { existsSync: () => boolean }).existsSync = () => true; ((fs as unknown) as { readFileSync: () => string }).readFileSync = () => AUTH; @@ -419,3 +420,72 @@ describe('NeuralWatt quota provider (VS Code parity)', () => { fsMock.readFileSync = ORIGINAL_FS.readFileSync; }); }); + +describe('DeepSeek quota provider (VS Code parity)', () => { + test('builds credits_balance window from documented USD payload (string balance)', async () => { + stubFetchReturning(() => Promise.resolve(mockResponse({ + is_available: true, + balance_infos: [ + { currency: 'USD', total_balance: '7.54', granted_balance: '0.00', topped_up_balance: '7.54' }, + ], + }))); + + const result = await fetchQuotaForProvider('deepseek'); + + assert.equal(result.ok, true); + assert.equal(result.providerId, 'deepseek'); + assert.equal(result.usage!.windows.credits_balance!.valueLabel, '$7.54'); + assert.equal(result.usage!.windows.credits_balance!.usedPercent, null); + assert.equal(result.usage!.windows.credits_balance!.windowSeconds, null); + assert.equal(result.usage!.windows.credits_balance!.resetAt, null); + }); + + test('falls back to CNY entry with ¥ symbol when no USD entry is present', async () => { + stubFetchReturning(() => Promise.resolve(mockResponse({ + is_available: true, + balance_infos: [ + { currency: 'CNY', total_balance: '100.00', granted_balance: '0.00', topped_up_balance: '100.00' }, + ], + }))); + + const result = await fetchQuotaForProvider('deepseek'); + + assert.equal(result.ok, true); + assert.equal(result.usage!.windows.credits_balance!.valueLabel, '¥100.00'); + }); + + test('maps 401 to session-expired', async () => { + stubFetchFailing(async () => ({}), { ok: false, status: 401 }); + + const result = await fetchQuotaForProvider('deepseek'); + + assert.equal(result.ok, false); + assert.equal(result.error, 'Session expired — please re-authenticate with DeepSeek'); + }); + + test('returns no-quota-data on a 200 payload with no usable balance', async () => { + stubFetchReturning(() => Promise.resolve(mockResponse({ + is_available: true, + balance_infos: [{ currency: 'USD', total_balance: '', granted_balance: '0.00', topped_up_balance: '0.00' }], + }))); + + const result = await fetchQuotaForProvider('deepseek'); + + assert.equal(result.ok, false); + assert.equal(result.configured, true); + assert.equal(result.error, 'No quota data in response'); + assert.equal(result.usage, null); + }); + + test('keeps a literal zero balance as a valid valueLabel', async () => { + stubFetchReturning(() => Promise.resolve(mockResponse({ + is_available: true, + balance_infos: [{ currency: 'USD', total_balance: '0.00', granted_balance: '0.00', topped_up_balance: '0.00' }], + }))); + + const result = await fetchQuotaForProvider('deepseek'); + + assert.equal(result.ok, true); + assert.equal(result.usage!.windows.credits_balance!.valueLabel, '$0.00'); + }); +}); diff --git a/packages/vscode/src/quotaProviders.ts b/packages/vscode/src/quotaProviders.ts index 807d8ace..b910e73b 100644 --- a/packages/vscode/src/quotaProviders.ts +++ b/packages/vscode/src/quotaProviders.ts @@ -124,6 +124,16 @@ type CrofPayload = { credits?: number | string; }; +type DeepseekPayload = { + is_available?: boolean; + balance_infos?: Array<{ + currency?: string; + total_balance?: number | string; + granted_balance?: number | string; + topped_up_balance?: number | string; + }>; +}; + type NeuralwattPayload = { balance?: { credits_remaining_usd?: number | string; @@ -492,6 +502,11 @@ export const listConfiguredQuotaProviders = () => { configured.add('neuralwatt'); } + const deepseekAuth = normalizeAuthEntry(getAuthEntry(auth, ['deepseek'])); + if (deepseekAuth && ((deepseekAuth as Record).key || (deepseekAuth as Record).token)) { + configured.add('deepseek'); + } + return Array.from(configured); }; @@ -2175,6 +2190,101 @@ const fetchCrofQuota = async (): Promise => { } }; +const DEEPSEEK_QUOTA_URL = 'https://api.deepseek.com/user/balance'; + +const fetchDeepseekQuota = async (): Promise => { + const auth = readAuthFile(); + const entry = normalizeAuthEntry(getAuthEntry(auth, ['deepseek'])) as Record | null; + const apiKey = (entry?.key as string | undefined) ?? (entry?.token as string | undefined); + + if (!apiKey) { + return buildResult({ + providerId: 'deepseek', + providerName: 'DeepSeek', + ok: false, + configured: false, + error: 'Not configured', + }); + } + + const timeoutSignal = AbortSignal.timeout(15_000); + + try { + const response = await fetch(DEEPSEEK_QUOTA_URL, { + method: 'GET', + headers: { + Authorization: `Bearer ${apiKey}`, + 'Accept-Encoding': 'identity', + }, + signal: timeoutSignal, + }); + + if (!response.ok) { + return buildResult({ + providerId: 'deepseek', + providerName: 'DeepSeek', + ok: false, + configured: true, + error: response.status === 401 || response.status === 403 + ? 'Session expired — please re-authenticate with DeepSeek' + : `API error: ${response.status}`, + }); + } + + const payload = await response.json() as DeepseekPayload; + const balanceInfos = Array.isArray(payload?.balance_infos) ? payload.balance_infos : []; + const balanceInfo = balanceInfos.find((info) => info?.currency === 'USD') + ?? balanceInfos.find((info) => info?.currency === 'CNY') + ?? null; + const rawBalance = balanceInfo?.total_balance; + const totalBalance = (typeof rawBalance === 'number' || (typeof rawBalance === 'string' && rawBalance.trim() !== '')) + ? toNumber(rawBalance) + : null; + + if (totalBalance === null) { + return buildResult({ + providerId: 'deepseek', + providerName: 'DeepSeek', + ok: false, + configured: true, + error: 'No quota data in response', + }); + } + + const symbol = balanceInfo?.currency === 'CNY' ? '¥' : '$'; + const windows: Record = { + credits_balance: toUsageWindow({ + usedPercent: null, + windowSeconds: null, + resetAt: null, + valueLabel: `${symbol}${formatMoney(totalBalance)}`, + }), + }; + + return buildResult({ + providerId: 'deepseek', + providerName: 'DeepSeek', + ok: true, + configured: true, + usage: { windows }, + }); + } catch (error) { + const isTimeout = error instanceof DOMException && error.name === 'AbortError' && timeoutSignal.aborted; + const isParseError = error instanceof SyntaxError; + return buildResult({ + providerId: 'deepseek', + providerName: 'DeepSeek', + ok: false, + configured: true, + error: isTimeout + ? 'Request timed out' + : isParseError + ? 'Invalid response from provider' + : (error instanceof Error ? error.message : 'Request failed'), + }); + } +}; + export const fetchQuotaForProvider = async (providerId: string): Promise => { switch (providerId) { case 'claude': @@ -2218,6 +2328,8 @@ export const fetchQuotaForProvider = async (providerId: string): Promise { + const auth = readAuthFile(); + const entry = normalizeAuthEntry(getAuthEntry(auth, aliases)); + return Boolean(entry?.key || entry?.token); +}; + +export const fetchQuota = async () => { + const auth = readAuthFile(); + const entry = normalizeAuthEntry(getAuthEntry(auth, aliases)); + const apiKey = entry?.key ?? entry?.token; + + if (!apiKey) { + return buildResult({ + providerId, + providerName, + ok: false, + configured: false, + error: 'Not configured' + }); + } + + const timeoutSignal = AbortSignal.timeout(15_000); + + try { + const response = await fetch(DEEPSEEK_QUOTA_URL, { + method: 'GET', + headers: { + Authorization: `Bearer ${apiKey}`, + 'Accept-Encoding': 'identity' + }, + signal: timeoutSignal + }); + + if (!response.ok) { + return buildResult({ + providerId, + providerName, + ok: false, + configured: true, + error: response.status === 401 || response.status === 403 + ? 'Session expired — please re-authenticate with DeepSeek' + : `API error: ${response.status}` + }); + } + + const payload = await response.json(); + const balanceInfos = Array.isArray(payload?.balance_infos) ? payload.balance_infos : []; + const balanceInfo = balanceInfos.find((info) => info?.currency === 'USD') + ?? balanceInfos.find((info) => info?.currency === 'CNY') + ?? null; + const rawBalance = balanceInfo?.total_balance; + const totalBalance = (typeof rawBalance === 'number' || (typeof rawBalance === 'string' && rawBalance.trim() !== '')) + ? toNumber(rawBalance) + : null; + + if (totalBalance === null) { + return buildResult({ + providerId, + providerName, + ok: false, + configured: true, + error: 'No quota data in response' + }); + } + + const isCny = balanceInfo?.currency === 'CNY'; + const symbol = isCny ? '¥' : '$'; + const valueLabel = `${symbol}${formatMoney(totalBalance)}`; + + const windows = { + credits_balance: toUsageWindow({ + usedPercent: null, + windowSeconds: null, + resetAt: null, + valueLabel + }) + }; + + return buildResult({ + providerId, + providerName, + ok: true, + configured: true, + usage: { windows } + }); + } catch (error) { + const isTimeout = error instanceof DOMException && error.name === 'AbortError' && timeoutSignal.aborted; + const isParseError = error instanceof SyntaxError; + return buildResult({ + providerId, + providerName, + ok: false, + configured: true, + error: isTimeout + ? 'Request timed out' + : isParseError + ? 'Invalid response from provider' + : (error instanceof Error ? error.message : 'Request failed') + }); + } +}; diff --git a/packages/web/server/lib/quota/providers/deepseek.test.js b/packages/web/server/lib/quota/providers/deepseek.test.js new file mode 100644 index 00000000..d92728c5 --- /dev/null +++ b/packages/web/server/lib/quota/providers/deepseek.test.js @@ -0,0 +1,147 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../opencode/auth.js', () => ({ + readAuthFile: () => ({ deepseek: { key: 'test-token' } }), +})); + +import { fetchQuota } from './deepseek.js'; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +const mockResponse = (body, init = {}) => ({ + ok: true, + status: 200, + json: async () => body, + ...init, +}); + +// Documented payload shape from https://api.deepseek.com/user/balance +const DOCUMENTED_PAYLOAD = { + is_available: true, + balance_infos: [ + { + currency: 'USD', + total_balance: '7.54', + granted_balance: '0.00', + topped_up_balance: '7.54' + } + ] +}; + +describe('DeepSeek quota provider', () => { + it('builds credits_balance window from documented USD payload (string balance)', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse(DOCUMENTED_PAYLOAD))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(true); + expect(result.providerId).toBe('deepseek'); + + const window = result.usage.windows.credits_balance; + expect(window).toBeDefined(); + expect(window.valueLabel).toBe('$7.54'); + expect(window.usedPercent).toBeNull(); + expect(window.windowSeconds).toBeNull(); + expect(window.resetAt).toBeNull(); + }); + + it('falls back to CNY entry when no USD entry is present', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse({ + is_available: true, + balance_infos: [ + { currency: 'CNY', total_balance: '100.00', granted_balance: '0.00', topped_up_balance: '100.00' } + ] + }))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(true); + expect(result.usage.windows.credits_balance.valueLabel).toBe('¥100.00'); + }); + + it('prefers the USD entry when both USD and CNY are present', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse({ + is_available: true, + balance_infos: [ + { currency: 'CNY', total_balance: '100.00', granted_balance: '0.00', topped_up_balance: '100.00' }, + { currency: 'USD', total_balance: '3.55', granted_balance: '0.00', topped_up_balance: '3.55' } + ] + }))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(true); + expect(result.usage.windows.credits_balance.valueLabel).toBe('$3.55'); + }); + + it('tolerates a numeric total_balance', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse({ + is_available: true, + balance_infos: [{ currency: 'USD', total_balance: 12.5, granted_balance: 0, topped_up_balance: 12.5 }] + }))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(true); + expect(result.usage.windows.credits_balance.valueLabel).toBe('$12.50'); + }); + + it('maps 401 to session-expired error', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 401, json: async () => ({}) })); + + const result = await fetchQuota(); + + expect(result.ok).toBe(false); + expect(result.error).toBe('Session expired — please re-authenticate with DeepSeek'); + }); + + it('maps 403 to session-expired error', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 403, json: async () => ({}) })); + + const result = await fetchQuota(); + + expect(result.ok).toBe(false); + expect(result.error).toBe('Session expired — please re-authenticate with DeepSeek'); + }); + + it('reports invalid-response on JSON parse failure', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => { throw new SyntaxError('Unexpected token'); }, + })); + + const result = await fetchQuota(); + + expect(result.ok).toBe(false); + expect(result.error).toBe('Invalid response from provider'); + }); + + it('returns no-quota-data on a 200 payload with no usable balance', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse({ + is_available: true, + balance_infos: [{ currency: 'USD', total_balance: '', granted_balance: '0.00', topped_up_balance: '0.00' }] + }))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(false); + expect(result.configured).toBe(true); + expect(result.error).toBe('No quota data in response'); + expect(result.usage).toBeNull(); + }); + + it('keeps a literal zero balance as a valid valueLabel', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse({ + is_available: true, + balance_infos: [{ currency: 'USD', total_balance: '0.00', granted_balance: '0.00', topped_up_balance: '0.00' }] + }))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(true); + expect(result.usage.windows.credits_balance.valueLabel).toBe('$0.00'); + }); +}); diff --git a/packages/web/server/lib/quota/providers/index.js b/packages/web/server/lib/quota/providers/index.js index 17bb6a9c..3d37e889 100644 --- a/packages/web/server/lib/quota/providers/index.js +++ b/packages/web/server/lib/quota/providers/index.js @@ -12,6 +12,7 @@ import * as codex from './codex.js'; import * as copilot from './copilot.js'; import * as crof from './crof.js'; import * as cursor from './cursor.js'; +import * as deepseek from './deepseek.js'; import * as google from './google/index.js'; import * as kimi from './kimi.js'; import * as nanogpt from './nanogpt.js'; @@ -51,6 +52,12 @@ const registry = { isConfigured: cursor.isConfigured, fetchQuota: cursor.fetchQuota }, + deepseek: { + providerId: deepseek.providerId, + providerName: deepseek.providerName, + isConfigured: deepseek.isConfigured, + fetchQuota: deepseek.fetchQuota + }, google: { providerId: google.providerId, providerName: google.providerName, @@ -184,6 +191,7 @@ export const fetchOpenaiQuota = openai.fetchQuota; export const fetchGoogleQuota = google.fetchGoogleQuota; export const fetchCodexQuota = codex.fetchQuota; export const fetchCursorQuota = cursor.fetchQuota; +export const fetchDeepseekQuota = deepseek.fetchQuota; export const fetchCopilotQuota = copilot.fetchQuota; export const fetchCopilotAddonQuota = copilot.fetchQuotaAddon; export const fetchKimiQuota = kimi.fetchQuota; From 635a70b24fb475e4dacaf1c5ba82a6a2a82a6bb9 Mon Sep 17 00:00:00 2001 From: Serhii Dziupin Date: Mon, 3 Aug 2026 16:39:14 +0300 Subject: [PATCH 08/13] fix: compute Kimi quota usage from used or remaining field --- packages/vscode/src/quotaProviders.ts | 28 ++++- .../web/server/lib/quota/DOCUMENTATION.md | 8 ++ .../web/server/lib/quota/providers/kimi.js | 24 +++- .../server/lib/quota/providers/kimi.test.js | 108 ++++++++++++++++++ 4 files changed, 156 insertions(+), 12 deletions(-) create mode 100644 packages/web/server/lib/quota/providers/kimi.test.js diff --git a/packages/vscode/src/quotaProviders.ts b/packages/vscode/src/quotaProviders.ts index 807d8ace..d3de2e8d 100644 --- a/packages/vscode/src/quotaProviders.ts +++ b/packages/vscode/src/quotaProviders.ts @@ -1137,6 +1137,24 @@ const fetchCopilotAddonQuota = async (): Promise => { } }; +// Kimi's weekly `usage` block reports `used`; its rate-limit `limits[].detail` +// blocks report `remaining` instead. Neither field is guaranteed present, so +// derive usedPercent from whichever one the API actually returned. +const computeKimiUsedPercent = ( + total: number | null, + used: number | null, + remaining: number | null, +): number | null => { + if (!total) return null; + if (used !== null) { + return Math.max(0, Math.min(100, (used / total) * 100)); + } + if (remaining !== null) { + return Math.max(0, Math.min(100, 100 - (remaining / total) * 100)); + } + return null; +}; + const fetchKimiQuota = async (): Promise => { const auth = readAuthFile(); const entry = normalizeAuthEntry(getAuthEntry(auth, ['kimi-for-coding', 'kimi'])) as Record | null; @@ -1176,10 +1194,9 @@ const fetchKimiQuota = async (): Promise => { const usage = payload.usage as Record | undefined; if (usage) { const limit = toNumber(usage.limit); + const used = toNumber(usage.used); const remaining = toNumber(usage.remaining); - const usedPercent = limit && remaining !== null - ? Math.max(0, Math.min(100, 100 - (remaining / limit) * 100)) - : null; + const usedPercent = computeKimiUsedPercent(limit, used, remaining); windows.weekly = toUsageWindow({ usedPercent, windowSeconds: null, @@ -1195,10 +1212,9 @@ const fetchKimiQuota = async (): Promise => { const windowSeconds = durationToSeconds(window?.duration as number | undefined, window?.timeUnit as string | undefined); const label = windowSeconds === 5 * 60 * 60 ? `Rate Limit (${rawLabel})` : rawLabel; const total = toNumber(detail?.limit); + const used = toNumber(detail?.used); const remaining = toNumber(detail?.remaining); - const usedPercent = total && remaining !== null - ? Math.max(0, Math.min(100, 100 - (remaining / total) * 100)) - : null; + const usedPercent = computeKimiUsedPercent(total, used, remaining); windows[label] = toUsageWindow({ usedPercent, windowSeconds, diff --git a/packages/web/server/lib/quota/DOCUMENTATION.md b/packages/web/server/lib/quota/DOCUMENTATION.md index 475ff2cf..40643b06 100644 --- a/packages/web/server/lib/quota/DOCUMENTATION.md +++ b/packages/web/server/lib/quota/DOCUMENTATION.md @@ -70,6 +70,14 @@ In 2025/2026 MiniMax rebranded "Coding Plan" to "Token Plan" alongside the M3 mo - **model_remains array**: Now contains entries for multiple model categories (chat, speech, video, image). The provider selects the chat-model entry by matching `MiniMax-M*`, then `general`/`chat`/`text` by name, then any entry with a remaining percent. - **Window status**: The `current_interval_status` and `current_weekly_status` fields indicate whether a window is active. Status `3` means the window is not applicable for the current plan tier (e.g. legacy plans without weekly limits). The provider omits inactive windows. +## Kimi for Coding field semantics + +`GET https://api.kimi.com/coding/v1/usages` is inconsistent about which field carries consumption: +- The weekly `usage` block returns `used` (consumed) with no `remaining` field. +- Each `limits[].detail` rate-limit block returns `remaining` (available) with no `used` field. + +The provider computes `usedPercent` from whichever of `used`/`remaining` is present (`used` takes precedence when both exist) rather than assuming one field name. Both `packages/web/server/lib/quota/providers/kimi.js` and `packages/vscode/src/quotaProviders.ts` (`fetchKimiQuota`) must stay in sync — the VS Code extension duplicates this parsing logic rather than importing it. + ## Notes for contributors - Keep provider IDs stable; clients use them directly. - Avoid adding alias-based dispatch in `fetchQuotaForProvider`; dispatch currently expects exact provider IDs. diff --git a/packages/web/server/lib/quota/providers/kimi.js b/packages/web/server/lib/quota/providers/kimi.js index a9d6c893..2ebd34a6 100644 --- a/packages/web/server/lib/quota/providers/kimi.js +++ b/packages/web/server/lib/quota/providers/kimi.js @@ -14,6 +14,20 @@ export const providerId = 'kimi-for-coding'; export const providerName = 'Kimi for Coding'; const aliases = ['kimi-for-coding', 'kimi']; +// Kimi's weekly `usage` block reports `used`; its rate-limit `limits[].detail` +// blocks report `remaining` instead. Neither field is guaranteed present, so +// derive usedPercent from whichever one the API actually returned. +const computeUsedPercent = (total, used, remaining) => { + if (!total) return null; + if (used !== null) { + return Math.max(0, Math.min(100, (used / total) * 100)); + } + if (remaining !== null) { + return Math.max(0, Math.min(100, 100 - (remaining / total) * 100)); + } + return null; +}; + export const isConfigured = () => { const auth = readAuthFile(); const entry = normalizeAuthEntry(getAuthEntry(auth, aliases)); @@ -59,10 +73,9 @@ export const fetchQuota = async () => { const usage = payload?.usage ?? null; if (usage) { const limit = toNumber(usage.limit); + const used = toNumber(usage.used); const remaining = toNumber(usage.remaining); - const usedPercent = limit && remaining !== null - ? Math.max(0, Math.min(100, 100 - (remaining / limit) * 100)) - : null; + const usedPercent = computeUsedPercent(limit, used, remaining); windows.weekly = toUsageWindow({ usedPercent, windowSeconds: null, @@ -78,10 +91,9 @@ export const fetchQuota = async () => { const windowSeconds = durationToSeconds(window?.duration, window?.timeUnit); const label = windowSeconds === 5 * 60 * 60 ? `Rate Limit (${rawLabel})` : rawLabel; const total = toNumber(detail?.limit); + const used = toNumber(detail?.used); const remaining = toNumber(detail?.remaining); - const usedPercent = total && remaining !== null - ? Math.max(0, Math.min(100, 100 - (remaining / total) * 100)) - : null; + const usedPercent = computeUsedPercent(total, used, remaining); windows[label] = toUsageWindow({ usedPercent, windowSeconds, diff --git a/packages/web/server/lib/quota/providers/kimi.test.js b/packages/web/server/lib/quota/providers/kimi.test.js new file mode 100644 index 00000000..c2eb3b00 --- /dev/null +++ b/packages/web/server/lib/quota/providers/kimi.test.js @@ -0,0 +1,108 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../opencode/auth.js', () => ({ + readAuthFile: () => ({ 'kimi-for-coding': { key: 'test-token' } }), +})); + +import { fetchQuota } from './kimi.js'; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +const mockResponse = (body, init = {}) => ({ + ok: true, + status: 200, + json: async () => body, + ...init, +}); + +describe('Kimi for Coding quota provider', () => { + it('computes weekly usedPercent from the used field (live API shape, no remaining field)', async () => { + // Captured from GET https://api.kimi.com/coding/v1/usages — the weekly + // `usage` block only ever includes `used`, never `remaining`. + vi.stubGlobal('fetch', vi.fn().mockResolvedValue( + mockResponse({ + usage: { limit: '100', used: '100', resetTime: '2026-08-04T06:21:48.514003Z' }, + limits: [{ + window: { duration: 300, timeUnit: 'TIME_UNIT_MINUTE' }, + detail: { limit: '100', remaining: '100', resetTime: '2026-08-03T07:21:48.514003Z' }, + }], + }), + )); + + const result = await fetchQuota(); + + expect(result.ok).toBe(true); + expect(result.usage.windows.weekly.usedPercent).toBe(100); + expect(result.usage.windows['Rate Limit (300m)'].usedPercent).toBe(0); + }); + + it('falls back to computing usedPercent from remaining when used is absent', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue( + mockResponse({ + usage: { limit: '2048', remaining: '512', resetTime: '2026-08-04T06:21:48.514003Z' }, + limits: [], + }), + )); + + const result = await fetchQuota(); + + expect(result.usage.windows.weekly.usedPercent).toBe(75); + }); + + it('prefers used over remaining when both fields are present', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue( + mockResponse({ + usage: { limit: '100', used: '30', remaining: '999', resetTime: null }, + limits: [], + }), + )); + + const result = await fetchQuota(); + + expect(result.usage.windows.weekly.usedPercent).toBe(30); + }); + + it('reports null usedPercent when neither used nor remaining is present', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue( + mockResponse({ + usage: { limit: '100', resetTime: null }, + limits: [], + }), + )); + + const result = await fetchQuota(); + + expect(result.usage.windows.weekly.usedPercent).toBeNull(); + }); + + it('reports not configured when no credentials are stored', async () => { + vi.doMock('../../opencode/auth.js', () => ({ readAuthFile: () => ({}) })); + vi.resetModules(); + const { fetchQuota: fetchQuotaFresh } = await import('./kimi.js'); + + const result = await fetchQuotaFresh(); + + expect(result.ok).toBe(false); + expect(result.configured).toBe(false); + expect(result.error).toBe('Not configured'); + + vi.doUnmock('../../opencode/auth.js'); + vi.resetModules(); + }); + + it('surfaces API errors with status', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, + status: 401, + json: async () => ({}), + })); + + const result = await fetchQuota(); + + expect(result.ok).toBe(false); + expect(result.configured).toBe(true); + expect(result.error).toBe('API error: 401'); + }); +}); From e4fddabb19303e51b70c17a898b606b6035d6f59 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 3 Aug 2026 18:42:31 +0300 Subject: [PATCH 09/13] fix(quota): normalize DeepSeek timeout errors --- packages/vscode/src/quotaProviders.test.ts | 21 +++++++++++++++++++ packages/vscode/src/quotaProviders.ts | 4 +++- .../server/lib/quota/providers/deepseek.js | 4 +++- .../lib/quota/providers/deepseek.test.js | 9 ++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/vscode/src/quotaProviders.test.ts b/packages/vscode/src/quotaProviders.test.ts index 7d00df14..94451eac 100644 --- a/packages/vscode/src/quotaProviders.test.ts +++ b/packages/vscode/src/quotaProviders.test.ts @@ -422,6 +422,12 @@ describe('NeuralWatt quota provider (VS Code parity)', () => { }); describe('DeepSeek quota provider (VS Code parity)', () => { + beforeEach(() => { + const fsMock = fs as unknown as { existsSync: () => boolean; readFileSync: () => string }; + fsMock.existsSync = () => true; + fsMock.readFileSync = () => AUTH; + }); + test('builds credits_balance window from documented USD payload (string balance)', async () => { stubFetchReturning(() => Promise.resolve(mockResponse({ is_available: true, @@ -463,6 +469,15 @@ describe('DeepSeek quota provider (VS Code parity)', () => { assert.equal(result.error, 'Session expired — please re-authenticate with DeepSeek'); }); + test('reports a normalized timeout error', async () => { + stubFetchReturning(() => Promise.reject(new DOMException('The operation timed out.', 'TimeoutError'))); + + const result = await fetchQuotaForProvider('deepseek'); + + assert.equal(result.ok, false); + assert.equal(result.error, 'Request timed out'); + }); + test('returns no-quota-data on a 200 payload with no usable balance', async () => { stubFetchReturning(() => Promise.resolve(mockResponse({ is_available: true, @@ -488,4 +503,10 @@ describe('DeepSeek quota provider (VS Code parity)', () => { assert.equal(result.ok, true); assert.equal(result.usage!.windows.credits_balance!.valueLabel, '$0.00'); }); + + test('teardown: restore fs', () => { + const fsMock = fs as unknown as { existsSync: unknown; readFileSync: unknown }; + fsMock.existsSync = ORIGINAL_FS.existsSync; + fsMock.readFileSync = ORIGINAL_FS.readFileSync; + }); }); diff --git a/packages/vscode/src/quotaProviders.ts b/packages/vscode/src/quotaProviders.ts index b275f593..f759e351 100644 --- a/packages/vscode/src/quotaProviders.ts +++ b/packages/vscode/src/quotaProviders.ts @@ -2285,7 +2285,9 @@ const fetchDeepseekQuota = async (): Promise => { usage: { windows }, }); } catch (error) { - const isTimeout = error instanceof DOMException && error.name === 'AbortError' && timeoutSignal.aborted; + const isTimeout = error instanceof DOMException && ( + error.name === 'TimeoutError' || (error.name === 'AbortError' && timeoutSignal.aborted) + ); const isParseError = error instanceof SyntaxError; return buildResult({ providerId: 'deepseek', diff --git a/packages/web/server/lib/quota/providers/deepseek.js b/packages/web/server/lib/quota/providers/deepseek.js index 3067783b..8963ca82 100644 --- a/packages/web/server/lib/quota/providers/deepseek.js +++ b/packages/web/server/lib/quota/providers/deepseek.js @@ -99,7 +99,9 @@ export const fetchQuota = async () => { usage: { windows } }); } catch (error) { - const isTimeout = error instanceof DOMException && error.name === 'AbortError' && timeoutSignal.aborted; + const isTimeout = error instanceof DOMException && ( + error.name === 'TimeoutError' || (error.name === 'AbortError' && timeoutSignal.aborted) + ); const isParseError = error instanceof SyntaxError; return buildResult({ providerId, diff --git a/packages/web/server/lib/quota/providers/deepseek.test.js b/packages/web/server/lib/quota/providers/deepseek.test.js index d92728c5..a133bb6d 100644 --- a/packages/web/server/lib/quota/providers/deepseek.test.js +++ b/packages/web/server/lib/quota/providers/deepseek.test.js @@ -119,6 +119,15 @@ describe('DeepSeek quota provider', () => { expect(result.error).toBe('Invalid response from provider'); }); + it('reports a normalized timeout error', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new DOMException('The operation timed out.', 'TimeoutError'))); + + const result = await fetchQuota(); + + expect(result.ok).toBe(false); + expect(result.error).toBe('Request timed out'); + }); + it('returns no-quota-data on a 200 payload with no usable balance', async () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockResponse({ is_available: true, From fe38f7a56bbf7113f383a2a97bc042e1426cb183 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 3 Aug 2026 23:09:43 +0300 Subject: [PATCH 10/13] fix: treat lost relay sends as ambiguous instead of failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prompt whose response is lost after the request left the client may already be running server-side. The relay tunnel reported those failures as plain text errors ("stream aborted by host", "relay keepalive timeout"), which matched none of the patterns in isAmbiguousSendFailure, so an accepted prompt was rolled back and the message queue re-sent it — two independent AI responses for one user message (#2425). Direct connections never hit the path. Transports now tag dispatched-but-unconfirmed failures and the classifier reads the tag before falling back to status/text heuristics. Confirmation waits for the connection to actually return (bounded) and retries with backoff instead of two attempts 150ms apart over the just-broken tunnel. --- packages/ui/src/lib/opencode/client.ts | 9 +++- packages/ui/src/lib/relay/transport-error.ts | 42 +++++++++++++++++ .../ui/src/lib/relay/tunnel-client.test.ts | 19 ++++++++ packages/ui/src/lib/relay/tunnel-client.ts | 22 +++++++-- packages/ui/src/sync/DOCUMENTATION.md | 1 + packages/ui/src/sync/session-actions.test.ts | 47 +++++++++++++++++++ packages/ui/src/sync/session-actions.ts | 30 ++++++++++-- 7 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 packages/ui/src/lib/relay/transport-error.ts diff --git a/packages/ui/src/lib/opencode/client.ts b/packages/ui/src/lib/opencode/client.ts index 3e8ab753..7b618f4a 100644 --- a/packages/ui/src/lib/opencode/client.ts +++ b/packages/ui/src/lib/opencode/client.ts @@ -12,6 +12,7 @@ import type { TextPartInput, FilePartInput, } from "@opencode-ai/sdk/v2"; +import { isAmbiguousTransportFailure, markAmbiguousTransportFailure } from "@/lib/relay/transport-error"; import type { PermissionRequest } from "@/types/permission"; import type { QuestionRequest } from "@/types/question"; @@ -878,7 +879,13 @@ class OpencodeService { // failure) — there is no HTTP response to report. Never fabricate a // status: surface it as a transport error so callers treat it like // any other network failure instead of a server 500. - throw new Error(`Message send transport failure: ${formatSdkError(result.error)}`); + // Preserve the transport's "dispatched, outcome unknown" tag through + // the wrap: without it the caller cannot tell a lost response from a + // send that never reached the server, and re-sends a running prompt. + const transportError = new Error(`Message send transport failure: ${formatSdkError(result.error)}`); + throw isAmbiguousTransportFailure(result.error) + ? markAmbiguousTransportFailure(transportError) + : transportError; } response = new Response(JSON.stringify(result.error), { status }); } else { diff --git a/packages/ui/src/lib/relay/transport-error.ts b/packages/ui/src/lib/relay/transport-error.ts new file mode 100644 index 00000000..8d3c1159 --- /dev/null +++ b/packages/ui/src/lib/relay/transport-error.ts @@ -0,0 +1,42 @@ +/** + * Ambiguous transport failures. + * + * When a request dies after it was already handed to the transport, the client + * knows the response was lost — it does NOT know whether the server processed + * the request. Over the relay tunnel this is the common case: a reconnect, a + * host-side stream abort, or a dead channel all fail an in-flight POST that may + * already be running server-side. + * + * Callers must be able to tell that state apart from a definite failure, and + * string-matching the message text is not a contract — a renamed abort reason + * silently reclassifies a send. Transports therefore tag these errors, and + * callers read the tag (see `isAmbiguousTransportFailure`). + * + * `prompt_async` is the motivating case: treating an ambiguous failure as a + * definite one rolls back the user message and lets the queue re-send a prompt + * the engine is already answering, producing two independent AI responses. + */ + +const AMBIGUOUS_TRANSPORT_FLAG = '__openchamberAmbiguousTransport'; + +/** + * Mark an error as "dispatched, outcome unknown". Returns the same error so it + * can be thrown inline. + */ +export const markAmbiguousTransportFailure = (error: T): T => { + Object.defineProperty(error, AMBIGUOUS_TRANSPORT_FLAG, { + value: true, + enumerable: false, + configurable: true, + }); + return error; +}; + +/** + * True when a transport tagged this error as dispatched-but-unconfirmed. + * Deliberately tag-only: text heuristics belong to the caller that owns them. + */ +export const isAmbiguousTransportFailure = (error: unknown): boolean => { + if (!error || typeof error !== 'object') return false; + return (error as Record)[AMBIGUOUS_TRANSPORT_FLAG] === true; +}; diff --git a/packages/ui/src/lib/relay/tunnel-client.test.ts b/packages/ui/src/lib/relay/tunnel-client.test.ts index 0fd903b8..7972b342 100644 --- a/packages/ui/src/lib/relay/tunnel-client.test.ts +++ b/packages/ui/src/lib/relay/tunnel-client.test.ts @@ -11,6 +11,7 @@ import { } from './crypto'; import { createHostHandshake } from './handshake'; import { TunnelFrameType } from './protocol'; +import { isAmbiguousTransportFailure } from './transport-error'; import { createFragmentAssembler, decodeFrameBatch, @@ -339,6 +340,24 @@ describe('createRelayTunnelClient', () => { await expect(reader.read()).rejects.toThrow(); }); + // A POST that dies after dispatch may already have been processed by the + // server. Callers must be able to tell that apart from a definite failure — + // a prompt re-sent on this error produces a second AI response (#2425). + test('tags an in-flight request killed by reconnect as an ambiguous failure', async () => { + const { client, killWire } = await setupClient({ silent: true }); + track(client); + const pending = client.fetch('/api/session/s1/prompt_async', { method: 'POST', body: '{}' }); + let caught: unknown = null; + const settled = pending.catch((error: unknown) => { + caught = error; + }); + await wait(20); + killWire(); + await settled; + expect(caught).toBeInstanceOf(Error); + expect(isAmbiguousTransportFailure(caught)).toBe(true); + }); + test('opens, echoes, and closes a tunneled WebSocket', async () => { const { client } = await setupClient(); track(client); diff --git a/packages/ui/src/lib/relay/tunnel-client.ts b/packages/ui/src/lib/relay/tunnel-client.ts index ab24d78f..fd12b3cb 100644 --- a/packages/ui/src/lib/relay/tunnel-client.ts +++ b/packages/ui/src/lib/relay/tunnel-client.ts @@ -35,6 +35,7 @@ import { isWsClosePayload, normalizeTunnelRequest, } from './tunnel-payloads'; +import { markAmbiguousTransportFailure } from './transport-error'; const EMPTY_PAYLOAD = new Uint8Array(0); const textEncoder = new TextEncoder(); @@ -721,6 +722,13 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela } }; + // The request head is written to the channel below before any of these + // failures can fire, so losing the stream never proves the server did + // not process the request — only that the response was lost. Callers + // that would otherwise retry (prompt sends) must see that distinction. + const dispatchedFailure = (message: string): Error => + markAmbiguousTransportFailure(new Error(message)); + onAbort = () => { sendAbort('aborted'); finishError(abortError()); @@ -735,7 +743,7 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela head = decodeJsonPayload(payload, isHttpResponsePayload); } catch (error) { sendAbort('malformed response head'); - finishError(toError(error)); + finishError(dispatchedFailure(toError(error).message)); return; } const nullBody = head.status === 204 || head.status === 205 || head.status === 304; @@ -773,7 +781,7 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela if (frameType === TunnelFrameType.StreamEnd) { if (finished) return; if (!responseDelivered) { - finishError(new Error('tunnel stream ended before response head')); + finishError(dispatchedFailure('tunnel stream ended before response head')); return; } finished = true; @@ -792,11 +800,15 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela } catch { // Keep the generic reason. } - finishError(new Error(reason)); + finishError(dispatchedFailure(reason)); } }, fail(error) { - finishError(error); + // Channel death (reconnect, keepalive timeout) with this stream still + // open — same rule as above: dispatched, outcome unknown. A fresh + // error is tagged instead of the shared one so the tag cannot leak to + // waiters whose request never reached the wire. + finishError(dispatchedFailure(error.message)); }, }); @@ -824,7 +836,7 @@ export const createRelayTunnelClient = (options: RelayTunnelClientOptions): Rela } } catch (error) { sendAbort('request body failed'); - finishError(toError(error)); + finishError(dispatchedFailure(toError(error).message)); } })(); }); diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 78152f32..96761339 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -232,6 +232,7 @@ Rules: 3. `session-ui-store.ts` should delegate to `session-actions.ts` for these mutations instead of duplicating SDK calls. 4. Sending after a revert commits the new branch optimistically: remove the reverted tail and marker before inserting the new message, and restore both if the send is rejected. 5. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session. +6. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/session-actions.test.ts b/packages/ui/src/sync/session-actions.test.ts index c9c4325a..d0cd9f16 100644 --- a/packages/ui/src/sync/session-actions.test.ts +++ b/packages/ui/src/sync/session-actions.test.ts @@ -1014,6 +1014,53 @@ describe("optimisticSend target directory", () => { expect(targetStore.getState().part[sentMessageID]?.[0]?.id).toBe("server-part") }) + // Relay tunnel aborts carry no HTTP status and no wording the text-matching + // heuristic recognizes. Without the transport tag they were classified as + // definite failures, the accepted prompt was rolled back, and the queue + // re-sent a message the engine was already answering (#2425). + test("confirms a tunnel-tagged transport failure that no text heuristic matches", async () => { + const targetStore = createStore({}) + const childStores = createChildStores([["/target/project", targetStore]]) + let optimisticRemove: OptimisticRemoveCall | null = null + let optimisticConfirm: OptimisticRemoveCall | null = null + let sentMessageID = "" + + const { markAmbiguousTransportFailure } = await import("@/lib/relay/transport-error") + const { optimisticSend, setActionRefs, setOptimisticRefs } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/target/project") + setOptimisticRefs( + () => {}, + (input) => { + optimisticRemove = input + }, + (input) => { + optimisticConfirm = input + }, + ) + + await optimisticSend({ + sessionId: "session-tunnel", + directory: "/target/project", + content: "hello", + providerID: "provider", + modelID: "model", + send: async (messageID) => { + sentMessageID = messageID + sessionMessagesResult = { + data: [{ + info: { id: messageID, role: "user", sessionID: "session-tunnel", time: { created: 1 } } as Message, + parts: [{ id: "server-part", type: "text", text: "hello" } as Part], + }], + } + throw markAmbiguousTransportFailure(new Error("stream aborted by host")) + }, + }) + + expect(optimisticRemove).toBe(null) + expect((optimisticConfirm as OptimisticRemoveCall | null)?.messageID).toBe(sentMessageID) + expect(targetStore.getState().message["session-tunnel"]?.[0]?.id).toBe(sentMessageID) + }) + test("rolls back an ambiguous send failure when recent messages do not contain the sent ID", async () => { const targetStore = createStore({}) const childStores = createChildStores([["/target/project", targetStore]]) diff --git a/packages/ui/src/sync/session-actions.ts b/packages/ui/src/sync/session-actions.ts index 5a6eab26..7e01309b 100644 --- a/packages/ui/src/sync/session-actions.ts +++ b/packages/ui/src/sync/session-actions.ts @@ -29,11 +29,21 @@ import { withContextObligatoryMessage, type ContextObligatoryMessage } from "@/l import { getImperativeSessionMessageLoader } from "./session-message-loader" import { cleanupPersistedSessionState } from "./session-deletion-cleanup" import { getRuntimeKey } from "@/lib/runtime-switch" +import { isAmbiguousTransportFailure } from "@/lib/relay/transport-error" const MESSAGE_REFETCH_LIMIT = 100 const SEND_CONFIRMATION_REFETCH_LIMIT = 30 -const SEND_CONFIRMATION_REFETCH_ATTEMPTS = 2 -const SEND_CONFIRMATION_REFETCH_RETRY_MS = 150 +// A relay-tunnel send fails when the tunnel drops, and the confirming refetch +// then has to travel over that same tunnel to answer "did my message land?". +// Two attempts 150ms apart always answered "no" on a remote connection, so an +// accepted prompt looked like a failed one and got re-sent — two AI responses +// for one user message. Wait for the connection to actually come back (an +// authoritative signal, not a blind sleep), then retry with backoff. A healthy +// connection skips the wait and answers on the first attempt. +const SEND_CONFIRMATION_REFETCH_ATTEMPTS = 3 +const SEND_CONFIRMATION_REFETCH_BASE_RETRY_MS = 250 +const SEND_CONFIRMATION_RECONNECT_TIMEOUT_MS = 3000 +const SEND_CONFIRMATION_RECONNECT_POLL_MS = 100 const MESSAGE_REFETCH_SKIP_PARTS = new Set(["patch", "step-start", "step-finish"]) const UNREVERT_REFETCH_ATTEMPTS = 3 const UNREVERT_REFETCH_RETRY_MS = 150 @@ -360,6 +370,13 @@ function getErrorStatus(error: unknown): number | null { } function isAmbiguousSendFailure(error: unknown): boolean { + // Authoritative first: the transport that lost the request says whether it + // had already been dispatched. The text matching below only covers direct + // fetch/HTTP failures, whose wording we do not control either — relay tunnel + // aborts ("stream aborted by host", "relay keepalive timeout", …) match none + // of those patterns and used to be misread as definite failures. + if (isAmbiguousTransportFailure(error)) return true + const status = getErrorStatus(error) if (status === 503 || status === 504 || status === 408) return true if (error instanceof TypeError) return true @@ -1255,8 +1272,15 @@ async function fetchRecentSendConfirmationRecords( messageID: string, directory?: string | null, ): Promise | null> { + // Bounded: a connection that never returns must still let the send fail + // rather than hang the composer. + const reconnectDeadline = Date.now() + SEND_CONFIRMATION_RECONNECT_TIMEOUT_MS + while (!useConfigStore.getState().isConnected && Date.now() < reconnectDeadline) { + await wait(SEND_CONFIRMATION_RECONNECT_POLL_MS) + } + for (let attempt = 0; attempt < SEND_CONFIRMATION_REFETCH_ATTEMPTS; attempt += 1) { - if (attempt > 0) await wait(SEND_CONFIRMATION_REFETCH_RETRY_MS) + if (attempt > 0) await wait(SEND_CONFIRMATION_REFETCH_BASE_RETRY_MS * 2 ** (attempt - 1)) try { const result = await sdk().session.messages({ sessionID: sessionId, From 237cae16b33b28f215480bd5b2010138aec96383 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 3 Aug 2026 23:14:14 +0300 Subject: [PATCH 11/13] fix: stop the composer re-sending a queued message already in flight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A queued message is removed from the queue only after its send resolves, so between dispatch and resolution it stays visible to every reader — and a composer submit merges the whole queue into its own send. Over a relay that window is seconds, long enough to deliver the same message twice. The queue now tracks which entries are awaiting the server. Dispatchers skip them, clearQueue retains them so the pending send can still remove or restore its own entry, and the flag is not persisted because a restart has no in-flight sends. --- packages/ui/src/components/chat/ChatInput.tsx | 12 +++- .../ui/src/hooks/useQueuedMessageAutoSend.ts | 9 ++- packages/ui/src/stores/DOCUMENTATION.md | 3 + .../ui/src/stores/messageQueueStore.test.ts | 47 ++++++++++++++- packages/ui/src/stores/messageQueueStore.ts | 60 ++++++++++++++++++- 5 files changed, 126 insertions(+), 5 deletions(-) diff --git a/packages/ui/src/components/chat/ChatInput.tsx b/packages/ui/src/components/chat/ChatInput.tsx index feda2c44..ba493f55 100644 --- a/packages/ui/src/components/chat/ChatInput.tsx +++ b/packages/ui/src/components/chat/ChatInput.tsx @@ -159,6 +159,7 @@ const MAX_MOBILE_COMPOSER_LINES = 16; */ const MOBILE_COMPOSER_BOUND_GAP_PX = 4; const EMPTY_QUEUE: QueuedMessage[] = []; +const EMPTY_SENDING_IDS: string[] = []; const COMPACT_CHAT_PLACEHOLDER_MAX_WIDTH = 560; const renameFileForAttachmentCitation = (file: File, filename: string): File => { if (file.name === filename) { @@ -945,9 +946,16 @@ const ChatInputComponent: React.FC = ({ onOpenSettings, scrollTo hasContent: options.presetText.trim().length > 0 || attachedFiles.length > 0 || hasDrafts, } : getCurrentInputSnapshot(); - const queuedMessagesToSend = queuedMessageId + // A queued item stays in the queue until its own send resolves, so the + // auto-send hook may already be delivering one of these. Merging it here + // would send the same message twice (the window is seconds over a relay). + const sendingIds = messageQueueTarget + ? useMessageQueueStore.getState().sendingIds[getMessageQueueKey(messageQueueTarget)] ?? EMPTY_SENDING_IDS + : EMPTY_SENDING_IDS; + const queuedMessagesToSend = (queuedMessageId ? queuedMessages.filter((message) => message.id === queuedMessageId) - : queuedMessages; + : queuedMessages + ).filter((message) => !sendingIds.includes(message.id)); if (queuedOnly && autoReviewRunning) { return; diff --git a/packages/ui/src/hooks/useQueuedMessageAutoSend.ts b/packages/ui/src/hooks/useQueuedMessageAutoSend.ts index bc327ebe..e7b39c97 100644 --- a/packages/ui/src/hooks/useQueuedMessageAutoSend.ts +++ b/packages/ui/src/hooks/useQueuedMessageAutoSend.ts @@ -221,7 +221,9 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? return; } - const payload = buildQueuedAutoSendPayload(queueSnapshot); + // Read the queue back at dispatch time and skip anything already being + // delivered, rather than trusting the render-time snapshot. + const payload = buildQueuedAutoSendPayload(useMessageQueueStore.getState().getSendableQueue(target)); if (!payload) { return; } @@ -248,6 +250,10 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? } inFlightSessionsRef.current.add(targetKey); + // The ref only guards this hook. Publish the dispatch to the store so the + // composer cannot merge the same item into a parallel send while this one + // is still awaiting the server. + useMessageQueueStore.getState().markSending(target, payload.queuedMessageId); try { await sendQueuedAutoSendPayload(sessionId, target.directory, payload, { @@ -271,6 +277,7 @@ export function useQueuedMessageAutoSend(enabledOrOptions?: boolean | { enabled? retryScheduler.schedule(nextAttemptAt); } finally { inFlightSessionsRef.current.delete(targetKey); + useMessageQueueStore.getState().clearSending(target, payload.queuedMessageId); } }; diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index bca5d525..65ad473a 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -47,9 +47,12 @@ Examples: - `useProjectsStore.ts` - `useGlobalSessionsStore.ts` - `useSessionFoldersStore.ts` +- `messageQueueStore.ts` These stores coordinate persistent project/session metadata across multiple views. +`messageQueueStore.ts` keeps a queued message until its own send resolves, so between dispatch and resolution the entry is still visible to every reader. Dispatchers must therefore mark the send (`markSending`/`clearSending`) and read `getSendableQueue()` — or filter `sendingIds` themselves — instead of dispatching straight from `queuedMessages`; otherwise a composer submit merges a message the auto-send hook is already delivering and it is sent twice (the window is seconds over a relay). `clearQueue()` retains in-flight entries for the same reason. `sendingIds` is deliberately not persisted: a restart has no in-flight sends, and a stale flag would strand a queued message. + `useGlobalSessionsStore.ts` owns cold/global active and archived session coverage, including `sessionsByDirectory`. It is complementary to directory child stores: it is not the source of live busy/retry status or session messages. User-visible session ordering is also not owned by the global cache array order. `sync/session-ordering.ts` combines lifecycle rank with timestamp fallbacks, and session surfaces must use that shared comparator instead of independently sorting global sessions by `time.updated`. diff --git a/packages/ui/src/stores/messageQueueStore.test.ts b/packages/ui/src/stores/messageQueueStore.test.ts index 6ec859a0..8d4b237d 100644 --- a/packages/ui/src/stores/messageQueueStore.test.ts +++ b/packages/ui/src/stores/messageQueueStore.test.ts @@ -8,7 +8,7 @@ import { } from "./messageQueueStore" beforeEach(() => { - useMessageQueueStore.setState({ queuedMessages: {}, quarantinedLegacyMessages: {} }) + useMessageQueueStore.setState({ queuedMessages: {}, quarantinedLegacyMessages: {}, sendingIds: {} }) }) describe("message queue runtime ownership", () => { @@ -49,3 +49,48 @@ describe("message queue runtime ownership", () => { expect(queue[0]?.content).toBe("message-5") }) }) + +describe("in-flight queued sends", () => { + test("hides a dispatched message from the sendable queue but keeps it visible", () => { + const target = createMessageQueueTarget("session-1", "/repo", "runtime-a")! + const store = useMessageQueueStore.getState() + store.addToQueue(target, { content: "first" }) + store.addToQueue(target, { content: "second" }) + const [first] = useMessageQueueStore.getState().getQueueForTarget(target) + + useMessageQueueStore.getState().markSending(target, first.id) + + expect(useMessageQueueStore.getState().getQueueForTarget(target)).toHaveLength(2) + const sendable = useMessageQueueStore.getState().getSendableQueue(target) + expect(sendable).toHaveLength(1) + expect(sendable[0]?.content).toBe("second") + + useMessageQueueStore.getState().clearSending(target, first.id) + expect(useMessageQueueStore.getState().getSendableQueue(target)).toHaveLength(2) + expect(useMessageQueueStore.getState().sendingIds).toEqual({}) + }) + + test("clearQueue retains a message whose send is still awaiting the server", () => { + const target = createMessageQueueTarget("session-1", "/repo", "runtime-a")! + const store = useMessageQueueStore.getState() + store.addToQueue(target, { content: "in flight" }) + store.addToQueue(target, { content: "merged by composer" }) + const [inFlight] = useMessageQueueStore.getState().getQueueForTarget(target) + useMessageQueueStore.getState().markSending(target, inFlight.id) + + useMessageQueueStore.getState().clearQueue(target) + + const remaining = useMessageQueueStore.getState().getQueueForTarget(target) + expect(remaining).toHaveLength(1) + expect(remaining[0]?.id).toBe(inFlight.id) + }) + + test("clearQueue drops everything once no send is in flight", () => { + const target = createMessageQueueTarget("session-1", "/repo", "runtime-a")! + useMessageQueueStore.getState().addToQueue(target, { content: "queued" }) + + useMessageQueueStore.getState().clearQueue(target) + + expect(useMessageQueueStore.getState().getQueueForTarget(target)).toHaveLength(0) + }) +}) diff --git a/packages/ui/src/stores/messageQueueStore.ts b/packages/ui/src/stores/messageQueueStore.ts index 2ac9f86f..8ee096ec 100644 --- a/packages/ui/src/stores/messageQueueStore.ts +++ b/packages/ui/src/stores/messageQueueStore.ts @@ -85,6 +85,19 @@ interface MessageQueueState { queuedMessages: Record; // runtime + directory + session → queue quarantinedLegacyMessages: Record; followUpBehavior: FollowUpBehavior; + /** + * Queued messages whose send is currently awaiting the server, per target. + * + * A queued item is removed only after its send resolves, so between + * dispatch and resolution it is still visible to every other reader — and + * a composer submit merges the whole queue into its own send. Over a relay + * that window is seconds, long enough for the same message to be delivered + * twice. Dispatchers must skip entries listed here. + * + * Never persisted: a restart has no in-flight sends, and a stale flag would + * strand a queued message permanently. + */ + sendingIds: Record; } interface MessageQueueActions { @@ -94,6 +107,9 @@ interface MessageQueueActions { popToInput: (target: MessageQueueTarget, messageId: string) => QueuedMessage | null; clearQueue: (target: MessageQueueTarget) => void; clearAllQueues: () => void; + markSending: (target: MessageQueueTarget, messageId: string) => void; + clearSending: (target: MessageQueueTarget, messageId: string) => void; + getSendableQueue: (target: MessageQueueTarget) => QueuedMessage[]; setFollowUpBehavior: (behavior: FollowUpBehavior) => void; getQueueForTarget: (target: MessageQueueTarget) => QueuedMessage[]; } @@ -127,6 +143,7 @@ export const useMessageQueueStore = create()( queuedMessages: {}, quarantinedLegacyMessages: {}, followUpBehavior: DEFAULT_FOLLOW_UP_BEHAVIOR, + sendingIds: {}, addToQueue: (target, message) => { const key = getMessageQueueKey(target); @@ -237,6 +254,14 @@ export const useMessageQueueStore = create()( clearQueue: (target) => { const key = getMessageQueueKey(target); set((state) => { + // Clearing drops what is still queued, never a message + // already handed to the server: that send will resolve + // and must find its entry to remove or restore. + const sending = state.sendingIds[key] ?? []; + const retained = (state.queuedMessages[key] ?? []).filter((m) => sending.includes(m.id)); + if (retained.length > 0) { + return { queuedMessages: { ...state.queuedMessages, [key]: retained } }; + } const { [key]: _removed, ...rest } = state.queuedMessages; void _removed; return { queuedMessages: rest }; @@ -244,7 +269,40 @@ export const useMessageQueueStore = create()( }, clearAllQueues: () => { - set({ queuedMessages: {} }); + set({ queuedMessages: {}, sendingIds: {} }); + }, + + markSending: (target, messageId) => { + const key = getMessageQueueKey(target); + set((state) => { + const current = state.sendingIds[key] ?? []; + if (current.includes(messageId)) return state; + return { sendingIds: { ...state.sendingIds, [key]: [...current, messageId] } }; + }); + }, + + clearSending: (target, messageId) => { + const key = getMessageQueueKey(target); + set((state) => { + const current = state.sendingIds[key]; + if (!current || !current.includes(messageId)) return state; + const next = current.filter((id) => id !== messageId); + if (next.length === 0) { + const { [key]: _removed, ...rest } = state.sendingIds; + void _removed; + return { sendingIds: rest }; + } + return { sendingIds: { ...state.sendingIds, [key]: next } }; + }); + }, + + getSendableQueue: (target) => { + const key = getMessageQueueKey(target); + const state = get(); + const queue = state.queuedMessages[key] ?? []; + const sending = state.sendingIds[key]; + if (!sending || sending.length === 0) return queue; + return queue.filter((message) => !sending.includes(message.id)); }, setFollowUpBehavior: (behavior) => { From 4c0fc25ac83fb7ec2a184d274d1c88cc62236cc8 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 3 Aug 2026 23:37:53 +0300 Subject: [PATCH 12/13] fix(worktree): stop writing worktree registration into OpenCode's storage Creating a worktree wrote the new directory straight into OpenCode's own project storage: the web server updated `storage/project/.json` and ran an `UPDATE project SET sandboxes` against `opencode.db` through better-sqlite3, and the VS Code extension wrote the same JSON. Both wrote behind the back of a running OpenCode process. OpenCode registers a sandbox through `project.addSandbox`, which emits a project-updated event; a direct row write emits nothing, so a worktree created while OpenCode was running stayed unknown to it until a restart. The SQLite write also opened a database file owned by another live process. The VS Code write was inert on top of that: OpenCode v2 reads sandboxes from the database, not from that JSON. Registration is not ours to perform. OpenCode records a worktree as a sandbox itself when an instance boots for that directory, and filters entries whose directory no longer exists when reading them back, so removal needs no counterpart either. The only consumer on our side, the project seed in sync/bootstrap.ts, already falls back to `project.current()` when the seed is absent; the worktree list itself comes from git, not from sandboxes. Reported symptom this targets: a worktree created after `openchamber restart` never answers prompts, and restarting OpenChamber makes it work. Not reproduced locally, so this is not confirmed as the cause. --- packages/vscode/src/gitService.ts | 105 +------------------- packages/web/server/lib/git/service.js | 127 ++----------------------- 2 files changed, 12 insertions(+), 220 deletions(-) diff --git a/packages/vscode/src/gitService.ts b/packages/vscode/src/gitService.ts index 4d915d80..35f24347 100644 --- a/packages/vscode/src/gitService.ts +++ b/packages/vscode/src/gitService.ts @@ -1385,74 +1385,11 @@ const loadProjectStartCommand = async (projectID: string): Promise => { } }; -const getProjectStoragePath = (projectID: string) => { - return path.join(getOpenCodeDataPath(), 'storage', 'project', `${projectID}.json`); -}; - -const updateProjectSandboxes = async ( - projectID: string, - primaryWorktree: string, - updater: (project: { - id: string; - worktree: string; - vcs: string; - sandboxes: string[]; - time: { created: number; updated: number }; - }) => void -) => { - const storagePath = getProjectStoragePath(projectID); - await fs.promises.mkdir(path.dirname(storagePath), { recursive: true }); - - const now = Date.now(); - const base = { - id: projectID, - worktree: primaryWorktree, - vcs: 'git', - sandboxes: [] as string[], - time: { created: now, updated: now }, - }; - - const parsed = await fs.promises.readFile(storagePath, 'utf8').then((raw) => JSON.parse(raw) as typeof base).catch(() => null); - const current = parsed && typeof parsed === 'object' ? { ...base, ...parsed } : base; - current.id = String(current.id || projectID); - current.worktree = String(current.worktree || primaryWorktree); - current.vcs = current.vcs || 'git'; - current.sandboxes = Array.isArray(current.sandboxes) - ? current.sandboxes.map((entry) => String(entry || '').trim()).filter(Boolean) - : []; - const createdAt = Number(current?.time?.created); - current.time = { - created: Number.isFinite(createdAt) && createdAt > 0 ? createdAt : now, - updated: now, - }; - - updater(current); - - current.sandboxes = [...new Set(current.sandboxes.map((entry) => String(entry || '').trim()).filter(Boolean))]; - await fs.promises.writeFile(storagePath, `${JSON.stringify(current, null, 2)}\n`, 'utf8'); -}; - -const syncProjectSandboxAdd = async (projectID: string, primaryWorktree: string, sandboxPath: string) => { - const sandbox = String(sandboxPath || '').trim(); - if (!sandbox) { - return; - } - await updateProjectSandboxes(projectID, primaryWorktree, (project) => { - if (!project.sandboxes.includes(sandbox)) { - project.sandboxes.push(sandbox); - } - }); -}; - -const syncProjectSandboxRemove = async (projectID: string, primaryWorktree: string, sandboxPath: string) => { - const sandbox = String(sandboxPath || '').trim(); - if (!sandbox) { - return; - } - await updateProjectSandboxes(projectID, primaryWorktree, (project) => { - project.sandboxes = project.sandboxes.filter((entry) => entry !== sandbox); - }); -}; +// OpenCode owns its own project/sandbox registry and records a worktree as a +// sandbox itself when an instance boots for that directory. OpenChamber used to +// write that state into OpenCode's storage JSON directly, behind the back of the +// running process — and since OpenCode v2 reads sandboxes from its database, the +// JSON write did not even reach it. Registration is not ours to perform. const isInsideOrSameDirectory = (root: string, target: string): boolean => { const relative = path.relative(root, target); @@ -1477,14 +1414,6 @@ const cleanupFailedFastWorktreeCreate = async ( const isInsideWorktreeRoot = isInsideOrSameDirectory(worktreeRoot, candidateDirectory) && candidateDirectory !== worktreeRoot; const isAttached = await isAttachedGitWorktreeDirectory(candidateDirectory); - if (!isAttached) { - try { - await syncProjectSandboxRemove(context.projectID, context.primaryWorktree, candidateDirectory); - } catch (error) { - console.warn('[GitService] Failed to clean up OpenCode sandbox metadata after worktree failure:', error instanceof Error ? error.message : String(error)); - } - } - if (!isInsideWorktreeRoot || isAttached) { return; } @@ -1963,12 +1892,6 @@ async function attachGitWorktreeToCandidate( await runGitCommandOrThrow(context.primaryWorktree, worktreeAddArgs, 'Failed to create git worktree'); - try { - await syncProjectSandboxAdd(context.projectID, context.primaryWorktree, candidate.directory); - } catch (error) { - console.warn('[GitService] Failed to sync OpenCode sandbox metadata (add):', error instanceof Error ? error.message : String(error)); - } - const shouldSetUpstream = Boolean(input?.setUpstream); const upstreamRemote = String(input?.upstreamRemote || inferredUpstream?.remote || '').trim(); const upstreamBranch = String(input?.upstreamBranch || inferredUpstream?.branch || '').trim(); @@ -2033,12 +1956,6 @@ export async function createWorktree(directory: string, input: CreateGitWorktree if (input?.returnAfterDirectoryCreated === true) { await fs.promises.mkdir(candidate.directory, { recursive: false }); - try { - await syncProjectSandboxAdd(context.projectID, context.primaryWorktree, candidate.directory); - } catch (error) { - console.warn('[GitService] Failed to sync OpenCode sandbox metadata (add):', error instanceof Error ? error.message : String(error)); - } - const bootstrapStatus = setWorktreeBootstrapState( candidate.directory, WORKTREE_BOOTSTRAP_PENDING, @@ -2129,12 +2046,6 @@ export async function removeWorktree(directory: string, input: RemoveGitWorktree await fs.promises.rm(targetDirectory, { recursive: true, force: true }); } - try { - await syncProjectSandboxRemove(context.projectID, context.primaryWorktree, targetDirectory); - } catch (error) { - console.warn('[GitService] Failed to sync OpenCode sandbox metadata (remove):', error instanceof Error ? error.message : String(error)); - } - clearWorktreeBootstrapState(targetDirectory); return true; @@ -2157,12 +2068,6 @@ export async function removeWorktree(directory: string, input: RemoveGitWorktree } } - try { - await syncProjectSandboxRemove(context.projectID, context.primaryWorktree, matchedEntry.worktree); - } catch (error) { - console.warn('[GitService] Failed to sync OpenCode sandbox metadata (remove):', error instanceof Error ? error.message : String(error)); - } - clearWorktreeBootstrapState(matchedEntry.worktree); return true; diff --git a/packages/web/server/lib/git/service.js b/packages/web/server/lib/git/service.js index ebe8c334..a3a99f62 100644 --- a/packages/web/server/lib/git/service.js +++ b/packages/web/server/lib/git/service.js @@ -1644,94 +1644,13 @@ const loadProjectStartCommand = async (projectID) => { } }; -const getProjectStoragePath = (projectID) => { - return path.join(getOpenCodeDataPath(), 'storage', 'project', `${projectID}.json`); -}; - -const syncSandboxesToOpenCodeDb = (projectID, sandboxes) => { - try { - const Database = require('better-sqlite3'); - const dbPath = path.join(getOpenCodeDataPath(), 'opencode.db'); - if (!fs.existsSync(dbPath)) return; - const db = new Database(dbPath); - try { - const row = db.prepare('SELECT sandboxes FROM project WHERE id = ?').get(projectID); - if (!row) return; - const json = JSON.stringify(sandboxes); - db.prepare('UPDATE project SET sandboxes = ?, time_updated = ? WHERE id = ?').run(json, Date.now(), projectID); - } finally { - db.close(); - } - } catch (error) { - console.warn('Failed to sync sandboxes to OpenCode DB:', error instanceof Error ? error.message : String(error)); - } -}; - -const updateProjectSandboxes = async (projectID, primaryWorktree, updater) => { - const storagePath = getProjectStoragePath(projectID); - await fsp.mkdir(path.dirname(storagePath), { recursive: true }); - - const now = Date.now(); - const base = { - id: projectID, - worktree: primaryWorktree, - vcs: 'git', - sandboxes: [], - time: { - created: now, - updated: now, - }, - }; - - const parsed = await fsp.readFile(storagePath, 'utf8').then((raw) => JSON.parse(raw)).catch(() => null); - const current = parsed && typeof parsed === 'object' ? { ...base, ...parsed } : base; - current.id = String(current.id || projectID); - current.worktree = String(current.worktree || primaryWorktree); - current.vcs = current.vcs || 'git'; - current.sandboxes = Array.isArray(current.sandboxes) - ? current.sandboxes.map((entry) => String(entry || '').trim()).filter(Boolean) - : []; - const createdAt = Number(current?.time?.created); - current.time = { - created: Number.isFinite(createdAt) && createdAt > 0 ? createdAt : now, - updated: now, - }; - - updater(current); - - current.sandboxes = [...new Set( - (Array.isArray(current.sandboxes) ? current.sandboxes : []) - .map((entry) => String(entry || '').trim()) - .filter(Boolean) - )]; - - await fsp.writeFile(storagePath, `${JSON.stringify(current, null, 2)}\n`, 'utf8'); - - // Sync to OpenCode's SQLite database so project.sandboxes is visible via the SDK - syncSandboxesToOpenCodeDb(projectID, current.sandboxes); -}; - -const syncProjectSandboxAdd = async (projectID, primaryWorktree, sandboxPath) => { - const sandbox = String(sandboxPath || '').trim(); - if (!sandbox) { - return; - } - await updateProjectSandboxes(projectID, primaryWorktree, (project) => { - if (!project.sandboxes.includes(sandbox)) { - project.sandboxes.push(sandbox); - } - }); -}; - -const syncProjectSandboxRemove = async (projectID, primaryWorktree, sandboxPath) => { - const sandbox = String(sandboxPath || '').trim(); - if (!sandbox) { - return; - } - await updateProjectSandboxes(projectID, primaryWorktree, (project) => { - project.sandboxes = project.sandboxes.filter((entry) => entry !== sandbox); - }); -}; +// OpenCode owns its own project/sandbox registry. It records a worktree as a +// sandbox itself when an instance boots for that directory, and filters entries +// whose directory no longer exists when reading them back. OpenChamber used to +// write that state directly into OpenCode's storage JSON and SQLite database, +// behind the back of the running process: the row changed but the server was +// never told, so a worktree created while OpenCode was running stayed unknown +// to it until a restart. Registration is not ours to perform. const isAttachedGitWorktreeDirectory = async (directory) => { try { @@ -1748,14 +1667,6 @@ const cleanupFailedFastWorktreeCreate = async (context, candidate) => { const isInsideWorktreeRoot = isInsideOrSameDirectory(worktreeRoot, candidateDirectory) && candidateDirectory !== worktreeRoot; const isAttached = await isAttachedGitWorktreeDirectory(candidateDirectory); - if (!isAttached) { - try { - await syncProjectSandboxRemove(context.projectID, context.primaryWorktree, candidateDirectory); - } catch (error) { - console.warn('Failed to clean up OpenCode sandbox metadata after worktree failure:', error instanceof Error ? error.message : String(error)); - } - } - if (!isInsideWorktreeRoot || isAttached) { return; } @@ -3940,12 +3851,6 @@ async function attachGitWorktreeToCandidate(context, candidate, input = {}) { await runGitCommandOrThrow(context.primaryWorktree, worktreeAddArgs, 'Failed to create git worktree'); - try { - await syncProjectSandboxAdd(context.projectID, context.primaryWorktree, candidate.directory); - } catch (error) { - console.warn('Failed to sync OpenCode sandbox metadata (add):', error instanceof Error ? error.message : String(error)); - } - const shouldSetUpstream = Boolean(input?.setUpstream); const upstreamRemote = String(input?.upstreamRemote || inferredUpstream?.remote || '').trim(); const upstreamBranch = String(input?.upstreamBranch || inferredUpstream?.branch || '').trim(); @@ -4005,12 +3910,6 @@ export async function createWorktree(directory, input = {}) { if (input?.returnAfterDirectoryCreated === true) { await fsp.mkdir(candidate.directory, { recursive: false }); - try { - await syncProjectSandboxAdd(context.projectID, context.primaryWorktree, candidate.directory); - } catch (error) { - console.warn('Failed to sync OpenCode sandbox metadata (add):', error instanceof Error ? error.message : String(error)); - } - const bootstrapStatus = setWorktreeBootstrapState( candidate.directory, WORKTREE_BOOTSTRAP_PENDING, @@ -4103,12 +4002,6 @@ export async function removeWorktree(directory, input = {}) { await fsp.rm(targetDirectory, { recursive: true, force: true }); } - try { - await syncProjectSandboxRemove(context.projectID, context.primaryWorktree, targetDirectory); - } catch (error) { - console.warn('Failed to sync OpenCode sandbox metadata (remove):', error instanceof Error ? error.message : String(error)); - } - clearWorktreeBootstrapState(targetDirectory); return true; @@ -4131,12 +4024,6 @@ export async function removeWorktree(directory, input = {}) { } } - try { - await syncProjectSandboxRemove(context.projectID, context.primaryWorktree, matchedEntry.worktree); - } catch (error) { - console.warn('Failed to sync OpenCode sandbox metadata (remove):', error instanceof Error ? error.message : String(error)); - } - clearWorktreeBootstrapState(matchedEntry.worktree); return true; From 56f2b972f00ff34e3490803f4c37ba384ebc6340 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Mon, 3 Aug 2026 23:38:01 +0300 Subject: [PATCH 13/13] chore(deps): drop better-sqlite3 and its desktop packaging support The SQLite write into OpenCode's database was the only consumer of better-sqlite3 in the repository. Everything that existed to ship its native binary went with it: - the dependency in @openchamber/web and @openchamber/electron - the afterPack hook staging better_sqlite3.node into app.asar.unpacked - a dedicated @electron/rebuild pass (onlyModules) and its binary assertion, so desktop packaging now runs one native rebuild instead of two - the bundler external entry and the AppImage required-native-module check Desktop packaging, the AppImage verification tests, and the extension bundle were re-validated after a clean reinstall, so no stale module could satisfy a missed import. --- .github/workflows/release.yml | 4 ++-- bun.lock | 8 -------- packages/electron/README.md | 2 +- packages/electron/package.json | 1 - packages/electron/scripts/after-pack.cjs | 17 ----------------- packages/electron/scripts/bundle-main.mjs | 1 - packages/electron/scripts/rebuild-native.mjs | 13 ------------- .../electron/scripts/verify-linux-appimage.mjs | 2 +- .../scripts/verify-linux-appimage.test.mjs | 4 ++-- packages/web/package.json | 1 - 10 files changed, 6 insertions(+), 47 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 47dbb0c5..28a3afc8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -204,8 +204,8 @@ jobs: bun run bundle:main # npmRebuild=false in package.json, so electron-builder won't # recompile native deps on its own — we must rebuild against the - # target Electron ABI before packaging, otherwise better-sqlite3/ - # node-pty/bun-pty crash on require inside the packaged app. + # target Electron ABI before packaging, otherwise node-pty/bun-pty + # crash on require inside the packaged app. bun run rebuild:native bunx electron-builder --mac --${{ matrix.arch }} --publish=never bun run verify:opencode-cli:packaged diff --git a/bun.lock b/bun.lock index 2209fc6d..370b1124 100644 --- a/bun.lock +++ b/bun.lock @@ -98,7 +98,6 @@ "version": "1.17.2", "dependencies": { "@openchamber/web": "workspace:*", - "better-sqlite3": "^12.10.0", "electron-context-menu": "^4.1.2", "electron-log": "^5.4.3", "electron-updater": "^6.8.3", @@ -270,7 +269,6 @@ "@opencode-ai/sdk": "1.18.11", "@simplewebauthn/server": "13.3.1", "adm-zip": "^0.5.16", - "better-sqlite3": "^12.10.0", "bun-pty": "^0.4.5", "compression": "^1.8.1", "cron-parser": "^4.9.0", @@ -1583,16 +1581,12 @@ "before-after-hook": ["before-after-hook@4.0.0", "", {}, "sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ=="], - "better-sqlite3": ["better-sqlite3@12.10.0", "", { "dependencies": { "bindings": "^1.5.0", "prebuild-install": "^7.1.1" } }, "sha512-CyzaZRQKyHkB2ZInfTTl2nvT33EbDpjkLEbE8/Zck3Ll6O0qqvuGdrJ45HgtH+HykRg88ITY3AdreBGN70aBSQ=="], - "big-integer": ["big-integer@1.6.52", "", {}, "sha512-QxD8cf2eVqJOOz63z6JIN9BzvVs/dlySa5HGSBH5xtR8dPteIRQnBxxKqkNTiT6jbDTF6jAfrd4oMcND9RGbQg=="], "binary-extensions": ["binary-extensions@2.3.0", "", {}, "sha512-Ceh+7ox5qe7LJuLHoY0feh3pHuUDHAcRUeyL2VYghZwfpkNIy/+8Ocg0a3UuSoYzavmylwuLWQOf3hl0jjMMIw=="], "binaryextensions": ["binaryextensions@6.11.0", "", { "dependencies": { "editions": "^6.21.0" } }, "sha512-sXnYK/Ij80TO3lcqZVV2YgfKN5QjUWIRk/XSm2J/4bd/lPko3lvk0O4ZppH6m+6hB2/GTu+ptNwVFe1xh+QLQw=="], - "bindings": ["bindings@1.5.0", "", { "dependencies": { "file-uri-to-path": "1.0.0" } }, "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ=="], - "bl": ["bl@4.1.0", "", { "dependencies": { "buffer": "^5.5.0", "inherits": "^2.0.4", "readable-stream": "^3.4.0" } }, "sha512-1W07cM9gS6DcLperZfFSj+bWLtaPGSOHWhPiGzXmvVJbRLdG82sH/Kn8EtW1VqWVA54AKf2h5k5BbnIbwF3h6w=="], "bn.js": ["bn.js@4.12.3", "", {}, "sha512-fGTi3gxV/23FTYdAoUtLYp6qySe2KE3teyZitipKNRuVYcBkoP/bB3guXN/XVKUe9mxCHXnc9C4ocyz8OmgN0g=="], @@ -2005,8 +1999,6 @@ "file-entry-cache": ["file-entry-cache@8.0.0", "", { "dependencies": { "flat-cache": "^4.0.0" } }, "sha512-XXTUwCvisa5oacNGRP9SfNtYBNAMi+RPwBFmblZEF7N7swHYQS6/Zfk7SRwx4D5j3CH211YNRco1DEMNVfZCnQ=="], - "file-uri-to-path": ["file-uri-to-path@1.0.0", "", {}, "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw=="], - "filelist": ["filelist@1.0.6", "", { "dependencies": { "minimatch": "^5.0.1" } }, "sha512-5giy2PkLYY1cP39p17Ech+2xlpTRL9HLspOfEgm0L6CwBXBTgsK5ou0JtzYuepxkaQ/tvhCFIJ5uXo0OrM2DxA=="], "fill-range": ["fill-range@7.1.1", "", { "dependencies": { "to-regex-range": "^5.0.1" } }, "sha512-YsGpe3WHLK8ZYi4tWDg2Jy3ebRz2rXowDxnld4bkQB00cc/1Zw9AWnC0i9ztDJitivtQvaI9KaLyKrc+hBW0yg=="], diff --git a/packages/electron/README.md b/packages/electron/README.md index 45722457..a2d4924e 100644 --- a/packages/electron/README.md +++ b/packages/electron/README.md @@ -67,7 +67,7 @@ That runs, in order: 2. `prepare:opencode-cli` to download/cache the pinned OpenCode CLI and copy it into `packages/electron/resources/opencode-cli`. 3. `bundle:main` to create `packages/electron/dist-bundle/main.mjs`. 4. `rebuild:native` to rebuild native modules for Electron. -5. `package.mjs` to run `electron-builder`; its `afterPack` hook stages the rebuilt `better-sqlite3` binary that Electron Builder's Bun dependency collector otherwise omits. +5. `package.mjs` to run `electron-builder`; its `afterPack` hook stages the compiled macOS icon asset catalog. Build output goes to `packages/electron/dist`. diff --git a/packages/electron/package.json b/packages/electron/package.json index 1ba7d13b..28755e6d 100644 --- a/packages/electron/package.json +++ b/packages/electron/package.json @@ -8,7 +8,6 @@ "main": "./dist-bundle/main.mjs", "dependencies": { "@openchamber/web": "workspace:*", - "better-sqlite3": "^12.10.0", "electron-context-menu": "^4.1.2", "electron-log": "^5.4.3", "electron-updater": "^6.8.3" diff --git a/packages/electron/scripts/after-pack.cjs b/packages/electron/scripts/after-pack.cjs index 542ea614..22b43ba3 100644 --- a/packages/electron/scripts/after-pack.cjs +++ b/packages/electron/scripts/after-pack.cjs @@ -5,23 +5,6 @@ module.exports = (context) => { const resourcesPath = context.electronPlatformName === 'darwin' ? path.join(context.appOutDir, `${context.packager.appInfo.productFilename}.app`, 'Contents', 'Resources') : path.join(context.appOutDir, 'resources'); - const betterSqliteDir = path.dirname(require.resolve('better-sqlite3/package.json')); - const betterSqliteBinary = path.join(betterSqliteDir, 'build', 'Release', 'better_sqlite3.node'); - if (!fs.existsSync(betterSqliteBinary)) { - throw new Error(`Missing rebuilt better-sqlite3 binary at ${betterSqliteBinary}`); - } - const packagedBetterSqliteBinary = path.join( - resourcesPath, - 'app.asar.unpacked', - 'node_modules', - 'better-sqlite3', - 'build', - 'Release', - 'better_sqlite3.node', - ); - fs.mkdirSync(path.dirname(packagedBetterSqliteBinary), { recursive: true }); - fs.copyFileSync(betterSqliteBinary, packagedBetterSqliteBinary); - if (context.electronPlatformName !== 'darwin') return; const sourceAssetsPath = path.join(__dirname, '..', 'resources', 'icons', 'Assets.car'); diff --git a/packages/electron/scripts/bundle-main.mjs b/packages/electron/scripts/bundle-main.mjs index 1c867134..5d925d9c 100644 --- a/packages/electron/scripts/bundle-main.mjs +++ b/packages/electron/scripts/bundle-main.mjs @@ -30,7 +30,6 @@ const result = await Bun.build({ '@openchamber/web/*', 'bun-pty', 'node-pty', - 'better-sqlite3', ], minify: false, sourcemap: 'none', diff --git a/packages/electron/scripts/rebuild-native.mjs b/packages/electron/scripts/rebuild-native.mjs index 47039cc5..f57ee368 100644 --- a/packages/electron/scripts/rebuild-native.mjs +++ b/packages/electron/scripts/rebuild-native.mjs @@ -133,19 +133,6 @@ const ensureWindowsNodeAddonApiForNodePty = async (rebuildRootPath) => { console.log(`[electron] rebuilding native modules against Electron ${electronVersion}...`); -await rebuild({ - buildPath: electronDir, - electronVersion, - force: true, - arch: targetArchitecture.electronBuilder, - onlyModules: ['better-sqlite3'], -}); -const betterSqliteDir = path.dirname(require.resolve('better-sqlite3/package.json')); -const betterSqliteBinary = path.join(betterSqliteDir, 'build', 'Release', 'better_sqlite3.node'); -if (!existsSync(betterSqliteBinary)) { - throw new Error(`better-sqlite3 rebuild did not produce ${betterSqliteBinary}`); -} - // Rebuild against the hoisted root node_modules (bun workspace layout). // force=true re-links regardless of cached state; prebuild-install lookup is // bypassed by @electron/rebuild in favor of direct node-gyp builds. diff --git a/packages/electron/scripts/verify-linux-appimage.mjs b/packages/electron/scripts/verify-linux-appimage.mjs index 97da9a05..697f7882 100644 --- a/packages/electron/scripts/verify-linux-appimage.mjs +++ b/packages/electron/scripts/verify-linux-appimage.mjs @@ -13,7 +13,7 @@ const ELF_MACHINE = { x64: 62, arm64: 183 }; // sherpa-onnx-node loads this Node-API addon from its platform-specific prebuilt // package in the separate server worker, so verify its architecture here rather // than Electron-rebuilding it with the source-built modules. -const REQUIRED_NATIVE_MODULES = ['better_sqlite3.node', 'pty.node', 'sherpa-onnx.node']; +const REQUIRED_NATIVE_MODULES = ['pty.node', 'sherpa-onnx.node']; /** electron-builder AppImage arch token: x64 → x86_64, arm64 → arm64 */ export const linuxAppImageArchSuffix = (architecture) => ( diff --git a/packages/electron/scripts/verify-linux-appimage.test.mjs b/packages/electron/scripts/verify-linux-appimage.test.mjs index 3ba2084d..7ed98c90 100644 --- a/packages/electron/scripts/verify-linux-appimage.test.mjs +++ b/packages/electron/scripts/verify-linux-appimage.test.mjs @@ -21,7 +21,7 @@ const createPayload = () => { ].join('\n')); writeElf(path.join(root, 'openchamber'), 'x64'); writeElf(path.join(root, 'resources/opencode-cli/opencode'), 'x64'); - for (const name of ['better_sqlite3.node', 'pty.node', 'sherpa-onnx.node']) { + for (const name of ['pty.node', 'sherpa-onnx.node']) { writeElf(path.join(root, 'resources/app.asar.unpacked/node_modules', name), 'x64'); } return root; @@ -53,7 +53,7 @@ test('verifies identity, version, and native payload architecture', () => { expectedOpenCodeVersion: '1.17.18', runCliVersion: () => '1.17.18', }); - assert.equal(result.nativeModuleCount, 3); + assert.equal(result.nativeModuleCount, 2); } finally { fs.rmSync(root, { recursive: true, force: true }); } diff --git a/packages/web/package.json b/packages/web/package.json index f5afcf43..86d3489b 100644 --- a/packages/web/package.json +++ b/packages/web/package.json @@ -28,7 +28,6 @@ "@opencode-ai/sdk": "1.18.11", "@simplewebauthn/server": "13.3.1", "adm-zip": "^0.5.16", - "better-sqlite3": "^12.10.0", "bun-pty": "^0.4.5", "compression": "^1.8.1", "cron-parser": "^4.9.0",