fix: resolve the account login shell and spawn terminals as login shells
Terminal tabs resolved the shell from the dsh process environment only, falling back to a hardcoded /bin/bash — service-managed launches without SHELL therefore ignored the account's login shell (e.g. zsh). They also spawned with an empty argv, so even a resolved zsh ran as a non-login shell and skipped ~/.zprofile. defaultShell() now falls back to the account's login shell (passwd) after $SHELL, and both registries spawn POSIX shells with -l so the tab behaves like a terminal emulator (loads ~/.profile / ~/.zprofile). Windows keeps powershell.exe with no login flag. Fixes omdsh-dev/DSH-better-sidebar#68
This commit is contained in:
+2
-2
@@ -13,7 +13,7 @@
|
||||
*/
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import * as nodePty from 'node-pty'
|
||||
import { ensureSpawnHelper } from './pty-manager.ts'
|
||||
import { ensureSpawnHelper, shellSpawnArgs } from './pty-manager.ts'
|
||||
import { SidebarError } from './wire.ts'
|
||||
|
||||
/** Per-agent-terminal transcript bound (bytes kept for replay and reads). */
|
||||
@@ -211,7 +211,7 @@ export class AgentPtyRegistry {
|
||||
): string {
|
||||
const uuid = randomUUID()
|
||||
const dims = clampDims(cols, rows)
|
||||
const pty = nodePty.spawn(this.shell, [], {
|
||||
const pty = nodePty.spawn(this.shell, shellSpawnArgs(), {
|
||||
name: 'xterm-256color',
|
||||
cols: dims.cols,
|
||||
rows: dims.rows,
|
||||
|
||||
+31
-4
@@ -9,6 +9,7 @@
|
||||
import { chmodSync, existsSync } from 'node:fs'
|
||||
import { dirname, join } from 'node:path'
|
||||
import { createRequire } from 'node:module'
|
||||
import { userInfo } from 'node:os'
|
||||
import * as nodePty from 'node-pty'
|
||||
import { SidebarError } from './wire.ts'
|
||||
|
||||
@@ -118,7 +119,7 @@ export class PtyManager {
|
||||
sessionId,
|
||||
tabId,
|
||||
cwd,
|
||||
pty: nodePty.spawn(this.shell, [], {
|
||||
pty: nodePty.spawn(this.shell, shellSpawnArgs(), {
|
||||
name: 'xterm-256color',
|
||||
cols: Math.max(2, Math.floor(cols)),
|
||||
rows: Math.max(2, Math.floor(rows)),
|
||||
@@ -191,9 +192,35 @@ export class PtyManager {
|
||||
}
|
||||
}
|
||||
|
||||
/** The interactive shell for this platform (empty SHELL falls back). */
|
||||
/**
|
||||
* The interactive shell for this platform, resolved like a terminal
|
||||
* emulator: an explicit `$SHELL` on the dsh process wins (deployment
|
||||
* override), then the account's login shell from passwd, then `/bin/bash`.
|
||||
* The passwd step matters because service managers and container inits
|
||||
* often start dsh without `SHELL`, and the tab should still open the
|
||||
* user's login shell (e.g. zsh) instead of silently degrading to bash.
|
||||
* Windows short-circuits to `powershell.exe` before any resolution.
|
||||
*/
|
||||
export function defaultShell(): string {
|
||||
if (process.platform === 'win32') return 'powershell.exe'
|
||||
const shell = process.env.SHELL
|
||||
return shell !== undefined && shell.trim() !== '' ? shell : '/bin/bash'
|
||||
const envShell = process.env.SHELL
|
||||
if (envShell !== undefined && envShell.trim() !== '') return envShell
|
||||
// userInfo() throws when the uid has no passwd entry (rare chroots);
|
||||
// without a login shell there is nothing better than the bash default.
|
||||
try {
|
||||
const loginShell = userInfo().shell
|
||||
if (typeof loginShell === 'string' && loginShell.trim() !== '') return loginShell
|
||||
} catch {
|
||||
// no passwd entry: fall through to /bin/bash
|
||||
}
|
||||
return '/bin/bash'
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn arguments that make the shell behave like a terminal-emulator tab:
|
||||
* POSIX shells start as login shells (`-l`) so they read the profile files
|
||||
* (`~/.profile`, `~/.zprofile`); Windows PowerShell takes no login flag.
|
||||
*/
|
||||
export function shellSpawnArgs(): string[] {
|
||||
return process.platform === 'win32' ? [] : ['-l']
|
||||
}
|
||||
|
||||
@@ -188,6 +188,34 @@ describe('host plugin smoke', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')('spawns the shell as a login shell (loads ~/.profile)', async () => {
|
||||
const home = mkdtempSync(join(tmpdir(), 'dsh-sidebar-login-'))
|
||||
const previousHome = process.env.HOME
|
||||
try {
|
||||
// A login bash reads ~/.profile (a non-login interactive bash reads
|
||||
// ~/.bashrc instead), so this marker proves the spawn used a login
|
||||
// argv — the terminal-emulator behavior the tab should match.
|
||||
writeFileSync(join(home, '.profile'), 'export DSH_LOGIN_MARKER=loaded-from-profile\n')
|
||||
process.env.HOME = home
|
||||
const manager = new PtyManager('/bin/bash', 3)
|
||||
try {
|
||||
const handle = manager.open('s5', 't1', process.cwd(), 80, 24)
|
||||
handle.pty.write('echo $DSH_LOGIN_MARKER\r')
|
||||
const deadline = Date.now() + 5000
|
||||
while (!handle.transcript.includes('loaded-from-profile') && Date.now() < deadline) {
|
||||
await new Promise(resolve => setTimeout(resolve, 50))
|
||||
}
|
||||
expect(handle.transcript).toContain('loaded-from-profile')
|
||||
} finally {
|
||||
manager.disposeAll()
|
||||
}
|
||||
} finally {
|
||||
if (previousHome === undefined) delete process.env.HOME
|
||||
else process.env.HOME = previousHome
|
||||
rmSync(home, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('lists the repository root level', async () => {
|
||||
const listing = await listDirectory(process.cwd(), 1000)
|
||||
expect(listing.entries.some(entry => entry.name === 'src' && entry.isDir)).toBe(true)
|
||||
|
||||
+26
-5
@@ -1,4 +1,17 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
// Pin the passwd login shell to a non-bash value for the pty helpers tests:
|
||||
// the CI runner's own login shell is /bin/bash, which the old hardcoded
|
||||
// fallback would have satisfied by coincidence, so the fallback chain needs
|
||||
// a mock passwd to be genuinely discriminating. No other test in this file
|
||||
// reads os.userInfo.
|
||||
vi.mock('node:os', async (importOriginal) => {
|
||||
const os = await importOriginal<typeof import('node:os')>()
|
||||
return {
|
||||
...os,
|
||||
userInfo: () => ({ ...os.userInfo(), shell: '/usr/bin/zsh' }),
|
||||
}
|
||||
})
|
||||
import { compareEntries, isWithin, parentOf, rootLabel, requireAbsolute } from '../src/fs-tree.ts'
|
||||
import { parseLogLines, parsePorcelainZ } from '../src/git.ts'
|
||||
import { parseUnifiedDiff } from '../src/client/DiffView.tsx'
|
||||
@@ -846,13 +859,21 @@ describe('agent terminal reconciliation', () => {
|
||||
})
|
||||
|
||||
describe('pty helpers', () => {
|
||||
it('falls back from an empty SHELL to a usable shell', () => {
|
||||
it('prefers an explicit SHELL, then falls back to the account login shell', () => {
|
||||
if (process.platform === 'win32') {
|
||||
// defaultShell() short-circuits to powershell.exe before env/passwd
|
||||
// resolution on Windows; the POSIX fallback chain is asserted below.
|
||||
expect(defaultShell()).toBe('powershell.exe')
|
||||
return
|
||||
}
|
||||
const previous = process.env.SHELL
|
||||
try {
|
||||
process.env.SHELL = ''
|
||||
expect(defaultShell()).toBe('/bin/bash')
|
||||
process.env.SHELL = '/explicit/zsh'
|
||||
expect(defaultShell()).toBe('/explicit/zsh')
|
||||
process.env.SHELL = ' '
|
||||
expect(defaultShell()).toBe('/usr/bin/zsh')
|
||||
delete process.env.SHELL
|
||||
expect(defaultShell()).toBe('/bin/bash')
|
||||
expect(defaultShell()).toBe('/usr/bin/zsh')
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.SHELL
|
||||
else process.env.SHELL = previous
|
||||
|
||||
Reference in New Issue
Block a user