fix: batch of review findings — dead install button, history pollution, TUI completion interference, ssh split session kill, scrollback live apply, zmodem second transfer, sftp shell quoting, replay buffer leak, release guards

This commit is contained in:
Bill committed 2026-09-14 02:20:40 +08:00
1 parent e6fc021903
commit 6c100542c5
11 files changed
+130 -19

No files matched your search

+37 -6
View File
@@ -31,6 +31,22 @@ const files = [
for (const f of files) { for (const f of files) {
if (!fs.existsSync(f)) { console.error('missing build artifact:', f); process.exit(1) } if (!fs.existsSync(f)) { console.error('missing build artifact:', f); process.exit(1) }
} }
// Guard rails: the artifacts must belong to the version being published, and
// the tag must not exist yet — otherwise a re-run silently republishes a
// *different* binary under an already-released version number, and installed
// clients never see it (their version check compares numbers, not hashes).
const pkgVersion = JSON.parse(fs.readFileSync(path.join(ROOT, 'package.json'), 'utf8')).version
if (pkgVersion !== V) {
console.error(`package.json is ${pkgVersion} but you asked to publish ${V}`)
process.exit(1)
}
const ymlPath = path.join(R, 'latest.yml')
if (!fs.existsSync(ymlPath)) { console.error('missing release/latest.yml — run npm run dist first'); process.exit(1) }
const ymlVersion = fs.readFileSync(ymlPath, 'utf8').match(/^version:\s*(\S+)/m)?.[1]
if (ymlVersion !== V) {
console.error(`release/latest.yml says ${ymlVersion} but you asked to publish ${V} — rebuild first`)
process.exit(1)
}
const proxy = process.env.HTTPS_PROXY || process.env.https_proxy || env.HTTPS_PROXY || env.PROXY const proxy = process.env.HTTPS_PROXY || process.env.https_proxy || env.HTTPS_PROXY || env.PROXY
if (proxy) { if (proxy) {
@@ -53,7 +69,10 @@ async function upload(url, file, token, authScheme) {
duplex: 'half' duplex: 'half'
}) })
console.log(` upload ${name}: HTTP ${resp.status}`) console.log(` upload ${name}: HTTP ${resp.status}`)
if (!resp.ok) console.log(' ', (await resp.text()).slice(0, 300)) if (!resp.ok) {
const body = (await resp.text()).slice(0, 300)
throw new Error(`upload failed (${resp.status}) for ${name}: ${body}`)
}
} }
async function gitea() { async function gitea() {
@@ -64,7 +83,7 @@ async function gitea() {
headers: { Authorization: `token ${env.GIT_TOKEN}`, 'Content-Type': 'application/json' }, headers: { Authorization: `token ${env.GIT_TOKEN}`, 'Content-Type': 'application/json' },
body: JSON.stringify({ tag_name: `v${V}`, name: `OpenTerminal v${V}`, body: notes, draft: false, prerelease: false }) body: JSON.stringify({ tag_name: `v${V}`, name: `OpenTerminal v${V}`, body: notes, draft: false, prerelease: false })
}) })
if (!resp.ok) { console.log('create failed:', resp.status, (await resp.text()).slice(0, 300)); return } if (!resp.ok) { throw new Error(`Gitea release create failed: ${resp.status} ${(await resp.text()).slice(0, 300)}`) }
const rel = await resp.json() const rel = await resp.json()
console.log('release created, id =', rel.id) console.log('release created, id =', rel.id)
for (const f of files) await upload(`${base}/releases/${rel.id}/assets`, f, env.GIT_TOKEN, 'token') for (const f of files) await upload(`${base}/releases/${rel.id}/assets`, f, env.GIT_TOKEN, 'token')
@@ -73,8 +92,6 @@ async function gitea() {
async function giteaChannel() { async function giteaChannel() {
console.log('=== Gitea update channel ===') console.log('=== Gitea update channel ===')
const base = 'https://git.codingplan.site/api/packages/admin/generic/openterminal-update/stable' const base = 'https://git.codingplan.site/api/packages/admin/generic/openterminal-update/stable'
const del = await fetch(base, { method: 'DELETE', headers: { Authorization: `token ${env.GIT_TOKEN}` } })
console.log(' delete old stable:', del.status)
const channelFiles = [ const channelFiles = [
[path.join(R, 'latest.yml'), 'latest.yml'], [path.join(R, 'latest.yml'), 'latest.yml'],
[path.join(R, `OpenTerminal-${V}-setup.exe.blockmap`), `OpenTerminal-${V}-setup.exe.blockmap`], [path.join(R, `OpenTerminal-${V}-setup.exe.blockmap`), `OpenTerminal-${V}-setup.exe.blockmap`],
@@ -83,7 +100,20 @@ async function giteaChannel() {
// releases API 404s), but the generic package is publicly readable. // releases API 404s), but the generic package is publicly readable.
[path.join(ROOT, 'RELEASE_NOTES.md'), 'release-notes.md'] [path.join(ROOT, 'RELEASE_NOTES.md'), 'release-notes.md']
] ]
for (const [f, name] of channelFiles) { // Validate before touching the channel: a missing file after the DELETE would
// leave the update channel empty (clients then fall back to GitHub, which is
// unreachable in China for most users).
for (const [f] of channelFiles) {
if (!fs.existsSync(f)) throw new Error(`missing channel file: ${f}`)
}
// latest.yml goes last: it is what makes clients start downloading, so the
// payload must already be in place.
const isLatest = ([f]) => path.basename(f) === 'latest.yml'
const ordered = [...channelFiles.filter((e) => !isLatest(e)), ...channelFiles.filter(isLatest)]
// Replace the version only once every file is known to be present on disk.
const del = await fetch(base, { method: 'DELETE', headers: { Authorization: `token ${env.GIT_TOKEN}` } })
console.log(' delete old stable:', del.status)
for (const [f, name] of ordered) {
const stat = fs.statSync(f) const stat = fs.statSync(f)
const resp = await fetch(`${base}/${encodeURIComponent(name)}`, { const resp = await fetch(`${base}/${encodeURIComponent(name)}`, {
method: 'PUT', method: 'PUT',
@@ -92,6 +122,7 @@ async function giteaChannel() {
duplex: 'half' duplex: 'half'
}) })
console.log(` put ${name}: HTTP ${resp.status}`) console.log(` put ${name}: HTTP ${resp.status}`)
if (!resp.ok) throw new Error(`channel put failed (${resp.status}) for ${name}: ${(await resp.text()).slice(0, 200)}`)
} }
} }
@@ -107,7 +138,7 @@ async function github() {
}, },
body: JSON.stringify({ tag_name: `v${V}`, name: `OpenTerminal v${V}`, body: notes, draft: false, prerelease: false }) body: JSON.stringify({ tag_name: `v${V}`, name: `OpenTerminal v${V}`, body: notes, draft: false, prerelease: false })
}) })
if (!resp.ok) { console.log('create failed:', resp.status, (await resp.text()).slice(0, 300)); return } if (!resp.ok) { throw new Error(`GitHub release create failed: ${resp.status} ${(await resp.text()).slice(0, 300)}`) }
const rel = await resp.json() const rel = await resp.json()
console.log('release created, id =', rel.id) console.log('release created, id =', rel.id)
const up = `https://uploads.github.com/repos/billowliu2/OpenTerminal/releases/${rel.id}/assets` const up = `https://uploads.github.com/repos/billowliu2/OpenTerminal/releases/${rel.id}/assets`
+4
View File
@@ -173,6 +173,9 @@ export function createPty(opts: PtyCreateOptions = {}): PtyCreateResult {
pty.onExit(({ exitCode }) => { pty.onExit(({ exitCode }) => {
try { try {
sessions.delete(id) sessions.delete(id)
// The session is gone: drop its replay buffer too (killPty was the only
// path that did, so naturally-exiting shells leaked up to 64KB each).
replayBuffers.delete(id)
safeStopLog(id) safeStopLog(id)
broadcast(Ipc.PTY_EXIT, { id, exitCode }) broadcast(Ipc.PTY_EXIT, { id, exitCode })
} catch { } catch {
@@ -258,6 +261,7 @@ export async function openSession(opts: SessionOpenOptions): Promise<{ id: strin
detachZmodem(handle.id) detachZmodem(handle.id)
safeStopLog(handle.id) safeStopLog(handle.id)
sessions.delete(handle.id) sessions.delete(handle.id)
replayBuffers.delete(handle.id)
deps.broadcast(Ipc.PTY_EXIT, { id: handle.id, exitCode: 0 }) deps.broadcast(Ipc.PTY_EXIT, { id: handle.id, exitCode: 0 })
} catch { } catch {
// never crash the event loop // never crash the event loop
+7 -3
View File
@@ -21,7 +21,9 @@ const DEFAULT_SYSTEM: SystemSettings = {
launchAtLogin: false, launchAtLogin: false,
preventSleep: false, preventSleep: false,
globalShowHide: '', globalShowHide: '',
closeAction: 'ask', // Must match DEFAULT_SETTINGS.system in @shared/settings — an "ask" here made
// a fresh install prompt on close while the docs and UI promised tray.
closeAction: 'tray',
autoCheckUpdate: true autoCheckUpdate: true
} }
@@ -43,9 +45,11 @@ function isHighlightRule(value: unknown): value is HighlightRule {
} }
function sanitizeRules(value: unknown): HighlightRule[] { function sanitizeRules(value: unknown): HighlightRule[] {
// An explicit empty array is a valid choice ("no highlighting"); only
// malformed data falls back to the built-in rules. Returning the defaults for
// [] made deleting the last rule look like it silently failed.
if (!Array.isArray(value)) return DEFAULT_HIGHLIGHT_RULES if (!Array.isArray(value)) return DEFAULT_HIGHLIGHT_RULES
const rules = value.filter(isHighlightRule) return value.filter(isHighlightRule)
return rules.length > 0 ? rules : DEFAULT_HIGHLIGHT_RULES
} }
function deepMerge(raw: unknown): { settings: AppSettings; errors: string[] } { function deepMerge(raw: unknown): { settings: AppSettings; errors: string[] } {
+12 -2
View File
@@ -195,13 +195,23 @@ export function deleteRemote(sessionId: string, paths: string[]): Promise<void>
* chmod/chown run over an exec channel: the JD test server's sftp subsystem * chmod/chown run over an exec channel: the JD test server's sftp subsystem
* accepts SETSTAT but silently ignores it, while shell chmod/chown work. * accepts SETSTAT but silently ignores it, while shell chmod/chown work.
*/ */
/**
* POSIX single-quote escaping. `JSON.stringify` only escapes `"`, so a remote
* file name containing `$`, a backtick or a quote would still be expanded by the
* far-side shell — that is remote command execution triggered by a file name.
*/
function shQuote(value: string): string {
return `'${value.replace(/'/g, `'\\''`)}'`
}
export function chmodRemote(sessionId: string, path: string, mode: string): Promise<void> { export function chmodRemote(sessionId: string, path: string, mode: string): Promise<void> {
if (!/^[0-7]{1,4}$/.test(mode)) throw new Error(`非法权限值: ${mode}`) if (!/^[0-7]{1,4}$/.test(mode)) throw new Error(`非法权限值: ${mode}`)
return execQuiet(sessionId, `chmod ${mode} ${JSON.stringify(path)}`) return execQuiet(sessionId, `chmod ${mode} ${shQuote(path)}`)
} }
export function chownRemote(sessionId: string, path: string, uid: number, gid: number): Promise<void> { export function chownRemote(sessionId: string, path: string, uid: number, gid: number): Promise<void> {
return execQuiet(sessionId, `chown ${uid}:${gid} ${JSON.stringify(path)}`) if (!Number.isInteger(uid) || !Number.isInteger(gid)) throw new Error('非法 uid/gid')
return execQuiet(sessionId, `chown ${uid}:${gid} ${shQuote(path)}`)
} }
/** Run a command on the session's shell channel and wait for it to finish. */ /** Run a command on the session's shell channel and wait for it to finish. */
+11
View File
@@ -309,6 +309,17 @@ export function attachZmodem(sessionId: string, deps: ZmodemDeps): void {
engine.mode = role === 'receive' ? 'receive' : 'send' engine.mode = role === 'receive' ? 'receive' : 'send'
engine.detection = detection engine.detection = detection
engine.active = true engine.active = true
// A detect is the start of a fresh transfer on this session, so the
// one-shot flags of the previous one must go with it. Leaving `confirmed`
// set made the *second* `sz`/`rz` on a session a no-op: respondZmodem
// bailed out, the engine stayed `active` (swallowing every keystroke and
// all terminal data) until the stall timer finally fired.
engine.confirmed = false
engine.progressEmitted = false
engine.doneEmitted = false
engine.dir = null
engine.session = null
engine.receiveStream = null
engine.transferId = `zm-${sessionId}` engine.transferId = `zm-${sessionId}`
emitProgress(engine, { file: '', bytes: 0, totalBytes: 0 }) emitProgress(engine, { file: '', bytes: 0, totalBytes: 0 })
try { try {
+1 -1
View File
@@ -121,7 +121,7 @@ const api: AppApi = {
updateCheck: () => ipcRenderer.invoke(Ipc.UPDATE_CHECK), updateCheck: () => ipcRenderer.invoke(Ipc.UPDATE_CHECK),
updateDownload: () => ipcRenderer.invoke(Ipc.UPDATE_DOWNLOAD), updateDownload: () => ipcRenderer.invoke(Ipc.UPDATE_DOWNLOAD),
updateInstall: () => ipcRenderer.send(Ipc.UPDATE_INSTALL), updateInstall: () => ipcRenderer.invoke(Ipc.UPDATE_INSTALL),
updateChangelog: () => ipcRenderer.invoke(Ipc.UPDATE_CHANGELOG), updateChangelog: () => ipcRenderer.invoke(Ipc.UPDATE_CHANGELOG),
onUpdateState: (cb: (s: UpdateState) => void) => { onUpdateState: (cb: (s: UpdateState) => void) => {
const listener = (_: unknown, s: UpdateState): void => cb(s) const listener = (_: unknown, s: UpdateState): void => cb(s)
+1 -1
View File
@@ -89,7 +89,7 @@ if (typeof window !== 'undefined' && !window.api) {
deleteLayout: async () => undefined, deleteLayout: async () => undefined,
updateCheck: async () => ({ status: 'dev' as const, currentVersion: 'dev' }), updateCheck: async () => ({ status: 'dev' as const, currentVersion: 'dev' }),
updateDownload: async () => undefined, updateDownload: async () => undefined,
updateInstall: noop, updateInstall: async () => undefined,
updateChangelog: async () => [], updateChangelog: async () => [],
onUpdateState: () => noop onUpdateState: () => noop
} }
+1 -1
View File
@@ -67,7 +67,7 @@ export function AboutTab(): React.JSX.Element {
</Button> </Button>
)} )}
{status === 'downloaded' && ( {status === 'downloaded' && (
<Button type="primary" onClick={() => window.api.updateInstall()}> <Button type="primary" onClick={() => void window.api.updateInstall()}>
重启安装 重启安装
</Button> </Button>
)} )}
+24 -2
View File
@@ -154,7 +154,7 @@ function stepLineBuffer(
// clear-line) only removes buffer content; never appends. // clear-line) only removes buffer content; never appends.
const first = data[0] const first = data[0]
if (b === '' && first === '\x1b') return { line: '' } if (b === '' && first === '\x1b') return { line: '' }
if (first === '\r') return submitOr(b) if (first === '\r') return submitOr(outBuf, b)
if (first === '\x7f') { if (first === '\x7f') {
outBuf.current = b.slice(0, -1) outBuf.current = b.slice(0, -1)
return { line: outBuf.current } return { line: outBuf.current }
@@ -168,8 +168,12 @@ function stepLineBuffer(
return { line: outBuf.current } return { line: outBuf.current }
} }
function submitOr(b: string): { line: string; submit?: string } { function submitOr(outBuf: React.MutableRefObject<string>, b: string): { line: string; submit?: string } {
const t = b.trim() const t = b.trim()
// Commit the line out of the buffer: the shell has it now, and anything typed
// next belongs to a fresh line. Leaving it in made every following command
// accumulate onto the previous one (`ls` + `pwd` → `lspwd` in the history).
outBuf.current = ''
return t ? { line: '', submit: t } : { line: '', submit: '' } return t ? { line: '', submit: t } : { line: '', submit: '' }
} }
@@ -684,6 +688,21 @@ export const TerminalView: ForwardRefExoticComponent<TerminalViewProps & { ref?:
// now so this redraw chunk replaces (not appends to) the line buffer. // now so this redraw chunk replaces (not appends to) the line buffer.
const consumeRewrite = diffRewriteRef.current const consumeRewrite = diffRewriteRef.current
diffRewriteRef.current = false diffRewriteRef.current = false
// Full-screen TUIs (vim, Claude Code, …) own the alternate buffer, their
// own key handling and their own cursor: the line buffer, the command
// history and the completion popup have no business there. Interfering
// swallowed ↑/↓/Tab/Esc (Esc could not even leave vim's insert mode) and
// wrote TUI keystrokes into the command history.
if (term.buffer.active.type === 'alternate') {
lineBufRef.current = ''
if (suggestionsRef.current.length > 0) {
suggestionsRef.current = []
selIndexRef.current = 0
setSuggestions([])
setSuggestionIndex(0)
}
return
}
// M5: line capture → recordCommand + inline completion overlay. // M5: line capture → recordCommand + inline completion overlay.
const step = stepLineBuffer(lineBufRef, data, consumeRewrite) const step = stepLineBuffer(lineBufRef, data, consumeRewrite)
if (typeof step.submit === 'string' && step.submit) { if (typeof step.submit === 'string' && step.submit) {
@@ -810,6 +829,9 @@ export const TerminalView: ForwardRefExoticComponent<TerminalViewProps & { ref?:
term.options.cursorBlink = tSettings.cursorBlink term.options.cursorBlink = tSettings.cursorBlink
term.options.cursorStyle = tSettings.cursorStyle term.options.cursorStyle = tSettings.cursorStyle
term.options.cursorInactiveStyle = tSettings.cursorInactiveStyle term.options.cursorInactiveStyle = tSettings.cursorInactiveStyle
// Live-appliable in xterm 6: without this the setting only took effect for
// terminals opened after the change, which reads as "the setting is broken".
term.options.scrollback = tSettings.scrollback
term.options.theme = withChromeColors(getThemeById(tSettings.themeId, settings.customThemes).colors as ITheme) term.options.theme = withChromeColors(getThemeById(tSettings.themeId, settings.customThemes).colors as ITheme)
scheduleFit() scheduleFit()
}, [tSettings, settings.customThemes, scheduleFit]) }, [tSettings, settings.customThemes, scheduleFit])
+31 -2
View File
@@ -243,9 +243,35 @@ export default function Workspace({ onOpenSettings }: WorkspaceProps): React.JSX
if (mode === 'terminal') terminalApiRef.current = api if (mode === 'terminal') terminalApiRef.current = api
else sshApiRef.current = api else sshApiRef.current = api
// One pty/ssh session can back several panels — an SSH split mirrors a
// single session on purpose. So closing a panel must not kill a session
// another panel is still showing; count the panels per session (seeding
// from the panels already present, e.g. a restored layout) and kill only
// when the last one goes away.
const useCount = new Map<string, number>()
for (const existing of api.panels) {
const sid = sessionIdOf(existing)
if (sid) useCount.set(sid, (useCount.get(sid) ?? 0) + 1)
}
const countPanel = (panel: IDockviewPanel): void => {
const sid = sessionIdOf(panel)
if (sid) useCount.set(sid, (useCount.get(sid) ?? 0) + 1)
}
const releaseSession = (panel: IDockviewPanel): void => {
const sid = sessionIdOf(panel)
if (!sid) return
const left = (useCount.get(sid) ?? 1) - 1
if (left > 0) {
useCount.set(sid, left)
return
}
useCount.delete(sid)
killSession(sid)
}
const cleanups = [ const cleanups = [
api.onDidRemovePanel((panel: IDockviewPanel) => { api.onDidRemovePanel((panel: IDockviewPanel) => {
killSession(sessionIdOf(panel)) releaseSession(panel)
recomputeLocalPanels() recomputeLocalPanels()
recountAll() recountAll()
}), }),
@@ -261,7 +287,10 @@ export default function Workspace({ onOpenSettings }: WorkspaceProps): React.JSX
recomputeLocalPanels() recomputeLocalPanels()
recountAll() recountAll()
}), }),
api.onDidAddPanel(() => recountAll()) api.onDidAddPanel((panel: IDockviewPanel) => {
countPanel(panel)
recountAll()
})
] ]
disposablesRef.current.push(...cleanups) disposablesRef.current.push(...cleanups)
}, },
+1 -1
View File
@@ -110,7 +110,7 @@ export interface AppApi {
// ---- updater (domestic feed first, GitHub fallback) ---- // ---- updater (domestic feed first, GitHub fallback) ----
updateCheck(): Promise<UpdateState> updateCheck(): Promise<UpdateState>
updateDownload(): Promise<void> updateDownload(): Promise<void>
updateInstall(): void updateInstall(): Promise<void>
updateChangelog(): Promise<ReleaseNote[]> updateChangelog(): Promise<ReleaseNote[]>
onUpdateState(cb: (s: UpdateState) => void): () => void onUpdateState(cb: (s: UpdateState) => void): () => void
} }