Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions js/.changeset/detached-status-killed-record.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'start-command': patch
---

Fix `--status` for detached executions resurrecting a completed (killed) record. `enrichDetachedStatus()` no longer flips an already-`executed` record back to `executing` (and nulls its exit code) just because `screen -ls`/`tmux`/`docker` still lists a same-named session — a lingering shell can outlive a SIGKILLed command (e.g. OOM, exit 137). The recorded exit code and the `Exit Code:` log footer that `start` itself writes are now treated as authoritative; the record only flips to `executing` when there is no recorded exit code and no terminal footer in the log.
20 changes: 16 additions & 4 deletions js/src/lib/status-formatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,22 @@ function enrichDetachedStatus(record) {
Object.assign(enriched, record);

if (alive && enriched.status === 'executed') {
// Session still running but record says executed - correct it
enriched.status = 'executing';
enriched.exitCode = null;
enriched.endTime = null;
// A live `screen -ls` (or `tmux`/`docker`) session does NOT mean the command
// is still running: a lingering shell can outlive a killed command (e.g. the
// OOM killer sends SIGKILL, exit 137, but the login shell stays up for a
// window after `start` already wrote the terminal footer). The footer/recorded
// exit code is authoritative. Only flip back to 'executing' when there is NO
// recorded terminal exit code AND no `Exit Code:` footer in the log.
const footerExit = readExitCodeFromLog(enriched.logPath);
const hasRecordedExit =
enriched.exitCode !== null && enriched.exitCode !== undefined;
if (!hasRecordedExit && footerExit === null) {
// Session still running and no terminal record - correct it
enriched.status = 'executing';
enriched.exitCode = null;
enriched.endTime = null;
}
// Otherwise keep the recorded/footer exit code - the command has finished.
} else if (!alive && enriched.status === 'executing') {
// Session ended but record says executing - correct it
enriched.status = 'executed';
Expand Down
120 changes: 120 additions & 0 deletions js/test/session-name-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,126 @@ describe('Issue #101: Detached status enrichment', () => {
expect(enriched.endTime).not.toBeNull();
}
});

// Issue #134: a lingering screen session must NOT resurrect a completed
// (killed, exit 137) record back to 'executing' / null exit code.
describe('Issue #134: completed record with a lingering live session', () => {
const screenAvailable = (() => {
const probe = spawnSync('screen', ['-v'], { encoding: 'utf8' });
return probe.status === 0 || /Screen version/.test(probe.stdout || '');
})();

let sessionName;
let logPath;

beforeEach(() => {
if (!screenAvailable) {
return;
}
sessionName = `enrich-134-${process.pid}-${Date.now()}`;
logPath = path.join(TEST_APP_FOLDER, `${sessionName}.log`);
if (!fs.existsSync(TEST_APP_FOLDER)) {
fs.mkdirSync(TEST_APP_FOLDER, { recursive: true });
}
// Footer exactly as `start` writes it for a SIGKILLed command.
fs.writeFileSync(
logPath,
`Killed\n\n${'='.repeat(50)}\nFinished: 2026-06-14 19:10:49.822\nExit Code: 137\n`
);
// A shell that outlives the (already-finished) command.
spawnSync('screen', ['-dmS', sessionName, 'sh', '-c', 'sleep 30'], {
encoding: 'utf8',
});
});

afterEach(() => {
if (!screenAvailable) {
return;
}
spawnSync('screen', ['-S', sessionName, '-X', 'quit'], {
stdio: 'ignore',
});
if (logPath && fs.existsSync(logPath)) {
fs.unlinkSync(logPath);
}
});

it('keeps the recorded exit code when the session is still listed', () => {
if (!screenAvailable) {
return;
}
const record = new ExecutionRecord({
command: 'sleep 60',
logPath,
options: {
sessionName,
isolated: 'screen',
isolationMode: 'detached',
},
});
record.complete(137);

// Sanity: the session must actually be alive for this test to be meaningful.
expect(isDetachedSessionAlive(record)).toBe(true);

const enriched = enrichDetachedStatus(record);
expect(enriched.status).toBe('executed');
expect(enriched.exitCode).toBe(137);
expect(enriched.endTime).not.toBeNull();
});

it('honors the log footer exit code even without a recorded exit code', () => {
if (!screenAvailable) {
return;
}
const record = new ExecutionRecord({
command: 'sleep 60',
logPath,
options: {
sessionName,
isolated: 'screen',
isolationMode: 'detached',
},
});
// Record was never reconciled: status 'executed' but exitCode still null.
record.status = 'executed';
record.exitCode = null;
record.endTime = null;

expect(isDetachedSessionAlive(record)).toBe(true);

const enriched = enrichDetachedStatus(record);
// Footer says 137, so it must stay finished, not flip to executing.
expect(enriched.status).toBe('executed');
});

it('flips to executing only when there is no terminal record at all', () => {
if (!screenAvailable) {
return;
}
// Log with NO Exit Code footer and no recorded exit code.
fs.writeFileSync(logPath, 'still running, no footer yet\n');
const record = new ExecutionRecord({
command: 'sleep 60',
logPath,
options: {
sessionName,
isolated: 'screen',
isolationMode: 'detached',
},
});
record.status = 'executed';
record.exitCode = null;
record.endTime = null;

expect(isDetachedSessionAlive(record)).toBe(true);

const enriched = enrichDetachedStatus(record);
expect(enriched.status).toBe('executing');
expect(enriched.exitCode).toBeNull();
expect(enriched.endTime).toBeNull();
});
});
});
});

Expand Down
5 changes: 5 additions & 0 deletions rust/changelog.d/135.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
bump: patch
---

Fixed detached `--status` resurrecting a killed (exit 137) record back to `executing`. The `alive && executed` branch in `enrich_detached_status` now consults the recorded exit code and the `Exit Code:` log footer before flipping, so a lingering shell that outlives a `SIGKILL`ed command no longer reports a completed command as still running.
18 changes: 14 additions & 4 deletions rust/src/lib/status_formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,20 @@ pub fn enrich_detached_status(record: &ExecutionRecord) -> ExecutionRecord {
let mut enriched = record.clone();

if alive && enriched.status == ExecutionStatus::Executed {
// Session still running but record says executed - correct it
enriched.status = ExecutionStatus::Executing;
enriched.exit_code = None;
enriched.end_time = None;
// A live `screen -ls` (or `tmux`/`docker`) session does NOT mean the command
// is still running: a lingering shell can outlive a killed command (e.g. the
// OOM killer sends SIGKILL, exit 137, but the login shell stays up for a
// window after `start` already wrote the terminal footer). The footer/recorded
// exit code is authoritative. Only flip back to "executing" when there is NO
// recorded terminal exit code AND no `Exit Code:` footer in the log.
let footer_exit = read_exit_code_from_log(&enriched.log_path);
if enriched.exit_code.is_none() && footer_exit.is_none() {
// Session still running and no terminal record - correct it
enriched.status = ExecutionStatus::Executing;
enriched.exit_code = None;
enriched.end_time = None;
}
// Otherwise keep the recorded/footer exit code - the command has finished.
} else if !alive && enriched.status == ExecutionStatus::Executing {
// Session ended but record says executing - correct it
enriched.status = ExecutionStatus::Executed;
Expand Down
150 changes: 150 additions & 0 deletions rust/tests/session_name_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,156 @@ fn test_enrich_detached_status_marks_dead_session_as_executed() {
}
}

// ===== Issue #134: lingering live session must not resurrect a completed record =====

/// Probe whether `screen` is usable in this environment.
fn screen_available() -> bool {
std::process::Command::new("screen")
.arg("-v")
.output()
.map(|o| {
o.status.success() || String::from_utf8_lossy(&o.stdout).contains("Screen version")
})
.unwrap_or(false)
}

/// Start a detached screen session that outlives the (already-finished) command.
/// Returns the session name. Caller must quit it.
fn start_lingering_screen(session_name: &str) {
let _ = std::process::Command::new("screen")
.args(["-dmS", session_name, "sh", "-c", "sleep 30"])
.output();
}

/// Whether the lingering session is observable as alive in this environment.
///
/// Some instrumented environments (notably `cargo tarpaulin`, which traces
/// every fork via ptrace) disrupt the `screen -dmS` daemon fork, so the
/// session never registers in `screen -ls`. There the sanity precondition for
/// these tests cannot hold, so they skip rather than fail.
fn session_observably_alive(record: &ExecutionRecord) -> bool {
is_detached_session_alive(record) == Some(true)
}

fn quit_screen(session_name: &str) {
let _ = std::process::Command::new("screen")
.args(["-S", session_name, "-X", "quit"])
.output();
}

#[test]
fn test_enrich_keeps_recorded_exit_code_when_session_lingers() {
if !screen_available() {
return;
}
let temp_dir = TempDir::new().unwrap();
let session_name = format!("enrich-134-recorded-{}", std::process::id());
let log_path = temp_dir.path().join(format!("{session_name}.log"));
// Footer exactly as `start` writes it for a SIGKILLed command.
std::fs::write(
&log_path,
"Killed\n\n==================================================\nFinished: 2026-06-14 19:10:49.822\nExit Code: 137\n",
)
.unwrap();

start_lingering_screen(&session_name);

let mut record = ExecutionRecord::with_options(ExecutionRecordOptions {
command: "sleep 60".to_string(),
log_path: Some(log_path.to_string_lossy().to_string()),
options: Some(make_isolation_options(&session_name, "screen", "detached")),
..Default::default()
});
record.complete(137);

// Sanity: the session must actually be alive for this test to be meaningful.
if !session_observably_alive(&record) {
quit_screen(&session_name);
return;
}

let enriched = enrich_detached_status(&record);
quit_screen(&session_name);

assert_eq!(enriched.status, ExecutionStatus::Executed);
assert_eq!(enriched.exit_code, Some(137));
assert!(enriched.end_time.is_some());
}

#[test]
fn test_enrich_honors_log_footer_when_no_recorded_exit_code() {
if !screen_available() {
return;
}
let temp_dir = TempDir::new().unwrap();
let session_name = format!("enrich-134-footer-{}", std::process::id());
let log_path = temp_dir.path().join(format!("{session_name}.log"));
std::fs::write(
&log_path,
"Killed\n\n==================================================\nFinished: 2026-06-14 19:10:49.822\nExit Code: 137\n",
)
.unwrap();

start_lingering_screen(&session_name);

// Status 'executed' but exit_code never recorded; the footer is authoritative.
let mut record = ExecutionRecord::with_options(ExecutionRecordOptions {
command: "sleep 60".to_string(),
status: Some(ExecutionStatus::Executed),
log_path: Some(log_path.to_string_lossy().to_string()),
options: Some(make_isolation_options(&session_name, "screen", "detached")),
..Default::default()
});
record.exit_code = None;
record.end_time = None;

if !session_observably_alive(&record) {
quit_screen(&session_name);
return;
}

let enriched = enrich_detached_status(&record);
quit_screen(&session_name);

assert_eq!(enriched.status, ExecutionStatus::Executed);
}

#[test]
fn test_enrich_flips_to_executing_when_no_terminal_record() {
if !screen_available() {
return;
}
let temp_dir = TempDir::new().unwrap();
let session_name = format!("enrich-134-nofooter-{}", std::process::id());
let log_path = temp_dir.path().join(format!("{session_name}.log"));
// Log with NO Exit Code footer.
std::fs::write(&log_path, "still running, no footer yet\n").unwrap();

start_lingering_screen(&session_name);

let mut record = ExecutionRecord::with_options(ExecutionRecordOptions {
command: "sleep 60".to_string(),
status: Some(ExecutionStatus::Executed),
log_path: Some(log_path.to_string_lossy().to_string()),
options: Some(make_isolation_options(&session_name, "screen", "detached")),
..Default::default()
});
record.exit_code = None;
record.end_time = None;

if !session_observably_alive(&record) {
quit_screen(&session_name);
return;
}

let enriched = enrich_detached_status(&record);
quit_screen(&session_name);

assert_eq!(enriched.status, ExecutionStatus::Executing);
assert_eq!(enriched.exit_code, None);
assert!(enriched.end_time.is_none());
}

#[test]
fn test_get_most_recent_session_name_match() {
let (_temp_dir, store) = create_test_store();
Expand Down
Loading